Skip to content

Commit 632a4f9

Browse files
committed
feat: Execute CommitDiff as BufferTask
1 parent 9f09261 commit 632a4f9

File tree

7 files changed

+188
-94
lines changed

7 files changed

+188
-94
lines changed

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,8 @@ impl BaseTask for ArgsTask {
132132
)))
133133
}
134134
ArgsTaskType::CommitDiff(value) => {
135-
// TODO (snawaz): Currently, we do not support executing CommitDiff
136-
// as BufferTask, which is why we're forcing CommitDiffTask to become CommitTask
137-
// before converting this task into BufferTask. Once CommitDiff is supported
138-
// by BufferTask, we do not have to do this, as it's essentially a downgrade.
139135
Ok(Box::new(BufferTask::new_preparation_required(
140-
BufferTaskType::Commit(CommitTask {
141-
commit_id: value.commit_id,
142-
allow_undelegation: value.allow_undelegation,
143-
committed_account: value.committed_account,
144-
}),
136+
BufferTaskType::CommitDiff(value),
145137
)))
146138
}
147139
ArgsTaskType::BaseAction(_)

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

Lines changed: 101 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use dlp::args::CommitStateFromBufferArgs;
1+
use dlp::{args::CommitStateFromBufferArgs, compute_diff};
22
use magicblock_committor_program::Chunks;
33
use magicblock_metrics::metrics::LabelValue;
44
use solana_instruction::Instruction;
@@ -11,15 +11,17 @@ use crate::tasks::TaskStrategy;
1111
use crate::{
1212
consts::MAX_WRITE_CHUNK_SIZE,
1313
tasks::{
14-
visitor::Visitor, BaseTask, BaseTaskError, BaseTaskResult, CommitTask,
15-
PreparationState, PreparationTask, TaskType,
14+
visitor::Visitor, BaseTask, BaseTaskError, BaseTaskResult,
15+
CommitDiffTask, CommitTask, PreparationState, PreparationTask,
16+
TaskType,
1617
},
1718
};
1819

1920
/// Tasks that could be executed using buffers
2021
#[derive(Clone)]
2122
pub enum BufferTaskType {
2223
Commit(CommitTask),
24+
CommitDiff(CommitDiffTask),
2325
// Action in the future
2426
}
2527

@@ -48,19 +50,37 @@ impl BufferTask {
4850
}
4951

5052
fn preparation_required(task_type: &BufferTaskType) -> PreparationState {
51-
let BufferTaskType::Commit(ref commit_task) = task_type;
52-
let committed_data = commit_task.committed_account.account.data.clone();
53-
let chunks = Chunks::from_data_length(
54-
committed_data.len(),
55-
MAX_WRITE_CHUNK_SIZE,
56-
);
57-
58-
PreparationState::Required(PreparationTask {
59-
commit_id: commit_task.commit_id,
60-
pubkey: commit_task.committed_account.pubkey,
61-
committed_data,
62-
chunks,
63-
})
53+
match task_type {
54+
BufferTaskType::Commit(task) => {
55+
let data = task.committed_account.account.data.clone();
56+
let chunks =
57+
Chunks::from_data_length(data.len(), MAX_WRITE_CHUNK_SIZE);
58+
59+
PreparationState::Required(PreparationTask {
60+
commit_id: task.commit_id,
61+
pubkey: task.committed_account.pubkey,
62+
committed_data: data,
63+
chunks,
64+
})
65+
}
66+
67+
BufferTaskType::CommitDiff(task) => {
68+
let diff = compute_diff(
69+
&task.base_account.data,
70+
&task.committed_account.account.data,
71+
)
72+
.to_vec();
73+
let chunks =
74+
Chunks::from_data_length(diff.len(), MAX_WRITE_CHUNK_SIZE);
75+
76+
PreparationState::Required(PreparationTask {
77+
commit_id: task.commit_id,
78+
pubkey: task.committed_account.pubkey,
79+
committed_data: diff,
80+
chunks,
81+
})
82+
}
83+
}
6484
}
6585
}
6686

@@ -69,34 +89,60 @@ impl From<ArgsTaskType> for BufferTaskType {
6989
fn from(value: ArgsTaskType) -> Self {
7090
match value {
7191
ArgsTaskType::Commit(task) => BufferTaskType::Commit(task),
72-
ArgsTaskType::CommitDiff(_) => panic!("BufferTask doesn't support CommitDiff yet. Disable your tests (if any) temporarily till the next PR"),
73-
_ => unimplemented!("Only commit task can be BufferTask currently. Fix your tests"),
92+
ArgsTaskType::CommitDiff(task) => BufferTaskType::CommitDiff(task),
93+
_ => unimplemented!(
94+
"Only commit task can be BufferTask currently. Fix your tests"
95+
),
7496
}
7597
}
7698
}
7799

