diff --git a/Cargo.lock b/Cargo.lock index 2115dcd8c..44333b422 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3624,20 +3624,15 @@ version = "0.12.2" dependencies = [ "async-trait", "bincode", - "magicblock-accounts-db", "magicblock-chainlink", - "magicblock-committor-service", - "magicblock-config", "magicblock-core", "magicblock-ledger", "magicblock-magic-program-api", "magicblock-program", - "magicblock-rpc-client", "rand 0.9.4", "solana-account 3.4.0", "solana-hash 3.1.0", "solana-instruction 3.4.0", - "solana-loader-v3-interface 6.1.1", "solana-loader-v4-interface 3.1.0", "solana-pubkey 3.0.0", "solana-sdk-ids 3.1.0", @@ -3655,21 +3650,11 @@ name = "magicblock-accounts" version = "0.12.2" dependencies = [ "async-trait", - "magicblock-account-cloner", - "magicblock-accounts-db", - "magicblock-chainlink", "magicblock-committor-service", - "magicblock-core", - "magicblock-metrics", - "magicblock-program", - "solana-hash 3.1.0", "solana-pubkey 3.0.0", - "solana-transaction", "solana-transaction-error 3.2.0", "thiserror 2.0.18", "tokio", - "tokio-util", - "tracing", "url", ] @@ -3907,8 +3892,10 @@ dependencies = [ "bincode", "borsh", "futures-util", - "lazy_static", "lru 0.16.4", + "magicblock-account-cloner", + "magicblock-accounts-db", + "magicblock-chainlink", "magicblock-committor-program", "magicblock-core", "magicblock-delegation-program-api", @@ -3918,10 +3905,8 @@ dependencies = [ "magicblock-table-mania", "rand 0.9.4", "rusqlite", - "serde_json", "solana-account 3.4.0", "solana-account-decoder", - "solana-address-lookup-table-interface 3.1.0", "solana-commitment-config", "solana-compute-budget-interface", "solana-hash 3.1.0", @@ -3935,19 +3920,15 @@ dependencies = [ "solana-rpc-client-api", "solana-signature 3.4.0", "solana-signer", - "solana-system-program", "solana-transaction", "solana-transaction-error 3.2.0", "solana-transaction-status-client-types", - "static_assertions", "tempfile", "test-kit", "thiserror 2.0.18", "tokio", "tokio-util", "tracing", - "tracing-log", - "tracing-subscriber", ] [[package]] diff --git a/magicblock-account-cloner/Cargo.toml b/magicblock-account-cloner/Cargo.toml index 7e9e48448..4ef318fe9 100644 --- a/magicblock-account-cloner/Cargo.toml +++ b/magicblock-account-cloner/Cargo.toml @@ -11,20 +11,15 @@ edition.workspace = true async-trait = { workspace = true } bincode = { workspace = true } tracing = { workspace = true } -magicblock-accounts-db = { workspace = true } magicblock-chainlink = { workspace = true } -magicblock-committor-service = { workspace = true } -magicblock-config = { workspace = true } magicblock-core = { workspace = true } magicblock-ledger = { workspace = true } magicblock-magic-program-api = { workspace = true } magicblock-program = { workspace = true } -magicblock-rpc-client = { workspace = true } rand = { workspace = true } solana-account = { workspace = true } solana-hash = { workspace = true } solana-instruction = { workspace = true } -solana-loader-v3-interface = { workspace = true } solana-loader-v4-interface = { workspace = true } solana-pubkey = { workspace = true } solana-sdk-ids = { workspace = true } @@ -34,8 +29,3 @@ solana-sysvar = { workspace = true } solana-transaction = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } - -[dev-dependencies] -magicblock-committor-service = { workspace = true, features = [ - "dev-context-only-utils", -] } diff --git a/magicblock-account-cloner/src/account_cloner.rs b/magicblock-account-cloner/src/account_cloner.rs deleted file mode 100644 index 67437653a..000000000 --- a/magicblock-account-cloner/src/account_cloner.rs +++ /dev/null @@ -1,68 +0,0 @@ -use std::sync::Arc; - -use magicblock_committor_service::{ - error::{CommittorServiceError, CommittorServiceResult}, - BaseIntentCommittor, -}; -use thiserror::Error; -use tokio::sync::oneshot; - -pub type AccountClonerResult = Result; - -#[derive(Debug, Clone, Error)] -pub enum AccountClonerError { - #[error(transparent)] - RecvError(#[from] tokio::sync::oneshot::error::RecvError), - - #[error("JoinError ({0})")] - JoinError(String), - - #[error("CommittorServiceError {0}")] - CommittorServiceError(String), -} - -pub async fn map_committor_request_result( - res: oneshot::Receiver>, - intent_committor: Arc, -) -> AccountClonerResult { - match res.await.map_err(|err| { - // Send request error - AccountClonerError::CommittorServiceError(format!( - "error sending request {err:?}" - )) - })? { - Ok(val) => Ok(val), - Err(err) => { - // Commit error - match err { - CommittorServiceError::TableManiaError(table_mania_err) => { - let Some(sig) = table_mania_err.signature() else { - return Err(AccountClonerError::CommittorServiceError( - format!("{:?}", table_mania_err), - )); - }; - let (logs, cus) = crate::util::get_tx_diagnostics( - &sig, - &intent_committor, - ) - .await; - - let cus_str = cus - .map(|cus| format!("{:?}", cus)) - .unwrap_or("N/A".to_string()); - let logs_str = logs - .map(|logs| format!("{:#?}", logs)) - .unwrap_or("N/A".to_string()); - Err(AccountClonerError::CommittorServiceError(format!( - "{:?}\nCUs: {cus_str}\nLogs: {logs_str}", - table_mania_err - ))) - } - _ => Err(AccountClonerError::CommittorServiceError(format!( - "{:?}", - err - ))), - } - } - } -} diff --git a/magicblock-account-cloner/src/lib.rs b/magicblock-account-cloner/src/lib.rs index a2dab89a8..4716073cf 100644 --- a/magicblock-account-cloner/src/lib.rs +++ b/magicblock-account-cloner/src/lib.rs @@ -67,10 +67,8 @@ use tracing::*; pub const MAX_INLINE_DATA_SIZE: usize = 63 * 1024; const MAX_INLINE_TRANSACTION_SIZE: usize = u16::MAX as usize; -mod account_cloner; mod util; -pub use account_cloner::*; pub use util::derive_buffer_pubkey; pub struct ChainlinkCloner { diff --git a/magicblock-account-cloner/src/util.rs b/magicblock-account-cloner/src/util.rs index 992508faa..fcfe876f6 100644 --- a/magicblock-account-cloner/src/util.rs +++ b/magicblock-account-cloner/src/util.rs @@ -1,10 +1,5 @@ -use std::sync::Arc; - -use magicblock_committor_service::BaseIntentCommittor; use magicblock_program::validator::validator_authority_id; -use magicblock_rpc_client::MagicblockRpcClient; use solana_pubkey::Pubkey; -use solana_signature::Signature; /// Seed for deriving buffer account PDA const BUFFER_SEED: &[u8] = b"buffer"; @@ -15,16 +10,3 @@ pub fn derive_buffer_pubkey(program_pubkey: &Pubkey) -> (Pubkey, u8) { let seeds: &[&[u8]] = &[BUFFER_SEED, program_pubkey.as_ref()]; Pubkey::find_program_address(seeds, &validator_authority_id()) } - -pub(crate) async fn get_tx_diagnostics( - sig: &Signature, - committor: &Arc, -) -> (Option>, Option) { - if let Ok(Ok(transaction)) = committor.get_transaction(sig).await { - let cus = MagicblockRpcClient::get_cus_from_transaction(&transaction); - let logs = MagicblockRpcClient::get_logs_from_transaction(&transaction); - (logs, cus) - } else { - (None, None) - } -} diff --git a/magicblock-accounts/Cargo.toml b/magicblock-accounts/Cargo.toml index aaf0879d4..5b48785a5 100644 --- a/magicblock-accounts/Cargo.toml +++ b/magicblock-accounts/Cargo.toml @@ -9,26 +9,10 @@ edition.workspace = true [dependencies] async-trait = { workspace = true } -tracing = { workspace = true } -magicblock-account-cloner = { workspace = true } -magicblock-accounts-db = { workspace = true } -magicblock-chainlink = { workspace = true } magicblock-committor-service = { workspace = true } -magicblock-core = { workspace = true } -magicblock-metrics = { workspace = true } -magicblock-program = { workspace = true } -solana-hash = { workspace = true } solana-pubkey = { workspace = true } solana-transaction-error = { workspace = true } -solana-transaction = { workspace = true } tokio = { workspace = true } -tokio-util = { workspace = true } thiserror = { workspace = true } url = { workspace = true } - -[dev-dependencies] -magicblock-committor-service = { workspace = true, features = [ - "dev-context-only-utils", -] } -tokio-util = { workspace = true } diff --git a/magicblock-accounts/src/errors.rs b/magicblock-accounts/src/errors.rs index 334ef7e60..d4a7b5980 100644 --- a/magicblock-accounts/src/errors.rs +++ b/magicblock-accounts/src/errors.rs @@ -1,9 +1,7 @@ use std::collections::HashSet; -use magicblock_account_cloner::AccountClonerError; use magicblock_committor_service::{ - error::CommittorServiceError, service_ext::CommittorServiceExtError, - ChangesetMeta, + error::CommittorServiceError, ChangesetMeta, }; use solana_pubkey::Pubkey; use solana_transaction_error::TransactionError; @@ -23,15 +21,9 @@ pub enum AccountsError { #[error("CommittorSerivceError: {0}")] CommittorSerivceError(#[from] CommittorServiceError), - #[error("CommittorServiceExtError: {0}")] - CommittorServiceExtError(#[from] CommittorServiceExtError), - #[error("TokioOneshotRecvError")] TokioOneshotRecvError(#[from] Box), - #[error("AccountClonerError")] - AccountClonerError(#[from] AccountClonerError), - #[error("InvalidRpcUrl '{0}'")] InvalidRpcUrl(String), diff --git a/magicblock-accounts/src/lib.rs b/magicblock-accounts/src/lib.rs index d11a013b2..c7eac2118 100644 --- a/magicblock-accounts/src/lib.rs +++ b/magicblock-accounts/src/lib.rs @@ -1,6 +1,5 @@ mod config; pub mod errors; -pub mod scheduled_commits_processor; mod traits; pub use config::*; diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs deleted file mode 100644 index 2df5e7f16..000000000 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ /dev/null @@ -1,504 +0,0 @@ -use std::{ - collections::{HashMap, HashSet}, - sync::{Arc, Mutex}, -}; - -use async_trait::async_trait; -use magicblock_account_cloner::ChainlinkCloner; -use magicblock_chainlink::{ProdChainlink, ProdInnerChainlink}; -use magicblock_committor_service::{ - error::CommittorServiceResult, - intent_execution_manager::BroadcastedIntentExecutionResult, - intent_executor::ExecutionOutput, BaseIntentCommittor, CommittorService, -}; -use magicblock_core::{ - link::transactions::{with_encoded, TransactionSchedulerHandle}, - traits::LatestBlockProvider, -}; -use magicblock_metrics::metrics::{self, AccountFetchOrigin}; -use magicblock_program::{ - instruction_utils::InstructionUtils, - magic_scheduled_base_intent::ScheduledIntentBundle, - register_scheduled_commit_sent, SentCommit, TransactionScheduler, -}; -use solana_hash::Hash; -use solana_pubkey::Pubkey; -use solana_transaction::Transaction; -use tokio::{ - sync::{broadcast, oneshot}, - task, -}; -use tokio_util::sync::CancellationToken; -use tracing::{debug, error, info, instrument, warn}; - -use crate::{ - errors::ScheduledCommitsProcessorResult, ScheduledCommitsProcessor, -}; - -const POISONED_MUTEX_MSG: &str = - "Mutex of RemoteScheduledCommitsProcessor.intents_meta_map is poisoned"; - -pub type InnerChainlinkImpl = ProdInnerChainlink; - -pub type ChainlinkImpl = ProdChainlink; - -pub struct ScheduledCommitsProcessorImpl { - committor: Arc, - chainlink: Arc, - cancellation_token: CancellationToken, - intents_meta_map: Arc>>, - transaction_scheduler: TransactionScheduler, -} - -impl ScheduledCommitsProcessorImpl { - pub fn new( - committor: Arc, - chainlink: Arc, - internal_transaction_scheduler: TransactionSchedulerHandle, - latest_block: impl LatestBlockProvider, - ) -> Self { - let result_subscriber = committor.subscribe_for_results(); - let intents_meta_map = Arc::new(Mutex::default()); - let cancellation_token = CancellationToken::new(); - tokio::spawn(Self::result_processor( - result_subscriber, - cancellation_token.clone(), - intents_meta_map.clone(), - internal_transaction_scheduler.clone(), - latest_block, - )); - - Self { - committor, - chainlink, - cancellation_token, - intents_meta_map, - transaction_scheduler: TransactionScheduler::default(), - } - } - - async fn process_undelegation_requests(&self, pubkeys: Vec) { - let mut join_set = task::JoinSet::new(); - for pubkey in pubkeys.into_iter() { - let chainlink = self.chainlink.clone(); - join_set.spawn(async move { - (pubkey, chainlink.undelegation_requested(pubkey).await) - }); - } - let sub_errors = join_set - .join_all() - .await - .into_iter() - .filter_map(|(pubkey, inner_result)| { - if let Err(err) = inner_result { - Some(format!( - "Subscribing to account {} failed: {}", - pubkey, err - )) - } else { - None - } - }) - .collect::>(); - if !sub_errors.is_empty() { - // Instead of aborting the entire commit we log an error here, however - // this means that the undelegated accounts stay in a problematic state - // in the validator and are not synced from chain. - // We could implement a retry mechanism inside of chainlink in the future. - error!( - error_count = sub_errors.len(), - "Failed to subscribe to accounts being undelegated" - ); - } - } - - async fn prepare_intent_bundles_for_scheduling( - &self, - intent_bundles: &[ScheduledIntentBundle], - ) { - let pubkeys_being_undelegated = { - let mut intent_metas = - self.intents_meta_map.lock().expect(POISONED_MUTEX_MSG); - let mut pubkeys_being_undelegated = HashSet::::new(); - - intent_bundles.iter().for_each(|intent| { - intent_metas - .insert(intent.id, ScheduledBaseIntentMeta::new(intent)); - if let Some(undelegate) = intent.get_undelegate_intent_pubkeys() - { - pubkeys_being_undelegated.extend(undelegate); - } - }); - - pubkeys_being_undelegated.into_iter().collect::>() - }; - - self.process_undelegation_requests(pubkeys_being_undelegated) - .await; - } - - /// Spawn the one-shot recovery pass for persisted pending commit intents. - /// Must be invoked only after ledger replay completes, so the local accounts - /// bank reflects the delegated state checked by `recover_pending_intents`. - pub fn spawn_pending_intents_recovery(self: &Arc) { - let this = self.clone(); - tokio::spawn(async move { - if let Err(err) = this.recover_pending_intents().await { - error!(error = ?err, "Failed to recover pending commit intents"); - } - }); - } - - #[instrument(skip(self))] - async fn recover_pending_intents( - &self, - ) -> ScheduledCommitsProcessorResult<()> { - let intent_bundles = - self.committor.get_pending_intent_bundles().await??; - if intent_bundles.is_empty() { - return Ok(()); - } - let intent_bundles = - self.recoverable_intent_bundles(intent_bundles).await; - self.process_recovered_intent_bundles(intent_bundles).await - } - - async fn process_recovered_intent_bundles( - &self, - intent_bundles: Vec, - ) -> ScheduledCommitsProcessorResult<()> { - if intent_bundles.is_empty() { - return Ok(()); - } - let intent_ids: Vec = - intent_bundles.iter().map(|b| b.id).collect(); - let intent_count = intent_ids.len(); - - let result = self - .process_intent_bundles(intent_bundles, |intent_bundles| { - self.committor - .schedule_recovered_intent_bundles(intent_bundles) - }) - .await; - if result.is_err() { - self.remove_intent_metas(&intent_ids); - } - result?; - info!(intent_count, "Scheduled recovered pending commit intents"); - Ok(()) - } - - async fn process_intent_bundles( - &self, - intent_bundles: Vec, - schedule: F, - ) -> ScheduledCommitsProcessorResult<()> - where - F: FnOnce( - Vec, - ) -> oneshot::Receiver>, - { - if intent_bundles.is_empty() { - return Ok(()); - } - self.prepare_intent_bundles_for_scheduling(&intent_bundles) - .await; - schedule(intent_bundles).await??; - Ok(()) - } - - async fn recoverable_intent_bundles( - &self, - intent_bundles: Vec, - ) -> Vec { - let mut recoverable = Vec::new(); - for intent_bundle in intent_bundles { - let pubkeys = intent_bundle.get_all_committed_pubkeys(); - let delegated = match self - .chainlink - .accounts_delegated_on_base_and_er( - &pubkeys, - AccountFetchOrigin::GetAccount, - ) - .await - { - Ok(delegated) => delegated, - Err(err) => { - error!( - intent_id = intent_bundle.id, - error = ?err, - "Failed to verify recovered commit intent accounts" - ); - continue; - } - }; - if delegated.into_iter().all(|d| d) { - recoverable.push(intent_bundle); - } else { - warn!( - intent_id = intent_bundle.id, - "Skipping recovered commit intent because not all accounts are delegated on base and ER" - ); - } - } - recoverable - } - - fn remove_intent_metas(&self, intent_ids: &[u64]) { - let mut intent_metas = match self.intents_meta_map.lock() { - Ok(intent_metas) => intent_metas, - Err(err) => { - error!( - error = %err, - "Recovered commit intent metadata map was poisoned" - ); - err.into_inner() - } - }; - for intent_id in intent_ids { - intent_metas.remove(intent_id); - } - } - - #[instrument(skip( - result_subscriber, - cancellation_token, - intents_meta_map, - internal_transaction_scheduler, - latest_block - ))] - async fn result_processor( - result_subscriber: oneshot::Receiver< - broadcast::Receiver, - >, - cancellation_token: CancellationToken, - intents_meta_map: Arc>>, - internal_transaction_scheduler: TransactionSchedulerHandle, - latest_block: impl LatestBlockProvider, - ) { - const SUBSCRIPTION_ERR_MSG: &str = - "Failed to get subscription of results of BaseIntents execution"; - - let mut result_receiver = - result_subscriber.await.expect(SUBSCRIPTION_ERR_MSG); - loop { - let execution_result = tokio::select! { - biased; - _ = cancellation_token.cancelled() => { - info!("Shutting down"); - return; - } - execution_result = result_receiver.recv() => { - match execution_result { - Ok(result) => result, - Err(broadcast::error::RecvError::Closed) => { - info!("Intent execution service shut down"); - break; - } - Err(broadcast::error::RecvError::Lagged(skipped)) => { - // SAFETY: This shouldn't happen as our tx execution is faster than Intent execution on Base layer - // If this ever happens it requires investigation - error!(skipped_count = skipped, "Lagging behind intent execution"); - continue; - } - } - } - }; - - let intent_id = execution_result.id; - // Remove intent from metas - let intent_meta = if let Some(intent_meta) = intents_meta_map - .lock() - .expect(POISONED_MUTEX_MSG) - .remove(&intent_id) - { - intent_meta - } else { - // Possible if we have duplicate Intents - // First one will remove id from map and second could fail. - // This should not happen and needs investigation! - error!(intent_id, "Failed to find intent metadata"); - continue; - }; - - Self::process_intent_result( - intent_id, - &internal_transaction_scheduler, - execution_result, - intent_meta, - latest_block.blockhash(), - ) - .await; - } - } - - #[instrument( - skip(internal_transaction_scheduler, result, intent_meta), - fields(intent_id) - )] - async fn process_intent_result( - intent_id: u64, - internal_transaction_scheduler: &TransactionSchedulerHandle, - result: BroadcastedIntentExecutionResult, - mut intent_meta: ScheduledBaseIntentMeta, - current_blockhash: Hash, - ) { - let intent_sent_transaction = intent_meta - .intent_sent_transaction - .take() - .unwrap_or_else(|| { - intent_meta.blockhash = current_blockhash; - InstructionUtils::scheduled_commit_sent( - intent_id, - current_blockhash, - ) - }); - let sent_commit = - Self::build_sent_commit(intent_id, intent_meta, result); - register_scheduled_commit_sent(sent_commit); - let Ok(txn) = with_encoded(intent_sent_transaction) else { - // Unreachable case, all intent transactions are smaller than 64KB by construction - error!("Failed to bincode intent transaction"); - return; - }; - match internal_transaction_scheduler.execute(txn).await { - Ok(()) => { - debug!("Sent commit signaled") - } - Err(err) => { - error!(error = ?err, "Failed to signal sent commit"); - } - } - } - - fn build_sent_commit( - intent_id: u64, - intent_meta: ScheduledBaseIntentMeta, - result: BroadcastedIntentExecutionResult, - ) -> SentCommit { - let error_message = - result.as_ref().err().map(|err| format!("{:?}", err)); - let chain_signatures = match result.inner { - Ok(value) => match value { - ExecutionOutput::SingleStage(signature) => vec![signature], - ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, - } => vec![commit_signature, finalize_signature], - }, - Err(err) => { - error!( - "Failed to commit intent: {}, slot: {}, blockhash: {}. {:?}", - intent_id, intent_meta.slot, intent_meta.blockhash, err - ); - err.signatures() - .map(|(commit, finalize)| { - finalize - .map(|finalize| vec![commit, finalize]) - .unwrap_or(vec![commit]) - }) - .unwrap_or_default() - } - }; - let patched_errors = result - .patched_errors - .iter() - .map(|err| { - info!("Patched intent: {}. error was: {}", intent_id, err); - err.to_string() - }) - .collect(); - - let callbacks_report = result - .callbacks_report - .iter() - .map(|r| match r { - Ok(sig) => { - format!("OK: {sig}") - } - Err(err) => { - error!( - "Callback failed to schedule: {}. error: {}", - intent_id, err - ); - format!("ERR: {err}") - } - }) - .collect(); - - SentCommit { - message_id: intent_id, - slot: intent_meta.slot, - blockhash: intent_meta.blockhash, - payer: intent_meta.payer, - chain_signatures, - included_pubkeys: intent_meta.included_pubkeys, - excluded_pubkeys: vec![], - requested_undelegation: intent_meta.requested_undelegation, - error_message, - patched_errors, - callbacks_scheduling_results: callbacks_report, - } - } -} - -#[async_trait] -impl ScheduledCommitsProcessor for ScheduledCommitsProcessorImpl { - #[instrument(skip(self))] - async fn process(&self) -> ScheduledCommitsProcessorResult<()> { - let intent_bundles = - self.transaction_scheduler.take_scheduled_intent_bundles(); - - if intent_bundles.is_empty() { - return Ok(()); - } - metrics::inc_committor_intents_count_by(intent_bundles.len() as u64); - - self.process_intent_bundles(intent_bundles, |intent_bundles| { - self.committor.schedule_intent_bundles(intent_bundles) - }) - .await - } - - fn scheduled_commits_len(&self) -> usize { - self.transaction_scheduler.scheduled_actions_len() - } - - fn clear_scheduled_commits(&self) { - self.transaction_scheduler.clear_scheduled_actions(); - } - - fn stop(&self) { - self.cancellation_token.cancel(); - } -} - -struct ScheduledBaseIntentMeta { - slot: u64, - blockhash: Hash, - payer: Pubkey, - included_pubkeys: Vec, - intent_sent_transaction: Option, - requested_undelegation: bool, -} - -impl ScheduledBaseIntentMeta { - fn new(intent: &ScheduledIntentBundle) -> Self { - Self { - slot: intent.slot, - blockhash: intent.blockhash, - payer: intent.payer, - included_pubkeys: intent.get_all_committed_pubkeys(), - intent_sent_transaction: if intent - .sent_transaction - .signatures - .is_empty() - { - None - } else { - Some(intent.sent_transaction.clone()) - }, - requested_undelegation: intent.has_undelegate_intent(), - } - } -} diff --git a/magicblock-api/src/errors.rs b/magicblock-api/src/errors.rs index 86c2c8834..4f80a6b72 100644 --- a/magicblock-api/src/errors.rs +++ b/magicblock-api/src/errors.rs @@ -1,4 +1,5 @@ use magicblock_accounts_db::error::AccountsDbError; +use magicblock_committor_service::service::IntentExecutionServiceError; use solana_pubkey::Pubkey; use thiserror::Error; @@ -15,9 +16,6 @@ pub enum ApiError { #[error("Accounts error: {0}")] AccountsError(Box), - #[error("AccountCloner error: {0}")] - AccountClonerError(Box), - #[error("Ledger error: {0}")] LedgerError(Box), @@ -41,6 +39,9 @@ pub enum ApiError { Box, ), + #[error("IntentExecutionServiceError: {0}")] + IntentExecutionServiceError(#[from] IntentExecutionServiceError), + #[error("Failed to load programs into bank: {0}")] FailedToLoadProgramsIntoBank(String), @@ -118,12 +119,6 @@ impl From for ApiError { } } -impl From for ApiError { - fn from(e: magicblock_account_cloner::AccountClonerError) -> Self { - Self::AccountClonerError(Box::new(e)) - } -} - impl From for ApiError { fn from(e: magicblock_ledger::errors::LedgerError) -> Self { Self::LedgerError(Box::new(e)) diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 1e13b44a7..3347d4dfa 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -1,6 +1,9 @@ use std::{collections::HashMap, sync::Arc, time::Duration}; -use magicblock_committor_service::CommittorService; +use magicblock_committor_service::{ + committor_processor::CommittorProcessor, + intent_executor::task_info_fetcher::TaskInfoFetcherResult, +}; use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use magicblock_metrics::metrics; use solana_instruction::error::InstructionError; @@ -9,7 +12,8 @@ use tracing::error; #[derive(Clone)] pub struct MagicSysAdapter { - committor_service: Option>, + handle: tokio::runtime::Handle, + committor_processor: Arc, } impl MagicSysAdapter { @@ -19,13 +23,43 @@ impl MagicSysAdapter { const TIMEOUT_ERR: u32 = 0xE000_0001; /// Returned when the fetch of current commit nonces fails. const FETCH_ERR: u32 = 0xE000_0002; - /// Returned when no committor service is configured. - const NO_COMMITTOR_ERR: u32 = 0xE000_0003; const FETCH_TIMEOUT: Duration = Duration::from_secs(30); - pub fn new(committor_service: Option>) -> Self { - Self { committor_service } + pub fn new( + handle: tokio::runtime::Handle, + committor_processor: Arc, + ) -> Self { + Self { + handle, + committor_processor, + } + } + + fn fetch_current_commit_nonces_sync( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> std::sync::mpsc::Receiver>> + { + let (sender, receiver) = std::sync::mpsc::channel(); + let committor_processor = self.committor_processor.clone(); + let pubkeys = pubkeys.to_owned(); + + // This is required to switch from TransactionExecutor runtime + // blocking on it would cause a panic + let _guard = self.handle.enter(); + tokio::spawn(async move { + let result = committor_processor + .fetch_current_commit_nonces(&pubkeys, min_context_slot) + .await; + if let Err(err) = sender.send(result) { + error!(error = ?err, "Failed to send result back"); + } + }); + drop(_guard); + + receiver } } @@ -37,12 +71,6 @@ impl MagicSys for MagicSysAdapter { if commits.is_empty() { return Ok(HashMap::new()); } - let committor_service = - if let Some(committor_service) = &self.committor_service { - Ok(committor_service) - } else { - Err(InstructionError::Custom(Self::NO_COMMITTOR_ERR)) - }?; let min_context_slot = commits .iter() @@ -53,8 +81,8 @@ impl MagicSys for MagicSysAdapter { commits.iter().map(|account| account.pubkey).collect(); let _timer = metrics::start_fetch_commit_nonces_wait_timer(); - let receiver = committor_service - .fetch_current_commit_nonces_sync(&pubkeys, min_context_slot); + let receiver = + self.fetch_current_commit_nonces_sync(&pubkeys, min_context_slot); receiver .recv_timeout(Self::FETCH_TIMEOUT) .map_err(|err| match err { diff --git a/magicblock-api/src/magic_validator.rs b/magicblock-api/src/magic_validator.rs index 080c8ff8d..4ac23c204 100644 --- a/magicblock-api/src/magic_validator.rs +++ b/magicblock-api/src/magic_validator.rs @@ -9,10 +9,6 @@ use std::{ }; use magicblock_account_cloner::ChainlinkCloner; -use magicblock_accounts::{ - scheduled_commits_processor::ScheduledCommitsProcessorImpl, - ScheduledCommitsProcessor, -}; use magicblock_accounts_db::{traits::AccountsBank, AccountsDb}; use magicblock_aperture::{ initialize_aperture, @@ -23,7 +19,9 @@ use magicblock_chainlink::{ ProdInnerChainlink, }; use magicblock_committor_service::{ - config::ChainConfig, BaseIntentCommittor, CommittorService, + committor_processor::CommittorProcessor, + config::ChainConfig, + service::{intent_client::InternalIntentRpcClient, IntentExecutionService}, ComputeBudgetConfig, DEFAULT_ACTIONS_TIMEOUT, }; use magicblock_config::{ @@ -93,13 +91,16 @@ use crate::{ write_validator_keypair_to_ledger, }, magic_sys_adapter::MagicSysAdapter, - tickers::{init_slot_ticker, init_system_metrics_ticker}, + tickers::init_system_metrics_ticker, }; type InnerChainlinkImpl = ProdInnerChainlink; type ChainlinkImpl = ProdChainlink; +type IntentExecutionServiceImpl = + IntentExecutionService>; + // ----------------- // MagicValidator // ----------------- @@ -110,10 +111,8 @@ pub struct MagicValidator { accountsdb: Arc, ledger: Arc, ledger_truncator: LedgerTruncator, - slot_ticker: Option>, - committor_service: Option>, + intent_execution_service: IntentExecutionServiceImpl, replication_service: Option, - scheduled_commits_processor: Option>, rpc_handle: thread::JoinHandle<()>, identity: Pubkey, transaction_scheduler: TransactionSchedulerHandle, @@ -218,18 +217,6 @@ impl MagicValidator { )) .then(Arc::::default); - let step_start = Instant::now(); - let committor_service = Self::init_committor_service( - &config, - ledger.latest_block(), - shared_chain_slot.clone(), - ) - .await?; - log_timing("startup", "committor_service_init", step_start); - init_magic_sys(Arc::new(MagicSysAdapter::new( - committor_service.clone(), - ))); - let step_start = Instant::now(); let chainlink = Arc::new( Self::init_chainlink( @@ -243,6 +230,30 @@ impl MagicValidator { ); log_timing("startup", "chainlink_init", step_start); + let step_start = Instant::now(); + let committor_processor = { + let processor = Self::init_committor_processor( + &config, + ledger.latest_block(), + &shared_chain_slot, + )?; + Arc::new(processor) + }; + let intent_execution_service = Self::init_intent_execution_service( + &accountsdb, + &chainlink, + &dispatch.transaction_scheduler, + ledger.latest_block(), + &committor_processor, + config.ledger.block_time, + &token, + ); + log_timing("startup", "committor_service_init", step_start); + init_magic_sys(Arc::new(MagicSysAdapter::new( + tokio::runtime::Handle::current(), + committor_processor.clone(), + ))); + let replication_service = if let Some((broker, is_fresh_start)) = broker { let messages_rx = dispatch.replication_messages.take().expect( @@ -300,16 +311,6 @@ impl MagicValidator { ); log_timing("startup", "system_metrics_ticker_start", step_start); - let scheduled_commits_processor = - committor_service.as_ref().map(|committor_service| { - Arc::new(ScheduledCommitsProcessorImpl::new( - committor_service.clone(), - chainlink.clone(), - dispatch.transaction_scheduler.clone(), - ledger.latest_block().clone(), - )) - }); - let step_start = Instant::now(); load_upgradeable_programs( &accountsdb, @@ -429,11 +430,8 @@ impl MagicValidator { config, exit, _metrics: (metrics_service, system_metrics_ticker), - // NOTE: set during [Self::start] - slot_ticker: None, - committor_service, + intent_execution_service, replication_service, - scheduled_commits_processor, token, ledger, ledger_truncator, @@ -450,41 +448,64 @@ impl MagicValidator { }) } - #[instrument(skip(config, latest_block))] - async fn init_committor_service( + pub fn init_committor_processor( config: &ValidatorParams, latest_block: &LatestBlock, - chain_slot: Option>, - ) -> ApiResult>> { + shared_chain_slot: &Option>, + ) -> ApiResult { + let authority = config.validator.keypair.insecure_clone(); let committor_persist_path = config.storage.join("committor_service.sqlite"); - debug!(path = %committor_persist_path.display(), "Initializing committor service"); + let base_chain_config = ChainConfig { + rpc_uri: config.rpc_url().to_owned(), + commitment: CommitmentConfig::confirmed(), + websocket_uri: config + .websocket_urls() + .next() + .map(ToOwned::to_owned), + compute_budget_config: ComputeBudgetConfig::new( + config.commit.compute_unit_price, + ), + actions_timeout: DEFAULT_ACTIONS_TIMEOUT, + }; + // TODO(thlorenz): if startup roles change, revisit whether this service is needed for that role. let actions_callback_executor = ActionsCallbackService::new( Arc::new(RpcClient::new(config.aperture.listen.http())), config.validator.keypair.insecure_clone(), latest_block.clone(), ); - let committor_service = Some(Arc::new(CommittorService::try_start( - config.validator.keypair.insecure_clone(), + Ok(CommittorProcessor::try_new( + authority, committor_persist_path, - ChainConfig { - rpc_uri: config.rpc_url().to_owned(), - websocket_uri: config - .websocket_urls() - .next() - .map(ToOwned::to_owned), - commitment: CommitmentConfig::confirmed(), - compute_budget_config: ComputeBudgetConfig::new( - config.commit.compute_unit_price, - ), - actions_timeout: DEFAULT_ACTIONS_TIMEOUT, - }, - chain_slot, + base_chain_config, + shared_chain_slot.clone(), actions_callback_executor, - )?)); + )?) + } + + fn init_intent_execution_service( + accounts_db: &Arc, + chainlink: &Arc, + transaction_scheduler: &TransactionSchedulerHandle, + latest_block: &LatestBlock, + committor_processor: &Arc, + slot_interval: Duration, + cancellation_token: &CancellationToken, + ) -> IntentExecutionServiceImpl { + let intent_client = InternalIntentRpcClient::new( + accounts_db.clone(), + transaction_scheduler.clone(), + latest_block.clone(), + ); - Ok(committor_service) + IntentExecutionServiceImpl::new( + chainlink.clone(), + intent_client, + committor_processor.clone(), + slot_interval, + cancellation_token.clone(), + ) } #[allow(clippy::too_many_arguments)] @@ -938,11 +959,6 @@ impl MagicValidator { )) })?; } - // Recovery of persisted pending commit intents reads the local accounts - // bank for delegation checks, so it must run only after replay + reset. - if let Some(processor) = self.scheduled_commits_processor.as_ref() { - processor.spawn_pending_intents_recovery(); - } } // Notify the scheduler that ledger replay and bank cleanup is complete. @@ -978,15 +994,8 @@ impl MagicValidator { } let step_start = Instant::now(); - self.slot_ticker = Some(init_slot_ticker( - self.accountsdb.clone(), - &self.scheduled_commits_processor, - self.ledger.latest_block().clone(), - self.config.ledger.block_time, - self.transaction_scheduler.clone(), - self.exit.clone(), - )); - log_timing("startup", "slot_ticker_start", step_start); + self.intent_execution_service.start()?; + log_timing("startup", "intent_execution_service_start", step_start); let step_start = Instant::now(); self.ledger_truncator.start(); @@ -1052,22 +1061,12 @@ impl MagicValidator { // Ordering is important here // Commitor service shall be stopped last self.token.cancel(); - if let Some(ref scheduled_commits_processor) = - self.scheduled_commits_processor - { - let step_start = Instant::now(); - scheduled_commits_processor.stop(); - log_timing( - "shutdown", - "scheduled_commits_processor_stop", - step_start, - ); - } - if let Some(ref committor_service) = self.committor_service { - let step_start = Instant::now(); - committor_service.stop(); - log_timing("shutdown", "committor_service_stop", step_start); + + let step_start = Instant::now(); + if let Err(err) = self.intent_execution_service.stop().await { + error!(error =? err, "Failure during stopping Intent Execution Service") } + log_timing("shutdown", "intent_execution_service_stop", step_start); let step_start = Instant::now(); self.claim_fees_task.stop().await; @@ -1086,11 +1085,6 @@ impl MagicValidator { let step_start = Instant::now(); let _ = self.rpc_handle.join(); log_timing("shutdown", "rpc_thread_join", step_start); - if let Some(handle) = self.slot_ticker { - let step_start = Instant::now(); - let _ = handle.await; - log_timing("shutdown", "slot_ticker_join", step_start); - } let step_start = Instant::now(); if let Err(err) = self.ledger_truncator.join() { error!(error = ?err, "Ledger truncator did not gracefully exit"); diff --git a/magicblock-api/src/tickers.rs b/magicblock-api/src/tickers.rs index 30e7e8e9a..c011e3f80 100644 --- a/magicblock-api/src/tickers.rs +++ b/magicblock-api/src/tickers.rs @@ -1,87 +1,11 @@ -use std::{ - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, - time::Duration, -}; +use std::{sync::Arc, time::Duration}; -use magicblock_accounts::ScheduledCommitsProcessor; -use magicblock_accounts_db::{traits::AccountsBank, AccountsDb}; -use magicblock_core::link::transactions::{ - with_encoded, TransactionSchedulerHandle, -}; -use magicblock_ledger::{LatestBlock, Ledger}; -use magicblock_magic_program_api as magic_program; +use magicblock_accounts_db::AccountsDb; +use magicblock_ledger::Ledger; use magicblock_metrics::metrics; -use magicblock_program::{instruction_utils::InstructionUtils, MagicContext}; -use solana_account::ReadableAccount; use tokio_util::sync::CancellationToken; use tracing::*; -pub fn init_slot_ticker( - accountsdb: Arc, - committor_processor: &Option>, - latest_block: LatestBlock, - tick_duration: Duration, - transaction_scheduler: TransactionSchedulerHandle, - exit: Arc, -) -> tokio::task::JoinHandle<()> { - let committor_processor = committor_processor.clone(); - - tokio::task::spawn(async move { - while !exit.load(Ordering::Relaxed) { - tokio::time::sleep(tick_duration).await; - - // Handle intents if such feature enabled - let Some(committor_processor) = &committor_processor else { - continue; - }; - - // If accounts were scheduled to be committed, we accept them here - // and processs the commits - let magic_context_acc = accountsdb.get_account(&magic_program::MAGIC_CONTEXT_PUBKEY) - .expect("Validator found to be running without MagicContext account!"); - if MagicContext::has_scheduled_commits(magic_context_acc.data()) { - handle_scheduled_commits( - committor_processor, - &transaction_scheduler, - &latest_block, - ) - .await; - } - } - }) -} - -#[instrument(skip(committor_processor, transaction_scheduler, latest_block))] -async fn handle_scheduled_commits( - committor_processor: &Arc, - transaction_scheduler: &TransactionSchedulerHandle, - latest_block: &LatestBlock, -) { - // 1. Send the transaction to move the scheduled commits from the MagicContext - // to the global ScheduledCommit store - let tx = InstructionUtils::accept_scheduled_commits( - latest_block.load().blockhash, - ); - let Ok(tx) = with_encoded(tx) else { - // Unreachable case, all schedule commit txns are smaller than 64KB by construction - error!("Failed to bincode intent transaction"); - return; - }; - if let Err(err) = transaction_scheduler.execute(tx).await { - error!(error = ?err, "Failed to accept scheduled commits"); - return; - } - - // 2. Process those scheduled commits - // TODO: fix the possible delay here - // https://github.com/magicblock-labs/magicblock-validator/issues/104 - if let Err(err) = committor_processor.process().await { - error!(error = ?err, "Failed to process scheduled commits"); - } -} #[allow(unused_variables)] pub fn init_system_metrics_ticker( tick_duration: Duration, diff --git a/magicblock-committor-service/Cargo.toml b/magicblock-committor-service/Cargo.toml index a81f3482a..d9728d739 100644 --- a/magicblock-committor-service/Cargo.toml +++ b/magicblock-committor-service/Cargo.toml @@ -17,6 +17,12 @@ borsh = { workspace = true } futures-util = { workspace = true } tracing = { workspace = true } lru = { workspace = true } +# TDOO: should be removed +magicblock-account-cloner = { workspace = true } +# TODO: should be removed +magicblock-accounts-db = { workspace = true } +# TODO: should be removed, hidden behinf the trait +magicblock-chainlink = { workspace = true } magicblock-committor-program = { workspace = true, features = [ "no-entrypoint", ] } @@ -29,7 +35,6 @@ magicblock-table-mania = { workspace = true } rusqlite = { workspace = true } solana-account = { workspace = true } solana-account-decoder = { workspace = true } -solana-address-lookup-table-interface = { workspace = true } solana-commitment-config = { workspace = true } solana-compute-budget-interface = { workspace = true } solana-hash = { workspace = true } @@ -43,25 +48,18 @@ solana-rpc-client = { workspace = true } solana-rpc-client-api = { workspace = true } solana-signature = { workspace = true } solana-signer = { workspace = true } -solana-system-program = { workspace = true } solana-transaction = { workspace = true } solana-transaction-error = { workspace = true } solana-transaction-status-client-types = { workspace = true } -static_assertions = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tokio-util = { workspace = true } [dev-dependencies] solana-signature = { workspace = true, features = ["rand"] } -serde_json = { workspace = true } -lazy_static = { workspace = true } rand = { workspace = true } tempfile = { workspace = true } test-kit = { workspace = true } -tracing-subscriber = { workspace = true } -tracing-log = { workspace = true } [features] default = [] -dev-context-only-utils = [] diff --git a/magicblock-committor-service/src/committor_processor.rs b/magicblock-committor-service/src/committor_processor.rs index 5061eb4b0..b610480f7 100644 --- a/magicblock-committor-service/src/committor_processor.rs +++ b/magicblock-committor-service/src/committor_processor.rs @@ -1,31 +1,25 @@ use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{hash_map::Entry, HashMap}, path::Path, - sync::{atomic::AtomicU64, Arc}, + sync::{atomic::AtomicU64, Arc, Mutex}, time::{SystemTime, UNIX_EPOCH}, }; -use magicblock_core::{ - intent::CommittedAccount, traits::ActionsCallbackScheduler, -}; -use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicIntentBundle, ScheduledIntentBundle, - UndelegateType, -}; +use futures_util::future::join_all; +use magicblock_core::traits::ActionsCallbackScheduler; +use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use magicblock_rpc_client::MagicblockRpcClient; use magicblock_table_mania::{GarbageCollectorConfig, TableMania}; -use solana_account::Account; use solana_keypair::Keypair; use solana_pubkey::Pubkey; use solana_rpc_client::nonblocking::rpc_client::RpcClient; use solana_signer::Signer; -use solana_transaction::Transaction; -use tokio::sync::broadcast; -use tracing::{error, info, instrument, warn}; +use tokio::sync::{broadcast, oneshot, oneshot::error::RecvError}; +use tracing::{error, info, instrument}; use crate::{ config::ChainConfig, - error::CommittorServiceResult, + error::{CommittorServiceError, CommittorServiceResult}, intent_execution_manager::{ db::DummyDB, BroadcastedIntentExecutionResult, IntentExecutionManager, }, @@ -37,20 +31,24 @@ use crate::{ }, }, persist::{ - CommitStatusRow, CommitType as PersistCommitType, IntentPersister, - IntentPersisterImpl, MessageSignatures, + CommitStatusRow, IntentPersister, IntentPersisterImpl, + MessageSignatures, }, }; +const POISONED_MUTEX_MSG: &str = + "CommittorProcessor pending messages mutex poisoned!"; +pub(crate) const RECOVERY_MAX_AGE_SECS: u64 = 14 * 24 * 60 * 60; -const RECOVERY_MAX_AGE_SECS: u64 = 14 * 24 * 60 * 60; +type BundleResultListener = oneshot::Sender; -pub(crate) struct CommittorProcessor { - pub(crate) magicblock_rpc_client: MagicblockRpcClient, - pub(crate) table_mania: TableMania, - pub(crate) authority: Keypair, +pub struct CommittorProcessor { + authority: Keypair, + _table_mania: TableMania, + magic_rpc_client: MagicblockRpcClient, persister: IntentPersisterImpl, commits_scheduler: IntentExecutionManager, task_info_fetcher: Arc>, + pending_result_listeners: Arc>>, } impl CommittorProcessor { @@ -118,42 +116,24 @@ impl CommittorProcessor { actions_callback_executor, ); + let result_subscription = commits_scheduler.subscribe_for_results(); + let pending_result_listeners = Arc::new(Mutex::new(HashMap::new())); + tokio::spawn(Self::dispatcher( + result_subscription, + pending_result_listeners.clone(), + )); + Ok(Self { authority, - magicblock_rpc_client: magic_block_rpc_client, - table_mania, + _table_mania: table_mania, + magic_rpc_client: magic_block_rpc_client, commits_scheduler, persister, task_info_fetcher, + pending_result_listeners, }) } - pub async fn active_lookup_tables(&self) -> Vec { - self.table_mania.active_table_addresses().await - } - - pub async fn released_lookup_tables(&self) -> Vec { - self.table_mania.released_table_addresses().await - } - - pub fn auth_pubkey(&self) -> Pubkey { - self.authority.pubkey() - } - - pub(crate) async fn reserve_pubkeys( - &self, - pubkeys: HashSet, - ) -> CommittorServiceResult<()> { - Ok(self - .table_mania - .reserve_pubkeys(&self.authority, &pubkeys) - .await?) - } - - pub(crate) async fn release_pubkeys(&self, pubkeys: HashSet) { - self.table_mania.release_pubkeys(&pubkeys).await - } - pub fn get_commit_statuses( &self, message_id: u64, @@ -174,25 +154,25 @@ impl CommittorProcessor { Ok(signatures) } + /// Fetches pending bundles from DB pub async fn pending_intent_bundles( &self, ) -> CommittorServiceResult> { - let recovery_time = unix_timestamp(); - let rows = self.persister.get_pending_commit_statuses( - recovery_time.saturating_sub(RECOVERY_MAX_AGE_SECS), + // Extract pending bundles satisfying predicate + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + let mut bundles = self.persister.pending_intent_bundles( + now.saturating_sub(RECOVERY_MAX_AGE_SECS), )?; - if rows.is_empty() { - return Ok(Vec::new()); + + if bundles.is_empty() { + return Ok(bundles); } - let recovery_base_slot = self.magicblock_rpc_client.get_slot().await?; - let bundles = pending_rows_to_scheduled_intent_bundles( - rows, - self.auth_pubkey(), - recovery_base_slot, - recovery_time, - ); - if !bundles.is_empty() { + // Log info about extracted bundles + { let accounts_count: usize = bundles .iter() .map(|bundle| bundle.get_all_committed_pubkeys().len()) @@ -204,11 +184,45 @@ impl CommittorProcessor { ); } + // Extracted bundles are out of data and missing some of the info + self.refresh_intent_bundles(&mut bundles).await?; + Ok(bundles) } + async fn refresh_intent_bundles( + &self, + intent_bundles: &mut [ScheduledIntentBundle], + ) -> CommittorServiceResult<()> { + let payer = self.authority.pubkey(); + let slot = self.magic_rpc_client.get_slot().await?; + + macro_rules! set_remote_slot { + ($field:expr, $slot:expr) => { + if let Some(ref mut v) = $field { + v.get_committed_accounts_mut() + .iter_mut() + .for_each(|a| a.remote_slot = slot); + } + }; + } + + intent_bundles.iter_mut().for_each(|b| { + b.payer = payer; + set_remote_slot!(b.intent_bundle.commit, slot); + set_remote_slot!(b.intent_bundle.commit_finalize, slot); + set_remote_slot!(b.intent_bundle.commit_and_undelegate, slot); + set_remote_slot!( + b.intent_bundle.commit_finalize_and_undelegate, + slot + ); + }); + + Ok(()) + } + #[instrument(skip(self, intent_bundles))] - pub async fn schedule_intent_bundle( + pub async fn schedule_intent_bundles( &self, intent_bundles: Vec, ) -> CommittorServiceResult<()> { @@ -229,6 +243,62 @@ impl CommittorProcessor { Ok(()) } + pub async fn execute_intent_bundles( + &self, + intent_bundles: Vec, + ) -> CommittorServiceResult> { + // Critical section + let (receivers, inserted_ids) = { + let mut result_listeners = self + .pending_result_listeners + .lock() + .expect(POISONED_MUTEX_MSG); + + let mut receivers = Vec::with_capacity(intent_bundles.len()); + let mut inserted_ids = Vec::with_capacity(intent_bundles.len()); + + for intent in &intent_bundles { + let (sender, receiver) = oneshot::channel(); + match result_listeners.entry(intent.id) { + Entry::Vacant(vacant) => { + vacant.insert(sender); + inserted_ids.push(intent.id); + receivers.push(receiver); + } + Entry::Occupied(_) => { + for id in &inserted_ids { + result_listeners.remove(id); + } + return Err( + CommittorServiceError::RepeatingMessageError( + intent.id, + ), + ); + } + } + } + (receivers, inserted_ids) + }; + + if let Err(err) = self.schedule_intent_bundles(intent_bundles).await { + let mut result_listeners = self + .pending_result_listeners + .lock() + .expect(POISONED_MUTEX_MSG); + for id in &inserted_ids { + result_listeners.remove(id); + } + return Err(err); + } + + let results = join_all(receivers.into_iter()) + .await + .into_iter() + .collect::, RecvError>>()?; + + Ok(results) + } + #[instrument(skip(self, intent_bundles))] pub async fn schedule_recovered_intent_bundles( &self, @@ -261,276 +331,48 @@ impl CommittorProcessor { .fetch_current_commit_nonces(pubkeys, min_context_slot) .await } -} - -fn pending_rows_to_scheduled_intent_bundles( - rows: Vec, - payer: Pubkey, - recovery_base_slot: u64, - recovery_time: u64, -) -> Vec { - let mut grouped_rows = BTreeMap::>::new(); - for row in rows { - grouped_rows.entry(row.message_id).or_default().push(row); - } - grouped_rows - .into_iter() - .filter_map(|(message_id, rows)| { - if rows.iter().any(|row| { - !pending_row_is_in_recovery_window(row, recovery_time) - }) - { - warn!( - intent_id = message_id, - "Skipping pending commit intent outside recovery window" - ); - return None; - } - - let first = rows.first()?; - let slot = first.slot; - let blockhash = first.ephemeral_blockhash; - if rows.iter().any(|r| { - r.slot != slot || r.ephemeral_blockhash != blockhash - }) { - warn!( - intent_id = message_id, - "Skipping pending commit intent: rows disagree on slot or ephemeral_blockhash" - ); - return None; - } - let mut commit_finalize_accounts = Vec::new(); - let mut commit_finalize_and_undelegate_accounts = Vec::new(); - - for row in rows { - let Some((account, undelegate)) = - committed_account_from_pending_row(row, recovery_base_slot) - else { + /// Dispatch worker + #[instrument(skip(pending_result_listeners, results_subscription))] + async fn dispatcher( + mut results_subscription: broadcast::Receiver< + BroadcastedIntentExecutionResult, + >, + pending_result_listeners: Arc< + Mutex>, + >, + ) { + loop { + let execution_result = match results_subscription.recv().await { + Ok(result) => result, + Err(broadcast::error::RecvError::Closed) => { + info!("Intent execution shutdown"); + break; + } + Err(broadcast::error::RecvError::Lagged(skipped)) => { + // SAFETY: not really feasible to happen as this function is way faster than Intent execution + // requires investigation if ever happens! + error!(skipped, "Dispatcher lag detected"); continue; - }; - if undelegate { - commit_finalize_and_undelegate_accounts.push(account); - } else { - commit_finalize_accounts.push(account); } - } - - let mut intent_bundle = MagicIntentBundle::default(); - if !commit_finalize_accounts.is_empty() { - intent_bundle.commit_finalize = - Some(CommitType::Standalone(commit_finalize_accounts)); - } - if !commit_finalize_and_undelegate_accounts.is_empty() { - intent_bundle.commit_finalize_and_undelegate = - Some(CommitAndUndelegate { - commit_action: CommitType::Standalone( - commit_finalize_and_undelegate_accounts, - ), - undelegate_action: UndelegateType::Standalone, - }); - } - if intent_bundle.is_empty() { - return None; - } - - Some(ScheduledIntentBundle { - id: message_id, - slot, - blockhash, - sent_transaction: Transaction::default(), - payer, - intent_bundle, - }) - }) - .collect() -} - -fn pending_row_is_in_recovery_window( - row: &CommitStatusRow, - recovery_time: u64, -) -> bool { - recovery_time.saturating_sub(row.created_at) <= RECOVERY_MAX_AGE_SECS -} - -fn unix_timestamp() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_secs() -} - -fn committed_account_from_pending_row( - row: CommitStatusRow, - recovery_base_slot: u64, -) -> Option<(CommittedAccount, bool)> { - if row.commit_type == PersistCommitType::DataAccount && row.data.is_none() { - warn!( - intent_id = row.message_id, - pubkey = %row.pubkey, - "Skipping pending data-account commit row without account data" - ); - return None; - } - - Some(( - CommittedAccount { - pubkey: row.pubkey, - account: Account { - lamports: row.lamports, - data: row.data.unwrap_or_default(), - owner: row.delegated_account_owner, - executable: false, - rent_epoch: 0, - }, - remote_slot: recovery_base_slot, - }, - row.undelegate, - )) -} - -#[cfg(test)] -mod tests { - use solana_hash::Hash; - - use super::*; - use crate::persist::CommitStatus; + }; - fn pending_row( - message_id: u64, - pubkey: Pubkey, - owner: Pubkey, - blockhash: Hash, - undelegate: bool, - data: Option>, - ) -> CommitStatusRow { - let commit_type = - if data.as_ref().map(|data| data.is_empty()) == Some(false) { - PersistCommitType::DataAccount + let sender = if let Some(sender) = pending_result_listeners + .lock() + .expect(POISONED_MUTEX_MSG) + .remove(&execution_result.id) + { + sender } else { - PersistCommitType::EmptyAccount + continue; }; - CommitStatusRow { - message_id, - pubkey, - commit_id: 0, - delegated_account_owner: owner, - slot: 42, - ephemeral_blockhash: blockhash, - undelegate, - lamports: 1_000, - data, - commit_type, - created_at: 1, - commit_status: CommitStatus::Pending, - commit_strategy: Default::default(), - last_retried_at: 1, - retries_count: 0, + if let Err(execution_result) = sender.send(execution_result) { + error!( + intent_id = execution_result.id, + "Failed to send execution result" + ); + } } } - - fn pending_row_with_timestamps( - message_id: u64, - created_at: u64, - last_retried_at: u64, - ) -> CommitStatusRow { - let mut row = pending_row( - message_id, - Pubkey::new_unique(), - Pubkey::new_unique(), - Hash::new_unique(), - false, - Some(vec![1]), - ); - row.created_at = created_at; - row.last_retried_at = last_retried_at; - row - } - - #[test] - fn pending_rows_reconstruct_commit_finalize_bundle() { - let payer = Pubkey::new_unique(); - let owner = Pubkey::new_unique(); - let blockhash = Hash::new_unique(); - let commit_pubkey = Pubkey::new_unique(); - let undelegate_pubkey = Pubkey::new_unique(); - - let bundles = pending_rows_to_scheduled_intent_bundles( - vec![ - pending_row( - 9, - commit_pubkey, - owner, - blockhash, - false, - Some(vec![1, 2, 3]), - ), - pending_row(9, undelegate_pubkey, owner, blockhash, true, None), - ], - payer, - 7, - 2, - ); - - assert_eq!(bundles.len(), 1); - let bundle = &bundles[0]; - assert_eq!(bundle.id, 9); - assert_eq!(bundle.slot, 42); - assert_eq!(bundle.blockhash, blockhash); - assert_eq!(bundle.payer, payer); - - let commit_accounts = - bundle.get_commit_finalize_intent_accounts().unwrap(); - assert_eq!(commit_accounts.len(), 1); - assert_eq!(commit_accounts[0].pubkey, commit_pubkey); - assert_eq!(commit_accounts[0].account.data, vec![1, 2, 3]); - assert_eq!(commit_accounts[0].remote_slot, 7); - - let undelegate_accounts = bundle - .get_commit_finalize_and_undelegate_intent_accounts() - .unwrap(); - assert_eq!(undelegate_accounts.len(), 1); - assert_eq!(undelegate_accounts[0].pubkey, undelegate_pubkey); - assert_eq!(undelegate_accounts[0].account.owner, owner); - assert!(bundle.has_undelegate_intent()); - } - - #[test] - fn pending_rows_skip_intents_older_than_recovery_window() { - let payer = Pubkey::new_unique(); - let recovery_time = RECOVERY_MAX_AGE_SECS + 1; - - let recoverable = pending_rows_to_scheduled_intent_bundles( - vec![pending_row_with_timestamps( - 1, - recovery_time - RECOVERY_MAX_AGE_SECS, - recovery_time, - )], - payer, - 7, - recovery_time, - ); - assert_eq!(recoverable.len(), 1); - - let recent = pending_rows_to_scheduled_intent_bundles( - vec![pending_row_with_timestamps(2, recovery_time, recovery_time)], - payer, - 7, - recovery_time, - ); - assert_eq!(recent.len(), 1); - - let too_old = pending_rows_to_scheduled_intent_bundles( - vec![pending_row_with_timestamps( - 3, - recovery_time - RECOVERY_MAX_AGE_SECS - 1, - recovery_time - RECOVERY_MAX_AGE_SECS - 1, - )], - payer, - 7, - recovery_time, - ); - assert!(too_old.is_empty()); - } } diff --git a/magicblock-committor-service/src/compute_budget.rs b/magicblock-committor-service/src/compute_budget.rs index 93a40e8e7..4a6b5e79f 100644 --- a/magicblock-committor-service/src/compute_budget.rs +++ b/magicblock-committor-service/src/compute_budget.rs @@ -1,26 +1,6 @@ use solana_compute_budget_interface::ComputeBudgetInstruction; use solana_instruction::Instruction; -// ----------------- -// Budgets -// ----------------- -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct Budget { - base_budget: u32, - per_committee: u32, - compute_unit_price: u64, -} - -impl Default for Budget { - fn default() -> Self { - Self { - base_budget: 80_000, - per_committee: 45_000, - compute_unit_price: 1_000_000, - } - } -} - #[derive(Debug, Clone)] pub struct BufferWithReallocBudget { base_budget: u32, @@ -67,20 +47,9 @@ impl BufferWriteChunkBudget { } } -// ----------------- -// ComputeBudgetConfig -// ----------------- #[derive(Debug, Clone)] pub struct ComputeBudgetConfig { pub compute_unit_price: u64, - pub args_process: Budget, - pub finalize: Budget, - pub buffer_close: Budget, - /// The budget used for processing and process + closing a buffer. - /// Since we mix pure process and process + close instructions, we need to - /// assume the worst case and use the process + close budget for all. - pub buffer_process_and_close: Budget, - pub undelegate: Budget, pub buffer_init: BufferWithReallocBudget, pub buffer_realloc: BufferWithReallocBudget, pub buffer_write: BufferWriteChunkBudget, @@ -90,31 +59,6 @@ impl ComputeBudgetConfig { pub fn new(compute_unit_price: u64) -> Self { Self { compute_unit_price, - args_process: Budget { - compute_unit_price, - base_budget: 80_000, - per_committee: 35_000, - }, - buffer_close: Budget { - compute_unit_price, - base_budget: 10_000, - per_committee: 25_000, - }, - buffer_process_and_close: Budget { - compute_unit_price, - base_budget: 40_000, - per_committee: 45_000, - }, - finalize: Budget { - compute_unit_price, - base_budget: 80_000, - per_committee: 25_000, - }, - undelegate: Budget { - compute_unit_price, - base_budget: 70_000, - per_committee: 35_000, - }, buffer_init: BufferWithReallocBudget { base_budget: 12_000, per_realloc_ix: 6_000, @@ -134,88 +78,6 @@ impl ComputeBudgetConfig { } } -impl ComputeBudgetConfig { - pub fn args_process_budget(&self) -> ComputeBudget { - ComputeBudget::Process(self.args_process) - } - pub fn buffer_close_budget(&self) -> ComputeBudget { - ComputeBudget::Close(self.buffer_close) - } - pub fn buffer_process_and_close_budget(&self) -> ComputeBudget { - ComputeBudget::ProcessAndClose(self.buffer_process_and_close) - } - pub fn finalize_budget(&self) -> ComputeBudget { - ComputeBudget::Finalize(self.finalize) - } - pub fn undelegate_budget(&self) -> ComputeBudget { - ComputeBudget::Undelegate(self.undelegate) - } -} - -// ----------------- -// ComputeBudget -// ----------------- -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum ComputeBudget { - Process(Budget), - Close(Budget), - ProcessAndClose(Budget), - Finalize(Budget), - Undelegate(Budget), -} - -impl ComputeBudget { - fn base_budget(&self) -> u32 { - use ComputeBudget::*; - match self { - Process(budget) => budget.base_budget, - Close(budget) => budget.base_budget, - ProcessAndClose(budget) => budget.base_budget, - Finalize(budget) => budget.base_budget, - Undelegate(budget) => budget.base_budget, - } - } - - fn per_committee(&self) -> u32 { - use ComputeBudget::*; - match self { - Process(budget) => budget.per_committee, - Close(budget) => budget.per_committee, - ProcessAndClose(budget) => budget.per_committee, - Finalize(budget) => budget.per_committee, - Undelegate(budget) => budget.per_committee, - } - } - - pub fn compute_unit_price(&self) -> u64 { - use ComputeBudget::*; - match self { - Process(budget) => budget.compute_unit_price, - Close(budget) => budget.compute_unit_price, - ProcessAndClose(budget) => budget.compute_unit_price, - Finalize(budget) => budget.compute_unit_price, - Undelegate(budget) => budget.compute_unit_price, - } - } - - fn total_budget(&self, committee_count: u32) -> u32 { - self.per_committee() - .checked_mul(committee_count) - .and_then(|product| product.checked_add(self.base_budget())) - .unwrap_or(u32::MAX) - } - - pub fn instructions(&self, committee_count: usize) -> Vec { - let committee_count = - u32::try_from(committee_count).unwrap_or(u32::MAX); - - instructions( - self.total_budget(committee_count), - self.compute_unit_price(), - ) - } -} - fn instructions( compute_budget: u32, compute_unit_price: u64, diff --git a/magicblock-committor-service/src/error.rs b/magicblock-committor-service/src/error.rs index fa13e49b8..a44977104 100644 --- a/magicblock-committor-service/src/error.rs +++ b/magicblock-committor-service/src/error.rs @@ -1,86 +1,26 @@ -use solana_pubkey::Pubkey; -use solana_signature::Signature; -use solana_transaction_error::TransactionError; use thiserror::Error; +use tokio::sync::oneshot::error::RecvError; -use crate::{ - intent_execution_manager::IntentExecutionManagerError, - intent_executor::task_info_fetcher::TaskInfoFetcherError, -}; +use crate::intent_execution_manager::IntentExecutionManagerError; pub type CommittorServiceResult = Result; #[derive(Error, Debug)] pub enum CommittorServiceError { - #[error("CommittorError: {0} ({0:?})")] - CommittorError(#[from] magicblock_committor_program::error::CommittorError), - #[error("CommitPersistError: {0} ({0:?})")] CommitPersistError(#[from] crate::persist::error::CommitPersistError), - #[error("MagicBlockRpcClientError: {0} ({0:?})")] - MagicBlockRpcClientError( - #[from] magicblock_rpc_client::MagicBlockRpcClientError, - ), - - #[error("TableManiaError: {0} ({0:?})")] - TableManiaError(#[from] magicblock_table_mania::error::TableManiaError), - #[error("IntentExecutionManagerError: {0} ({0:?})")] IntentExecutionManagerError(#[from] IntentExecutionManagerError), - #[error("TaskInfoFetcherError: {0} ({0:?})")] - TaskInfoFetcherError(#[from] TaskInfoFetcherError), - - #[error( - "Failed send and confirm transaction to {0} on chain: {1} ({1:?})" - )] - FailedToSendAndConfirmTransaction( - String, - magicblock_rpc_client::MagicBlockRpcClientError, - ), - - #[error("The transaction to {0} was sent and confirmed, but encountered an error: {1} ({1:?})")] - EncounteredTransactionError(String, TransactionError), - - #[error("Failed to send init changeset account: {0} ({0:?})")] - FailedToSendInitChangesetAccount( - Box, - ), - - #[error("Failed to confirm init changeset account: {0} ({0:?})")] - FailedToConfirmInitChangesetAccount( - Box, - ), - #[error("Init transaction '{0}' was not confirmed")] - InitChangesetAccountNotConfirmed(String), - - #[error("Task {0} failed to compile transaction message: {1} ({1:?})")] - FailedToCompileTransactionMessage(String, solana_message::CompileError), - - #[error("Task {0} failed to create transaction: {1} ({1:?})")] - FailedToCreateTransaction(String, solana_signer::SignerError), - - #[error("Could not find commit strategy for bundle {0}")] - CouldNotFindCommitStrategyForBundle(u64), + #[error("RecvError: {0}")] + IntentResultRecvError(#[from] RecvError), - #[error("Failed to fetch metadata account for {0}")] - FailedToFetchDelegationMetadata(Pubkey), - - #[error("Failed to deserialize metadata account for {0}, {1:?}")] - FailedToDeserializeDelegationMetadata( - Pubkey, - solana_program::program_error::ProgramError, + #[error("MagicBlockRpcClientError: {0} ({0:?})")] + MagicBlockRpcClientError( + #[from] magicblock_rpc_client::MagicBlockRpcClientError, ), -} -impl CommittorServiceError { - pub fn signature(&self) -> Option { - use CommittorServiceError::*; - match self { - MagicBlockRpcClientError(e) => e.signature(), - FailedToSendAndConfirmTransaction(_, e) => e.signature(), - _ => None, - } - } + #[error("Attempt to schedule already scheduled message id: {0}")] + RepeatingMessageError(u64), } diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 7b435a85a..593504f7e 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -283,7 +283,6 @@ where // Execute an Intent let result = executor.execute(intent.clone(), persister).await; - let execution_succeeded = result.inner.is_ok(); let _ = result.inner.as_ref().inspect_err(|err| { error!(intent_id = intent.id, error = ?err, "Failed to execute intent bundle"); }); @@ -308,24 +307,11 @@ where .complete(&intent) .expect("Valid completion of previously scheduled message"); - // Cleaning buffers on failure isn't safe due to potential race condition: - // Assume pubkey set A being committed - // Intent1 fails and cleans up, another Intent2 with set A executes right away - // That could lead for Intent2 init of buffers executing prior of Intent1 buffer cleanup - // With same set A buffers will have same address - // - // To avoid this race on buffers we cleanup only succesfully executed intents - // With intent retries all buffers will be eventually closed once intent succeeds - // - // Note: not sure this is safa for TableMania as it susceptible to same race condition - // unless it does handle it internally somehow - if execution_succeeded { - tokio::spawn(async move { - if let Err(err) = executor.cleanup().await { - error!(error = ?err, "Failed to cleanup after intent"); - } - }); - } + tokio::spawn(async move { + if let Err(err) = executor.cleanup().await { + error!(error = ?err, "Failed to cleanup after intent"); + } + }); // Free worker drop(execution_permit); diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index f7c8744bb..3a11bd45a 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -158,6 +158,10 @@ pub struct IntentExecutorImpl { pub started_at: Instant, /// Junk that needs to be cleaned up junk: Vec, + /// Set to false on execution failure so cleanup only releases ALT + /// reservations without closing buffer PDAs (see race condition note in + /// intent_execution_engine) + close_buffers: bool, } impl IntentExecutorImpl @@ -185,6 +189,7 @@ where started_at: Instant::now(), junk: vec![], + close_buffers: true, } } @@ -567,6 +572,7 @@ where }); }); + self.close_buffers = result.is_ok(); self.junk = execution_report.junk; IntentExecutionResult { inner: result, @@ -575,13 +581,14 @@ where } } - /// Cleanup after intent using junk async fn cleanup(mut self) -> Result<(), BufferExecutionError> { + let close_buffers = self.close_buffers; let cleanup_futs = self.junk.iter().map(|to_cleanup| { self.transaction_preparator.cleanup_for_strategy( &self.authority, &to_cleanup.optimized_tasks, &to_cleanup.lookup_tables_keys, + close_buffers, ) }); diff --git a/magicblock-committor-service/src/lib.rs b/magicblock-committor-service/src/lib.rs index 9836160f4..48d90930f 100644 --- a/magicblock-committor-service/src/lib.rs +++ b/magicblock-committor-service/src/lib.rs @@ -1,4 +1,4 @@ -mod committor_processor; +pub mod committor_processor; mod compute_budget; pub mod config; mod consts; @@ -6,16 +6,12 @@ pub mod error; pub mod intent_execution_manager; pub mod intent_executor; pub mod persist; -mod pubkeys_provider; -mod service; -pub mod service_ext; -#[cfg(feature = "dev-context-only-utils")] -pub mod stubs; pub mod tasks; pub mod transaction_preparator; pub mod transactions; pub(crate) mod utils; +pub mod service; #[cfg(test)] pub mod test_utils; @@ -24,4 +20,3 @@ pub use config::DEFAULT_ACTIONS_TIMEOUT; pub use magicblock_committor_program::{ ChangedAccount, Changeset, ChangesetMeta, }; -pub use service::{BaseIntentCommittor, CommittorService}; diff --git a/magicblock-committor-service/src/persist/commit_persister.rs b/magicblock-committor-service/src/persist/commit_persister.rs index 4d17be308..375dd7f59 100644 --- a/magicblock-committor-service/src/persist/commit_persister.rs +++ b/magicblock-committor-service/src/persist/commit_persister.rs @@ -1,11 +1,18 @@ use std::{ + collections::BTreeMap, path::Path, sync::{Arc, Mutex}, }; use magicblock_core::intent::CommittedAccount; -use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; +use magicblock_program::magic_scheduled_base_intent::{ + CommitAndUndelegate, CommitType as IntentCommitType, MagicIntentBundle, + ScheduledIntentBundle, UndelegateType, +}; +use solana_account::Account; use solana_pubkey::Pubkey; +use solana_transaction::Transaction; +use tracing::warn; use super::{ db::CommitStatusRow, error::CommitPersistResult, utils::now, CommitStatus, @@ -57,10 +64,10 @@ pub trait IntentPersister: Send + Sync + Clone + 'static { &self, message_id: u64, ) -> CommitPersistResult>; - fn get_pending_commit_statuses( + fn pending_intent_bundles( &self, min_created_at: u64, - ) -> CommitPersistResult>; + ) -> CommitPersistResult>; fn get_commit_status_by_message( &self, message_id: u64, @@ -247,14 +254,25 @@ impl IntentPersister for IntentPersisterImpl { .get_commit_statuses_by_id(message_id) } - fn get_pending_commit_statuses( + /// Returns pending bundles created at or after `min_created_at`. + /// NOTE: this constructs `ScheduleIntentBundle` only from existing information. + /// As persister doesn't save `ScheduleIntentBundle::payer` info, Pubkey::default is used. + /// `CommittedAccount` information like slot may also be outdated. + /// It is the responsibility of the calling site to refresh data in intent. + fn pending_intent_bundles( &self, min_created_at: u64, - ) -> CommitPersistResult> { - self.commits_db + ) -> CommitPersistResult> { + let rows = self + .commits_db .lock() .expect(POISONED_MUTEX_MSG) - .get_pending_commit_statuses(min_created_at) + .get_pending_commit_statuses(min_created_at)?; + + Ok(pending_rows_to_scheduled_intent_bundles( + rows, + min_created_at, + )) } fn get_commit_status_by_message( @@ -403,14 +421,12 @@ impl IntentPersister for Option { } } - fn get_pending_commit_statuses( + fn pending_intent_bundles( &self, min_created_at: u64, - ) -> CommitPersistResult> { + ) -> CommitPersistResult> { match self { - Some(persister) => { - persister.get_pending_commit_statuses(min_created_at) - } + Some(persister) => persister.pending_intent_bundles(min_created_at), None => Ok(Vec::new()), } } @@ -465,6 +481,126 @@ impl IntentPersister for Option { } } +fn pending_rows_to_scheduled_intent_bundles( + rows: Vec, + min_created_at: u64, +) -> Vec { + let mut grouped_rows = BTreeMap::>::new(); + for row in rows { + grouped_rows.entry(row.message_id).or_default().push(row); + } + + grouped_rows + .into_iter() + // Filter bundles where any row is older than the recovery window + .filter(|(message_id, rows)| { + if rows.iter().any(|row| row.created_at < min_created_at) { + warn!( + intent_id = message_id, + "Skipping pending commit intent: too old to recover" + ); + false + } else { + true + } + }) + // Filter malformed items + .filter_map(|(message_id, rows)| { + let first = rows.first()?; + let slot = first.slot; + let blockhash = first.ephemeral_blockhash; + if rows.iter().any(|r| r.slot != slot || r.ephemeral_blockhash != blockhash) { + warn!( + intent_id = message_id, + "Skipping pending commit intent: rows disagree on slot or ephemeral_blockhash" + ); + None + } else { + Some(rows) + } + }) + .filter_map(intent_bundle_from_rows) + .collect() +} + +fn intent_bundle_from_rows( + rows: Vec, +) -> Option { + let first = rows.first()?; + let message_id = first.message_id; + let slot = first.slot; + let blockhash = first.ephemeral_blockhash; + + let mut commit_finalize_accounts = Vec::new(); + let mut commit_finalize_and_undelegate_accounts = Vec::new(); + for row in rows { + let Some((account, undelegate)) = committed_account_from_row(row) + else { + continue; + }; + if undelegate { + commit_finalize_and_undelegate_accounts.push(account); + } else { + commit_finalize_accounts.push(account); + } + } + + let mut intent_bundle = MagicIntentBundle::default(); + if !commit_finalize_accounts.is_empty() { + intent_bundle.commit_finalize = + Some(IntentCommitType::Standalone(commit_finalize_accounts)); + } + if !commit_finalize_and_undelegate_accounts.is_empty() { + intent_bundle.commit_finalize_and_undelegate = + Some(CommitAndUndelegate { + commit_action: IntentCommitType::Standalone( + commit_finalize_and_undelegate_accounts, + ), + undelegate_action: UndelegateType::Standalone, + }); + } + if intent_bundle.is_empty() { + return None; + } + + Some(ScheduledIntentBundle { + id: message_id, + slot, + blockhash, + sent_transaction: Transaction::default(), + payer: Pubkey::default(), + intent_bundle, + }) +} + +fn committed_account_from_row( + row: CommitStatusRow, +) -> Option<(CommittedAccount, bool)> { + if row.commit_type == CommitType::DataAccount && row.data.is_none() { + warn!( + intent_id = row.message_id, + pubkey = %row.pubkey, + "Skipping pending data-account commit row without account data" + ); + return None; + } + + Some(( + CommittedAccount { + pubkey: row.pubkey, + account: Account { + lamports: row.lamports, + data: row.data.unwrap_or_default(), + owner: row.delegated_account_owner, + executable: false, + rent_epoch: 0, + }, + remote_slot: 0, + }, + row.undelegate, + )) +} + #[cfg(test)] mod tests { use magicblock_core::intent::CommittedAccount; @@ -480,6 +616,7 @@ mod tests { use super::*; use crate::{ + committor_processor::RECOVERY_MAX_AGE_SECS, persist::{types, CommitStatusSignatures}, test_utils, }; @@ -792,4 +929,139 @@ mod tests { let statuses = persister.get_commit_statuses_by_message(1).unwrap(); assert_eq!(statuses.len(), 0); } + + fn pending_row( + message_id: u64, + pubkey: Pubkey, + owner: Pubkey, + blockhash: Hash, + undelegate: bool, + data: Option>, + ) -> CommitStatusRow { + let commit_type = + if data.as_ref().map(|data| data.is_empty()) == Some(false) { + types::CommitType::DataAccount + } else { + types::CommitType::EmptyAccount + }; + CommitStatusRow { + message_id, + pubkey, + commit_id: 0, + delegated_account_owner: owner, + slot: 42, + ephemeral_blockhash: blockhash, + undelegate, + lamports: 1_000, + data, + commit_type, + created_at: 1, + commit_status: CommitStatus::Pending, + commit_strategy: Default::default(), + last_retried_at: 1, + retries_count: 0, + } + } + + #[test] + fn pending_rows_reconstruct_commit_finalize_bundle() { + let owner = Pubkey::new_unique(); + let blockhash = Hash::new_unique(); + let commit_pubkey = Pubkey::new_unique(); + let undelegate_pubkey = Pubkey::new_unique(); + + // default rows have created_at=1; use min_created_at=0 so nothing is filtered + let bundles = pending_rows_to_scheduled_intent_bundles( + vec![ + pending_row( + 9, + commit_pubkey, + owner, + blockhash, + false, + Some(vec![1, 2, 3]), + ), + pending_row(9, undelegate_pubkey, owner, blockhash, true, None), + ], + 0, + ); + + assert_eq!(bundles.len(), 1); + let bundle = &bundles[0]; + assert_eq!(bundle.id, 9); + assert_eq!(bundle.slot, 42); + assert_eq!(bundle.blockhash, blockhash); + + let commit_accounts = + bundle.get_commit_finalize_intent_accounts().unwrap(); + assert_eq!(commit_accounts.len(), 1); + assert_eq!(commit_accounts[0].pubkey, commit_pubkey); + assert_eq!(commit_accounts[0].account.data, vec![1, 2, 3]); + assert_eq!(commit_accounts[0].remote_slot, 0); + + let undelegate_accounts = bundle + .get_commit_finalize_and_undelegate_intent_accounts() + .unwrap(); + assert_eq!(undelegate_accounts.len(), 1); + assert_eq!(undelegate_accounts[0].pubkey, undelegate_pubkey); + assert_eq!(undelegate_accounts[0].account.owner, owner); + assert!(bundle.has_undelegate_intent()); + } + + #[test] + fn pending_rows_skip_intents_older_than_recovery_window() { + let owner = Pubkey::new_unique(); + let blockhash = Hash::new_unique(); + let recovery_time = RECOVERY_MAX_AGE_SECS + 1; + let min_created_at = recovery_time - RECOVERY_MAX_AGE_SECS; // = 1 + + // At the boundary → included + let mut in_window = pending_row( + 1, + Pubkey::new_unique(), + owner, + blockhash, + false, + Some(vec![1]), + ); + in_window.created_at = min_created_at; + let included = pending_rows_to_scheduled_intent_bundles( + vec![in_window], + min_created_at, + ); + assert_eq!(included.len(), 1); + assert_eq!(included[0].id, 1); + + // Recent (just created) → included + let mut recent = pending_row( + 2, + Pubkey::new_unique(), + owner, + blockhash, + false, + Some(vec![1]), + ); + recent.created_at = recovery_time; + let recent_result = pending_rows_to_scheduled_intent_bundles( + vec![recent], + min_created_at, + ); + assert_eq!(recent_result.len(), 1); + + // Too old → excluded + let mut too_old = pending_row( + 3, + Pubkey::new_unique(), + owner, + blockhash, + false, + Some(vec![1]), + ); + too_old.created_at = min_created_at - 1; // = 0 + let excluded_old = pending_rows_to_scheduled_intent_bundles( + vec![too_old], + min_created_at, + ); + assert!(excluded_old.is_empty()); + } } diff --git a/magicblock-committor-service/src/pubkeys_provider.rs b/magicblock-committor-service/src/pubkeys_provider.rs deleted file mode 100644 index 9995c174e..000000000 --- a/magicblock-committor-service/src/pubkeys_provider.rs +++ /dev/null @@ -1,67 +0,0 @@ -use std::collections::HashSet; - -use dlp_api::pda; -use solana_pubkey::Pubkey; -use solana_system_program::id as system_program_id; -use tracing::*; - -/// Returns all accounts needed to process/finalize a commit for the account -/// with the provided `delegated_account`. -/// NOTE: that buffer and chunk accounts are different for each commit and -/// thus are not included -pub fn provide_committee_pubkeys( - committee: &Pubkey, - owner_program: Option<&Pubkey>, -) -> HashSet { - let mut set = HashSet::new(); - set.insert(*committee); - set.insert(pda::delegation_record_pda_from_delegated_account(committee)); - set.insert(pda::delegation_metadata_pda_from_delegated_account( - committee, - )); - set.insert(pda::commit_state_pda_from_delegated_account(committee)); - set.insert(pda::commit_record_pda_from_delegated_account(committee)); - set.insert(pda::undelegate_buffer_pda_from_delegated_account(committee)); - - // NOTE: ideally we'd also include the rent_fee_payer here, but that is - // not known to the validator at the time of cloning since it is - // stored inside the delegation metadata account instead of the - // delegation record - - if let Some(owner_program) = owner_program { - set.insert(pda::program_config_from_program_id(owner_program)); - } else { - warn!(committee = %committee, "No owner program provided for committee"); - } - set -} - -/// Returns common accounts needed for process/finalize transactions, -/// namely the program ids used and the fees vaults and the validator itself. -pub fn provide_common_pubkeys(validator: &Pubkey) -> HashSet { - let mut set = HashSet::new(); - - let deleg_program = dlp_api::id(); - let protocol_fees_vault = pda::fees_vault_pda(); - let validator_fees_vault = - pda::validator_fees_vault_pda_from_validator(validator); - let committor_program = magicblock_committor_program::id(); - - trace!( - validator = %validator, - deleg_program = %deleg_program, - protocol_fees_vault = %protocol_fees_vault, - validator_fees_vault = %validator_fees_vault, - committor_program = %committor_program, - "Common pubkeys loaded" - ); - - set.insert(*validator); - set.insert(system_program_id()); - set.insert(deleg_program); - set.insert(protocol_fees_vault); - set.insert(validator_fees_vault); - set.insert(committor_program); - - set -} diff --git a/magicblock-committor-service/src/service.rs b/magicblock-committor-service/src/service.rs index b1415b8bc..cfcd4f10f 100644 --- a/magicblock-committor-service/src/service.rs +++ b/magicblock-committor-service/src/service.rs @@ -1,630 +1,529 @@ +pub mod intent_client; + use std::{ - collections::HashMap, - path::Path, - sync::{atomic::AtomicU64, Arc}, - time::Instant, + collections::{HashMap, HashSet}, + future::Future, + mem, + sync::{Arc, Mutex}, + time::Duration, }; -use magicblock_core::traits::ActionsCallbackScheduler; -use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; -use solana_keypair::Keypair; -use solana_pubkey::Pubkey; -use solana_signature::Signature; -use solana_transaction_status_client_types::EncodedConfirmedTransactionWithStatusMeta; +use futures_util::future::join_all; +use intent_client::{ERIntentClient, InternalIntentClientError}; +use magicblock_account_cloner::ChainlinkCloner; +use magicblock_chainlink::{ProdChainlink, ProdInnerChainlink}; +use magicblock_metrics::metrics::{self, AccountFetchOrigin}; +use magicblock_program::{ + magic_scheduled_base_intent::ScheduledIntentBundle, Pubkey, SentCommit, +}; +use solana_hash::Hash; +use solana_transaction::Transaction; use tokio::{ - select, - sync::{ - broadcast, - mpsc::{self, error::TrySendError}, - oneshot, - }, + sync::broadcast, + task, + task::{JoinError, JoinHandle}, }; -use tokio_util::sync::{CancellationToken, WaitForCancellationFutureOwned}; -use tracing::*; +use tokio_util::sync::CancellationToken; +use tracing::{error, info, instrument, warn}; use crate::{ - committor_processor::CommittorProcessor, - config::ChainConfig, - error::{CommittorServiceError, CommittorServiceResult}, + committor_processor::CommittorProcessor, error::CommittorServiceResult, intent_execution_manager::BroadcastedIntentExecutionResult, - persist::{CommitStatusRow, MessageSignatures}, - pubkeys_provider::{provide_committee_pubkeys, provide_common_pubkeys}, + intent_executor::ExecutionOutput, }; -#[derive(Debug)] -pub struct LookupTables { - pub active: Vec, - pub released: Vec, -} +const POISONED_MUTEX_MSG: &str = "ServiceInner intents_meta_map mutex poisoned"; -#[derive(Debug)] -pub enum CommittorMessage { - ReservePubkeysForCommittee { - /// When the request was initiated - initiated: Instant, - /// Called once the pubkeys have been reserved and includes that timestamp - /// at which the request was initiated - respond_to: oneshot::Sender>, - /// The committee whose pubkeys to reserve in a lookup table - /// These pubkeys are used to process/finalize the commit - committee: Pubkey, - /// The owner program of the committee - owner: Pubkey, - }, - ReserveCommonPubkeys { - /// Called once the pubkeys have been reserved - respond_to: oneshot::Sender>, - }, - ReleaseCommonPubkeys { - /// Called once the pubkeys have been released - respond_to: oneshot::Sender<()>, - }, - ScheduleIntentBundle { - /// The [`ScheduleIntentBundle`]s to commit - intent_bundles: Vec, - respond_to: oneshot::Sender>, - }, - GetPendingIntentBundles { - respond_to: - oneshot::Sender>>, - }, - ScheduleRecoveredIntentBundle { - /// Recovered [`ScheduleIntentBundle`]s to commit without re-persisting rows. - intent_bundles: Vec, - respond_to: oneshot::Sender>, - }, - GetCommitStatuses { - respond_to: - oneshot::Sender>>, - message_id: u64, - }, - GetCommitSignatures { - respond_to: - oneshot::Sender>>, - commit_id: u64, - pubkey: Pubkey, - }, - GetLookupTables { - respond_to: oneshot::Sender, - }, - GetTransaction { - respond_to: oneshot::Sender< - CommittorServiceResult, - >, - signature: Signature, - }, - SubscribeForResults { - respond_to: oneshot::Sender< - broadcast::Receiver, - >, - }, - FetchCurrentCommitNonces { - respond_to: - oneshot::Sender>>, - pubkeys: Vec, - min_context_slot: u64, - }, - FetchCurrentCommitNoncesSync { - respond_to: std::sync::mpsc::Sender< - CommittorServiceResult>, - >, - pubkeys: Vec, - min_context_slot: u64, - }, -} +pub type InnerChainlinkImpl = ProdInnerChainlink; +pub type ChainlinkImpl = ProdChainlink; -// ----------------- -// CommittorActor -// ----------------- -struct CommittorActor { - receiver: mpsc::Receiver, - processor: Arc, +pub enum IntentExecutionService { + Created(ServiceInner), + Started(JoinHandle<()>), + Stopped, + Error, } -impl CommittorActor { - pub fn try_new( - receiver: mpsc::Receiver, - authority: Keypair, - persist_file: P, - chain_config: ChainConfig, - chain_slot: Option>, - actions_callback_executor: A, - ) -> CommittorServiceResult - where - P: AsRef, - A: ActionsCallbackScheduler, - { - let processor = Arc::new(CommittorProcessor::try_new( - authority, - persist_file, - chain_config, - chain_slot, - actions_callback_executor, - )?); - - Ok(Self { - receiver, +impl IntentExecutionService +where + R: ERIntentClient, + R::Error: Into, +{ + pub fn new( + chainlink: Arc, + intent_rpc_client: R, + processor: Arc, + slot_interval: Duration, + cancellation_token: CancellationToken, + ) -> Self { + Self::Created(ServiceInner::new( + chainlink, + intent_rpc_client, processor, - }) + slot_interval, + cancellation_token, + )) } - #[instrument(skip(self))] - async fn handle_msg(&self, msg: CommittorMessage) { - use CommittorMessage::*; - match msg { - ReservePubkeysForCommittee { - initiated, - respond_to, - committee, - owner, - } => { - let processor = self.processor.clone(); - tokio::task::spawn(async move { - let pubkeys = - provide_committee_pubkeys(&committee, Some(&owner)); - // NOTE: we wait here until the reservation is done which causes the - // cloning of a particular account to be blocked. - // This leads to larger delays on the first clone of an account, but also - // ensures that the account could be committed via a lookup table later. - let result = processor - .reserve_pubkeys(pubkeys) - .await - .map(|_| initiated); - if let Err(e) = respond_to.send(result) { - error!(message_type = "ReservePubkeysForCommittee", error = ?e, "Failed to send response"); - } - }); - } - ReserveCommonPubkeys { respond_to } => { - let processor = self.processor.clone(); - tokio::task::spawn(async move { - let pubkeys = - provide_common_pubkeys(&processor.auth_pubkey()); - let reqid = processor.reserve_pubkeys(pubkeys).await; - if let Err(e) = respond_to.send(reqid) { - error!(message_type = "ReserveCommonPubkeys", error = ?e, "Failed to send response"); - } - }); - } - ReleaseCommonPubkeys { respond_to } => { - let processor = self.processor.clone(); - tokio::task::spawn(async move { - let pubkeys = - provide_common_pubkeys(&processor.auth_pubkey()); - processor.release_pubkeys(pubkeys).await; - if let Err(e) = respond_to.send(()) { - error!(message_type = "ReleaseCommonPubkeys", error = ?e, "Failed to send response"); - } - }); - } - ScheduleIntentBundle { - intent_bundles, - respond_to, - } => { - let result = - self.processor.schedule_intent_bundle(intent_bundles).await; - if let Err(e) = respond_to.send(result) { - error!(message_type = "ScheduleBaseIntents", error = ?e, "Failed to send response"); - } - } - GetPendingIntentBundles { respond_to } => { - let pending_intents = - self.processor.pending_intent_bundles().await; - if let Err(e) = respond_to.send(pending_intents) { - error!(message_type = "GetPendingIntentBundles", error = ?e, "Failed to send response"); - } - } - ScheduleRecoveredIntentBundle { - intent_bundles, - respond_to, - } => { - let result = self - .processor - .schedule_recovered_intent_bundles(intent_bundles) - .await; - if let Err(e) = respond_to.send(result) { - error!(message_type = "ScheduleRecoveredIntentBundle", error = ?e, "Failed to send response"); - } - } - GetCommitStatuses { - message_id, - respond_to, - } => { - let commit_statuses = - self.processor.get_commit_statuses(message_id); - if let Err(e) = respond_to.send(commit_statuses) { - error!(message_type = "GetCommitStatuses", error = ?e, "Failed to send response"); - } - } - GetCommitSignatures { - commit_id, - respond_to, - pubkey, - } => { - let sig = - self.processor.get_commit_signature(commit_id, pubkey); - if let Err(e) = respond_to.send(sig) { - error!(message_type = "GetCommitSignatures", error = ?e, "Failed to send response"); - } - } - GetTransaction { - signature, - respond_to, - } => { - let processor = self.processor.clone(); - tokio::task::spawn(async move { - let res = processor - .magicblock_rpc_client - .get_transaction(&signature, None) - .await - .map_err(Into::into); - if let Err(err) = respond_to.send(res) { - error!(message_type = "GetTransaction", error = ?err, "Failed to send response"); - } - }); - } - GetLookupTables { respond_to } => { - let active_tables = self.processor.active_lookup_tables().await; - let released_tables = - self.processor.released_lookup_tables().await; - if let Err(e) = respond_to.send(LookupTables { - active: active_tables, - released: released_tables, - }) { - error!(message_type = "GetLookupTables", error = ?e, "Failed to send response"); - } - } - SubscribeForResults { respond_to } => { - let subscription = self.processor.subscribe_for_results(); - if let Err(err) = respond_to.send(subscription) { - error!(message_type = "SubscribeForResults", error = ?err, "Failed to send response"); - } - } - FetchCurrentCommitNonces { - respond_to, - pubkeys, - min_context_slot, - } => { - let processor = self.processor.clone(); - tokio::spawn(async move { - let result = processor - .fetch_current_commit_nonces(&pubkeys, min_context_slot) - .await; - if let Err(err) = respond_to - .send(result.map_err(CommittorServiceError::from)) - { - error!(message_type = "FetchCurrentCommitNonces", error = ?err, "Failed to send response"); - } - }); - } - FetchCurrentCommitNoncesSync { - respond_to, - pubkeys, - min_context_slot, - } => { - let processor = self.processor.clone(); - tokio::spawn(async move { - let result = processor - .fetch_current_commit_nonces(&pubkeys, min_context_slot) - .await; - if let Err(err) = respond_to - .send(result.map_err(CommittorServiceError::from)) - { - error!(message_type = "FetchCurrentCommitNoncesSync", error = ?err, "Failed to send response"); - } - }); - } - } + fn take(&mut self) -> Self { + mem::replace(self, Self::Error) } - #[instrument(skip(self, cancel_token))] - pub async fn run(&mut self, cancel_token: CancellationToken) { - loop { - select! { - msg = self.receiver.recv() => { - if let Some(msg) = msg { - self.handle_msg(msg).await; - } else { - break; - } - } - _ = cancel_token.cancelled() => { - break; - } - } - } + pub fn start(&mut self) -> Result<(), IntentExecutionServiceError> { + let Self::Created(service) = self.take() else { + return Err(IntentExecutionServiceError::InvalidState( + "service must be in Created state to start".into(), + )); + }; - info!("Actor shutdown"); + let handle = service.start(); + *self = Self::Started(handle); + Ok(()) + } + + pub async fn stop(&mut self) -> Result<(), IntentExecutionServiceError> { + let Self::Started(handle) = self.take() else { + return Err(IntentExecutionServiceError::InvalidState( + "service must be in Started state to stop".into(), + )); + }; + + handle.await?; + *self = Self::Stopped; + Ok(()) } } -// ----------------- -// CommittorService -// ----------------- -pub struct CommittorService { - sender: mpsc::Sender, - cancel_token: CancellationToken, +pub struct ServiceInner { + /// Chainlink for notifying of undelegations + chainlink: Arc, + /// ER client specific for Intent needs. Could be switched to RpcClient + intent_rpc_client: Arc, + /// Processor of accepted intents + processor: Arc, + /// Time interval to scrape MagicContext(ER slot interval) + // TODO(edwin): can be removed if LatestBlocK moved into magicblock-core + slot_interval: Duration, + cancellation_token: CancellationToken, + /// Meta for ongoing executing intents + intents_meta_map: Arc>>, } -impl CommittorService { - pub fn try_start( - authority: Keypair, - persist_file: P, - chain_config: ChainConfig, - chain_slot: Option>, - actions_callback_executor: A, - ) -> CommittorServiceResult - where - P: AsRef, - A: ActionsCallbackScheduler, - { - debug!("Starting committor service"); - let (sender, receiver) = mpsc::channel(1_000); - let cancel_token = CancellationToken::new(); - { - let cancel_token = cancel_token.clone(); - let mut actor = CommittorActor::try_new( - receiver, - authority, - persist_file, - chain_config, - chain_slot, - actions_callback_executor, - )?; - tokio::spawn(async move { - actor.run(cancel_token).await; - }); +impl ServiceInner +where + R: ERIntentClient, + R::Error: Into, +{ + pub fn new( + chainlink: Arc, + intent_rpc_client: R, + processor: Arc, + slot_interval: Duration, + cancellation_token: CancellationToken, + ) -> Self { + Self { + chainlink, + intent_rpc_client: Arc::new(intent_rpc_client), + processor, + slot_interval, + cancellation_token, + intents_meta_map: Arc::new(Mutex::default()), } - Ok(Self { - sender, - cancel_token, - }) } - pub fn reserve_common_pubkeys( - &self, - ) -> oneshot::Receiver> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::ReserveCommonPubkeys { - respond_to: tx, - }); - rx + /// Starts 2 workers: one accepting intents, another awaiting and handling results + fn start(self) -> JoinHandle<()> { + let result_subscriber = self.processor.subscribe_for_results(); + let cancellation_token = self.cancellation_token.clone(); + tokio::spawn(Self::result_processor( + result_subscriber, + cancellation_token, + self.intents_meta_map.clone(), + self.intent_rpc_client.clone(), + )); + + tokio::task::spawn(self.accept_worker()) } - pub fn release_common_pubkeys(&self) -> oneshot::Receiver<()> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::ReleaseCommonPubkeys { - respond_to: tx, - }); - rx - } + async fn accept_worker(self) { + if let Err(err) = self.reschedule_pending_bundles().await { + error!(error = ?err, "Failed to reschedule pending bundles") + } - pub fn get_pending_intent_bundles( - &self, - ) -> oneshot::Receiver>> - { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetPendingIntentBundles { - respond_to: tx, - }); - rx + let mut interval = tokio::time::interval(self.slot_interval); + loop { + tokio::select! { + biased; + _ = self.cancellation_token.cancelled() => { + break; + } + _ = interval.tick() => { + let accept_result = self + .intent_rpc_client + .accept_scheduled_intents() + .await; + let intent_bundles = match accept_result { + Ok(value) => value, + Err(err) => { + error!("Failed to accept intents: {}", err); + continue; + } + }; + + if let Err(err) = self.schedule_intent_execution(intent_bundles).await { + error!("Failed to schedule intent execution: {}", err); + } + } + } + } } - pub fn schedule_recovered_intent_bundles( - &self, - intent_bundles: Vec, - ) -> oneshot::Receiver> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::ScheduleRecoveredIntentBundle { - intent_bundles, - respond_to: tx, - }); - rx - } + async fn reschedule_pending_bundles(&self) -> CommittorServiceResult<()> { + // Fetch pending bundles from DB + let mut bundles = + self.processor.pending_intent_bundles().await.inspect_err(|err| { + error!(error = ?err, "Failed to load pending intent bundles for recovery"); + })?; + if bundles.is_empty() { + return Ok(()); + } - pub fn get_commit_signatures( - &self, - commit_id: u64, - pubkey: Pubkey, - ) -> oneshot::Receiver>> - { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetCommitSignatures { - respond_to: tx, - commit_id, - pubkey, - }); - rx - } + // Retain only recoverable bundles + self.retain_recoverable_intent_bundles(&mut bundles).await; - pub fn get_lookup_tables(&self) -> oneshot::Receiver { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetLookupTables { respond_to: tx }); - rx + // Schedule without initial persisitance as bundle already exists in db + self.process_intent_bundles(bundles, |bundles| { + self.processor.schedule_recovered_intent_bundles(bundles) + }) + .await } - pub fn fetch_current_commit_nonces_sync( + async fn schedule_intent_execution( &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> std::sync::mpsc::Receiver>> - { - let (tx, rx) = std::sync::mpsc::channel(); - self.try_send(CommittorMessage::FetchCurrentCommitNoncesSync { - respond_to: tx, - pubkeys: pubkeys.to_vec(), - min_context_slot, - }); - rx - } - - fn try_send(&self, msg: CommittorMessage) { - if let Err(e) = self.sender.try_send(msg) { - match e { - TrySendError::Full(msg) => error!( - "Channel full, failed to send commit message {:?}", - msg - ), - TrySendError::Closed(msg) => error!( - "Channel closed, failed to send commit message {:?}", - msg - ), - } + intent_bundles: Vec, + ) -> CommittorServiceResult<()> { + if intent_bundles.is_empty() { + return Ok(()); } - } -} -impl BaseIntentCommittor for CommittorService { - fn reserve_pubkeys_for_committee( - &self, - committee: Pubkey, - owner: Pubkey, - ) -> oneshot::Receiver> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::ReservePubkeysForCommittee { - initiated: Instant::now(), - respond_to: tx, - committee, - owner, - }); - rx + metrics::inc_committor_intents_count_by(intent_bundles.len() as u64); + + self.process_intent_bundles(intent_bundles, |bundles| { + self.processor.schedule_intent_bundles(bundles) + }) + .await } - fn schedule_intent_bundles( + async fn process_intent_bundles( &self, intent_bundles: Vec, - ) -> oneshot::Receiver> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::ScheduleIntentBundle { - intent_bundles, - respond_to: tx, - }); - rx - } + schedule: F, + ) -> CommittorServiceResult<()> + where + F: FnOnce(Vec) -> Fut, + Fut: Future>, + { + if intent_bundles.is_empty() { + return Ok(()); + } - fn get_commit_statuses( - &self, - message_id: u64, - ) -> oneshot::Receiver>> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetCommitStatuses { - respond_to: tx, - message_id, - }); - rx + // Add metas for intent we schedule + let intent_ids: Vec; + let pubkeys_being_undelegated = { + let mut intent_metas = + self.intents_meta_map.lock().expect(POISONED_MUTEX_MSG); + let mut pubkeys_being_undelegated = HashSet::::new(); + intent_bundles.iter().for_each(|intent| { + intent_metas + .insert(intent.id, ScheduledBaseIntentMeta::new(intent)); + if let Some(undelegate) = intent.get_undelegate_intent_pubkeys() + { + pubkeys_being_undelegated.extend(undelegate); + } + }); + intent_ids = intent_bundles.iter().map(|b| b.id).collect(); + pubkeys_being_undelegated.into_iter().collect::>() + }; + + self.process_undelegation_requests(pubkeys_being_undelegated) + .await; + + let result = schedule(intent_bundles).await; + // If scheduling failed remove from map + if result.is_err() { + let mut intent_metas = + self.intents_meta_map.lock().expect(POISONED_MUTEX_MSG); + intent_ids.iter().for_each(|id| { + intent_metas.remove(id); + }); + } + result } - fn get_commit_signatures( - &self, - commit_id: u64, - pubkey: Pubkey, - ) -> oneshot::Receiver>> - { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetCommitSignatures { - respond_to: tx, - commit_id, - pubkey, - }); - rx + async fn process_undelegation_requests(&self, pubkeys: Vec) { + let mut join_set = task::JoinSet::new(); + for pubkey in pubkeys.into_iter() { + let chainlink = self.chainlink.clone(); + join_set.spawn(async move { + (pubkey, chainlink.undelegation_requested(pubkey).await) + }); + } + let sub_errors = join_set + .join_all() + .await + .into_iter() + .filter_map(|(pubkey, inner_result)| { + if let Err(err) = inner_result { + Some(format!( + "Subscribing to account {} failed: {}", + pubkey, err + )) + } else { + None + } + }) + .collect::>(); + if !sub_errors.is_empty() { + // Instead of aborting the entire commit we log an error here, however + // this means that the undelegated accounts stay in a problematic state + // in the validator and are not synced from chain. + // We could implement a retry mechanism inside of chainlink in the future. + error!( + error_count = sub_errors.len(), + "Failed to subscribe to accounts being undelegated" + ); + } } - fn subscribe_for_results( - &self, - ) -> oneshot::Receiver> - { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::SubscribeForResults { respond_to: tx }); - rx + #[instrument(skip( + result_subscription, + cancellation_token, + intents_meta_map, + intent_client + ))] + async fn result_processor( + mut result_subscription: broadcast::Receiver< + BroadcastedIntentExecutionResult, + >, + cancellation_token: CancellationToken, + intents_meta_map: Arc>>, + intent_client: Arc, + ) { + loop { + let execution_result = tokio::select! { + biased; + _ = cancellation_token.cancelled() => { + info!("Shutting down"); + return; + } + execution_result = result_subscription.recv() => { + match execution_result { + Ok(result) => result, + Err(broadcast::error::RecvError::Closed) => { + info!("Intent execution service shut down"); + break; + } + Err(broadcast::error::RecvError::Lagged(skipped)) => { + // SAFETY: This shouldn't happen as our tx execution is faster than Intent execution on Base layer + // If this ever happens it requires investigation + error!(skipped_count = skipped, "Lagging behind intent execution"); + continue; + } + } + } + }; + + if let Err(err) = ServiceInner::::process_execution_result( + &intent_client, + execution_result, + &intents_meta_map, + ) + .await + { + error!(error = ?err, "Failed process intent execution results"); + } + } } - fn get_transaction( - &self, - signature: &Signature, - ) -> oneshot::Receiver< - CommittorServiceResult, - > { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::GetTransaction { - respond_to: tx, - signature: *signature, - }); + async fn process_execution_result( + intent_client: &Arc, + execution_result: BroadcastedIntentExecutionResult, + intents_meta_map: &Arc>>, + ) -> Result<(), R::Error> { + // Create IntentMeta + let intent_id = execution_result.id; + // Remove intent from metas + let Some(mut intent_meta) = intents_meta_map + .lock() + .expect(POISONED_MUTEX_MSG) + .remove(&intent_id) + else { + // Possible if we have duplicate Intents + // First one will remove id from map and second could fail. + // This should not happen and needs investigation! + error!(intent_id, "Failed to find intent metadata"); + return Ok(()); + }; + + let sent_transaction = + mem::take(&mut intent_meta.intent_sent_transaction); + let sent_commit = ServiceInner::::build_sent_commit( + intent_id, + intent_meta, + execution_result, + ); + intent_client + .notify_commit_sent(sent_transaction, sent_commit) + .await?; + + Ok(()) + } - rx + fn build_sent_commit( + intent_id: u64, + intent_meta: ScheduledBaseIntentMeta, + result: BroadcastedIntentExecutionResult, + ) -> SentCommit { + let error_message = + result.as_ref().err().map(|err| format!("{:?}", err)); + let chain_signatures = match result.inner { + Ok(value) => match value { + ExecutionOutput::SingleStage(signature) => vec![signature], + ExecutionOutput::TwoStage { + commit_signature, + finalize_signature, + } => vec![commit_signature, finalize_signature], + }, + Err(err) => { + error!( + "Failed to commit intent: {}, slot: {}, blockhash: {}. {:?}", + intent_id, intent_meta.slot, intent_meta.blockhash, err + ); + err.signatures() + .map(|(commit, finalize)| { + finalize + .map(|finalize| vec![commit, finalize]) + .unwrap_or(vec![commit]) + }) + .unwrap_or_default() + } + }; + let patched_errors = result + .patched_errors + .iter() + .map(|err| { + info!("Patched intent: {}. error was: {}", intent_id, err); + err.to_string() + }) + .collect(); + + let callbacks_report = result + .callbacks_report + .iter() + .map(|r| match r { + Ok(sig) => { + format!("OK: {sig}") + } + Err(err) => { + error!( + "Callback failed to schedule: {}. error: {}", + intent_id, err + ); + format!("ERR: {err}") + } + }) + .collect(); + + SentCommit { + message_id: intent_id, + slot: intent_meta.slot, + blockhash: intent_meta.blockhash, + payer: intent_meta.payer, + chain_signatures, + included_pubkeys: intent_meta.included_pubkeys, + excluded_pubkeys: vec![], + requested_undelegation: intent_meta.requested_undelegation, + error_message, + patched_errors, + callbacks_scheduling_results: callbacks_report, + } } - fn fetch_current_commit_nonces( + /// Retains bundles whose accounts are still delegated + async fn retain_recoverable_intent_bundles( &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> oneshot::Receiver>> { - let (tx, rx) = oneshot::channel(); - self.try_send(CommittorMessage::FetchCurrentCommitNonces { - respond_to: tx, - pubkeys: pubkeys.to_vec(), - min_context_slot, + bundles: &mut Vec, + ) { + let results = join_all( + bundles.iter().map(|b| b.get_all_committed_pubkeys()).map( + |pubkeys| async move { + self.chainlink + .accounts_delegated_on_base_and_er( + &pubkeys, + AccountFetchOrigin::GetAccount, + ) + .await + }, + ), + ) + .await; + + let mut results_iter = results.into_iter(); + bundles.retain(|bundle| { + let Some(result) = results_iter.next() else { + error!("Results and bundles must have equal length"); + return false; + }; + match result { + Ok(delegated) if delegated.iter().all(|d| *d) => true, + Ok(_) => { + warn!( + intent_id = bundle.id, + "Skipping recovered commit intent because not all accounts are delegated on base and ER" + ); + false + } + Err(err) => { + error!( + intent_id = bundle.id, + error = ?err, + "Failed to verify recovered commit intent accounts" + ); + false + } + } }); - - rx } +} - fn stop(&self) { - self.cancel_token.cancel(); - } +struct ScheduledBaseIntentMeta { + slot: u64, + blockhash: Hash, + payer: Pubkey, + included_pubkeys: Vec, + intent_sent_transaction: Transaction, + requested_undelegation: bool, +} - fn stopped(&self) -> WaitForCancellationFutureOwned { - self.cancel_token.clone().cancelled_owned() +impl ScheduledBaseIntentMeta { + fn new(intent: &ScheduledIntentBundle) -> Self { + Self { + slot: intent.slot, + blockhash: intent.blockhash, + payer: intent.payer, + included_pubkeys: intent.get_all_committed_pubkeys(), + intent_sent_transaction: intent.sent_transaction.clone(), + requested_undelegation: intent.has_undelegate_intent(), + } } } -pub trait BaseIntentCommittor: Send + Sync + 'static { - /// Reserves pubkeys used in most commits in a lookup table - fn reserve_pubkeys_for_committee( - &self, - committee: Pubkey, - owner: Pubkey, - ) -> oneshot::Receiver>; - - /// Commits the changeset and returns - fn schedule_intent_bundles( - &self, - intent_bundles: Vec, - ) -> oneshot::Receiver>; - - /// Subscribes for results of BaseIntent execution - fn subscribe_for_results( - &self, - ) -> oneshot::Receiver>; - - /// Gets statuses of accounts that were committed as part of a request with provided message_id - fn get_commit_statuses( - &self, - message_id: u64, - ) -> oneshot::Receiver>>; - - /// Gets signatures for commit of particular accounts - fn get_commit_signatures( - &self, - commit_id: u64, - pubkey: Pubkey, - ) -> oneshot::Receiver>>; - - fn get_transaction( - &self, - signature: &Signature, - ) -> oneshot::Receiver< - CommittorServiceResult, - >; - - fn fetch_current_commit_nonces( - &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> oneshot::Receiver>>; - - /// Stops Committor service - fn stop(&self); - - /// Returns future which resolves once committor `stop` got called - fn stopped(&self) -> WaitForCancellationFutureOwned; +#[derive(thiserror::Error, Debug)] +pub enum IntentExecutionServiceError { + #[error("Invalid state: {0}")] + InvalidState(String), + #[error("JoinError: {0}")] + JoinError(#[from] JoinError), + #[error("IntentRpcClientError: {0}")] + IntentRpcClientError(#[from] InternalIntentClientError), } diff --git a/magicblock-committor-service/src/service/intent_client.rs b/magicblock-committor-service/src/service/intent_client.rs new file mode 100644 index 000000000..e22ee704a --- /dev/null +++ b/magicblock-committor-service/src/service/intent_client.rs @@ -0,0 +1,130 @@ +use std::sync::Arc; + +use async_trait::async_trait; +use magicblock_accounts_db::{traits::AccountsBank, AccountsDb}; +use magicblock_core::{ + link::transactions::{with_encoded, TransactionSchedulerHandle}, + traits::LatestBlockProvider, +}; +use magicblock_program::{ + instruction_utils::InstructionUtils, + magic_scheduled_base_intent::ScheduledIntentBundle, + register_scheduled_commit_sent, MagicContext, SentCommit, + TransactionScheduler, MAGIC_CONTEXT_PUBKEY, +}; +use solana_account::ReadableAccount; +use solana_transaction::Transaction; +use solana_transaction_error::TransactionError; +use tracing::{debug, error}; + +#[async_trait] +pub trait ERIntentClient: Send + Sync + 'static { + type Error: std::error::Error + Send; + + /// Executes `Accept` tx and returns accepted intents + async fn accept_scheduled_intents( + &self, + ) -> Result, Self::Error>; + + /// Processes intent results, submitting them on chain(ER) + async fn notify_commit_sent( + &self, + sent_tx: Transaction, + sent_commit: SentCommit, + ) -> Result<(), Self::Error>; + + // TODO(edwin): probably more proper place to load pending intent + // CommittorProcessor::pending_intent_bundles could be moved here in the future +} + +pub struct InternalIntentRpcClient { + /// Provides access to MagicContext + accounts_db: Arc, + /// Internal endpoint for scheduling ER TXs + transaction_scheduler: TransactionSchedulerHandle, + /// Provides access to ER latest block for TX creation + latest_block_provider: L, +} + +impl InternalIntentRpcClient { + pub fn new( + accounts_db: Arc, + transaction_scheduler: TransactionSchedulerHandle, + latest_block_provider: L, + ) -> Self { + Self { + accounts_db, + transaction_scheduler, + latest_block_provider, + } + } + + /// Sends transaction to move the scheduled commits from the `MagicContext` + /// to the global ScheduledCommit store + async fn send_accept_tx(&self) -> Result<(), InternalIntentClientError> { + let tx = InstructionUtils::accept_scheduled_commits( + self.latest_block_provider.blockhash(), + ); + let encoded_tx = with_encoded(tx).inspect_err(|err| { + error!(error = ?err, "Failed to bincode intent transaction"); + })?; + self.transaction_scheduler + .execute(encoded_tx) + .await + .inspect_err(|err| { + error!(error = ?err, "Failed to accept scheduled commits"); + })?; + + Ok(()) + } +} + +#[async_trait] +impl ERIntentClient for InternalIntentRpcClient { + type Error = InternalIntentClientError; + + async fn accept_scheduled_intents( + &self, + ) -> Result, Self::Error> { + // If accounts were scheduled to be committed, we accept them here + // and processs the commits + let magic_context_acc = + self.accounts_db.get_account(&MAGIC_CONTEXT_PUBKEY).expect( + "Validator found to be running without MagicContext account!", + ); + if !MagicContext::has_scheduled_commits(magic_context_acc.data()) { + return Ok(vec![]); + } + self.send_accept_tx().await?; + + // Return intents from global store + Ok(TransactionScheduler::default().take_scheduled_intent_bundles()) + } + + async fn notify_commit_sent( + &self, + sent_tx: Transaction, + sent_commit: SentCommit, + ) -> Result<(), Self::Error> { + register_scheduled_commit_sent(sent_commit); + let txn = with_encoded(sent_tx).inspect_err(|err| { + // Unreachable case, all intent transactions are smaller than 64KB by construction + error!(error = ?err, "Failed to bincode intent transaction"); + })?; + self.transaction_scheduler + .execute(txn) + .await + .inspect(|_| debug!("Sent commit signaled")) + .inspect_err( + |err| error!(error = ?err, "Failed to signal sent commit"), + )?; + + Ok(()) + } +} + +#[derive(thiserror::Error, Debug)] +pub enum InternalIntentClientError { + #[error("TransactionError: {0}")] + TransactionError(#[from] TransactionError), +} diff --git a/magicblock-committor-service/src/service_ext.rs b/magicblock-committor-service/src/service_ext.rs deleted file mode 100644 index 024d0423e..000000000 --- a/magicblock-committor-service/src/service_ext.rs +++ /dev/null @@ -1,253 +0,0 @@ -use std::{ - collections::{hash_map::Entry, HashMap}, - ops::Deref, - sync::{Arc, Mutex}, - time::Instant, -}; - -use async_trait::async_trait; -use futures_util::future::join_all; -use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; -use solana_pubkey::Pubkey; -use solana_signature::Signature; -use solana_transaction_status_client_types::EncodedConfirmedTransactionWithStatusMeta; -use tokio::sync::{broadcast, oneshot, oneshot::error::RecvError}; -use tokio_util::sync::WaitForCancellationFutureOwned; -use tracing::{error, info, instrument}; - -use crate::{ - error::{CommittorServiceError, CommittorServiceResult}, - intent_execution_manager::BroadcastedIntentExecutionResult, - persist::{CommitStatusRow, MessageSignatures}, - BaseIntentCommittor, -}; - -const POISONED_MUTEX_MSG: &str = - "CommittorServiceExt pending messages mutex poisoned!"; - -#[async_trait] -pub trait BaseIntentCommittorExt: BaseIntentCommittor { - /// Schedules Base Intents and waits for their results - async fn schedule_intent_bundles_waiting( - &self, - intent_bundles: Vec, - ) -> BaseIntentCommitorExtResult>; -} - -type MessageResultListener = oneshot::Sender; -pub struct CommittorServiceExt { - inner: Arc, - pending_messages: Arc>>, -} - -impl CommittorServiceExt { - pub fn new(inner: Arc) -> Self { - let pending_messages = Arc::new(Mutex::new(HashMap::new())); - let results_subscription = inner.subscribe_for_results(); - let committor_stopped = inner.stopped(); - tokio::spawn(Self::dispatcher( - committor_stopped, - results_subscription, - pending_messages.clone(), - )); - - Self { - inner, - pending_messages, - } - } - - #[instrument(skip( - committor_stopped, - pending_message, - results_subscription - ))] - async fn dispatcher( - committor_stopped: WaitForCancellationFutureOwned, - results_subscription: oneshot::Receiver< - broadcast::Receiver, - >, - pending_message: Arc>>, - ) { - let mut results_subscription = results_subscription.await.unwrap(); - - tokio::pin!(committor_stopped); - loop { - let execution_result = tokio::select! { - biased; - _ = &mut committor_stopped => { - info!("Shutting down extension"); - return; - } - execution_result = results_subscription.recv() => { - match execution_result { - Ok(result) => result, - Err(broadcast::error::RecvError::Closed) => { - info!("Intent execution shutdown"); - break; - } - Err(broadcast::error::RecvError::Lagged(skipped)) => { - // SAFETY: not really feasible to happen as this function is way faster than Intent execution - // requires investigation if ever happens! - error!(skipped, "Dispatcher lag detected"); - continue; - } - } - } - }; - - let sender = if let Some(sender) = pending_message - .lock() - .expect(POISONED_MUTEX_MSG) - .remove(&execution_result.id) - { - sender - } else { - continue; - }; - - if let Err(execution_result) = sender.send(execution_result) { - error!( - intent_id = execution_result.id, - "Failed to send execution result" - ); - } - } - } -} - -#[async_trait] -impl BaseIntentCommittorExt - for CommittorServiceExt -{ - async fn schedule_intent_bundles_waiting( - &self, - base_intents: Vec, - ) -> BaseIntentCommitorExtResult> - { - // Critical section - let receivers = { - let mut pending_messages = - self.pending_messages.lock().expect(POISONED_MUTEX_MSG); - - base_intents - .iter() - .map(|intent| { - let (sender, receiver) = oneshot::channel(); - match pending_messages.entry(intent.id) { - Entry::Vacant(vacant) => { - vacant.insert(sender); - Ok(receiver) - } - Entry::Occupied(_) => Err( - CommittorServiceExtError::RepeatingMessageError( - intent.id, - ), - ), - } - }) - .collect::, _>>()? - }; - - self.schedule_intent_bundles(base_intents).await??; - let results = join_all(receivers.into_iter()) - .await - .into_iter() - .collect::, RecvError>>()?; - - Ok(results) - } -} - -impl BaseIntentCommittor for CommittorServiceExt { - fn reserve_pubkeys_for_committee( - &self, - committee: Pubkey, - owner: Pubkey, - ) -> oneshot::Receiver> { - self.inner.reserve_pubkeys_for_committee(committee, owner) - } - - fn schedule_intent_bundles( - &self, - intent_bundles: Vec, - ) -> oneshot::Receiver> { - self.inner.schedule_intent_bundles(intent_bundles) - } - - fn subscribe_for_results( - &self, - ) -> oneshot::Receiver> - { - self.inner.subscribe_for_results() - } - - fn get_commit_statuses( - &self, - message_id: u64, - ) -> oneshot::Receiver>> { - self.inner.get_commit_statuses(message_id) - } - - fn get_commit_signatures( - &self, - commit_id: u64, - pubkey: Pubkey, - ) -> oneshot::Receiver>> - { - self.inner.get_commit_signatures(commit_id, pubkey) - } - - fn get_transaction( - &self, - signature: &Signature, - ) -> oneshot::Receiver< - CommittorServiceResult, - > { - self.inner.get_transaction(signature) - } - - fn fetch_current_commit_nonces( - &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> oneshot::Receiver>> { - self.inner - .fetch_current_commit_nonces(pubkeys, min_context_slot) - } - - fn stop(&self) { - self.inner.stop(); - } - - fn stopped(&self) -> WaitForCancellationFutureOwned { - self.inner.stopped() - } -} - -impl Deref for CommittorServiceExt { - type Target = Arc; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -#[derive(thiserror::Error, Debug)] -pub enum CommittorServiceExtError { - #[error("Attempt to schedule already scheduled message id: {0}")] - RepeatingMessageError(u64), - #[error("RecvError: {0}")] - RecvError(#[from] RecvError), - #[error("CommittorServiceError: {0:?}")] - CommittorServiceError(Box), -} - -impl From for CommittorServiceExtError { - fn from(e: CommittorServiceError) -> Self { - Self::CommittorServiceError(Box::new(e)) - } -} - -pub type BaseIntentCommitorExtResult = - Result; diff --git a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs deleted file mode 100644 index 0b417db47..000000000 --- a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs +++ /dev/null @@ -1,299 +0,0 @@ -use std::{ - collections::HashMap, - sync::{Arc, Mutex}, - time::{Instant, SystemTime, UNIX_EPOCH}, -}; - -use async_trait::async_trait; -use magicblock_program::magic_scheduled_base_intent::{ - CommitType, ScheduledIntentBundle, UndelegateType, -}; -use solana_account::Account; -use solana_pubkey::Pubkey; -use solana_signature::Signature; -use solana_transaction_status_client_types::{ - EncodedConfirmedTransactionWithStatusMeta, EncodedTransaction, - EncodedTransactionWithStatusMeta, -}; -use tokio::sync::{broadcast, oneshot}; -use tokio_util::sync::{CancellationToken, WaitForCancellationFutureOwned}; - -use crate::{ - error::CommittorServiceResult, - intent_execution_manager::BroadcastedIntentExecutionResult, - intent_executor::ExecutionOutput, - persist::{CommitStatusRow, IntentPersisterImpl, MessageSignatures}, - service_ext::{BaseIntentCommitorExtResult, BaseIntentCommittorExt}, - BaseIntentCommittor, -}; - -#[derive(Default)] -pub struct ChangesetCommittorStub { - cancellation_token: CancellationToken, - reserved_pubkeys_for_committee: Arc>>, - #[allow(clippy::type_complexity)] - committed_changesets: Arc>>, - committed_accounts: Arc>>, -} - -impl ChangesetCommittorStub { - #[allow(clippy::len_without_is_empty)] - pub fn len(&self) -> usize { - self.committed_changesets.lock().unwrap().len() - } - - pub fn committed(&self, pubkey: &Pubkey) -> Option { - self.committed_accounts.lock().unwrap().get(pubkey).cloned() - } -} - -impl BaseIntentCommittor for ChangesetCommittorStub { - fn reserve_pubkeys_for_committee( - &self, - committee: Pubkey, - owner: Pubkey, - ) -> oneshot::Receiver> { - let initiated = Instant::now(); - let (tx, rx) = oneshot::channel::>(); - self.reserved_pubkeys_for_committee - .lock() - .unwrap() - .insert(committee, owner); - - tx.send(Ok(initiated)).unwrap_or_else(|_| { - tracing::error!( - message_type = "ReservePubkeysForCommittee", - "Failed to send response" - ); - }); - rx - } - - fn schedule_intent_bundles( - &self, - intent_bundles: Vec, - ) -> oneshot::Receiver> { - let (sender, receiver) = oneshot::channel(); - let _ = sender.send(Ok(())); - - { - let mut committed_accounts = - self.committed_accounts.lock().unwrap(); - intent_bundles.iter().for_each(|intent| { - intent - .get_all_committed_accounts() - .iter() - .for_each(|account| { - committed_accounts - .insert(account.pubkey, account.account.clone()); - }) - }) - } - - { - let mut changesets = self.committed_changesets.lock().unwrap(); - intent_bundles.into_iter().for_each(|intent| { - changesets.insert(intent.id, intent); - }); - } - - receiver - } - - fn subscribe_for_results( - &self, - ) -> oneshot::Receiver> - { - let (_, receiver) = oneshot::channel(); - receiver - } - - fn get_commit_statuses( - &self, - message_id: u64, - ) -> oneshot::Receiver>> { - let (tx, rx) = oneshot::channel(); - - let commit = self - .committed_changesets - .lock() - .unwrap() - .remove(&message_id); - let Some(base_intent) = commit else { - tx.send(Ok(vec![])).unwrap_or_else(|_| { - tracing::error!( - message_type = "GetCommitStatuses", - "Failed to send response" - ); - }); - return rx; - }; - - let status_rows = IntentPersisterImpl::create_commit_rows(&base_intent); - tx.send(Ok(status_rows)).unwrap_or_else(|_| { - tracing::error!( - message_type = "GetCommitStatuses", - "Failed to send response" - ); - }); - - rx - } - - fn get_commit_signatures( - &self, - _commit_id: u64, - _pubkey: Pubkey, - ) -> oneshot::Receiver>> - { - let (tx, rx) = oneshot::channel(); - let message_signature = MessageSignatures { - commit_stage_signature: Signature::new_unique(), - finalize_stage_signature: Some(Signature::new_unique()), - created_at: now(), - }; - - tx.send(Ok(Some(message_signature))).unwrap_or_else(|_| { - tracing::error!( - message_type = "GetCommitSignatures", - "Failed to send response" - ); - }); - - rx - } - - fn get_transaction( - &self, - _: &Signature, - ) -> oneshot::Receiver< - CommittorServiceResult, - > { - let (tx, rx) = oneshot::channel(); - if let Err(_err) = - tx.send(Ok(EncodedConfirmedTransactionWithStatusMeta { - slot: 0, - transaction: EncodedTransactionWithStatusMeta { - transaction: EncodedTransaction::LegacyBinary( - "".to_string(), - ), - meta: None, - version: None, - }, - block_time: None, - })) - { - tracing::error!( - message_type = "GetTransaction", - "Failed to send response" - ); - }; - - rx - } - - fn fetch_current_commit_nonces( - &self, - pubkeys: &[Pubkey], - _min_context_slot: u64, - ) -> oneshot::Receiver>> { - let (tx, rx) = oneshot::channel(); - let nonces = pubkeys.iter().map(|p| (*p, 0u64)).collect(); - tx.send(Ok(nonces)).unwrap_or_else(|_| { - tracing::error!( - message_type = "FetchCurrentCommitNonces", - "Failed to send response" - ); - }); - rx - } - - fn stop(&self) { - self.cancellation_token.cancel(); - } - - fn stopped(&self) -> WaitForCancellationFutureOwned { - self.cancellation_token.clone().cancelled_owned() - } -} - -#[async_trait] -impl BaseIntentCommittorExt for ChangesetCommittorStub { - async fn schedule_intent_bundles_waiting( - &self, - intent_bundles: Vec, - ) -> BaseIntentCommitorExtResult> - { - self.schedule_intent_bundles(intent_bundles.clone()) - .await??; - let res = intent_bundles - .into_iter() - .map(|message| { - let callbacks_report = (0..count_bundle_callbacks(&message)) - .map(|_| Ok(Signature::new_unique())) - .collect(); - BroadcastedIntentExecutionResult { - id: message.id, - inner: Ok(ExecutionOutput::TwoStage { - commit_signature: Signature::new_unique(), - finalize_signature: Signature::new_unique(), - }), - patched_errors: Arc::new(vec![]), - callbacks_report, - } - }) - .collect::>(); - - Ok(res) - } -} - -fn count_bundle_callbacks(bundle: &ScheduledIntentBundle) -> usize { - let ib = &bundle.intent_bundle; - - let from_commit = ib - .commit - .as_ref() - .map(|ct| match ct { - CommitType::Standalone(_) => 0, - CommitType::WithBaseActions { base_actions, .. } => { - base_actions.iter().filter(|a| a.callback.is_some()).count() - } - }) - .unwrap_or(0); - - let from_cau = ib - .commit_and_undelegate - .as_ref() - .map(|cau| { - let from_commit_action = match &cau.commit_action { - CommitType::Standalone(_) => 0, - CommitType::WithBaseActions { base_actions, .. } => { - base_actions.iter().filter(|a| a.callback.is_some()).count() - } - }; - let from_undelegate = match &cau.undelegate_action { - UndelegateType::Standalone => 0, - UndelegateType::WithBaseActions(actions) => { - actions.iter().filter(|a| a.callback.is_some()).count() - } - }; - from_commit_action + from_undelegate - }) - .unwrap_or(0); - - let from_standalone = ib - .standalone_actions - .iter() - .filter(|a| a.callback.is_some()) - .count(); - - from_commit + from_cau + from_standalone -} - -fn now() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_secs() -} diff --git a/magicblock-committor-service/src/stubs/mod.rs b/magicblock-committor-service/src/stubs/mod.rs deleted file mode 100644 index 9cfb6e45c..000000000 --- a/magicblock-committor-service/src/stubs/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -mod changeset_committor_stub; -pub use changeset_committor_stub::ChangesetCommittorStub; diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index 426004a62..b55cbd7dc 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -19,6 +19,7 @@ use crate::{ pub struct TransactionStrategy { pub optimized_tasks: Vec, pub lookup_tables_keys: Vec, + // TODO(edwin): remove this pub standalone_action_nonce: Option, } diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index fc4e47e84..f5d7a8c9e 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -192,7 +192,7 @@ impl DeliveryPreparator { let preparation_task = preparation_task.clone(); let cleanup_task = preparation_task.cleanup_task(); - self.cleanup(authority, &[cleanup_task], &[]).await?; + self.cleanup_buffers(authority, &[cleanup_task]).await?; self.rpc_client.invalidate_cached_blockhash().await; // Restore preparation stage for retry @@ -439,20 +439,35 @@ impl DeliveryPreparator { Ok(alts) } - /// Releases pubkeys from TableMania and - /// cleans up after buffer tasks + /// Releases pubkeys from TableMania and cleans up after buffer tasks. + // + // Cleaning buffers on failure isn't safe due to potential race condition: + // Assume pubkey set A being committed + // Intent1 fails and cleans up, another Intent2 with set A executes right away + // That could lead for Intent2 init of buffers executing prior of Intent1 buffer cleanup + // With same set A buffers will have same address + // + // To avoid this race on buffers we cleanup only succesfully executed intents + // With intent retries all buffers will be eventually closed once intent succeeds pub async fn cleanup( &self, authority: &Keypair, cleanup_tasks: &[CleanupTask], lookup_table_keys: &[Pubkey], + close_buffers: bool, ) -> DeliveryPreparatorResult<(), BufferExecutionError> { - let fut1 = self.cleanup_buffers(authority, cleanup_tasks); let alt_set = HashSet::from_iter(lookup_table_keys.iter().cloned()); let fut2 = self.table_mania.release_pubkeys(&alt_set); - let (res, ()) = join(fut1, fut2).await; - res + if close_buffers { + let (res, ()) = + join(self.cleanup_buffers(authority, cleanup_tasks), fut2) + .await; + res + } else { + fut2.await; + Ok(()) + } } async fn cleanup_buffers( diff --git a/magicblock-committor-service/src/transaction_preparator/mod.rs b/magicblock-committor-service/src/transaction_preparator/mod.rs index 1f5b514fc..08f3af024 100644 --- a/magicblock-committor-service/src/transaction_preparator/mod.rs +++ b/magicblock-committor-service/src/transaction_preparator/mod.rs @@ -34,12 +34,14 @@ pub trait TransactionPreparator: Send + Sync + 'static { intent_persister: &Option

, ) -> PreparatorResult; - /// Cleans up after strategy + /// Cleans up after strategy. + /// `close_buffers`: if false, only ALT reservations are released. async fn cleanup_for_strategy( &self, authority: &Keypair, tasks: &[BaseTaskImpl], lookup_table_keys: &[Pubkey], + close_buffers: bool, ) -> DeliveryPreparatorResult<(), BufferExecutionError>; } @@ -118,6 +120,7 @@ impl TransactionPreparator for TransactionPreparatorImpl { authority: &Keypair, tasks: &[BaseTaskImpl], lookup_table_keys: &[Pubkey], + close_buffers: bool, ) -> DeliveryPreparatorResult<(), BufferExecutionError> { let cleanup_tasks: Vec<_> = tasks .iter() @@ -136,7 +139,12 @@ impl TransactionPreparator for TransactionPreparatorImpl { }) .collect(); self.delivery_preparator - .cleanup(authority, &cleanup_tasks, lookup_table_keys) + .cleanup( + authority, + &cleanup_tasks, + lookup_table_keys, + close_buffers, + ) .await } } diff --git a/magicblock-committor-service/src/transactions.rs b/magicblock-committor-service/src/transactions.rs index 422b7c83b..2c25c7d42 100644 --- a/magicblock-committor-service/src/transactions.rs +++ b/magicblock-committor-service/src/transactions.rs @@ -1,631 +1,12 @@ use solana_packet::PACKET_DATA_SIZE; use solana_rpc_client::rpc_client::SerializableTransaction; -use static_assertions::const_assert; /// Maximum serialized transaction size that can be sent over the wire. pub(crate) const MAX_TRANSACTION_WIRE_SIZE: usize = PACKET_DATA_SIZE; -/// How many process and commit buffer instructions fit into a single transaction -#[allow(unused)] // serves as documentation as well -pub const MAX_PROCESS_PER_TX: u8 = 3; - -/// How many process and commit buffer instructions fit into a single transaction -/// when using address lookup tables but not including the buffer account in the -/// lookup table -#[allow(unused)] // serves as documentation as well -pub const MAX_PROCESS_PER_TX_USING_LOOKUP: u8 = 12; - -/// How many close buffer instructions fit into a single transaction -#[allow(unused)] // serves as documentation as well -pub const MAX_CLOSE_PER_TX: u8 = 8; - -/// How many close buffer instructions fit into a single transaction -/// when using address lookup tables but not including the buffer account -/// nor chunk account in the lookup table -#[allow(unused)] // serves as documentation as well -pub const MAX_CLOSE_PER_TX_USING_LOOKUP: u8 = 8; - -/// How many process and commit buffer instructions combined with close buffer instructions -/// fit into a single transaction -#[allow(unused)] // serves as documentation as well -pub const MAX_PROCESS_AND_CLOSE_PER_TX: u8 = 2; - -/// How many process and commit buffer instructions combined with -/// close buffer instructions fit into a single transaction when -/// using lookup tables but not including the buffer account -#[allow(unused)] // serves as documentation as well -pub const MAX_PROCESS_AND_CLOSE_PER_TX_USING_LOOKUP: u8 = 4; - -/// How many finalize instructions fit into a single transaction -#[allow(unused)] // serves as documentation as well -pub const MAX_FINALIZE_PER_TX: u8 = 5; - -/// How many finalize instructions fit into a single transaction -/// when using address lookup tables -#[allow(unused)] // serves as documentation as well -pub const MAX_FINALIZE_PER_TX_USING_LOOKUP: u8 = 48; - -/// How many undelegate instructions fit into a single transaction -/// NOTE: that we assume the rent reimbursement account to be the delegated account -#[allow(unused)] // serves as documentation as well -pub const MAX_UNDELEGATE_PER_TX: u8 = 3; - -/// How many undelegate instructions fit into a single transaction -/// when using address lookup tables -/// NOTE: that we assume the rent reimbursement account to be the delegated account -#[allow(unused)] // serves as documentation as well -pub const MAX_UNDELEGATE_PER_TX_USING_LOOKUP: u8 = 15; - -// Allows us to run undelegate instructions without rechunking them since we know -// that we didn't process more than we also can undelegate -const_assert!(MAX_PROCESS_PER_TX <= MAX_UNDELEGATE_PER_TX,); - -// Allows us to run undelegate instructions using lookup tables without rechunking -// them since we know that we didn't process more than we also can undelegate -const_assert!( - MAX_PROCESS_PER_TX_USING_LOOKUP <= MAX_UNDELEGATE_PER_TX_USING_LOOKUP -); - pub fn serialized_transaction_size( transaction: &impl SerializableTransaction, ) -> usize { // SAFETY: runs on transactions we already serialize before sending. usize::try_from(bincode::serialized_size(transaction).unwrap()).unwrap() } - -#[cfg(test)] -mod test { - use std::collections::HashSet; - - use dlp_api::args::{CommitStateArgs, CommitStateFromBufferArgs}; - use lazy_static::lazy_static; - use magicblock_committor_program::instruction_builder::close_buffer::{ - create_close_ix, CreateCloseIxArgs, - }; - use solana_address_lookup_table_interface::state::LOOKUP_TABLE_MAX_ADDRESSES; - use solana_hash::Hash; - use solana_instruction::Instruction; - use solana_keypair::Keypair; - use solana_message::{ - v0::Message, AddressLookupTableAccount, VersionedMessage, - }; - use solana_pubkey::Pubkey; - use solana_signer::Signer; - use solana_transaction::versioned::VersionedTransaction; - use tracing::info; - - use super::*; - use crate::{ - compute_budget::{Budget, ComputeBudget}, - error::{ - CommittorServiceError::{ - FailedToCompileTransactionMessage, FailedToCreateTransaction, - }, - CommittorServiceResult, - }, - pubkeys_provider::{provide_committee_pubkeys, provide_common_pubkeys}, - test_utils, - }; - - fn get_lookup_tables( - ixs: &[Instruction], - ) -> Vec { - let pubkeys = ixs - .iter() - .flat_map(|ix| ix.accounts.iter().map(|acc| acc.pubkey)) - .collect::>(); - - let lookup_table = AddressLookupTableAccount { - key: Pubkey::default(), - addresses: pubkeys.into_iter().collect(), - }; - vec![lookup_table] - } - - // ----------------- - // Helpers - // ----------------- - #[allow(clippy::enum_variant_names)] - enum TransactionOpts { - NoLookupTable, - UseLookupTable, - } - fn transaction_wire_size( - auth: &Keypair, - ixs: &[Instruction], - opts: &TransactionOpts, - ) -> CommittorServiceResult { - use TransactionOpts::*; - let lookup_tables = match opts { - NoLookupTable => vec![], - UseLookupTable => get_lookup_tables(ixs), - }; - - let versioned_msg = Message::try_compile( - &auth.pubkey(), - ixs, - &lookup_tables, - Hash::default(), - ) - .map_err(|err| { - FailedToCompileTransactionMessage( - "Calculating transaction size".to_string(), - err, - ) - })?; - let versioned_tx = VersionedTransaction::try_new( - VersionedMessage::V0(versioned_msg), - &[&auth], - ) - .map_err(|err| { - FailedToCreateTransaction( - "Calculating transaction size".to_string(), - err, - ) - })?; - - Ok(serialized_transaction_size(&versioned_tx)) - } - - // ----------------- - // Process Commitables and Close Buffers - // ----------------- - pub(crate) fn process_commits_ix( - validator_auth: Pubkey, - pubkey: &Pubkey, - delegated_account_owner: &Pubkey, - buffer_pda: &Pubkey, - commit_args: CommitStateFromBufferArgs, - ) -> Instruction { - dlp_api::instruction_builder::commit_state_from_buffer( - validator_auth, - *pubkey, - *delegated_account_owner, - *buffer_pda, - commit_args, - ) - } - - pub(crate) fn close_buffers_ix( - validator_auth: Pubkey, - pubkey: &Pubkey, - commit_id: u64, - ) -> Instruction { - create_close_ix(CreateCloseIxArgs { - authority: validator_auth, - pubkey: *pubkey, - commit_id, - }) - } - - pub(crate) fn process_and_close_ixs( - validator_auth: Pubkey, - pubkey: &Pubkey, - delegated_account_owner: &Pubkey, - buffer_pda: &Pubkey, - commit_id: u64, - commit_args: CommitStateFromBufferArgs, - ) -> Vec { - let process_ix = process_commits_ix( - validator_auth, - pubkey, - delegated_account_owner, - buffer_pda, - commit_args, - ); - let close_ix = close_buffers_ix(validator_auth, pubkey, commit_id); - - vec![process_ix, close_ix] - } - - // ----------------- - // Finalize - // ----------------- - pub(crate) fn finalize_ix( - validator_auth: Pubkey, - pubkey: &Pubkey, - ) -> Instruction { - dlp_api::instruction_builder::finalize(validator_auth, *pubkey) - } - - // These tests statically determine the optimal ix count to fit into a single - // transaction and assert that the const we export in prod match those numbers. - // Thus when an instruction changes and one of those numbers with it a failing - // test alerts us. - // This is less overhead than running those static functions each time at - // startup. - - #[test] - fn test_max_process_per_tx() { - test_utils::init_test_logger(); - assert_eq!(super::MAX_PROCESS_PER_TX, *MAX_PROCESS_PER_TX); - assert_eq!( - super::MAX_PROCESS_PER_TX_USING_LOOKUP, - *MAX_PROCESS_PER_TX_USING_LOOKUP - ); - } - - #[test] - fn test_max_close_per_tx() { - test_utils::init_test_logger(); - assert_eq!(super::MAX_CLOSE_PER_TX, *MAX_CLOSE_PER_TX); - assert_eq!( - super::MAX_CLOSE_PER_TX_USING_LOOKUP, - *MAX_CLOSE_PER_TX_USING_LOOKUP - ); - } - - #[test] - fn test_max_process_and_closes_per_tx() { - test_utils::init_test_logger(); - assert_eq!( - super::MAX_PROCESS_AND_CLOSE_PER_TX, - *MAX_PROCESS_AND_CLOSE_PER_TX - ); - assert_eq!( - super::MAX_PROCESS_AND_CLOSE_PER_TX_USING_LOOKUP, - *MAX_PROCESS_AND_CLOSE_PER_TX_USING_LOOKUP - ); - } - - #[test] - fn test_max_finalize_per_tx() { - test_utils::init_test_logger(); - assert_eq!(super::MAX_FINALIZE_PER_TX, *MAX_FINALIZE_PER_TX); - assert_eq!( - super::MAX_FINALIZE_PER_TX_USING_LOOKUP, - *MAX_FINALIZE_PER_TX_USING_LOOKUP - ); - } - - #[test] - fn test_max_undelegate_per_tx() { - test_utils::init_test_logger(); - assert_eq!(super::MAX_UNDELEGATE_PER_TX, *MAX_UNDELEGATE_PER_TX); - assert_eq!( - super::MAX_UNDELEGATE_PER_TX_USING_LOOKUP, - *MAX_UNDELEGATE_PER_TX_USING_LOOKUP - ); - } - - // ----------------- - // Process Commitables using Args - // ----------------- - #[test] - fn test_log_commit_args_ix_sizes() { - test_utils::init_test_logger(); - // This test is used to investigate the size of the transaction related to - // the amount of committed accounts and their data size. - fn run(auth: &Keypair, ixs: usize) { - let mut tx_lines = vec![]; - use TransactionOpts::*; - for tx_opts in [NoLookupTable, UseLookupTable] { - let mut tx_sizes = vec![]; - for size in [0, 10, 20, 50, 100, 200, 500, 1024] { - let ixs = (0..ixs) - .map(|_| make_ix(auth, size)) - .collect::>(); - - let tx_size = - transaction_wire_size(auth, &ixs, &tx_opts).unwrap(); - tx_sizes.push((size, tx_size)); - } - tx_lines.push(tx_sizes); - } - let sizes = tx_lines - .into_iter() - .map(|line| { - line.into_iter() - .map(|(size, len)| format!("{:4}:{:5}", size, len)) - .collect::>() - .join("|") - }) - .collect::>() - .join("\n"); - info!(instruction_count = ixs, sizes = %sizes, "Transaction size report"); - } - fn make_ix(auth: &Keypair, data_size: usize) -> Instruction { - let data = vec![1; data_size]; - let args = CommitStateArgs { - data, - ..CommitStateArgs::default() - }; - dlp_api::instruction_builder::commit_state( - auth.pubkey(), - Pubkey::new_unique(), - Pubkey::new_unique(), - args, - ) - } - - let auth = &Keypair::new(); - run(auth, 0); - run(auth, 1); - run(auth, 2); - run(auth, 5); - run(auth, 8); - run(auth, 10); - run(auth, 15); - run(auth, 20); - // This logs raw wire sizes. The sendability limit is - // MAX_TRANSACTION_WIRE_SIZE. - } - - // ----------------- - // Process Commitables and Close Buffers - // ----------------- - lazy_static! { - pub(crate) static ref MAX_PROCESS_PER_TX: u8 = { - max_chunks_per_transaction("Max process per tx", |auth_pubkey| { - let pubkey = Pubkey::new_unique(); - let delegated_account_owner = Pubkey::new_unique(); - let buffer_pda = Pubkey::new_unique(); - let commit_args = CommitStateFromBufferArgs::default(); - vec![process_commits_ix( - auth_pubkey, - &pubkey, - &delegated_account_owner, - &buffer_pda, - commit_args, - )] - }) - }; - pub(crate) static ref MAX_PROCESS_PER_TX_USING_LOOKUP: u8 = { - max_chunks_per_transaction_using_lookup_table( - "Max process per tx using lookup", - |auth_pubkey, committee, delegated_account_owner| { - let buffer_pda = Pubkey::new_unique(); - let commit_args = CommitStateFromBufferArgs::default(); - vec![process_commits_ix( - auth_pubkey, - &committee, - &delegated_account_owner, - &buffer_pda, - commit_args, - )] - }, - None, - ) - }; - pub(crate) static ref MAX_CLOSE_PER_TX: u8 = { - let commit_id = 0; - max_chunks_per_transaction("Max close per tx", |auth_pubkey| { - let pubkey = Pubkey::new_unique(); - vec![close_buffers_ix(auth_pubkey, &pubkey, commit_id)] - }) - }; - pub(crate) static ref MAX_CLOSE_PER_TX_USING_LOOKUP: u8 = { - let commit_id = 0; - max_chunks_per_transaction_using_lookup_table( - "Max close per tx using lookup", - |auth_pubkey, committee, _| { - vec![close_buffers_ix(auth_pubkey, &committee, commit_id)] - }, - None, - ) - }; - pub(crate) static ref MAX_PROCESS_AND_CLOSE_PER_TX: u8 = { - let commit_id = 0; - max_chunks_per_transaction( - "Max process and close per tx", - |auth_pubkey| { - let pubkey = Pubkey::new_unique(); - let delegated_account_owner = Pubkey::new_unique(); - let buffer_pda = Pubkey::new_unique(); - let commit_args = CommitStateFromBufferArgs::default(); - process_and_close_ixs( - auth_pubkey, - &pubkey, - &delegated_account_owner, - &buffer_pda, - commit_id, - commit_args, - ) - }, - ) - }; - pub(crate) static ref MAX_PROCESS_AND_CLOSE_PER_TX_USING_LOOKUP: u8 = { - let commit_id = 0; - max_chunks_per_transaction_using_lookup_table( - "Max process and close per tx using lookup", - |auth_pubkey, committee, delegated_account_owner| { - let commit_args = CommitStateFromBufferArgs::default(); - let buffer_pda = Pubkey::new_unique(); - process_and_close_ixs( - auth_pubkey, - &committee, - &delegated_account_owner, - &buffer_pda, - commit_id, - commit_args, - ) - }, - None, - ) - }; - } - - // ----------------- - // Finalize - // ----------------- - lazy_static! { - pub(crate) static ref MAX_FINALIZE_PER_TX: u8 = { - max_chunks_per_transaction("Max finalize per tx", |auth_pubkey| { - let pubkey = Pubkey::new_unique(); - vec![finalize_ix(auth_pubkey, &pubkey)] - }) - }; - pub(crate) static ref MAX_FINALIZE_PER_TX_USING_LOOKUP: u8 = { - max_chunks_per_transaction_using_lookup_table( - "Max finalize per tx using lookup", - |auth_pubkey, committee, _| { - vec![finalize_ix(auth_pubkey, &committee)] - }, - Some(40), - ) - }; - } - - // ----------------- - // Undelegate - // ----------------- - lazy_static! { - pub(crate) static ref MAX_UNDELEGATE_PER_TX: u8 = { - max_chunks_per_transaction("Max undelegate per tx", |auth_pubkey| { - let pubkey = Pubkey::new_unique(); - let owner_program = Pubkey::new_unique(); - vec![dlp_api::instruction_builder::undelegate( - auth_pubkey, - pubkey, - owner_program, - auth_pubkey, - )] - }) - }; - pub(crate) static ref MAX_UNDELEGATE_PER_TX_USING_LOOKUP: u8 = { - max_chunks_per_transaction_using_lookup_table( - "Max undelegate per tx using lookup", - |auth_pubkey, committee, owner_program| { - vec![dlp_api::instruction_builder::undelegate( - auth_pubkey, - committee, - owner_program, - auth_pubkey, - )] - }, - None, - ) - }; - } - - // ----------------- - // Max Chunks Per Transaction - // ----------------- - - fn max_chunks_per_transaction Vec>( - label: &str, - create_ixs: F, - ) -> u8 { - info!(test_label = label, "Starting transaction chunking test"); - - let auth = Keypair::new(); - let auth_pubkey = auth.pubkey(); - // NOTE: the size of the budget instructions is always the same, no matter - // which budget we provide - let mut ixs = ComputeBudget::Process(Budget::default()).instructions(1); - let mut chunks = 0_u8; - loop { - ixs.extend(create_ixs(auth_pubkey)); - chunks += 1; - - // SAFETY: runs statically - let versioned_msg = - Message::try_compile(&auth_pubkey, &ixs, &[], Hash::default()) - .unwrap(); - // SAFETY: runs statically - let versioned_tx = VersionedTransaction::try_new( - VersionedMessage::V0(versioned_msg), - &[&auth], - ) - .unwrap(); - let tx_size = serialized_transaction_size(&versioned_tx); - info!( - chunks = chunks, - size_bytes = tx_size, - "Transaction size measured" - ); - if tx_size > MAX_TRANSACTION_WIRE_SIZE { - return chunks - 1; - } - } - } - - fn extend_lookup_table( - lookup_table: &mut AddressLookupTableAccount, - auth_pubkey: Pubkey, - committee: Pubkey, - owner: Option<&Pubkey>, - ) { - let keys = provide_committee_pubkeys(&committee, owner) - .into_iter() - .chain(provide_common_pubkeys(&auth_pubkey)) - .chain(lookup_table.addresses.iter().cloned()) - .collect::>(); - lookup_table.addresses = keys.into_iter().collect(); - assert!( - lookup_table.addresses.len() <= LOOKUP_TABLE_MAX_ADDRESSES, - "Lookup table has too many ({}) addresses", - lookup_table.addresses.len() - ); - } - - fn max_chunks_per_transaction_using_lookup_table< - FI: Fn(Pubkey, Pubkey, Pubkey) -> Vec, - >( - label: &str, - create_ixs: FI, - start_at: Option, - ) -> u8 { - info!(test_label = label, start_at = ?start_at, "Starting lookup table transaction test"); - let auth = Keypair::new(); - let auth_pubkey = auth.pubkey(); - let mut ixs = ComputeBudget::Process(Budget::default()).instructions(1); - let mut chunks = start_at.unwrap_or_default(); - let mut lookup_table = AddressLookupTableAccount { - key: Pubkey::default(), - addresses: vec![], - }; - // If we start at specific chunk size let's prep the ixs and assume - // we are using the same addresses to avoid blowing out the lookup table - if chunks > 0 { - let committee = Pubkey::new_unique(); - let owner_program = Pubkey::new_unique(); - extend_lookup_table( - &mut lookup_table, - auth_pubkey, - committee, - Some(&owner_program), - ); - for _ in 0..chunks { - ixs.extend(create_ixs(auth_pubkey, committee, owner_program)); - } - } - loop { - let committee = Pubkey::new_unique(); - let owner_program = Pubkey::new_unique(); - ixs.extend(create_ixs(auth_pubkey, committee, owner_program)); - - chunks += 1; - extend_lookup_table( - &mut lookup_table, - auth_pubkey, - committee, - Some(&owner_program), - ); - - // SAFETY: runs statically - let versioned_msg = Message::try_compile( - &auth_pubkey, - &ixs, - &[lookup_table.clone()], - Hash::default(), - ) - .unwrap(); - // SAFETY: runs statically - let versioned_tx = VersionedTransaction::try_new( - VersionedMessage::V0(versioned_msg), - &[&auth], - ) - .unwrap(); - let tx_size = serialized_transaction_size(&versioned_tx); - info!( - chunks = chunks, - size_bytes = tx_size, - "Transaction size measured with lookup table" - ); - if tx_size > MAX_TRANSACTION_WIRE_SIZE { - return chunks - 1; - } - } - } -} diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index a536c20a2..44f2d5db2 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -4181,20 +4181,15 @@ version = "0.12.2" dependencies = [ "async-trait", "bincode", - "magicblock-accounts-db", "magicblock-chainlink", - "magicblock-committor-service", - "magicblock-config", "magicblock-core", "magicblock-ledger", "magicblock-magic-program-api 0.12.2", "magicblock-program", - "magicblock-rpc-client", "rand 0.9.2", "solana-account", "solana-hash 3.1.0", "solana-instruction", - "solana-loader-v3-interface", "solana-loader-v4-interface", "solana-pubkey 3.0.0", "solana-sdk-ids", @@ -4212,21 +4207,11 @@ name = "magicblock-accounts" version = "0.12.2" dependencies = [ "async-trait", - "magicblock-account-cloner", - "magicblock-accounts-db", - "magicblock-chainlink", "magicblock-committor-service", - "magicblock-core", - "magicblock-metrics", - "magicblock-program", - "solana-hash 3.1.0", "solana-pubkey 3.0.0", - "solana-transaction", "solana-transaction-error", "thiserror 2.0.18", "tokio", - "tokio-util 0.7.17", - "tracing", "url", ] @@ -4454,6 +4439,9 @@ dependencies = [ "borsh", "futures-util", "lru 0.16.2", + "magicblock-account-cloner", + "magicblock-accounts-db", + "magicblock-chainlink", "magicblock-committor-program", "magicblock-core", "magicblock-delegation-program-api 0.3.0 (git+https://github.com/magicblock-labs/delegation-program.git?rev=25386a7c1d406d06b8d07a4d5b0fd37d5e74213b)", @@ -4464,7 +4452,6 @@ dependencies = [ "rusqlite", "solana-account", "solana-account-decoder", - "solana-address-lookup-table-interface", "solana-commitment-config", "solana-compute-budget-interface", "solana-hash 3.1.0", @@ -4478,11 +4465,9 @@ dependencies = [ "solana-rpc-client-api", "solana-signature", "solana-signer", - "solana-system-program", "solana-transaction", "solana-transaction-error", "solana-transaction-status-client-types", - "static_assertions", "thiserror 2.0.18", "tokio", "tokio-util 0.7.17", diff --git a/test-integration/test-committor-service/Cargo.toml b/test-integration/test-committor-service/Cargo.toml index 53e29921d..6035b5ec8 100644 --- a/test-integration/test-committor-service/Cargo.toml +++ b/test-integration/test-committor-service/Cargo.toml @@ -12,9 +12,7 @@ magicblock-core = { workspace = true } magicblock-committor-program = { workspace = true, features = [ "no-entrypoint", ] } -magicblock-committor-service = { workspace = true, features = [ - "dev-context-only-utils", -] } +magicblock-committor-service = { workspace = true } magicblock-delegation-program-api = { workspace = true } magicblock-program = { workspace = true } magicblock-rpc-client = { workspace = true } diff --git a/test-integration/test-committor-service/tests/test_intent_executor.rs b/test-integration/test-committor-service/tests/test_intent_executor.rs index 5355379d3..26acbc58a 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -834,6 +834,7 @@ async fn test_cpi_limits_error_recovery() { &fixture.authority, &to_cleanup.optimized_tasks, &to_cleanup.lookup_tables_keys, + true, ) }); assert!(try_join_all(cleanup_futs).await.is_ok()); @@ -981,6 +982,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { &fixture.authority, &to_cleanup.optimized_tasks, &to_cleanup.lookup_tables_keys, + true, ) }); assert!(try_join_all(cleanup_futs).await.is_ok()); diff --git a/test-integration/test-committor-service/tests/test_ix_commit_local.rs b/test-integration/test-committor-service/tests/test_ix_commit_local.rs index c34cdfdea..72ae07121 100644 --- a/test-integration/test-committor-service/tests/test_ix_commit_local.rs +++ b/test-integration/test-committor-service/tests/test_ix_commit_local.rs @@ -1,23 +1,18 @@ -use std::{ - collections::{HashMap, HashSet}, - sync::Arc, - time::{Duration, Instant}, -}; +use std::{collections::HashMap, sync::Arc}; use borsh::to_vec; use magicblock_committor_service::{ + committor_processor::CommittorProcessor, config::ChainConfig, intent_executor::{error::IntentExecutorError, ExecutionOutput}, persist::CommitStrategy, - service_ext::{BaseIntentCommittorExt, CommittorServiceExt}, - BaseIntentCommittor, CommittorService, ComputeBudgetConfig, + ComputeBudgetConfig, }; use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, ScheduledIntentBundle, UndelegateType, }; -use magicblock_rpc_client::MagicblockRpcClient; use program_flexi_counter::state::FlexiCounter; use solana_account::{Account, ReadableAccount}; use solana_commitment_config::CommitmentConfig; @@ -244,15 +239,16 @@ async fn commit_single_account( fund_validator_auth_and_ensure_validator_fees_vault(&validator_auth).await; // Run each test with and without finalizing - let service = CommittorService::try_start( - validator_auth.insecure_clone(), - ":memory:", - ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), - None, - common::MockActionsCallbackExecutor::default(), - ) - .unwrap(); - let service = CommittorServiceExt::new(Arc::new(service)); + let processor = Arc::new( + CommittorProcessor::try_new( + validator_auth.insecure_clone(), + ":memory:", + ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), + None, + common::MockActionsCallbackExecutor::default(), + ) + .unwrap(), + ); let counter_auth = Keypair::new(); let (pubkey, mut account) = @@ -308,7 +304,7 @@ async fn commit_single_account( // We should always be able to Commit & Finalize 1 account either with Args or Buffers ix_commit_local( - service, + processor, vec![intent], expect_strategies(&[(expected_strategy, 1)]), program_flexi_counter::ID, @@ -327,15 +323,16 @@ async fn commit_book_order_account( fund_validator_auth_and_ensure_validator_fees_vault(&validator_auth).await; // Run each test with and without finalizing - let service = CommittorService::try_start( - validator_auth.insecure_clone(), - ":memory:", - ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), - None, - common::MockActionsCallbackExecutor::default(), - ) - .unwrap(); - let service = CommittorServiceExt::new(Arc::new(service)); + let processor = Arc::new( + CommittorProcessor::try_new( + validator_auth.insecure_clone(), + ":memory:", + ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), + None, + common::MockActionsCallbackExecutor::default(), + ) + .unwrap(), + ); let payer = Keypair::new(); let (order_book_pk, mut order_book_ac) = @@ -388,7 +385,7 @@ async fn commit_book_order_account( }; ix_commit_local( - service, + processor, vec![intent], expect_strategies(&[(expected_strategy, 1)]), program_schedulecommit::ID, @@ -801,15 +798,16 @@ async fn commit_multiple_accounts( let validator_auth = ensure_validator_authority(); fund_validator_auth_and_ensure_validator_fees_vault(&validator_auth).await; - let service = CommittorService::try_start( - validator_auth.insecure_clone(), - ":memory:", - ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), - None, - common::MockActionsCallbackExecutor::default(), - ) - .unwrap(); - let service = CommittorServiceExt::new(Arc::new(service)); + let processor = Arc::new( + CommittorProcessor::try_new( + validator_auth.insecure_clone(), + ":memory:", + ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), + None, + common::MockActionsCallbackExecutor::default(), + ) + .unwrap(), + ); // Create bundles of committed accounts let bundles_of_committees = create_bundles(bundle_size, bytess).await; @@ -852,7 +850,7 @@ async fn commit_multiple_accounts( .collect::>(); ix_commit_local( - service, + processor, intents, expected_strategies, program_flexi_counter::ID, @@ -871,15 +869,16 @@ async fn execute_intent_bundle( let validator_auth = ensure_validator_authority(); fund_validator_auth_and_ensure_validator_fees_vault(&validator_auth).await; - let service = CommittorService::try_start( - validator_auth.insecure_clone(), - ":memory:", - ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), - None, - common::MockActionsCallbackExecutor::default(), - ) - .unwrap(); - let service = CommittorServiceExt::new(Arc::new(service)); + let processor = Arc::new( + CommittorProcessor::try_new( + validator_auth.insecure_clone(), + ":memory:", + ChainConfig::local(ComputeBudgetConfig::new(1_000_000)), + None, + common::MockActionsCallbackExecutor::default(), + ) + .unwrap(), + ); // Create bundles of committed accounts let to_commit = create_and_delegate_accounts(bytess_to_commit); @@ -914,7 +913,7 @@ async fn execute_intent_bundle( intent_bundle, }; ix_commit_local( - service, + processor, vec![intent_bundle], expected_strategies, program_flexi_counter::id(), @@ -950,13 +949,13 @@ async fn execute_intent_bundle( // Test Executor // ----------------- async fn ix_commit_local( - service: CommittorServiceExt, + processor: Arc, intent_bundles: Vec, expected_strategies: ExpectedStrategies, program_id: Pubkey, ) { - let execution_outputs = service - .schedule_intent_bundles_waiting(intent_bundles.clone()) + let execution_outputs = processor + .execute_intent_bundles(intent_bundles.clone()) .await .unwrap() .into_iter() @@ -964,7 +963,6 @@ async fn ix_commit_local( // Assert that all completed assert_eq!(execution_outputs.len(), intent_bundles.len()); - service.release_common_pubkeys().await.unwrap(); let rpc_client = RpcClient::new("http://localhost:7799".to_string()); let mut strategies = ExpectedStrategies::new(); @@ -1064,11 +1062,7 @@ async fn ix_commit_local( }) .collect(); - let statuses = service - .get_commit_statuses(base_intent.id) - .await - .unwrap() - .unwrap(); + let statuses = processor.get_commit_statuses(base_intent.id).unwrap(); debug!( "{}", statuses @@ -1124,76 +1118,6 @@ async fn ix_commit_local( strategies, expected_strategies, "Strategies used do not match expected ones" ); - - let expect_empty_lookup_tables = false; - // changeset.accounts.len() == changeset.accounts_to_undelegate.len(); - if expect_empty_lookup_tables { - let lookup_tables = service.get_lookup_tables().await.unwrap(); - assert!(lookup_tables.active.is_empty()); - - if utils::TEST_TABLE_CLOSE { - let mut closing_tables = lookup_tables.released; - - // Tables deactivate after ~2.5 mins (150secs), but most times - // it takes a lot longer so we allow double the time - const MAX_TIME_TO_CLOSE: Duration = Duration::from_secs(300); - info!( - "Waiting for lookup tables close for up to {} secs", - MAX_TIME_TO_CLOSE.as_secs() - ); - - let start = Instant::now(); - let rpc_client = MagicblockRpcClient::from(rpc_client); - loop { - let accs = rpc_client - .get_multiple_accounts_with_commitment( - &closing_tables, - CommitmentConfig::confirmed(), - None, - ) - .await - .unwrap(); - let closed_pubkeys = accs - .into_iter() - .zip(closing_tables.iter()) - .filter_map(|(acc, pubkey)| { - if acc.is_none() { - Some(*pubkey) - } else { - None - } - }) - .collect::>(); - closing_tables.retain(|pubkey| { - if closed_pubkeys.contains(pubkey) { - debug!("Table {} closed", pubkey); - false - } else { - true - } - }); - if closing_tables.is_empty() { - break; - } - debug!( - "Still waiting for {} released table(s) to close", - closing_tables.len() - ); - if Instant::now() - start > MAX_TIME_TO_CLOSE { - panic!( - "Timed out waiting for tables close after {} seconds. Still open: {}", - MAX_TIME_TO_CLOSE.as_secs(), - closing_tables - .iter() - .map(|x| x.to_string()) - .collect::>() - .join(", ") - ); - } - utils::sleep_millis(10_000).await; - } - } - } } fn validate_account(