Skip to content

feat: fail intent if it will hit MaxInstructionTraceLengthExceeded on Solana #1162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
taco-paco wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

feat: fail intent if it will hit MaxInstructionTraceLengthExceeded on Solana #1162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0b97fa
fix: undelegation of account via CommitFinalize in intent bundle entr…
taco-paco Apr 16, 2026
901562e
refactor: doc -> comment
taco-paco Apr 16, 2026
baa08bd
wip
taco-paco Apr 16, 2026
4c27d7e
Merge branch 'master' into fix/committor/committees-limit
taco-paco Apr 17, 2026
76f4b0b
feat: add tests for cpi restriction
taco-paco Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 70 additions & 7 deletions programs/magicblock/src/magic_scheduled_base_intent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ pub const COMMIT_FEE_LAMPORTS: u64 = 100_000;
/// denominated in micro-lamports per CU (mirrors Solana's priority fee model).
pub const COMPUTE_UNIT_PRICE_MICRO_LAMPORTS: u64 = 50_000;

/// Number of CPIS limited to 64 on Solana
pub const CPI_LIMIT: usize = 64;
/// Fixed base CPIs per tx stage (SetComputeUnitLimit + SetComputeUnitPrice).
const BASE_STAGE_CPIS: usize = 2;
Comment on lines +43 to +46
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

CPI_LIMIT doc and threshold semantics.

Solana's actual constant is MAX_INSTRUCTION_TRACE_LENGTH = 64, which bounds total instructions in the trace (top-level + all CPIs), not strictly CPIs. With stepped formulas (2 + 3·n, 2 + 6·n, 2 + 5·n) the estimate never lands on exactly 64, so >= vs > is indistinguishable in practice today — but the naming and doc-comment suggest "max CPIs = 64" while the check semantics are "reject at 64 or more instructions", and a future helper that produces an estimate of exactly 64 would be incorrectly rejected.

Recommend either:

  • Rename to MAX_INSTRUCTION_TRACE_LENGTH and update the comment to match Solana's terminology, or
  • Keep the name but set it to 63 (one less than Solana's hard limit) to preserve a safety margin against the inherent estimate imprecision.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@programs/magicblock/src/magic_scheduled_base_intent.rs` around lines 43 - 46,
The constant CPI_LIMIT is named and documented as "Number of CPIS limited to 64"
but Solana's real bound is MAX_INSTRUCTION_TRACE_LENGTH (total instructions
including top-level + CPIs) and the code's comparisons treat 64 as a hard reject
threshold; update to avoid off-by-one/semantic mismatch by either renaming
CPI_LIMIT to MAX_INSTRUCTION_TRACE_LENGTH and updating its doc-comment to state
it bounds total instruction trace length (so places using CPI_LIMIT and
BASE_STAGE_CPIS keep the same numeric value but correct meaning), or if you want
to preserve the "max CPIs" intent keep the symbol CPI_LIMIT and change its value
to 63 to maintain a one-instruction safety margin; adjust any uses of
CPI_LIMIT/BASE_STAGE_CPIS accordingly so checks remain consistent with the
chosen interpretation.


