diff --git a/.gitignore b/.gitignore index dcb5c76b8..9ea638117 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ target/ # Ledger test-ledger/ test-ledger-magicblock/ +magicblock-test-storage/ # Mac **/.DS_Store diff --git a/magicblock-account-cloner/src/bpf_loader_v1.rs b/magicblock-account-cloner/src/bpf_loader_v1.rs index a0ba1de66..4340df0fa 100644 --- a/magicblock-account-cloner/src/bpf_loader_v1.rs +++ b/magicblock-account-cloner/src/bpf_loader_v1.rs @@ -51,6 +51,7 @@ impl BpfUpgradableProgramModifications { rent_epoch: Some(u64::MAX), delegated: Some(false), confined: Some(false), + remote_slot: Some(loaded_program.remote_slot), } }; @@ -71,6 +72,7 @@ impl BpfUpgradableProgramModifications { rent_epoch: Some(u64::MAX), delegated: Some(false), confined: Some(false), + remote_slot: Some(loaded_program.remote_slot), } }; diff --git a/magicblock-account-cloner/src/lib.rs b/magicblock-account-cloner/src/lib.rs index db544646f..ef70f478b 100644 --- a/magicblock-account-cloner/src/lib.rs +++ b/magicblock-account-cloner/src/lib.rs @@ -100,6 +100,7 @@ impl ChainlinkCloner { executable: Some(request.account.executable()), delegated: Some(request.account.delegated()), confined: Some(request.account.confined()), + remote_slot: Some(request.account.remote_slot()), }; let modify_ix = InstructionUtils::modify_accounts_instruction(vec![ @@ -222,6 +223,7 @@ impl ChainlinkCloner { // and then deploy it and finally set the authority to match the // one on chain let slot = self.accounts_db.slot(); + let program_remote_slot = program.remote_slot; let DeployableV4Program { pre_deploy_loader_state, deploy_instruction, @@ -248,6 +250,7 @@ impl ChainlinkCloner { executable: Some(true), data: Some(pre_deploy_loader_state), confined: Some(false), + remote_slot: Some(program_remote_slot), ..Default::default() }]; InstructionUtils::modify_accounts_instruction( @@ -260,6 +263,7 @@ impl ChainlinkCloner { pubkey: program_id, data: Some(post_deploy_loader_state), confined: Some(false), + remote_slot: Some(program_remote_slot), ..Default::default() }]; InstructionUtils::modify_accounts_instruction( diff --git a/magicblock-chainlink/src/chainlink/fetch_cloner.rs b/magicblock-chainlink/src/chainlink/fetch_cloner.rs index dc2256b01..55e61a438 100644 --- a/magicblock-chainlink/src/chainlink/fetch_cloner.rs +++ b/magicblock-chainlink/src/chainlink/fetch_cloner.rs @@ -217,7 +217,20 @@ where if in_bank.undelegating() { // We expect the account to still be delegated, but with the delegation // program owner - debug!("Received update for undelegating account {pubkey} delegated in bank={} delegated on chain={}", in_bank.delegated(), account.delegated()); + debug!("Received update for undelegating account {pubkey} \ + in_bank.delegated={}, \ + in_bank.owner={}, \ + in_bank.remote_slot={}, \ + chain.delegated={}, \ + chain.owner={}, \ + chain.remote_slot={}", + in_bank.delegated(), + in_bank.owner(), + in_bank.remote_slot(), + account.delegated(), + account.owner(), + account.remote_slot() + ); // This will only be true in the following case: // 1. a commit was triggered for the account @@ -414,7 +427,7 @@ where if log::log_enabled!(log::Level::Trace) { trace!("Delegation record found for {pubkey}: {delegation_record:?}"); trace!( - "Cloning delegated account: {pubkey} (remote slot {}, owner: {})", + "Resolving delegated account: {pubkey} (remote slot {}, owner: {})", account.remote_slot(), delegation_record.owner ); @@ -1262,7 +1275,16 @@ where } RefreshDecision::No => { // Account is in bank and subscribed correctly - no fetch needed - trace!("Account {pubkey} found in bank in valid state, no fetch needed"); + if log::log_enabled!(log::Level::Trace) { + let undelegating = account_in_bank.undelegating(); + let delegated = account_in_bank.delegated(); + let owner = account_in_bank.owner().to_string(); + trace!("Account {pubkey} found in bank in valid state, no fetch needed \ + undelegating = {undelegating}, \ + delegated = {delegated}, \ + owner={owner}" + ); + } in_bank.push(*pubkey); } } diff --git a/magicblock-chainlink/src/chainlink/mod.rs b/magicblock-chainlink/src/chainlink/mod.rs index a5d416c69..d1f37ab51 100644 --- a/magicblock-chainlink/src/chainlink/mod.rs +++ b/magicblock-chainlink/src/chainlink/mod.rs @@ -219,7 +219,37 @@ Kept: {} delegated, {} blacklisted", task::spawn(async move { while let Some(pubkey) = removed_accounts_rx.recv().await { - accounts_bank.remove_account(&pubkey); + accounts_bank.remove_account_conditionally( + &pubkey, + |account| { + // Accounts that are still undelegating need to be kept in the bank + // until the undelegation completes on chain. + // Otherwise we might loose data in case the undelegation fails to + // complete. + // Another issue we avoid this way is that an account update received + // before the account completes undelegation would overwrite the in-bank + // account and thus also set unedelegating to false. + let undelegating = account.undelegating(); + let delegated = account.delegated(); + let remove = !undelegating && !delegated; + if log::log_enabled!(log::Level::Trace) { + if remove { + trace!( + "Removing unsubscribed account '{pubkey}' from bank" + ); + } else { + let owner = account.owner().to_string(); + trace!( + "Keeping unsubscribed account {pubkey} in bank \ + undelegating = {undelegating}, \ + delegated = {delegated}, \ + owner={owner}" + ); + } + } + remove + }, + ); } warn!("Removed accounts channel closed, stopping subscription"); }) diff --git a/magicblock-chainlink/tests/07_redeleg_us_same_slot.rs b/magicblock-chainlink/tests/07_redeleg_us_same_slot.rs index a23139d0d..1d5dd1502 100644 --- a/magicblock-chainlink/tests/07_redeleg_us_same_slot.rs +++ b/magicblock-chainlink/tests/07_redeleg_us_same_slot.rs @@ -23,7 +23,13 @@ async fn setup(slot: Slot) -> TestContext { TestContext::init(slot).await } +// NOTE: disabled for now since we detect redelegation as follows: +// - if `account.remote_slot >= undelegation_slot` assume it is the first delegation, i.e. it was +// not re-delegated +// - in order to support same slot re-delegation we need an delegation index or similar to +// distinguish the two delegations #[tokio::test] +#[ignore = "Same slot redelegation is currently not possible and currently it is detected as a single delegation operation."] async fn test_undelegate_redelegate_to_us_in_same_slot() { let mut slot: u64 = 11; diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index e0c9d7604..9f6d9b0b6 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -15,4 +15,16 @@ pub trait AccountsBank: Send + Sync + 'static { &self, predicate: impl Fn(&Pubkey, &AccountSharedData) -> bool, ) -> usize; + + fn remove_account_conditionally( + &self, + pubkey: &Pubkey, + predicate: impl Fn(&AccountSharedData) -> bool, + ) { + if let Some(acc) = self.get_account(pubkey) { + if predicate(&acc) { + self.remove_account(pubkey); + } + } + } } diff --git a/magicblock-magic-program-api/src/instruction.rs b/magicblock-magic-program-api/src/instruction.rs index a137815b8..cdb9acffd 100644 --- a/magicblock-magic-program-api/src/instruction.rs +++ b/magicblock-magic-program-api/src/instruction.rs @@ -120,6 +120,7 @@ pub struct AccountModification { pub rent_epoch: Option, pub delegated: Option, pub confined: Option, + pub remote_slot: Option, } #[derive(Default, Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] @@ -133,4 +134,5 @@ pub struct AccountModificationForInstruction { pub rent_epoch: Option, pub delegated: Option, pub confined: Option, + pub remote_slot: Option, } diff --git a/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs b/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs index 62e68005d..30a039463 100644 --- a/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs +++ b/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs @@ -223,6 +223,14 @@ pub(crate) fn process_mutate_accounts( ); account.borrow_mut().set_confined(confined); } + if let Some(remote_slot) = modification.remote_slot { + ic_msg!( + invoke_context, + "MutateAccounts: setting remote_slot to {}", + remote_slot + ); + account.borrow_mut().set_remote_slot(remote_slot); + } } if lamports_to_debit != 0 { @@ -329,6 +337,7 @@ mod tests { rent_epoch: None, delegated: Some(true), confined: Some(true), + remote_slot: None, }; let ix = InstructionUtils::modify_accounts_instruction(vec![ modification.clone(), @@ -623,4 +632,46 @@ mod tests { } ); } + + #[test] + fn test_mod_remote_slot() { + init_logger!(); + + let mod_key = Pubkey::new_unique(); + let remote_slot = 12345u64; + let mut account_data = { + let mut map = HashMap::new(); + map.insert(mod_key, AccountSharedData::new(100, 0, &mod_key)); + map + }; + ensure_started_validator(&mut account_data); + + let ix = InstructionUtils::modify_accounts_instruction(vec![ + AccountModification { + pubkey: mod_key, + remote_slot: Some(remote_slot), + ..Default::default() + }, + ]); + let transaction_accounts = ix + .accounts + .iter() + .flat_map(|acc| { + account_data + .remove(&acc.pubkey) + .map(|shared_data| (acc.pubkey, shared_data)) + }) + .collect(); + + let mut accounts = process_instruction( + ix.data.as_slice(), + transaction_accounts, + ix.accounts, + Ok(()), + ); + + let _account_authority = accounts.drain(0..1).next().unwrap(); + let modified_account = accounts.drain(0..1).next().unwrap(); + assert_eq!(modified_account.remote_slot(), remote_slot); + } } diff --git a/programs/magicblock/src/utils/instruction_utils.rs b/programs/magicblock/src/utils/instruction_utils.rs index ebdeb5d6d..58dd626bf 100644 --- a/programs/magicblock/src/utils/instruction_utils.rs +++ b/programs/magicblock/src/utils/instruction_utils.rs @@ -182,6 +182,7 @@ impl InstructionUtils { rent_epoch: account_modification.rent_epoch, delegated: account_modification.delegated, confined: account_modification.confined, + remote_slot: account_modification.remote_slot, }; account_mods.insert( account_modification.pubkey, diff --git a/test-integration/configs/schedulecommit-conf-fees.ephem.toml b/test-integration/configs/schedulecommit-conf-fees.ephem.toml index 8479233d1..349f92fed 100644 --- a/test-integration/configs/schedulecommit-conf-fees.ephem.toml +++ b/test-integration/configs/schedulecommit-conf-fees.ephem.toml @@ -5,14 +5,14 @@ listen = "0.0.0.0:8899" [accountsdb] # size of the main storage, we have to preallocate in advance -# it's advised to set this value based on formula 1KB * N * 3, -# where N is the number of accounts expected to be stored in +# it's advised to set this value based on formula 1KB * N * 3, +# where N is the number of accounts expected to be stored in # database, e.g. for million accounts this would be 3GB database-size = 1048576000 # 1GB # minimal indivisible unit of addressing in main storage # offsets are calculated in terms of blocks block-size = "block256" # possible values block128 | block256 | block512 -# size of index file, we have to preallocate, +# size of index file, we have to preallocate, # can be as low as 1% of main storage size, but setting it to higher values won't hurt index-size = 2048576 # max number of snapshots to keep around @@ -20,6 +20,8 @@ max-snapshots = 7 # how frequently (slot-wise) we should take snapshots snapshot-frequency = 1024 +reset = true + [ledger] reset = true diff --git a/test-integration/programs/schedulecommit/src/api.rs b/test-integration/programs/schedulecommit/src/api.rs index 4af491322..adecc662b 100644 --- a/test-integration/programs/schedulecommit/src/api.rs +++ b/test-integration/programs/schedulecommit/src/api.rs @@ -256,6 +256,16 @@ pub fn increase_count_instruction(committee: Pubkey) -> Instruction { ) } +pub fn set_count_instruction(committee: Pubkey, count: u64) -> Instruction { + let program_id = crate::id(); + let account_metas = vec![AccountMeta::new(committee, false)]; + Instruction::new_with_borsh( + program_id, + &ScheduleCommitInstruction::SetCount(count), + account_metas, + ) +} + // ----------------- // PDA // ----------------- diff --git a/test-integration/programs/schedulecommit/src/lib.rs b/test-integration/programs/schedulecommit/src/lib.rs index 411ca3509..cdd207408 100644 --- a/test-integration/programs/schedulecommit/src/lib.rs +++ b/test-integration/programs/schedulecommit/src/lib.rs @@ -32,6 +32,8 @@ pub mod api; pub mod magicblock_program; mod utils; +pub const FAIL_UNDELEGATION_COUNT: u64 = u64::MAX - 1; + declare_id!("9hgprgZiRWmy8KkfvUuaVkDGrqo9GzeXMohwq6BazgUY"); #[cfg(not(feature = "no-entrypoint"))] @@ -113,6 +115,13 @@ pub enum ScheduleCommitInstruction { /// # Account references: /// - **0.** `[WRITE]` PDA Account to increase count of IncreaseCount, + + /// Sets the count of a PDA of this program to the provided arbitrary u64 value. + /// This instruction can only run on the ephemeral after the account was + /// delegated or on chain while it is undelegated. + /// # Account references: + /// - **0.** `[WRITE]` PDA Account to write the counter to set to the supplied value + SetCount(u64), // This is invoked by the delegation program when we request to undelegate // accounts. // # Account references: @@ -170,6 +179,7 @@ pub fn process_instruction<'a>( ) } IncreaseCount => process_increase_count(accounts), + SetCount(value) => process_set_count(accounts, value), } } @@ -398,6 +408,27 @@ fn process_increase_count(accounts: &[AccountInfo]) -> ProgramResult { Ok(()) } +fn process_set_count(accounts: &[AccountInfo], value: u64) -> ProgramResult { + msg!("Processing set_count instruction"); + // NOTE: we don't check if the player owning the PDA is signer here for simplicity + let accounts_iter = &mut accounts.iter(); + let account = next_account_info(accounts_iter)?; + msg!("Counter account key {}", account.key); + let mut main_account = { + let main_account_data = account.try_borrow_data()?; + MainAccount::try_from_slice(&main_account_data)? + }; + msg!("Owner: {}", account.owner); + msg!("Counter account {:#?}", main_account); + main_account.count = value; + msg!("Set count to {:#?}", main_account); + let mut mut_data = account.try_borrow_mut_data()?; + let mut as_mut: &mut [u8] = mut_data.as_mut(); + msg!("Mutating buffer of len: {}", as_mut.len()); + main_account.serialize(&mut as_mut)?; + msg!("Serialized counter"); + Ok(()) +} // ----------------- // process_schedulecommit_and_undelegation_cpi_with_mod_after // ----------------- @@ -586,5 +617,18 @@ fn process_undelegate_request( system_program, account_seeds, )?; + + { + let data = delegated_account.try_borrow_data()?; + match MainAccount::try_from_slice(&data) { + Ok(counter) => { + msg!("counter: {:?}", counter); + if counter.count == FAIL_UNDELEGATION_COUNT { + return Err(ProgramError::Custom(111)); + } + } + Err(err) => msg!("Failed to deserialize: {:?}", err), + } + }; Ok(()) } diff --git a/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs b/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs index 9b4e30b3f..550cdd508 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs @@ -4,10 +4,15 @@ use integration_test_tools::{ transactions::send_and_confirm_instructions_with_payer, }; use log::*; -use program_schedulecommit::api::{ - increase_count_instruction, schedule_commit_and_undelegate_cpi_instruction, - schedule_commit_and_undelegate_cpi_twice, - schedule_commit_and_undelegate_cpi_with_mod_after_instruction, +use program_schedulecommit::{ + api::{ + increase_count_instruction, + schedule_commit_and_undelegate_cpi_instruction, + schedule_commit_and_undelegate_cpi_twice, + schedule_commit_and_undelegate_cpi_with_mod_after_instruction, + set_count_instruction, + }, + FAIL_UNDELEGATION_COUNT, }; use schedulecommit_client::{ verify, ScheduleCommitTestContext, ScheduleCommitTestContextFields, @@ -598,3 +603,108 @@ fn test_committing_and_undelegating_two_accounts_twice() { debug!("✅ Verified that not commit was scheduled since tx failed"); }); } + +#[test] +fn test_committing_after_failed_undelegation() { + run_test!({ + let ctx = get_context_with_delegated_committees(1); + let ScheduleCommitTestContextFields { + payer_ephem: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + let [(_committee_authority, committee_pda)] = committees.as_slice() + else { + panic!("Unexpected num of committees"); + }; + + let set_counter = |counter, expect_failure| { + let ix = set_count_instruction(*committee_pda, counter); + let ephem_blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + ephem_blockhash, + ); + let tx_res = ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ); + + match tx_res { + Ok(sig) if expect_failure => { + panic!("Expected failure when setting counter to {} with sig {}", counter, sig); + } + Ok(sig) => { + debug!("set_counter sigs: {:?}", sig); + let res = ctx.confirm_transaction_ephem(&sig, None); + assert!(res.unwrap()); + } + Err(err) if !expect_failure => { + panic!("Did not expect failure when setting counter to {} ({:?}", counter, err); + } + _ => {} + } + }; + + // Set counter to FAIL_UNDELEGATION_COUNT so undelegation fails + // NOTE: undelegation will get patched so as result account will be only committed + set_counter(FAIL_UNDELEGATION_COUNT, false); + + // Commit & Undelegate + { + let ix = schedule_commit_and_undelegate_cpi_instruction( + payer.pubkey(), + magicblock_magic_program_api::id(), + magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY, + &committees + .iter() + .map(|(player, _)| player.pubkey()) + .collect::>(), + &committees.iter().map(|(_, pda)| *pda).collect::>(), + ); + + let ephem_blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + ephem_blockhash, + ); + + let sig = tx.get_signature(); + let tx_res = ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ); + debug!("Commit and Undelegate Transaction result: '{:?}'", tx_res); + + // 2. Retrieve the signature of the scheduled commit sent + let logs = ctx.fetch_ephemeral_logs(*sig).unwrap(); + let scheduled_commmit_sent_sig = + extract_scheduled_commit_sent_signature_from_logs(&logs) + .unwrap(); + + debug!("sent_sig: {}", scheduled_commmit_sent_sig); + // 3. Confirm commit was scheduled + assert!(ctx + .confirm_transaction_ephem(&scheduled_commmit_sent_sig, None) + .unwrap()); + } + + set_counter(2222, true); + }); +}