Skip to content

compile instruction accounts via mock_compile_message#531

Open
buffalojoec wants to merge 5 commits into
firedancer-io:agave-v4.1.0-beta.1from
buffalojoec:mock-compile-message
Open

compile instruction accounts via mock_compile_message#531
buffalojoec wants to merge 5 commits into
firedancer-io:agave-v4.1.0-beta.1from
buffalojoec:mock-compile-message

Conversation

@buffalojoec

Copy link
Copy Markdown
Contributor

In Agave, top-level instruction accounts are queried from the compiled transaction Message, which compiles its account keys here (as CompiledKeys). When a message doesn't properly compile its account keys, duplicate entries are caught by Bank through this check. You can see this being tested here.

This means that a sanitized message in SVM's message processor is guaranteed to have no duplicate account keys. Due to the protocol-enforced layout of a Solana Message, no two instruction accounts sharing the same index can have different roles (writable/signer). This is because a Message uses an account key's index in its account_keys list to query its role. See is_signer and is_writable in the SDK.

In CPI, we don't have a Message layout to enforce uniqueness, but we know the transaction accounts have already been deduped (above). So, the program-runtime will merge a CPI instruction's instruction accounts to the highest role manually via prepare_next_cpi_instruction.

SolFuzz-Agave incorrectly builds the list of instruction accounts because it does not merge instruction accounts to their highest role. Instead, it assigns them whatever role was specified in the AccountMeta, meaning two accounts sharing the same index can have two different roles.

solfuzz-agave/src/instr.rs

Lines 233 to 242 in 4cb2fbc

for account_meta in acct_metas.iter() {
let index_in_transaction = txn_context
.find_index_of_account(&account_meta.pubkey)
.expect("invariant violation: account not found in transaction context")
as IndexOfAccount;
instruction_accounts.push(InstructionAccount::new(
index_in_transaction,
account_meta.is_signer,
account_meta.is_writable,
));

This PR patches this discrepancy and rectifies all input instruction accounts to resemble a valid message by actually using a real Message via mock_compile_message, a DCOU function from solana-program-runtime.

The issue is: this will break 100% of the instruction fixtures and all CPI syscall fixtures, since the syscall harness shares the same invoke context setup.

Hold the decoded instruction as a solana_instruction::Instruction. Lets the
harnesses pass it straight to mock_compile_message without rebuilding a copy.
Build the transaction accounts and instruction-account privileges through a
compiled Message (mock_compile_message + InvokeContext::prepare_top_level_
instructions) across the instr, vm_syscall and vm_serialization harnesses,
matching what the runtime does. Message compilation dedups account keys and
merges per-key privileges (is_signer/is_writable to the highest role), the
behavior the validator enforces before an instruction reaches the VM.

Drops the iterative get_instr_accounts / configure_top_level_instruction_for_tests
path and the per-call instruction snapshots/copies that fed it.

Note: this changes the returned account set/order (compiled-message accounts
only, message order) so resulting_accounts no longer echoes every input account
in input order; fixtures need regeneration to match.
Copilot AI review requested due to automatic review settings June 3, 2026 05:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates instruction and syscall execution setup to use Solana’s SanitizedMessage-based path (prepare_top_level_instructions) instead of the older test-only configure_top_level_instruction_for_tests, removing reliance on stable-layout instruction/account snapshots.

Changes:

  • Replace StableInstruction/stable-layout snapshots with solana_instruction::Instruction and SanitizedMessage.
  • Build a SanitizedMessage + transaction accounts via mock_compile_message and use prepare_top_level_instructions.
  • Extend VM syscall cleanup to drop a leaked SanitizedMessage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/vm_syscalls.rs Switches VM syscall execution to prepare_top_level_instructions and manages SanitizedMessage lifetime via leak + explicit cleanup.
src/vm_serialization.rs Updates VM serialization execution to use SanitizedMessage and prepare_top_level_instructions.
src/instr.rs Replaces stable-layout instruction types with Instruction, compiles a SanitizedMessage, and wires it into invoke context setup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/vm_syscalls.rs Outdated
Comment thread src/vm_serialization.rs
Comment thread src/vm_serialization.rs
Comment thread src/instr.rs Outdated
Comment thread src/vm_syscalls.rs Outdated
Comment thread src/instr.rs Outdated
Compiling the instruction via mock_compile_message builds the transaction
context the way the runtime does: only accounts referenced by the message are
included, deduped and reordered into message order. As a result the returned
modified_accounts dropped any input account the instruction didn't reference
and no longer matched the order they were provided.

Rebuild the result from the input accounts: emit every input account in input
order, using its post-execution version from the transaction context when
present and falling back to the provided input version otherwise. This restores
the full, input-ordered account set the fixtures/effects compare against.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants