Make unit-tests work again

Author: snawaz

magicblock-committor-service/src/tasks/args_task.rs

@@ -44,13 +44,7 @@ impl ArgsTask {
 impl BaseTask for ArgsTask {
     fn instruction(&self, validator: &Pubkey) -> Instruction {
         match &self.task_type {
-            ArgsTaskType::Commit(value) => {
-                if value.is_commit_diff() {
-                    value.create_commit_diff_ix(validator)
-                } else {
-                    value.create_commit_state_ix(validator)
-                }
-            }
+            ArgsTaskType::Commit(value) => value.create_commit_ix(validator),
             ArgsTaskType::Finalize(value) => {
                 dlp::instruction_builder::finalize(
                     *validator,

magicblock-committor-service/src/tasks/mod.rs

@@ -17,7 +17,7 @@ use magicblock_committor_program::{
 use magicblock_program::magic_scheduled_base_intent::{
     BaseAction, CommittedAccount,
 };
-use solana_account::ReadableAccount;
+use solana_account::{Account, ReadableAccount};
 use solana_pubkey::Pubkey;
 use solana_rpc_client::rpc_client::RpcClient;
 use solana_sdk::{
@@ -114,61 +114,93 @@ pub struct CommitTask {
     pub commit_id: u64,
     pub allow_undelegation: bool,
     pub committed_account: CommittedAccount,
+    fetched_account: Option<Account>,
 }
 
 impl CommitTask {
     const COMMIT_STATE_SIZE_THRESHOLD: usize = 200;
 
+    pub fn new(
+        commit_id: u64,
+        allow_undelegation: bool,
+        committed_account: CommittedAccount,
+    ) -> Self {
+        let chain_config =
+            ChainConfig::local(ComputeBudgetConfig::new(1_000_000));
+
+        let rpc_client = RpcClient::new_with_commitment(
+            chain_config.rpc_uri.to_string(),
+            CommitmentConfig {
+                commitment: chain_config.commitment,
+            },
+        );
+
+        let fetched_account = if committed_account.account.data.len()
+            > CommitTask::COMMIT_STATE_SIZE_THRESHOLD
+        {
+            rpc_client.get_account(&committed_account.pubkey).ok()
+        } else {
+            None
+        };
+
+        Self {
+            commit_id,
+            allow_undelegation,
+            committed_account,
+            fetched_account,
+        }
+    }
+
+    // TODO (snawaz): it is infinitely bad implementation
+    // as it's making a network call, but we'll fix it soon once
+    // we start using caching and fetched accounts.
     pub fn is_commit_diff(&self) -> bool {
         self.committed_account.account.data.len()
             > CommitTask::COMMIT_STATE_SIZE_THRESHOLD
+            && self.fetched_account.is_some()
     }
 
-    pub fn create_commit_state_ix(&self, validator: &Pubkey) -> Instruction {
+    pub fn create_commit_ix(&self, validator: &Pubkey) -> Instruction {
+        if let Some(fetched_account) = self.fetched_account.as_ref() {
+            self.create_commit_diff_ix(validator, fetched_account)
+        } else {
+            self.create_commit_state_ix(validator)
+        }
+    }
+
+    fn create_commit_state_ix(&self, validator: &Pubkey) -> Instruction {
         let args = CommitStateArgs {
             nonce: self.commit_id,
             lamports: self.committed_account.account.lamports,
             data: self.committed_account.account.data.clone(),
             allow_undelegation: self.allow_undelegation,
         };
+        println!("create_commit_state_ix, data: {}", args.data.len());
         dlp::instruction_builder::commit_state(
             *validator,
             self.committed_account.pubkey,
             self.committed_account.account.owner,
             args,
         )
     }
-    pub fn create_commit_diff_ix(&self, validator: &Pubkey) -> Instruction {
-        let chain_config =
-            ChainConfig::local(ComputeBudgetConfig::new(1_000_000));
-
-        let rpc_client = RpcClient::new_with_commitment(
-            chain_config.rpc_uri.to_string(),
-            CommitmentConfig {
-                commitment: chain_config.commitment,
-            },
-        );
-
-        let account = match rpc_client
-            .get_account(&self.committed_account.pubkey)
-        {
-            Ok(account) => account,
-            Err(e) => {
-                log::warn!("Fallback to commit_state and send full-bytes, as rpc failed to fetch the delegated-account from base chain, commmit_id: {} , error: {}", self.commit_id, e);
-                return self.create_commit_state_ix(validator);
-            }
-        };
 
+    fn create_commit_diff_ix(
+        &self,
+        validator: &Pubkey,
+        fetched_account: &Account,
+    ) -> Instruction {
         let args = CommitDiffArgs {
             nonce: self.commit_id,
             lamports: self.committed_account.account.lamports,
             diff: compute_diff(
-                account.data(),
+                fetched_account.data(),
                 self.committed_account.account.data(),
             )
             .to_vec(),
             allow_undelegation: self.allow_undelegation,
         };
+        println!("create_commit_diff_ix, diff: {}", args.diff.len());
+
         dlp::instruction_builder::commit_diff(
             *validator,
             self.committed_account.pubkey,
@@ -382,10 +414,10 @@ mod serialization_safety_test {
         let validator = Pubkey::new_unique();
 
         // Test Commit variant
-        let commit_task: ArgsTask = ArgsTaskType::Commit(CommitTask {
-            commit_id: 123,
-            allow_undelegation: true,
-            committed_account: CommittedAccount {
+        let commit_task: ArgsTask = ArgsTaskType::Commit(CommitTask::new(
+            123,
+            true,
+            CommittedAccount {
                 pubkey: Pubkey::new_unique(),
                 account: Account {
                     lamports: 1000,
@@ -395,7 +427,7 @@ mod serialization_safety_test {
                     rent_epoch: 0,
                 },
             },
-        })
+        ))
         .into();
         assert_serializable(&commit_task.instruction(&validator));
 
@@ -442,10 +474,10 @@ mod serialization_safety_test {
         let validator = Pubkey::new_unique();
 
         let buffer_task = BufferTask::new_preparation_required(
-            BufferTaskType::Commit(CommitTask {
-                commit_id: 456,
-                allow_undelegation: false,
-                committed_account: CommittedAccount {
+            BufferTaskType::Commit(CommitTask::new(
+                456,
+                false,
+                CommittedAccount {
                     pubkey: Pubkey::new_unique(),
                     account: Account {
                         lamports: 2000,
@@ -455,7 +487,7 @@ mod serialization_safety_test {
                         rent_epoch: 0,
                     },
                 },
-            }),
+            )),
         );
         assert_serializable(&buffer_task.instruction(&validator));
     }
@@ -467,10 +499,10 @@ mod serialization_safety_test {
 
         // Test BufferTask preparation
         let buffer_task = BufferTask::new_preparation_required(
-            BufferTaskType::Commit(CommitTask {
-                commit_id: 789,
-                allow_undelegation: true,
-                committed_account: CommittedAccount {
+            BufferTaskType::Commit(CommitTask::new(
+                789,
+                true,
+                CommittedAccount {
                     pubkey: Pubkey::new_unique(),
                     account: Account {
                         lamports: 3000,
@@ -480,7 +512,7 @@ mod serialization_safety_test {
                         rent_epoch: 0,
                     },
                 },
-            }),
+            )),
         );
 
         let PreparationState::Required(preparation_task) =

magicblock-committor-service/src/tasks/task_builder.rs

@@ -89,11 +89,11 @@ impl TasksBuilder for TaskBuilderImpl {
             .iter()
             .map(|account| {
                 let commit_id = *commit_ids.get(&account.pubkey).expect("CommitIdFetcher provide commit ids for all listed pubkeys, or errors!");
-                let task = ArgsTaskType::Commit(CommitTask {
+                let task = ArgsTaskType::Commit(CommitTask::new(
                     commit_id,
                     allow_undelegation,
-                    committed_account: account.clone(),
-                });
+                    account.clone(),
+                ));
 
                 Box::new(ArgsTask::new(task)) as Box<dyn BaseTask>
             })

magicblock-committor-service/src/tasks/task_strategist.rs

@@ -95,6 +95,7 @@ impl TaskStrategist {
                 lookup_tables_keys,
             })
         } else {
+            println!("snawaz: inside build_strategy");
             Err(TaskStrategistError::FailedToFitError)
         }
     }
@@ -165,7 +166,10 @@ impl TaskStrategist {
                 &[],
             ) {
                 Ok(tx) => Ok(serialize_and_encode_base64(&tx).len()),
-                Err(TaskStrategistError::FailedToFitError) => Ok(usize::MAX),
+                Err(TaskStrategistError::FailedToFitError) => {
+                    println!("snawaz: inside optimize_strategy");
+                    Ok(usize::MAX)
+                }
                 Err(TaskStrategistError::SignerError(err)) => Err(err),
             }
         };
@@ -264,10 +268,10 @@ mod tests {
 
     // Helper to create a simple commit task
     fn create_test_commit_task(commit_id: u64, data_size: usize) -> ArgsTask {
-        ArgsTask::new(ArgsTaskType::Commit(CommitTask {
+        ArgsTask::new(ArgsTaskType::Commit(CommitTask::new(
             commit_id,
-            allow_undelegation: false,
-            committed_account: CommittedAccount {
+            false,
+            CommittedAccount {
                 pubkey: Pubkey::new_unique(),
                 account: Account {
                     lamports: 1000,
@@ -277,7 +281,7 @@ mod tests {
                     rent_epoch: 0,
                 },
             },
-        }))
+        )))
     }
 
     // Helper to create a Base action task

test-integration/programs/schedulecommit-security/src/lib.rs

@@ -1,5 +1,5 @@
 use borsh::{BorshDeserialize, BorshSerialize};
-use ephemeral_rollups_sdk::ephem::{create_schedule_commit_ix, CommitPolicy};
+use ephemeral_rollups_sdk::ephem::create_schedule_commit_ix;
 use program_schedulecommit::{
     api::schedule_commit_cpi_instruction, process_schedulecommit_cpi,
     ProcessSchedulecommitCpiArgs,
@@ -146,7 +146,6 @@ fn process_sibling_schedule_cpis(
             magic_context,
             magic_program,
             false,
-            CommitPolicy::UseFullBytes,
         );
         invoke(
             &direct_ix,

test-integration/programs/schedulecommit/src/order_book.rs

@@ -204,7 +204,7 @@ impl<'a> OrderBook<'a> {
     ) -> &'a mut [OrderLevel] {
         slice::from_raw_parts_mut(
             self.levels.add(self.capacity - asks_len),
-            asks_len as usize,
+            asks_len,
         )
     }