Skip to content

Commit 4d7b5b6

Browse files
committed
refactor: Simplify Task types
1 parent 5de4b20 commit 4d7b5b6

18 files changed

Lines changed: 970 additions & 679 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ magicblock-committor-program = { path = "./magicblock-committor-program", featur
9595
magicblock-committor-service = { path = "./magicblock-committor-service" }
9696
magicblock-config = { path = "./magicblock-config" }
9797
magicblock-core = { path = "./magicblock-core" }
98-
magicblock-delegation-program = { git = "https://github.com/magicblock-labs/delegation-program.git", rev = "e8d03936", features = [
98+
magicblock-delegation-program = { git = "https://github.com/magicblock-labs/delegation-program.git", rev = "ea1f2f916268132248fe8d5de5f07d76765dd937", features = [
9999
"no-entrypoint",
100100
] }
101101
magicblock-geyser-plugin = { path = "./magicblock-geyser-plugin" }

magicblock-committor-service/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ IntentExecutor - responsible for execution of Intent. Calls **TransactionPrepar
2323
TransactionPreparator - is an entity that handles all of the above "Transaction preparation" calling **TaskBuilderV1**, **TaskStrategist**, **DeliveryPreparator** and then assempling it all and passing to **MessageExecutor**
2424

2525
## DeliveryPreparator
26-
After our **BaseTask**s are ready we need to prepare eveything for their successful execution. **DeliveryPreparator** - handles ALTs and commit buffers
26+
After our **Task**s are ready we need to prepare eveything for their successful execution. **DeliveryPreparator** - handles ALTs and commit buffers
2727

2828
## TaskBuilder
2929
First, lets build atomic tasks from scheduled message/intent.
3030

31-
High level: TaskBuilder responsible for creating BaseTasks(to be renamed...) from ScheduledBaseIntent(to be renamed...).
31+
High level: TaskBuilder responsible for creating Tasks(to be renamed...) from ScheduledBaseIntent(to be renamed...).
3232
Details: To do that is requires additional information from DelegationMetadata, it is provided **CommitIdFetcher**
3333

34-
### BaseTask
35-
High level: BaseTask - is an atomic operation that is to be performed on the Base layer, like: Commit, Undelegate, Finalize, Action.
34+
### Task
35+
High level: Task - is an atomic operation that is to be performed on the Base layer, like: Commit, Undelegate, Finalize, Action.
3636

37-
Details: There's to implementation of BaseTask: ArgsTask, BufferTask. ArgsTask - gives instruction using args. BufferTask - gives instruction using buffer. BufferTask at the moment supports only commits
37+
Details: There's to implementation of Task: ArgsTask, BufferTask. ArgsTask - gives instruction using args. BufferTask - gives instruction using buffer. BufferTask at the moment supports only commits
3838

3939
### TaskInfoFetcher
4040
High level: for account to be accepted by `dlp` it needs to have incremental commit ids. TaskInfoFetcher provides a user with the correct ids/nonces for set of committees

magicblock-committor-service/src/intent_executor/error.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use solana_transaction_error::TransactionError;
1111
use crate::{
1212
tasks::{
1313
task_builder::TaskBuilderError, task_strategist::TaskStrategistError,
14-
BaseTask, TaskType,
14+
Task, TaskType,
1515
},
1616
transaction_preparator::error::TransactionPreparatorError,
1717
};
@@ -174,7 +174,7 @@ impl TransactionStrategyExecutionError {
174174
pub fn try_from_transaction_error(
175175
err: TransactionError,
176176
signature: Option<Signature>,
177-
tasks: &[Box<dyn BaseTask>],
177+
tasks: &[Task],
178178
) -> Result<Self, TransactionError> {
179179
// There's always 2 budget instructions in front
180180
const OFFSET: u8 = 2;
@@ -256,7 +256,7 @@ impl metrics::LabelValue for TransactionStrategyExecutionError {
256256
}
257257

258258
pub(crate) struct IntentTransactionErrorMapper<'a> {
259-
pub tasks: &'a [Box<dyn BaseTask>],
259+
pub tasks: &'a [Task],
260260
}
261261
impl TransactionErrorMapper for IntentTransactionErrorMapper<'_> {
262262
type ExecutionError = TransactionStrategyExecutionError;

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub use null_task_info_fetcher::*;
3535
use solana_pubkey::Pubkey;
3636
use solana_rpc_client_api::config::RpcTransactionConfig;
3737
use solana_signature::Signature;
38-
use solana_signer::Signer;
38+
use solana_signer::{Signer, SignerError};
3939
use solana_transaction::versioned::VersionedTransaction;
4040

4141
use crate::{
@@ -53,10 +53,11 @@ use crate::{
5353
tasks::{
5454
task_builder::{TaskBuilderError, TaskBuilderImpl, TasksBuilder},
5555
task_strategist::{
56-
StrategyExecutionMode, TaskStrategist, TransactionStrategy,
56+
StrategyExecutionMode, TaskStrategist, TaskStrategistError,
57+
TransactionStrategy,
5758
},
5859
task_visitors::utility_visitor::TaskVisitorUtils,
59-
BaseTask, TaskType,
60+
Task, TaskType,
6061
},
6162
transaction_preparator::{
6263
delivery_preparator::BufferExecutionError,
@@ -145,6 +146,40 @@ where
145146
}
146147
}
147148

149+
/// Checks if it is possible to unite Commit & Finalize stages in 1 transaction
150+
/// Returns corresponding `TransactionStrategy` if possible, otherwise `None`
151+
fn try_unite_tasks<P: IntentPersister>(
152+
commit_tasks: &[Task],
153+
finalize_task: &[Task],
154+
authority: &Pubkey,
155+
persister: &Option<P>,
156+
) -> Result<Option<TransactionStrategy>, SignerError> {
157+
const MAX_UNITED_TASKS_LEN: usize = 22;
158+
159+
// We can unite in 1 tx a lot of commits
160+
// but then there's a possibility of hitting CPI limit, aka
161+
// MaxInstructionTraceLengthExceeded error.
162+
// So we limit tasks len with 22 total tasks
163+
// In case this fails as well, it will be retried with TwoStage approach
164+
// on retry, once retries are introduced
165+
if commit_tasks.len() + finalize_task.len() > MAX_UNITED_TASKS_LEN {
166+
return Ok(None);
167+
}
168+
169+
// Clone tasks since strategies applied to united case maybe suboptimal for regular one
170+
let mut commit_tasks = commit_tasks.to_owned();
171+
let finalize_task = finalize_task.to_owned();
172+
173+
// Unite tasks to attempt running as single tx
174+
commit_tasks.extend(finalize_task);
175+
match TaskStrategist::build_strategy(commit_tasks, authority, persister)
176+
{
177+
Ok(strategy) => Ok(Some(strategy)),
178+
Err(TaskStrategistError::FailedToFitError) => Ok(None),
179+
Err(TaskStrategistError::SignerError(err)) => Err(err),
180+
}
181+
}
182+
148183
async fn execute_inner<P: IntentPersister>(
149184
&mut self,
150185
base_intent: ScheduledBaseIntent,
@@ -169,7 +204,7 @@ where
169204
Some(value) => value,
170205
None => {
171206
// Build tasks for commit stage
172-
let commit_tasks = TaskBuilderImpl::commit_tasks(
207+
let commit_tasks = TaskBuilderImpl::create_commit_tasks(
173208
&self.task_info_fetcher,
174209
&base_intent,
175210
persister,
@@ -194,7 +229,7 @@ where
194229

195230
// Build tasks for commit & finalize stages
196231
let (commit_tasks, finalize_tasks) = {
197-
let commit_tasks_fut = TaskBuilderImpl::commit_tasks(
232+
let commit_tasks_fut = TaskBuilderImpl::create_commit_tasks(
198233
&self.task_info_fetcher,
199234
&base_intent,
200235
persister,
@@ -641,7 +676,7 @@ where
641676
async fn execute_message_with_retries(
642677
&self,
643678
prepared_message: VersionedMessage,
644-
tasks: &[Box<dyn BaseTask>],
679+
tasks: &[Task],
645680
) -> IntentExecutorResult<Signature, TransactionStrategyExecutionError>
646681
{
647682
struct IntentErrorMapper<TxMap> {
Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
use magicblock_committor_program::{
2+
instruction_builder::{
3+
close_buffer::{create_close_ix, CreateCloseIxArgs},
4+
init_buffer::{create_init_ix, CreateInitIxArgs},
5+
realloc_buffer::{
6+
create_realloc_buffer_ixs, CreateReallocBufferIxArgs,
7+
},
8+
write_buffer::{create_write_ix, CreateWriteIxArgs},
9+
},
10+
pdas, ChangesetChunks, Chunks,
11+
};
12+
use magicblock_program::magic_scheduled_base_intent::CommittedAccount;
13+
use solana_account::Account;
14+
use solana_instruction::Instruction;
15+
use solana_pubkey::Pubkey;
16+
17+
use crate::consts::MAX_WRITE_CHUNK_SIZE;
18+
19+
#[derive(Debug, Clone)]
20+
pub struct BufferLifecycle {
21+
// TODO (snawaz): rename
22+
// PreparationTask -> CreateBufferTask
23+
// CleanupTask -> DestroyBufferTask
24+
pub preparation: PreparationTask,
25+
pub cleanup: CleanupTask,
26+
}
27+
28+
impl BufferLifecycle {
29+
pub fn new(
30+
commit_id: u64,
31+
account: &CommittedAccount,
32+
base_account: Option<&Account>,
33+
) -> BufferLifecycle {
34+
let data = if let Some(base_account) = base_account {
35+
dlp::compute_diff(&base_account.data, &account.account.data)
36+
.to_vec()
37+
} else {
38+
account.account.data.clone()
39+
};
40+
41+
BufferLifecycle {
42+
preparation: PreparationTask {
43+
commit_id,
44+
pubkey: account.pubkey,
45+
chunks: Chunks::from_data_length(
46+
data.len(),
47+
MAX_WRITE_CHUNK_SIZE,
48+
),
49+
state_or_diff: data,
50+
},
51+
cleanup: CleanupTask {
52+
pubkey: account.pubkey,
53+
commit_id,
54+
},
55+
}
56+
}
57+
}
58+
59+
#[derive(Clone, Debug)]
60+
pub struct PreparationTask {
61+
pub commit_id: u64,
62+
pub pubkey: Pubkey,
63+
pub chunks: Chunks,
64+
pub state_or_diff: Vec<u8>,
65+
}
66+
67+
impl PreparationTask {
68+
/// Returns initialization [`Instruction`]
69+
pub fn instruction(&self, authority: &Pubkey) -> Instruction {
70+
// // SAFETY: as object_length internally uses only already allocated or static buffers,
71+
// // and we don't use any fs writers, so the only error that may occur here is of kind
72+
// // OutOfMemory or WriteZero. This is impossible due to:
73+
// // Chunks::new panics if its size exceeds MAX_ACCOUNT_ALLOC_PER_INSTRUCTION_SIZE or 10_240
74+
// // https://github.com/near/borsh-rs/blob/f1b75a6b50740bfb6231b7d0b1bd93ea58ca5452/borsh/src/ser/helpers.rs#L59
75+
let chunks_account_size =
76+
borsh::object_length(&self.chunks).unwrap() as u64;
77+
let buffer_account_size = self.state_or_diff.len() as u64;
78+
79+
let (instruction, _, _) = create_init_ix(CreateInitIxArgs {
80+
authority: *authority,
81+
pubkey: self.pubkey,
82+
chunks_account_size,
83+
buffer_account_size,
84+
commit_id: self.commit_id,
85+
chunk_count: self.chunks.count(),
86+
chunk_size: self.chunks.chunk_size(),
87+
});
88+
89+
instruction
90+
}
91+
92+
/// Returns compute units required for realloc instruction
93+
pub fn init_compute_units(&self) -> u32 {
94+
12_000
95+
}
96+
97+
/// Returns realloc instruction required for Buffer preparation
98+
#[allow(clippy::let_and_return)]
99+
pub fn realloc_instructions(&self, authority: &Pubkey) -> Vec<Instruction> {
100+
let buffer_account_size = self.state_or_diff.len() as u64;
101+
let realloc_instructions =
102+
create_realloc_buffer_ixs(CreateReallocBufferIxArgs {
103+
authority: *authority,
104+
pubkey: self.pubkey,
105+
buffer_account_size,
106+
commit_id: self.commit_id,
107+
});
108+
109+
realloc_instructions
110+
}
111+
112+
/// Returns compute units required for realloc instruction
113+
pub fn realloc_compute_units(&self) -> u32 {
114+
6_000
115+
}
116+
117+
/// Returns realloc instruction required for Buffer preparation
118+
#[allow(clippy::let_and_return)]
119+
pub fn write_instructions(&self, authority: &Pubkey) -> Vec<Instruction> {
120+
let chunks_iter =
121+
ChangesetChunks::new(&self.chunks, self.chunks.chunk_size())
122+
.iter(&self.state_or_diff);
123+
let write_instructions = chunks_iter
124+
.map(|chunk| {
125+
create_write_ix(CreateWriteIxArgs {
126+
authority: *authority,
127+
pubkey: self.pubkey,
128+
offset: chunk.offset,
129+
data_chunk: chunk.data_chunk,
130+
commit_id: self.commit_id,
131+
})
132+
})
133+
.collect::<Vec<_>>();
134+
135+
write_instructions
136+
}
137+
138+
pub fn write_compute_units(&self, bytes_count: usize) -> u32 {
139+
const PER_BYTE: u32 = 3;
140+
141+
u32::try_from(bytes_count)
142+
.ok()
143+
.and_then(|bytes_count| bytes_count.checked_mul(PER_BYTE))
144+
.unwrap_or(u32::MAX)
145+
}
146+
147+
pub fn chunks_pda(&self, authority: &Pubkey) -> Pubkey {
148+
pdas::chunks_pda(
149+
authority,
150+
&self.pubkey,
151+
self.commit_id.to_le_bytes().as_slice(),
152+
)
153+
.0
154+
}
155+
156+
pub fn buffer_pda(&self, authority: &Pubkey) -> Pubkey {
157+
pdas::buffer_pda(
158+
authority,
159+
&self.pubkey,
160+
self.commit_id.to_le_bytes().as_slice(),
161+
)
162+
.0
163+
}
164+
165+
pub fn cleanup_task(&self) -> CleanupTask {
166+
CleanupTask {
167+
pubkey: self.pubkey,
168+
commit_id: self.commit_id,
169+
}
170+
}
171+
}
172+
173+
#[derive(Clone, Debug)]
174+
pub struct CleanupTask {
175+
pub pubkey: Pubkey,
176+
pub commit_id: u64,
177+
}
178+
179+
impl CleanupTask {
180+
pub fn instruction(&self, authority: &Pubkey) -> Instruction {
181+
create_close_ix(CreateCloseIxArgs {
182+
authority: *authority,
183+
pubkey: self.pubkey,
184+
commit_id: self.commit_id,
185+
})
186+
}
187+
188+
/// Returns compute units required to execute [`CleanupTask`]
189+
pub fn compute_units(&self) -> u32 {
190+
30_000
191+
}
192+
193+
/// Returns a number of [`CleanupTask`]s that is possible to fit in single
194+
pub const fn max_tx_fit_count_with_budget() -> usize {
195+
8
196+
}
197+
198+
pub fn chunks_pda(&self, authority: &Pubkey) -> Pubkey {
199+
pdas::chunks_pda(
200+
authority,
201+
&self.pubkey,
202+
self.commit_id.to_le_bytes().as_slice(),
203+
)
204+
.0
205+
}
206+
207+
pub fn buffer_pda(&self, authority: &Pubkey) -> Pubkey {
208+
pdas::buffer_pda(
209+
authority,
210+
&self.pubkey,
211+
self.commit_id.to_le_bytes().as_slice(),
212+
)
213+
.0
214+
}
215+
}

0 commit comments

Comments
 (0)