Address taco's feedback

Author: snawaz

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

@@ -6,7 +6,7 @@ pub mod two_stage_executor;
 
 use std::{mem, ops::ControlFlow, sync::Arc, time::Duration};
 
-#[cfg(any(test, feature = "dev-context-only-utils"))]
+#[cfg(test)]
 mod null_task_info_fetcher;
 
 use async_trait::async_trait;
@@ -25,7 +25,7 @@ use magicblock_rpc_client::{
     MagicBlockRpcClientError, MagicBlockSendTransactionConfig,
     MagicBlockSendTransactionOutcome, MagicblockRpcClient,
 };
-#[cfg(any(test, feature = "dev-context-only-utils"))]
+#[cfg(test)]
 pub use null_task_info_fetcher::*;
 use solana_keypair::Keypair;
 use solana_message::VersionedMessage;

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

@@ -303,33 +303,28 @@ pub type BaseTaskResult<T> = Result<T, BaseTaskError>;
 
 #[cfg(test)]
 mod serialization_safety_test {
-    use std::sync::Arc;
 
     use magicblock_program::{
         args::ShortAccountMeta, magic_scheduled_base_intent::ProgramArgs,
     };
     use solana_account::Account;
 
-    use crate::{
-        intent_executor::NullTaskInfoFetcher,
-        tasks::{
-            args_task::{ArgsTask, ArgsTaskType},
-            buffer_task::BufferTask,
-            task_builder::TaskBuilderImpl,
-            *,
-        },
+    use crate::tasks::{
+        args_task::{ArgsTask, ArgsTaskType},
+        buffer_task::{BufferTask, BufferTaskType},
+        *,
     };
 
     // Test all ArgsTask variants
-    #[tokio::test]
-    async fn test_args_task_instruction_serialization() {
+    #[test]
+    fn test_args_task_instruction_serialization() {
         let validator = Pubkey::new_unique();
 
         // Test Commit variant
-        let commit_task: ArgsTask = TaskBuilderImpl::create_commit_task(
-            123,
-            true,
-            CommittedAccount {
+        let commit_task: ArgsTask = ArgsTaskType::Commit(CommitTask {
+            commit_id: 123,
+            allow_undelegation: true,
+            committed_account: CommittedAccount {
                 pubkey: Pubkey::new_unique(),
                 account: Account {
                     lamports: 1000,
@@ -339,9 +334,8 @@ mod serialization_safety_test {
                     rent_epoch: 0,
                 },
             },
-            &Arc::new(NullTaskInfoFetcher),
-        )
-        .await;
+        })
+        .into();
         assert_serializable(&commit_task.instruction(&validator));
 
         // Test Finalize variant
@@ -382,15 +376,15 @@ mod serialization_safety_test {
     }
 
     // Test BufferTask variants
-    #[tokio::test]
-    async fn test_buffer_task_instruction_serialization() {
+    #[test]
+    fn test_buffer_task_instruction_serialization() {
         let validator = Pubkey::new_unique();
 
         let buffer_task = BufferTask::new_preparation_required(
-            TaskBuilderImpl::create_commit_task(
-                456,
-                false,
-                CommittedAccount {
+            BufferTaskType::Commit(CommitTask {
+                commit_id: 456,
+                allow_undelegation: false,
+                committed_account: CommittedAccount {
                     pubkey: Pubkey::new_unique(),
                     account: Account {
                         lamports: 2000,
@@ -400,26 +394,22 @@ mod serialization_safety_test {
                         rent_epoch: 0,
                     },
                 },
-                &Arc::new(NullTaskInfoFetcher),
-            )
-            .await
-            .task_type
-            .into(),
+            }),
         );
         assert_serializable(&buffer_task.instruction(&validator));
     }
 
     // Test preparation instructions
-    #[tokio::test]
-    async fn test_preparation_instructions_serialization() {
+    #[test]
+    fn test_preparation_instructions_serialization() {
         let authority = Pubkey::new_unique();
 
         // Test BufferTask preparation
         let buffer_task = BufferTask::new_preparation_required(
-            TaskBuilderImpl::create_commit_task(
-                789,
-                true,
-                CommittedAccount {
+            BufferTaskType::Commit(CommitTask {
+                commit_id: 789,
+                allow_undelegation: true,
+                committed_account: CommittedAccount {
                     pubkey: Pubkey::new_unique(),
                     account: Account {
                         lamports: 3000,
@@ -429,11 +419,7 @@ mod serialization_safety_test {
                         rent_epoch: 0,
                     },
                 },
-                &Arc::new(NullTaskInfoFetcher),
-            )
-            .await
-            .task_type
-            .into(),
+            }),
         );
 
         let PreparationState::Required(preparation_task) =

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

@@ -13,6 +13,21 @@ use crate::{
     ScheduleCommitInstruction,
 };
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+pub enum UserSeeds {
+    MagicScheduleCommit,
+    OrderBook,
+}
+
+impl UserSeeds {
+    pub fn bytes(&self) -> &'static [u8] {
+        match self {
+            UserSeeds::MagicScheduleCommit => b"magic_schedule_commit",
+            UserSeeds::OrderBook => b"order_book",
+        }
+    }
+}
+
 pub fn init_account_instruction(
     payer: Pubkey,
     player: Pubkey,
@@ -101,20 +116,15 @@ pub fn delegate_account_cpi_instruction(
     payer: Pubkey,
     validator: Option<Pubkey>,
     player_or_book_manager: Pubkey,
-    user_seed: &[u8],
+    user_seed: UserSeeds,
 ) -> Instruction {
     let program_id = crate::id();
     let (pda, _) = Pubkey::find_program_address(
-        &[user_seed, player_or_book_manager.as_ref()],
+        &[user_seed.bytes(), player_or_book_manager.as_ref()],
         &crate::ID,
     );
 
     let delegate_accounts = DelegateAccounts::new(pda, program_id);
-    println!("delegate_accounts: {:#?}", delegate_accounts);
-    println!(
-        "config: {:#?}",
-        dlp::pda::program_config_from_program_id(&crate::ID)
-    );
 
     let delegate_metas = DelegateAccountMetas::from(delegate_accounts);
     let account_metas = vec![
@@ -130,21 +140,24 @@ pub fn delegate_account_cpi_instruction(
 
     Instruction::new_with_borsh(
         program_id,
-        &if user_seed == b"magic_schedule_commit" {
-            ScheduleCommitInstruction::DelegateCpi(DelegateCpiArgs {
-                valid_until: i64::MAX,
-                commit_frequency_ms: 1_000_000_000,
-                player: player_or_book_manager,
-                validator,
-            })
-        } else {
-            ScheduleCommitInstruction::DelegateOrderBook(
-                DelegateOrderBookArgs {
+        &match user_seed {
+            UserSeeds::MagicScheduleCommit => {
+                ScheduleCommitInstruction::DelegateCpi(DelegateCpiArgs {
+                    valid_until: i64::MAX,
                     commit_frequency_ms: 1_000_000_000,
-                    book_manager: player_or_book_manager,
+                    player: player_or_book_manager,
                     validator,
-                },
-            )
+                })
+            }
+            UserSeeds::OrderBook => {
+                ScheduleCommitInstruction::DelegateOrderBook(
+                    DelegateOrderBookArgs {
+                        commit_frequency_ms: 1_000_000_000,
+                        book_manager: player_or_book_manager,
+                        validator,
+                    },
+                )
+            }
         },
         account_metas,
     )

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

@@ -230,7 +230,7 @@ pub struct MainAccount {
 }
 
 impl MainAccount {
-    pub const SIZE: u64 = std::mem::size_of::<Self>() as u64;
+    pub const SIZE: usize = std::mem::size_of::<Self>();
 
     pub fn try_decode(data: &[u8]) -> std::io::Result<Self> {
         Self::try_from_slice(data)
@@ -821,7 +821,7 @@ fn process_undelegate_request(
 
     {
         let mut data = delegated_account.try_borrow_mut_data()?;
-        match data.len() as u64 {
+        match data.len() {
             MainAccount::SIZE => match MainAccount::try_from_slice(&data) {
                 Ok(counter) => {
                     msg!("counter: {:?}", counter);

test-integration/programs/schedulecommit/src/utils/mod.rs

@@ -50,7 +50,7 @@ pub struct AllocateAndAssignAccountArgs<'a, 'b> {
     pub payer_info: &'a AccountInfo<'a>,
     pub account_info: &'a AccountInfo<'a>,
     pub owner: &'a Pubkey,
-    pub size: u64,
+    pub size: usize,
     pub signer_seeds: &'b [&'b [u8]],
 }
 
@@ -68,7 +68,7 @@ pub fn allocate_account_and_assign_owner(
     } = args;
 
     let required_lamports = rent
-        .minimum_balance(size as usize)
+        .minimum_balance(size)
         .max(1)
         .saturating_sub(account_info.lamports());
 
@@ -87,7 +87,10 @@ pub fn allocate_account_and_assign_owner(
     //    At this point the account is still owned by the system program
     msg!("  create_account() allocate space");
     invoke_signed(
-        &system_instruction::allocate(account_info.key, size),
+        &system_instruction::allocate(
+            account_info.key,
+            size.try_into().unwrap(),
+        ),
         // 0. `[WRITE, SIGNER]` New account
         std::slice::from_ref(account_info),
         &[signer_seeds],

test-integration/schedulecommit/client/src/schedule_commit_context.rs

@@ -5,7 +5,7 @@ use integration_test_tools::IntegrationTestContext;
 use log::*;
 use program_schedulecommit::api::{
     delegate_account_cpi_instruction, init_account_instruction,
-    init_order_book_instruction, init_payer_escrow,
+    init_order_book_instruction, init_payer_escrow, UserSeeds,
 };
 use solana_rpc_client::rpc_client::{RpcClient, SerializableTransaction};
 use solana_rpc_client_api::config::RpcSendTransactionConfig;
@@ -31,7 +31,7 @@ pub struct ScheduleCommitTestContext {
     pub payer_ephem: Keypair,
     // The Payer keypairs along with its PDA pubkey which we'll commit
     pub committees: Vec<(Keypair, Pubkey)>,
-    user_seed: Vec<u8>,
+    user_seed: UserSeeds,
 
     common_ctx: IntegrationTestContext,
 }
@@ -65,18 +65,18 @@ impl ScheduleCommitTestContext {
     // -----------------
     pub fn try_new_random_keys(
         ncommittees: usize,
-        user_seed: &[u8],
+        user_seed: UserSeeds,
     ) -> Result<Self> {
         Self::try_new_internal(ncommittees, true, user_seed)
     }
-    pub fn try_new(ncommittees: usize, user_seed: &[u8]) -> Result<Self> {
+    pub fn try_new(ncommittees: usize, user_seed: UserSeeds) -> Result<Self> {
         Self::try_new_internal(ncommittees, false, user_seed)
     }
 
     fn try_new_internal(
         ncommittees: usize,
         random_keys: bool,
-        user_seed: &[u8],
+        user_seed: UserSeeds,
     ) -> Result<Self> {
         let ictx = IntegrationTestContext::try_new()?;
 
@@ -113,7 +113,7 @@ impl ScheduleCommitTestContext {
                 )
                 .unwrap();
                 let (pda, _bump) = Pubkey::find_program_address(
-                    &[user_seed, payer_ephem.pubkey().as_ref()],
+                    &[user_seed.bytes(), payer_ephem.pubkey().as_ref()],
                     &program_schedulecommit::ID,
                 );
                 (payer_ephem, pda)
@@ -155,7 +155,7 @@ impl ScheduleCommitTestContext {
             payer_ephem,
             committees,
             common_ctx: ictx,
-            user_seed: user_seed.to_vec(),
+            user_seed,
         })
     }
 
@@ -167,8 +167,8 @@ impl ScheduleCommitTestContext {
             ComputeBudgetInstruction::set_compute_unit_limit(1_400_000),
             ComputeBudgetInstruction::set_compute_unit_price(10_000),
         ];
-        match self.user_seed.as_slice() {
-            b"magic_schedule_commit" => {
+        match self.user_seed {
+            UserSeeds::MagicScheduleCommit => {
                 ixs.extend(self.committees.iter().map(
                     |(player, committee)| {
                         init_account_instruction(
@@ -179,7 +179,7 @@ impl ScheduleCommitTestContext {
                     },
                 ));
             }
-            b"order_book" => {
+            UserSeeds::OrderBook => {
                 ixs.extend(self.committees.iter().map(
                     |(book_manager, committee)| {
                         init_order_book_instruction(
@@ -205,9 +205,6 @@ impl ScheduleCommitTestContext {
                 //     },
                 // ));
             }
-            _ => {
-                return Err(anyhow::anyhow!("Unsupported user_seed: {:?} ; expected b\"magic_schedule_commit\" or b\"order_book\"", self.user_seed));
-            }
         };
 
         let mut signers = self
@@ -272,7 +269,7 @@ impl ScheduleCommitTestContext {
                 self.payer_chain.pubkey(),
                 self.ephem_validator_identity,
                 player.pubkey(),
-                &self.user_seed,
+                self.user_seed,
             );
             ixs.push(ix);
         }