/// Context necessary for construction of Schedule Action
pub struct ConstructionContext<'a, 'ic> {
parent_program_id: Option<Pubkey>,
Expand Down Expand Up @@ -277,7 +282,7 @@ impl MagicIntentBundle {
commit_finalize_and_undelegate,
standalone_actions: actions,
};
this.post_validation(context)?;
this.post_validation(&context.invoke_context)?;

Ok(this)
}
Expand All @@ -286,7 +291,7 @@ impl MagicIntentBundle {
/// 1. Set of committed accounts shall not overlap with
/// set of undelegated accounts
/// 2. None for now :)
fn validate(
pub fn validate(
args: &MagicIntentBundleArgs,
context: &ConstructionContext<'_, '_>,
) -> Result<(), InstructionError> {
Expand Down Expand Up @@ -321,13 +326,13 @@ impl MagicIntentBundle {
/// Post cross intent validation:
/// 1. Validates that all committed accounts across the entire intent bundle
/// are globally unique by pubkey.
fn post_validation(
pub fn post_validation(
&self,
context: &ConstructionContext<'_, '_>,
invoke_context: &&mut InvokeContext<'_>,
) -> Result<(), InstructionError> {
if self.is_empty() {
ic_msg!(
context.invoke_context,
invoke_context,
"ScheduleCommit ERR: intent bundle must not be empty.",
);
return Err(InstructionError::InvalidInstructionData);
Expand All @@ -339,7 +344,7 @@ impl MagicIntentBundle {
for el in accounts {
if !seen.insert(el.pubkey) {
ic_msg!(
context.invoke_context,
invoke_context,
"ScheduleCommit ERR: duplicate committed account pubkey across bundle: {}",
el.pubkey
);
Expand All @@ -364,7 +369,25 @@ impl MagicIntentBundle {
check(commit_finalize_and_undelegate.get_committed_accounts())?;
}

Ok(())
self.validate_cpi_budget(invoke_context)
}

pub fn validate_cpi_budget(
&self,
invoke_context: &&mut InvokeContext<'_>,
) -> Result<(), InstructionError> {
let (commit_stage_cpis, finalize_stage_cpis) =
(self.commit_stage_cpis(), self.finalize_stage_cpis());
if commit_stage_cpis >= CPI_LIMIT || finalize_stage_cpis >= CPI_LIMIT {
ic_msg!(
invoke_context,
"ScheduleCommit ERR: too many committed accounts.",
);

Err(InstructionError::MaxAccountsExceeded)
} else {
Ok(())
}
}
Comment on lines +375 to 391
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Diagnostic: include which stage overflowed and by how much.

The log currently only says "too many committed accounts"; it doesn't tell operators whether the commit stage or the finalize stage tripped, nor the estimated CPI count vs CPI_LIMIT. When users hit this they'll need to figure out whether to split their intent or switch commit types — surfacing the numbers is cheap and very helpful.

✏️ Proposed change 
     pub fn validate_cpi_budget(
         &self,
         invoke_context: &&mut InvokeContext<'_>,
     ) -> Result<(), InstructionError> {
         let (commit_stage_cpis, finalize_stage_cpis) =
             (self.commit_stage_cpis(), self.finalize_stage_cpis());
-        if commit_stage_cpis >= CPI_LIMIT || finalize_stage_cpis >= CPI_LIMIT {
+        if commit_stage_cpis >= CPI_LIMIT {
             ic_msg!(
                 invoke_context,
-                "ScheduleCommit ERR: too many committed accounts.",
+                "ScheduleCommit ERR: commit-stage CPI estimate {} >= limit {}",
+                commit_stage_cpis, CPI_LIMIT,
             );
-
-            Err(InstructionError::MaxAccountsExceeded)
-        } else {
-            Ok(())
+            return Err(InstructionError::MaxAccountsExceeded);
         }
+        if finalize_stage_cpis >= CPI_LIMIT {
+            ic_msg!(
+                invoke_context,
+                "ScheduleCommit ERR: finalize-stage CPI estimate {} >= limit {}",
+                finalize_stage_cpis, CPI_LIMIT,
+            );
+            return Err(InstructionError::MaxAccountsExceeded);
+        }
+        Ok(())
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn validate_cpi_budget(
&self,
invoke_context: &&mut InvokeContext<'_>,
) -> Result<(), InstructionError> {
let (commit_stage_cpis, finalize_stage_cpis) =
(self.commit_stage_cpis(), self.finalize_stage_cpis());
if commit_stage_cpis >= CPI_LIMIT || finalize_stage_cpis >= CPI_LIMIT {
ic_msg!(
invoke_context,
"ScheduleCommit ERR: too many committed accounts.",
);
Err(InstructionError::MaxAccountsExceeded)
} else {
Ok(())
}
}
pub fn validate_cpi_budget(
&self,
invoke_context: &&mut InvokeContext<'_>,
) -> Result<(), InstructionError> {
let (commit_stage_cpis, finalize_stage_cpis) =
(self.commit_stage_cpis(), self.finalize_stage_cpis());
if commit_stage_cpis >= CPI_LIMIT {
ic_msg!(
invoke_context,
"ScheduleCommit ERR: commit-stage CPI estimate {} >= limit {}",
commit_stage_cpis, CPI_LIMIT,
);
return Err(InstructionError::MaxAccountsExceeded);
}
if finalize_stage_cpis >= CPI_LIMIT {
ic_msg!(
invoke_context,
"ScheduleCommit ERR: finalize-stage CPI estimate {} >= limit {}",
finalize_stage_cpis, CPI_LIMIT,
);
return Err(InstructionError::MaxAccountsExceeded);
}
Ok(())
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@programs/magicblock/src/magic_scheduled_base_intent.rs` around lines 375 -
391, The current validate_cpi_budget function logs a generic "too many committed
accounts" message without indicating which stage overflowed or the CPI counts;
update the ic_msg! call inside validate_cpi_budget to include which stage
exceeded CPI_LIMIT (commit or finalize), the offending stage's CPI value (from
commit_stage_cpis() or finalize_stage_cpis()), the other stage's CPI value, and
the CPI_LIMIT constant so operators can see "commit_stage_cpis=X,
finalize_stage_cpis=Y, CPI_LIMIT=Z" and which stage triggered the error; keep
returning InstructionError::MaxAccountsExceeded and use the existing
invoke_context/ic_msg! plumbing.


pub fn calculate_fee(
Expand Down Expand Up @@ -584,6 +607,46 @@ impl MagicIntentBundle {

self.standalone_actions.get_mut(index.checked_sub(offset)?)
}

fn commit_stage_cpis(&self) -> usize {
const COMMIT_CPIS: usize = 3;
const COMMIT_FINALIZE_CPIS: usize = 1;

let mut cpis = BASE_STAGE_CPIS;
if let Some(c) = &self.commit {
cpis += COMMIT_CPIS * c.get_committed_accounts().len();
}
if let Some(cau) = &self.commit_and_undelegate {
cpis += COMMIT_CPIS * cau.get_committed_accounts().len();
}
if let Some(cf) = &self.commit_finalize {
cpis += COMMIT_FINALIZE_CPIS * cf.get_committed_accounts().len();
}
if let Some(cfau) = &self.commit_finalize_and_undelegate {
cpis += COMMIT_FINALIZE_CPIS * cfau.get_committed_accounts().len();
}

cpis
}

fn finalize_stage_cpis(&self) -> usize {
const FINALIZE_CPIS: usize = 1;
const UNDELEGATE_CPIS: usize = 5;

let mut cpis = BASE_STAGE_CPIS;
if let Some(c) = &self.commit {
cpis += FINALIZE_CPIS * c.get_committed_accounts().len();
}
if let Some(cau) = &self.commit_and_undelegate {
cpis += (FINALIZE_CPIS + UNDELEGATE_CPIS)
* cau.get_committed_accounts().len();
}
if let Some(cfau) = &self.commit_finalize_and_undelegate {
cpis += UNDELEGATE_CPIS * cfau.get_committed_accounts().len();
}

cpis
}
}

impl MagicBaseIntent {
Expand Down
5 changes: 3 additions & 2 deletions programs/magicblock/src/schedule_transactions/process_schedule_commit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use solana_pubkey::Pubkey;
use crate::{
magic_scheduled_base_intent::{
calculate_commit_fee, validate_commit_schedule_permissions,
CommitAndUndelegate, CommitType, MagicBaseIntent,
CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle,
ScheduledIntentBundle, UndelegateType,
},
magic_sys::fetch_current_commit_nonces,
Expand Down Expand Up @@ -297,7 +297,7 @@ pub(crate) fn process_schedule_commit(
InstructionUtils::scheduled_commit_sent(intent_id, blockhash);
let commit_sent_sig = action_sent_transaction.signatures[0];

let base_intent = if opts.request_undelegation {
let base_intent: MagicIntentBundle = if opts.request_undelegation {
MagicBaseIntent::CommitAndUndelegate(CommitAndUndelegate {
commit_action: CommitType::Standalone(committed_accounts),
undelegate_action: UndelegateType::Standalone,
Expand All @@ -306,6 +306,7 @@ pub(crate) fn process_schedule_commit(
MagicBaseIntent::Commit(CommitType::Standalone(committed_accounts))
}
.into();
base_intent.validate_cpi_budget(&invoke_context)?;

let scheduled_base_intent = ScheduledIntentBundle {
id: intent_id,
Expand Down
204 changes: 101 additions & 103 deletions test-integration/schedulecommit/client/src/schedule_commit_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use solana_sdk::{
commitment_config::CommitmentConfig,
compute_budget::ComputeBudgetInstruction,
hash::Hash,
instruction::Instruction,
native_token::LAMPORTS_PER_SOL,
pubkey::Pubkey,
signature::{Keypair, Signature},
Expand Down Expand Up @@ -162,82 +163,74 @@ impl ScheduleCommitTestContext {
// -----------------
// Schedule Commit specific Transactions
// -----------------
pub fn init_committees(&self) -> Result<Signature> {
fn init_committee_ixs(
&self,
chunk: &[(Keypair, Pubkey)],
) -> Vec<Instruction> {
let mut ixs = vec![
ComputeBudgetInstruction::set_compute_unit_limit(1_400_000),
ComputeBudgetInstruction::set_compute_unit_price(10_000),
];
match self.user_seed {
UserSeeds::MagicScheduleCommit => {
ixs.extend(self.committees.iter().map(
|(player, committee)| {
init_account_instruction(
self.payer_chain.pubkey(),
player.pubkey(),
*committee,
)
},
));
ixs.extend(chunk.iter().map(|(player, committee)| {
init_account_instruction(
self.payer_chain.pubkey(),
player.pubkey(),
*committee,
)
}));
}
UserSeeds::OrderBook => {
ixs.extend(self.committees.iter().map(
|(book_manager, committee)| {
init_order_book_instruction(
self.payer_chain.pubkey(),
book_manager.pubkey(),
*committee,
)
},
));

//// TODO (snawaz): currently the size of delegatable-account cannot be
//// more than 10K, else delegation will fail. So Let's revisit this when
//// we relax the limit on the account size, then we can use larger
//// account, say even 10 MB, and execute CommitDiff.
//
// ixs.extend(self.committees.iter().flat_map(
// |(payer, committee)| {
// [grow_order_book_instruction(
// payer.pubkey(),
// *committee,
// 10 * 1024
// )]
// },
// ));
ixs.extend(chunk.iter().map(|(book_manager, committee)| {
init_order_book_instruction(
self.payer_chain.pubkey(),
book_manager.pubkey(),
*committee,
)
}));
}
};
ixs
}

let mut signers = self
.committees
.iter()
.map(|(payer, _)| payer)
.collect::<Vec<_>>();
signers.push(&self.payer_chain);

let tx = Transaction::new_signed_with_payer(
&ixs,
Some(&self.payer_chain.pubkey()),
&signers,
self.try_chain_blockhash()?,
);
let sig = self.try_chain_client()?
.send_and_confirm_transaction_with_spinner_and_config(
&tx,
self.commitment,
RpcSendTransactionConfig {
skip_preflight: true,
..Default::default()
},
)
.with_context(|| {
format!(
"Failed to initialize committees. Transaction signature: {}",
tx.get_signature()
)
})?;

debug!("Initialized committees: {sig}");
Ok(sig)
pub fn init_committees(&self) -> Result<Vec<Signature>> {
const CHUNK_SIZE: usize = 5;
let chain_client = self.try_chain_client()?;

self.committees
.chunks(CHUNK_SIZE)
.map(|chunk| {
let ixs = self.init_committee_ixs(chunk);
let mut signers =
chunk.iter().map(|(p, _)| p).collect::<Vec<_>>();
signers.push(&self.payer_chain);
(ixs, signers)
})
.map(|(ixs, signers)| -> Result<Signature> {
let tx = Transaction::new_signed_with_payer(
&ixs,
Some(&self.payer_chain.pubkey()),
&signers,
self.try_chain_blockhash()?,
);
chain_client
.send_and_confirm_transaction_with_spinner_and_config(
&tx,
self.commitment,
RpcSendTransactionConfig {
skip_preflight: true,
..Default::default()
},
)
.with_context(|| {
format!(
"Failed to initialize committees. Transaction signature: {}",
tx.get_signature()
)
})
})
.collect::<Result<Vec<_>>>()
}
Comment on lines +197 to 234
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Partial-failure semantics for chunked init/delegate.

init_committees now sends multiple transactions. If chunk N+1 fails, chunks 1..N are already committed on-chain and the returned Err provides no signal about what succeeded, leaving tests in a partially-initialized state. The same applies to delegate_committees (Lines 258-301). If this is acceptable for test harness usage, consider a comment documenting the non-atomic behavior; otherwise return the accumulated signatures alongside the error.


pub fn escrow_lamports_for_payer(&self) -> Result<Signature> {
Expand All @@ -262,44 +255,49 @@ impl ScheduleCommitTestContext {
.with_context(|| "Failed to escrow fund for payer")
}

pub fn delegate_committees(&self) -> Result<Signature> {
let mut ixs = vec![];
for (player, _) in &self.committees {
let ix = delegate_account_cpi_instruction(
self.payer_chain.pubkey(),
self.ephem_validator_identity,
player.pubkey(),
self.user_seed,
);
ixs.push(ix);
}

let chain_blockhash = self.try_chain_blockhash()?;

let tx = Transaction::new_signed_with_payer(
&ixs,
Some(&self.payer_chain.pubkey()),
&[&self.payer_chain],
chain_blockhash,
);
let sig = self
.try_chain_client()?
.send_and_confirm_transaction_with_spinner_and_config(
&tx,
self.commitment,
RpcSendTransactionConfig {
skip_preflight: true,
..Default::default()
},
)
.with_context(|| {
format!(
"Failed to delegate committees on chain '{:?}'",
tx.signatures[0]
)
})?;
debug!("Delegated committees: {sig}");
Ok(sig)
pub fn delegate_committees(&self) -> Result<Vec<Signature>> {
const CHUNK_SIZE: usize = 4;
let chain_client = self.try_chain_client()?;

self.committees
.chunks(CHUNK_SIZE)
.map(|chunk| {
chunk
.iter()
.map(|(player, _)| {
delegate_account_cpi_instruction(
self.payer_chain.pubkey(),
self.ephem_validator_identity,
player.pubkey(),
self.user_seed,
)
})
.collect::<Vec<_>>()
})
.map(|ixs| -> Result<Signature> {
let tx = Transaction::new_signed_with_payer(
&ixs,
Some(&self.payer_chain.pubkey()),
&[&self.payer_chain],
self.try_chain_blockhash()?,
);
chain_client
.send_and_confirm_transaction_with_spinner_and_config(
&tx,
self.commitment,
RpcSendTransactionConfig {
skip_preflight: true,
..Default::default()
},
)
.with_context(|| {
format!(
"Failed to delegate committees on chain '{:?}'",
tx.signatures[0]
)
})
})
.collect::<Result<Vec<_>>>()
}

// -----------------
Expand Down
Loading
Loading