78100
impl BaseTask for BufferTask {
79101
fn instruction(&self, validator: &Pubkey) -> Instruction {
80-
let BufferTaskType::Commit(ref value) = self.task_type;
81-
let commit_id_slice = value.commit_id.to_le_bytes();
82-
let (commit_buffer_pubkey, _) =
83-
magicblock_committor_program::pdas::buffer_pda(
84-
validator,
85-
&value.committed_account.pubkey,
86-
&commit_id_slice,
87-
);
88-
89-
dlp::instruction_builder::commit_state_from_buffer(
90-
*validator,
91-
value.committed_account.pubkey,
92-
value.committed_account.account.owner,
93-
commit_buffer_pubkey,
94-
CommitStateFromBufferArgs {
95-
nonce: value.commit_id,
96-
lamports: value.committed_account.account.lamports,
97-
allow_undelegation: value.allow_undelegation,
98-
},
99-
)
102+
match &self.task_type {
103+
BufferTaskType::Commit(task) => {
104+
let commit_id_slice = task.commit_id.to_le_bytes();
105+
let (commit_buffer_pubkey, _) =
106+
magicblock_committor_program::pdas::buffer_pda(
107+
validator,
108+
&task.committed_account.pubkey,
109+
&commit_id_slice,
110+
);
111+
112+
dlp::instruction_builder::commit_state_from_buffer(
113+
*validator,
114+
task.committed_account.pubkey,
115+
task.committed_account.account.owner,
116+
commit_buffer_pubkey,
117+
CommitStateFromBufferArgs {
118+
nonce: task.commit_id,
119+
lamports: task.committed_account.account.lamports,
120+
allow_undelegation: task.allow_undelegation,
121+
},
122+
)
123+
}
124+
BufferTaskType::CommitDiff(task) => {
125+
let commit_id_slice = task.commit_id.to_le_bytes();
126+
let (commit_buffer_pubkey, _) =
127+
magicblock_committor_program::pdas::buffer_pda(
128+
validator,
129+
&task.committed_account.pubkey,
130+
&commit_id_slice,
131+
);
132+
133+
dlp::instruction_builder::commit_diff_from_buffer(
134+
*validator,
135+
task.committed_account.pubkey,
136+
task.committed_account.account.owner,
137+
commit_buffer_pubkey,
138+
CommitStateFromBufferArgs {
139+
nonce: task.commit_id,
140+
lamports: task.committed_account.account.lamports,
141+
allow_undelegation: task.allow_undelegation,
142+
},
143+
)
144+
}
145+
}
100146
}
101147

102148
/// No further optimizations
@@ -125,6 +171,7 @@ impl BaseTask for BufferTask {
125171
fn compute_units(&self) -> u32 {
126172
match self.task_type {
127173
BufferTaskType::Commit(_) => 70_000,
174+
BufferTaskType::CommitDiff(_) => 70_000,
128175
}
129176
}
130177

@@ -136,6 +183,7 @@ impl BaseTask for BufferTask {
136183
fn task_type(&self) -> TaskType {
137184
match self.task_type {
138185
BufferTaskType::Commit(_) => TaskType::Commit,
186+
BufferTaskType::CommitDiff(_) => TaskType::Commit,
139187
}
140188
}
141189

@@ -145,12 +193,21 @@ impl BaseTask for BufferTask {
145193
}
146194

147195
fn reset_commit_id(&mut self, commit_id: u64) {
148-
let BufferTaskType::Commit(commit_task) = &mut self.task_type;
149-
if commit_id == commit_task.commit_id {
150-
return;
151-
}
196+
match &mut self.task_type {
197+
BufferTaskType::Commit(commit_task) => {
198+
if commit_id == commit_task.commit_id {
199+
return;
200+
}
201+
commit_task.commit_id = commit_id;
202+
}
203+
BufferTaskType::CommitDiff(task) => {
204+
if commit_id == task.commit_id {
205+
return;
206+
}
207+
task.commit_id = commit_id;
208+
}
209+
};
152210

153-
commit_task.commit_id = commit_id;
154211
self.preparation_state = Self::preparation_required(&self.task_type)
155212
}
156213
}
@@ -159,6 +216,7 @@ impl LabelValue for BufferTask {
159216
fn value(&self) -> &str {
160217
match self.task_type {
161218
BufferTaskType::Commit(_) => "buffer_commit",
219+
BufferTaskType::CommitDiff(_) => "buffer_commit_diff",
162220
}
163221
}
164222
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ pub enum PreparationState {
4646
Cleanup(CleanupTask),
4747
}
4848

49-
#[cfg(test)]
5049
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
5150
pub enum TaskStrategy {
5251
Args,

magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,40 @@ where
2626
fn visit_args_task(&mut self, task: &ArgsTask) {
2727
match self.context {
2828
PersistorContext::PersistStrategy { uses_lookup_tables } => {
29-
let ArgsTaskType::Commit(ref commit_task) = task.task_type
30-
else {
31-
return;
32-
};
33-
3429
let commit_strategy = if uses_lookup_tables {
3530
CommitStrategy::ArgsWithLookupTable
3631
} else {
3732
CommitStrategy::Args
3833
};
3934

40-
if let Err(err) = self.persistor.set_commit_strategy(
41-
commit_task.commit_id,
42-
&commit_task.committed_account.pubkey,
43-
commit_strategy,
44-
) {
45-
error!(
46-
"Failed to persist commit strategy {}: {}",
47-
commit_strategy.as_str(),
48-
err
49-
);
35+
match &task.task_type {
36+
ArgsTaskType::Commit(task) => {
37+
if let Err(err) = self.persistor.set_commit_strategy(
38+
task.commit_id,
39+
&task.committed_account.pubkey,
40+
commit_strategy,
41+
) {
42+
error!(
43+
"Failed to persist commit strategy {}: {}",
44+
commit_strategy.as_str(),
45+
err
46+
);
47+
}
48+
}
49+
ArgsTaskType::CommitDiff(task) => {
50+
if let Err(err) = self.persistor.set_commit_strategy(
51+
task.commit_id,
52+
&task.committed_account.pubkey,
53+
commit_strategy,
54+
) {
55+
error!(
56+
"Failed to persist commit strategy {}: {}",
57+
commit_strategy.as_str(),
58+
err
59+
);
60+
}
61+
}
62+
_ => {}
5063
}
5164
}
5265
}
@@ -55,23 +68,39 @@ where
5568
fn visit_buffer_task(&mut self, task: &BufferTask) {
5669
match self.context {
5770
PersistorContext::PersistStrategy { uses_lookup_tables } => {
58-
let BufferTaskType::Commit(ref commit_task) = task.task_type;
5971
let commit_strategy = if uses_lookup_tables {
6072
CommitStrategy::FromBufferWithLookupTable
6173
} else {
6274
CommitStrategy::FromBuffer
6375
};
6476

65-
if let Err(err) = self.persistor.set_commit_strategy(
66-
commit_task.commit_id,
67-
&commit_task.committed_account.pubkey,
68-
commit_strategy,
69-
) {
70-
error!(
71-
"Failed to persist commit strategy {}: {}",
72-
commit_strategy.as_str(),
73-
err
74-
);
77+
match &task.task_type {
78+
BufferTaskType::Commit(task) => {
79+
if let Err(err) = self.persistor.set_commit_strategy(
80+
task.commit_id,
81+
&task.committed_account.pubkey,
82+
commit_strategy,
83+
) {
84+
error!(
85+
"Failed to persist commit strategy {}: {}",
86+
commit_strategy.as_str(),
87+
err
88+
);
89+
}
90+
}
91+
BufferTaskType::CommitDiff(task) => {
92+
if let Err(err) = self.persistor.set_commit_strategy(
93+
task.commit_id,
94+
&task.committed_account.pubkey,
95+
commit_strategy,
96+
) {
97+
error!(
98+
"Failed to persist commit strategy {}: {}",
99+
commit_strategy.as_str(),
100+
err
101+
);
102+
}
103+
}
75104
}
76105
}
77106
}

magicblock-committor-service/src/tasks/task_visitors/utility_visitor.rs

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,39 @@ impl Visitor for TaskVisitorUtils {
1919
fn visit_args_task(&mut self, task: &ArgsTask) {
2020
let Self::GetCommitMeta(commit_meta) = self;
2121

22-
if let ArgsTaskType::Commit(ref commit_task) = task.task_type {
23-
*commit_meta = Some(CommitMeta {
24-
committed_pubkey: commit_task.committed_account.pubkey,
25-
commit_id: commit_task.commit_id,
26-
})
27-
} else {
28-
*commit_meta = None
22+
match &task.task_type {
23+
ArgsTaskType::Commit(task) => {
24+
*commit_meta = Some(CommitMeta {
25+
committed_pubkey: task.committed_account.pubkey,
26+
commit_id: task.commit_id,
27+
})
28+
}
29+
ArgsTaskType::CommitDiff(task) => {
30+
*commit_meta = Some(CommitMeta {
31+
committed_pubkey: task.committed_account.pubkey,
32+
commit_id: task.commit_id,
33+
})
34+
}
35+
_ => *commit_meta = None,
2936
}
3037
}
3138

3239
fn visit_buffer_task(&mut self, task: &BufferTask) {
3340
let Self::GetCommitMeta(commit_meta) = self;
3441

35-
let BufferTaskType::Commit(ref commit_task) = task.task_type;
36-
*commit_meta = Some(CommitMeta {
37-
committed_pubkey: commit_task.committed_account.pubkey,
38-
commit_id: commit_task.commit_id,
39-
})
42+
match &task.task_type {
43+
BufferTaskType::Commit(task) => {
44+
*commit_meta = Some(CommitMeta {
45+
committed_pubkey: task.committed_account.pubkey,
46+
commit_id: task.commit_id,
47+
})
48+
}
49+
BufferTaskType::CommitDiff(task) => {
50+
*commit_meta = Some(CommitMeta {
51+
committed_pubkey: task.committed_account.pubkey,
52+
commit_id: task.commit_id,
53+
})
54+
}
55+
}
4056
}
4157
}

0 commit comments

Comments
 (0)