fix: double undelegation

magicblock-accounts/src/scheduled_commits_processor.rs

@@ -92,51 +92,45 @@ impl ScheduledCommitsProcessorImpl {
     fn preprocess_intent(
         &self,
         mut base_intent: ScheduledBaseIntent,
-    ) -> (ScheduledBaseIntentWrapper, Vec<Pubkey>, Vec<Pubkey>) {
+    ) -> (ScheduledBaseIntentWrapper, Vec<Pubkey>) {
         let is_undelegate = base_intent.is_undelegate();
         let Some(committed_accounts) = base_intent.get_committed_accounts_mut()
         else {
             let intent = ScheduledBaseIntentWrapper {
                 inner: base_intent,
                 trigger_type: TriggerType::OnChain,
             };
-            return (intent, vec![], vec![]);
+            return (intent, vec![]);
         };
 
-        let mut excluded_pubkeys = vec![];
-        let mut pubkeys_being_undelegated = vec![];
-        // Retains only account that are valid to be committed (all delegated ones)
-        committed_accounts.retain_mut(|account| {
-            let pubkey = account.pubkey;
-            let acc = self.accounts_bank.get_account(&pubkey);
-            match acc {
-                Some(acc) => {
-                    if acc.delegated() {
-                        if is_undelegate {
-                            pubkeys_being_undelegated.push(pubkey);
-                        }
-                        true
-                    } else {
-                        excluded_pubkeys.push(pubkey);
-                        false
-                    }
+        // dump undelegated pubkeys
+        let pubkeys_being_undelegated: Vec<_> = committed_accounts
+            .iter()
+            .inspect(|account| {
+                let pubkey = account.pubkey;
+                if self.accounts_bank.get_account(&pubkey).is_none() {
+                    // This doesn't affect intent validity
+                    // We assume that intent is correct at the moment of scheduling
+                    // All the checks are performed by runtime & magic-program at the moment of scheduling
+                    // This log could be a sign of eviction or a bug in implementation
+                    info!("Account got evicted from AccountsDB after intent was scheduled!");
                 }
-                None => {
-                    warn!(
-                        "Account {} not found in AccountsDb, skipping from commit",
-                        pubkey
-                    );
-                    false
+            })
+            .filter_map(|account| {
+                if is_undelegate {
+                    Some(account.pubkey)
+                } else {
+                    None
                 }
-            }
-        });
+            })
+            .collect();
 
         let intent = ScheduledBaseIntentWrapper {
             inner: base_intent,
             trigger_type: TriggerType::OnChain,
         };
 
-        (intent, excluded_pubkeys, pubkeys_being_undelegated)
+        (intent, pubkeys_being_undelegated)
     }
 
     async fn process_undelegation_requests(&self, pubkeys: Vec<Pubkey>) {
@@ -341,7 +335,7 @@ impl ScheduledCommitsProcessorImpl {
             payer: intent_meta.payer,
             chain_signatures,
             included_pubkeys: intent_meta.included_pubkeys,
-            excluded_pubkeys: intent_meta.excluded_pubkeys,
+            excluded_pubkeys: vec![],
             requested_undelegation: intent_meta.requested_undelegation,
         }
     }
@@ -350,14 +344,14 @@ impl ScheduledCommitsProcessorImpl {
 #[async_trait]
 impl ScheduledCommitsProcessor for ScheduledCommitsProcessorImpl {
     async fn process(&self) -> ScheduledCommitsProcessorResult<()> {
-        let scheduled_base_intent =
+        let scheduled_base_intents =
             self.transaction_scheduler.take_scheduled_actions();
 
-        if scheduled_base_intent.is_empty() {
+        if scheduled_base_intents.is_empty() {
             return Ok(());
         }
 
-        let intents = scheduled_base_intent
+        let intents = scheduled_base_intents
             .into_iter()
             .map(|intent| self.preprocess_intent(intent));
 
@@ -368,10 +362,10 @@ impl ScheduledCommitsProcessor for ScheduledCommitsProcessorImpl {
             let mut pubkeys_being_undelegated = HashSet::new();
 
             let intents = intents
-                .map(|(intent, excluded_pubkeys, undelegated)| {
+                .map(|(intent, undelegated)| {
                     intent_metas.insert(
                         intent.id,
-                        ScheduledBaseIntentMeta::new(&intent, excluded_pubkeys),
+                        ScheduledBaseIntentMeta::new(&intent),
                     );
                     pubkeys_being_undelegated.extend(undelegated);
 
@@ -409,24 +403,19 @@ struct ScheduledBaseIntentMeta {
     blockhash: Hash,
     payer: Pubkey,
     included_pubkeys: Vec<Pubkey>,
-    excluded_pubkeys: Vec<Pubkey>,
     intent_sent_transaction: Transaction,
     requested_undelegation: bool,
 }
 
 impl ScheduledBaseIntentMeta {
-    fn new(
-        intent: &ScheduledBaseIntent,
-        excluded_pubkeys: Vec<Pubkey>,
-    ) -> Self {
+    fn new(intent: &ScheduledBaseIntent) -> Self {
         Self {
             slot: intent.slot,
             blockhash: intent.blockhash,
             payer: intent.payer,
             included_pubkeys: intent
                 .get_committed_pubkeys()
                 .unwrap_or_default(),
-            excluded_pubkeys,
             intent_sent_transaction: intent.action_sent_transaction.clone(),
             requested_undelegation: intent.is_undelegate(),
         }

programs/magicblock/src/magic_scheduled_base_intent.rs

@@ -217,13 +217,14 @@ impl CommitAndUndelegate {
     ) -> Result<(), InstructionError> {
         account_indices.iter().copied().try_for_each(|idx| {
             let is_writable = get_writable_with_idx(context.transaction_context, idx as u16)?;
-            if is_writable {
+            let delegated = get_instruction_account_with_idx(context.transaction_context, idx as u16)?;
+            if is_writable && delegated.borrow().delegated() {
                 Ok(())
             } else {
                 let pubkey = get_instruction_pubkey_with_idx(context.transaction_context, idx as u16)?;
                 ic_msg!(
                     context.invoke_context,
-                    "ScheduleCommit ERR: account {} is required to be writable in order to be undelegated",
+                    "ScheduleCommit ERR: account {} is required to be writable and delegated in order to be undelegated",
                     pubkey
                 );
                 Err(InstructionError::ReadonlyDataModified)

programs/magicblock/src/schedule_transactions/process_schedule_base_intent.rs

@@ -14,7 +14,7 @@ use crate::{
     },
     schedule_transactions::check_magic_context_id,
     utils::{
-        account_actions::mark_account_as_undelegating,
+        account_actions::mark_account_as_undelegated,
         accounts::{
             get_instruction_account_with_idx, get_instruction_pubkey_with_idx,
         },
@@ -143,7 +143,7 @@ pub(crate) fn process_schedule_base_intent(
         // Once account is undelegated we need to make it immutable in our validator.
         for (pubkey, account_ref) in undelegated_accounts_ref.iter() {
             undelegated_pubkeys.push(pubkey.to_string());
-            mark_account_as_undelegating(account_ref);
+            mark_account_as_undelegated(account_ref);
         }
     }
     if !undelegated_pubkeys.is_empty() {

programs/magicblock/src/schedule_transactions/process_schedule_commit.rs

@@ -16,7 +16,7 @@ use crate::{
     },
     schedule_transactions,
     utils::{
-        account_actions::mark_account_as_undelegating,
+        account_actions::mark_account_as_undelegated,
         accounts::{
             get_instruction_account_with_idx, get_instruction_pubkey_with_idx,
             get_writable_with_idx,
@@ -133,14 +133,15 @@ pub(crate) fn process_schedule_commit(
             get_instruction_account_with_idx(transaction_context, idx as u16)?;
         {
             if opts.request_undelegation {
-                // Since we need to modify the account during undelegation, we expect it to be writable
-                // We rely on invariant "writable means delegated"
+                // Check if account is writable and also undelegated
+                // SVM doesn't check delegated, so we need to do extra checks here
+                // Otherwise account could be undelegated twice
                 let acc_writable =
                     get_writable_with_idx(transaction_context, idx as u16)?;
-                if !acc_writable {
+                if !acc_writable || !acc.borrow().delegated() {
                     ic_msg!(
                         invoke_context,
-                        "ScheduleCommit ERR: account {} is required to be writable in order to be undelegated",
+                        "ScheduleCommit ERR: account {} is required to be writable and delegated in order to be undelegated",
                         acc_pubkey
                     );
                     return Err(InstructionError::ReadonlyDataModified);
@@ -201,7 +202,7 @@ pub(crate) fn process_schedule_commit(
             //
             // We also set the undelegating flag on the account in order to detect
             // undelegations for which we miss updates
-            mark_account_as_undelegating(acc);
+            mark_account_as_undelegated(acc);
             ic_msg!(
                 invoke_context,
                 "ScheduleCommit: Marking account {} as undelegating",

programs/magicblock/src/utils/account_actions.rs

@@ -14,8 +14,10 @@ pub(crate) fn set_account_owner(
     acc.borrow_mut().set_owner(pubkey);
 }
 
-/// Sets proper values on account during undelegation
-pub(crate) fn mark_account_as_undelegating(acc: &RefCell<AccountSharedData>) {
+/// Sets proper account values during undelegation
+pub(crate) fn mark_account_as_undelegated(acc: &RefCell<AccountSharedData>) {
     set_account_owner(acc, DELEGATION_PROGRAM_ID);
-    acc.borrow_mut().set_undelegating(true);
+    let mut acc = acc.borrow_mut();
+    acc.set_undelegating(true);
+    acc.set_delegated(false);
 }