Skip to content

Fix InstructionHolder scratch ownership - #165

Open
edo-chan wants to merge 3 commits into
mainfrom
codex/swi-483-instruction-holder-ownership
Open

Fix InstructionHolder scratch ownership#165
edo-chan wants to merge 3 commits into
mainfrom
codex/swi-483-instruction-holder-ownership

Conversation

@edo-chan

@edo-chan edo-chan commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR is stacked on codex/sign-v2-cu-comparison and fixes the InstructionHolder scratch lifetime issue without adding heap allocation.

Key changes:

  • replace returned slices into parser-local MaybeUninit arrays with a lending parser API: InstructionIterator::process_next
  • add InstructionScratch fixed buffers for AccountMeta, original account indexes, and CPI Account values
  • execute/inspect each parsed instruction inside the callback while scratch-backed slices are still valid
  • update SignV2, SOL/token destination parsing, and SubAccountSignV1 call sites to use the fixed-scratch callback flow
  • keep parse errors mapped at call sites while preserving callback permission/execution errors
  • add a focused InstructionHolder borrow test for scratch-backed account metadata

CU Benchmark

cargo test -p swig --test sign_v2_cu_comparison -- --nocapture

Case CU
All 2856
SOL limit only 4125
SOL limit + destination 4760
Token limit only 9559
Token limit + destination 10386

These match the current codex/sign-v2-cu-comparison results, so the fixed-scratch rewrite did not increase the LiteSVM CU harness numbers.

Validation

  • cargo fmt
  • cargo test -p swig-compact-instructions
  • cargo test -p swig --test sub_account_test test_sub_account_sign -- --nocapture
  • cargo test -p swig --test sign_v2 test_sign_v2_transfer_sol -- --nocapture
  • cargo test -p swig --test sol_destination_limit_v2 --test token_destination_limit_v2 -- --nocapture
  • cargo test -p swig --test sign_v2_cu_comparison -- --nocapture
  • git diff --check
  • cargo build-sbf --arch v1

Comment thread instructions/src/lib.rs Outdated
@edo-chan
edo-chan force-pushed the codex/swi-483-instruction-holder-ownership branch from e43fb7b to 5bfdbaf Compare June 17, 2026 01:30
@edo-chan edo-chan changed the title Fix InstructionHolder account metadata ownership Fix InstructionHolder scratch ownership Jun 17, 2026
@edo-chan
edo-chan changed the base branch from main to codex/sign-v2-cu-comparison June 17, 2026 01:30
@edo-chan

Copy link
Copy Markdown
Contributor Author

Rebased this on top of codex/sign-v2-cu-comparison and replaced the old Vec<AccountMeta> / Vec<usize> approach with fixed scratch for all parsed instruction buffers, including cpi_accounts.

CU rerun on the updated branch:

Case CU
All 2856
SOL limit only 4125
SOL limit + destination 4760
Token limit only 9559
Token limit + destination 10386

These match the current codex/sign-v2-cu-comparison numbers, so the fixed-scratch rewrite did not increase the LiteSVM CU harness results.

Validation run:

  • cargo fmt
  • cargo test -p swig-compact-instructions
  • cargo test -p swig --test sub_account_test test_sub_account_sign -- --nocapture
  • cargo test -p swig --test sign_v2 test_sign_v2_transfer_sol -- --nocapture
  • cargo test -p swig --test sol_destination_limit_v2 --test token_destination_limit_v2 -- --nocapture
  • cargo test -p swig --test sign_v2_cu_comparison -- --nocapture
  • git diff --check
  • cargo build-sbf --arch v1

@edo-chan
edo-chan requested a review from tracy-codes June 17, 2026 01:41
@edo-chan
edo-chan changed the base branch from codex/sign-v2-cu-comparison to main June 17, 2026 18:00
@tracy-codes

tracy-codes commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@edo-chan since this is a code change to the instruction parser, let's also test this branch with the litesvm test suite of the typescript sdk to ensure the typescript sdk remains compatible with the program and there's no edges or regressions.

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.

2 participants