Skip to content

Commit adb36a5

Browse files
snawazsnawaz
committed
Remove CommitTaskBuilder
1 parent 54a1a4c commit adb36a5

File tree

5 files changed

+64
-64
lines changed

5 files changed

+64
-64
lines changed

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

Lines changed: 11 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -110,54 +110,6 @@ pub trait BaseTask: Send + Sync + DynClone + LabelValue {
110110

111111
dyn_clone::clone_trait_object!(BaseTask);
112112

113-
pub struct CommitTaskBuilder;
114-
115-
impl CommitTaskBuilder {
116-
// Accounts larger than COMMIT_STATE_SIZE_THRESHOLD, use CommitDiff to
117-
// reduce instruction size. Below this, commit is sent as CommitState.
118-
// Chose 256 as thresold seems good enough as it could hold 8 u32 fields
119-
// or 4 u64 fields.
120-
const COMMIT_STATE_SIZE_THRESHOLD: usize = 256;
121-
122-
pub async fn create_commit_task<C: TaskInfoFetcher>(
123-
commit_id: u64,
124-
allow_undelegation: bool,
125-
committed_account: CommittedAccount,
126-
task_info_fetcher: &Arc<C>,
127-
) -> CommitTask {
128-
let base_account = if committed_account.account.data.len()
129-
> CommitTaskBuilder::COMMIT_STATE_SIZE_THRESHOLD
130-
{
131-
match task_info_fetcher
132-
.get_base_account(&committed_account.pubkey)
133-
.await
134-
{
135-
Ok(Some(account)) => Some(account),
136-
Ok(None) => {
137-
log::warn!("AccountNotFound for commit_diff, pubkey: {}, commit_id: {}, Falling back to commit_state.",
138-
committed_account.pubkey, commit_id);
139-
None
140-
}
141-
Err(e) => {
142-
log::warn!("Failed to fetch base account for commit diff, pubkey: {}, commit_id: {}, error: {}. Falling back to commit_state.",
143-
committed_account.pubkey, commit_id, e);
144-
None
145-
}
146-
}
147-
} else {
148-
None
149-
};
150-
151-
CommitTask {
152-
commit_id,
153-
allow_undelegation,
154-
committed_account,
155-
base_account,
156-
force_commit_state: false,
157-
}
158-
}
159-
}
160-
161113
#[derive(Clone)]
162114
pub struct CommitTask {
163115
pub commit_id: u64,
@@ -168,10 +120,16 @@ pub struct CommitTask {
168120
}
169121

170122
impl CommitTask {
123+
// Accounts larger than COMMIT_STATE_SIZE_THRESHOLD, use CommitDiff to
124+
// reduce instruction size. Below this, commit is sent as CommitState.
125+
// Chose 256 as thresold seems good enough as it could hold 8 u32 fields
126+
// or 4 u64 fields.
127+
const COMMIT_STATE_SIZE_THRESHOLD: usize = 256;
128+
171129
pub fn is_commit_diff(&self) -> bool {
172130
!self.force_commit_state
173131
&& self.committed_account.account.data.len()
174-
> CommitTaskBuilder::COMMIT_STATE_SIZE_THRESHOLD
132+
> Self::COMMIT_STATE_SIZE_THRESHOLD
175133
&& self.base_account.is_some()
176134
}
177135

@@ -428,6 +386,7 @@ mod serialization_safety_test {
428386
tasks::{
429387
args_task::{ArgsTask, ArgsTaskType},
430388
buffer_task::{BufferTask, BufferTaskType},
389+
task_builder::TaskBuilderImpl,
431390
*,
432391
},
433392
};
@@ -439,7 +398,7 @@ mod serialization_safety_test {
439398

440399
// Test Commit variant
441400
let commit_task: ArgsTask = ArgsTaskType::Commit(
442-
CommitTaskBuilder::create_commit_task(
401+
TaskBuilderImpl::create_commit_task(
443402
123,
444403
true,
445404
CommittedAccount {
@@ -503,7 +462,7 @@ mod serialization_safety_test {
503462

504463
let buffer_task =
505464
BufferTask::new_preparation_required(BufferTaskType::Commit(
506-
CommitTaskBuilder::create_commit_task(
465+
TaskBuilderImpl::create_commit_task(
507466
456,
508467
false,
509468
CommittedAccount {
@@ -531,7 +490,7 @@ mod serialization_safety_test {
531490
// Test BufferTask preparation
532491
let buffer_task =
533492
BufferTask::new_preparation_required(BufferTaskType::Commit(
534-
CommitTaskBuilder::create_commit_task(
493+
TaskBuilderImpl::create_commit_task(
535494
789,
536495
true,
537496
CommittedAccount {

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ use crate::{
1717
persist::IntentPersister,
1818
tasks::{
1919
args_task::{ArgsTask, ArgsTaskType},
20-
BaseActionTask, BaseTask, CommitTaskBuilder, FinalizeTask,
21-
UndelegateTask,
20+
BaseActionTask, BaseTask, FinalizeTask, UndelegateTask,
2221
},
2322
};
2423

24+
use super::CommitTask;
25+
2526
#[async_trait]
2627
pub trait TasksBuilder {
2728
// Creates tasks for commit stage
@@ -42,6 +43,46 @@ pub trait TasksBuilder {
4243
/// V1: Actions are part of finalize tx
4344
pub struct TaskBuilderImpl;
4445

46+
impl TaskBuilderImpl {
47+
pub async fn create_commit_task<C: TaskInfoFetcher>(
48+
commit_id: u64,
49+
allow_undelegation: bool,
50+
committed_account: CommittedAccount,
51+
task_info_fetcher: &Arc<C>,
52+
) -> CommitTask {
53+
let base_account = if committed_account.account.data.len()
54+
> CommitTask::COMMIT_STATE_SIZE_THRESHOLD
55+
{
56+
match task_info_fetcher
57+
.get_base_account(&committed_account.pubkey)
58+
.await
59+
{
60+
Ok(Some(account)) => Some(account),
61+
Ok(None) => {
62+
log::warn!("AccountNotFound for commit_diff, pubkey: {}, commit_id: {}, Falling back to commit_state.",
63+
committed_account.pubkey, commit_id);
64+
None
65+
}
66+
Err(e) => {
67+
log::warn!("Failed to fetch base account for commit diff, pubkey: {}, commit_id: {}, error: {}. Falling back to commit_state.",
68+
committed_account.pubkey, commit_id, e);
69+
None
70+
}
71+
}
72+
} else {
73+
None
74+
};
75+
76+
CommitTask {
77+
commit_id,
78+
allow_undelegation,
79+
committed_account,
80+
base_account,
81+
force_commit_state: false,
82+
}
83+
}
84+
}
85+
4586
#[async_trait]
4687
impl TasksBuilder for TaskBuilderImpl {
4788
/// Returns [`Task`]s for Commit stage
@@ -92,7 +133,7 @@ impl TasksBuilder for TaskBuilderImpl {
92133
.iter()
93134
.map(|account| async {
94135
let commit_id = *commit_ids.get(&account.pubkey).expect("CommitIdFetcher provide commit ids for all listed pubkeys, or errors!");
95-
let task = ArgsTaskType::Commit(CommitTaskBuilder::create_commit_task(
136+
let task = ArgsTaskType::Commit(Self::create_commit_task(
96137
commit_id,
97138
allow_undelegation,
98139
account.clone(),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ mod tests {
424424
data_size: usize,
425425
) -> ArgsTask {
426426
ArgsTask::new(ArgsTaskType::Commit(
427-
CommitTaskBuilder::create_commit_task(
427+
TaskBuilderImpl::create_commit_task(
428428
commit_id,
429429
false,
430430
CommittedAccount {

‎test-integration/test-committor-service/tests/common.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use magicblock_committor_service::{
1414
},
1515
IntentExecutorImpl, NullTaskInfoFetcher,
1616
},
17-
tasks::{CommitTask, CommitTaskBuilder},
17+
tasks::{CommitTask, TaskBuilderImpl},
1818
transaction_preparator::{
1919
delivery_preparator::DeliveryPreparator, TransactionPreparatorImpl,
2020
},
@@ -161,7 +161,7 @@ pub fn generate_random_bytes(length: usize) -> Vec<u8> {
161161
#[allow(dead_code)]
162162
pub async fn create_commit_task(data: &[u8]) -> CommitTask {
163163
static COMMIT_ID: AtomicU64 = AtomicU64::new(0);
164-
CommitTaskBuilder::create_commit_task(
164+
TaskBuilderImpl::create_commit_task(
165165
COMMIT_ID.fetch_add(1, Ordering::Relaxed),
166166
false,
167167
CommittedAccount {

‎test-integration/test-committor-service/tests/test_transaction_preparator.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use magicblock_committor_service::{
77
buffer_task::{BufferTask, BufferTaskType},
88
task_strategist::{TaskStrategist, TransactionStrategy},
99
utils::TransactionUtils,
10-
BaseActionTask, BaseTask, CommitTaskBuilder, FinalizeTask,
11-
PreparationState, UndelegateTask,
10+
BaseActionTask, BaseTask, FinalizeTask, PreparationState,
11+
TaskBuilderImpl, UndelegateTask,
1212
},
1313
transaction_preparator::TransactionPreparator,
1414
};
@@ -36,7 +36,7 @@ async fn test_prepare_commit_tx_with_single_account() {
3636

3737
let tasks = vec![
3838
Box::new(ArgsTask::new(ArgsTaskType::Commit(
39-
CommitTaskBuilder::create_commit_task(
39+
TaskBuilderImpl::create_commit_task(
4040
1,
4141
true,
4242
committed_account.clone(),
@@ -95,7 +95,7 @@ async fn test_prepare_commit_tx_with_multiple_accounts() {
9595

9696
let buffer_commit_task =
9797
BufferTask::new_preparation_required(BufferTaskType::Commit(
98-
CommitTaskBuilder::create_commit_task(
98+
TaskBuilderImpl::create_commit_task(
9999
1,
100100
true,
101101
committed_account2.clone(),
@@ -107,7 +107,7 @@ async fn test_prepare_commit_tx_with_multiple_accounts() {
107107
let tasks = vec![
108108
// account 1
109109
Box::new(ArgsTask::new(ArgsTaskType::Commit(
110-
CommitTaskBuilder::create_commit_task(
110+
TaskBuilderImpl::create_commit_task(
111111
1,
112112
true,
113113
committed_account1.clone(),
@@ -199,7 +199,7 @@ async fn test_prepare_commit_tx_with_base_actions() {
199199

200200
let buffer_commit_task =
201201
BufferTask::new_preparation_required(BufferTaskType::Commit(
202-
CommitTaskBuilder::create_commit_task(
202+
TaskBuilderImpl::create_commit_task(
203203
1,
204204
true,
205205
committed_account.clone(),

0 commit comments

Comments
 (0)