From 69705160f0309824ba2e5b48c974de47c4c256c8 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 3a3f643be..502a2031b 100644 --- a/magicblock-committor-service/src/tasks/args_task.rs +++ b/magicblock-committor-service/src/tasks/args_task.rs @@ -122,7 +122,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 324accedb..b3d8f4e38 100644 --- a/magicblock-committor-service/src/tasks/buffer_task.rs +++ b/magicblock-committor-service/src/tasks/buffer_task.rs @@ -146,7 +146,7 @@ impl BaseTask for BufferTask { } /// No further optimizations - fn optimize( + fn minimize_tx_size( self: Box, ) -> Result, Box> { Err(self) diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 1719b84ae..5eab63ff7 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -66,8 +66,9 @@ pub trait BaseTask: Send + Sync + DynClone + LabelValue { /// 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 edbbd74d3..271520072 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -171,7 +171,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 { @@ -270,7 +271,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 @@ -325,7 +326,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 @@ -704,7 +705,7 @@ mod tests { Box::new(create_test_commit_task(3, 1000, 0)) 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 99dc29b027d17b643f2cab74647774bd1a009c0f 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 502a2031b..1aec8326e 100644 --- a/magicblock-committor-service/src/tasks/args_task.rs +++ b/magicblock-committor-service/src/tasks/args_task.rs @@ -122,7 +122,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 b3d8f4e38..19c59b7ac 100644 --- a/magicblock-committor-service/src/tasks/buffer_task.rs +++ b/magicblock-committor-service/src/tasks/buffer_task.rs @@ -146,7 +146,7 @@ impl BaseTask for BufferTask { } /// No further optimizations - fn minimize_tx_size( + fn try_optimize_tx_size( self: Box, ) -> Result, Box> { Err(self) diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 5eab63ff7..d8b4a178e 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -68,7 +68,7 @@ pub trait BaseTask: Send + Sync + DynClone + LabelValue { /// 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 271520072..33bbd29e4 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -171,7 +171,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 @@ -269,9 +269,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 @@ -326,7 +328,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 @@ -705,7 +707,7 @@ mod tests { Box::new(create_test_commit_task(3, 1000, 0)) 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));