From a58c61d33116c213c3bcf85aa15eda3fea0e970a Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Wed, 5 Nov 2025 11:30:12 +0530 Subject: [PATCH] feat: Make TaskStrategy more granular --- .../src/persist/commit_persister.rs | 4 +- .../src/persist/db.rs | 4 +- .../src/persist/types/commit_strategy.rs | 34 +++++----- .../tasks/task_visitors/persistor_visitor.rs | 8 +-- .../tests/test_ix_commit_local.rs | 64 ++++++++++--------- 5 files changed, 61 insertions(+), 53 deletions(-) diff --git a/magicblock-committor-service/src/persist/commit_persister.rs b/magicblock-committor-service/src/persist/commit_persister.rs index 3df48997f..739972592 100644 --- a/magicblock-committor-service/src/persist/commit_persister.rs +++ b/magicblock-committor-service/src/persist/commit_persister.rs @@ -584,14 +584,14 @@ mod tests { persister.set_commit_id(1, &pubkey, 100).unwrap(); persister - .set_commit_strategy(100, &pubkey, CommitStrategy::Args) + .set_commit_strategy(100, &pubkey, CommitStrategy::StateArgs) .unwrap(); let updated = persister .get_commit_status_by_message(1, &pubkey) .unwrap() .unwrap(); - assert_eq!(updated.commit_strategy, CommitStrategy::Args); + assert_eq!(updated.commit_strategy, CommitStrategy::StateArgs); } #[test] diff --git a/magicblock-committor-service/src/persist/db.rs b/magicblock-committor-service/src/persist/db.rs index 15d68c0a8..b19526931 100644 --- a/magicblock-committor-service/src/persist/db.rs +++ b/magicblock-committor-service/src/persist/db.rs @@ -769,7 +769,7 @@ mod tests { commit_type: CommitType::DataAccount, created_at: 1000, commit_status: CommitStatus::Pending, - commit_strategy: CommitStrategy::Args, + commit_strategy: CommitStrategy::StateArgs, last_retried_at: 1000, retries_count: 0, } @@ -900,7 +900,7 @@ mod tests { let row = create_test_row(1, 100); db.insert_commit_status_rows(&[row.clone()]).unwrap(); - let new_strategy = CommitStrategy::FromBuffer; + let new_strategy = CommitStrategy::StateBuffer; db.set_commit_strategy(100, &row.pubkey, new_strategy) .unwrap(); diff --git a/magicblock-committor-service/src/persist/types/commit_strategy.rs b/magicblock-committor-service/src/persist/types/commit_strategy.rs index c9261f70d..1485f2949 100644 --- a/magicblock-committor-service/src/persist/types/commit_strategy.rs +++ b/magicblock-committor-service/src/persist/types/commit_strategy.rs @@ -4,39 +4,39 @@ use crate::persist::error::CommitPersistError; pub enum CommitStrategy { /// Args without the use of a lookup table #[default] - Args, + StateArgs, /// Args with the use of a lookup table - ArgsWithLookupTable, + StateArgsWithLookupTable, /// Buffer and chunks which has the most overhead - FromBuffer, + StateBuffer, /// Buffer and chunks with the use of a lookup table - FromBufferWithLookupTable, + StateBufferWithLookupTable, } impl CommitStrategy { pub fn args(use_lookup: bool) -> Self { if use_lookup { - Self::ArgsWithLookupTable + Self::StateArgsWithLookupTable } else { - Self::Args + Self::StateArgs } } pub fn as_str(&self) -> &str { use CommitStrategy::*; match self { - Args => "Args", - ArgsWithLookupTable => "ArgsWithLookupTable", - FromBuffer => "FromBuffer", - FromBufferWithLookupTable => "FromBufferWithLookupTable", + StateArgs => "StateArgs", + StateArgsWithLookupTable => "StageArgsWithLookupTable", + StateBuffer => "StageBuffer", + StateBufferWithLookupTable => "StageBufferWithLookupTable", } } pub fn uses_lookup(&self) -> bool { matches!( self, - CommitStrategy::ArgsWithLookupTable - | CommitStrategy::FromBufferWithLookupTable + CommitStrategy::StateArgsWithLookupTable + | CommitStrategy::StateBufferWithLookupTable ) } } @@ -45,10 +45,12 @@ impl TryFrom<&str> for CommitStrategy { type Error = CommitPersistError; fn try_from(value: &str) -> Result { match value { - "Args" => Ok(Self::Args), - "ArgsWithLookupTable" => Ok(Self::ArgsWithLookupTable), - "FromBuffer" => Ok(Self::FromBuffer), - "FromBufferWithLookupTable" => Ok(Self::FromBufferWithLookupTable), + "StateArgs" => Ok(Self::StateArgs), + "StateArgsWithLookupTable" => Ok(Self::StateArgsWithLookupTable), + "StageBuffer" => Ok(Self::StateBuffer), + "StageBufferWithLookupTable" => { + Ok(Self::StateBufferWithLookupTable) + } _ => Err(CommitPersistError::InvalidCommitStrategy( value.to_string(), )), diff --git a/magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs b/magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs index c608f2ef9..5aac14fe7 100644 --- a/magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs +++ b/magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs @@ -32,9 +32,9 @@ where }; let commit_strategy = if uses_lookup_tables { - CommitStrategy::ArgsWithLookupTable + CommitStrategy::StateArgsWithLookupTable } else { - CommitStrategy::Args + CommitStrategy::StateArgs }; if let Err(err) = self.persistor.set_commit_strategy( @@ -57,9 +57,9 @@ where PersistorContext::PersistStrategy { uses_lookup_tables } => { let BufferTaskType::Commit(ref commit_task) = task.task_type; let commit_strategy = if uses_lookup_tables { - CommitStrategy::FromBufferWithLookupTable + CommitStrategy::StateBufferWithLookupTable } else { - CommitStrategy::FromBuffer + CommitStrategy::StateBuffer }; if let Err(err) = self.persistor.set_commit_strategy( diff --git a/test-integration/test-committor-service/tests/test_ix_commit_local.rs b/test-integration/test-committor-service/tests/test_ix_commit_local.rs index 68746d1c0..2f8459803 100644 --- a/test-integration/test-committor-service/tests/test_ix_commit_local.rs +++ b/test-integration/test-committor-service/tests/test_ix_commit_local.rs @@ -64,32 +64,32 @@ fn expect_strategies( #[tokio::test] async fn test_ix_commit_single_account_100_bytes() { - commit_single_account(100, CommitStrategy::Args, false).await; + commit_single_account(100, CommitStrategy::StateArgs, false).await; } #[tokio::test] async fn test_ix_commit_single_account_100_bytes_and_undelegate() { - commit_single_account(100, CommitStrategy::Args, true).await; + commit_single_account(100, CommitStrategy::StateArgs, true).await; } #[tokio::test] async fn test_ix_commit_single_account_800_bytes() { - commit_single_account(800, CommitStrategy::FromBuffer, false).await; + commit_single_account(800, CommitStrategy::StateBuffer, false).await; } #[tokio::test] async fn test_ix_commit_single_account_800_bytes_and_undelegate() { - commit_single_account(800, CommitStrategy::FromBuffer, true).await; + commit_single_account(800, CommitStrategy::StateBuffer, true).await; } #[tokio::test] async fn test_ix_commit_single_account_one_kb() { - commit_single_account(1024, CommitStrategy::FromBuffer, false).await; + commit_single_account(1024, CommitStrategy::StateBuffer, false).await; } #[tokio::test] async fn test_ix_commit_single_account_ten_kb() { - commit_single_account(10 * 1024, CommitStrategy::FromBuffer, false).await; + commit_single_account(10 * 1024, CommitStrategy::StateBuffer, false).await; } async fn commit_single_account( @@ -162,7 +162,7 @@ async fn test_ix_commit_two_accounts_1kb_2kb() { &[1024, 2048], 1, false, - expect_strategies(&[(CommitStrategy::Args, 2)]), + expect_strategies(&[(CommitStrategy::StateArgs, 2)]), ) .await; } @@ -174,7 +174,7 @@ async fn test_ix_commit_two_accounts_512kb() { &[512, 512], 1, false, - expect_strategies(&[(CommitStrategy::Args, 2)]), + expect_strategies(&[(CommitStrategy::StateArgs, 2)]), ) .await; } @@ -186,7 +186,7 @@ async fn test_ix_commit_three_accounts_512kb() { &[512, 512, 512], 1, false, - expect_strategies(&[(CommitStrategy::Args, 3)]), + expect_strategies(&[(CommitStrategy::StateArgs, 3)]), ) .await; } @@ -198,7 +198,7 @@ async fn test_ix_commit_six_accounts_512kb() { &[512, 512, 512, 512, 512, 512], 1, false, - expect_strategies(&[(CommitStrategy::Args, 6)]), + expect_strategies(&[(CommitStrategy::StateArgs, 6)]), ) .await; } @@ -210,22 +210,25 @@ async fn test_ix_commit_four_accounts_1kb_2kb_5kb_10kb_single_bundle() { &[1024, 2 * 1024, 5 * 1024, 10 * 1024], 1, false, - expect_strategies(&[(CommitStrategy::Args, 4)]), + expect_strategies(&[(CommitStrategy::StateArgs, 4)]), ) .await; } #[tokio::test] async fn test_commit_20_accounts_1kb_bundle_size_2() { - commit_20_accounts_1kb(2, expect_strategies(&[(CommitStrategy::Args, 20)])) - .await; + commit_20_accounts_1kb( + 2, + expect_strategies(&[(CommitStrategy::StateArgs, 20)]), + ) + .await; } #[tokio::test] async fn test_commit_5_accounts_1kb_bundle_size_3() { commit_5_accounts_1kb( 3, - expect_strategies(&[(CommitStrategy::Args, 5)]), + expect_strategies(&[(CommitStrategy::StateArgs, 5)]), false, ) .await; @@ -237,8 +240,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_3_undelegate_all() { 3, expect_strategies(&[ // Intent fits in 1 TX only with ALT, see IntentExecutorImpl::try_unite_tasks - (CommitStrategy::FromBufferWithLookupTable, 3), - (CommitStrategy::Args, 2), + (CommitStrategy::StateBufferWithLookupTable, 3), + (CommitStrategy::StateArgs, 2), ]), true, ) @@ -250,8 +253,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_4() { commit_5_accounts_1kb( 4, expect_strategies(&[ - (CommitStrategy::Args, 1), - (CommitStrategy::FromBufferWithLookupTable, 4), + (CommitStrategy::StateArgs, 1), + (CommitStrategy::StateBufferWithLookupTable, 4), ]), false, ) @@ -263,8 +266,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_4_undelegate_all() { commit_5_accounts_1kb( 4, expect_strategies(&[ - (CommitStrategy::Args, 1), - (CommitStrategy::FromBufferWithLookupTable, 4), + (CommitStrategy::StateArgs, 1), + (CommitStrategy::StateBufferWithLookupTable, 4), ]), true, ) @@ -275,7 +278,7 @@ async fn test_commit_5_accounts_1kb_bundle_size_4_undelegate_all() { async fn test_commit_5_accounts_1kb_bundle_size_5_undelegate_all() { commit_5_accounts_1kb( 5, - expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 5)]), + expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 5)]), true, ) .await; @@ -283,15 +286,18 @@ async fn test_commit_5_accounts_1kb_bundle_size_5_undelegate_all() { #[tokio::test] async fn test_commit_20_accounts_1kb_bundle_size_3() { - commit_20_accounts_1kb(3, expect_strategies(&[(CommitStrategy::Args, 20)])) - .await; + commit_20_accounts_1kb( + 3, + expect_strategies(&[(CommitStrategy::StateArgs, 20)]), + ) + .await; } #[tokio::test] async fn test_commit_20_accounts_1kb_bundle_size_4() { commit_20_accounts_1kb( 4, - expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 20)]), + expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 20)]), ) .await; } @@ -301,9 +307,9 @@ async fn test_commit_20_accounts_1kb_bundle_size_6() { commit_20_accounts_1kb( 6, expect_strategies(&[ - (CommitStrategy::FromBufferWithLookupTable, 18), + (CommitStrategy::StateBufferWithLookupTable, 18), // Two accounts don't make it into the bundles of size 6 - (CommitStrategy::Args, 2), + (CommitStrategy::StateArgs, 2), ]), ) .await; @@ -313,7 +319,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_6() { async fn test_commit_20_accounts_1kb_bundle_size_20() { commit_20_accounts_1kb( 20, - expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 20)]), + expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 20)]), ) .await; } @@ -325,7 +331,7 @@ async fn test_commit_8_accounts_1kb_bundle_size_8() { expect_strategies(&[ // Four accounts don't make it into the bundles of size 8, but // that bundle also needs lookup tables - (CommitStrategy::FromBufferWithLookupTable, 8), + (CommitStrategy::StateBufferWithLookupTable, 8), ]), ) .await; @@ -338,7 +344,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_8() { expect_strategies(&[ // Four accounts don't make it into the bundles of size 8, but // that bundle also needs lookup tables - (CommitStrategy::FromBufferWithLookupTable, 20), + (CommitStrategy::StateBufferWithLookupTable, 20), ]), ) .await;