diff --git a/engine/Cargo.toml b/engine/Cargo.toml index a2c58377..59e623c8 100644 --- a/engine/Cargo.toml +++ b/engine/Cargo.toml @@ -51,6 +51,7 @@ magicblock-engine = { path = ".", features = ["testkit"] } nucleus = { workspace = true, features = ["testkit"] } v42-calculator-interface = { workspace = true, features = ["builder"] } +solana-instruction = { workspace = true } solana-instruction-error = { workspace = true } solana-packet = { workspace = true } solana-signer = { workspace = true } diff --git a/engine/src/accessor.rs b/engine/src/accessor.rs index 7b56f03d..e00f0ae7 100644 --- a/engine/src/accessor.rs +++ b/engine/src/accessor.rs @@ -1,11 +1,11 @@ //! Account- and transaction-scoped operation facades. -use std::{sync::atomic::Ordering, time::Duration}; +use std::{collections::BTreeSet, sync::atomic::Ordering, time::Duration}; use keeper::{ExecutionRecord, TransactionView}; use magic_root_interface::MagicRootInstruction; use processor::{SequencerMessage, Simulation, SimulatorMessage}; -use solana_account::OwnedAccount; +use solana_account::{AccountMode, OwnedAccount}; use solana_instruction::Instruction; use solana_pubkey::Pubkey; use solana_transaction::TransactionResult; @@ -35,18 +35,58 @@ pub struct TransactionAccessor<'a> { impl AccountAccessor<'_> { /// Creates the account by patching in every field and finalizing it, /// optionally running follow-up `actions` once it is finalized. + /// + /// Missing writable accounts named by `actions` (ER-only PDAs such as an + /// auction tree) are asserted with [`MagicRootInstruction::Prepare`] in the + /// same transaction, so a follow-up action can initialize them (for example + /// `create_ephemeral`). `Prepare` materializes the placeholder only if the + /// account still does not exist at execution time, so concurrent creates + /// that share such an account converge on the first materialization instead + /// of failing each other. pub async fn create( &self, acc: impl Into, actions: Option>, ) -> Result<()> { - let mut instructions = MagicRootInstruction::compose_account(self.pubkey, acc.into())?; + let mut instructions = Vec::new(); + if let Some(actions) = actions.as_ref() { + for pubkey in self.missing_writable_action_accounts(actions) { + instructions.push(MagicRootInstruction::Prepare.compose(pubkey)?); + } + } + instructions.extend(MagicRootInstruction::compose_account( + self.pubkey, + acc.into(), + )?); if let Some(actions) = actions { instructions.push(MagicRootInstruction::PostFinalize(actions).compose(self.pubkey)?); } self.execute(instructions).await } + /// Writable action accounts that are absent or only a slot-0 Placeholder. + fn missing_writable_action_accounts(&self, actions: &[Instruction]) -> Vec { + let accounts = self.engine.accounts(); + let loader = accounts.loader(); + let mut seen = BTreeSet::new(); + let mut missing = Vec::new(); + for action in actions { + for meta in &action.accounts { + if !meta.is_writable || meta.pubkey == self.pubkey || !seen.insert(meta.pubkey) { + continue; + } + let needs_prepare = + loader.load(&meta.pubkey).ok().flatten().is_none_or(|account| { + account.is(AccountMode::Placeholder) && account.slot() == 0 + }); + if needs_prepare { + missing.push(meta.pubkey); + } + } + } + missing + } + /// Updates the account by patching in every field of `account` pub async fn update(&self, acc: impl Into) -> Result<()> { let instructions = MagicRootInstruction::compose_account(self.pubkey, acc.into())?; diff --git a/engine/tests/accounts.rs b/engine/tests/accounts.rs index 28e0e739..8f29567a 100644 --- a/engine/tests/accounts.rs +++ b/engine/tests/accounts.rs @@ -12,6 +12,7 @@ use keeper::testkit::{ }; use magic_root_interface::MagicRootInstruction; use solana_account::{AccountBuilder, AccountMode, OwnedAccount, ReadableAccount}; +use solana_instruction::AccountMeta; use solana_instruction_error::InstructionError; use solana_pubkey::Pubkey; use solana_system_interface::MAX_PERMITTED_DATA_LENGTH; @@ -458,3 +459,122 @@ async fn create_runs_post_finalize_actions() { te.close().await; } + +// A writable action account that is never initialized stays a Placeholder. +// PostFinalize must reject that and roll back, not commit an empty Ephemeral. +#[tokio::test(flavor = "multi_thread")] +async fn create_rejects_uninitialized_writable_action_accounts() { + let te = TestEngine::new().await; + + let source = store_v42(&te, 0, AccountMode::Delegated); + let ok_key = Pubkey::new_unique(); + let missing = Pubkey::new_unique(); + let mut benign = transfer(source, ok_key, 1); + benign.accounts.push(AccountMeta::new(missing, false)); + let acc = v42_builder(0, AccountMode::Delegated); + let result = te.account(ok_key).create(acc, Some(vec![benign])).await; + assert!( + result.is_err(), + "untouched writable placeholder must fail PostFinalize" + ); + assert!( + te.get_account(ok_key).is_none(), + "create rolls back when a writable action account is left uninitialized" + ); + assert!( + te.get_account(missing).is_none(), + "the prepared placeholder rolls back with the transaction" + ); + + te.close().await; +} + +// Concurrent creates of different accounts can name the same missing writable +// in their actions (the auction-tree pattern). Prepare materializes it only if +// it still does not exist at execution time, so no create fails another one: +// each fails solely its own PostFinalize check (the action never initializes +// the shared writable), never with InvalidArgument or AlreadyProcessed. +#[tokio::test(flavor = "multi_thread")] +async fn concurrent_creates_share_one_missing_writable() { + let te = TestEngine::new().await; + + let source = store_v42(&te, 0, AccountMode::Delegated); + let shared = Pubkey::new_unique(); + let engine = Engine::clone(&te); + let mut handles = Vec::new(); + for _ in 0..8 { + let engine = engine.clone(); + let target = Pubkey::new_unique(); + let mut action = transfer(source, target, 1); + action.accounts.push(AccountMeta::new(shared, false)); + handles.push(tokio::spawn(async move { + let result = engine + .account(target) + .create(v42_builder(0, AccountMode::Delegated), Some(vec![action])) + .await; + (target, result) + })); + } + for handle in handles { + let (target, result) = handle.await.expect("create task"); + let error = result.expect_err("untouched shared writable fails PostFinalize"); + assert!( + matches!( + error, + EngineError::TransactionExecution(TransactionError::InstructionError( + _, + InstructionError::Immutable, + )) + ), + "each create fails only its own PostFinalize check: {error:?}" + ); + assert!( + te.get_account(target).is_none(), + "failed create commits nothing" + ); + } + assert!( + te.get_account(shared).is_none(), + "rolled-back creates leave no shared placeholder behind" + ); + + te.close().await; +} + +// Prepare materializes a missing account as an empty placeholder and leaves any +// existing account untouched — the execution-time idempotence the concurrent +// create path relies on. +#[tokio::test(flavor = "multi_thread")] +async fn prepare_creates_missing_and_noops_on_existing() { + let te = TestEngine::new().await; + + let fresh = Pubkey::new_unique(); + te.execute(&[MagicRootInstruction::Prepare.compose(fresh).unwrap()]) + .await + .expect("prepare materializes a missing account"); + let placeholder = te.get_account(fresh).expect("placeholder committed"); + assert!(placeholder.is(AccountMode::Placeholder)); + assert_eq!(placeholder.slot(), 1); + + let existing = store_v42(&te, 7, AccountMode::Delegated); + let before = te.get_account(existing).expect("existing account"); + let other = Pubkey::new_unique(); + te.execute(&[ + MagicRootInstruction::Prepare.compose(existing).unwrap(), + MagicRootInstruction::Prepare.compose(other).unwrap(), + ]) + .await + .expect("prepare of an existing account is a no-op"); + let unchanged = te.get_account(existing).expect("existing account remains"); + assert!(unchanged.is(AccountMode::Delegated), "mode untouched"); + assert_eq!(unchanged.slot(), before.slot(), "slot untouched"); + assert_eq!(unchanged.data(), before.data(), "data untouched"); + assert!( + te.get_account(other) + .expect("second prepare committed") + .is(AccountMode::Placeholder), + "prepare still materializes the missing account in the same transaction" + ); + + te.close().await; +} diff --git a/programs/magic-root-interface/src/lib.rs b/programs/magic-root-interface/src/lib.rs index 6d0d2811..aaf0c5ce 100644 --- a/programs/magic-root-interface/src/lib.rs +++ b/programs/magic-root-interface/src/lib.rs @@ -21,6 +21,12 @@ pub enum MagicRootInstruction { /// (e.g. initializing a freshly created account); each is invoked via CPI /// against the accounts it declares. PostFinalize(Vec), + /// Materialize the target as an empty system-owned placeholder if it still + /// does not exist at execution time; a target that already exists is left + /// untouched. This makes the "create the shared writables my actions need" + /// step idempotent, so concurrent creates that name the same missing + /// account all converge on whichever transaction materializes it first. + Prepare, } impl MagicRootInstruction { diff --git a/programs/magic-root-program/Cargo.toml b/programs/magic-root-program/Cargo.toml index 9df75161..a8a77a24 100644 --- a/programs/magic-root-program/Cargo.toml +++ b/programs/magic-root-program/Cargo.toml @@ -19,6 +19,7 @@ solana-account = { workspace = true } solana-instruction = { workspace = true } solana-instruction-error = { workspace = true } solana-program-runtime = { workspace = true } +solana-sdk-ids = { workspace = true } solana-svm-log-collector = { workspace = true } solana-transaction-context = { workspace = true } diff --git a/programs/magic-root-program/README.md b/programs/magic-root-program/README.md index ca2abe63..95afefba 100644 --- a/programs/magic-root-program/README.md +++ b/programs/magic-root-program/README.md @@ -35,6 +35,12 @@ authorization. The SVM's separate top-level-only privilege rule is unchanged. placed immediately after the target's `Finalize` by internal composers. It rejects any immutable instruction account marked writable and any action that targets MagicRoot itself. +- `Prepare` materializes the target as an empty system-owned placeholder at + slot 1 when the target still does not exist at execution time (loaded as a + slot-0 placeholder); any existing target is left untouched. The check runs + under the transaction's account locks, so concurrent transactions asserting + the same missing account converge on the first materialization instead of + failing each other. After authority and caller checks pass, MagicRoot does not determine whether a complete account image is stale. Callers must supply current state; slot and diff --git a/programs/magic-root-program/src/account.rs b/programs/magic-root-program/src/account.rs index 325bee70..5bcebbe3 100644 --- a/programs/magic-root-program/src/account.rs +++ b/programs/magic-root-program/src/account.rs @@ -70,6 +70,35 @@ pub(crate) fn finalize( Ok(()) } +/// Materializes the target as an empty system-owned placeholder when it does +/// not exist yet; a target that already exists in any form is left untouched. +/// +/// The existence check runs at execution time under the transaction's account +/// locks, so concurrent transactions asserting the same missing account cannot +/// race: the first one materializes it and the rest no-op. +pub(crate) fn prepare( + ctx: &InvokeContext<'_, '_>, + target: IndexOfAccount, +) -> Result<(), InstructionError> { + let mut account = ctx.transaction_context.accounts().try_borrow_mut(target)?; + if !account.is(AccountMode::Placeholder) || account.slot() != 0 { + ic_msg!(ctx, "MagicRoot: prepare skipped, target exists"); + return Ok(()); + } + let patches = [ + AccountFieldPatch::Owner(solana_sdk_ids::system_program::id()), + AccountFieldPatch::Slot(1), + ]; + for patch in patches { + if let Err(error) = patch.apply(&mut account) { + ic_msg!(ctx, "MagicRoot: {}", error); + return Err(InstructionError::InvalidArgument); + } + } + ic_msg!(ctx, "MagicRoot: prepared placeholder"); + Ok(()) +} + /// Marks the target account closed for removal from storage. pub(crate) fn delete( ctx: &mut InvokeContext<'_, '_>, diff --git a/programs/magic-root-program/src/processor.rs b/programs/magic-root-program/src/processor.rs index ebb29fd4..e70b207d 100644 --- a/programs/magic-root-program/src/processor.rs +++ b/programs/magic-root-program/src/processor.rs @@ -71,6 +71,7 @@ fn dispatch( MagicRootInstruction::Finalize(flags) => account::finalize(ctx, target, flags), MagicRootInstruction::Delete => account::delete(ctx, target), MagicRootInstruction::PostFinalize(actions) => post_finalize::process(ctx, actions), + MagicRootInstruction::Prepare => account::prepare(ctx, target), } }