From b469c5b909b17938a23ced160bc06e0b07caffe8 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 3 Mar 2026 12:12:50 +0700 Subject: [PATCH 01/19] fix: address now possible race conditions in CacheTaskInfoFetcher --- Cargo.lock | 1 + magicblock-accounts/Cargo.toml | 1 + .../src/scheduled_commits_processor.rs | 2 + magicblock-api/src/lib.rs | 1 + magicblock-api/src/magic_sys_adapter.rs | 7 + magicblock-chainlink/src/lib.rs | 116 +++++++++ .../src/intent_executor/mod.rs | 2 +- .../src/intent_executor/task_info_fetcher.rs | 222 ++++++++++++++---- .../src/tasks/task_builder.rs | 2 +- .../src/tasks/task_strategist.rs | 4 +- magicblock-core/src/traits.rs | 9 +- .../src/store/data_mod_persister.rs | 4 +- .../src/mutate_accounts/account_mod_data.rs | 6 +- programs/magicblock/src/test_utils/mod.rs | 12 +- test-integration/Cargo.lock | 1 + .../test-committor-service/tests/common.rs | 4 +- .../tests/test_intent_executor.rs | 14 +- 17 files changed, 341 insertions(+), 67 deletions(-) create mode 100644 magicblock-api/src/magic_sys_adapter.rs diff --git a/Cargo.lock b/Cargo.lock index e55581927..45df90064 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2962,6 +2962,7 @@ dependencies = [ "magicblock-chainlink", "magicblock-committor-service", "magicblock-core", + "magicblock-metrics", "magicblock-program", "solana-hash", "solana-pubkey", diff --git a/magicblock-accounts/Cargo.toml b/magicblock-accounts/Cargo.toml index 617931422..aaf0879d4 100644 --- a/magicblock-accounts/Cargo.toml +++ b/magicblock-accounts/Cargo.toml @@ -16,6 +16,7 @@ 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 } diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 88c4ed4ed..619abe562 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -19,6 +19,7 @@ use magicblock_committor_service::{ intent_executor::ExecutionOutput, BaseIntentCommittor, CommittorService, }; use magicblock_core::link::transactions::TransactionSchedulerHandle; +use magicblock_metrics::metrics; use magicblock_program::{ magic_scheduled_base_intent::ScheduledIntentBundle, register_scheduled_commit_sent, SentCommit, TransactionScheduler, @@ -275,6 +276,7 @@ impl ScheduledCommitsProcessor for ScheduledCommitsProcessorImpl { if intent_bundles.is_empty() { return Ok(()); } + metrics::inc_committor_intents_count_by(intent_bundles.len() as u64); // Add metas for intent we schedule let pubkeys_being_undelegated = { diff --git a/magicblock-api/src/lib.rs b/magicblock-api/src/lib.rs index a82f4bc97..853019317 100644 --- a/magicblock-api/src/lib.rs +++ b/magicblock-api/src/lib.rs @@ -3,6 +3,7 @@ pub mod errors; mod fund_account; mod genesis_utils; pub mod ledger; +mod magic_sys_adapter; pub mod magic_validator; mod slot; mod tickers; diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs new file mode 100644 index 000000000..e4c5c3ce8 --- /dev/null +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -0,0 +1,7 @@ +use std::sync::Arc; + +use magicblock_ledger::Ledger; + +pub struct MagicSysAdapter { + pub ledger: Arc, +} diff --git a/magicblock-chainlink/src/lib.rs b/magicblock-chainlink/src/lib.rs index 1be32d4f9..45d198ba0 100644 --- a/magicblock-chainlink/src/lib.rs +++ b/magicblock-chainlink/src/lib.rs @@ -11,3 +11,119 @@ mod filters; #[cfg(any(test, feature = "dev-context"))] pub mod testing; + +#[test] +fn test() { + // The issue: + // 1. We need to clone inner ArcMutex to hold locks + // We can't have reference as that would mean blocking outer Mutex for func duration + // 2. When we clone we have a problem - the entry may get evicted from LruCache + // That would lead to race conditions: + // New thread comes in, sees no entry for pubkey, creates new ArcMutex instance + // Other thread will overwrite it + + // Goal: we still need to cleanup space ut without race conditions + + // That means - we can't use LruCache directly + // We need to use HashMap and clean post factum + + // When can we insert and clean? + // Cleaning: + // We clean after execution. + + // Alg for fetch next + // 1. lock outer mutex + // 2. get inner mutexes + // a. exists + // i. value == u64::MAX, clone move to_request + // ii. value != u64::MAX, clone it into ready group + // b. absent - insert in HashMap + clone into to_request + // 3. update LruCache. // Here some locked keys could be evicted. TODO: update or maybe not/ + // 4. drop outer mutex + // 5. lock.await inner mutexes + // 6. request data for uninitialized ones + // 7. On failure + // a. lock outer + // b. TODO: ensure safety for uninitialized, check if locked + // 8. On success - lock outer + // 9. Update locked inner values + // 10. compile result + // 11. get pubkey in LRU + // a. exists - continue + // b. !exists - push + // i. None returned - continue + // ii. Some(el) - if not in use remove from hashmap + + // Problem wuth 2.b + // Once inserted in map it could be accessed, even tho it is uninitialized + // That would mean that we can't just remove it on failure + // If it is cloned by someone - we need to leave it alone + + // The scenario we're trying to cover - requests exceed capacity + // Key getting before RPC finished - means that so many values pushed that our key got evicted + // We could insert it back into LRU or clean it upo + // We need cleanup logic. The last active function has to cleanup + // If it isn't in Lru & key NotInUse - cleanup + // If it isn't in Lru & key InUse - keep, do nothing it will be cleaned up by occupying party + // !!!WITH THIS we don't need to care of evicted keys on first outer lock, + // If it is in use the working thread will cleanup itself + + // Problem: + // If we don't insert in the beggining in LRU, then if key absent in the end + // We can't know if it was evicted or not + // What is cleanup logic in that case? + // If we insert at the end and Some key evicted, this key is in use + // If we don't dispose of it the other thread won't be able + + // OPTION 1 + // Handle LRU at the END get pubkey in LRU + // a. exists - continue, no need to cleanup + // b. !exists - push + // i. None returned - continue, no need to cleanup + // ii. Evicted(el) && NotInUse - remove from hashmap + // iii. Evicted(el) && InUse - continue?? + // Evicted key at the end has to be cleaned up by someone + // if ii - excess dealt with right away + // if iii - will be dealt with by some running instance, since it will be inserted in LRU at some point + + // OPTION 2 + // Handle LRU at the BEGGINING + // LRU.get + // a. exists - continue, no need to cleanup + // b. !exists - push + // i. None returned - continue, no need to cleanup + // ii. Evicted(el) && NotInUse - remove from hashmap + // iii. Evicted(el) && InUse - continue + // What to do at the END? + // LRU.peek() + // a. exists - continue, no need to cleanup + // b. !exists + // i. InUse - continue + // ii. NotInUse - cleanup + + // We need to define how LRU works + // I come and place request + // Not to repeat work someone may cache it so If i come again the could just give cached request + // But while they make my order many other people came and asked other things + + use std::{ + num::NonZeroUsize, + sync::{Arc, Mutex}, + }; + + use lru::LruCache; + + let mut map: Mutex>>> = + Mutex::new(LruCache::new(NonZeroUsize::new(2).unwrap())); + let mut v = vec![]; + { + let mut map = map.lock().unwrap(); + let entry1 = map.get_or_insert("hi1".to_string(), || { + Arc::new(Mutex::new(u64::MAX)) + }); + let entry1 = entry1.lock().unwrap(); + v = vec![entry1] + } + + *v[0] = 1; +} diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index fb1426705..c510f503a 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -338,7 +338,7 @@ where .reset(ResetType::Specific(committed_pubkeys)); let commit_ids = self .task_info_fetcher - .fetch_next_commit_ids(committed_pubkeys, min_context_slot) + .fetch_next_commit_nonces(committed_pubkeys, min_context_slot) .await .map_err(TaskBuilderError::CommitTasksBuildError)?; diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 415d12437..3e67cb5e1 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -1,11 +1,16 @@ use std::{ - collections::HashMap, num::NonZeroUsize, sync::Mutex, time::Duration, + collections::{HashMap, HashSet}, + num::NonZeroUsize, + ops::Add, + sync::{Arc, Mutex}, + time::Duration, }; use async_trait::async_trait; use dlp::{ delegation_metadata_seeds_from_delegated_account, state::DelegationMetadata, }; +use dyn_clone::clone; use lru::LruCache; use magicblock_metrics::metrics; use magicblock_rpc_client::{MagicBlockRpcClientError, MagicblockRpcClient}; @@ -18,6 +23,7 @@ use solana_rpc_client_api::{ request::RpcError, }; use solana_signature::Signature; +use tokio::sync::Mutex as TMutex; use tracing::{error, info, warn}; const NUM_FETCH_RETRIES: NonZeroUsize = NonZeroUsize::new(5).unwrap(); @@ -25,9 +31,17 @@ const MUTEX_POISONED_MSG: &str = "CacheTaskInfoFetcher mutex poisoned!"; #[async_trait] pub trait TaskInfoFetcher: Send + Sync + 'static { - /// Fetches correct next ids for pubkeys - /// Those ids can be used as correct commit_id during Commit - async fn fetch_next_commit_ids( + /// Fetches correct commit nonces for pubkeys + /// Those nonces can be used as correct commit_nonce during Commit + async fn fetch_next_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult>; + + /// Fetches current commit nonces for pubkeys + /// Missing nonces will be fetched from chain + async fn fetch_current_commit_nonces( &self, pubkeys: &[Pubkey], min_context_slot: u64, @@ -41,7 +55,7 @@ pub trait TaskInfoFetcher: Send + Sync + 'static { ) -> TaskInfoFetcherResult>; /// Peeks current commit ids for pubkeys - fn peek_commit_id(&self, pubkey: &Pubkey) -> Option; + fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option; /// Resets cache for some or all accounts fn reset(&self, reset_type: ResetType); @@ -58,9 +72,25 @@ pub enum ResetType<'a> { Specific(&'a [Pubkey]), } +type NonceLock = Arc>; + +struct CacheInner { + active: LruCache, + retiring: HashMap, +} + +impl CacheInner { + fn new(capacity: NonZeroUsize) -> Self { + Self { + active: LruCache::new(capacity), + retiring: HashMap::new(), + } + } +} + pub struct CacheTaskInfoFetcher { rpc_client: MagicblockRpcClient, - cache: Mutex>, + cache: Mutex, } impl CacheTaskInfoFetcher { @@ -69,7 +99,7 @@ impl CacheTaskInfoFetcher { Self { rpc_client, - cache: Mutex::new(LruCache::new(CACHE_SIZE)), + cache: Mutex::new(CacheInner::new(CACHE_SIZE)), } } @@ -229,7 +259,7 @@ impl CacheTaskInfoFetcher { impl TaskInfoFetcher for CacheTaskInfoFetcher { /// Returns next ids for requested pubkeys /// If key isn't in cache, it will be requested - async fn fetch_next_commit_ids( + async fn fetch_next_commit_nonces( &self, pubkeys: &[Pubkey], min_context_slot: u64, @@ -238,63 +268,163 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(HashMap::new()); } - let mut result = HashMap::new(); - let mut to_request = Vec::new(); - // Lock cache and extract whatever ids we can + let mut pubkeys = pubkeys.to_vec(); + // Sorted order is required: all callers acquire per-key locks in the same + // order, preventing the A→B / B→A circular-wait deadlock. + pubkeys.sort_unstable(); + pubkeys.dedup(); + + let mut nonce_locks = vec![]; { - let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - for pubkey in pubkeys { - // in case already inserted - if result.contains_key(pubkey) { - continue; + let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + for pubkey in pubkeys.iter() { + let (lock, eviceted) = + if let Some(val) = inner.active.get(pubkey) { + (val.clone(), None) + } else if let Some(val) = inner.retiring.remove(pubkey) { + // This promotes retiring to active + let evicted = inner.active.push(*pubkey, val.clone()); + (val, evicted) + } else { + let val = Arc::new(TMutex::new(u64::MAX)); + let evicted = inner.active.push(*pubkey, val.clone()); + (val, evicted) + }; + + if let Some((evicted_pk, evicted_lock)) = eviceted { + // If value isn't used by anyone then it can be dropped + if Arc::strong_count(&evicted_lock) > 1 { + // Value used in by another request + // We can't drop evicted lock in that case + // We move it to retiring, which will be cleaned up on exit + // Race condition scenario: + // 1. set of accs A evicted due to surge of requests - locks are dropped + // 2. request for set A still ongoing + // 3, another request with set A comes in, creating new locks in `CacheInner::active` + // 4. 2 simultaneous requestors receive same value + let old = inner.retiring.insert(evicted_pk, evicted_lock); + if old.is_some() { + // Safety + // assume that is true: + // That means that value was active & retiring at the same time + // This is impossible as per logic above, contradiction. чтд. + debug_assert!( + false, + "Just eviceted value can't be in retiring" + ); + error!("Retiring map already contained lock with pubkey: {}", evicted_pk); + } + } } - if let Some(id) = cache.get(pubkey) { - result.insert(*pubkey, *id + 1); - } else { - to_request.push(*pubkey); - } + nonce_locks.push((pubkey, lock)); + } + } + + // Acquire per-account locks sequentially in sorted order (see sort above). + // join_all would poll all futures concurrently, allowing partial acquisition + // and producing the classic A→B / B→A deadlock across concurrent callers. + let mut guard_nonces = vec![]; + for (pubkey, lock) in nonce_locks.iter() { + guard_nonces.push((pubkey, lock.lock().await)); + } + let (mut existing, mut to_request) = (vec![], vec![]); + for (pubkey, guard) in guard_nonces { + if *guard == u64::MAX { + to_request.push((pubkey, guard)); + } else { + existing.push((pubkey, guard)) } } // If all in cache - great! return if to_request.is_empty() { - let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - result.iter().for_each(|(pubkey, id)| { - cache.push(*pubkey, *id); - }); + let mut result = HashMap::with_capacity(existing.len()); + // Consume guards & write result + for (pubkey, mut guard) in existing { + *guard += 1; + result.insert(**pubkey, *guard); + } + + let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + for (pubkey, lock) in nonce_locks { + // Drop our clone first so strong_count reflects only other + // live holders when we check below + drop(lock); + let should_remove = inner + .retiring + .get(&pubkey) + .is_some_and(|l| Arc::strong_count(l) == 1); + if should_remove { + inner.retiring.remove(&pubkey); + } + } return Ok(result); } // Remove duplicates - to_request.sort(); - to_request.dedup(); - - let remaining_ids = Self::fetch_metadata_with_retries( + let to_request_pubkeys: Vec<_> = + to_request.iter().map(|(pubkey, _)| ***pubkey).collect(); + let res = Self::fetch_metadata_with_retries( &self.rpc_client, - &to_request, + &to_request_pubkeys, min_context_slot, NUM_FETCH_RETRIES, ) - .await? - .into_iter() - .map(|metadata| metadata.last_update_nonce); + .await; + + let remaining_ids = match res { + Ok(value) => value, + Err(err) => { + let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + for (pubkey, lock) in nonce_locks { + // Drop our clone first so strong_count reflects only other + // live holders when we check below + drop(lock); + let should_remove = inner + .retiring + .get(&pubkey) + .is_some_and(|l| Arc::strong_count(l) == 1); + if should_remove { + inner.retiring.remove(&pubkey); + } + } + return Err(err); + } + }; + + let remaining_ids = remaining_ids + .into_iter() + .map(|metadata| metadata.last_update_nonce); // We don't care if anything changed in between with cache - just update and return our ids. + + let mut result = HashMap::with_capacity(existing.len()); + // Consume guards & write result + for (pubkey, mut guard) in existing { + *guard += 1; + result.insert(**pubkey, *guard); + } + for ((pubkey, mut guard), nonce) in to_request.into_iter().zip(remaining_ids) { + *guard = nonce + 1; + result.insert(**pubkey, *guard); + } + { - let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - // Avoid changes to LRU until all data is ready - atomic update - result.iter().for_each(|(pubkey, id)| { - cache.push(*pubkey, *id); - }); - to_request - .iter() - .zip(remaining_ids) - .for_each(|(pubkey, id)| { - result.insert(*pubkey, id + 1); - cache.push(*pubkey, id + 1); - }); + let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + for (pubkey, lock) in nonce_locks { + // Drop our clone first so strong_count reflects only other + // live holders when we check below + drop(lock); + let should_remove = inner + .retiring + .get(&pubkey) + .is_some_and(|l| Arc::strong_count(l) == 1); + if should_remove { + inner.retiring.remove(&pubkey); + } + } } Ok(result) @@ -320,7 +450,7 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { } /// Returns current commit id without raising priority - fn peek_commit_id(&self, pubkey: &Pubkey) -> Option { + fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { let cache = self.cache.lock().expect(MUTEX_POISONED_MSG); cache.peek(pubkey).copied() } diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index 507d7358b..cb3c385fb 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -115,7 +115,7 @@ impl TaskBuilderImpl { .collect::>(); task_info_fetcher - .fetch_next_commit_ids(&committed_pubkeys, min_context_slot) + .fetch_next_commit_nonces(&committed_pubkeys, min_context_slot) .await } diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index de8a4740e..95146f87a 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -409,7 +409,7 @@ mod tests { #[async_trait::async_trait] impl TaskInfoFetcher for MockInfoFetcher { - async fn fetch_next_commit_ids( + async fn fetch_next_commit_nonces( &self, pubkeys: &[Pubkey], _: u64, @@ -425,7 +425,7 @@ mod tests { Ok(pubkeys.iter().map(|_| Pubkey::new_unique()).collect()) } - fn peek_commit_id(&self, _pubkey: &Pubkey) -> Option { + fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { Some(0) } diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index bbd84d9d7..075c78cc3 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -1,6 +1,13 @@ use std::{error::Error, fmt}; -pub trait PersistsAccountModData: Sync + Send + fmt::Display + 'static { +use solana_program::instruction::InstructionError; +use solana_pubkey::Pubkey; + +pub trait MagicSys: Sync + Send + fmt::Display + 'static { fn persist(&self, id: u64, data: Vec) -> Result<(), Box>; fn load(&self, id: u64) -> Result>, Box>; + fn validate_commit_nonces( + &self, + pubkeys: &[Pubkey], + ) -> Result<(), InstructionError>; } diff --git a/magicblock-ledger/src/store/data_mod_persister.rs b/magicblock-ledger/src/store/data_mod_persister.rs index 53fc5d864..e7d7daac5 100644 --- a/magicblock-ledger/src/store/data_mod_persister.rs +++ b/magicblock-ledger/src/store/data_mod_persister.rs @@ -1,11 +1,11 @@ use std::error::Error; -use magicblock_core::traits::PersistsAccountModData; +use magicblock_core::traits::MagicSys; use tracing::*; use crate::Ledger; -impl PersistsAccountModData for Ledger { +impl MagicSys for Ledger { fn persist(&self, id: u64, data: Vec) -> Result<(), Box> { trace!(id, data_len = data.len(), "Persisting data"); self.write_account_mod_data(id, &data.into())?; diff --git a/programs/magicblock/src/mutate_accounts/account_mod_data.rs b/programs/magicblock/src/mutate_accounts/account_mod_data.rs index d313b0202..943a316d3 100644 --- a/programs/magicblock/src/mutate_accounts/account_mod_data.rs +++ b/programs/magicblock/src/mutate_accounts/account_mod_data.rs @@ -7,7 +7,7 @@ use std::{ }; use lazy_static::lazy_static; -use magicblock_core::traits::PersistsAccountModData; +use magicblock_core::traits::MagicSys; use solana_log_collector::ic_msg; use solana_program_runtime::invoke_context::InvokeContext; @@ -24,7 +24,7 @@ lazy_static! { /// loaded from the [DATA_MODS] /// During replay the [DATA_MODS] won't have the data for the particular id in which /// case it is loaded via the persister instead. - static ref PERSISTER: RwLock>> = RwLock::new(None); + static ref PERSISTER: RwLock>> = RwLock::new(None); static ref DATA_MOD_ID: AtomicU64 = AtomicU64::new(0); @@ -75,7 +75,7 @@ pub(super) fn get_data(id: u64) -> Option> { DATA_MODS.lock().expect("DATA_MODS poisoned").remove(&id) } -pub fn init_persister(persister: Arc) { +pub fn init_persister(persister: Arc) { PERSISTER .write() .expect("PERSISTER poisoned") diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index 7929fc14a..d0ab16f5b 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -8,7 +8,7 @@ use std::{ }, }; -use magicblock_core::traits::PersistsAccountModData; +use magicblock_core::traits::MagicSys; use magicblock_magic_program_api::{id, EPHEMERAL_VAULT_PUBKEY}; use solana_account::AccountSharedData; use solana_instruction::{error::InstructionError, AccountMeta}; @@ -83,7 +83,7 @@ impl fmt::Display for PersisterStub { } } -impl PersistsAccountModData for PersisterStub { +impl MagicSys for PersisterStub { fn persist(&self, id: u64, data: Vec) -> Result<(), Box> { debug!("Persisting data for id '{}' with len {}", id, data.len()); Ok(()) @@ -92,4 +92,12 @@ impl PersistsAccountModData for PersisterStub { fn load(&self, _id: u64) -> Result>, Box> { Err("Loading from ledger not supported in tests".into()) } + + fn validate_commit_nonces( + &self, + pubkeys: &[Pubkey], + ) -> Result<(), InstructionError> { + // TODO(edwin) + todo!() + } } diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index 0ed2c9332..6eb399f60 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3313,6 +3313,7 @@ dependencies = [ "magicblock-chainlink", "magicblock-committor-service", "magicblock-core", + "magicblock-metrics", "magicblock-program", "solana-hash", "solana-pubkey", diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index 81adae320..2518164e7 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -123,7 +123,7 @@ pub struct MockTaskInfoFetcher(MagicblockRpcClient); #[async_trait] impl TaskInfoFetcher for MockTaskInfoFetcher { - async fn fetch_next_commit_ids( + async fn fetch_next_commit_nonces( &self, pubkeys: &[Pubkey], _: u64, @@ -139,7 +139,7 @@ impl TaskInfoFetcher for MockTaskInfoFetcher { Ok(pubkeys.to_vec()) } - fn peek_commit_id(&self, _pubkey: &Pubkey) -> Option { + fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { None } 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 fd53a00bf..9f18d770f 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -137,7 +137,7 @@ async fn test_commit_id_error_parsing() { // Invalidate ids before execution task_info_fetcher - .fetch_next_commit_ids( + .fetch_next_commit_nonces( &intent.get_undelegate_intent_pubkeys().unwrap(), remote_slot, ) @@ -444,7 +444,7 @@ async fn test_commit_id_error_recovery() { // Invalidate commit nonce cache let res = task_info_fetcher - .fetch_next_commit_ids(&[committed_account.pubkey], remote_slot) + .fetch_next_commit_nonces(&[committed_account.pubkey], remote_slot) .await; assert!(res.is_ok()); assert!(res.unwrap().contains_key(&committed_account.pubkey)); @@ -480,7 +480,7 @@ async fn test_commit_id_error_recovery() { .map(|el| { ( el.pubkey, - task_info_fetcher.peek_commit_id(&el.pubkey).unwrap(), + task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), ) }) .collect(); @@ -645,7 +645,7 @@ async fn test_commit_id_and_action_errors_recovery() { // Invalidate commit nonce cache let res = task_info_fetcher - .fetch_next_commit_ids(&[committed_account.pubkey], remote_slot) + .fetch_next_commit_nonces(&[committed_account.pubkey], remote_slot) .await; assert!(res.is_ok()); assert!(res.unwrap().contains_key(&committed_account.pubkey)); @@ -777,7 +777,7 @@ async fn test_cpi_limits_error_recovery() { .map(|el| { ( el.pubkey, - task_info_fetcher.peek_commit_id(&el.pubkey).unwrap(), + task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), ) }) .collect(); @@ -851,7 +851,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { // Force CommitIDError by invalidating the commit-nonce cache before running let pubkeys: Vec<_> = committed_accounts.iter().map(|c| c.pubkey).collect(); let mut invalidated_keys = task_info_fetcher - .fetch_next_commit_ids(&pubkeys, Default::default()) + .fetch_next_commit_nonces(&pubkeys, Default::default()) .await .unwrap(); @@ -912,7 +912,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { .map(|el| { ( el.pubkey, - task_info_fetcher.peek_commit_id(&el.pubkey).unwrap(), + task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), ) }) .collect(); From b7b37d3f0ec5d487251f9431866e925273ec7b3b Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 3 Mar 2026 15:40:09 +0700 Subject: [PATCH 02/19] wip --- magicblock-chainlink/src/lib.rs | 47 +++- .../src/intent_executor/task_info_fetcher.rs | 222 +++++++++++------- .../src/store/data_mod_persister.rs | 9 + 3 files changed, 179 insertions(+), 99 deletions(-) diff --git a/magicblock-chainlink/src/lib.rs b/magicblock-chainlink/src/lib.rs index 45d198ba0..1d467fba8 100644 --- a/magicblock-chainlink/src/lib.rs +++ b/magicblock-chainlink/src/lib.rs @@ -5,6 +5,8 @@ pub mod cloner; pub mod remote_account_provider; pub mod submux; +use std::ops::Add; + pub use chainlink::*; pub use magicblock_metrics::metrics::AccountFetchOrigin; mod filters; @@ -12,8 +14,8 @@ mod filters; #[cfg(any(test, feature = "dev-context"))] pub mod testing; -#[test] -fn test() { +#[tokio::test] +async fn test_trtr() { // The issue: // 1. We need to clone inner ArcMutex to hold locks // We can't have reference as that would mean blocking outer Mutex for func duration @@ -112,18 +114,37 @@ fn test() { }; use lru::LruCache; + use tokio::sync::{Mutex as TMutex, MutexGuard, OwnedMutexGuard}; + + struct Locker<'a> { + val: &'a i32, + m: Arc>, + }; + + impl<'a> Locker<'a> { + pub fn new(val: &'a i32, m: Arc>) -> Self { + Self { val, m } + } - let mut map: Mutex>>> = - Mutex::new(LruCache::new(NonZeroUsize::new(2).unwrap())); - let mut v = vec![]; - { - let mut map = map.lock().unwrap(); - let entry1 = map.get_or_insert("hi1".to_string(), || { - Arc::new(Mutex::new(u64::MAX)) - }); - let entry1 = entry1.lock().unwrap(); - v = vec![entry1] + pub async fn lock<'s>(&'s self) -> MutexGuard<'s, u64> { + self.m.lock().await + } } - *v[0] = 1; + impl<'a> Drop for Locker<'a> { + fn drop(&mut self) { + println!("lpol"); + } + } + + let val = 1; + let m = Arc::new(TMutex::new(10)); + let locker = Locker::new(&val, m); + let mut asd = locker.lock().await; + *asd = 1; + assert_eq!(*asd, 1); + drop(asd); + drop(locker); + + println!("{}", val); } diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 3e67cb5e1..be81b21fe 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -1,5 +1,6 @@ use std::{ collections::{HashMap, HashSet}, + mem, num::NonZeroUsize, ops::Add, sync::{Arc, Mutex}, @@ -23,7 +24,7 @@ use solana_rpc_client_api::{ request::RpcError, }; use solana_signature::Signature; -use tokio::sync::Mutex as TMutex; +use tokio::sync::{Mutex as TMutex, MutexGuard}; use tracing::{error, info, warn}; const NUM_FETCH_RETRIES: NonZeroUsize = NonZeroUsize::new(5).unwrap(); @@ -79,6 +80,45 @@ struct CacheInner { retiring: HashMap, } +struct CacheInnerGuard<'a> { + inner: &'a Mutex, + nonce_locks: Vec<(Pubkey, NonceLock)>, +} + +impl<'a> CacheInnerGuard<'a> { + // Acquire per-account locks sequentially in sorted order (see sort above). + // join_all would poll all futures concurrently, allowing partial acquisition + // and producing the classic A→B / B→A deadlock across concurrent callers. + async fn lock<'s>(&'s self) -> Vec<(&'s Pubkey, MutexGuard<'s, u64>)> { + let mut output = Vec::with_capacity(self.nonce_locks.len()); + for (pubkey, lock) in self.nonce_locks.iter() { + let guard = lock.lock().await; + output.push((pubkey, guard)) + } + + output + } +} + +impl<'a> Drop for CacheInnerGuard<'a> { + fn drop(&mut self) { + let mut inner = self.inner.lock().expect(MUTEX_POISONED_MSG); + let nonce_locks = mem::take(&mut self.nonce_locks); + for (pubkey, lock) in nonce_locks { + // Drop our clone first so strong_count reflects only other + // live holders when we check below + drop(lock); + let should_remove = inner + .retiring + .get(&pubkey) + .is_some_and(|l| Arc::strong_count(l) == 1); + if should_remove { + inner.retiring.remove(&pubkey); + } + } + } +} + impl CacheInner { fn new(capacity: NonZeroUsize) -> Self { Self { @@ -252,42 +292,28 @@ impl CacheTaskInfoFetcher { Ok(accounts) } -} - -/// TaskInfoFetcher implementation that also caches most used 1000 keys -#[async_trait] -impl TaskInfoFetcher for CacheTaskInfoFetcher { - /// Returns next ids for requested pubkeys - /// If key isn't in cache, it will be requested - async fn fetch_next_commit_nonces( - &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> TaskInfoFetcherResult> { - if pubkeys.is_empty() { - return Ok(HashMap::new()); - } - let mut pubkeys = pubkeys.to_vec(); + fn reserve_locks(&self, pubkeys: &[Pubkey]) -> CacheInnerGuard<'_> { // Sorted order is required: all callers acquire per-key locks in the same // order, preventing the A→B / B→A circular-wait deadlock. + let mut pubkeys = pubkeys.to_vec(); pubkeys.sort_unstable(); pubkeys.dedup(); let mut nonce_locks = vec![]; { let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); - for pubkey in pubkeys.iter() { + for pubkey in pubkeys { let (lock, eviceted) = - if let Some(val) = inner.active.get(pubkey) { + if let Some(val) = inner.active.get(&pubkey) { (val.clone(), None) - } else if let Some(val) = inner.retiring.remove(pubkey) { + } else if let Some(val) = inner.retiring.remove(&pubkey) { // This promotes retiring to active - let evicted = inner.active.push(*pubkey, val.clone()); + let evicted = inner.active.push(pubkey, val.clone()); (val, evicted) } else { let val = Arc::new(TMutex::new(u64::MAX)); - let evicted = inner.active.push(*pubkey, val.clone()); + let evicted = inner.active.push(pubkey, val.clone()); (val, evicted) }; @@ -302,7 +328,8 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { // 2. request for set A still ongoing // 3, another request with set A comes in, creating new locks in `CacheInner::active` // 4. 2 simultaneous requestors receive same value - let old = inner.retiring.insert(evicted_pk, evicted_lock); + let old = + inner.retiring.insert(evicted_pk, evicted_lock); if old.is_some() { // Safety // assume that is true: @@ -321,13 +348,33 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { } } + CacheInnerGuard { + inner: &self.cache, + nonce_locks, + } + } +} + +/// TaskInfoFetcher implementation that also caches most used 1000 keys +#[async_trait] +impl TaskInfoFetcher for CacheTaskInfoFetcher { + /// Returns next ids for requested pubkeys + /// If key isn't in cache, it will be requested + async fn fetch_next_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + if pubkeys.is_empty() { + return Ok(HashMap::new()); + } + + let locks_guard = self.reserve_locks(pubkeys); + // Acquire per-account locks sequentially in sorted order (see sort above). // join_all would poll all futures concurrently, allowing partial acquisition // and producing the classic A→B / B→A deadlock across concurrent callers. - let mut guard_nonces = vec![]; - for (pubkey, lock) in nonce_locks.iter() { - guard_nonces.push((pubkey, lock.lock().await)); - } + let mut guard_nonces = locks_guard.lock().await; let (mut existing, mut to_request) = (vec![], vec![]); for (pubkey, guard) in guard_nonces { if *guard == u64::MAX { @@ -340,93 +387,96 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { // If all in cache - great! return if to_request.is_empty() { let mut result = HashMap::with_capacity(existing.len()); - // Consume guards & write result for (pubkey, mut guard) in existing { *guard += 1; - result.insert(**pubkey, *guard); + result.insert(*pubkey, *guard); } - let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); - for (pubkey, lock) in nonce_locks { - // Drop our clone first so strong_count reflects only other - // live holders when we check below - drop(lock); - let should_remove = inner - .retiring - .get(&pubkey) - .is_some_and(|l| Arc::strong_count(l) == 1); - if should_remove { - inner.retiring.remove(&pubkey); - } - } return Ok(result); } // Remove duplicates let to_request_pubkeys: Vec<_> = - to_request.iter().map(|(pubkey, _)| ***pubkey).collect(); - let res = Self::fetch_metadata_with_retries( + to_request.iter().map(|(pubkey, _)| **pubkey).collect(); + let remaining_ids = Self::fetch_metadata_with_retries( &self.rpc_client, &to_request_pubkeys, min_context_slot, NUM_FETCH_RETRIES, ) - .await; - - let remaining_ids = match res { - Ok(value) => value, - Err(err) => { - let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); - for (pubkey, lock) in nonce_locks { - // Drop our clone first so strong_count reflects only other - // live holders when we check below - drop(lock); - let should_remove = inner - .retiring - .get(&pubkey) - .is_some_and(|l| Arc::strong_count(l) == 1); - if should_remove { - inner.retiring.remove(&pubkey); - } - } - return Err(err); - } - }; - - let remaining_ids = remaining_ids - .into_iter() - .map(|metadata| metadata.last_update_nonce); + .await? + .into_iter() + .map(|metadata| metadata.last_update_nonce); // We don't care if anything changed in between with cache - just update and return our ids. - let mut result = HashMap::with_capacity(existing.len()); // Consume guards & write result for (pubkey, mut guard) in existing { *guard += 1; - result.insert(**pubkey, *guard); + result.insert(*pubkey, *guard); } - for ((pubkey, mut guard), nonce) in to_request.into_iter().zip(remaining_ids) { + for ((pubkey, mut guard), nonce) in + to_request.into_iter().zip(remaining_ids) + { *guard = nonce + 1; - result.insert(**pubkey, *guard); + result.insert(*pubkey, *guard); } - { - let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); - for (pubkey, lock) in nonce_locks { - // Drop our clone first so strong_count reflects only other - // live holders when we check below - drop(lock); - let should_remove = inner - .retiring - .get(&pubkey) - .is_some_and(|l| Arc::strong_count(l) == 1); - if should_remove { - inner.retiring.remove(&pubkey); - } + Ok(result) + } + + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + if pubkeys.is_empty() { + return Ok(HashMap::new()); + } + + let locks_guard = self.reserve_locks(pubkeys); + + // Acquire per-account locks sequentially in sorted order (see sort above). + let mut guard_nonces = locks_guard.lock().await; + let (mut existing, mut to_request) = (vec![], vec![]); + let mut result = HashMap::with_capacity(existing.len()); + for (pubkey, guard) in guard_nonces { + if *guard == u64::MAX { + to_request.push((pubkey, guard)); + } else { + result.insert(*pubkey, *guard); + existing.push((pubkey, guard)) } } + if to_request.is_empty() { + return Ok(result); + } + + let to_request_pubkeys: Vec<_> = + to_request.iter().map(|(pubkey, _)| **pubkey).collect(); + let remaining_ids = Self::fetch_metadata_with_retries( + &self.rpc_client, + &to_request_pubkeys, + min_context_slot, + NUM_FETCH_RETRIES, + ) + .await? + .into_iter() + .map(|metadata| metadata.last_update_nonce); + + // Store the on-chain nonce as-is (no +1): recording current state, not + // reserving the next slot. A subsequent fetch_next_commit_nonces call will + // increment from here correctly. + for ((pubkey, mut guard), nonce) in + to_request.into_iter().zip(remaining_ids) + { + *guard = nonce; + result.insert(*pubkey, nonce); + // guard dropped here + } + Ok(result) } diff --git a/magicblock-ledger/src/store/data_mod_persister.rs b/magicblock-ledger/src/store/data_mod_persister.rs index e7d7daac5..26b1e5ac1 100644 --- a/magicblock-ledger/src/store/data_mod_persister.rs +++ b/magicblock-ledger/src/store/data_mod_persister.rs @@ -1,6 +1,8 @@ use std::error::Error; use magicblock_core::traits::MagicSys; +use solana_instruction::error::InstructionError; +use solana_pubkey::Pubkey; use tracing::*; use crate::Ledger; @@ -23,4 +25,11 @@ impl MagicSys for Ledger { } Ok(data) } + // TODO(edwin): remove + fn validate_commit_nonces( + &self, + pubkeys: &[Pubkey], + ) -> Result<(), InstructionError> { + todo!() + } } From 420195f174995b45779401266db05281b6387535 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 3 Mar 2026 15:44:36 +0700 Subject: [PATCH 03/19] fix: free locks of existing accs --- .../src/intent_executor/task_info_fetcher.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index be81b21fe..706a6bfca 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -439,14 +439,13 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { // Acquire per-account locks sequentially in sorted order (see sort above). let mut guard_nonces = locks_guard.lock().await; - let (mut existing, mut to_request) = (vec![], vec![]); - let mut result = HashMap::with_capacity(existing.len()); + let mut to_request = vec![]; + let mut result = HashMap::with_capacity(guard_nonces.len()); for (pubkey, guard) in guard_nonces { if *guard == u64::MAX { to_request.push((pubkey, guard)); } else { result.insert(*pubkey, *guard); - existing.push((pubkey, guard)) } } From 6df51315602660e457ecc4350de9e2d6ecdc67cf Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 3 Mar 2026 15:45:54 +0700 Subject: [PATCH 04/19] refactor: unnecessary comment --- .../src/intent_executor/task_info_fetcher.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 706a6bfca..e89ee68ae 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -473,7 +473,6 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { { *guard = nonce; result.insert(*pubkey, nonce); - // guard dropped here } Ok(result) From 27d30621389be08deb30da5da1e28b8d121ebea2 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 3 Mar 2026 19:03:09 +0700 Subject: [PATCH 05/19] fix: other parts --- .../src/intent_executor/task_info_fetcher.rs | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index e89ee68ae..2345a217e 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -56,7 +56,7 @@ pub trait TaskInfoFetcher: Send + Sync + 'static { ) -> TaskInfoFetcherResult>; /// Peeks current commit ids for pubkeys - fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option; + async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option; /// Resets cache for some or all accounts fn reset(&self, reset_type: ResetType); @@ -498,22 +498,41 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { } /// Returns current commit id without raising priority - fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { - let cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - cache.peek(pubkey).copied() + async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { + let lock = { + // Peek without promoting LRU order; also check retiring for in-flight keys. + // Outer lock held only to clone the Arc, released before awaiting per-key lock. + let inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + inner + .active + .peek(pubkey) + .or_else(|| inner.retiring.get(pubkey)) + .cloned() + }?; + + let locks_guard = CacheInnerGuard { + inner: &self.cache, + nonce_locks: vec![(*pubkey, lock)], + }; + let guards = locks_guard.lock().await; + let value = *guards[0].1; + + (value != u64::MAX).then_some(value) } /// Reset cache fn reset(&self, reset_type: ResetType) { + let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); match reset_type { ResetType::All => { - self.cache.lock().expect(MUTEX_POISONED_MSG).clear() + cache.active.clear(); + cache.retiring.clear(); } ResetType::Specific(pubkeys) => { - let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - pubkeys.iter().for_each(|pubkey| { - let _ = cache.pop(pubkey); - }); + for pubkey in pubkeys { + cache.active.pop(pubkey); + cache.retiring.remove(pubkey); + } } } } From 854c92a35ad1347e13945dd01c754d289f7697ef Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 16:01:35 +0700 Subject: [PATCH 06/19] feat: implementing MagicSysAdapter --- Cargo.lock | 3 + magicblock-api/src/magic_sys_adapter.rs | 87 +++- magicblock-committor-service/Cargo.toml | 2 + .../src/committor_processor.rs | 25 +- magicblock-committor-service/src/error.rs | 8 +- .../src/intent_execution_manager.rs | 5 +- .../intent_scheduler.rs | 9 +- .../intent_executor_factory.rs | 4 +- .../src/intent_executor/task_info_fetcher.rs | 385 +++++++++++++++++- .../src/persist/commit_persister.rs | 9 +- magicblock-committor-service/src/service.rs | 48 ++- .../src/service_ext.rs | 9 + magicblock-committor-service/src/tasks/mod.rs | 6 +- .../src/tasks/task_builder.rs | 4 +- .../src/tasks/task_strategist.rs | 13 +- magicblock-core/Cargo.toml | 1 + magicblock-core/src/intent.rs | 59 +++ magicblock-core/src/lib.rs | 1 + magicblock-core/src/traits.rs | 12 +- .../src/store/data_mod_persister.rs | 35 -- magicblock-ledger/src/store/mod.rs | 1 - .../src/magic_scheduled_base_intent.rs | 52 +-- .../src/mutate_accounts/account_mod_data.rs | 9 - .../magicblock/src/mutate_accounts/mod.rs | 2 +- .../process_schedule_commit.rs | 4 +- programs/magicblock/src/test_utils/mod.rs | 6 +- test-integration/Cargo.lock | 2 + .../test-committor-service/tests/common.rs | 2 +- .../tests/test_intent_executor.rs | 4 +- .../tests/test_ix_commit_local.rs | 3 +- 30 files changed, 669 insertions(+), 141 deletions(-) create mode 100644 magicblock-core/src/intent.rs delete mode 100644 magicblock-ledger/src/store/data_mod_persister.rs diff --git a/Cargo.lock b/Cargo.lock index 45df90064..2893269bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3181,6 +3181,7 @@ dependencies = [ "lazy_static", "lru 0.16.2", "magicblock-committor-program", + "magicblock-core", "magicblock-delegation-program", "magicblock-metrics", "magicblock-program", @@ -3188,6 +3189,7 @@ dependencies = [ "magicblock-table-mania", "rand 0.9.2", "rusqlite", + "serde_json", "solana-account", "solana-account-decoder", "solana-address-lookup-table-interface", @@ -3244,6 +3246,7 @@ version = "0.8.1" dependencies = [ "flume", "magicblock-magic-program-api", + "serde", "solana-account", "solana-account-decoder", "solana-hash", diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index e4c5c3ce8..5229245f7 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -1,7 +1,90 @@ -use std::sync::Arc; +use std::{error::Error, sync::Arc}; +use magicblock_committor_service::{BaseIntentCommittor, CommittorService}; +use magicblock_core::{ + intent::CommittedAccount, + traits::{MagicSys, NONCE_LIMIT_ERR}, +}; use magicblock_ledger::Ledger; +use solana_instruction::error::InstructionError; +use solana_pubkey::Pubkey; +use tracing::{enabled, error, trace, Level}; +const NONCE_LIMIT: u64 = 400; + +#[derive(Clone)] pub struct MagicSysAdapter { - pub ledger: Arc, + handle: tokio::runtime::Handle, + ledger: Arc, + committor_service: Arc, +} + +impl MagicSysAdapter { + const RECV_ERR: u32 = 0xE000_0000; + const FETCH_ERR: u32 = 0xE001_0000; + + pub fn new( + ledger: Arc, + committor_service: Arc, + ) -> Self { + Self { + ledger, + committor_service, + } + } +} + +impl MagicSys for MagicSysAdapter { + fn persist(&self, id: u64, data: Vec) -> Result<(), Box> { + trace!(id, data_len = data.len(), "Persisting data"); + self.ledger.write_account_mod_data(id, &data.into())?; + Ok(()) + } + + fn load(&self, id: u64) -> Result>, Box> { + let data = self.ledger.read_account_mod_data(id)?.map(|x| x.data); + if enabled!(Level::TRACE) { + if let Some(data) = &data { + trace!(id, data_len = data.len(), "Loading data"); + } else { + trace!(id, found = false, "Loading data"); + } + } + Ok(data) + } + + fn validate_commits( + &self, + commits: &[CommittedAccount], + ) -> Result<(), InstructionError> { + let min_context_slot = commits + .iter() + .map(|account| account.remote_slot) + .max() + .unwrap_or(0); + let pubkeys: Vec<_> = + commits.iter().map(|account| account.pubkey).collect(); + + let receiver = self + .committor_service + .fetch_current_commit_nonces(&pubkeys, min_context_slot); + let nonces_map = self.handle.block_on(receiver) + .inspect_err(|err| { + error!(error = ?err, "Failed to receive nonces from CommittorService") + }) + .map_err(|err| InstructionError::Custom(Self::RECV_ERR))? + .inspect_err(|err| { + error!(error = ?err, "Failed to fetch current commit nonces") + }) + .map_err(|err| InstructionError::Custom(Self::FETCH_ERR))?; + + for (pubkey, nonce) in nonces_map { + if nonce >= NONCE_LIMIT { + trace!("Limit of commits exceeded for: {}", pubkey); + return Err(InstructionError::Custom(NONCE_LIMIT_ERR)); + } + } + + Ok(()) + } } diff --git a/magicblock-committor-service/Cargo.toml b/magicblock-committor-service/Cargo.toml index 4aa583f8d..de5433181 100644 --- a/magicblock-committor-service/Cargo.toml +++ b/magicblock-committor-service/Cargo.toml @@ -22,6 +22,7 @@ lru = { workspace = true } magicblock-committor-program = { workspace = true, features = [ "no-entrypoint", ] } +magicblock-core = { workspace = true } magicblock-delegation-program = { workspace = true, features = [ "no-entrypoint", ] } @@ -56,6 +57,7 @@ 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 } diff --git a/magicblock-committor-service/src/committor_processor.rs b/magicblock-committor-service/src/committor_processor.rs index 342c25a85..5b124b5b2 100644 --- a/magicblock-committor-service/src/committor_processor.rs +++ b/magicblock-committor-service/src/committor_processor.rs @@ -1,4 +1,8 @@ -use std::{collections::HashSet, path::Path, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + path::Path, + sync::Arc, +}; use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use magicblock_rpc_client::MagicblockRpcClient; @@ -16,6 +20,9 @@ use crate::{ intent_execution_manager::{ db::DummyDB, BroadcastedIntentExecutionResult, IntentExecutionManager, }, + intent_executor::task_info_fetcher::{ + CacheTaskInfoFetcher, TaskInfoFetcher, TaskInfoFetcherResult, + }, persist::{ CommitStatusRow, IntentPersister, IntentPersisterImpl, MessageSignatures, @@ -28,6 +35,7 @@ pub(crate) struct CommittorProcessor { pub(crate) authority: Keypair, persister: IntentPersisterImpl, commits_scheduler: IntentExecutionManager, + task_info_fetcher: Arc, } impl CommittorProcessor { @@ -58,9 +66,12 @@ impl CommittorProcessor { let persister = IntentPersisterImpl::try_new(persist_file)?; // Create commit scheduler + let task_info_fetcher = + Arc::new(CacheTaskInfoFetcher::new(magic_block_rpc_client.clone())); let commits_scheduler = IntentExecutionManager::new( magic_block_rpc_client.clone(), DummyDB::new(), + task_info_fetcher.clone(), Some(persister.clone()), table_mania.clone(), chain_config.compute_budget_config.clone(), @@ -72,6 +83,7 @@ impl CommittorProcessor { table_mania, commits_scheduler, persister, + task_info_fetcher, }) } @@ -149,4 +161,15 @@ impl CommittorProcessor { ) -> broadcast::Receiver { self.commits_scheduler.subscribe_for_results() } + + /// Fetches current commit nonces + pub async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + self.task_info_fetcher + .fetch_current_commit_nonces(pubkeys, min_context_slot) + .await + } } diff --git a/magicblock-committor-service/src/error.rs b/magicblock-committor-service/src/error.rs index 266c1388c..fa13e49b8 100644 --- a/magicblock-committor-service/src/error.rs +++ b/magicblock-committor-service/src/error.rs @@ -3,7 +3,10 @@ use solana_signature::Signature; use solana_transaction_error::TransactionError; use thiserror::Error; -use crate::intent_execution_manager::IntentExecutionManagerError; +use crate::{ + intent_execution_manager::IntentExecutionManagerError, + intent_executor::task_info_fetcher::TaskInfoFetcherError, +}; pub type CommittorServiceResult = Result; @@ -26,6 +29,9 @@ pub enum CommittorServiceError { #[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:?})" )] diff --git a/magicblock-committor-service/src/intent_execution_manager.rs b/magicblock-committor-service/src/intent_execution_manager.rs index bb13063c6..6ded30e43 100644 --- a/magicblock-committor-service/src/intent_execution_manager.rs +++ b/magicblock-committor-service/src/intent_execution_manager.rs @@ -33,19 +33,18 @@ impl IntentExecutionManager { pub fn new( rpc_client: MagicblockRpcClient, db: D, + task_info_fetcher: Arc, intent_persister: Option

, table_mania: TableMania, compute_budget_config: ComputeBudgetConfig, ) -> Self { let db = Arc::new(db); - let commit_id_tracker = - Arc::new(CacheTaskInfoFetcher::new(rpc_client.clone())); let executor_factory = IntentExecutorFactoryImpl { rpc_client, table_mania, compute_budget_config, - commit_id_tracker, + task_info_fetcher, }; let (sender, receiver) = mpsc::channel(1000); diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs index 57f497131..c9a123933 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs @@ -1,5 +1,6 @@ use std::collections::{hash_map::Entry, HashMap, VecDeque}; +use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_pubkey::Pubkey; use thiserror::Error; @@ -637,7 +638,7 @@ mod edge_cases_test { #[cfg(test)] mod complete_error_test { - use magicblock_program::magic_scheduled_base_intent::CommittedAccount; + use magicblock_core::intent::CommittedAccount; use solana_account::Account; use solana_pubkey::pubkey; @@ -854,8 +855,9 @@ pub(crate) fn create_test_intent( pubkeys: &[Pubkey], is_undelegate: bool, ) -> ScheduledIntentBundle { + use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, CommittedAccount, MagicIntentBundle, + CommitAndUndelegate, CommitType, MagicIntentBundle, ScheduledIntentBundle, UndelegateType, }; use solana_account::Account; @@ -903,8 +905,9 @@ pub(crate) fn create_test_intent_bundle( commit_pubkeys: &[Pubkey], commit_and_undelegate_pubkeys: &[Pubkey], ) -> ScheduledIntentBundle { + use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, CommittedAccount, MagicIntentBundle, + CommitAndUndelegate, CommitType, MagicIntentBundle, ScheduledIntentBundle, UndelegateType, }; use solana_account::Account; diff --git a/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs b/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs index 24ac3d33c..1d46e43a9 100644 --- a/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs +++ b/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs @@ -23,7 +23,7 @@ pub struct IntentExecutorFactoryImpl { pub rpc_client: MagicblockRpcClient, pub table_mania: TableMania, pub compute_budget_config: ComputeBudgetConfig, - pub commit_id_tracker: Arc, + pub task_info_fetcher: Arc, } impl IntentExecutorFactory for IntentExecutorFactoryImpl { @@ -39,7 +39,7 @@ impl IntentExecutorFactory for IntentExecutorFactoryImpl { IntentExecutorImpl::::new( self.rpc_client.clone(), transaction_preparator, - self.commit_id_tracker.clone(), + self.task_info_fetcher.clone(), ) } } diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 2345a217e..bdb2f3125 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -1,8 +1,7 @@ use std::{ - collections::{HashMap, HashSet}, + collections::HashMap, mem, num::NonZeroUsize, - ops::Add, sync::{Arc, Mutex}, time::Duration, }; @@ -11,7 +10,6 @@ use async_trait::async_trait; use dlp::{ delegation_metadata_seeds_from_delegated_account, state::DelegationMetadata, }; -use dyn_clone::clone; use lru::LruCache; use magicblock_metrics::metrics; use magicblock_rpc_client::{MagicBlockRpcClientError, MagicblockRpcClient}; @@ -143,6 +141,16 @@ impl CacheTaskInfoFetcher { } } + pub fn with_capacity( + capacity: NonZeroUsize, + rpc_client: MagicblockRpcClient, + ) -> Self { + Self { + rpc_client, + cache: Mutex::new(CacheInner::new(capacity)), + } + } + /// Fetches [`DelegationMetadata`]s with some num of retries pub async fn fetch_metadata_with_retries( rpc_client: &MagicblockRpcClient, @@ -374,7 +382,7 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { // Acquire per-account locks sequentially in sorted order (see sort above). // join_all would poll all futures concurrently, allowing partial acquisition // and producing the classic A→B / B→A deadlock across concurrent callers. - let mut guard_nonces = locks_guard.lock().await; + let guard_nonces = locks_guard.lock().await; let (mut existing, mut to_request) = (vec![], vec![]); for (pubkey, guard) in guard_nonces { if *guard == u64::MAX { @@ -438,7 +446,7 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { let locks_guard = self.reserve_locks(pubkeys); // Acquire per-account locks sequentially in sorted order (see sort above). - let mut guard_nonces = locks_guard.lock().await; + let guard_nonces = locks_guard.lock().await; let mut to_request = vec![]; let mut result = HashMap::with_capacity(guard_nonces.len()); for (pubkey, guard) in guard_nonces { @@ -619,3 +627,370 @@ impl TaskInfoFetcherError { } pub type TaskInfoFetcherResult = Result; + +#[cfg(test)] +mod tests { + use std::{collections::VecDeque, sync::Arc, time::Duration}; + + use async_trait::async_trait; + use base64::{prelude::BASE64_STANDARD, Engine}; + use dlp::state::DelegationMetadata; + use solana_account_decoder::{UiAccount, UiAccountData, UiAccountEncoding}; + use solana_pubkey::Pubkey; + use solana_rpc_client::{ + nonblocking::rpc_client::RpcClient, + rpc_client::RpcClientConfig, + rpc_sender::{RpcSender, RpcTransportStats}, + }; + use solana_rpc_client_api::{ + client_error::Result as ClientResult, + request::RpcRequest, + response::{Response, RpcResponseContext}, + }; + + use super::*; + + // ---- mock ---- + + struct TestSender { + calls: std::sync::Mutex>, + rpc_delay: Option, + } + + impl TestSender { + fn new(responses: Vec) -> Self { + Self { + calls: std::sync::Mutex::new(responses.into()), + rpc_delay: None, + } + } + + fn with_delay(mut self, delay: Duration) -> Self { + self.rpc_delay = Some(delay); + self + } + } + + #[async_trait] + impl RpcSender for TestSender { + async fn send( + &self, + request: RpcRequest, + params: serde_json::Value, + ) -> ClientResult { + if let Some(delay) = self.rpc_delay { + tokio::time::sleep(delay).await; + } + assert_eq!(request, RpcRequest::GetMultipleAccounts); + let n = params[0].as_array().map(|a| a.len()).unwrap_or(0); + let metas: Vec = { + let mut q = self.calls.lock().unwrap(); + (0..n) + .map(|_| { + q.pop_front() + .expect("unexpected RPC call: queue exhausted") + }) + .collect() + }; + let accounts: Vec> = + metas.iter().map(|m| Some(encode_meta(m))).collect(); + Ok(serde_json::to_value(Response { + context: RpcResponseContext { + slot: 1, + api_version: None, + }, + value: accounts, + }) + .unwrap()) + } + + fn get_transport_stats(&self) -> RpcTransportStats { + RpcTransportStats::default() + } + + fn url(&self) -> String { + "test".to_string() + } + } + + fn encode_meta(m: &DelegationMetadata) -> UiAccount { + let mut bytes = Vec::new(); + m.to_bytes_with_discriminator(&mut bytes).unwrap(); + UiAccount { + lamports: 1_000_000, + data: UiAccountData::Binary( + BASE64_STANDARD.encode(&bytes), + UiAccountEncoding::Base64, + ), + owner: dlp::id().to_string(), + executable: false, + rent_epoch: 0, + space: Some(bytes.len() as u64), + } + } + + fn meta(nonce: u64) -> DelegationMetadata { + DelegationMetadata { + last_update_nonce: nonce, + is_undelegatable: false, + seeds: vec![], + rent_payer: Pubkey::default(), + } + } + + struct FetcherBuilder { + sender: TestSender, + capacity: Option, + } + + impl FetcherBuilder { + fn new(responses: Vec) -> Self { + Self { + sender: TestSender::new(responses), + capacity: None, + } + } + + fn rpc_delay(mut self, d: Duration) -> Self { + self.sender = self.sender.with_delay(d); + self + } + + fn capacity(mut self, n: usize) -> Self { + self.capacity = Some(n.try_into().unwrap()); + self + } + + fn build(self) -> CacheTaskInfoFetcher { + let rpc = MagicblockRpcClient::new(Arc::new( + RpcClient::new_sender(self.sender, RpcClientConfig::default()), + )); + match self.capacity { + Some(cap) => CacheTaskInfoFetcher::with_capacity(cap, rpc), + None => CacheTaskInfoFetcher::new(rpc), + } + } + } + + // ---- tests ---- + + #[tokio::test] + async fn cache_miss_then_hit() { + let pk = Pubkey::new_unique(); + let fetcher = FetcherBuilder::new(vec![meta(10)]).build(); + + let r1 = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r1[&pk], 11); + + // Cache hit: no RPC (only 1 response queued), increments + let r2 = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r2[&pk], 12); + } + + #[tokio::test] + async fn partial_cache_hit() { + let pk1 = Pubkey::new_unique(); + let pk2 = Pubkey::new_unique(); + // prime pk1 (nonce 5), then mixed call fetches only cold pk2 (nonce 20) + let fetcher = FetcherBuilder::new(vec![meta(5), meta(20)]).build(); + + fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 6 + let r = fetcher + .fetch_next_commit_nonces(&[pk1, pk2], 0) + .await + .unwrap(); + assert_eq!(r[&pk1], 7); // cached, incremented + assert_eq!(r[&pk2], 21); // fetched from chain + } + + #[tokio::test] + async fn lru_eviction_forces_refetch() { + let pk1 = Pubkey::new_unique(); + let pk2 = Pubkey::new_unique(); + // pk1 initial, pk2 evicts pk1, pk1 re-fetch after eviction + let fetcher = FetcherBuilder::new(vec![meta(1), meta(2), meta(10)]) + .capacity(1) + .build(); + + fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 + fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3, pk1 evicted + let r = fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); + assert_eq!(r[&pk1], 11); // re-fetched (10 + 1) + } + + // Phase 1: fetch phase1_keys one-by-one → barrier → outer verification → + // barrier. Phase 2: fetch phase2_keys one-by-one for `iters` passes, then + // fetch shared_b in chunks of 2. + async fn run_worker( + fetcher: Arc, + barrier: Arc, + phase1_keys: Vec, + phase2_keys: Vec, + shared_b: Vec, + iters: usize, + ) { + for pk in &phase1_keys { + fetcher.fetch_next_commit_nonces(&[*pk], 0).await.unwrap(); + } + barrier.wait().await; // signal phase 1 done + barrier.wait().await; // wait for outer verification + for _ in 0..iters { + for pk in &phase2_keys { + fetcher.fetch_next_commit_nonces(&[*pk], 0).await.unwrap(); + } + } + for chunk in shared_b.chunks(2) { + fetcher.fetch_next_commit_nonces(chunk, 0).await.unwrap(); + } + } + + // Three concurrent workers operating in two phases, separated by a barrier. + #[tokio::test(flavor = "multi_thread")] + async fn three_concurrent_workers_two_phase() { + const ITERS: usize = 50; + const NUM_WORKERS: usize = 3; + const SHARED_A: usize = 10; + const SHARED_B: usize = 40; + const EXCLUSIVE: usize = 50; + const CAPACITY: usize = 30; + const PHASE2_KEYS: usize = EXCLUSIVE + SHARED_A; // 60 + + let shared_a: Vec = + (0..SHARED_A).map(|_| Pubkey::new_unique()).collect(); + let shared_b: Vec = + (0..SHARED_B).map(|_| Pubkey::new_unique()).collect(); + let excl: [Vec; NUM_WORKERS] = std::array::from_fn(|_| { + (0..EXCLUSIVE).map(|_| Pubkey::new_unique()).collect() + }); + let phase2_keys: [Vec; NUM_WORKERS] = + std::array::from_fn(|i| { + excl[i].iter().chain(shared_a.iter()).cloned().collect() + }); + + // Flat queue: each RPC call pops exactly N entries (N cold keys). + // Upper bounds: + // phase 1 loop: SHARED_A × 1 + // phase 2 excl+sa: ITERS × PHASE2_KEYS × NUM_WORKERS × 1 + // phase 2 shared_b: (SHARED_B / chunk_size) × chunk_size × NUM_WORKERS + // = SHARED_B × NUM_WORKERS + let total = SHARED_A + + ITERS * PHASE2_KEYS * NUM_WORKERS + + SHARED_B * NUM_WORKERS; + let responses: Vec = + (0..total).map(|_| meta(0)).collect(); + + let fetcher = Arc::new( + FetcherBuilder::new(responses) + .capacity(CAPACITY) + .rpc_delay(Duration::from_millis(2)) + .build(), + ); + + // Barrier resets automatically: round 1 syncs after phase 1, round 2 + // releases workers into phase 2 after outer verification. + let barrier = Arc::new(tokio::sync::Barrier::new(NUM_WORKERS + 1)); + + let handles: Vec<_> = (0..NUM_WORKERS) + .map(|i| { + tokio::spawn(run_worker( + fetcher.clone(), + barrier.clone(), + shared_a.clone(), + phase2_keys[i].clone(), + shared_b.clone(), + ITERS, + )) + }) + .collect(); + + barrier.wait().await; // all workers done with phase 1 + + // No eviction during phase 1 (10 keys < capacity 30). + // Per-key lock serialises 3 workers → exactly NUM_WORKERS increments each. + for pk in &shared_a { + assert_eq!( + fetcher.peek_commit_nonce(pk).await, + Some(NUM_WORKERS as u64) + ); + } + + barrier.wait().await; // release workers into phase 2 + + for h in handles { + h.await.unwrap(); + } + + // Workers fetched shared_b in chunks of 2 (40 / 2 = 20 calls each). + // Due to eviction (40 keys > capacity 30) not all may remain in cache. + // Any key still in cache must have nonce >= 1. + let mut found = 0usize; + for pk in &shared_b { + if let Some(n) = fetcher.peek_commit_nonce(pk).await { + assert!(n >= 1); + found += 1; + } + } + assert!( + found > 0, + "expected some shared_b keys in cache after workers" + ); + } + + #[tokio::test] + async fn fetch_current_no_increment() { + let pk = Pubkey::new_unique(); + let fetcher = FetcherBuilder::new(vec![meta(10)]).build(); + + let r1 = fetcher.fetch_current_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r1[&pk], 10); // stored as-is + + // Cache hit: still 10, fetch_current never increments + let r2 = fetcher.fetch_current_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r2[&pk], 10); + } + + #[tokio::test] + async fn reset_all_forces_refetch() { + let pk = Pubkey::new_unique(); + let fetcher = FetcherBuilder::new(vec![meta(5), meta(99)]).build(); + + fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); // pk = 6 + fetcher.reset(ResetType::All); + + let r = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r[&pk], 100); // re-fetched + } + + #[tokio::test] + async fn reset_specific_only_clears_that_key() { + let pk1 = Pubkey::new_unique(); + let pk2 = Pubkey::new_unique(); + // pk1 initial, pk2 initial, pk1 after reset + let fetcher = + FetcherBuilder::new(vec![meta(1), meta(2), meta(50)]).build(); + + fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 + fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3 + fetcher.reset(ResetType::Specific(&[pk1])); + + let r1 = fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); + let r2 = fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); + assert_eq!(r1[&pk1], 51); // re-fetched (50 + 1) + assert_eq!(r2[&pk2], 4); // still cached (3 + 1) + } + + #[tokio::test] + async fn peek_does_not_increment() { + let pk = Pubkey::new_unique(); + let fetcher = FetcherBuilder::new(vec![meta(7)]).build(); + + assert_eq!(fetcher.peek_commit_nonce(&pk).await, None); + + fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); // pk = 8 + assert_eq!(fetcher.peek_commit_nonce(&pk).await, Some(8)); + assert_eq!(fetcher.peek_commit_nonce(&pk).await, Some(8)); // unchanged + + let r = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); + assert_eq!(r[&pk], 9); // peek didn't change the value + } +} diff --git a/magicblock-committor-service/src/persist/commit_persister.rs b/magicblock-committor-service/src/persist/commit_persister.rs index c9399bff9..d16a621ae 100644 --- a/magicblock-committor-service/src/persist/commit_persister.rs +++ b/magicblock-committor-service/src/persist/commit_persister.rs @@ -3,9 +3,8 @@ use std::{ sync::{Arc, Mutex}, }; -use magicblock_program::magic_scheduled_base_intent::{ - CommittedAccount, ScheduledIntentBundle, -}; +use magicblock_core::intent::CommittedAccount; +use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_pubkey::Pubkey; use super::{ @@ -436,9 +435,9 @@ impl IntentPersister for Option { #[cfg(test)] mod tests { + use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, CommittedAccount, MagicIntentBundle, - UndelegateType, + CommitAndUndelegate, CommitType, MagicIntentBundle, UndelegateType, }; use solana_account::Account; use solana_hash::Hash; diff --git a/magicblock-committor-service/src/service.rs b/magicblock-committor-service/src/service.rs index 6101e41a0..289e91c56 100644 --- a/magicblock-committor-service/src/service.rs +++ b/magicblock-committor-service/src/service.rs @@ -1,4 +1,4 @@ -use std::{path::Path, sync::Arc, time::Instant}; +use std::{collections::HashMap, path::Path, sync::Arc, time::Instant}; use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_keypair::Keypair; @@ -19,7 +19,7 @@ use tracing::*; use crate::{ committor_processor::CommittorProcessor, config::ChainConfig, - error::CommittorServiceResult, + error::{CommittorServiceError, CommittorServiceResult}, intent_execution_manager::BroadcastedIntentExecutionResult, persist::{CommitStatusRow, MessageSignatures}, pubkeys_provider::{provide_committee_pubkeys, provide_common_pubkeys}, @@ -83,6 +83,12 @@ pub enum CommittorMessage { broadcast::Receiver, >, }, + FetchCurrentCommitNonces { + respond_to: + oneshot::Sender>>, + pubkeys: Vec, + min_context_slot: u64, + }, } // ----------------- @@ -228,6 +234,23 @@ impl CommittorActor { 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"); + } + }); + } } } @@ -424,6 +447,21 @@ impl BaseIntentCommittor for CommittorService { rx } + fn fetch_current_commit_nonces( + &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, + }); + + rx + } + fn stop(&self) { self.cancel_token.cancel(); } @@ -472,6 +510,12 @@ pub trait BaseIntentCommittor: Send + Sync + 'static { CommittorServiceResult, >; + fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> oneshot::Receiver>>; + /// Stops Committor service fn stop(&self); diff --git a/magicblock-committor-service/src/service_ext.rs b/magicblock-committor-service/src/service_ext.rs index 5a77d9880..024d0423e 100644 --- a/magicblock-committor-service/src/service_ext.rs +++ b/magicblock-committor-service/src/service_ext.rs @@ -207,6 +207,15 @@ impl BaseIntentCommittor for CommittorServiceExt { 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(); } diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index b4af90c58..6659df60c 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -11,10 +11,9 @@ use magicblock_committor_program::{ }, pdas, ChangesetChunks, Chunks, }; +use magicblock_core::intent::CommittedAccount; use magicblock_metrics::metrics::LabelValue; -use magicblock_program::magic_scheduled_base_intent::{ - BaseAction, CommittedAccount, -}; +use magicblock_program::magic_scheduled_base_intent::BaseAction; use solana_account::Account; use solana_instruction::{AccountMeta, Instruction}; use solana_pubkey::Pubkey; @@ -356,6 +355,7 @@ pub type BaseTaskResult = Result; #[cfg(test)] mod serialization_safety_test { + use magicblock_core::intent::CommittedAccount; use magicblock_program::{ args::ShortAccountMeta, magic_scheduled_base_intent::ProgramArgs, }; diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index cb3c385fb..3035d5756 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -1,9 +1,9 @@ use std::{collections::HashMap, sync::Arc}; use async_trait::async_trait; +use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - BaseAction, CommitType, CommittedAccount, ScheduledIntentBundle, - UndelegateType, + BaseAction, CommitType, ScheduledIntentBundle, UndelegateType, }; use solana_account::Account; use solana_pubkey::Pubkey; diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index 95146f87a..4e446d944 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -381,8 +381,9 @@ pub type TaskStrategistResult = Result; mod tests { use std::{collections::HashMap, sync::Arc}; + use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - BaseAction, CommittedAccount, ProgramArgs, + BaseAction, ProgramArgs, }; use solana_account::Account; use solana_program::system_program; @@ -417,6 +418,14 @@ mod tests { Ok(pubkeys.iter().map(|pubkey| (*pubkey, 0)).collect()) } + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + Ok(pubkeys.iter().map(|pubkey| (*pubkey, 0)).collect()) + } + async fn fetch_rent_reimbursements( &self, pubkeys: &[Pubkey], @@ -425,7 +434,7 @@ mod tests { Ok(pubkeys.iter().map(|_| Pubkey::new_unique()).collect()) } - fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { + async fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { Some(0) } diff --git a/magicblock-core/Cargo.toml b/magicblock-core/Cargo.toml index 308b441a1..ce7a883ac 100644 --- a/magicblock-core/Cargo.toml +++ b/magicblock-core/Cargo.toml @@ -12,6 +12,7 @@ edition.workspace = true tokio = { workspace = true, features = ["sync"] } flume = { workspace = true } +serde = { workspace = true, features = ["derive"] } solana-account = { workspace = true } solana-account-decoder = { workspace = true } solana-hash = { workspace = true } diff --git a/magicblock-core/src/intent.rs b/magicblock-core/src/intent.rs new file mode 100644 index 000000000..69a23a85e --- /dev/null +++ b/magicblock-core/src/intent.rs @@ -0,0 +1,59 @@ +use std::cell::RefCell; + +use serde::{Deserialize, Serialize}; +use solana_account::{Account, AccountSharedData}; +use solana_pubkey::Pubkey; + +use crate::token_programs::try_remap_ata_to_eata; + +pub type CommittedAccountRef<'a> = (Pubkey, &'a RefCell); + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CommittedAccount { + pub pubkey: Pubkey, + pub account: Account, + pub remote_slot: u64, +} + +impl<'a> From> for CommittedAccount { + fn from(value: CommittedAccountRef<'a>) -> Self { + let account = value.1.borrow(); + let remote_slot = account.remote_slot(); + Self { + pubkey: value.0, + account: account.to_owned().into(), + remote_slot, + } + } +} + +impl CommittedAccount { + /// Build a CommittedAccount from an AccountSharedData reference, optionally + /// overriding the owner with `parent_program_id` and remapping ATA -> eATA + /// if applicable. + pub fn from_account_shared( + pubkey: Pubkey, + account_shared: &AccountSharedData, + parent_program_id: Option, + ) -> Self { + let remote_slot = account_shared.remote_slot(); + if let Some((eata_pubkey, eata)) = + try_remap_ata_to_eata(&pubkey, account_shared) + { + return CommittedAccount { + pubkey: eata_pubkey, + account: eata.into(), + remote_slot, + }; + } + + let mut account: Account = account_shared.to_owned().into(); + account.owner = parent_program_id.unwrap_or(account.owner); + + CommittedAccount { + pubkey, + account, + remote_slot, + } + } +} diff --git a/magicblock-core/src/lib.rs b/magicblock-core/src/lib.rs index f18f8a6f1..fcb7126f6 100644 --- a/magicblock-core/src/lib.rs +++ b/magicblock-core/src/lib.rs @@ -18,4 +18,5 @@ pub mod tls; pub mod token_programs; pub mod traits; +pub mod intent; pub mod logger; diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index 075c78cc3..74c8d374f 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -1,13 +1,17 @@ use std::{error::Error, fmt}; use solana_program::instruction::InstructionError; -use solana_pubkey::Pubkey; -pub trait MagicSys: Sync + Send + fmt::Display + 'static { +use crate::intent::CommittedAccount; + +pub const NONCE_LIMIT_ERR: u32 = 0xA000_0000; + +pub trait MagicSys: Sync + Send + 'static { fn persist(&self, id: u64, data: Vec) -> Result<(), Box>; fn load(&self, id: u64) -> Result>, Box>; - fn validate_commit_nonces( + + fn validate_commits( &self, - pubkeys: &[Pubkey], + pubkeys: &[CommittedAccount], ) -> Result<(), InstructionError>; } diff --git a/magicblock-ledger/src/store/data_mod_persister.rs b/magicblock-ledger/src/store/data_mod_persister.rs deleted file mode 100644 index 26b1e5ac1..000000000 --- a/magicblock-ledger/src/store/data_mod_persister.rs +++ /dev/null @@ -1,35 +0,0 @@ -use std::error::Error; - -use magicblock_core::traits::MagicSys; -use solana_instruction::error::InstructionError; -use solana_pubkey::Pubkey; -use tracing::*; - -use crate::Ledger; - -impl MagicSys for Ledger { - fn persist(&self, id: u64, data: Vec) -> Result<(), Box> { - trace!(id, data_len = data.len(), "Persisting data"); - self.write_account_mod_data(id, &data.into())?; - Ok(()) - } - - fn load(&self, id: u64) -> Result>, Box> { - let data = self.read_account_mod_data(id)?.map(|x| x.data); - if enabled!(Level::TRACE) { - if let Some(data) = &data { - trace!(id, data_len = data.len(), "Loading data"); - } else { - trace!(id, found = false, "Loading data"); - } - } - Ok(data) - } - // TODO(edwin): remove - fn validate_commit_nonces( - &self, - pubkeys: &[Pubkey], - ) -> Result<(), InstructionError> { - todo!() - } -} diff --git a/magicblock-ledger/src/store/mod.rs b/magicblock-ledger/src/store/mod.rs index 1280c2685..dcc5f0cff 100644 --- a/magicblock-ledger/src/store/mod.rs +++ b/magicblock-ledger/src/store/mod.rs @@ -1,3 +1,2 @@ pub mod api; -pub mod data_mod_persister; mod utils; diff --git a/programs/magicblock/src/magic_scheduled_base_intent.rs b/programs/magicblock/src/magic_scheduled_base_intent.rs index 08c63153e..f5e3921fd 100644 --- a/programs/magicblock/src/magic_scheduled_base_intent.rs +++ b/programs/magicblock/src/magic_scheduled_base_intent.rs @@ -1,6 +1,7 @@ use std::{cell::RefCell, collections::HashSet}; use magicblock_core::{ + intent::{CommittedAccount, CommittedAccountRef}, token_programs::{ try_remap_ata_to_eata, EATA_PROGRAM_ID, TOKEN_PROGRAM_ID, }, @@ -605,57 +606,6 @@ impl BaseAction { } } -type CommittedAccountRef<'a> = (Pubkey, &'a RefCell); -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct CommittedAccount { - pub pubkey: Pubkey, - pub account: Account, - pub remote_slot: u64, -} - -impl<'a> From> for CommittedAccount { - fn from(value: CommittedAccountRef<'a>) -> Self { - let account = value.1.borrow(); - let remote_slot = account.remote_slot(); - Self { - pubkey: value.0, - account: account.to_owned().into(), - remote_slot, - } - } -} - -impl CommittedAccount { - /// Build a CommittedAccount from an AccountSharedData reference, optionally - /// overriding the owner with `parent_program_id` and remapping ATA -> eATA - /// if applicable. - pub fn from_account_shared( - pubkey: Pubkey, - account_shared: &AccountSharedData, - parent_program_id: Option, - ) -> Self { - let remote_slot = account_shared.remote_slot(); - if let Some((eata_pubkey, eata)) = - try_remap_ata_to_eata(&pubkey, account_shared) - { - return CommittedAccount { - pubkey: eata_pubkey, - account: eata.into(), - remote_slot, - }; - } - - let mut account: Account = account_shared.to_owned().into(); - account.owner = parent_program_id.unwrap_or(account.owner); - - CommittedAccount { - pubkey, - account, - remote_slot, - } - } -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum CommitType { /// Regular commit without actions diff --git a/programs/magicblock/src/mutate_accounts/account_mod_data.rs b/programs/magicblock/src/mutate_accounts/account_mod_data.rs index 943a316d3..7a64465f7 100644 --- a/programs/magicblock/src/mutate_accounts/account_mod_data.rs +++ b/programs/magicblock/src/mutate_accounts/account_mod_data.rs @@ -82,15 +82,6 @@ pub fn init_persister(persister: Arc) { .replace(persister); } -pub fn persister_info() -> String { - PERSISTER - .read() - .expect("PERSISTER poisoned") - .as_ref() - .map(|p| p.to_string()) - .unwrap_or_else(|| "None".to_string()) -} - fn load_data(id: u64) -> Result>, Box> { PERSISTER .read() diff --git a/programs/magicblock/src/mutate_accounts/mod.rs b/programs/magicblock/src/mutate_accounts/mod.rs index 041dcc697..eb89b7dda 100644 --- a/programs/magicblock/src/mutate_accounts/mod.rs +++ b/programs/magicblock/src/mutate_accounts/mod.rs @@ -1,5 +1,5 @@ mod account_mod_data; mod process_mutate_accounts; +pub use account_mod_data::init_persister; pub(crate) use account_mod_data::*; -pub use account_mod_data::{init_persister, persister_info}; pub(crate) use process_mutate_accounts::process_mutate_accounts; diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index 8817e5985..fc59ea5d0 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; +use magicblock_core::intent::CommittedAccount; // no direct token remap helpers needed here; handled in CommittedAccount builder use solana_account::{state_traits::StateMut, ReadableAccount}; use solana_instruction::error::InstructionError; @@ -10,8 +11,7 @@ use solana_pubkey::Pubkey; use crate::{ magic_scheduled_base_intent::{ validate_commit_schedule_permissions, CommitAndUndelegate, CommitType, - CommittedAccount, MagicBaseIntent, ScheduledIntentBundle, - UndelegateType, + MagicBaseIntent, ScheduledIntentBundle, UndelegateType, }, schedule_transactions, utils::{ diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index d0ab16f5b..2409c2405 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -8,7 +8,7 @@ use std::{ }, }; -use magicblock_core::traits::MagicSys; +use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use magicblock_magic_program_api::{id, EPHEMERAL_VAULT_PUBKEY}; use solana_account::AccountSharedData; use solana_instruction::{error::InstructionError, AccountMeta}; @@ -93,9 +93,9 @@ impl MagicSys for PersisterStub { Err("Loading from ledger not supported in tests".into()) } - fn validate_commit_nonces( + fn validate_commits( &self, - pubkeys: &[Pubkey], + commits: &[CommittedAccount], ) -> Result<(), InstructionError> { // TODO(edwin) todo!() diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index 6eb399f60..d3c4e8aed 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3522,6 +3522,7 @@ dependencies = [ "futures-util", "lru", "magicblock-committor-program", + "magicblock-core", "magicblock-delegation-program 1.1.3 (git+https://github.com/magicblock-labs/delegation-program.git?rev=2cb491032f372)", "magicblock-metrics", "magicblock-program", @@ -3578,6 +3579,7 @@ version = "0.8.1" dependencies = [ "flume", "magicblock-magic-program-api 0.8.1", + "serde", "solana-account", "solana-account-decoder", "solana-hash", diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index 2518164e7..71b45e604 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -21,7 +21,7 @@ use magicblock_committor_service::{ }, ComputeBudgetConfig, }; -use magicblock_program::magic_scheduled_base_intent::CommittedAccount; +use magicblock_core::intent::CommittedAccount; use magicblock_rpc_client::MagicblockRpcClient; use magicblock_table_mania::{GarbageCollectorConfig, TableMania}; use solana_account::Account; 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 9f18d770f..8c15f17e8 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -32,7 +32,7 @@ use magicblock_committor_service::{ use magicblock_program::{ args::ShortAccountMeta, magic_scheduled_base_intent::{ - BaseAction, CommitAndUndelegate, CommitType, CommittedAccount, + BaseAction, CommitAndUndelegate, CommitType, MagicBaseIntent, ProgramArgs, ScheduledIntentBundle, UndelegateType, }, validator::validator_authority_id, @@ -55,7 +55,7 @@ use solana_sdk::{ signature::{Keypair, Signer}, transaction::{Transaction, TransactionError}, }; - +use magicblock_core::intent::CommittedAccount; use crate::{ common::TestFixture, utils::{ 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 0a0417fce..a3d2190eb 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 @@ -13,7 +13,7 @@ use magicblock_committor_service::{ BaseIntentCommittor, CommittorService, ComputeBudgetConfig, }; use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, CommittedAccount, MagicBaseIntent, + CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, ScheduledIntentBundle, UndelegateType, }; use magicblock_rpc_client::MagicblockRpcClient; @@ -28,6 +28,7 @@ use solana_sdk::{ use test_kit::init_logger; use tokio::task::JoinSet; use tracing::*; +use magicblock_core::intent::CommittedAccount; use utils::transactions::tx_logs_contain; use self::utils::transactions::init_and_delegate_order_book_on_chain; From 3e0508744cbb4d915dee4ac44bb00b18231f50e4 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 18:15:38 +0700 Subject: [PATCH 07/19] feat: support MagicSys through out validator --- magicblock-api/src/magic_sys_adapter.rs | 21 +++++--- magicblock-api/src/magic_validator.rs | 9 +++- magicblock-chainlink/src/lib.rs | 2 - .../intent_scheduler.rs | 1 - .../src/intent_executor/task_info_fetcher.rs | 1 + magicblock-core/src/traits.rs | 2 +- programs/magicblock/src/lib.rs | 3 +- .../src/magic_scheduled_base_intent.rs | 8 ++- programs/magicblock/src/magic_sys.rs | 54 +++++++++++++++++++ .../src/mutate_accounts/account_mod_data.rs | 43 +++------------ .../magicblock/src/mutate_accounts/mod.rs | 1 - .../process_schedule_commit.rs | 12 ++++- .../process_schedule_intent_bundle.rs | 12 +++++ programs/magicblock/src/test_utils/mod.rs | 19 ++++--- 14 files changed, 120 insertions(+), 68 deletions(-) create mode 100644 programs/magicblock/src/magic_sys.rs diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 5229245f7..c953b6309 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -7,7 +7,6 @@ use magicblock_core::{ }; use magicblock_ledger::Ledger; use solana_instruction::error::InstructionError; -use solana_pubkey::Pubkey; use tracing::{enabled, error, trace, Level}; const NONCE_LIMIT: u64 = 400; @@ -16,18 +15,20 @@ const NONCE_LIMIT: u64 = 400; pub struct MagicSysAdapter { handle: tokio::runtime::Handle, ledger: Arc, - committor_service: Arc, + committor_service: Option>, } impl MagicSysAdapter { const RECV_ERR: u32 = 0xE000_0000; const FETCH_ERR: u32 = 0xE001_0000; + const NO_COMMITTOR_ERR: u32 = 0xE002_0000; pub fn new( ledger: Arc, - committor_service: Arc, + committor_service: Option>, ) -> Self { Self { + handle: tokio::runtime::Handle::current(), ledger, committor_service, } @@ -57,6 +58,13 @@ impl MagicSys for MagicSysAdapter { &self, commits: &[CommittedAccount], ) -> Result<(), InstructionError> { + 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() .map(|account| account.remote_slot) @@ -65,18 +73,17 @@ impl MagicSys for MagicSysAdapter { let pubkeys: Vec<_> = commits.iter().map(|account| account.pubkey).collect(); - let receiver = self - .committor_service + let receiver = committor_service .fetch_current_commit_nonces(&pubkeys, min_context_slot); let nonces_map = self.handle.block_on(receiver) .inspect_err(|err| { error!(error = ?err, "Failed to receive nonces from CommittorService") }) - .map_err(|err| InstructionError::Custom(Self::RECV_ERR))? + .map_err(|_| InstructionError::Custom(Self::RECV_ERR))? .inspect_err(|err| { error!(error = ?err, "Failed to fetch current commit nonces") }) - .map_err(|err| InstructionError::Custom(Self::FETCH_ERR))?; + .map_err(|_| InstructionError::Custom(Self::FETCH_ERR))?; for (pubkey, nonce) in nonces_map { if nonce >= NONCE_LIMIT { diff --git a/magicblock-api/src/magic_validator.rs b/magicblock-api/src/magic_validator.rs index c8231c9e9..c36d516ec 100644 --- a/magicblock-api/src/magic_validator.rs +++ b/magicblock-api/src/magic_validator.rs @@ -57,7 +57,7 @@ use magicblock_processor::{ scheduler::{state::TransactionSchedulerState, TransactionScheduler}, }; use magicblock_program::{ - init_persister, + init_magic_sys, validator::{self, validator_authority}, TransactionScheduler as ActionTransactionScheduler, }; @@ -91,6 +91,7 @@ use crate::{ self, read_validator_keypair_from_ledger, validator_keypair_path, write_validator_keypair_to_ledger, }, + magic_sys_adapter::MagicSysAdapter, slot::advance_slot_and_update_ledger, tickers::{init_slot_ticker, init_system_metrics_ticker}, }; @@ -209,6 +210,11 @@ impl MagicValidator { let step_start = Instant::now(); let committor_service = Self::init_committor_service(&config).await?; log_timing("startup", "committor_service_init", step_start); + init_magic_sys(Arc::new(MagicSysAdapter::new( + ledger.clone(), + committor_service.clone(), + ))); + let step_start = Instant::now(); let chainlink = Arc::new( Self::init_chainlink( @@ -474,7 +480,6 @@ impl MagicValidator { ) -> ApiResult<(Arc, Slot)> { let (ledger, last_slot) = ledger::init(storage, ledger_config)?; let ledger_shared = Arc::new(ledger); - init_persister(ledger_shared.clone()); Ok((ledger_shared, last_slot)) } diff --git a/magicblock-chainlink/src/lib.rs b/magicblock-chainlink/src/lib.rs index 1d467fba8..7a022f6fa 100644 --- a/magicblock-chainlink/src/lib.rs +++ b/magicblock-chainlink/src/lib.rs @@ -5,8 +5,6 @@ pub mod cloner; pub mod remote_account_provider; pub mod submux; -use std::ops::Add; - pub use chainlink::*; pub use magicblock_metrics::metrics::AccountFetchOrigin; mod filters; diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs index c9a123933..580acebf6 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs @@ -1,6 +1,5 @@ use std::collections::{hash_map::Entry, HashMap, VecDeque}; -use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_pubkey::Pubkey; use thiserror::Error; diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index bdb2f3125..3a65534e4 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -161,6 +161,7 @@ impl CacheTaskInfoFetcher { if pubkeys.is_empty() { return Ok(Vec::new()); } + tokio::runtime::Handle::current(); let pda_accounts: Vec = pubkeys .iter() diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index 74c8d374f..41dfea385 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -1,4 +1,4 @@ -use std::{error::Error, fmt}; +use std::error::Error; use solana_program::instruction::InstructionError; diff --git a/programs/magicblock/src/lib.rs b/programs/magicblock/src/lib.rs index 33888f27a..725ec69fd 100644 --- a/programs/magicblock/src/lib.rs +++ b/programs/magicblock/src/lib.rs @@ -1,6 +1,7 @@ mod ephemeral_accounts; pub mod errors; mod magic_context; +pub mod magic_sys; mod mutate_accounts; mod schedule_task; mod schedule_transactions; @@ -12,8 +13,8 @@ pub mod test_utils; mod utils; pub mod validator; +pub use magic_sys::init_magic_sys; pub use magicblock_magic_program_api::*; -pub use mutate_accounts::*; pub use schedule_transactions::{ process_scheduled_commit_sent, register_scheduled_commit_sent, transaction_scheduler::TransactionScheduler, SentCommit, diff --git a/programs/magicblock/src/magic_scheduled_base_intent.rs b/programs/magicblock/src/magic_scheduled_base_intent.rs index f5e3921fd..f2f234b03 100644 --- a/programs/magicblock/src/magic_scheduled_base_intent.rs +++ b/programs/magicblock/src/magic_scheduled_base_intent.rs @@ -1,10 +1,8 @@ -use std::{cell::RefCell, collections::HashSet}; +use std::collections::HashSet; use magicblock_core::{ intent::{CommittedAccount, CommittedAccountRef}, - token_programs::{ - try_remap_ata_to_eata, EATA_PROGRAM_ID, TOKEN_PROGRAM_ID, - }, + token_programs::{EATA_PROGRAM_ID, TOKEN_PROGRAM_ID}, Slot, }; use magicblock_magic_program_api::args::{ @@ -13,7 +11,7 @@ use magicblock_magic_program_api::args::{ UndelegateTypeArgs, }; use serde::{Deserialize, Serialize}; -use solana_account::{Account, AccountSharedData, ReadableAccount}; +use solana_account::ReadableAccount; use solana_hash::Hash; use solana_log_collector::ic_msg; use solana_program_runtime::{ diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs new file mode 100644 index 000000000..25cf0b36b --- /dev/null +++ b/programs/magicblock/src/magic_sys.rs @@ -0,0 +1,54 @@ +use std::{ + error::Error, + sync::{Arc, RwLock}, +}; + +use lazy_static::lazy_static; +use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; +use solana_instruction::error::InstructionError; + +lazy_static! { + static ref MAGIC_SYS: RwLock>> = RwLock::new(None); +} + +const MAGIC_SYS_POISONED_MSG: &str = "MAGIC_SYS poisoned"; +const MAGIC_SYS_UNSET_MSG: &str = "MagicSys needs to be set on startup"; + +pub fn init_magic_sys(magic_sys: Arc) { + MAGIC_SYS + .write() + .expect(MAGIC_SYS_POISONED_MSG) + .replace(magic_sys); +} + +pub(crate) fn load_data(id: u64) -> Result>, Box> { + MAGIC_SYS + .read() + .expect(MAGIC_SYS_POISONED_MSG) + .as_ref() + .ok_or(MAGIC_SYS_UNSET_MSG)? + .load(id) +} + +pub(crate) fn persist_data( + id: u64, + data: Vec, +) -> Result<(), Box> { + MAGIC_SYS + .read() + .expect(MAGIC_SYS_POISONED_MSG) + .as_ref() + .ok_or(MAGIC_SYS_UNSET_MSG)? + .persist(id, data) +} + +pub(crate) fn validate_commits( + commits: &[CommittedAccount], +) -> Result<(), InstructionError> { + MAGIC_SYS + .read() + .expect(MAGIC_SYS_POISONED_MSG) + .as_ref() + .ok_or(InstructionError::UninitializedAccount)? + .validate_commits(commits) +} diff --git a/programs/magicblock/src/mutate_accounts/account_mod_data.rs b/programs/magicblock/src/mutate_accounts/account_mod_data.rs index 7a64465f7..2df4cf6ae 100644 --- a/programs/magicblock/src/mutate_accounts/account_mod_data.rs +++ b/programs/magicblock/src/mutate_accounts/account_mod_data.rs @@ -2,16 +2,19 @@ use std::{ collections::HashMap, sync::{ atomic::{AtomicU64, Ordering}, - Arc, Mutex, RwLock, + Mutex, }, }; use lazy_static::lazy_static; -use magicblock_core::traits::MagicSys; use solana_log_collector::ic_msg; use solana_program_runtime::invoke_context::InvokeContext; -use crate::{errors::MagicBlockProgramError, validator}; +use crate::{ + errors::MagicBlockProgramError, + magic_sys::{load_data, persist_data}, + validator, +}; lazy_static! { /// In order to modify large data chunks we cannot include all the data as part of the @@ -20,12 +23,6 @@ lazy_static! { /// processed it resolved that data from the key that we provide in its place. static ref DATA_MODS: Mutex>> = Mutex::default(); - /// In order to support replaying transactions we need to persist the data that is - /// loaded from the [DATA_MODS] - /// During replay the [DATA_MODS] won't have the data for the particular id in which - /// case it is loaded via the persister instead. - static ref PERSISTER: RwLock>> = RwLock::new(None); - static ref DATA_MOD_ID: AtomicU64 = AtomicU64::new(0); static ref MAX_REPLAY_DATA_MOD_ID: Mutex> = Mutex::default(); @@ -75,34 +72,6 @@ pub(super) fn get_data(id: u64) -> Option> { DATA_MODS.lock().expect("DATA_MODS poisoned").remove(&id) } -pub fn init_persister(persister: Arc) { - PERSISTER - .write() - .expect("PERSISTER poisoned") - .replace(persister); -} - -fn load_data(id: u64) -> Result>, Box> { - PERSISTER - .read() - .expect("PERSISTER poisoned") - .as_ref() - .ok_or("AccountModPersister needs to be set on startup")? - .load(id) -} - -fn persist_data( - id: u64, - data: Vec, -) -> Result<(), Box> { - PERSISTER - .read() - .expect("PERSISTER poisoned") - .as_ref() - .ok_or("AccounModPersister needs to be set on startup")? - .persist(id, data) -} - /// The resolved data including an indication about how it was resolved. pub(super) enum ResolvedAccountModData { /// The data was resolved from memory while the validator was processing diff --git a/programs/magicblock/src/mutate_accounts/mod.rs b/programs/magicblock/src/mutate_accounts/mod.rs index eb89b7dda..530cd1986 100644 --- a/programs/magicblock/src/mutate_accounts/mod.rs +++ b/programs/magicblock/src/mutate_accounts/mod.rs @@ -1,5 +1,4 @@ mod account_mod_data; mod process_mutate_accounts; -pub use account_mod_data::init_persister; pub(crate) use account_mod_data::*; pub(crate) use process_mutate_accounts::process_mutate_accounts; diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index fc59ea5d0..f6310c2d7 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::{intent::CommittedAccount, traits::NONCE_LIMIT_ERR}; // no direct token remap helpers needed here; handled in CommittedAccount builder use solana_account::{state_traits::StateMut, ReadableAccount}; use solana_instruction::error::InstructionError; @@ -13,6 +13,7 @@ use crate::{ validate_commit_schedule_permissions, CommitAndUndelegate, CommitType, MagicBaseIntent, ScheduledIntentBundle, UndelegateType, }, + magic_sys::validate_commits, schedule_transactions, utils::{ account_actions::mark_account_as_undelegated, @@ -233,6 +234,15 @@ pub(crate) fn process_schedule_commit( } } + validate_commits(&committed_accounts).inspect_err(|err| { + if err == &InstructionError::Custom(NONCE_LIMIT_ERR) { + ic_msg!( + invoke_context, + "ScheduleCommit ERR: commit nonce limit exceeded for one or more accounts" + ); + } + })?; + // NOTE: this is only protected by all the above checks however if the // instruction fails for other reasons detected afterward then the commit // stays scheduled diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs index 5b846c984..e84a95e5b 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; +use magicblock_core::traits::NONCE_LIMIT_ERR; use magicblock_magic_program_api::args::MagicIntentBundleArgs; use solana_account::state_traits::StateMut; use solana_instruction::error::InstructionError; @@ -12,6 +13,7 @@ use crate::{ magic_scheduled_base_intent::{ CommitType, ConstructionContext, ScheduledIntentBundle, }, + magic_sys::validate_commits, schedule_transactions::check_magic_context_id, utils::{ account_actions::mark_account_as_undelegated, @@ -157,6 +159,16 @@ pub(crate) fn process_schedule_intent_bundle( ); } + validate_commits(&scheduled_intent.get_all_committed_accounts()) + .inspect_err(|err| { + if err == &InstructionError::Custom(NONCE_LIMIT_ERR) { + ic_msg!( + invoke_context, + "ScheduleCommit ERR: commit nonce limit exceeded for one or more accounts" + ); + } + })?; + let action_sent_signature = scheduled_intent.sent_transaction.signatures[0]; context.add_scheduled_action(scheduled_intent); diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index 2409c2405..408a73e4c 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -38,8 +38,8 @@ pub fn ensure_started_validator(map: &mut HashMap) { vault }); - let stub = Arc::new(PersisterStub::default()); - init_persister(stub); + let stub = Arc::new(MagicSysStub::default()); + init_magic_sys(stub); validator::ensure_started_up(); } @@ -63,11 +63,11 @@ pub fn process_instruction( ) } -pub struct PersisterStub { +pub struct MagicSysStub { id: u64, } -impl Default for PersisterStub { +impl Default for MagicSysStub { fn default() -> Self { static ID: AtomicU64 = AtomicU64::new(0); @@ -77,13 +77,13 @@ impl Default for PersisterStub { } } -impl fmt::Display for PersisterStub { +impl fmt::Display for MagicSysStub { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "PersisterStub({})", self.id) + write!(f, "MagicSysStub({})", self.id) } } -impl MagicSys for PersisterStub { +impl MagicSys for MagicSysStub { fn persist(&self, id: u64, data: Vec) -> Result<(), Box> { debug!("Persisting data for id '{}' with len {}", id, data.len()); Ok(()) @@ -95,9 +95,8 @@ impl MagicSys for PersisterStub { fn validate_commits( &self, - commits: &[CommittedAccount], + _commits: &[CommittedAccount], ) -> Result<(), InstructionError> { - // TODO(edwin) - todo!() + Ok(()) } } From cf4f7dff9704be42b52f9f1355ebe8b9822e5980 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 18:51:15 +0700 Subject: [PATCH 08/19] feat: return map into magic-validator instead of pure validation within MagicSys --- magicblock-api/src/magic_sys_adapter.rs | 28 +++++---------- magicblock-core/src/traits.rs | 11 +++--- programs/magicblock/src/magic_sys.rs | 12 +++++-- .../src/schedule_transactions/mod.rs | 35 ++++++++++++++++++- .../process_schedule_commit.rs | 24 +++++++------ .../process_schedule_intent_bundle.rs | 17 +++------ programs/magicblock/src/test_utils/mod.rs | 9 ++--- 7 files changed, 79 insertions(+), 57 deletions(-) diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index c953b6309..0fd3daa86 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -1,16 +1,12 @@ -use std::{error::Error, sync::Arc}; +use std::{collections::HashMap, error::Error, sync::Arc}; use magicblock_committor_service::{BaseIntentCommittor, CommittorService}; -use magicblock_core::{ - intent::CommittedAccount, - traits::{MagicSys, NONCE_LIMIT_ERR}, -}; +use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use magicblock_ledger::Ledger; use solana_instruction::error::InstructionError; +use solana_pubkey::Pubkey; use tracing::{enabled, error, trace, Level}; -const NONCE_LIMIT: u64 = 400; - #[derive(Clone)] pub struct MagicSysAdapter { handle: tokio::runtime::Handle, @@ -54,10 +50,10 @@ impl MagicSys for MagicSysAdapter { Ok(data) } - fn validate_commits( + fn fetch_current_commit_nonces( &self, commits: &[CommittedAccount], - ) -> Result<(), InstructionError> { + ) -> Result, InstructionError> { let committor_service = if let Some(committor_service) = &self.committor_service { Ok(committor_service) @@ -75,7 +71,8 @@ impl MagicSys for MagicSysAdapter { let receiver = committor_service .fetch_current_commit_nonces(&pubkeys, min_context_slot); - let nonces_map = self.handle.block_on(receiver) + self.handle + .block_on(receiver) .inspect_err(|err| { error!(error = ?err, "Failed to receive nonces from CommittorService") }) @@ -83,15 +80,6 @@ impl MagicSys for MagicSysAdapter { .inspect_err(|err| { error!(error = ?err, "Failed to fetch current commit nonces") }) - .map_err(|_| InstructionError::Custom(Self::FETCH_ERR))?; - - for (pubkey, nonce) in nonces_map { - if nonce >= NONCE_LIMIT { - trace!("Limit of commits exceeded for: {}", pubkey); - return Err(InstructionError::Custom(NONCE_LIMIT_ERR)); - } - } - - Ok(()) + .map_err(|_| InstructionError::Custom(Self::FETCH_ERR)) } } diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index 41dfea385..11a3d3620 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -1,17 +1,16 @@ -use std::error::Error; +use std::{collections::HashMap, error::Error}; use solana_program::instruction::InstructionError; +use solana_pubkey::Pubkey; use crate::intent::CommittedAccount; -pub const NONCE_LIMIT_ERR: u32 = 0xA000_0000; - pub trait MagicSys: Sync + Send + 'static { fn persist(&self, id: u64, data: Vec) -> Result<(), Box>; fn load(&self, id: u64) -> Result>, Box>; - fn validate_commits( + fn fetch_current_commit_nonces( &self, - pubkeys: &[CommittedAccount], - ) -> Result<(), InstructionError>; + commits: &[CommittedAccount], + ) -> Result, InstructionError>; } diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs index 25cf0b36b..ccf6c715c 100644 --- a/programs/magicblock/src/magic_sys.rs +++ b/programs/magicblock/src/magic_sys.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashMap, error::Error, sync::{Arc, RwLock}, }; @@ -6,6 +7,11 @@ use std::{ use lazy_static::lazy_static; use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use solana_instruction::error::InstructionError; +use solana_pubkey::Pubkey; + +pub(crate) const COMMIT_LIMIT: u64 = 400; +pub(crate) const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; +pub(crate) const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; lazy_static! { static ref MAGIC_SYS: RwLock>> = RwLock::new(None); @@ -42,13 +48,13 @@ pub(crate) fn persist_data( .persist(id, data) } -pub(crate) fn validate_commits( +pub(crate) fn fetch_current_commit_nonces( commits: &[CommittedAccount], -) -> Result<(), InstructionError> { +) -> Result, InstructionError> { MAGIC_SYS .read() .expect(MAGIC_SYS_POISONED_MSG) .as_ref() .ok_or(InstructionError::UninitializedAccount)? - .validate_commits(commits) + .fetch_current_commit_nonces(commits) } diff --git a/programs/magicblock/src/schedule_transactions/mod.rs b/programs/magicblock/src/schedule_transactions/mod.rs index 942a5991c..bfe18899f 100644 --- a/programs/magicblock/src/schedule_transactions/mod.rs +++ b/programs/magicblock/src/schedule_transactions/mod.rs @@ -6,6 +6,7 @@ mod process_schedule_intent_bundle; mod process_scheduled_commit_sent; pub(crate) mod transaction_scheduler; +use magicblock_core::intent::CommittedAccount; use magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY; pub(crate) use process_accept_scheduled_commits::*; pub(crate) use process_schedule_commit::*; @@ -17,7 +18,39 @@ use solana_instruction::error::InstructionError; use solana_log_collector::ic_msg; use solana_program_runtime::invoke_context::InvokeContext; -use crate::utils::accounts::get_instruction_pubkey_with_idx; +use crate::{ + magic_sys::{ + fetch_current_commit_nonces, COMMIT_LIMIT, COMMIT_LIMIT_ERR, + MISSING_COMMIT_NONCE_ERR, + }, + utils::accounts::get_instruction_pubkey_with_idx, +}; + +pub(crate) fn check_commit_limits( + commits: &[CommittedAccount], + invoke_context: &InvokeContext, +) -> Result<(), InstructionError> { + let mut nonces = fetch_current_commit_nonces(commits)?; + let mut limit_exceeded = false; + for account in commits { + let nonce = nonces + .remove(&account.pubkey) + .ok_or(InstructionError::Custom(MISSING_COMMIT_NONCE_ERR))?; + if nonce >= COMMIT_LIMIT { + ic_msg!( + invoke_context, + "ScheduleCommit ERR: commit limit exceeded for account {}, only undelegation is allowed", + account.pubkey + ); + limit_exceeded = true; + } + } + if limit_exceeded { + Err(InstructionError::Custom(COMMIT_LIMIT_ERR)) + } else { + Ok(()) + } +} pub fn check_magic_context_id( invoke_context: &InvokeContext, diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index f6310c2d7..67cb339b0 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; -use magicblock_core::{intent::CommittedAccount, traits::NONCE_LIMIT_ERR}; +use magicblock_core::intent::CommittedAccount; // no direct token remap helpers needed here; handled in CommittedAccount builder use solana_account::{state_traits::StateMut, ReadableAccount}; use solana_instruction::error::InstructionError; @@ -13,8 +13,7 @@ use crate::{ validate_commit_schedule_permissions, CommitAndUndelegate, CommitType, MagicBaseIntent, ScheduledIntentBundle, UndelegateType, }, - magic_sys::validate_commits, - schedule_transactions, + schedule_transactions::{self, check_commit_limits}, utils::{ account_actions::mark_account_as_undelegated, accounts::{ @@ -234,14 +233,17 @@ pub(crate) fn process_schedule_commit( } } - validate_commits(&committed_accounts).inspect_err(|err| { - if err == &InstructionError::Custom(NONCE_LIMIT_ERR) { - ic_msg!( - invoke_context, - "ScheduleCommit ERR: commit nonce limit exceeded for one or more accounts" - ); - } - })?; + // NOTE: + // We validate commit nonces only for plain commits + // If accounts are undelegated we don't validate + // It may result in failed undelegation due to insufficient balance + // But it is better than progibiting undelegation overall + // In case account is borked user shall reach out us + // 1. Account is topped up by user + // 2. Manual undelegation is triggered by us + if !opts.request_undelegation { + check_commit_limits(&committed_accounts, invoke_context)?; + } // NOTE: this is only protected by all the above checks however if the // instruction fails for other reasons detected afterward then the commit diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs index e84a95e5b..f2edd342c 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs @@ -1,6 +1,5 @@ use std::collections::HashSet; -use magicblock_core::traits::NONCE_LIMIT_ERR; use magicblock_magic_program_api::args::MagicIntentBundleArgs; use solana_account::state_traits::StateMut; use solana_instruction::error::InstructionError; @@ -13,8 +12,7 @@ use crate::{ magic_scheduled_base_intent::{ CommitType, ConstructionContext, ScheduledIntentBundle, }, - magic_sys::validate_commits, - schedule_transactions::check_magic_context_id, + schedule_transactions::{check_commit_limits, check_magic_context_id}, utils::{ account_actions::mark_account_as_undelegated, accounts::{ @@ -159,15 +157,10 @@ pub(crate) fn process_schedule_intent_bundle( ); } - validate_commits(&scheduled_intent.get_all_committed_accounts()) - .inspect_err(|err| { - if err == &InstructionError::Custom(NONCE_LIMIT_ERR) { - ic_msg!( - invoke_context, - "ScheduleCommit ERR: commit nonce limit exceeded for one or more accounts" - ); - } - })?; + if let Some(commit_accounts) = scheduled_intent.get_commit_intent_accounts() + { + check_commit_limits(commit_accounts, invoke_context)?; + } let action_sent_signature = scheduled_intent.sent_transaction.signatures[0]; diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index 408a73e4c..ad7480132 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -14,6 +14,7 @@ use solana_account::AccountSharedData; use solana_instruction::{error::InstructionError, AccountMeta}; use solana_log_collector::log::debug; use solana_program_runtime::invoke_context::mock_process_instruction; +use solana_pubkey::Pubkey; use solana_sdk_ids::system_program; use self::magicblock_processor::Entrypoint; @@ -93,10 +94,10 @@ impl MagicSys for MagicSysStub { Err("Loading from ledger not supported in tests".into()) } - fn validate_commits( + fn fetch_current_commit_nonces( &self, - _commits: &[CommittedAccount], - ) -> Result<(), InstructionError> { - Ok(()) + commits: &[CommittedAccount], + ) -> Result, InstructionError> { + Ok(commits.iter().map(|c| (c.pubkey, 0)).collect()) } } From 027039195ec0b98eb98063351dd5bc412224b8ae Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 19:32:42 +0700 Subject: [PATCH 09/19] fix: lint --- magicblock-chainlink/src/lib.rs | 135 ------------------ .../src/stubs/changeset_committor_stub.rs | 16 +++ .../process_mutate_accounts.rs | 16 +-- .../process_schedule_commit_tests.rs | 96 ++++++++++++- .../process_scheduled_commit_sent.rs | 8 +- programs/magicblock/src/test_utils/mod.rs | 20 ++- 6 files changed, 139 insertions(+), 152 deletions(-) diff --git a/magicblock-chainlink/src/lib.rs b/magicblock-chainlink/src/lib.rs index 7a022f6fa..1be32d4f9 100644 --- a/magicblock-chainlink/src/lib.rs +++ b/magicblock-chainlink/src/lib.rs @@ -11,138 +11,3 @@ mod filters; #[cfg(any(test, feature = "dev-context"))] pub mod testing; - -#[tokio::test] -async fn test_trtr() { - // The issue: - // 1. We need to clone inner ArcMutex to hold locks - // We can't have reference as that would mean blocking outer Mutex for func duration - // 2. When we clone we have a problem - the entry may get evicted from LruCache - // That would lead to race conditions: - // New thread comes in, sees no entry for pubkey, creates new ArcMutex instance - // Other thread will overwrite it - - // Goal: we still need to cleanup space ut without race conditions - - // That means - we can't use LruCache directly - // We need to use HashMap and clean post factum - - // When can we insert and clean? - // Cleaning: - // We clean after execution. - - // Alg for fetch next - // 1. lock outer mutex - // 2. get inner mutexes - // a. exists - // i. value == u64::MAX, clone move to_request - // ii. value != u64::MAX, clone it into ready group - // b. absent - insert in HashMap + clone into to_request - // 3. update LruCache. // Here some locked keys could be evicted. TODO: update or maybe not/ - // 4. drop outer mutex - // 5. lock.await inner mutexes - // 6. request data for uninitialized ones - // 7. On failure - // a. lock outer - // b. TODO: ensure safety for uninitialized, check if locked - // 8. On success - lock outer - // 9. Update locked inner values - // 10. compile result - // 11. get pubkey in LRU - // a. exists - continue - // b. !exists - push - // i. None returned - continue - // ii. Some(el) - if not in use remove from hashmap - - // Problem wuth 2.b - // Once inserted in map it could be accessed, even tho it is uninitialized - // That would mean that we can't just remove it on failure - // If it is cloned by someone - we need to leave it alone - - // The scenario we're trying to cover - requests exceed capacity - // Key getting before RPC finished - means that so many values pushed that our key got evicted - // We could insert it back into LRU or clean it upo - // We need cleanup logic. The last active function has to cleanup - // If it isn't in Lru & key NotInUse - cleanup - // If it isn't in Lru & key InUse - keep, do nothing it will be cleaned up by occupying party - // !!!WITH THIS we don't need to care of evicted keys on first outer lock, - // If it is in use the working thread will cleanup itself - - // Problem: - // If we don't insert in the beggining in LRU, then if key absent in the end - // We can't know if it was evicted or not - // What is cleanup logic in that case? - // If we insert at the end and Some key evicted, this key is in use - // If we don't dispose of it the other thread won't be able - - // OPTION 1 - // Handle LRU at the END get pubkey in LRU - // a. exists - continue, no need to cleanup - // b. !exists - push - // i. None returned - continue, no need to cleanup - // ii. Evicted(el) && NotInUse - remove from hashmap - // iii. Evicted(el) && InUse - continue?? - // Evicted key at the end has to be cleaned up by someone - // if ii - excess dealt with right away - // if iii - will be dealt with by some running instance, since it will be inserted in LRU at some point - - // OPTION 2 - // Handle LRU at the BEGGINING - // LRU.get - // a. exists - continue, no need to cleanup - // b. !exists - push - // i. None returned - continue, no need to cleanup - // ii. Evicted(el) && NotInUse - remove from hashmap - // iii. Evicted(el) && InUse - continue - // What to do at the END? - // LRU.peek() - // a. exists - continue, no need to cleanup - // b. !exists - // i. InUse - continue - // ii. NotInUse - cleanup - - // We need to define how LRU works - // I come and place request - // Not to repeat work someone may cache it so If i come again the could just give cached request - // But while they make my order many other people came and asked other things - - use std::{ - num::NonZeroUsize, - sync::{Arc, Mutex}, - }; - - use lru::LruCache; - use tokio::sync::{Mutex as TMutex, MutexGuard, OwnedMutexGuard}; - - struct Locker<'a> { - val: &'a i32, - m: Arc>, - }; - - impl<'a> Locker<'a> { - pub fn new(val: &'a i32, m: Arc>) -> Self { - Self { val, m } - } - - pub async fn lock<'s>(&'s self) -> MutexGuard<'s, u64> { - self.m.lock().await - } - } - - impl<'a> Drop for Locker<'a> { - fn drop(&mut self) { - println!("lpol"); - } - } - - let val = 1; - let m = Arc::new(TMutex::new(10)); - let locker = Locker::new(&val, m); - let mut asd = locker.lock().await; - *asd = 1; - assert_eq!(*asd, 1); - drop(asd); - drop(locker); - - println!("{}", val); -} diff --git a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs index f12cb7806..1e1136ef8 100644 --- a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs +++ b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs @@ -190,6 +190,22 @@ impl BaseIntentCommittor for ChangesetCommittorStub { 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(); } diff --git a/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs b/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs index 17f29ccb7..3b93d7b2d 100644 --- a/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs +++ b/programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs @@ -366,7 +366,7 @@ mod tests { map.insert(mod_key, AccountSharedData::new(100, 0, &mod_key)); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let modification = AccountModification { pubkey: mod_key, @@ -450,7 +450,7 @@ mod tests { map.insert(mod_key2, AccountSharedData::new(200, 0, &mod_key2)); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![ @@ -545,7 +545,7 @@ mod tests { map.insert(mod_key, delegated_account); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![AccountModification { @@ -588,7 +588,7 @@ mod tests { map.insert(mod_key, undelegating_account); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![AccountModification { @@ -656,7 +656,7 @@ mod tests { map.insert(mod_key4, AccountSharedData::new(400, 0, &mod_key4)); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![ @@ -796,7 +796,7 @@ mod tests { map.insert(mod_key, AccountSharedData::new(100, 0, &mod_key)); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![AccountModification { @@ -840,7 +840,7 @@ mod tests { map.insert(mod_key, account); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![AccountModification { @@ -881,7 +881,7 @@ mod tests { map.insert(mod_key, account); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::modify_accounts_instruction( vec![AccountModification { diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs index 128e777b8..6b23e9eea 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs @@ -18,6 +18,7 @@ use solana_signer::Signer; use crate::{ magic_context::MagicContext, magic_scheduled_base_intent::ScheduledIntentBundle, + magic_sys::COMMIT_LIMIT, schedule_transactions::transaction_scheduler::TransactionScheduler, test_utils::{ensure_started_validator, process_instruction}, utils::DELEGATION_PROGRAM_ID, @@ -63,7 +64,7 @@ fn prepare_transaction_with_single_committee( map.insert(committee, committee_account); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let transaction_accounts: Vec<(Pubkey, AccountSharedData)> = vec![( clock::id(), @@ -122,7 +123,7 @@ fn prepare_transaction_with_three_committees( } map }; - ensure_started_validator(&mut accounts_data); + ensure_started_validator(&mut accounts_data, None); let transaction_accounts: Vec<(Pubkey, AccountSharedData)> = vec![( clock::id(), @@ -255,6 +256,8 @@ mod tests { // ---------- Helpers for ATA/eATA remapping tests ---------- // Use shared SPL/ATA/eATA constants and helpers // Reuse test helper to create proper SPL ATA account data + use std::sync::Mutex; + use magicblock_chainlink::testing::eatas::create_ata_account; use magicblock_core::token_programs::{derive_ata, derive_eata}; use solana_seed_derivable::SeedDerivable; @@ -263,6 +266,10 @@ mod tests { use super::*; use crate::utils::instruction_utils::InstructionUtils; + // Prevents race conditions between parallel tests that share the global + // static MagicSys stub. + static TEST_LOCK: Mutex<()> = Mutex::new(()); + fn make_delegated_spl_ata_account( owner: &Pubkey, mint: &Pubkey, @@ -276,6 +283,7 @@ mod tests { #[test] fn test_schedule_commit_single_account_success() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_single_account_success") .unwrap(); @@ -361,6 +369,7 @@ mod tests { #[test] fn test_schedule_commit_single_account_and_request_undelegate_success() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"single_account_with_undelegate_success") .unwrap(); @@ -447,6 +456,7 @@ mod tests { #[test] fn test_schedule_commit_remaps_delegated_ata_to_eata() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_remap_ata_to_eata").unwrap(); @@ -528,6 +538,7 @@ mod tests { #[test] fn test_schedule_commit_and_undelegate_remaps_delegated_ata_to_eata() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_undelegate_remap_ata_eata") @@ -612,6 +623,7 @@ mod tests { #[test] fn test_schedule_commit_three_accounts_success() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_three_accounts_success") @@ -724,6 +736,7 @@ mod tests { #[test] fn test_schedule_commit_three_accounts_and_request_undelegate_success() { + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed( b"three_accounts_and_request_undelegate_success", ) @@ -877,6 +890,7 @@ mod tests { #[test] fn test_schedule_commit_no_pdas_provided_to_ix() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_no_pdas_provided_to_ix") @@ -912,6 +926,7 @@ mod tests { #[test] fn test_schedule_commit_undelegate_with_readonly() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_undelegate_with_readonly") @@ -955,6 +970,7 @@ mod tests { #[test] fn test_schedule_commit_with_non_delegated_account() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_with_non_delegated_account") @@ -995,6 +1011,7 @@ mod tests { fn test_schedule_commit_three_accounts_second_not_owned_by_program_and_not_signer( ) { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"three_accounts_last_not_owned_by_program") @@ -1042,6 +1059,7 @@ mod tests { #[test] fn test_schedule_commit_with_confined_account() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_with_confined_account") @@ -1082,9 +1100,83 @@ mod tests { ); } + #[test] + fn test_schedule_commit_fails_when_commit_limit_exceeded() { + init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + + let payer = + Keypair::from_seed(b"schedule_commit_limit_exceeded____").unwrap(); + let program = Pubkey::new_unique(); + let committee = Pubkey::new_unique(); + + let (mut account_data, mut transaction_accounts) = + prepare_transaction_with_single_committee( + &payer, program, committee, + ); + + // Override stub to return nonce at the commit limit + ensure_started_validator(&mut account_data, Some(COMMIT_LIMIT)); + + let ix = InstructionUtils::schedule_commit_instruction( + &payer.pubkey(), + vec![committee], + ); + extend_transaction_accounts_from_ix( + &ix, + &mut account_data, + &mut transaction_accounts, + ); + + process_instruction( + ix.data.as_slice(), + transaction_accounts, + ix.accounts, + Err(InstructionError::Custom(crate::magic_sys::COMMIT_LIMIT_ERR)), + ); + } + + #[test] + fn test_schedule_commit_and_undelegate_succeeds_when_commit_limit_exceeded() + { + init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + + let payer = + Keypair::from_seed(b"undelegate_succeeds_limit_exceeded").unwrap(); + let program = Pubkey::new_unique(); + let committee = Pubkey::new_unique(); + + let (mut account_data, mut transaction_accounts) = + prepare_transaction_with_single_committee( + &payer, program, committee, + ); + + // Override stub to return nonce at the commit limit + ensure_started_validator(&mut account_data, Some(COMMIT_LIMIT)); + + let ix = InstructionUtils::schedule_commit_and_undelegate_instruction( + &payer.pubkey(), + vec![committee], + ); + extend_transaction_accounts_from_ix( + &ix, + &mut account_data, + &mut transaction_accounts, + ); + + process_instruction( + ix.data.as_slice(), + transaction_accounts, + ix.accounts, + Ok(()), + ); + } + #[test] fn test_schedule_commit_three_accounts_one_confined() { init_logger!(); + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"three_accounts_one_confined_______").unwrap(); diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 4b886930f..14053bf6b 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -305,7 +305,7 @@ mod tests { let mut account_data = HashMap::new(); - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let mut ix = InstructionUtils::scheduled_commit_sent_instruction( &crate::id(), @@ -342,7 +342,7 @@ mod tests { ); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::scheduled_commit_sent_instruction( &crate::id(), @@ -377,7 +377,7 @@ mod tests { ); map }; - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::scheduled_commit_sent_instruction( &fake_program.pubkey(), @@ -406,7 +406,7 @@ mod tests { let mut account_data = HashMap::new(); - ensure_started_validator(&mut account_data); + ensure_started_validator(&mut account_data, None); let ix = InstructionUtils::scheduled_commit_sent_instruction( &crate::id(), diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index ad7480132..0bf1b5a55 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -25,7 +25,10 @@ pub const AUTHORITY_BALANCE: u64 = u64::MAX / 2; pub const COUNTER_PROGRAM_ID: Pubkey = Pubkey::from_str_const("2jQZbSfAfqT5nZHGrLpDG2vXuEGtTgZYnNy7AZEjMCYz"); -pub fn ensure_started_validator(map: &mut HashMap) { +pub fn ensure_started_validator( + map: &mut HashMap, + nonce: Option, +) { validator::generate_validator_authority_if_needed(); let validator_authority_id = validator::validator_authority_id(); map.entry(validator_authority_id).or_insert_with(|| { @@ -39,7 +42,7 @@ pub fn ensure_started_validator(map: &mut HashMap) { vault }); - let stub = Arc::new(MagicSysStub::default()); + let stub = Arc::new(MagicSysStub::with_nonce(nonce.unwrap_or(0))); init_magic_sys(stub); validator::ensure_started_up(); @@ -66,6 +69,7 @@ pub fn process_instruction( pub struct MagicSysStub { id: u64, + nonce: u64, } impl Default for MagicSysStub { @@ -74,6 +78,16 @@ impl Default for MagicSysStub { Self { id: ID.fetch_add(1, Ordering::Relaxed), + nonce: 0, + } + } +} + +impl MagicSysStub { + pub fn with_nonce(nonce: u64) -> Self { + Self { + nonce, + ..Self::default() } } } @@ -98,6 +112,6 @@ impl MagicSys for MagicSysStub { &self, commits: &[CommittedAccount], ) -> Result, InstructionError> { - Ok(commits.iter().map(|c| (c.pubkey, 0)).collect()) + Ok(commits.iter().map(|c| (c.pubkey, self.nonce)).collect()) } } From 78e29702626cbb80c26fba6d799cfa4390d587df Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 20:42:15 +0700 Subject: [PATCH 10/19] fix: compilation --- programs/magicblock/src/magic_sys.rs | 4 +- test-integration/Cargo.lock | 2 + .../schedulecommit/test-scenarios/Cargo.toml | 1 + .../test-scenarios/tests/03_commit_limit.rs | 202 ++++++++++++++++++ .../test-committor-service/Cargo.toml | 1 + .../test-committor-service/tests/common.rs | 10 +- .../tests/test_intent_executor.rs | 58 +++-- .../tests/test_ix_commit_local.rs | 6 +- test-integration/test-runner/bin/run_tests.rs | 3 +- .../test-tools/src/toml_to_args.rs | 12 +- test-integration/test-tools/src/validator.rs | 17 +- 11 files changed, 265 insertions(+), 51 deletions(-) create mode 100644 test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs index ccf6c715c..7d281cd2c 100644 --- a/programs/magicblock/src/magic_sys.rs +++ b/programs/magicblock/src/magic_sys.rs @@ -9,8 +9,8 @@ use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; -pub(crate) const COMMIT_LIMIT: u64 = 400; -pub(crate) const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; +pub const COMMIT_LIMIT: u64 = 400; +pub const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; pub(crate) const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; lazy_static! { diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index d3c4e8aed..5fdd6ed02 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -5801,6 +5801,7 @@ dependencies = [ "futures", "magicblock-committor-program", "magicblock-committor-service", + "magicblock-core", "magicblock-delegation-program 1.1.3 (git+https://github.com/magicblock-labs/delegation-program.git?rev=2cb491032f372)", "magicblock-program", "magicblock-rpc-client", @@ -5827,6 +5828,7 @@ dependencies = [ "integration-test-tools", "magicblock-core", "magicblock-magic-program-api 0.8.1", + "magicblock-program", "program-schedulecommit", "rand 0.8.5", "schedulecommit-client", diff --git a/test-integration/schedulecommit/test-scenarios/Cargo.toml b/test-integration/schedulecommit/test-scenarios/Cargo.toml index f1913660d..ba385ac5b 100644 --- a/test-integration/schedulecommit/test-scenarios/Cargo.toml +++ b/test-integration/schedulecommit/test-scenarios/Cargo.toml @@ -10,6 +10,7 @@ tracing = { workspace = true } program-schedulecommit = { workspace = true, features = ["no-entrypoint"] } schedulecommit-client = { workspace = true } magicblock-core = { workspace = true } +magicblock-program = { workspace = true } magicblock_magic_program_api = { workspace = true } solana-program = { workspace = true } solana-rpc-client = { workspace = true } diff --git a/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs new file mode 100644 index 000000000..527d218ae --- /dev/null +++ b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs @@ -0,0 +1,202 @@ +use std::sync::OnceLock; + +use integration_test_tools::run_test; +use magicblock_program::magic_sys::{COMMIT_LIMIT, COMMIT_LIMIT_ERR}; +use program_schedulecommit::api::{ + schedule_commit_and_undelegate_cpi_instruction, + schedule_commit_cpi_instruction, UserSeeds, +}; +use schedulecommit_client::{ + verify, ScheduleCommitTestContext, ScheduleCommitTestContextFields, +}; +use solana_rpc_client::rpc_client::SerializableTransaction; +use solana_rpc_client_api::config::RpcSendTransactionConfig; +use solana_sdk::{ + instruction::InstructionError, signer::Signer, transaction::Transaction, +}; +use test_kit::init_logger; +use tracing::*; +use utils::{ + assert_is_instruction_error, + assert_one_committee_account_was_undelegated_on_chain, + assert_one_committee_was_committed, extract_transaction_error, + get_context_with_delegated_committees, +}; + +mod utils; + +// --------------------------------------------------------------------------- +// Shared prepared context +// +// One context holding NUM_TESTS committees. All accounts are committed +// COMMIT_LIMIT times together (one tx per round, all PDAs in each tx) so +// they sit at the boundary of failure. Each test then uses its own committee +// slot independently. +// +// OnceLock ensures preparation runs exactly once regardless of which test +// triggers it first; the other test blocks until the pool is ready. +// --------------------------------------------------------------------------- + +const NUM_TESTS: usize = 2; +const IDX_COMMIT_FAILS: usize = 0; +const IDX_UNDELEGATE_SUCCEEDS: usize = 1; + +static PREPARED: OnceLock = OnceLock::new(); + +fn get_prepared() -> &'static ScheduleCommitTestContext { + PREPARED.get_or_init(|| { + let ctx = get_context_with_delegated_committees( + NUM_TESTS, + UserSeeds::MagicScheduleCommit, + ); + + let ScheduleCommitTestContextFields { + payer_ephem: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + + let players: Vec<_> = + committees.iter().map(|(p, _)| p.pubkey()).collect(); + let pdas: Vec<_> = committees.iter().map(|(_, pda)| *pda).collect(); + + for n in 0..COMMIT_LIMIT { + let ix = schedule_commit_cpi_instruction( + payer.pubkey(), + magicblock_magic_program_api::id(), + magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY, + &players, + &pdas, + ); + let blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + blockhash, + ); + let sig = *tx.get_signature(); + ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ) + .unwrap_or_else(|e| { + panic!( + "prepare commit {}/{} failed: {:?}", + n + 1, + COMMIT_LIMIT, + e + ) + }); + info!("prepare commit {}/{}: {}", n + 1, COMMIT_LIMIT, sig); + verify::fetch_and_verify_commit_result_from_logs(&ctx, sig); + } + + ctx + }) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[test] +fn test_schedule_commit_fails_at_commit_limit() { + run_test!({ + let ctx = get_prepared(); + let ScheduleCommitTestContextFields { + payer_ephem: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + + let committee = &committees[IDX_COMMIT_FAILS]; + let ix = schedule_commit_cpi_instruction( + payer.pubkey(), + magicblock_magic_program_api::id(), + magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY, + &[committee.0.pubkey()], + &[committee.1], + ); + let blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + blockhash, + ); + + let res = ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ); + + let (tx_result_err, tx_err) = extract_transaction_error(res); + assert_is_instruction_error( + tx_err.unwrap(), + &tx_result_err, + InstructionError::Custom(COMMIT_LIMIT_ERR), + ); + }); +} + +#[test] +fn test_schedule_commit_and_undelegate_succeeds_at_commit_limit() { + run_test!({ + let ctx = get_prepared(); + let ScheduleCommitTestContextFields { + payer_ephem: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + + let committee = &committees[IDX_UNDELEGATE_SUCCEEDS]; + let ix = schedule_commit_and_undelegate_cpi_instruction( + payer.pubkey(), + magicblock_magic_program_api::id(), + magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY, + &[committee.0.pubkey()], + &[committee.1], + ); + let blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + blockhash, + ); + + let sig = *tx.get_signature(); + ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ) + .unwrap_or_else(|e| panic!("undelegate at limit failed: {:?}", e)); + + // verify via logs using a single-committee context view + let res = verify::fetch_and_verify_commit_result_from_logs(ctx, sig); + assert_one_committee_was_committed(ctx, &res, true); + assert_one_committee_account_was_undelegated_on_chain(ctx); + }); +} diff --git a/test-integration/test-committor-service/Cargo.toml b/test-integration/test-committor-service/Cargo.toml index 4d6af1f58..e61a5c1d5 100644 --- a/test-integration/test-committor-service/Cargo.toml +++ b/test-integration/test-committor-service/Cargo.toml @@ -8,6 +8,7 @@ async-trait = { workspace = true } borsh = { workspace = true } tracing = { workspace = true } futures = { workspace = true } +magicblock-core = { workspace = true } magicblock-committor-program = { workspace = true, features = [ "no-entrypoint", ] } diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index 71b45e604..31f5c2de5 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -131,6 +131,14 @@ impl TaskInfoFetcher for MockTaskInfoFetcher { Ok(pubkeys.iter().map(|pubkey| (*pubkey, 0)).collect()) } + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + Ok(pubkeys.iter().map(|pubkey| (*pubkey, 0)).collect()) + } + async fn fetch_rent_reimbursements( &self, pubkeys: &[Pubkey], @@ -139,7 +147,7 @@ impl TaskInfoFetcher for MockTaskInfoFetcher { Ok(pubkeys.to_vec()) } - fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { + async fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { None } 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 8c15f17e8..33706c627 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -29,11 +29,12 @@ use magicblock_committor_service::{ }, transaction_preparator::TransactionPreparatorImpl, }; +use magicblock_core::intent::CommittedAccount; use magicblock_program::{ args::ShortAccountMeta, magic_scheduled_base_intent::{ - BaseAction, CommitAndUndelegate, CommitType, - MagicBaseIntent, ProgramArgs, ScheduledIntentBundle, UndelegateType, + BaseAction, CommitAndUndelegate, CommitType, MagicBaseIntent, + ProgramArgs, ScheduledIntentBundle, UndelegateType, }, validator::validator_authority_id, }; @@ -55,7 +56,7 @@ use solana_sdk::{ signature::{Keypair, Signer}, transaction::{Transaction, TransactionError}, }; -use magicblock_core::intent::CommittedAccount; + use crate::{ common::TestFixture, utils::{ @@ -475,15 +476,14 @@ async fn test_commit_id_error_recovery() { // Cleanup succeeds assert!(intent_executor.cleanup().await.is_ok()); - let commit_ids_by_pk: HashMap<_, _> = [&committed_account] - .iter() - .map(|el| { - ( - el.pubkey, - task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), - ) - }) - .collect(); + let mut commit_ids_by_pk = HashMap::new(); + for el in [&committed_account].iter() { + let nonce = task_info_fetcher + .peek_commit_nonce(&el.pubkey) + .await + .unwrap(); + commit_ids_by_pk.insert(el.pubkey, nonce); + } verify( &fixture.table_mania, @@ -772,15 +772,14 @@ async fn test_cpi_limits_error_recovery() { // Cleanup after intent assert!(intent_executor.cleanup().await.is_ok()); - let commit_ids_by_pk: HashMap<_, _> = committed_accounts - .iter() - .map(|el| { - ( - el.pubkey, - task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), - ) - }) - .collect(); + let mut commit_ids_by_pk = HashMap::new(); + for el in committed_accounts.iter() { + let nonce = task_info_fetcher + .peek_commit_nonce(&el.pubkey) + .await + .unwrap(); + commit_ids_by_pk.insert(el.pubkey, nonce); + } verify( &fixture.table_mania, @@ -907,15 +906,14 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { // Cleanup after intent assert!(intent_executor.cleanup().await.is_ok()); - let commit_ids_by_pk: HashMap<_, _> = committed_accounts - .iter() - .map(|el| { - ( - el.pubkey, - task_info_fetcher.peek_commit_nonce(&el.pubkey).unwrap(), - ) - }) - .collect(); + let mut commit_ids_by_pk = HashMap::new(); + for el in committed_accounts.iter() { + let nonce = task_info_fetcher + .peek_commit_nonce(&el.pubkey) + .await + .unwrap(); + commit_ids_by_pk.insert(el.pubkey, nonce); + } verify( &fixture.table_mania, fixture.rpc_client.get_inner(), 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 a3d2190eb..f25bab459 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 @@ -12,9 +12,10 @@ use magicblock_committor_service::{ service_ext::{BaseIntentCommittorExt, CommittorServiceExt}, BaseIntentCommittor, CommittorService, ComputeBudgetConfig, }; +use magicblock_core::intent::CommittedAccount; use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicBaseIntent, - MagicIntentBundle, ScheduledIntentBundle, UndelegateType, + CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, + ScheduledIntentBundle, UndelegateType, }; use magicblock_rpc_client::MagicblockRpcClient; use program_flexi_counter::state::FlexiCounter; @@ -28,7 +29,6 @@ use solana_sdk::{ use test_kit::init_logger; use tokio::task::JoinSet; use tracing::*; -use magicblock_core::intent::CommittedAccount; use utils::transactions::tx_logs_contain; use self::utils::transactions::init_and_delegate_order_book_on_chain; diff --git a/test-integration/test-runner/bin/run_tests.rs b/test-integration/test-runner/bin/run_tests.rs index 8d117337c..06a78c0b8 100644 --- a/test-integration/test-runner/bin/run_tests.rs +++ b/test-integration/test-runner/bin/run_tests.rs @@ -793,7 +793,8 @@ fn run_test( if let Some(test) = config.test { cmd.arg(format!("'{}'", test)); } - let test_threads = std::env::var("CARGO_TEST_THREADS").unwrap_or_else(|_| "1".to_string()); + let test_threads = + std::env::var("CARGO_TEST_THREADS").unwrap_or_else(|_| "1".to_string()); cmd.arg("--") .arg(format!("--test-threads={}", test_threads)) .arg("--nocapture"); diff --git a/test-integration/test-tools/src/toml_to_args.rs b/test-integration/test-tools/src/toml_to_args.rs index 8bb81cb9d..7b9f486f4 100644 --- a/test-integration/test-tools/src/toml_to_args.rs +++ b/test-integration/test-tools/src/toml_to_args.rs @@ -86,19 +86,17 @@ pub fn config_to_args( args.push(path.to_str().unwrap().to_string()); } Err(e) => { - let abs_config_dir = fs::canonicalize(config_dir).unwrap_or(config_dir.to_path_buf()); + let abs_config_dir = fs::canonicalize(config_dir) + .unwrap_or(config_dir.to_path_buf()); eprintln!( "Error: Failed to resolve program path.\n\ Config Dir: {:?}\n\ Relative Path: {:?}\n\ Resolution Attempt: {:?}\n\ OS Error: {:?}", - abs_config_dir, - program.path, - full_path_to_resolve, - e + abs_config_dir, program.path, full_path_to_resolve, e ); - + // List directory contents to aid debugging in CI environments if let Some(parent) = full_path_to_resolve.parent() { eprintln!("Directory contents of {:?}:", parent); @@ -112,7 +110,7 @@ pub fn config_to_args( eprintln!(" (Unable to read directory)"); } } - + panic!("Program file not found: {:?}", full_path_to_resolve); } } diff --git a/test-integration/test-tools/src/validator.rs b/test-integration/test-tools/src/validator.rs index c70eff3e3..e77aea3b6 100644 --- a/test-integration/test-tools/src/validator.rs +++ b/test-integration/test-tools/src/validator.rs @@ -39,7 +39,7 @@ pub fn start_magic_block_validator_with_config( let target_dir = std::env::var("CARGO_TARGET_DIR") .map(PathBuf::from) .unwrap_or_else(|_| workspace_dir.join("../target")); - + let validator_bin = target_dir.join("debug/magicblock-validator"); let mut command = if validator_bin.exists() { @@ -48,20 +48,23 @@ pub fn start_magic_block_validator_with_config( cmd.arg(config_path); cmd } else { - eprintln!("Prebuilt validator not found at {:?}, falling back to cargo run", validator_bin); + eprintln!( + "Prebuilt validator not found at {:?}, falling back to cargo run", + validator_bin + ); // Fallback for local development let mut cmd = process::Command::new("cargo"); cmd.arg("run") - .arg("-p") - .arg("magicblock-validator") - .arg("--") - .arg(config_path); + .arg("-p") + .arg("magicblock-validator") + .arg("--") + .arg(config_path); cmd }; let rust_log_style = std::env::var("RUST_LOG_STYLE").unwrap_or(log_suffix.to_string()); - + command .env("RUST_LOG_STYLE", rust_log_style) .env("MBV_VALIDATOR__KEYPAIR", keypair_base58.clone()) From 31761e319217b2f39ab6a482df2fcab6bc435f8f Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 21:54:34 +0700 Subject: [PATCH 11/19] feat: add integration tests --- Cargo.lock | 1 + magicblock-api/Cargo.toml | 1 + magicblock-api/src/magic_sys_adapter.rs | 9 +++++---- programs/magicblock/src/magic_sys.rs | 2 +- test-integration/Cargo.lock | 1 + .../test-scenarios/tests/03_commit_limit.rs | 13 ++++++++----- .../test-scenarios/tests/utils/mod.rs | 19 ++++++++++++++----- 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2893269bf..3bc836ebe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3052,6 +3052,7 @@ dependencies = [ "anyhow", "borsh 1.6.0", "fd-lock", + "futures", "magic-domain-program", "magicblock-account-cloner", "magicblock-accounts", diff --git a/magicblock-api/Cargo.toml b/magicblock-api/Cargo.toml index 1a1c8dab8..f48c457c5 100644 --- a/magicblock-api/Cargo.toml +++ b/magicblock-api/Cargo.toml @@ -11,6 +11,7 @@ edition.workspace = true anyhow = { workspace = true } borsh = "1.5.3" fd-lock = { workspace = true } +futures = { workspace = true } tracing = { workspace = true } magic-domain-program = { workspace = true } diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 0fd3daa86..19220e1e4 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -9,7 +9,6 @@ use tracing::{enabled, error, trace, Level}; #[derive(Clone)] pub struct MagicSysAdapter { - handle: tokio::runtime::Handle, ledger: Arc, committor_service: Option>, } @@ -24,7 +23,6 @@ impl MagicSysAdapter { committor_service: Option>, ) -> Self { Self { - handle: tokio::runtime::Handle::current(), ledger, committor_service, } @@ -71,8 +69,11 @@ impl MagicSys for MagicSysAdapter { let receiver = committor_service .fetch_current_commit_nonces(&pubkeys, min_context_slot); - self.handle - .block_on(receiver) + // Tx execution is sync and runs on a tokio worker thread. handle.block_on + // would panic (nested runtime). futures::executor::block_on parks this + // thread independently of tokio — safe because the thread is already + // committed to this tx until execution completes. + futures::executor::block_on(receiver) .inspect_err(|err| { error!(error = ?err, "Failed to receive nonces from CommittorService") }) diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs index 7d281cd2c..03856c2ee 100644 --- a/programs/magicblock/src/magic_sys.rs +++ b/programs/magicblock/src/magic_sys.rs @@ -9,7 +9,7 @@ use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; -pub const COMMIT_LIMIT: u64 = 400; +pub const COMMIT_LIMIT: u64 = 15; pub const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; pub(crate) const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index 5fdd6ed02..8ba82aa06 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3396,6 +3396,7 @@ dependencies = [ "anyhow", "borsh 1.6.0", "fd-lock", + "futures", "magic-domain-program", "magicblock-account-cloner", "magicblock-accounts", diff --git a/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs index 527d218ae..c501adfc4 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs @@ -17,9 +17,8 @@ use solana_sdk::{ use test_kit::init_logger; use tracing::*; use utils::{ - assert_is_instruction_error, - assert_one_committee_account_was_undelegated_on_chain, - assert_one_committee_was_committed, extract_transaction_error, + assert_account_was_undelegated_on_chain, assert_committee_was_committed, + assert_is_instruction_error, extract_transaction_error, get_context_with_delegated_committees, }; @@ -196,7 +195,11 @@ fn test_schedule_commit_and_undelegate_succeeds_at_commit_limit() { // verify via logs using a single-committee context view let res = verify::fetch_and_verify_commit_result_from_logs(ctx, sig); - assert_one_committee_was_committed(ctx, &res, true); - assert_one_committee_account_was_undelegated_on_chain(ctx); + assert_committee_was_committed(committee.1, &res, true); + assert_account_was_undelegated_on_chain( + ctx, + committee.1, + program_schedulecommit::id(), + ); }); } diff --git a/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs b/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs index 342f94ed2..ca63ce2b3 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs @@ -37,16 +37,14 @@ pub fn get_context_with_delegated_committees( // ----------------- // Asserts // ----------------- -#[allow(dead_code)] // used in 02_commit_and_undelegate.rs -pub fn assert_one_committee_was_committed( - ctx: &ScheduleCommitTestContext, +#[allow(dead_code)] +pub fn assert_committee_was_committed( + pda: Pubkey, res: &ScheduledCommitResult, is_single_stage: bool, ) where T: std::fmt::Debug + borsh::BorshDeserialize + PartialEq + Eq, { - let pda = ctx.committees[0].1; - assert_eq!(res.included.len(), 1, "includes 1 pda"); assert_eq!(res.excluded.len(), 0, "excludes 0 pdas"); @@ -64,6 +62,17 @@ pub fn assert_one_committee_was_committed( ); } +#[allow(dead_code)] // used in 02_commit_and_undelegate.rs +pub fn assert_one_committee_was_committed( + ctx: &ScheduleCommitTestContext, + res: &ScheduledCommitResult, + is_single_stage: bool, +) where + T: std::fmt::Debug + borsh::BorshDeserialize + PartialEq + Eq, +{ + assert_committee_was_committed(ctx.committees[0].1, res, is_single_stage); +} + #[allow(dead_code)] // used in 02_commit_and_undelegate.rs pub fn assert_two_committees_were_committed( ctx: &ScheduleCommitTestContext, From 251c8ed52c570d8362b0713ff6dcf35b8c3e1229 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 23:09:20 +0700 Subject: [PATCH 12/19] feat: added docs --- .../src/intent_executor/task_info_fetcher.rs | 33 +++++++++++-------- magicblock-core/src/traits.rs | 2 ++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 3a65534e4..bdf4fa80f 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -71,13 +71,19 @@ pub enum ResetType<'a> { Specific(&'a [Pubkey]), } +/// Per-account async mutex protecting the cached nonce value. type NonceLock = Arc>; +/// Split-map cache: `active` is the live LRU window; `retiring` holds locks +/// evicted from `active` that are still held by in-flight requests. struct CacheInner { active: LruCache, retiring: HashMap, } +/// RAII guard returned by [`CacheTaskInfoFetcher::acquire_nonce_locks`]. +/// Holds clones of the reserved [`NonceLock`]s and cleans up retiring entries +/// on drop. struct CacheInnerGuard<'a> { inner: &'a Mutex, nonce_locks: Vec<(Pubkey, NonceLock)>, @@ -161,7 +167,6 @@ impl CacheTaskInfoFetcher { if pubkeys.is_empty() { return Ok(Vec::new()); } - tokio::runtime::Handle::current(); let pda_accounts: Vec = pubkeys .iter() @@ -302,7 +307,9 @@ impl CacheTaskInfoFetcher { Ok(accounts) } - fn reserve_locks(&self, pubkeys: &[Pubkey]) -> CacheInnerGuard<'_> { + /// Ensures a [`NonceLock`] exists in the cache for each pubkey and returns + /// a [`CacheInnerGuard`] holding clones of those locks. + fn acquire_nonce_locks(&self, pubkeys: &[Pubkey]) -> CacheInnerGuard<'_> { // Sorted order is required: all callers acquire per-key locks in the same // order, preventing the A→B / B→A circular-wait deadlock. let mut pubkeys = pubkeys.to_vec(); @@ -313,7 +320,7 @@ impl CacheTaskInfoFetcher { { let mut inner = self.cache.lock().expect(MUTEX_POISONED_MSG); for pubkey in pubkeys { - let (lock, eviceted) = + let (lock, evicted) = if let Some(val) = inner.active.get(&pubkey) { (val.clone(), None) } else if let Some(val) = inner.retiring.remove(&pubkey) { @@ -326,7 +333,7 @@ impl CacheTaskInfoFetcher { (val, evicted) }; - if let Some((evicted_pk, evicted_lock)) = eviceted { + if let Some((evicted_pk, evicted_lock)) = evicted { // If value isn't used by anyone then it can be dropped if Arc::strong_count(&evicted_lock) > 1 { // Value used in by another request @@ -346,7 +353,7 @@ impl CacheTaskInfoFetcher { // This is impossible as per logic above, contradiction. чтд. debug_assert!( false, - "Just eviceted value can't be in retiring" + "Just evicted value can't be in retiring" ); error!("Retiring map already contained lock with pubkey: {}", evicted_pk); } @@ -378,14 +385,14 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(HashMap::new()); } - let locks_guard = self.reserve_locks(pubkeys); + let locks_guard = self.acquire_nonce_locks(pubkeys); // Acquire per-account locks sequentially in sorted order (see sort above). // join_all would poll all futures concurrently, allowing partial acquisition // and producing the classic A→B / B→A deadlock across concurrent callers. - let guard_nonces = locks_guard.lock().await; + let nonce_guards = locks_guard.lock().await; let (mut existing, mut to_request) = (vec![], vec![]); - for (pubkey, guard) in guard_nonces { + for (pubkey, guard) in nonce_guards { if *guard == u64::MAX { to_request.push((pubkey, guard)); } else { @@ -444,13 +451,13 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(HashMap::new()); } - let locks_guard = self.reserve_locks(pubkeys); + let locks_guard = self.acquire_nonce_locks(pubkeys); // Acquire per-account locks sequentially in sorted order (see sort above). - let guard_nonces = locks_guard.lock().await; + let nonce_guards = locks_guard.lock().await; let mut to_request = vec![]; - let mut result = HashMap::with_capacity(guard_nonces.len()); - for (pubkey, guard) in guard_nonces { + let mut result = HashMap::with_capacity(nonce_guards.len()); + for (pubkey, guard) in nonce_guards { if *guard == u64::MAX { to_request.push((pubkey, guard)); } else { @@ -506,7 +513,7 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { Ok(rent_reimbursements) } - /// Returns current commit id without raising priority + /// Returns the cached nonce without promoting LRU order or incrementing. async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { let lock = { // Peek without promoting LRU order; also check retiring for in-flight keys. diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index 11a3d3620..38f71f0c7 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -5,6 +5,8 @@ use solana_pubkey::Pubkey; use crate::intent::CommittedAccount; +/// Trait that provides access to system calls implemented outside of SVM, +/// accessible in magic-program. pub trait MagicSys: Sync + Send + 'static { fn persist(&self, id: u64, data: Vec) -> Result<(), Box>; fn load(&self, id: u64) -> Result>, Box>; From 84a3af0048c590c0f54bedebdb451b288fa6c093 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 4 Mar 2026 23:21:09 +0700 Subject: [PATCH 13/19] fix: COMMIT_LIMIT --- programs/magicblock/src/magic_sys.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs index 03856c2ee..0fed11e79 100644 --- a/programs/magicblock/src/magic_sys.rs +++ b/programs/magicblock/src/magic_sys.rs @@ -9,7 +9,11 @@ use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; -pub const COMMIT_LIMIT: u64 = 15; +/// Maximum number of times an account may be committed before it must be +/// undelegated. A plain commit at or beyond this limit fails with [`COMMIT_LIMIT_ERR`]. +pub const COMMIT_LIMIT: u64 = 10; +/// [`InstructionError::Custom`] code returned when a commit is attempted on an +/// account that has reached [`COMMIT_LIMIT`]. pub const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; pub(crate) const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; From 2de4dc1f5a5d58b82f1d70d48b7fdb613b230afd Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 5 Mar 2026 00:57:12 +0700 Subject: [PATCH 14/19] feat: separate cache and rpc task info fetchers --- .../src/committor_processor.rs | 10 +- .../src/intent_execution_manager.rs | 4 +- .../intent_executor_factory.rs | 10 +- .../src/intent_executor/mod.rs | 6 +- .../src/intent_executor/task_info_fetcher.rs | 420 ++++++++++-------- .../src/tasks/task_strategist.rs | 8 +- .../test-committor-service/tests/common.rs | 16 +- .../tests/test_intent_executor.rs | 14 +- 8 files changed, 270 insertions(+), 218 deletions(-) diff --git a/magicblock-committor-service/src/committor_processor.rs b/magicblock-committor-service/src/committor_processor.rs index 5b124b5b2..2e3e1335c 100644 --- a/magicblock-committor-service/src/committor_processor.rs +++ b/magicblock-committor-service/src/committor_processor.rs @@ -21,7 +21,8 @@ use crate::{ db::DummyDB, BroadcastedIntentExecutionResult, IntentExecutionManager, }, intent_executor::task_info_fetcher::{ - CacheTaskInfoFetcher, TaskInfoFetcher, TaskInfoFetcherResult, + CacheTaskInfoFetcher, RpcTaskInfoFetcher, TaskInfoFetcher, + TaskInfoFetcherResult, }, persist::{ CommitStatusRow, IntentPersister, IntentPersisterImpl, @@ -35,7 +36,7 @@ pub(crate) struct CommittorProcessor { pub(crate) authority: Keypair, persister: IntentPersisterImpl, commits_scheduler: IntentExecutionManager, - task_info_fetcher: Arc, + task_info_fetcher: Arc>, } impl CommittorProcessor { @@ -66,8 +67,9 @@ impl CommittorProcessor { let persister = IntentPersisterImpl::try_new(persist_file)?; // Create commit scheduler - let task_info_fetcher = - Arc::new(CacheTaskInfoFetcher::new(magic_block_rpc_client.clone())); + let task_info_fetcher = Arc::new(CacheTaskInfoFetcher::new( + RpcTaskInfoFetcher::new(magic_block_rpc_client.clone()), + )); let commits_scheduler = IntentExecutionManager::new( magic_block_rpc_client.clone(), DummyDB::new(), diff --git a/magicblock-committor-service/src/intent_execution_manager.rs b/magicblock-committor-service/src/intent_execution_manager.rs index 6ded30e43..d53918f13 100644 --- a/magicblock-committor-service/src/intent_execution_manager.rs +++ b/magicblock-committor-service/src/intent_execution_manager.rs @@ -17,7 +17,7 @@ use crate::{ }, intent_executor::{ intent_executor_factory::IntentExecutorFactoryImpl, - task_info_fetcher::CacheTaskInfoFetcher, + task_info_fetcher::{CacheTaskInfoFetcher, RpcTaskInfoFetcher}, }, persist::IntentPersister, ComputeBudgetConfig, @@ -33,7 +33,7 @@ impl IntentExecutionManager { pub fn new( rpc_client: MagicblockRpcClient, db: D, - task_info_fetcher: Arc, + task_info_fetcher: Arc>, intent_persister: Option

, table_mania: TableMania, compute_budget_config: ComputeBudgetConfig, diff --git a/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs b/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs index 1d46e43a9..6f61e389c 100644 --- a/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs +++ b/magicblock-committor-service/src/intent_executor/intent_executor_factory.rs @@ -5,8 +5,8 @@ use magicblock_table_mania::TableMania; use crate::{ intent_executor::{ - task_info_fetcher::CacheTaskInfoFetcher, IntentExecutor, - IntentExecutorImpl, + task_info_fetcher::{CacheTaskInfoFetcher, RpcTaskInfoFetcher}, + IntentExecutor, IntentExecutorImpl, }, transaction_preparator::TransactionPreparatorImpl, ComputeBudgetConfig, @@ -23,12 +23,12 @@ pub struct IntentExecutorFactoryImpl { pub rpc_client: MagicblockRpcClient, pub table_mania: TableMania, pub compute_budget_config: ComputeBudgetConfig, - pub task_info_fetcher: Arc, + pub task_info_fetcher: Arc>, } impl IntentExecutorFactory for IntentExecutorFactoryImpl { type Executor = - IntentExecutorImpl; + IntentExecutorImpl; fn create_instance(&self) -> Self::Executor { let transaction_preparator = TransactionPreparatorImpl::new( @@ -36,7 +36,7 @@ impl IntentExecutorFactory for IntentExecutorFactoryImpl { self.table_mania.clone(), self.compute_budget_config.clone(), ); - IntentExecutorImpl::::new( + IntentExecutorImpl::::new( self.rpc_client.clone(), transaction_preparator, self.task_info_fetcher.clone(), diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index c510f503a..7a1668d1b 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -38,7 +38,7 @@ use crate::{ TransactionStrategyExecutionError, }, single_stage_executor::SingleStageExecutor, - task_info_fetcher::{ResetType, TaskInfoFetcher}, + task_info_fetcher::{CacheTaskInfoFetcher, ResetType, TaskInfoFetcher}, two_stage_executor::TwoStageExecutor, }, persist::{CommitStatus, CommitStatusSignatures, IntentPersister}, @@ -108,7 +108,7 @@ pub struct IntentExecutorImpl { authority: Keypair, rpc_client: MagicblockRpcClient, transaction_preparator: T, - task_info_fetcher: Arc, + task_info_fetcher: Arc>, /// Junk that needs to be cleaned up pub junk: Vec, @@ -124,7 +124,7 @@ where pub fn new( rpc_client: MagicblockRpcClient, transaction_preparator: T, - task_info_fetcher: Arc, + task_info_fetcher: Arc>, ) -> Self { let authority = validator_authority(); Self { diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index bdf4fa80f..e82649733 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -53,12 +53,6 @@ pub trait TaskInfoFetcher: Send + Sync + 'static { min_context_slot: u64, ) -> TaskInfoFetcherResult>; - /// Peeks current commit ids for pubkeys - async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option; - - /// Resets cache for some or all accounts - fn reset(&self, reset_type: ResetType); - async fn get_base_accounts( &self, pubkeys: &[Pubkey], @@ -71,90 +65,18 @@ pub enum ResetType<'a> { Specific(&'a [Pubkey]), } -/// Per-account async mutex protecting the cached nonce value. -type NonceLock = Arc>; - -/// Split-map cache: `active` is the live LRU window; `retiring` holds locks -/// evicted from `active` that are still held by in-flight requests. -struct CacheInner { - active: LruCache, - retiring: HashMap, -} - -/// RAII guard returned by [`CacheTaskInfoFetcher::acquire_nonce_locks`]. -/// Holds clones of the reserved [`NonceLock`]s and cleans up retiring entries -/// on drop. -struct CacheInnerGuard<'a> { - inner: &'a Mutex, - nonce_locks: Vec<(Pubkey, NonceLock)>, -} - -impl<'a> CacheInnerGuard<'a> { - // Acquire per-account locks sequentially in sorted order (see sort above). - // join_all would poll all futures concurrently, allowing partial acquisition - // and producing the classic A→B / B→A deadlock across concurrent callers. - async fn lock<'s>(&'s self) -> Vec<(&'s Pubkey, MutexGuard<'s, u64>)> { - let mut output = Vec::with_capacity(self.nonce_locks.len()); - for (pubkey, lock) in self.nonce_locks.iter() { - let guard = lock.lock().await; - output.push((pubkey, guard)) - } - - output - } -} - -impl<'a> Drop for CacheInnerGuard<'a> { - fn drop(&mut self) { - let mut inner = self.inner.lock().expect(MUTEX_POISONED_MSG); - let nonce_locks = mem::take(&mut self.nonce_locks); - for (pubkey, lock) in nonce_locks { - // Drop our clone first so strong_count reflects only other - // live holders when we check below - drop(lock); - let should_remove = inner - .retiring - .get(&pubkey) - .is_some_and(|l| Arc::strong_count(l) == 1); - if should_remove { - inner.retiring.remove(&pubkey); - } - } - } -} - -impl CacheInner { - fn new(capacity: NonZeroUsize) -> Self { - Self { - active: LruCache::new(capacity), - retiring: HashMap::new(), - } - } -} +// --------------------------------------------------------------------------- +// RpcTaskInfoFetcher +// --------------------------------------------------------------------------- -pub struct CacheTaskInfoFetcher { +/// Pure RPC implementation of [`TaskInfoFetcher`] — no caching. +pub struct RpcTaskInfoFetcher { rpc_client: MagicblockRpcClient, - cache: Mutex, } -impl CacheTaskInfoFetcher { +impl RpcTaskInfoFetcher { pub fn new(rpc_client: MagicblockRpcClient) -> Self { - const CACHE_SIZE: NonZeroUsize = NonZeroUsize::new(1000).unwrap(); - - Self { - rpc_client, - cache: Mutex::new(CacheInner::new(CACHE_SIZE)), - } - } - - pub fn with_capacity( - capacity: NonZeroUsize, - rpc_client: MagicblockRpcClient, - ) -> Self { - Self { - rpc_client, - cache: Mutex::new(CacheInner::new(capacity)), - } + Self { rpc_client } } /// Fetches [`DelegationMetadata`]s with some num of retries @@ -235,7 +157,6 @@ impl CacheTaskInfoFetcher { break Err(err); } TaskInfoFetcherError::MinContextSlotNotReachedError(_, _) => { - // Get some extra sleep info!( min_context_slot, attempt = i, @@ -262,7 +183,6 @@ impl CacheTaskInfoFetcher { pubkeys: &[Pubkey], min_context_slot: u64, ) -> TaskInfoFetcherResult> { - // Early return if no pubkeys to process if pubkeys.is_empty() { return Ok(Vec::new()); } @@ -306,6 +226,205 @@ impl CacheTaskInfoFetcher { Ok(accounts) } +} + +#[async_trait] +impl TaskInfoFetcher for RpcTaskInfoFetcher { + async fn fetch_next_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + if pubkeys.is_empty() { + return Ok(HashMap::new()); + } + let nonces = Self::fetch_metadata_with_retries( + &self.rpc_client, + pubkeys, + min_context_slot, + NUM_FETCH_RETRIES, + ) + .await? + .into_iter() + .map(|m| m.last_update_nonce + 1); + Ok(pubkeys.iter().copied().zip(nonces).collect()) + } + + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + if pubkeys.is_empty() { + return Ok(HashMap::new()); + } + let nonces = Self::fetch_metadata_with_retries( + &self.rpc_client, + pubkeys, + min_context_slot, + NUM_FETCH_RETRIES, + ) + .await? + .into_iter() + .map(|m| m.last_update_nonce); + Ok(pubkeys.iter().copied().zip(nonces).collect()) + } + + async fn fetch_rent_reimbursements( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + Ok(Self::fetch_metadata_with_retries( + &self.rpc_client, + pubkeys, + min_context_slot, + NUM_FETCH_RETRIES, + ) + .await? + .into_iter() + .map(|m| m.rent_payer) + .collect()) + } + + async fn get_base_accounts( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + let accounts = Self::fetch_accounts_with_retries( + &self.rpc_client, + pubkeys, + min_context_slot, + NUM_FETCH_RETRIES, + ) + .await?; + Ok(pubkeys.iter().copied().zip(accounts).collect()) + } +} + +/// Per-account async mutex protecting the cached nonce value. +type NonceLock = Arc>; + +/// Split-map cache: `active` is the live LRU window; `retiring` holds locks +/// evicted from `active` that are still held by in-flight requests. +struct CacheInner { + active: LruCache, + retiring: HashMap, +} + +/// RAII guard returned by [`CacheTaskInfoFetcher::acquire_nonce_locks`]. +/// Holds clones of the reserved [`NonceLock`]s and cleans up retiring entries +/// on drop. +struct CacheInnerGuard<'a> { + inner: &'a Mutex, + nonce_locks: Vec<(Pubkey, NonceLock)>, +} + +impl<'a> CacheInnerGuard<'a> { + // Acquire per-account locks sequentially in sorted order (see sort above). + // join_all would poll all futures concurrently, allowing partial acquisition + // and producing the classic A→B / B→A deadlock across concurrent callers. + async fn lock<'s>(&'s self) -> Vec<(&'s Pubkey, MutexGuard<'s, u64>)> { + let mut output = Vec::with_capacity(self.nonce_locks.len()); + for (pubkey, lock) in self.nonce_locks.iter() { + let guard = lock.lock().await; + output.push((pubkey, guard)) + } + + output + } +} + +impl<'a> Drop for CacheInnerGuard<'a> { + fn drop(&mut self) { + let mut inner = self.inner.lock().expect(MUTEX_POISONED_MSG); + let nonce_locks = mem::take(&mut self.nonce_locks); + for (pubkey, lock) in nonce_locks { + // Drop our clone first so strong_count reflects only other + // live holders when we check below + drop(lock); + let should_remove = inner + .retiring + .get(&pubkey) + .is_some_and(|l| Arc::strong_count(l) == 1); + if should_remove { + inner.retiring.remove(&pubkey); + } + } + } +} + +impl CacheInner { + fn new(capacity: NonZeroUsize) -> Self { + Self { + active: LruCache::new(capacity), + retiring: HashMap::new(), + } + } +} + +/// [`TaskInfoFetcher`] that caches the most recently used nonces, delegating +/// cache misses and all non-nonce queries to an inner `T: TaskInfoFetcher`. +pub struct CacheTaskInfoFetcher { + inner: T, + cache: Mutex, +} + +impl CacheTaskInfoFetcher { + pub fn new(inner: T) -> Self { + const CACHE_SIZE: NonZeroUsize = NonZeroUsize::new(1000).unwrap(); + + Self { + inner, + cache: Mutex::new(CacheInner::new(CACHE_SIZE)), + } + } + + pub fn with_capacity(capacity: NonZeroUsize, inner: T) -> Self { + Self { + inner, + cache: Mutex::new(CacheInner::new(capacity)), + } + } + + /// Returns the cached nonce without promoting LRU order or incrementing. + pub async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { + let lock = { + let inner = self.cache.lock().expect(MUTEX_POISONED_MSG); + inner + .active + .peek(pubkey) + .or_else(|| inner.retiring.get(pubkey)) + .cloned() + }?; + + let locks_guard = CacheInnerGuard { + inner: &self.cache, + nonce_locks: vec![(*pubkey, lock)], + }; + let guards = locks_guard.lock().await; + let value = *guards[0].1; + + (value != u64::MAX).then_some(value) + } + + /// Resets cache for some or all accounts + pub fn reset(&self, reset_type: ResetType) { + let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); + match reset_type { + ResetType::All => { + cache.active.clear(); + cache.retiring.clear(); + } + ResetType::Specific(pubkeys) => { + for pubkey in pubkeys { + cache.active.pop(pubkey); + cache.retiring.remove(pubkey); + } + } + } + } /// Ensures a [`NonceLock`] exists in the cache for each pubkey and returns /// a [`CacheInnerGuard`] holding clones of those locks. @@ -371,9 +490,9 @@ impl CacheTaskInfoFetcher { } } -/// TaskInfoFetcher implementation that also caches most used 1000 keys +/// TaskInfoFetcher implementation that caches the most used 1000 nonces #[async_trait] -impl TaskInfoFetcher for CacheTaskInfoFetcher { +impl TaskInfoFetcher for CacheTaskInfoFetcher { /// Returns next ids for requested pubkeys /// If key isn't in cache, it will be requested async fn fetch_next_commit_nonces( @@ -403,40 +522,34 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { // If all in cache - great! return if to_request.is_empty() { let mut result = HashMap::with_capacity(existing.len()); - // Consume guards & write result for (pubkey, mut guard) in existing { *guard += 1; result.insert(*pubkey, *guard); } - return Ok(result); } - // Remove duplicates let to_request_pubkeys: Vec<_> = to_request.iter().map(|(pubkey, _)| **pubkey).collect(); - let remaining_ids = Self::fetch_metadata_with_retries( - &self.rpc_client, - &to_request_pubkeys, - min_context_slot, - NUM_FETCH_RETRIES, - ) - .await? - .into_iter() - .map(|metadata| metadata.last_update_nonce); + let nonces = self + .inner + .fetch_current_commit_nonces(&to_request_pubkeys, min_context_slot) + .await?; // We don't care if anything changed in between with cache - just update and return our ids. let mut result = HashMap::with_capacity(existing.len()); - // Consume guards & write result for (pubkey, mut guard) in existing { *guard += 1; result.insert(*pubkey, *guard); } - for ((pubkey, mut guard), nonce) in - to_request.into_iter().zip(remaining_ids) - { - *guard = nonce + 1; - result.insert(*pubkey, *guard); + for (pubkey, mut guard) in to_request { + if let Some(&nonce) = nonces.get(pubkey) { + *guard = nonce + 1; + result.insert(*pubkey, *guard); + Ok(()) + } else { + Err(TaskInfoFetcherError::AccountNotFoundError(*pubkey)) + }?; } Ok(result) @@ -471,24 +584,22 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { let to_request_pubkeys: Vec<_> = to_request.iter().map(|(pubkey, _)| **pubkey).collect(); - let remaining_ids = Self::fetch_metadata_with_retries( - &self.rpc_client, - &to_request_pubkeys, - min_context_slot, - NUM_FETCH_RETRIES, - ) - .await? - .into_iter() - .map(|metadata| metadata.last_update_nonce); + let nonces = self + .inner + .fetch_current_commit_nonces(&to_request_pubkeys, min_context_slot) + .await?; // Store the on-chain nonce as-is (no +1): recording current state, not // reserving the next slot. A subsequent fetch_next_commit_nonces call will // increment from here correctly. - for ((pubkey, mut guard), nonce) in - to_request.into_iter().zip(remaining_ids) - { - *guard = nonce; - result.insert(*pubkey, nonce); + for (pubkey, mut guard) in to_request { + if let Some(&nonce) = nonces.get(pubkey) { + *guard = nonce; + result.insert(*pubkey, nonce); + Ok(()) + } else { + Err(TaskInfoFetcherError::AccountNotFoundError(*pubkey)) + }? } Ok(result) @@ -499,58 +610,9 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { pubkeys: &[Pubkey], min_context_slot: u64, ) -> TaskInfoFetcherResult> { - let rent_reimbursements = Self::fetch_metadata_with_retries( - &self.rpc_client, - pubkeys, - min_context_slot, - NUM_FETCH_RETRIES, - ) - .await? - .into_iter() - .map(|metadata| metadata.rent_payer) - .collect(); - - Ok(rent_reimbursements) - } - - /// Returns the cached nonce without promoting LRU order or incrementing. - async fn peek_commit_nonce(&self, pubkey: &Pubkey) -> Option { - let lock = { - // Peek without promoting LRU order; also check retiring for in-flight keys. - // Outer lock held only to clone the Arc, released before awaiting per-key lock. - let inner = self.cache.lock().expect(MUTEX_POISONED_MSG); - inner - .active - .peek(pubkey) - .or_else(|| inner.retiring.get(pubkey)) - .cloned() - }?; - - let locks_guard = CacheInnerGuard { - inner: &self.cache, - nonce_locks: vec![(*pubkey, lock)], - }; - let guards = locks_guard.lock().await; - let value = *guards[0].1; - - (value != u64::MAX).then_some(value) - } - - /// Reset cache - fn reset(&self, reset_type: ResetType) { - let mut cache = self.cache.lock().expect(MUTEX_POISONED_MSG); - match reset_type { - ResetType::All => { - cache.active.clear(); - cache.retiring.clear(); - } - ResetType::Specific(pubkeys) => { - for pubkey in pubkeys { - cache.active.pop(pubkey); - cache.retiring.remove(pubkey); - } - } - } + self.inner + .fetch_rent_reimbursements(pubkeys, min_context_slot) + .await } async fn get_base_accounts( @@ -558,15 +620,9 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { pubkeys: &[Pubkey], min_context_slot: u64, ) -> TaskInfoFetcherResult> { - let accounts = Self::fetch_accounts_with_retries( - &self.rpc_client, - pubkeys, - min_context_slot, - NUM_FETCH_RETRIES, - ) - .await?; - - Ok(pubkeys.iter().copied().zip(accounts).collect()) + self.inner + .get_base_accounts(pubkeys, min_context_slot) + .await } } diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index 4e446d944..d46de96bc 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -394,7 +394,7 @@ mod tests { use crate::{ intent_execution_manager::intent_scheduler::create_test_intent, intent_executor::task_info_fetcher::{ - ResetType, TaskInfoFetcher, TaskInfoFetcherResult, + TaskInfoFetcher, TaskInfoFetcherResult, }, persist::IntentPersisterImpl, tasks::{ @@ -434,12 +434,6 @@ mod tests { Ok(pubkeys.iter().map(|_| Pubkey::new_unique()).collect()) } - async fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { - Some(0) - } - - fn reset(&self, _: ResetType) {} - async fn get_base_accounts( &self, _pubkeys: &[Pubkey], diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index 31f5c2de5..e3cbdba49 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -10,7 +10,7 @@ use async_trait::async_trait; use magicblock_committor_service::{ intent_executor::{ task_info_fetcher::{ - ResetType, TaskInfoFetcher, TaskInfoFetcherError, + CacheTaskInfoFetcher, TaskInfoFetcher, TaskInfoFetcherError, TaskInfoFetcherResult, }, IntentExecutorImpl, @@ -114,8 +114,12 @@ impl TestFixture { } #[allow(dead_code)] - pub fn create_task_info_fetcher(&self) -> Arc { - Arc::new(MockTaskInfoFetcher(self.rpc_client.clone())) + pub fn create_task_info_fetcher( + &self, + ) -> Arc> { + Arc::new(CacheTaskInfoFetcher::new(MockTaskInfoFetcher( + self.rpc_client.clone(), + ))) } } @@ -147,12 +151,6 @@ impl TaskInfoFetcher for MockTaskInfoFetcher { Ok(pubkeys.to_vec()) } - async fn peek_commit_nonce(&self, _pubkey: &Pubkey) -> Option { - None - } - - fn reset(&self, _: ResetType) {} - async fn get_base_accounts( &self, pubkeys: &[Pubkey], 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 33706c627..b81a16056 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -17,7 +17,8 @@ use magicblock_committor_service::{ intent_executor::{ error::{IntentExecutorError, TransactionStrategyExecutionError}, task_info_fetcher::{ - CacheTaskInfoFetcher, TaskInfoFetcher, TaskInfoFetcherError, + CacheTaskInfoFetcher, RpcTaskInfoFetcher, TaskInfoFetcher, + TaskInfoFetcherError, }, ExecutionOutput, IntentExecutionResult, IntentExecutor, IntentExecutorImpl, @@ -75,9 +76,9 @@ const ACTOR_ESCROW_INDEX: u8 = 1; struct TestEnv { fixture: TestFixture, - task_info_fetcher: Arc, + task_info_fetcher: Arc>, intent_executor: - IntentExecutorImpl, + IntentExecutorImpl, pre_test_tablemania_state: HashMap, } @@ -89,8 +90,9 @@ impl TestEnv { .await; let transaction_preparator = fixture.create_transaction_preparator(); - let task_info_fetcher = - Arc::new(CacheTaskInfoFetcher::new(fixture.rpc_client.clone())); + let task_info_fetcher = Arc::new(CacheTaskInfoFetcher::new( + RpcTaskInfoFetcher::new(fixture.rpc_client.clone()), + )); let tm = &fixture.table_mania; let mut pre_test_tablemania_state = HashMap::new(); @@ -1237,7 +1239,7 @@ fn create_scheduled_intent( async fn single_flow_transaction_strategy( authority: &Pubkey, - task_info_fetcher: &Arc, + task_info_fetcher: &Arc>, intent: &ScheduledIntentBundle, ) -> TransactionStrategy { let mut tasks = TaskBuilderImpl::commit_tasks( From fc1834202d3bcce93ca542dfd0277ac794933c3d Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 5 Mar 2026 01:07:09 +0700 Subject: [PATCH 15/19] fix: tests --- .../src/intent_executor/task_info_fetcher.rs | 170 +++++++----------- 1 file changed, 66 insertions(+), 104 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index e82649733..e1a710ba2 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -697,126 +697,94 @@ mod tests { use std::{collections::VecDeque, sync::Arc, time::Duration}; use async_trait::async_trait; - use base64::{prelude::BASE64_STANDARD, Engine}; - use dlp::state::DelegationMetadata; - use solana_account_decoder::{UiAccount, UiAccountData, UiAccountEncoding}; + use solana_account::Account; use solana_pubkey::Pubkey; - use solana_rpc_client::{ - nonblocking::rpc_client::RpcClient, - rpc_client::RpcClientConfig, - rpc_sender::{RpcSender, RpcTransportStats}, - }; - use solana_rpc_client_api::{ - client_error::Result as ClientResult, - request::RpcRequest, - response::{Response, RpcResponseContext}, - }; use super::*; // ---- mock ---- - struct TestSender { - calls: std::sync::Mutex>, - rpc_delay: Option, + struct MockInfoFetcher { + nonces: std::sync::Mutex>, + delay: Option, } - impl TestSender { - fn new(responses: Vec) -> Self { + impl MockInfoFetcher { + fn new(nonces: Vec) -> Self { Self { - calls: std::sync::Mutex::new(responses.into()), - rpc_delay: None, + nonces: std::sync::Mutex::new(nonces.into()), + delay: None, } } fn with_delay(mut self, delay: Duration) -> Self { - self.rpc_delay = Some(delay); + self.delay = Some(delay); self } } #[async_trait] - impl RpcSender for TestSender { - async fn send( + impl TaskInfoFetcher for MockInfoFetcher { + async fn fetch_next_commit_nonces( &self, - request: RpcRequest, - params: serde_json::Value, - ) -> ClientResult { - if let Some(delay) = self.rpc_delay { - tokio::time::sleep(delay).await; - } - assert_eq!(request, RpcRequest::GetMultipleAccounts); - let n = params[0].as_array().map(|a| a.len()).unwrap_or(0); - let metas: Vec = { - let mut q = self.calls.lock().unwrap(); - (0..n) - .map(|_| { - q.pop_front() - .expect("unexpected RPC call: queue exhausted") - }) - .collect() - }; - let accounts: Vec> = - metas.iter().map(|m| Some(encode_meta(m))).collect(); - Ok(serde_json::to_value(Response { - context: RpcResponseContext { - slot: 1, - api_version: None, - }, - value: accounts, - }) - .unwrap()) + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + self.fetch_current_commit_nonces(pubkeys, min_context_slot) + .await } - fn get_transport_stats(&self) -> RpcTransportStats { - RpcTransportStats::default() - } - - fn url(&self) -> String { - "test".to_string() - } - } - - fn encode_meta(m: &DelegationMetadata) -> UiAccount { - let mut bytes = Vec::new(); - m.to_bytes_with_discriminator(&mut bytes).unwrap(); - UiAccount { - lamports: 1_000_000, - data: UiAccountData::Binary( - BASE64_STANDARD.encode(&bytes), - UiAccountEncoding::Base64, - ), - owner: dlp::id().to_string(), - executable: false, - rent_epoch: 0, - space: Some(bytes.len() as u64), + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + if let Some(delay) = self.delay { + tokio::time::sleep(delay).await; + } + let mut q = self.nonces.lock().unwrap(); + Ok(pubkeys + .iter() + .map(|pk| { + let nonce = + q.pop_front().expect("mock nonce queue exhausted"); + (*pk, nonce) + }) + .collect()) + } + + async fn fetch_rent_reimbursements( + &self, + _: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + unimplemented!() } - } - fn meta(nonce: u64) -> DelegationMetadata { - DelegationMetadata { - last_update_nonce: nonce, - is_undelegatable: false, - seeds: vec![], - rent_payer: Pubkey::default(), + async fn get_base_accounts( + &self, + _: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + unimplemented!() } } struct FetcherBuilder { - sender: TestSender, + inner: MockInfoFetcher, capacity: Option, } impl FetcherBuilder { - fn new(responses: Vec) -> Self { + fn new(nonces: Vec) -> Self { Self { - sender: TestSender::new(responses), + inner: MockInfoFetcher::new(nonces), capacity: None, } } fn rpc_delay(mut self, d: Duration) -> Self { - self.sender = self.sender.with_delay(d); + self.inner = self.inner.with_delay(d); self } @@ -825,13 +793,12 @@ mod tests { self } - fn build(self) -> CacheTaskInfoFetcher { - let rpc = MagicblockRpcClient::new(Arc::new( - RpcClient::new_sender(self.sender, RpcClientConfig::default()), - )); + fn build(self) -> CacheTaskInfoFetcher { match self.capacity { - Some(cap) => CacheTaskInfoFetcher::with_capacity(cap, rpc), - None => CacheTaskInfoFetcher::new(rpc), + Some(cap) => { + CacheTaskInfoFetcher::with_capacity(cap, self.inner) + } + None => CacheTaskInfoFetcher::new(self.inner), } } } @@ -841,7 +808,7 @@ mod tests { #[tokio::test] async fn cache_miss_then_hit() { let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![meta(10)]).build(); + let fetcher = FetcherBuilder::new(vec![10]).build(); let r1 = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); assert_eq!(r1[&pk], 11); @@ -856,7 +823,7 @@ mod tests { let pk1 = Pubkey::new_unique(); let pk2 = Pubkey::new_unique(); // prime pk1 (nonce 5), then mixed call fetches only cold pk2 (nonce 20) - let fetcher = FetcherBuilder::new(vec![meta(5), meta(20)]).build(); + let fetcher = FetcherBuilder::new(vec![5, 20]).build(); fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 6 let r = fetcher @@ -872,9 +839,7 @@ mod tests { let pk1 = Pubkey::new_unique(); let pk2 = Pubkey::new_unique(); // pk1 initial, pk2 evicts pk1, pk1 re-fetch after eviction - let fetcher = FetcherBuilder::new(vec![meta(1), meta(2), meta(10)]) - .capacity(1) - .build(); + let fetcher = FetcherBuilder::new(vec![1, 2, 10]).capacity(1).build(); fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3, pk1 evicted @@ -886,7 +851,7 @@ mod tests { // barrier. Phase 2: fetch phase2_keys one-by-one for `iters` passes, then // fetch shared_b in chunks of 2. async fn run_worker( - fetcher: Arc, + fetcher: Arc>, barrier: Arc, phase1_keys: Vec, phase2_keys: Vec, @@ -931,7 +896,7 @@ mod tests { excl[i].iter().chain(shared_a.iter()).cloned().collect() }); - // Flat queue: each RPC call pops exactly N entries (N cold keys). + // Flat queue: each mock call pops exactly N entries (N cold keys). // Upper bounds: // phase 1 loop: SHARED_A × 1 // phase 2 excl+sa: ITERS × PHASE2_KEYS × NUM_WORKERS × 1 @@ -940,11 +905,9 @@ mod tests { let total = SHARED_A + ITERS * PHASE2_KEYS * NUM_WORKERS + SHARED_B * NUM_WORKERS; - let responses: Vec = - (0..total).map(|_| meta(0)).collect(); let fetcher = Arc::new( - FetcherBuilder::new(responses) + FetcherBuilder::new(vec![0; total]) .capacity(CAPACITY) .rpc_delay(Duration::from_millis(2)) .build(), @@ -1003,7 +966,7 @@ mod tests { #[tokio::test] async fn fetch_current_no_increment() { let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![meta(10)]).build(); + let fetcher = FetcherBuilder::new(vec![10]).build(); let r1 = fetcher.fetch_current_commit_nonces(&[pk], 0).await.unwrap(); assert_eq!(r1[&pk], 10); // stored as-is @@ -1016,7 +979,7 @@ mod tests { #[tokio::test] async fn reset_all_forces_refetch() { let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![meta(5), meta(99)]).build(); + let fetcher = FetcherBuilder::new(vec![5, 99]).build(); fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); // pk = 6 fetcher.reset(ResetType::All); @@ -1030,8 +993,7 @@ mod tests { let pk1 = Pubkey::new_unique(); let pk2 = Pubkey::new_unique(); // pk1 initial, pk2 initial, pk1 after reset - let fetcher = - FetcherBuilder::new(vec![meta(1), meta(2), meta(50)]).build(); + let fetcher = FetcherBuilder::new(vec![1, 2, 50]).build(); fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3 @@ -1046,7 +1008,7 @@ mod tests { #[tokio::test] async fn peek_does_not_increment() { let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![meta(7)]).build(); + let fetcher = FetcherBuilder::new(vec![7]).build(); assert_eq!(fetcher.peek_commit_nonce(&pk).await, None); From 23268c45cfc008ca1849f29d02c9bb5435162e1b Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 5 Mar 2026 13:45:11 +0700 Subject: [PATCH 16/19] fix: tests --- magicblock-api/src/magic_sys_adapter.rs | 3 + .../src/intent_executor/task_info_fetcher.rs | 362 ++++++++++-------- .../process_schedule_commit.rs | 2 +- 3 files changed, 200 insertions(+), 167 deletions(-) diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 19220e1e4..0bf29ea42 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -14,8 +14,11 @@ pub struct MagicSysAdapter { } impl MagicSysAdapter { + /// Returned when receiving the nonce result from the async channel fails. const RECV_ERR: u32 = 0xE000_0000; + /// Returned when the async fetch of current commit nonces fails. const FETCH_ERR: u32 = 0xE001_0000; + /// Returned when no committor service is configured. const NO_COMMITTOR_ERR: u32 = 0xE002_0000; pub fn new( diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index e1a710ba2..81de307fc 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -60,11 +60,6 @@ pub trait TaskInfoFetcher: Send + Sync + 'static { ) -> TaskInfoFetcherResult>; } -pub enum ResetType<'a> { - All, - Specific(&'a [Pubkey]), -} - // --------------------------------------------------------------------------- // RpcTaskInfoFetcher // --------------------------------------------------------------------------- @@ -313,6 +308,15 @@ struct CacheInner { retiring: HashMap, } +impl CacheInner { + fn new(capacity: NonZeroUsize) -> Self { + Self { + active: LruCache::new(capacity), + retiring: HashMap::new(), + } + } +} + /// RAII guard returned by [`CacheTaskInfoFetcher::acquire_nonce_locks`]. /// Holds clones of the reserved [`NonceLock`]s and cleans up retiring entries /// on drop. @@ -355,15 +359,6 @@ impl<'a> Drop for CacheInnerGuard<'a> { } } -impl CacheInner { - fn new(capacity: NonZeroUsize) -> Self { - Self { - active: LruCache::new(capacity), - retiring: HashMap::new(), - } - } -} - /// [`TaskInfoFetcher`] that caches the most recently used nonces, delegating /// cache misses and all non-nonce queries to an inner `T: TaskInfoFetcher`. pub struct CacheTaskInfoFetcher { @@ -504,23 +499,24 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(HashMap::new()); } + // Acquire locks on requested nonces let locks_guard = self.acquire_nonce_locks(pubkeys); // Acquire per-account locks sequentially in sorted order (see sort above). // join_all would poll all futures concurrently, allowing partial acquisition // and producing the classic A→B / B→A deadlock across concurrent callers. let nonce_guards = locks_guard.lock().await; - let (mut existing, mut to_request) = (vec![], vec![]); + let (mut existing, mut missing) = (vec![], vec![]); for (pubkey, guard) in nonce_guards { if *guard == u64::MAX { - to_request.push((pubkey, guard)); + missing.push((pubkey, guard)); } else { existing.push((pubkey, guard)) } } // If all in cache - great! return - if to_request.is_empty() { + if missing.is_empty() { let mut result = HashMap::with_capacity(existing.len()); for (pubkey, mut guard) in existing { *guard += 1; @@ -529,12 +525,14 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(result); } - let to_request_pubkeys: Vec<_> = - to_request.iter().map(|(pubkey, _)| **pubkey).collect(); - let nonces = self - .inner - .fetch_current_commit_nonces(&to_request_pubkeys, min_context_slot) - .await?; + // Fetch missing nonces in cache + let fetched_nonces = { + let missing_pubkeys: Vec<_> = + missing.iter().map(|(pubkey, _)| **pubkey).collect(); + self.inner + .fetch_current_commit_nonces(&missing_pubkeys, min_context_slot) + .await? + }; // We don't care if anything changed in between with cache - just update and return our ids. let mut result = HashMap::with_capacity(existing.len()); @@ -542,8 +540,8 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { *guard += 1; result.insert(*pubkey, *guard); } - for (pubkey, mut guard) in to_request { - if let Some(&nonce) = nonces.get(pubkey) { + for (pubkey, mut guard) in missing { + if let Some(&nonce) = fetched_nonces.get(pubkey) { *guard = nonce + 1; result.insert(*pubkey, *guard); Ok(()) @@ -564,36 +562,39 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { return Ok(HashMap::new()); } + // Acquire locks on requested nonces let locks_guard = self.acquire_nonce_locks(pubkeys); // Acquire per-account locks sequentially in sorted order (see sort above). let nonce_guards = locks_guard.lock().await; - let mut to_request = vec![]; + let mut missing = vec![]; let mut result = HashMap::with_capacity(nonce_guards.len()); for (pubkey, guard) in nonce_guards { if *guard == u64::MAX { - to_request.push((pubkey, guard)); + missing.push((pubkey, guard)); } else { result.insert(*pubkey, *guard); } } - if to_request.is_empty() { + if missing.is_empty() { return Ok(result); } - let to_request_pubkeys: Vec<_> = - to_request.iter().map(|(pubkey, _)| **pubkey).collect(); - let nonces = self - .inner - .fetch_current_commit_nonces(&to_request_pubkeys, min_context_slot) - .await?; + // Fetch missing nonces in cache + let fetched_nonces = { + let missing_pubkeys: Vec<_> = + missing.iter().map(|(pubkey, _)| **pubkey).collect(); + self.inner + .fetch_current_commit_nonces(&missing_pubkeys, min_context_slot) + .await? + }; // Store the on-chain nonce as-is (no +1): recording current state, not // reserving the next slot. A subsequent fetch_next_commit_nonces call will // increment from here correctly. - for (pubkey, mut guard) in to_request { - if let Some(&nonce) = nonces.get(pubkey) { + for (pubkey, mut guard) in missing { + if let Some(&nonce) = fetched_nonces.get(pubkey) { *guard = nonce; result.insert(*pubkey, nonce); Ok(()) @@ -626,6 +627,11 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher { } } +pub enum ResetType<'a> { + All, + Specific(&'a [Pubkey]), +} + #[derive(thiserror::Error, Debug)] pub enum TaskInfoFetcherError { #[error("Metadata not found for: {0}")] @@ -702,109 +708,6 @@ mod tests { use super::*; - // ---- mock ---- - - struct MockInfoFetcher { - nonces: std::sync::Mutex>, - delay: Option, - } - - impl MockInfoFetcher { - fn new(nonces: Vec) -> Self { - Self { - nonces: std::sync::Mutex::new(nonces.into()), - delay: None, - } - } - - fn with_delay(mut self, delay: Duration) -> Self { - self.delay = Some(delay); - self - } - } - - #[async_trait] - impl TaskInfoFetcher for MockInfoFetcher { - async fn fetch_next_commit_nonces( - &self, - pubkeys: &[Pubkey], - min_context_slot: u64, - ) -> TaskInfoFetcherResult> { - self.fetch_current_commit_nonces(pubkeys, min_context_slot) - .await - } - - async fn fetch_current_commit_nonces( - &self, - pubkeys: &[Pubkey], - _: u64, - ) -> TaskInfoFetcherResult> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } - let mut q = self.nonces.lock().unwrap(); - Ok(pubkeys - .iter() - .map(|pk| { - let nonce = - q.pop_front().expect("mock nonce queue exhausted"); - (*pk, nonce) - }) - .collect()) - } - - async fn fetch_rent_reimbursements( - &self, - _: &[Pubkey], - _: u64, - ) -> TaskInfoFetcherResult> { - unimplemented!() - } - - async fn get_base_accounts( - &self, - _: &[Pubkey], - _: u64, - ) -> TaskInfoFetcherResult> { - unimplemented!() - } - } - - struct FetcherBuilder { - inner: MockInfoFetcher, - capacity: Option, - } - - impl FetcherBuilder { - fn new(nonces: Vec) -> Self { - Self { - inner: MockInfoFetcher::new(nonces), - capacity: None, - } - } - - fn rpc_delay(mut self, d: Duration) -> Self { - self.inner = self.inner.with_delay(d); - self - } - - fn capacity(mut self, n: usize) -> Self { - self.capacity = Some(n.try_into().unwrap()); - self - } - - fn build(self) -> CacheTaskInfoFetcher { - match self.capacity { - Some(cap) => { - CacheTaskInfoFetcher::with_capacity(cap, self.inner) - } - None => CacheTaskInfoFetcher::new(self.inner), - } - } - } - - // ---- tests ---- - #[tokio::test] async fn cache_miss_then_hit() { let pk = Pubkey::new_unique(); @@ -841,10 +744,17 @@ mod tests { // pk1 initial, pk2 evicts pk1, pk1 re-fetch after eviction let fetcher = FetcherBuilder::new(vec![1, 2, 10]).capacity(1).build(); - fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 - fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3, pk1 evicted + fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 cached = 2 + fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 cached = 3, pk1 evicted + + assert!(fetcher.peek_commit_nonce(&pk1).await.is_none()); // evicted + let r = fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); assert_eq!(r[&pk1], 11); // re-fetched (10 + 1) + + // Sequential eviction: pk1's guard was dropped before pk2 evicted it, + // so Arc strong_count was 1 — never moved to retiring. + assert_eq!(fetcher.cache.lock().unwrap().retiring.len(), 0); } // Phase 1: fetch phase1_keys one-by-one → barrier → outer verification → @@ -976,18 +886,6 @@ mod tests { assert_eq!(r2[&pk], 10); } - #[tokio::test] - async fn reset_all_forces_refetch() { - let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![5, 99]).build(); - - fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); // pk = 6 - fetcher.reset(ResetType::All); - - let r = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); - assert_eq!(r[&pk], 100); // re-fetched - } - #[tokio::test] async fn reset_specific_only_clears_that_key() { let pk1 = Pubkey::new_unique(); @@ -995,28 +893,160 @@ mod tests { // pk1 initial, pk2 initial, pk1 after reset let fetcher = FetcherBuilder::new(vec![1, 2, 50]).build(); - fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 = 2 - fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 = 3 + fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); // pk1 cached = 2 + fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); // pk2 cached = 3 fetcher.reset(ResetType::Specific(&[pk1])); + assert!(fetcher.peek_commit_nonce(&pk1).await.is_none()); // cleared + assert_eq!(fetcher.peek_commit_nonce(&pk2).await, Some(3)); // still cached + let r1 = fetcher.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); - let r2 = fetcher.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); assert_eq!(r1[&pk1], 51); // re-fetched (50 + 1) - assert_eq!(r2[&pk2], 4); // still cached (3 + 1) } #[tokio::test] - async fn peek_does_not_increment() { - let pk = Pubkey::new_unique(); - let fetcher = FetcherBuilder::new(vec![7]).build(); + async fn peek_awaits_inflight_fetch() { + let pk1 = Pubkey::new_unique(); + let pk2 = Pubkey::new_unique(); + // capacity=1: pk2 fetch evicts pk1 into retiring while Task A is still in-flight. + // pk1 nonce=7 (stored as 8), pk2 nonce=9 (stored as 10). + let fetcher = Arc::new( + FetcherBuilder::new(vec![7, 9]) + .rpc_delay(Duration::from_millis(50)) + .capacity(1) + .build(), + ); + + // Spawn Task A: slow fetch for pk1 acquires its nonce lock and sleeps. + let fetcher2 = fetcher.clone(); + let task_a = tokio::spawn(async move { + fetcher2.fetch_next_commit_nonces(&[pk1], 0).await.unwrap(); + }); + + // Let Task A acquire pk1's nonce lock and start the slow fetch. + tokio::task::yield_now().await; + + // Spawn Task B: inserts pk2 (capacity=1), evicting pk1 into retiring + // because Task A's guard still holds a clone of pk1's Arc. + let fetcher3 = fetcher.clone(); + let task_b = tokio::spawn(async move { + fetcher3.fetch_next_commit_nonces(&[pk2], 0).await.unwrap(); + }); + + // Let Task B run through acquire_nonce_locks (eviction happens here). + tokio::task::yield_now().await; + + // pk1 is in retiring: Task A is in-flight and holds its Arc clone. + assert_eq!(fetcher.cache.lock().unwrap().retiring.len(), 1); + + // peek finds pk1 in retiring, blocks on its in-flight lock, returns the value. + let peeked = fetcher.peek_commit_nonce(&pk1).await; + assert_eq!(peeked, Some(8)); // 7 + 1 + + task_a.await.unwrap(); + task_b.await.unwrap(); + + // All CacheInnerGuards dropped: retiring fully cleaned up. + assert_eq!(fetcher.cache.lock().unwrap().retiring.len(), 0); + } + + /// Fetcher mock + struct MockInfoFetcher { + nonces: Mutex>, + delay: Option, + } + + impl MockInfoFetcher { + fn new(nonces: Vec) -> Self { + Self { + nonces: Mutex::new(nonces.into()), + delay: None, + } + } + + fn with_delay(mut self, delay: Duration) -> Self { + self.delay = Some(delay); + self + } + } + + #[async_trait] + impl TaskInfoFetcher for MockInfoFetcher { + async fn fetch_next_commit_nonces( + &self, + pubkeys: &[Pubkey], + min_context_slot: u64, + ) -> TaskInfoFetcherResult> { + self.fetch_current_commit_nonces(pubkeys, min_context_slot) + .await + } + + async fn fetch_current_commit_nonces( + &self, + pubkeys: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + if let Some(delay) = self.delay { + tokio::time::sleep(delay).await; + } + let mut q = self.nonces.lock().unwrap(); + Ok(pubkeys + .iter() + .map(|pk| { + let nonce = + q.pop_front().expect("mock nonce queue exhausted"); + (*pk, nonce) + }) + .collect()) + } + + async fn fetch_rent_reimbursements( + &self, + _: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + unimplemented!() + } + + async fn get_base_accounts( + &self, + _: &[Pubkey], + _: u64, + ) -> TaskInfoFetcherResult> { + unimplemented!() + } + } - assert_eq!(fetcher.peek_commit_nonce(&pk).await, None); + struct FetcherBuilder { + inner: MockInfoFetcher, + capacity: Option, + } - fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); // pk = 8 - assert_eq!(fetcher.peek_commit_nonce(&pk).await, Some(8)); - assert_eq!(fetcher.peek_commit_nonce(&pk).await, Some(8)); // unchanged + impl FetcherBuilder { + fn new(nonces: Vec) -> Self { + Self { + inner: MockInfoFetcher::new(nonces), + capacity: None, + } + } - let r = fetcher.fetch_next_commit_nonces(&[pk], 0).await.unwrap(); - assert_eq!(r[&pk], 9); // peek didn't change the value + fn rpc_delay(mut self, d: Duration) -> Self { + self.inner = self.inner.with_delay(d); + self + } + + fn capacity(mut self, n: usize) -> Self { + self.capacity = Some(n.try_into().unwrap()); + self + } + + fn build(self) -> CacheTaskInfoFetcher { + match self.capacity { + Some(cap) => { + CacheTaskInfoFetcher::with_capacity(cap, self.inner) + } + None => CacheTaskInfoFetcher::new(self.inner), + } + } } } diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index 67cb339b0..3422a0d17 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -237,7 +237,7 @@ pub(crate) fn process_schedule_commit( // We validate commit nonces only for plain commits // If accounts are undelegated we don't validate // It may result in failed undelegation due to insufficient balance - // But it is better than progibiting undelegation overall + // But it is better than prohibiting undelegation overall // In case account is borked user shall reach out us // 1. Account is topped up by user // 2. Manual undelegation is triggered by us From 977f43084035a3ab55893af44eb9e72ac0ae6f7c Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 5 Mar 2026 14:48:37 +0700 Subject: [PATCH 17/19] fix: coderabbit --- magicblock-api/src/magic_sys_adapter.rs | 3 +++ .../src/schedule_transactions/process_schedule_commit.rs | 7 +------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 0bf29ea42..a494c47d5 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -55,6 +55,9 @@ impl MagicSys for MagicSysAdapter { &self, commits: &[CommittedAccount], ) -> Result, InstructionError> { + if commits.is_empty() { + return Ok(HashMap::new()); + } let committor_service = if let Some(committor_service) = &self.committor_service { Ok(committor_service) diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index 3422a0d17..aaf26538e 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -235,12 +235,7 @@ pub(crate) fn process_schedule_commit( // NOTE: // We validate commit nonces only for plain commits - // If accounts are undelegated we don't validate - // It may result in failed undelegation due to insufficient balance - // But it is better than prohibiting undelegation overall - // In case account is borked user shall reach out us - // 1. Account is topped up by user - // 2. Manual undelegation is triggered by us + // If accounts are undelegated we don't want to fail if !opts.request_undelegation { check_commit_limits(&committed_accounts, invoke_context)?; } From 11b0a0207154f27e21cbd1f6b2cb2c1a9beb2360 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 6 Mar 2026 21:36:27 +0700 Subject: [PATCH 18/19] fix: pr comments --- Cargo.lock | 2 +- magicblock-api/Cargo.toml | 1 - magicblock-api/src/magic_sys_adapter.rs | 41 +++++++++++-------- .../src/intent_executor/task_info_fetcher.rs | 9 +++- magicblock-committor-service/src/service.rs | 39 ++++++++++++++++++ magicblock-metrics/src/metrics/mod.rs | 23 +++++++++++ programs/magicblock/Cargo.toml | 1 + .../process_schedule_commit_tests.rs | 35 +++++++--------- test-integration/Cargo.lock | 1 - .../test-committor-service/tests/common.rs | 23 ----------- 10 files changed, 112 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 219b35ee5..8f7d1c7ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3052,7 +3052,6 @@ dependencies = [ "anyhow", "borsh 1.6.0", "fd-lock", - "futures", "magic-domain-program", "magicblock-account-cloner", "magicblock-accounts", @@ -3413,6 +3412,7 @@ dependencies = [ "parking_lot", "rand 0.9.2", "serde", + "serial_test", "solana-account", "solana-account-info", "solana-clock", diff --git a/magicblock-api/Cargo.toml b/magicblock-api/Cargo.toml index f48c457c5..1a1c8dab8 100644 --- a/magicblock-api/Cargo.toml +++ b/magicblock-api/Cargo.toml @@ -11,7 +11,6 @@ edition.workspace = true anyhow = { workspace = true } borsh = "1.5.3" fd-lock = { workspace = true } -futures = { workspace = true } tracing = { workspace = true } magic-domain-program = { workspace = true } diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index a494c47d5..9515eea16 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -1,8 +1,9 @@ -use std::{collections::HashMap, error::Error, sync::Arc}; +use std::{collections::HashMap, error::Error, sync::Arc, time::Duration}; -use magicblock_committor_service::{BaseIntentCommittor, CommittorService}; +use magicblock_committor_service::CommittorService; use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use magicblock_ledger::Ledger; +use magicblock_metrics::metrics; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; use tracing::{enabled, error, trace, Level}; @@ -14,12 +15,16 @@ pub struct MagicSysAdapter { } impl MagicSysAdapter { - /// Returned when receiving the nonce result from the async channel fails. + /// Returned when the sync channel is disconnected (sender dropped). const RECV_ERR: u32 = 0xE000_0000; - /// Returned when the async fetch of current commit nonces fails. - const FETCH_ERR: u32 = 0xE001_0000; + /// Returned when waiting for the nonce fetch times out. + 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 = 0xE002_0000; + const NO_COMMITTOR_ERR: u32 = 0xE000_0003; + + const FETCH_TIMEOUT: Duration = Duration::from_secs(30); pub fn new( ledger: Arc, @@ -73,17 +78,21 @@ impl MagicSys for MagicSysAdapter { let pubkeys: Vec<_> = commits.iter().map(|account| account.pubkey).collect(); + let _timer = metrics::start_fetch_commit_nonces_wait_timer(); let receiver = committor_service - .fetch_current_commit_nonces(&pubkeys, min_context_slot); - // Tx execution is sync and runs on a tokio worker thread. handle.block_on - // would panic (nested runtime). futures::executor::block_on parks this - // thread independently of tokio — safe because the thread is already - // committed to this tx until execution completes. - futures::executor::block_on(receiver) - .inspect_err(|err| { - error!(error = ?err, "Failed to receive nonces from CommittorService") - }) - .map_err(|_| InstructionError::Custom(Self::RECV_ERR))? + .fetch_current_commit_nonces_sync(&pubkeys, min_context_slot); + receiver + .recv_timeout(Self::FETCH_TIMEOUT) + .map_err(|err| match err { + std::sync::mpsc::RecvTimeoutError::Timeout => { + error!("Timed out waiting for commit nonces from CommittorService"); + InstructionError::Custom(Self::TIMEOUT_ERR) + } + std::sync::mpsc::RecvTimeoutError::Disconnected => { + error!("CommittorService channel disconnected while waiting for commit nonces"); + InstructionError::Custom(Self::RECV_ERR) + } + })? .inspect_err(|err| { error!(error = ?err, "Failed to fetch current commit nonces") }) diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index 81de307fc..a53211514 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -356,6 +356,10 @@ impl<'a> Drop for CacheInnerGuard<'a> { inner.retiring.remove(&pubkey); } } + + metrics::set_task_info_fetcher_retiring_count( + inner.retiring.len() as i64 + ); } } @@ -464,7 +468,7 @@ impl CacheTaskInfoFetcher { // Safety // assume that is true: // That means that value was active & retiring at the same time - // This is impossible as per logic above, contradiction. чтд. + // This is impossible as per logic above, contradiction. Q.E.D. debug_assert!( false, "Just evicted value can't be in retiring" @@ -476,6 +480,9 @@ impl CacheTaskInfoFetcher { nonce_locks.push((pubkey, lock)); } + metrics::set_task_info_fetcher_retiring_count( + inner.retiring.len() as i64 + ); } CacheInnerGuard { diff --git a/magicblock-committor-service/src/service.rs b/magicblock-committor-service/src/service.rs index 289e91c56..62cb7de2d 100644 --- a/magicblock-committor-service/src/service.rs +++ b/magicblock-committor-service/src/service.rs @@ -89,6 +89,13 @@ pub enum CommittorMessage { pubkeys: Vec, min_context_slot: u64, }, + FetchCurrentCommitNoncesSync { + respond_to: std::sync::mpsc::Sender< + CommittorServiceResult>, + >, + pubkeys: Vec, + min_context_slot: u64, + }, } // ----------------- @@ -251,6 +258,23 @@ impl CommittorActor { } }); } + 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"); + } + }); + } } } @@ -352,6 +376,21 @@ impl CommittorService { rx } + pub fn fetch_current_commit_nonces_sync( + &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 { diff --git a/magicblock-metrics/src/metrics/mod.rs b/magicblock-metrics/src/metrics/mod.rs index d340c28f9..f93c8d679 100644 --- a/magicblock-metrics/src/metrics/mod.rs +++ b/magicblock-metrics/src/metrics/mod.rs @@ -393,6 +393,11 @@ lazy_static::lazy_static! { "task_info_fetcher_a_count", "Get mupltiple account count" ).unwrap(); + static ref TASK_INFO_FETCHER_RETIRING_GAUGE: IntGauge = IntGauge::new( + "task_info_fetcher_retiring_gauge", + "Number of pubkeys currently in the retiring map of CacheTaskInfoFetcher" + ).unwrap(); + static ref TABLE_MANIA_A_COUNT: IntCounter = IntCounter::new( "table_mania_a_count", "Get mupltiple account count" ).unwrap(); @@ -423,6 +428,14 @@ lazy_static::lazy_static! { ), ).unwrap(); + static ref COMMITTOR_FETCH_COMMIT_NONCES_WAIT_TIME: Histogram = Histogram::with_opts( + HistogramOpts::new( + "committor_fetch_commit_nonces_wait_time_second", + "Time in seconds spent waiting for fetch_current_commit_nonces response" + ) + .buckets(vec![0.1, 0.5, 1.0, 2.0, 5.0, 10.0, 20.0, 30.0]), + ).unwrap(); + // ----------------- // Pubsub Clients // ----------------- @@ -557,6 +570,7 @@ pub(crate) fn register() { register!(COMMITTOR_INTENT_CU_USAGE); register!(COMMITTOR_INTENT_TASK_PREPARATION_TIME); register!(COMMITTOR_INTENT_ALT_PREPARATION_TIME); + register!(COMMITTOR_FETCH_COMMIT_NONCES_WAIT_TIME); register!(ENSURE_ACCOUNTS_TIME); register!(RPC_REQUEST_HANDLING_TIME); register!(TRANSACTION_PROCESSING_TIME); @@ -577,6 +591,7 @@ pub(crate) fn register() { register!(MAX_LOCK_CONTENTION_QUEUE_SIZE); register!(REMOTE_ACCOUNT_PROVIDER_A_COUNT); register!(TASK_INFO_FETCHER_A_COUNT); + register!(TASK_INFO_FETCHER_RETIRING_GAUGE); register!(TABLE_MANIA_A_COUNT); register!(TABLE_MANIA_CLOSED_A_COUNT); register!(CONNECTED_PUBSUB_CLIENTS_GAUGE); @@ -759,6 +774,10 @@ pub fn observe_committor_intent_alt_preparation_time() -> HistogramTimer { COMMITTOR_INTENT_ALT_PREPARATION_TIME.start_timer() } +pub fn start_fetch_commit_nonces_wait_timer() -> HistogramTimer { + COMMITTOR_FETCH_COMMIT_NONCES_WAIT_TIME.start_timer() +} + pub fn inc_account_fetches_success(count: u64) { ACCOUNT_FETCHES_SUCCESS_COUNT.inc_by(count); } @@ -843,6 +862,10 @@ pub fn inc_task_info_fetcher_a_count() { TASK_INFO_FETCHER_A_COUNT.inc() } +pub fn set_task_info_fetcher_retiring_count(count: i64) { + TASK_INFO_FETCHER_RETIRING_GAUGE.set(count); +} + pub fn inc_table_mania_a_count() { TABLE_MANIA_A_COUNT.inc() } diff --git a/programs/magicblock/Cargo.toml b/programs/magicblock/Cargo.toml index 0ad4396b9..a24516966 100644 --- a/programs/magicblock/Cargo.toml +++ b/programs/magicblock/Cargo.toml @@ -39,6 +39,7 @@ thiserror = { workspace = true } test-kit = { workspace = true } assert_matches = { workspace = true } rand = { workspace = true } +serial_test = { workspace = true } solana-signature = { workspace = true } solana-signer = { workspace = true } magicblock-chainlink = { workspace = true, features = ["dev-context"] } diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs index 6b23e9eea..01f72650b 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs @@ -256,20 +256,15 @@ mod tests { // ---------- Helpers for ATA/eATA remapping tests ---------- // Use shared SPL/ATA/eATA constants and helpers // Reuse test helper to create proper SPL ATA account data - use std::sync::Mutex; - use magicblock_chainlink::testing::eatas::create_ata_account; use magicblock_core::token_programs::{derive_ata, derive_eata}; + use serial_test::serial; use solana_seed_derivable::SeedDerivable; use test_kit::init_logger; use super::*; use crate::utils::instruction_utils::InstructionUtils; - // Prevents race conditions between parallel tests that share the global - // static MagicSys stub. - static TEST_LOCK: Mutex<()> = Mutex::new(()); - fn make_delegated_spl_ata_account( owner: &Pubkey, mint: &Pubkey, @@ -281,9 +276,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_single_account_success() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_single_account_success") .unwrap(); @@ -367,9 +362,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_single_account_and_request_undelegate_success() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"single_account_with_undelegate_success") .unwrap(); @@ -454,9 +449,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_remaps_delegated_ata_to_eata() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_remap_ata_to_eata").unwrap(); @@ -536,9 +531,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_and_undelegate_remaps_delegated_ata_to_eata() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_undelegate_remap_ata_eata") @@ -621,9 +616,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_three_accounts_success() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_three_accounts_success") @@ -735,8 +730,8 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_three_accounts_and_request_undelegate_success() { - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed( b"three_accounts_and_request_undelegate_success", ) @@ -888,9 +883,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_no_pdas_provided_to_ix() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_no_pdas_provided_to_ix") @@ -924,9 +919,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_undelegate_with_readonly() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_undelegate_with_readonly") @@ -968,9 +963,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_with_non_delegated_account() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_with_non_delegated_account") @@ -1008,10 +1003,10 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_three_accounts_second_not_owned_by_program_and_not_signer( ) { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"three_accounts_last_not_owned_by_program") @@ -1057,9 +1052,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_with_confined_account() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_with_confined_account") @@ -1101,9 +1096,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_fails_when_commit_limit_exceeded() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"schedule_commit_limit_exceeded____").unwrap(); @@ -1137,10 +1132,10 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_and_undelegate_succeeds_when_commit_limit_exceeded() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"undelegate_succeeds_limit_exceeded").unwrap(); @@ -1174,9 +1169,9 @@ mod tests { } #[test] + #[serial] fn test_schedule_commit_three_accounts_one_confined() { init_logger!(); - let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let payer = Keypair::from_seed(b"three_accounts_one_confined_______").unwrap(); diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index a1f7342d0..a65a17de8 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3396,7 +3396,6 @@ dependencies = [ "anyhow", "borsh 1.6.0", "fd-lock", - "futures", "magic-domain-program", "magicblock-account-cloner", "magicblock-accounts", diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index e3cbdba49..e3c8160c4 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -98,29 +98,6 @@ impl TestFixture { self.compute_budget_config.clone(), ) } - - #[allow(dead_code)] - pub fn create_intent_executor( - &self, - ) -> IntentExecutorImpl - { - let transaction_preparator = self.create_transaction_preparator(); - - IntentExecutorImpl::new( - self.rpc_client.clone(), - transaction_preparator, - self.create_task_info_fetcher(), - ) - } - - #[allow(dead_code)] - pub fn create_task_info_fetcher( - &self, - ) -> Arc> { - Arc::new(CacheTaskInfoFetcher::new(MockTaskInfoFetcher( - self.rpc_client.clone(), - ))) - } } pub struct MockTaskInfoFetcher(MagicblockRpcClient); From c621d8c2ae2c2efc5e495f704309135cbebeb76e Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 6 Mar 2026 21:38:15 +0700 Subject: [PATCH 19/19] fix: .lock --- test-integration/Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index a65a17de8..e793be94b 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3677,7 +3677,7 @@ dependencies = [ [[package]] name = "magicblock-magic-program-api" version = "0.8.0" -source = "git+https://github.com/magicblock-labs/magicblock-validator.git?branch=snawaz%2Fschedule-commit-finalize#33f3097e3ceb40538b2983ff89623c788469620e" +source = "git+https://github.com/magicblock-labs/magicblock-validator.git?branch=snawaz%2Fschedule-commit-finalize#a281d23937ac6b95e6b6ab4ef6af7089946b002e" dependencies = [ "bincode", "serde",