From c60188b2d4892aac3e8e47a03197f4d5ea6bc826 Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Wed, 5 Nov 2025 19:50:23 +0530 Subject: [PATCH 1/2] refactor: Rename optimize() -> minimize_tx_size() --- magicblock-committor-service/src/tasks/args_task.rs | 2 +- magicblock-committor-service/src/tasks/buffer_task.rs | 2 +- magicblock-committor-service/src/tasks/mod.rs | 5 +++-- .../src/tasks/task_strategist.rs | 9 +++++---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/magicblock-committor-service/src/tasks/args_task.rs b/magicblock-committor-service/src/tasks/args_task.rs index fdf774a44..353bb7406 100644 --- a/magicblock-committor-service/src/tasks/args_task.rs +++ b/magicblock-committor-service/src/tasks/args_task.rs @@ -84,7 +84,7 @@ impl BaseTask for ArgsTask { } } - fn optimize( + fn minimize_tx_size( self: Box, ) -> Result, Box> { match self.task_type { diff --git a/magicblock-committor-service/src/tasks/buffer_task.rs b/magicblock-committor-service/src/tasks/buffer_task.rs index 994ddd73c..d48a189f7 100644 --- a/magicblock-committor-service/src/tasks/buffer_task.rs +++ b/magicblock-committor-service/src/tasks/buffer_task.rs @@ -69,7 +69,7 @@ impl BaseTask for BufferTask { } /// No further optimizations - fn optimize( + fn minimize_tx_size( self: Box, ) -> Result, Box> { // Since the buffer in BufferTask doesn't contribute to the size of diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 1c26231b0..8766d8937 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -72,8 +72,9 @@ pub trait BaseTask: Send + Sync + DynClone { /// Gets instruction for task execution fn instruction(&self, validator: &Pubkey) -> Instruction; - /// Optimizes Task strategy if possible, otherwise returns itself - fn optimize( + /// Optimize for transaction size so that more instructions can be buddled together in a single + /// transaction. Return Ok(new_tx_optimized_task), else Err(self) if task cannot be optimized. + fn minimize_tx_size( self: Box, ) -> Result, Box>; diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index 4a16db257..da2165b7a 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -53,7 +53,8 @@ impl TaskStrategist { persistor: &Option

, ) -> TaskStrategistResult { // Attempt optimizing tasks themselves(using buffers) - if Self::optimize_strategy(&mut tasks)? <= MAX_ENCODED_TRANSACTION_SIZE + if Self::minimize_tx_size_if_needed(&mut tasks)? + <= MAX_ENCODED_TRANSACTION_SIZE { // Persist tasks strategy if let Some(persistor) = persistor { @@ -152,7 +153,7 @@ impl TaskStrategist { /// Optimizes set of [`TaskDeliveryStrategy`] to fit [`MAX_ENCODED_TRANSACTION_SIZE`] /// Returns size of tx after optimizations - fn optimize_strategy( + fn minimize_tx_size_if_needed( tasks: &mut [Box], ) -> Result { // Get initial transaction size @@ -207,7 +208,7 @@ impl TaskStrategist { let tmp_task = Box::new(tmp_task) as Box; std::mem::replace(&mut tasks[index], tmp_task) }; - match task.optimize() { + match task.minimize_tx_size() { // If we can decrease: // 1. Calculate new tx size & ix size // 2. Insert item's data back in the heap @@ -456,7 +457,7 @@ mod tests { Box::new(create_test_commit_task(3, 1000)) as Box, // Larger task ]; - let _ = TaskStrategist::optimize_strategy(&mut tasks); + let _ = TaskStrategist::minimize_tx_size_if_needed(&mut tasks); // The larger task should have been optimized first assert!(matches!(tasks[0].strategy(), TaskStrategy::Args)); assert!(matches!(tasks[1].strategy(), TaskStrategy::Buffer)); From 53427bba3737823add8b7339976dac8e967938ee Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Thu, 6 Nov 2025 23:09:55 +0530 Subject: [PATCH 2/2] Address @taco-paco's feedback --- .../src/tasks/args_task.rs | 2 +- .../src/tasks/buffer_task.rs | 2 +- magicblock-committor-service/src/tasks/mod.rs | 2 +- .../src/tasks/task_strategist.rs | 14 ++++++++------ 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/magicblock-committor-service/src/tasks/args_task.rs b/magicblock-committor-service/src/tasks/args_task.rs index 353bb7406..a2a0342ec 100644 --- a/magicblock-committor-service/src/tasks/args_task.rs +++ b/magicblock-committor-service/src/tasks/args_task.rs @@ -84,7 +84,7 @@ impl BaseTask for ArgsTask { } } - fn minimize_tx_size( + fn try_optimize_tx_size( self: Box, ) -> Result, Box> { match self.task_type { diff --git a/magicblock-committor-service/src/tasks/buffer_task.rs b/magicblock-committor-service/src/tasks/buffer_task.rs index d48a189f7..caeb77f50 100644 --- a/magicblock-committor-service/src/tasks/buffer_task.rs +++ b/magicblock-committor-service/src/tasks/buffer_task.rs @@ -69,7 +69,7 @@ impl BaseTask for BufferTask { } /// No further optimizations - fn minimize_tx_size( + fn try_optimize_tx_size( self: Box, ) -> Result, Box> { // Since the buffer in BufferTask doesn't contribute to the size of diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 8766d8937..d9c5ca863 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -74,7 +74,7 @@ pub trait BaseTask: Send + Sync + DynClone { /// Optimize for transaction size so that more instructions can be buddled together in a single /// transaction. Return Ok(new_tx_optimized_task), else Err(self) if task cannot be optimized. - fn minimize_tx_size( + fn try_optimize_tx_size( self: Box, ) -> Result, Box>; diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index da2165b7a..a29f19927 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -53,7 +53,7 @@ impl TaskStrategist { persistor: &Option

, ) -> TaskStrategistResult { // Attempt optimizing tasks themselves(using buffers) - if Self::minimize_tx_size_if_needed(&mut tasks)? + if Self::try_optimize_tx_size_if_needed(&mut tasks)? <= MAX_ENCODED_TRANSACTION_SIZE { // Persist tasks strategy @@ -151,9 +151,11 @@ impl TaskStrategist { ) } - /// Optimizes set of [`TaskDeliveryStrategy`] to fit [`MAX_ENCODED_TRANSACTION_SIZE`] - /// Returns size of tx after optimizations - fn minimize_tx_size_if_needed( + /// Optimizes tasks so as to bring the transaction size within the limit [`MAX_ENCODED_TRANSACTION_SIZE`] + /// Returns Ok(size of tx after optimizations) else Err(SignerError). + /// Note that the returned size, though possibly optimized one, may still not be under + /// the limit MAX_ENCODED_TRANSACTION_SIZE. The caller needs to check and make decision accordingly. + fn try_optimize_tx_size_if_needed( tasks: &mut [Box], ) -> Result { // Get initial transaction size @@ -208,7 +210,7 @@ impl TaskStrategist { let tmp_task = Box::new(tmp_task) as Box; std::mem::replace(&mut tasks[index], tmp_task) }; - match task.minimize_tx_size() { + match task.try_optimize_tx_size() { // If we can decrease: // 1. Calculate new tx size & ix size // 2. Insert item's data back in the heap @@ -457,7 +459,7 @@ mod tests { Box::new(create_test_commit_task(3, 1000)) as Box, // Larger task ]; - let _ = TaskStrategist::minimize_tx_size_if_needed(&mut tasks); + let _ = TaskStrategist::try_optimize_tx_size_if_needed(&mut tasks); // The larger task should have been optimized first assert!(matches!(tasks[0].strategy(), TaskStrategy::Args)); assert!(matches!(tasks[1].strategy(), TaskStrategy::Buffer));