Conversation
ananas-block
requested review from
SwenSchaeferjohann and
sergeytimoshin
as code owners
WalkthroughIntroduces a new CPI-context v2 system: adds zero-copy CPI context state/types, processing, and write paths; migrates discriminators; adds reinit instruction (feature-gated). Refactors account iteration/checks, expands error codes, and integrates profiling. Updates SDK and tests to new APIs, replaces small_ix with v2, and adjusts manifests/features. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant InvokingProgram
participant SystemProgram
participant CPIContextAccount
participant CompressionProgram
Note over Client,InvokingProgram: Invoke CPI with optional CPI context (v2)
Client->>InvokingProgram: build CpiInputs + Account metas (v2)
InvokingProgram->>SystemProgram: Instruction (Invoke)
activate SystemProgram
SystemProgram->>SystemProgram: Parse inputs (AccountIterator, Result<?>)
alt first_set_context or set_context
SystemProgram->>CPIContextAccount: set_cpi_context (store packed inputs/outputs)
CPIContextAccount-->>SystemProgram: Ok
else no context write
SystemProgram-->>SystemProgram: proceed without CPI context mutation
end
SystemProgram->>CompressionProgram: enqueue inputs/outputs/proofs
CompressionProgram-->>SystemProgram: Ok
opt CPI context outputs present
SystemProgram->>CPIContextAccount: copy_cpi_context_outputs(bytes)
CPIContextAccount-->>SystemProgram: Ok
end
SystemProgram-->>InvokingProgram: Result
deactivate SystemProgram
sequenceDiagram
autonumber
actor Operator
participant SystemProgram
participant CPIContextAccount(v1)
participant CPIContextAccount(v2)
Note over Operator,SystemProgram: Re-initialize legacy CPI context (feature "reinit")
Operator->>SystemProgram: ReInitCpiContextAccount
SystemProgram->>CPIContextAccount(v1): Deserialize legacy
CPIContextAccount(v1)-->>SystemProgram: associated_merkle_tree
SystemProgram->>CPIContextAccount(v2): cpi_context_account_new(RE_INIT=true, init params)
CPIContextAccount(v2)-->>SystemProgram: ZCpiContextAccount2
SystemProgram-->>Operator: Ok
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ananas-block
force-pushed
the
jorrit/refactor-cpi-context
branch
4 times, most recently
from
3179f80 to
e540210
Compare
ananas-block
force-pushed
the
jorrit/refactor-cpi-context
branch
2 times, most recently
from
37ae132 to
cee8fd4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
programs/system/src/processor/read_only_address.rs (1)
17-28: Unchecked index can panic; validate index and variant before use.
remaining_accounts[... as usize]will panic on out‑of‑bounds, allowing untrusted input to abort the program. Use checked conversion +.get_mut()and keep the existing error mapping.Apply:
- let merkle_tree = if let AcpAccount::BatchedAddressTree(tree) = - &mut remaining_accounts[read_only_address.address_merkle_tree_account_index as usize] - { - tree - } else { - msg!(format!( - "Read only address account is not a BatchedAddressTree {:?}", - read_only_address - ) - .as_str()); - return Err(SystemProgramError::InvalidAccount.into()); - }; + let idx = usize::try_from(read_only_address.address_merkle_tree_account_index) + .map_err(|_| SystemProgramError::InvalidAccount)?; + let merkle_tree = match remaining_accounts.get_mut(idx) { + Some(AcpAccount::BatchedAddressTree(tree)) => tree, + _ => { + msg!(format!( + "Read only address account is not a BatchedAddressTree {:?}", + read_only_address + ) + .as_str()); + return Err(SystemProgramError::InvalidAccount.into()); + } + };programs/system/src/processor/create_address_cpi_data.rs (1)
44-49: Avoid unwrap in on‑chain path. Return a typed error instead.
unwrap()can abort the program; prefer a fallible conversion.- context - .get_legacy_merkle_context(new_address_params.address_queue_index()) - .unwrap() - .rollover_fee, + context + .get_legacy_merkle_context(new_address_params.address_queue_index()) + .map(|ctx| ctx.rollover_fee) + .ok_or(ProgramError::InvalidArgument)?,program-libs/compressed-account/src/indexer_event/parse.rs (2)
376-399: Same guard needed for ACCOUNT_INFO path; fix comment and length checkMirror the READ_ONLY fix to avoid split_at panics and correct the comment.
- // Min len for a small instruction 4 accounts + 1 tree or queue - // Fee payer + authority + registered program + account compression program + account compression authority + // Min len incl. system_program: + // fee_payer + authority + registered_program + account_compression_program + + // account_compression_authority + system_program = 6 if accounts.len() < 5 { return Err(ParseIndexerEventError::DeserializeSystemInstructionError); } @@ - let system_accounts_len = if data.mode == 0 { + let system_accounts_len = if data.mode == 0 { 11 } else { - let mut len = 6; // fee_payer + authority + registered_program + account_compression_program + account_compression_authority + system_program + let mut len = 6; // fee_payer + authority + registered_program + account_compression_program + account_compression_authority + system_program if data.compress_or_decompress_lamports > 0 { len += 1; } if !data.is_compress && data.compress_or_decompress_lamports > 0 { len += 1; } if data.with_cpi_context { len += 1; } len }; - let accounts = accounts.split_at(system_accounts_len).1; + if accounts.len() < system_accounts_len { + return Err(ParseIndexerEventError::DeserializeSystemInstructionError); + } + let accounts = accounts.split_at(system_accounts_len).1;
329-351: Prevent panic: validate against computed system_accounts_len before split_atComputed system_accounts_len can be > 5 while the current guard only checks for 5; split_at(system_accounts_len) may panic. Add a guard against system_accounts_len and update the comment to reflect inclusion of system_program.
Apply:
- // Min len for a small instruction 3 accounts + 1 tree or queue - // Fee payer + authority + registered program + account compression program + account compression authority + // Min len incl. system_program: + // fee_payer + authority + registered_program + account_compression_program + + // account_compression_authority + system_program = 6 if accounts.len() < 5 { return Err(ParseIndexerEventError::DeserializeSystemInstructionError); } @@ - let system_accounts_len = if data.mode == 0 { + let system_accounts_len = if data.mode == 0 { 11 } else { - let mut len = 6; // fee_payer + authority + registered_program + account_compression_program + account_compression_authority + system_program + let mut len = 6; // fee_payer + authority + registered_program + account_compression_program + account_compression_authority + system_program if data.compress_or_decompress_lamports > 0 { len += 1; } if !data.is_compress && data.compress_or_decompress_lamports > 0 { len += 1; } if data.with_cpi_context { len += 1; } len }; + if accounts.len() < system_accounts_len { + return Err(ParseIndexerEventError::DeserializeSystemInstructionError); + } let accounts = accounts.split_at(system_accounts_len).1;Optional: confirm the differing is_compress gating vs INVOKE/INVOKE_CPI is intentional.
programs/system/src/processor/sum_check.rs (1)
7-13: Profiling is enabled unconditionally — gate/remove #[profile] on hot-paths (sum_check).Findings: #[profile] is applied to programs/system/src/processor/sum_check.rs (lines 7–13) and appears across programs/system/src (e.g., programs/system/src/utils.rs, programs/system/src/cpi_context/state.rs, programs/system/src/processor/process.rs). No Cargo feature or #[cfg] gating was found.
Action required:
- Apply conditional attribute: #[cfg_attr(feature = "light_program_profiler", profile)] and add a corresponding Cargo feature (or make the profiler a no-op in release builds).
- Remove or avoid #[profile] on critical hot-path functions like sum_check, or ensure profiling is disabled in production; run targeted microbenchmarks to confirm no runtime impact.
program-tests/utils/src/assert_rollover.rs (2)
154-156: Fix unsatisfiable lamports-conservation assertion in program-tests/utils/src/assert_rollover.rsFile: program-tests/utils/src/assert_rollover.rs (lines 154–156) — the current assertion reduces to 0 == new_merkle_tree_lamports + new_queue_lamports (always false). Replace RHS to exclude the duplicated old value:
- assert_eq!( - old_merkle_tree_lamports, - new_merkle_tree_lamports + new_queue_lamports + old_merkle_tree_lamports - ); + assert_eq!( + old_merkle_tree_lamports, + new_merkle_tree_lamports + new_queue_lamports + );If rollover/network fees apply, include expected fees in the RHS or pass expected fee lamports into this helper and assert equality including fees.
118-122: Remove ambiguous comment and clarify index invariant. Replace the// Isn’t this wrong???line with a clear note—e.g.// index must be preserved across queue rolloverprograms/system/src/processor/create_outputs_cpi_data.rs (1)
172-185: Bug: removing fromcontext.addressesuses index from filtered iterator.
position()afterfilter(|x| x.is_some())returns the index within the filtered view, not the original vector, soremove(position)can delete the wrong element whenever there are leadingNones. Use an index over the original iterator.- if let Some(address) = account.address() { - if let Some(position) = context - .addresses - .iter() - .filter(|x| x.is_some()) - .position(|&x| x.unwrap() == address) - { - context.addresses.remove(position); - } else { - msg!(format!("context.addresses: {:?}", context.addresses).as_str()); - return Err(SystemProgramError::InvalidAddress.into()); - } - } + if let Some(address) = account.address() { + if let Some(position) = context + .addresses + .iter() + .position(|x| matches!(x, Some(a) if *a == address)) + { + context.addresses.remove(position); + } else { + msg!(format!("context.addresses len: {}", context.addresses.len()).as_str()); + return Err(SystemProgramError::InvalidAddress.into()); + } + }program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
525-767: Remove noisy debug prints in the randomized test; they will flood CI logs.Printing full structs and serialized vectors for 1000 iters is excessive.
Apply:
- println!("\n\ne2e test seed {}\n\n", seed); + eprintln!("e2e test seed {}", seed); @@ - println!("value {:?}", value); - println!("vec {:?}", vec); + // Uncomment for targeted debugging: + // eprintln!("value: {:?}\nvec: {:?}", value, vec);Optionally reduce
num_iterslocally or gate verbose output behind an env var.anchor-programs/system/src/cpi_context_account.rs (1)
50-72: Return type mismatch between function signature and implementationThe
deserialize_cpi_context_accountfunction returnsZCpiContextAccount2<'a>which includes acontextfield (line 47), but this field is not defined in theZCpiContextAccount2struct declaration (lines 44-48). This appears to be a compilation error.The struct definition and the deserialization return value don't match. Add the missing
contextfield to the struct:pub struct ZCpiContextAccount2<'a> { pub fee_payer: Ref<&'a mut [u8], light_compressed_account::pubkey::Pubkey>, pub associated_merkle_tree: Ref<&'a mut [u8], light_compressed_account::pubkey::Pubkey>, + pub context: Vec<ZInstructionDataInvokeCpi<'a>>, }programs/system/src/processor/cpi.rs (1)
22-33: Update create_cpi_data_and_context call in process.rs
The call at programs/system/src/processor/process.rs:107 currently passes onlyctxandnum_output_compressed_accounts as u8; it must be updated to include the new parameters—num_leaves,num_nullifiers,num_new_addresses,hashed_pubkeys_capacity,cpi_data_len,invoking_program_id, andremaining_accounts.programs/system/src/processor/create_inputs_cpi_data.rs (1)
77-84: Remove unwrap; return a proper error instead of panicking on-chain
get_legacy_merkle_context(...).unwrap()can panic if the context is missing. Return a program error like other branches.- AcpAccount::StateTree(_) => { - is_batched = false; - context - .get_legacy_merkle_context(current_mt_index) - .unwrap() - .hashed_pubkey - } + AcpAccount::StateTree(_) => { + is_batched = false; + let legacy = context + .get_legacy_merkle_context(current_mt_index) + .ok_or(SystemProgramError::InputMerkleTreeIndexOutOfBounds)?; + legacy.hashed_pubkey + }
| pub fn re_init_cpi_context_account(ctx: Context<InitializeCpiContextAccount>) -> Result<()> { | ||
| unimplemented!("anchor wrapper not implemented") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate unimplemented Anchor wrappers to avoid accidental on-chain panics.
Since these handlers are unimplemented!(), ensure they can’t be compiled into a deployed SBF build.
Apply per-function gating now, and consider gating the entire #[program] block similarly:
+#[cfg(feature = "idl-build")]
pub fn re_init_cpi_context_account(ctx: Context<InitializeCpiContextAccount>) -> Result<()> {
unimplemented!("anchor wrapper not implemented")
}Additionally (outside this hunk), add at crate root:
#[cfg(all(target_os = "solana"), not(feature = "idl-build"))]
compile_error!("Anchor wrappers must not be built for on-chain use; enable feature `idl-build` only for IDL generation.");Incorrect context type for re-init instruction.
This should use the re-init accounts context, not InitializeCpiContextAccount. Otherwise the IDL and account constraints for re-init will be wrong.
Apply:
-pub fn re_init_cpi_context_account(ctx: Context<InitializeCpiContextAccount>) -> Result<()> {
+pub fn re_init_cpi_context_account(ctx: Context<ReInitCpiContextAccount>) -> Result<()> {
unimplemented!("anchor wrapper not implemented")
}Please replace ReInitCpiContextAccount with the exact type name defined in instructions::* if it differs.
📝 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.
| pub fn re_init_cpi_context_account(ctx: Context<InitializeCpiContextAccount>) -> Result<()> { | |
| unimplemented!("anchor wrapper not implemented") | |
| } | |
| pub fn re_init_cpi_context_account(ctx: Context<ReInitCpiContextAccount>) -> Result<()> { | |
| unimplemented!("anchor wrapper not implemented") | |
| } |
🤖 Prompt for AI Agents
In anchor-programs/system/src/lib.rs around lines 28 to 30, the
re_init_cpi_context_account function is using the wrong context type
(Context<InitializeCpiContextAccount>); replace it with the re-init accounts
context type defined in instructions::* (e.g., Context<ReInitCpiContextAccount>
or the exact name in instructions), update any imports to bring that type into
scope, and adjust any references so the function signature and IDL reflect the
correct re-init account constraints.
| profile-program = [] | ||
| profile-heap = ["dep:light-heap"] | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Gate profiling deps; avoid unconditional pulls.
Suggest feature-gating profiler/logging crates to keep minimal dependency sets for non‑profile builds.
[features]
default = []
@@
-profile-program = []
+profile-program = ["dep:solana-msg", "dep:light-program-profiler"]
profile-heap = ["dep:light-heap"]
@@
-light-zero-copy = { workspace = true, features = ["std", "mut", "derive"] }
+light-zero-copy = { workspace = true, features = ["std", "mut", "derive"] }
@@
-solana-msg = { workspace = true }
+solana-msg = { workspace = true, optional = true }
@@
-light-program-profiler = { workspace = true }
+light-program-profiler = { workspace = true, optional = true }
light-heap = { workspace = true, optional = true }Verify usage is behind the feature:
Also applies to: 23-24, 27-27, 33-34
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\b(solana_msg!|profile!\(|light_program_profiler)\b'Length of output: 8511
Gate profiling imports and macro calls behind the profile-program feature
Wrap every use light_program_profiler::profile, any profile!(…) invocation, and the use solana_msg::msg import in all crates (including program-libs/compressed-account/src/instruction_data/zero_copy_set.rs and every programs/system/... file) with #[cfg(feature = "profile-program")] so that optional dependencies remain unused when the feature is disabled.
| fn cpi_context(&self) -> Option<CompressedCpiContext>; | ||
| fn bump(&self) -> Option<u8>; | ||
| fn account_option_config(&self) -> AccountOptions; | ||
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError>; |
There was a problem hiding this comment.
🧩 Analysis chain
Good: account_option_config now fallible.
Returning Result<AccountOptions, CompressedAccountError> enables early validation (e.g., CPI‑context presence). Implementations in with_account_info/with_readonly look consistent.
Please confirm all implementers updated (including zero_copy.rs stubs) and call sites handle errors rather than assuming success.
🏁 Script executed:
#!/bin/bash
rg -nP 'fn\s+account_option_config\(' -C2 program-libs/compressed-account/src
rg -nP 'account_option_config\(\)' -C2 --type=rustLength of output: 15703
Fix remaining unimplemented account_option_config() in zero_copy.rs; ensure callers don't unwrap
- Unimplemented stub at program-libs/compressed-account/src/instruction_data/zero_copy.rs:442 — implement it or make it unreachable.
- Production call-site programs/system/src/lib.rs uses inputs.account_option_config()? (errors are propagated).
- Many tests call .account_option_config().unwrap() (acceptable in tests); remove any non-test .unwrap() usages.
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | ||
| unimplemented!() | ||
| } |
There was a problem hiding this comment.
Unimplemented account_option_config() will panic at runtime if invoked.
Provide a minimal, correct implementation rather than unimplemented!().
Apply this diff:
- fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> {
- unimplemented!()
- }
+ fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> {
+ let sol_pool_pda = self.compress_or_decompress_lamports().is_some();
+ let decompression_recipient = sol_pool_pda && !self.is_compress();
+ Ok(AccountOptions {
+ sol_pool_pda,
+ decompression_recipient,
+ cpi_context_account: false,
+ write_to_cpi_context: false,
+ })
+ }📝 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.
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | |
| unimplemented!() | |
| } | |
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | |
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | |
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | |
| Ok(AccountOptions { | |
| sol_pool_pda, | |
| decompression_recipient, | |
| cpi_context_account: false, | |
| write_to_cpi_context: false, | |
| }) | |
| } |
🤖 Prompt for AI Agents
In program-libs/compressed-account/src/instruction_data/zero_copy.rs around
lines 442–444, replace the unimplemented!() with a real implementation that
constructs and returns an AccountOptions based on this struct's fields and
returns Result<AccountOptions, CompressedAccountError>; specifically map the
instance's signer/writable/optional/public-key related fields into the
AccountOptions struct, perform any necessary validation, and on success return
Ok(account_options) or on validation/conversion failure return the appropriate
CompressedAccountError via Err(...).
| account-compression = { workspace = true, features = ["cpi"] } | ||
| light-compressed-account = { workspace = true, features = ["anchor"] } | ||
| light-sdk = { workspace = true, features = ["anchor", "v2", "small_ix"] } | ||
| light-sdk = { workspace = true, features = ["anchor", "v2"] } |
There was a problem hiding this comment.
🧩 Analysis chain
Removed small_ix feature — looks aligned with CPI-context refactor; verify no lingering gates.
Change matches the PR direction to consolidate around the v2 path. Please confirm no crates/tests still gate code on cfg(feature = "small_ix") or expect the smaller IX layout.
Run this script to detect any remaining references and to inspect light-sdk’s declared features:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Look for code gated by feature \"small_ix\""
rg -n --hidden -S -g '!**/target/**' -C2 '\bcfg\s*\(\s*feature\s*=\s*"small_ix"\s*\)'
echo
echo "2) Look for manifests enabling \"small_ix\""
rg -n --hidden -S -g '**/Cargo.toml' -C2 '"small_ix"'
echo
echo "3) Inspect light-sdk features"
LIGHTSDK=$(rg -nPl --hidden -g '**/Cargo.toml' '^\s*name\s*=\s*"light-sdk"\s*$' | head -n1)
echo "light-sdk manifest: ${LIGHTSDK:-<not found>}"
if [[ -n "${LIGHTSDK:-}" ]]; then
awk 'f{if($0 ~ /^\[/){exit} print} /\[features\]/{f=1}' "$LIGHTSDK"
fi
echo
echo "4) Ensure this test program does not reference small_ix symbols"
rg -n -S -g 'program-tests/create-address-test-program/**' -C2 '(?i)\bsmall[_-]?ix\b|SmallIx'
echo
echo "Expected: no matches in (1), (2), (4); (3) shows anchor/v2 paths sufficient without small_ix."Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root: $(pwd)"
echo
echo "1) Search for token occurrences of 'small_ix' / 'small-ix' / 'SmallIx'"
rg -n -uu -S -C3 '\bsmall[_-]?ix\b' || true
echo
echo "2) Search for cfg(...) feature uses that mention small_ix"
rg -n -uu -S -C3 '\bcfg\s*\([^)]*small[_-]?ix[^)]*\)' || true
echo
echo "3) Search Cargo.toml files for feature entries named small_ix"
rg -n -uu -S -C3 '"small_ix"' --glob '**/Cargo.toml' || true
echo
echo "4) Locate light-sdk Cargo.toml and print its [features] block (first match)"
LIGHTSDK_FILE=$(rg -n -uu -S --hidden --glob '**/Cargo.toml' '^\s*name\s*=\s*"light-sdk"\s*$' | sed -n '1p' | cut -d: -f1 || true)
echo "light-sdk manifest: ${LIGHTSDK_FILE:-<not found>}"
if [[ -n "${LIGHTSDK_FILE:-}" ]]; then
awk 'f{if($0 ~ /^\[/){exit} print} /\[features\]/{f=1}' "$LIGHTSDK_FILE" || true
fi
echo
echo "5) Check the test program tree for small_ix mentions"
rg -n -uu -S -C3 '(?i)\bsmall[_-]?ix\b|SmallIx' program-tests/create-address-test-program/** || true
echo
echo "Search complete."Length of output: 14948
Remaining runtime references to small_ix found — update tests/dev script or restore equivalent API.
- No compile-time gating found: no
cfg(feature = "small_ix")matches and no Cargo.toml with"small_ix"(feature removal confirmed). - Runtime/API references still remain and must be updated:
- scripts/devenv.sh — exports CARGO_FEATURES="${CARGO_FEATURES:-small_ix}"
- program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs — multiple uses of
small_ix,config.small_ix, branches, and mode selection - program-tests/create-address-test-program/src/lib.rs —
small_ixfn param,light_sdk::cpi::CpiAccountsSmallusage, and instruction fieldsmall_ix
- Fix: remove or update these references to the removed feature (or reintroduce/alias an equivalent in light-sdk), then run CI/compile/tests to verify no regressions.
| let mut account_data = account_info | ||
| .try_borrow_mut_data() | ||
| .map_err(|_| SystemProgramError::BorrowingDataFailed)?; | ||
| // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid | ||
| let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) }; | ||
| let (discriminator, data) = data.split_at_mut(8); | ||
| if discriminator != CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR { | ||
| msg!("Invalid cpi context account discriminator."); | ||
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | ||
| } |
There was a problem hiding this comment.
Guard against short account data and avoid unnecessary unsafe.
split_at_mut(8) will panic on short data; switch to a bounds check and avoid raw-parts construction.
- let mut account_data = account_info
- .try_borrow_mut_data()
- .map_err(|_| SystemProgramError::BorrowingDataFailed)?;
- // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid
- let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) };
- let (discriminator, data) = data.split_at_mut(8);
+ let mut account_data = account_info
+ .try_borrow_mut_data()
+ .map_err(|_| SystemProgramError::BorrowingDataFailed)?;
+ let data: &mut [u8] = &mut *account_data;
+ if data.len() < 8 {
+ msg!("Cpi context account data too small.");
+ return Err(ProgramError::InvalidAccountData);
+ }
+ let (discriminator, data) = data.split_at_mut(8);📝 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.
| let mut account_data = account_info | |
| .try_borrow_mut_data() | |
| .map_err(|_| SystemProgramError::BorrowingDataFailed)?; | |
| // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid | |
| let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) }; | |
| let (discriminator, data) = data.split_at_mut(8); | |
| if discriminator != CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } | |
| let mut account_data = account_info | |
| .try_borrow_mut_data() | |
| .map_err(|_| SystemProgramError::BorrowingDataFailed)?; | |
| let data: &mut [u8] = &mut *account_data; | |
| if data.len() < 8 { | |
| msg!("Cpi context account data too small."); | |
| return Err(ProgramError::InvalidAccountData); | |
| } | |
| let (discriminator, data) = data.split_at_mut(8); | |
| if discriminator != CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } |
🤖 Prompt for AI Agents
In programs/system/src/cpi_context/state.rs around lines 294 to 303, the code
uses unsafe slice::from_raw_parts_mut and split_at_mut(8) which can panic on
short account data; replace this with a safe bounds check and non-unsafe
slicing: first check account_data.len() >= 8 and return
SystemProgramError::BorrowingDataFailed or InvalidCpiContextDiscriminator (per
existing semantics) if too short, then obtain a safe mutable slice (no raw
pointers) and compare the first 8 bytes against
CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR; remove the unsafe block entirely and use
safe slice indexing/splitting to avoid panics and unsoundness.
| // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid | ||
| let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) }; | ||
| let (discriminator, data) = data.split_at_mut(8); | ||
| if RE_INIT { | ||
| // Check discriminator matches | ||
| if discriminator != CPI_CONTEXT_ACCOUNT_1_DISCRIMINATOR { | ||
| msg!("Invalid cpi context account discriminator."); | ||
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | ||
| } | ||
| // Zero out account data | ||
| data.fill(0); | ||
| } else if discriminator != [0u8; 8] { | ||
| msg!("Invalid cpi context account discriminator."); | ||
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | ||
| } | ||
| discriminator.copy_from_slice(&CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR); |
There was a problem hiding this comment.
Same here: short-data guard and safe slice for init path.
- // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid
- let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) };
- let (discriminator, data) = data.split_at_mut(8);
+ let data: &mut [u8] = &mut *account_data;
+ if data.len() < 8 {
+ msg!("Cpi context account data too small.");
+ return Err(ProgramError::InvalidAccountData);
+ }
+ let (discriminator, data) = data.split_at_mut(8);📝 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.
| // SAFETY: account_data is a valid RefMut<[u8]>, pointer and length are valid | |
| let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) }; | |
| let (discriminator, data) = data.split_at_mut(8); | |
| if RE_INIT { | |
| // Check discriminator matches | |
| if discriminator != CPI_CONTEXT_ACCOUNT_1_DISCRIMINATOR { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } | |
| // Zero out account data | |
| data.fill(0); | |
| } else if discriminator != [0u8; 8] { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } | |
| discriminator.copy_from_slice(&CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR); | |
| let data: &mut [u8] = &mut *account_data; | |
| if data.len() < 8 { | |
| msg!("Cpi context account data too small."); | |
| return Err(ProgramError::InvalidAccountData); | |
| } | |
| let (discriminator, data) = data.split_at_mut(8); | |
| if RE_INIT { | |
| // Check discriminator matches | |
| if discriminator != CPI_CONTEXT_ACCOUNT_1_DISCRIMINATOR { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } | |
| // Zero out account data | |
| data.fill(0); | |
| } else if discriminator != [0u8; 8] { | |
| msg!("Invalid cpi context account discriminator."); | |
| return Err(SystemProgramError::InvalidCpiContextDiscriminator.into()); | |
| } | |
| discriminator.copy_from_slice(&CPI_CONTEXT_ACCOUNT_2_DISCRIMINATOR); |
🤖 Prompt for AI Agents
In programs/system/src/cpi_context/state.rs around lines 416 to 431, add a
short-data guard and avoid using from_raw_parts_mut directly on account_data for
the init path: first check account_data.len() is at least 8 + expected payload
length and return an error if too short; then safely obtain mutable slices
(e.g., split_at_mut) from the safe RefMut (or use safe methods provided by the
account data wrapper) instead of raw pointer arithmetic; ensure the init branch
zeroes only the payload slice and that discriminator checks/assignments operate
on the validated 8-byte slice; keep the existing discriminator error returns but
perform length validation before any unsafe slice creation.
| let registered_program_pda = accounts.next_non_mut("registered_program_pda")?; | ||
|
|
||
| let authority = check_authority(accounts.next())?; | ||
| let account_compression_authority = | ||
| accounts.next_non_mut("account_compression_authority")?; | ||
| let account_compression_program = | ||
| accounts.next_non_mut("account_compression_program")?; | ||
|
|
||
| let registered_program_pda = check_non_mut_account_info(accounts.next())?; | ||
| let system_program = accounts.next_non_mut("system_program")?; | ||
|
|
There was a problem hiding this comment.
Validate program IDs for account_compression_program and system_program.
next_non_mut() only checks mutability; it doesn’t verify the passed accounts are the expected programs. This weakens authZ and could allow spoofed program accounts to slip through. Mirror invoke.rs by asserting identities.
Apply this diff:
use crate::{
accounts::{
account_checks::{
- check_option_cpi_context_account, check_option_decompression_recipient,
- check_option_sol_pool_pda,
+ check_option_cpi_context_account, check_option_decompression_recipient,
+ check_option_sol_pool_pda, check_account_compression_program, check_system_program,
},
account_traits::{CpiContextAccountTrait, InvokeAccounts, SignerAccounts},
},
errors::SystemProgramError,
Result,
};
@@
- let account_compression_program =
- accounts.next_non_mut("account_compression_program")?;
+ let account_compression_program = {
+ let acc = accounts.next_non_mut("account_compression_program")?;
+ check_account_compression_program(acc)?;
+ acc
+ };
@@
- let system_program = accounts.next_non_mut("system_program")?;
+ let system_program = {
+ let acc = accounts.next_non_mut("system_program")?;
+ check_system_program(acc)?;
+ acc
+ };📝 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.
| let registered_program_pda = accounts.next_non_mut("registered_program_pda")?; | |
| let authority = check_authority(accounts.next())?; | |
| let account_compression_authority = | |
| accounts.next_non_mut("account_compression_authority")?; | |
| let account_compression_program = | |
| accounts.next_non_mut("account_compression_program")?; | |
| let registered_program_pda = check_non_mut_account_info(accounts.next())?; | |
| let system_program = accounts.next_non_mut("system_program")?; | |
| use crate::{ | |
| accounts::{ | |
| account_checks::{ | |
| check_option_cpi_context_account, check_option_decompression_recipient, | |
| check_option_sol_pool_pda, check_account_compression_program, check_system_program, | |
| }, | |
| account_traits::{CpiContextAccountTrait, InvokeAccounts, SignerAccounts}, | |
| }, | |
| errors::SystemProgramError, | |
| Result, | |
| }; | |
| let registered_program_pda = accounts.next_non_mut("registered_program_pda")?; | |
| let account_compression_authority = | |
| accounts.next_non_mut("account_compression_authority")?; | |
| let account_compression_program = { | |
| let acc = accounts.next_non_mut("account_compression_program")?; | |
| check_account_compression_program(acc)?; | |
| acc | |
| }; | |
| let system_program = { | |
| let acc = accounts.next_non_mut("system_program")?; | |
| check_system_program(acc)?; | |
| acc | |
| }; |
| let remaining_accounts = if !account_options.write_to_cpi_context { | ||
| accounts.remaining()? | ||
| } else { | ||
| &[] | ||
| }; |
There was a problem hiding this comment.
remaining() will Err on zero remainder; use remaining_unchecked().
AccountIterator::remaining() returns NotEnoughAccountKeys when none remain, which will spuriously fail in the common case of exactly consumed inputs.
Apply this diff:
- let remaining_accounts = if !account_options.write_to_cpi_context {
- accounts.remaining()?
+ let remaining_accounts = if !account_options.write_to_cpi_context {
+ accounts.remaining_unchecked()?
} else {
&[]
};📝 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.
| let remaining_accounts = if !account_options.write_to_cpi_context { | |
| accounts.remaining()? | |
| } else { | |
| &[] | |
| }; | |
| let remaining_accounts = if !account_options.write_to_cpi_context { | |
| accounts.remaining_unchecked()? | |
| } else { | |
| &[] | |
| }; |
🤖 Prompt for AI Agents
In programs/system/src/invoke_cpi/instruction_small.rs around lines 78 to 82,
the code calls accounts.remaining() which returns an Err when no accounts
remain; replace this with accounts.remaining_unchecked() to avoid spurious
NotEnoughAccountKeys errors when inputs are exactly consumed, ensuring you
preserve the existing branch logic (returning &[] when write_to_cpi_context is
true) and update any error handling/comments accordingly.
| ReInitCpiContextAccount, | ||
| } |
There was a problem hiding this comment.
Build break when reinit feature is disabled: enum variant is unconditional but match arm is gated.
With reinit off, the match is non‑exhaustive. Gate the enum variant too (or add a fallback arm under not(feature="reinit")).
pub enum InstructionDiscriminator {
InitializeCpiContextAccount,
Invoke,
InvokeCpi,
InvokeCpiWithReadOnly,
InvokeCpiWithAccountInfo,
- ReInitCpiContextAccount,
+ #[cfg(feature = "reinit")]
+ ReInitCpiContextAccount,
}
@@
- #[cfg(feature = "reinit")]
- InstructionDiscriminator::ReInitCpiContextAccount => reinit_cpi_context_account(accounts),
+ #[cfg(feature = "reinit")]
+ InstructionDiscriminator::ReInitCpiContextAccount => reinit_cpi_context_account(accounts),
+ // Optional: if you prefer keeping the variant always present, add this instead:
+ // #[cfg(not(feature = "reinit"))]
+ // InstructionDiscriminator::ReInitCpiContextAccount => return Err(ProgramError::InvalidInstructionData),Also applies to: 87-89
🤖 Prompt for AI Agents
In programs/system/src/lib.rs around lines 56-57 (and similarly at 87-89) an
enum variant ReInitCpiContextAccount is declared unconditionally while its match
arm is gated by #[cfg(feature = "reinit")], causing non‑exhaustive match when
the feature is disabled; fix by gating the enum variant with the same
#[cfg(feature = "reinit")] attribute (or alternatively add a #[cfg(not(feature =
"reinit"))] fallback match arm) so declaration and matching are consistent and
compilation succeeds with the feature off.
cee8fd4 to
281bf48
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
programs/system/src/processor/cpi.rs (2)
45-52: Bug: truncation before min (len() as u8).Casting remaining_accounts.len() to u8 before min can undercount when len > 255 (e.g., 300 → 44). Min in usize first, then cast.
- let bytes_size = InsertIntoQueuesInstructionDataMut::required_size_for_capacity( - num_leaves, - num_nullifiers, - num_new_addresses, - min(remaining_accounts.len() as u8, num_leaves), - min(remaining_accounts.len() as u8, num_nullifiers), - min(remaining_accounts.len() as u8, num_new_addresses), - ); + let rem_len = remaining_accounts.len(); + let capped_leaves = rem_len.min(num_leaves as usize) as u8; + let capped_nullifiers = rem_len.min(num_nullifiers as usize) as u8; + let capped_new_addrs = rem_len.min(num_new_addresses as usize) as u8; + let bytes_size = InsertIntoQueuesInstructionDataMut::required_size_for_capacity( + num_leaves, + num_nullifiers, + num_new_addresses, + capped_leaves, + capped_nullifiers, + capped_new_addrs, + );
39-41: AccountMeta param order is (pubkey, is_signer, is_writable) — flip the args.
AccountMeta::new(account_infos[0].key(), false, true) sets is_signer=false and is_writable=true (writable but not signer); change to AccountMeta::new(account_infos[0].key(), true, false) to make the PDA a signer but not writable.
File: programs/system/src/processor/cpi.rs lines 39–41.programs/system/src/accounts/remaining_account_checks.rs (2)
120-128: OOB risk: slice uses fixed LEN without checking data lengthBefore slicing [8..StateMerkleTreeAccount::LEN], ensure data.len() >= StateMerkleTreeAccount::LEN to avoid panics on malformed accounts.
- let merkle_tree = bytemuck::from_bytes::<StateMerkleTreeAccount>( - &data[8..StateMerkleTreeAccount::LEN], - ); + if data.len() < StateMerkleTreeAccount::LEN { + return Err(SystemProgramError::InvalidAccount); + } + let merkle_tree = bytemuck::from_bytes::<StateMerkleTreeAccount>(&data[8..StateMerkleTreeAccount::LEN]);
164-167: Same OOB risk for AddressMerkleTreeAccountAdd the same length check for AddressMerkleTreeAccount::LEN.
- let merkle_tree = bytemuck::from_bytes::<AddressMerkleTreeAccount>( - &data[8..AddressMerkleTreeAccount::LEN], - ); + if data.len() < AddressMerkleTreeAccount::LEN { + return Err(SystemProgramError::InvalidAccount); + } + let merkle_tree = bytemuck::from_bytes::<AddressMerkleTreeAccount>(&data[8..AddressMerkleTreeAccount::LEN]);program-libs/compressed-account/src/instruction_data/data.rs (1)
98-106: PackedReadOnlyAddress likely incurs tail padding (36B) while Borsh encodes 35Brepr(C) with [u8;32], u16, u8 → size_of = 36 due to tail padding to 2‑byte alignment. Anchor/Borsh will serialize 35 bytes. If this struct is used for zero‑copy over serialized data, that’s a mismatch. Either:
- avoid ZeroCopyMut here and keep it purely serialized, or
- introduce an explicit padding byte in both layout and serialization, or
- split into “in‑memory” vs “serialized” representations.
Add a test to catch regressions.
+#[cfg(test)] +mod layout_tests_packed_readonly { + use super::*; + #[test] + fn size_is_multiple_of_alignment() { + assert_eq!(core::mem::size_of::<PackedReadOnlyAddress>(), 36); + } +}anchor-programs/system/src/cpi_context_account.rs (1)
50-72: Fix unsafe zero-copy deserialization (prevent panics/UB).
- Do not unwrap the borrow; replace account_info.try_borrow_mut_data().unwrap() with proper error mapping/return.
- Validate buffer length (>= 8) and check the discriminator before indexing/slicing (&mut data[8..]) to avoid out‑of‑bounds panics.
- Avoid unchecked unsafe slice::from_raw_parts_mut where possible; use split_at_mut or add a precise SAFETY comment documenting the invariants that guarantee pointer+length validity.
- Audit and fix other occurrences of this pattern found in the repo: anchor-programs/system/src/cpi_context_account.rs (deserialize_cpi_context_account), programs/system/src/cpi_context/state.rs:298,417; programs/system/src/accounts/remaining_account_checks.rs:146,176; program-libs/batched-merkle-tree/src/merkle_tree.rs:165; programs/account-compression/src/context.rs:192,212; programs/account-compression/src/instructions/migrate_state.rs:205.
program-libs/compressed-account/src/instruction_data/zero_copy.rs (1)
487-489: cpi_context() should return None, not unimplemented!().Current code will panic when invoked.
Apply:
- fn cpi_context(&self) -> Option<CompressedCpiContext> { - unimplemented!() - } + fn cpi_context(&self) -> Option<CompressedCpiContext> { + None + }
🧹 Nitpick comments (43)
program-libs/zero-copy/src/vec.rs (1)
146-153: zero_out(): add trait bound; consider zeroing used region only
- Add a method-level bound so this compiles regardless of the impl block bounds.
- Optional: zero only the used portion to avoid O(capacity) work on large buffers.
- pub fn zero_out(&mut self) { + pub fn zero_out(&mut self) + where + T: IntoBytes, + { *self.get_len_mut() = 0 .try_into() .map_err(|_| ZeroCopyError::InvalidConversion) .unwrap(); - self.slice.as_mut_bytes().fill(0); + // Optional perf: wipe only the active region + // self.as_mut_slice().as_mut_bytes().fill(0); + // Current behavior: wipe entire capacity + self.slice.as_mut_bytes().fill(0); }programs/system/tests/cpi_context.rs (2)
132-145: Consider makingcalculate_vector_offsetspublic for reusabilityThis helper function calculates offsets for zero-copy vectors and could be useful in other test modules. Consider making it public rather than private inline if other modules need similar offset calculations.
327-332: Document the purpose ofclean_input_dataThe function clears specific fields but doesn't document why these particular fields need cleaning. Adding a brief doc comment would help clarify its role in test data preparation.
+/// Clears optional fields that should not be persisted in CPI context fn clean_input_data(instruction_data: &mut InstructionDataInvokeCpi) { instruction_data.cpi_context = None; instruction_data.compress_or_decompress_lamports = None; instruction_data.relay_fee = None; instruction_data.proof = None; }programs/system/src/processor/cpi.rs (8)
7-7: Gate profiling import behind a feature.Avoid unconditional dependency/overhead in production builds.
-use light_program_profiler::profile; +#[cfg(feature = "profile")] +use light_program_profiler::profile;
22-22: Gate the profiling attribute.Keeps profiling opt‑in.
-#[profile] +#[cfg_attr(feature = "profile", profile)]
82-82: Same here: gate the profiling attribute on the CPI invoke.-#[profile] +#[cfg_attr(feature = "profile", profile)]
42-42: Overflow risk when summing u8s for capacity.
num_nullifiers + num_leaves + num_new_addressescan overflow u8 (wraps in release, panics in debug). Compute in usize.- let account_indices = - Vec::<u8>::with_capacity((num_nullifiers + num_leaves + num_new_addresses) as usize); + let total_indices = + usize::from(num_nullifiers) + usize::from(num_leaves) + usize::from(num_new_addresses); + let account_indices = Vec::<u8>::with_capacity(total_indices);
56-60: Consider centralizing the 10KB constant.Move MAX_CPI_BUFFER_SIZE to a shared constants module to keep on‑chain/off‑chain limits in sync.
64-64: Avoid unwrap on length conversion.Safer and self‑documenting given the bound above.
- bytes[8..12].copy_from_slice(&u32::try_from(byte_len - 12).unwrap().to_le_bytes()); + debug_assert!(byte_len >= 12 && byte_len <= MAX_CPI_BUFFER_SIZE); + let vec_len = u32::try_from(byte_len - 12).expect("bounded by MAX_CPI_BUFFER_SIZE"); + bytes[8..12].copy_from_slice(&vec_len.to_le_bytes());
71-71: Same overflow concern for addresses capacity.Sum in usize to avoid u8 wrap.
- addresses: Vec::with_capacity((num_nullifiers + num_new_addresses) as usize), + addresses: Vec::with_capacity( + usize::from(num_nullifiers) + usize::from(num_new_addresses), + ),
22-33: Introduce an args struct for create_cpi_data_and_context. Reduces call‑site brittleness and improves readability. One call site found: programs/system/src/processor/process.rs:107 — update that invocation to pass the new args struct.sdk-libs/sdk-pinocchio/src/cpi/mod.rs (1)
2-8: Remove commented code or guard with cfgCommented-out accounts_small lines should be deleted or reintroduced behind a feature flag to avoid API ambiguity.
-//#[cfg(feature = "v2")] -//pub mod accounts_small; ... -//#[cfg(feature = "v2")] -//pub use accounts_small::*;programs/system/src/accounts/remaining_account_checks.rs (1)
50-61: Profiler attribute gating#[profile] requires light_program_profiler at build time. Consider cfg gating (e.g., cfg(feature = "profiler")) to avoid pulling it into all builds.
-#[profile] +#[cfg_attr(feature = "profiler", profile)]program-libs/compressed-account/src/indexer_event/parse.rs (2)
485-493: Avoid extra allocations when concatenating outputsCurrent concat clones both vectors. Prefer reserve+extend to avoid copies.
- output_compressed_accounts: [ - associated_instructions.cpi_context_outputs.clone(), - associated_instructions - .executing_system_instruction - .output_compressed_accounts - .clone(), - ] - .concat(), + output_compressed_accounts: { + let mut v = Vec::with_capacity( + associated_instructions.cpi_context_outputs.len() + + associated_instructions.executing_system_instruction.output_compressed_accounts.len() + ); + v.extend_from_slice(&associated_instructions.cpi_context_outputs); + v.extend_from_slice( + &associated_instructions.executing_system_instruction.output_compressed_accounts + ); + v + },
327-333: Doc/comments mismatchComments mention “Min len for a small instruction 3/4 accounts + 1 tree or queue” but the new code requires at least 5 (fee_payer+authority+registered_program+account_compression_program+account_compression_authority). Update comments to match.
Also applies to: 375-381
program-libs/compressed-account/src/lib.rs (1)
38-41: Typos in user-facing messages“thresold” → “threshold”; “lenght” → “length”.
- #[error("Invalid rollover thresold")] + #[error("Invalid rollover threshold")] @@ - #[error("Invalid input lenght")] + #[error("Invalid input length")]program-libs/compressed-account/src/instruction_data/cpi_context.rs (1)
25-43: Consider usingInto<u8>trait for bool-to-u8 conversions.The current implementation correctly casts bools to u8, but you could make the intent clearer and more idiomatic by implementing or using the
Into<u8>trait for bool conversions.impl CompressedCpiContextTrait for ZCompressedCpiContext { fn first_set_context(&self) -> u8 { - self.first_set_context() as u8 + u8::from(self.first_set_context()) } fn set_context(&self) -> u8 { - self.set_context() as u8 + u8::from(self.set_context()) } } impl CompressedCpiContextTrait for CompressedCpiContext { fn first_set_context(&self) -> u8 { - self.first_set_context as u8 + u8::from(self.first_set_context) } fn set_context(&self) -> u8 { - self.set_context as u8 + u8::from(self.set_context) } }programs/system/tests/invoke_cpi_instruction_small.rs (2)
136-136: Address the TODO comment forwrite_to_cpi_contexttesting.The TODO indicates missing test coverage for the new
write_to_cpi_contextfield. This should be tested to ensure the CPI context write functionality works correctly.Would you like me to generate test cases for the
write_to_cpi_context: truescenario?
503-510: Improve error handling for missing required accounts.The comment mentions this will panic with index out of bounds, but the test actually returns an error. Consider making the assertion more specific about the expected error type.
- assert!(result.is_err(), "Expected a panic due to missing accounts"); + assert!(matches!(result, Err(ProgramError::NotEnoughAccountKeys)), + "Expected NotEnoughAccountKeys error due to missing accounts");program-libs/compressed-account/src/pubkey.rs (1)
124-130: Consider documenting why Pubkey is its own inner type.The implementations of
ZeroCopyStructInnerandZeroCopyStructInnerMutboth returnPubkeyas the inner type. While this is correct for a simple wrapper type, it might be worth adding a brief comment explaining this design choice.+// Pubkey is a simple wrapper around [u8; 32] with no additional structure, +// so it serves as its own inner type for zero-copy operations. impl ZeroCopyStructInner for Pubkey { type ZeroCopyInner = Pubkey; } impl ZeroCopyStructInnerMut for Pubkey { type ZeroCopyInnerMut = Pubkey; }sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs (1)
5-5: Unused imports after commenting out codeAccountMeta and Pubkey are now unused. Clean them up to avoid warnings.
-use pinocchio::{account_info::AccountInfo, instruction::AccountMeta, pubkey::Pubkey}; +use pinocchio::account_info::AccountInfo;Also drop unused light_sdk_types constants from the import list if this file no longer builds metas.
-use light_sdk_types::{ - CpiAccountsSmall as GenericCpiAccountsSmall, ACCOUNT_COMPRESSION_AUTHORITY_PDA, - ACCOUNT_COMPRESSION_PROGRAM_ID, REGISTERED_PROGRAM_PDA, SMALL_SYSTEM_ACCOUNTS_LEN, -}; +use light_sdk_types::CpiAccountsSmall as GenericCpiAccountsSmall;program-libs/compressed-account/src/instruction_data/data.rs (2)
36-45: OK to mark packed params as repr(C) + ZeroCopyMut; add size checksThe field ordering yields a 36‑byte layout; add a small test to guard the layout.
+#[cfg(test)] +mod layout_tests_new_address_params_packed { + use super::*; + #[test] + fn size_is_stable() { + assert_eq!(core::mem::size_of::<NewAddressParamsPacked>(), 36); + } +}
47-58: Prefer u8 over bool for FFI‑stable layoutsassigned_to_account: bool is 1 byte today but can be brittle across FFI. If this struct crosses language/FFI boundaries, consider u8 (0/1) for stability.
- pub assigned_to_account: bool, + pub assigned_to_account: u8, // 0 = false, 1 = trueFollow-up: adapt new()/assigned_account_index() accordingly.
sdk-libs/sdk/src/cpi/accounts_cpi_context.rs (1)
5-13: Consider more descriptive function name and add documentation.The function creates account metas in a specific order but the name doesn't clearly convey this. Consider adding documentation to clarify the order and purpose of the returned accounts.
Consider this improvement:
+/// Creates account metas for CPI context operations in the following order: +/// - fee_payer: writable signer +/// - cpi_signer: read-only signer +/// - cpi_context: writable non-signer pub fn get_account_metas_from_config_cpi_context( config: CpiContextWriteAccounts<AccountInfo>, ) -> [AccountMeta; 3] {sdk-libs/sdk-types/src/cpi_context_write.rs (2)
4-4: TODO comment should be addressed.The TODO comment indicates this should be moved to ctoken types. Consider creating an issue to track this if it's not being addressed in this PR.
Do you want me to generate a GitHub issue to track moving this to ctoken types?
22-28: Consider using references instead of cloning.The
to_account_infosmethod clones all accounts. If the caller only needs references, this could be inefficient.The existing
to_account_info_refsmethod (lines 30-32) already provides the reference version, so this approach offers both options which is good design.anchor-programs/system/src/cpi_context_account.rs (1)
44-48: Zero-copy struct needs proper validation.The
ZCpiContextAccount2struct uses zero-copy references but lacks validation of the underlying data integrity during construction.Consider adding validation methods to ensure data consistency:
impl<'a> ZCpiContextAccount2<'a> { pub fn validate(&self) -> Result<(), ZeroCopyError> { // Validate fee_payer is not all zeros when account is active // Validate associated_merkle_tree is a valid pubkey // Validate context vector integrity Ok(()) } }program-libs/compressed-account/tests/zero_copy_set.rs (1)
978-1009: Good panic test but consider more robust validation.The panic test for invalid address length is good for catching immediate failures, but consider adding non-panic error handling for production robustness.
Consider adding a non-panicking validation method:
pub fn validate_address_length(address: &[u8]) -> Result<(), CompressedAccountError> { if address.len() != 32 { return Err(CompressedAccountError::InvalidAddressLength); } Ok(()) }programs/system/src/processor/create_inputs_cpi_data.rs (1)
57-90: Improved bounds checking but consider error message clarity.The replacement of direct indexing with bounds-checked
get()is a good security improvement. However, the error message in line 85 could be more descriptive.Improve the error message for better debugging:
- msg!(format!("create_inputs_cpi_data {} ", current_mt_index).as_str()); + msg!("Invalid Merkle tree account type at index {}: expected BatchedStateTree or StateTree", current_mt_index);programs/system/src/invoke_cpi/instruction_small.rs (1)
125-130: Inconsistent error handling pattern for optional fields.
get_sol_pool_pda()returnsOk(None)whenexec_accountsis None, butget_registered_program_pda()andget_account_compression_authority()returnErr. This creates an inconsistent API for optional accounts.The current implementation might lead to confusion - either all optional account getters should return
Result<Option<_>>or none should. Consider aligning the error handling pattern:fn get_sol_pool_pda(&self) -> Result<Option<&'info AccountInfo>> { - Ok(self - .exec_accounts - .as_ref() - .and_then(|exec| exec.sol_pool_pda)) + self.exec_accounts + .as_ref() + .map(|exec| exec.sol_pool_pda) + .ok_or(SystemProgramError::CpiContextPassedAsSetContext.into()) }sdk-libs/sdk/src/cpi/invoke.rs (2)
107-109: Use a more descriptive error message for unsupported operations.The
unimplemented!()macro with a generic message doesn't provide enough context about why new_addresses must be new assigned addresses.if cpi_inputs.new_addresses.is_some() { - unimplemented!("new_addresses must be new assigned addresses."); + unimplemented!("Legacy new_addresses field is not supported in small instruction variant. Use new_assigned_addresses instead."); }
150-152: Duplicate validation logic between small and context write functions.The validation for
new_addressesis duplicated betweencreate_light_system_progam_instruction_invoke_cpi_smallandcreate_light_system_progam_instruction_invoke_cpi_context_write.Consider extracting a validation helper to reduce duplication:
+fn validate_cpi_inputs(cpi_inputs: &CpiInputs) -> Result<()> { + if cpi_inputs.new_addresses.is_some() { + unimplemented!("Legacy new_addresses field is not supported. Use new_assigned_addresses instead."); + } + Ok(()) +} + pub fn create_light_system_progam_instruction_invoke_cpi_small( cpi_inputs: CpiInputs, cpi_accounts: CpiAccountsSmall<'_, '_>, ) -> Result<Instruction> { - if cpi_inputs.new_addresses.is_some() { - unimplemented!("new_addresses must be new assigned addresses."); - } + validate_cpi_inputs(&cpi_inputs)?;program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
501-507: Potential issue with CPI context generation in tests.The
get_rnd_cpi_contextfunction usesrng.gen() && with_cpi_contextfor both flags, which means ifwith_cpi_contextis false, both flags will always be false. However, the cpi_context is still created with these values, which might not reflect realistic scenarios where the struct exists but shouldn't be used.Consider making the test data generation more explicit:
fn get_rnd_cpi_context(rng: &mut StdRng, with_cpi_context: bool) -> CompressedCpiContext { + if !with_cpi_context { + // Return a default/empty context when not needed + return CompressedCpiContext::default(); + } CompressedCpiContext { - first_set_context: rng.gen() && with_cpi_context, - set_context: rng.gen() && with_cpi_context, + first_set_context: rng.gen(), + set_context: rng.gen(), cpi_context_account_index: rng.gen(), } }program-libs/compressed-account/src/instruction_data/zero_copy.rs (1)
319-327: Minor: prefer helper for clarity.Use merkle_context.prove_by_index() rather than == 1 to avoid magic numbers.
Apply:
- prove_by_index: merkle_context.prove_by_index == 1, + prove_by_index: merkle_context.prove_by_index(),sdk-libs/sdk-types/src/cpi_accounts_small.rs (4)
10-19: Fix index annotations; comments don’t match enum layout.LightSystemProgram precedes Authority, so the “index 0/1/2…” notes are wrong and can mislead.
Apply:
- Authority, // index 0 - Cpi authority of the custom program, used to invoke the light system program. - RegisteredProgramPda, // index 1 - registered_program_pda - AccountCompressionAuthority, // index 2 - account_compression_authority - AccountCompressionProgram, // index 3 - account_compression_program - SystemProgram, // index 4 - system_program + Authority, // index 1 - CPI authority of the custom program, used to invoke the light system program + RegisteredProgramPda, // index 2 - registered_program_pda + AccountCompressionAuthority, // index 3 - account_compression_authority + AccountCompressionProgram, // index 4 - account_compression_program + SystemProgram, // index 5 - system_program SolPoolPda, // index 5 - Optional DecompressionRecipient, // index 6 - Optional CpiContext, // index 7 - Optional
103-113: CPI-context index math: verify with flags.Index is decremented for sol_pool_pda/sol_compression_recipient, but not guarded by config.cpi_context. If cpi_context is disabled, this will still compute an index into accounts and rely on OOB. Consider early error or documenting the contract.
166-174: Unnecessary cloning in to_account_infos().Returns owned Vec and clones every account. If callers can use &'a [T], prefer exposing a borrowed view or an iterator to reduce allocations/copies.
Alternative:
- Keep to_account_infos() but add to_account_info_refs(&self) -> &'a [T] for zero-copy paths.
186-192: Minor: simplify collection.Collect directly without explicit turbofish.
Apply:
- Ok(self - .tree_accounts()? - .iter() - .map(|x| x.pubkey()) - .collect::<Vec<T::Pubkey>>()) + Ok(self.tree_accounts()?.iter().map(|x| x.pubkey()).collect())programs/system/src/context.rs (1)
89-128: AccountMeta always writable; consider narrowing.get_index_or_insert() marks all metas writable/signers=false. This may violate least-privilege and increase CU costs. If feasible, thread writability/signature requirements from call sites.
program-libs/account-checks/src/account_iterator.rs (3)
83-93: Name implies mutability but returns shared ref.next_option_mut enforces writable but returns &T. Consider renaming to next_writable_option to avoid confusion.
143-154: remaining(): returning Err on exhaustion is surprising.Most callers expect Ok([]). Since remaining_unchecked exists, consider making remaining() return Ok(&[]) when empty.
Apply:
- if self.position >= self.accounts.len() { - let location = Location::caller(); - let account_name = "remaining accounts"; - solana_msg::msg!( ... ); - return Err(AccountError::NotEnoughAccountKeys); - } - Ok(&self.accounts[self.position..]) + Ok(if self.position >= self.accounts.len() { + &[] + } else { + &self.accounts[self.position..] + })
190-203: Minor logging nit.“for account … at index …” has double spacing before file:line:col. Consider a single space.
programs/system/src/cpi_context/state.rs (1)
134-151: Potential truncation: assigned_account_index cast to u8.assigned_index is usize; casting to u8 before checked_add can truncate. Validate with try_from first.
Apply:
- let assigned_account_index = (assigned_index.unwrap_or(0) as u8) - .checked_add(pre_address_len as u8) - .ok_or(ZeroCopyError::Size)?; + let base: u8 = assigned_index + .map(u8::try_from) + .transpose() + .map_err(|_| ZeroCopyError::Size)? + .unwrap_or(0); + let assigned_account_index = base + .checked_add(u8::try_from(pre_address_len).map_err(|_| ZeroCopyError::Size)?) + .ok_or(ZeroCopyError::Size)?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
.cargo/config.toml(0 hunks)anchor-programs/system/Cargo.toml(1 hunks)anchor-programs/system/src/cpi_context_account.rs(2 hunks)anchor-programs/system/src/lib.rs(1 hunks)cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/epoch_pda_34w7KcLBXabMkHuXE2fY368vFe6kP3v5EJn8nPvQ8SKn.json(1 hunks)cli/accounts/forester_epoch_pda_3FBt1BPQHCQkS8k3wrUXMfB6JBhtMhEqQXueHRw2ojZV.json(1 hunks)cli/accounts/governance_authority_pda_CuEtcKkkbTn6qy2qxqDswq5U2ADsqoipYDAYfRvxPjcp.json(1 hunks)cli/accounts/registered_forester_pda_2KNqEh23Se8AHecuzR1UkxL26euq2qXSpQPTH1jH7VqU.json(1 hunks)program-libs/account-checks/Cargo.toml(1 hunks)program-libs/account-checks/src/account_info/pinocchio.rs(1 hunks)program-libs/account-checks/src/account_iterator.rs(1 hunks)program-libs/account-checks/src/error.rs(2 hunks)program-libs/account-checks/src/lib.rs(1 hunks)program-libs/account-checks/src/packed_accounts.rs(1 hunks)program-libs/account-checks/tests/account_iterator.rs(1 hunks)program-libs/compressed-account/Cargo.toml(1 hunks)program-libs/compressed-account/src/compressed_account.rs(6 hunks)program-libs/compressed-account/src/indexer_event/parse.rs(5 hunks)program-libs/compressed-account/src/instruction_data/compressed_proof.rs(2 hunks)program-libs/compressed-account/src/instruction_data/cpi_context.rs(2 hunks)program-libs/compressed-account/src/instruction_data/data.rs(3 hunks)program-libs/compressed-account/src/instruction_data/invoke_cpi.rs(2 hunks)program-libs/compressed-account/src/instruction_data/mod.rs(1 hunks)program-libs/compressed-account/src/instruction_data/traits.rs(3 hunks)program-libs/compressed-account/src/instruction_data/with_account_info.rs(5 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(5 hunks)program-libs/compressed-account/src/instruction_data/zero_copy.rs(6 hunks)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs(1 hunks)program-libs/compressed-account/src/lib.rs(2 hunks)program-libs/compressed-account/src/pubkey.rs(3 hunks)program-libs/compressed-account/tests/account_option_config.rs(1 hunks)program-libs/compressed-account/tests/pubkey.rs(1 hunks)program-libs/compressed-account/tests/zero_copy_set.rs(1 hunks)program-libs/hasher/Cargo.toml(2 hunks)program-libs/hasher/src/to_byte_array.rs(1 hunks)program-libs/zero-copy/src/vec.rs(2 hunks)program-tests/compressed-token-test/tests/test.rs(1 hunks)program-tests/create-address-test-program/Cargo.toml(1 hunks)program-tests/create-address-test-program/src/lib.rs(5 hunks)program-tests/sdk-anchor-test/package.json(0 hunks)program-tests/system-cpi-test/tests/test.rs(4 hunks)program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs(6 hunks)program-tests/system-test/Cargo.toml(2 hunks)program-tests/system-test/tests/test_re_init_cpi_account.rs(1 hunks)program-tests/utils/Cargo.toml(1 hunks)program-tests/utils/src/assert_rollover.rs(1 hunks)program-tests/utils/src/legacy_cpi_context_account.rs(1 hunks)program-tests/utils/src/lib.rs(1 hunks)programs/account-compression/src/processor/insert_into_queues.rs(1 hunks)programs/compressed-token/src/process_transfer.rs(2 hunks)programs/package.json(1 hunks)programs/system/Cargo.toml(2 hunks)programs/system/README.md(0 hunks)programs/system/src/account_compression_state/address.rs(3 hunks)programs/system/src/accounts/account_checks.rs(7 hunks)programs/system/src/accounts/account_traits.rs(1 hunks)programs/system/src/accounts/init_context_account.rs(3 hunks)programs/system/src/accounts/remaining_account_checks.rs(5 hunks)programs/system/src/constants.rs(2 hunks)programs/system/src/context.rs(13 hunks)programs/system/src/cpi_context/account.rs(1 hunks)programs/system/src/cpi_context/address.rs(1 hunks)programs/system/src/cpi_context/instruction_data_trait.rs(1 hunks)programs/system/src/cpi_context/mod.rs(1 hunks)programs/system/src/cpi_context/process_cpi_context.rs(1 hunks)programs/system/src/cpi_context/state.rs(1 hunks)programs/system/src/errors.rs(2 hunks)programs/system/src/invoke/instruction.rs(1 hunks)programs/system/src/invoke_cpi/account.rs(0 hunks)programs/system/src/invoke_cpi/instruction.rs(1 hunks)programs/system/src/invoke_cpi/instruction_small.rs(2 hunks)programs/system/src/invoke_cpi/mod.rs(0 hunks)programs/system/src/invoke_cpi/process_cpi_context.rs(0 hunks)programs/system/src/invoke_cpi/processor.rs(3 hunks)programs/system/src/invoke_cpi/verify_signer.rs(6 hunks)programs/system/src/lib.rs(5 hunks)programs/system/src/processor/cpi.rs(5 hunks)programs/system/src/processor/create_address_cpi_data.rs(2 hunks)programs/system/src/processor/create_inputs_cpi_data.rs(5 hunks)programs/system/src/processor/create_outputs_cpi_data.rs(9 hunks)programs/system/src/processor/process.rs(6 hunks)programs/system/src/processor/read_only_account.rs(2 hunks)programs/system/src/processor/read_only_address.rs(1 hunks)programs/system/src/processor/sol_compression.rs(8 hunks)programs/system/src/processor/sum_check.rs(1 hunks)programs/system/src/processor/verify_proof.rs(6 hunks)programs/system/src/utils.rs(2 hunks)programs/system/tests/cpi_context.rs(1 hunks)programs/system/tests/invoke_cpi_instruction_small.rs(21 hunks)programs/system/tests/invoke_instruction.rs(1 hunks)sdk-libs/sdk-pinocchio/Cargo.toml(0 hunks)sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs(2 hunks)sdk-libs/sdk-pinocchio/src/cpi/mod.rs(1 hunks)sdk-libs/sdk-types/Cargo.toml(0 hunks)sdk-libs/sdk-types/src/constants.rs(1 hunks)sdk-libs/sdk-types/src/cpi_accounts.rs(5 hunks)sdk-libs/sdk-types/src/cpi_accounts_small.rs(4 hunks)sdk-libs/sdk-types/src/cpi_context_write.rs(1 hunks)sdk-libs/sdk-types/src/instruction/tree_info.rs(1 hunks)sdk-libs/sdk-types/src/lib.rs(2 hunks)sdk-libs/sdk/Cargo.toml(1 hunks)sdk-libs/sdk/src/cpi/accounts_cpi_context.rs(1 hunks)sdk-libs/sdk/src/cpi/accounts_small_ix.rs(1 hunks)sdk-libs/sdk/src/cpi/invoke.rs(4 hunks)
⛔ Files not processed due to max files limit (7)
- sdk-libs/sdk/src/cpi/mod.rs
- sdk-libs/sdk/src/error.rs
- sdk-libs/sdk/src/instruction/pack_accounts.rs
- sdk-libs/sdk/src/instruction/system_accounts.rs
- sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/test.rs
- sdk-tests/sdk-native-test/tests/test.rs
- sdk-tests/sdk-pinocchio-test/tests/test.rs
💤 Files with no reviewable changes (8)
- .cargo/config.toml
- sdk-libs/sdk-types/Cargo.toml
- sdk-libs/sdk-pinocchio/Cargo.toml
- programs/system/README.md
- programs/system/src/invoke_cpi/mod.rs
- programs/system/src/invoke_cpi/process_cpi_context.rs
- program-tests/sdk-anchor-test/package.json
- programs/system/src/invoke_cpi/account.rs
✅ Files skipped from review due to trivial changes (1)
- program-libs/account-checks/tests/account_iterator.rs
🚧 Files skipped from review as they are similar to previous changes (43)
- cli/accounts/forester_epoch_pda_3FBt1BPQHCQkS8k3wrUXMfB6JBhtMhEqQXueHRw2ojZV.json
- program-libs/account-checks/src/packed_accounts.rs
- programs/system/src/lib.rs
- program-libs/compressed-account/src/instruction_data/traits.rs
- cli/accounts/registered_forester_pda_2KNqEh23Se8AHecuzR1UkxL26euq2qXSpQPTH1jH7VqU.json
- program-tests/utils/src/assert_rollover.rs
- program-libs/compressed-account/src/instruction_data/compressed_proof.rs
- programs/system/src/processor/create_address_cpi_data.rs
- programs/system/src/processor/read_only_address.rs
- program-tests/system-cpi-test/tests/test.rs
- program-libs/hasher/Cargo.toml
- programs/package.json
- program-libs/account-checks/src/account_info/pinocchio.rs
- program-tests/utils/Cargo.toml
- programs/system/src/accounts/account_traits.rs
- cli/accounts/governance_authority_pda_CuEtcKkkbTn6qy2qxqDswq5U2ADsqoipYDAYfRvxPjcp.json
- program-libs/compressed-account/src/instruction_data/invoke_cpi.rs
- programs/system/src/cpi_context/instruction_data_trait.rs
- programs/system/src/cpi_context/address.rs
- program-libs/compressed-account/Cargo.toml
- programs/system/src/cpi_context/mod.rs
- programs/system/src/processor/sum_check.rs
- programs/system/src/processor/read_only_account.rs
- programs/system/src/account_compression_state/address.rs
- program-tests/compressed-token-test/tests/test.rs
- programs/system/src/processor/sol_compression.rs
- programs/system/src/errors.rs
- program-tests/create-address-test-program/Cargo.toml
- program-libs/account-checks/Cargo.toml
- program-tests/utils/src/legacy_cpi_context_account.rs
- program-tests/system-test/Cargo.toml
- programs/system/src/invoke_cpi/processor.rs
- program-libs/compressed-account/src/instruction_data/mod.rs
- programs/compressed-token/src/process_transfer.rs
- cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json
- program-libs/account-checks/src/lib.rs
- program-libs/compressed-account/src/instruction_data/zero_copy_set.rs
- programs/system/src/invoke_cpi/verify_signer.rs
- cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json
- programs/system/src/cpi_context/account.rs
- program-tests/utils/src/lib.rs
- anchor-programs/system/Cargo.toml
- programs/account-compression/src/processor/insert_into_queues.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T00:36:53.191Z
Learnt from: ananas-block
PR: Lightprotocol/light-protocol#1909
File: program-libs/zero-copy/src/init_mut.rs:241-249
Timestamp: 2025-08-14T00:36:53.191Z
Learning: In the light-protocol zero-copy crate, performance is prioritized over safety checks for edge cases like Vec lengths exceeding u32::MAX, even when there might be wire format inconsistencies.
Applied to files:
program-libs/compressed-account/src/instruction_data/with_readonly.rsprogram-libs/compressed-account/src/pubkey.rsprogram-libs/zero-copy/src/vec.rs
🧬 Code graph analysis (34)
programs/system/src/invoke/instruction.rs (3)
programs/system/src/accounts/account_traits.rs (4)
get_registered_program_pda(6-6)get_account_compression_authority(7-7)get_sol_pool_pda(8-8)get_decompression_recipient(9-9)programs/system/src/invoke_cpi/instruction.rs (4)
get_registered_program_pda(101-103)get_account_compression_authority(105-107)get_sol_pool_pda(109-111)get_decompression_recipient(113-115)programs/system/src/invoke_cpi/instruction_small.rs (4)
get_registered_program_pda(111-116)get_account_compression_authority(118-123)get_sol_pool_pda(125-130)get_decompression_recipient(132-137)
program-tests/create-address-test-program/src/lib.rs (3)
sdk-libs/sdk/src/cpi/accounts.rs (1)
get_account_metas_from_config(26-134)sdk-libs/sdk-types/src/cpi_accounts_small.rs (5)
fee_payer(49-51)account_infos(137-139)new_with_config(41-47)config(119-121)system_accounts_end_offset(123-135)sdk-libs/sdk/src/cpi/accounts_small_ix.rs (1)
to_account_metas_small(131-134)
programs/system/src/accounts/init_context_account.rs (2)
program-libs/account-checks/src/checks.rs (2)
check_owner(128-136)check_signer(114-119)programs/system/src/cpi_context/state.rs (2)
cpi_context_account_new(403-495)new(378-389)
program-libs/compressed-account/src/instruction_data/with_account_info.rs (4)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_readonly.rs (6)
account_option_config(265-283)with_cpi_context(245-247)rng(676-685)rng(701-721)rng(722-728)rng(729-740)program-libs/compressed-account/src/instruction_data/zero_copy.rs (3)
account_option_config(442-444)account_option_config(560-572)get_rnd_cpi_context(1014-1020)programs/system/src/cpi_context/instruction_data_trait.rs (1)
account_option_config(75-82)
program-libs/compressed-account/src/instruction_data/zero_copy.rs (6)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
account_option_config(308-328)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
account_option_config(265-283)programs/system/src/cpi_context/instruction_data_trait.rs (1)
account_option_config(75-82)program-libs/compressed-account/src/instruction_data/cpi_context.rs (2)
first_set_context(26-28)first_set_context(36-38)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs (1)
first_set_context(122-122)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (4)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
account_option_config(308-328)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
account_option_config(442-444)account_option_config(560-572)programs/system/src/cpi_context/instruction_data_trait.rs (1)
account_option_config(75-82)
program-libs/compressed-account/src/instruction_data/data.rs (3)
js/stateless.js/src/state/types.ts (1)
OutputCompressedAccountWithPackedContext(170-173)js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)js/stateless.js/src/utils/address.ts (1)
NewAddressParamsPacked(64-83)
programs/system/src/invoke_cpi/instruction_small.rs (3)
programs/system/src/accounts/account_checks.rs (3)
check_option_cpi_context_account(134-167)check_option_decompression_recipient(117-130)check_option_sol_pool_pda(170-183)programs/system/src/invoke_cpi/instruction.rs (5)
from_account_infos(41-81)get_registered_program_pda(101-103)get_account_compression_authority(105-107)get_sol_pool_pda(109-111)get_decompression_recipient(113-115)programs/system/src/accounts/account_traits.rs (4)
get_registered_program_pda(6-6)get_account_compression_authority(7-7)get_sol_pool_pda(8-8)get_decompression_recipient(9-9)
programs/system/src/processor/process.rs (7)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
cpi_context(22-22)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
cpi_context(354-364)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
cpi_context(312-322)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
cpi_context(487-489)cpi_context(616-624)programs/system/src/context.rs (1)
cpi_context(351-353)programs/system/src/cpi_context/instruction_data_trait.rs (1)
cpi_context(66-68)programs/system/src/cpi_context/process_cpi_context.rs (2)
process_cpi_context(48-88)copy_cpi_context_outputs(144-190)
sdk-libs/sdk/src/cpi/accounts_cpi_context.rs (1)
sdk-libs/sdk-types/src/cpi_accounts.rs (3)
config(223-225)new(21-28)new(72-78)
sdk-libs/sdk-types/src/cpi_context_write.rs (2)
sdk-libs/sdk-types/src/cpi_accounts.rs (4)
bump(43-45)bump(219-221)invoking_program(146-151)to_account_infos(276-283)sdk-libs/sdk-types/src/cpi_accounts_small.rs (3)
bump(175-177)invoking_program(179-181)to_account_infos(166-174)
program-tests/system-test/tests/test_re_init_cpi_account.rs (2)
anchor-programs/system/src/cpi_context_account.rs (1)
deserialize_cpi_context_account(50-72)program-tests/utils/src/legacy_cpi_context_account.rs (1)
get_legacy_cpi_context_account(4-12)
sdk-libs/sdk/src/cpi/accounts_small_ix.rs (6)
sdk-libs/sdk-types/src/cpi_accounts.rs (1)
config(223-225)sdk-libs/sdk-types/src/cpi_accounts_small.rs (1)
config(119-121)sdk-libs/sdk/src/error.rs (3)
from(92-94)from(98-125)from(129-170)sdk-libs/sdk/src/instruction/system_accounts.rs (1)
default(48-70)sdk-libs/sdk/src/cpi/accounts.rs (1)
try_from(139-161)program-tests/create-address-test-program/src/lib.rs (1)
to_account_metas_small(258-344)
program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs (1)
sdk-libs/sdk/src/instruction/system_accounts.rs (3)
new(17-24)get_light_system_account_metas(73-113)default(48-70)
program-libs/compressed-account/tests/account_option_config.rs (2)
program-libs/compressed-account/tests/zero_copy_set.rs (6)
None(1474-1474)None(1508-1508)None(1542-1542)None(1587-1587)None(1677-1677)None(1784-1784)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
with_cpi_context(245-247)
programs/system/src/context.rs (4)
programs/system/src/cpi_context/state.rs (6)
std(93-93)std(95-95)std(97-97)std(98-98)std(100-100)std(472-472)program-libs/compressed-account/src/instruction_data/traits.rs (3)
cpi_context(22-22)output_accounts(16-16)input_accounts(15-15)programs/system/src/cpi_context/instruction_data_trait.rs (3)
cpi_context(66-68)output_accounts(30-32)input_accounts(26-28)programs/system/src/cpi_context/process_cpi_context.rs (1)
set_cpi_context(91-137)
programs/system/tests/invoke_instruction.rs (2)
sdk-libs/sdk-types/src/cpi_accounts.rs (2)
account_compression_authority(167-172)registered_program_pda(153-158)sdk-libs/sdk-types/src/cpi_accounts_small.rs (2)
account_compression_authority(67-72)registered_program_pda(60-65)
program-libs/compressed-account/src/instruction_data/cpi_context.rs (2)
program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
first_set_context(661-663)set_context(657-659)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs (2)
first_set_context(122-122)set_context(121-121)
programs/system/src/cpi_context/process_cpi_context.rs (4)
program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_account_info(152-159)programs/system/src/cpi_context/state.rs (3)
deserialize_cpi_context_account(276-280)deserialize_cpi_context_account_cleared(283-287)is_empty(63-70)programs/system/src/context.rs (2)
cpi_context(351-353)set_cpi_context(203-222)program-libs/compressed-account/src/pubkey.rs (3)
new_zero_copy(59-64)new_from_array(68-70)to_bytes(189-191)
programs/system/src/invoke_cpi/instruction.rs (3)
programs/system/src/accounts/account_traits.rs (4)
get_registered_program_pda(6-6)get_account_compression_authority(7-7)get_sol_pool_pda(8-8)get_decompression_recipient(9-9)programs/system/src/invoke/instruction.rs (4)
get_registered_program_pda(98-100)get_account_compression_authority(102-104)get_sol_pool_pda(106-108)get_decompression_recipient(110-112)programs/system/src/invoke_cpi/instruction_small.rs (4)
get_registered_program_pda(111-116)get_account_compression_authority(118-123)get_sol_pool_pda(125-130)get_decompression_recipient(132-137)
sdk-libs/sdk/src/cpi/invoke.rs (4)
sdk-libs/sdk/src/cpi/accounts_cpi_context.rs (1)
get_account_metas_from_config_cpi_context(5-13)sdk-libs/sdk/src/cpi/accounts.rs (1)
get_account_metas_from_config(26-134)sdk-libs/sdk/src/cpi/accounts_small_ix.rs (1)
to_account_metas_small(131-134)sdk-libs/sdk-types/src/cpi_context_write.rs (1)
bump(14-16)
programs/system/src/processor/cpi.rs (5)
programs/system/src/accounts/account_traits.rs (2)
get_account_compression_authority(7-7)get_registered_program_pda(6-6)programs/system/src/invoke/instruction.rs (2)
get_account_compression_authority(102-104)get_registered_program_pda(98-100)programs/system/src/invoke_cpi/instruction.rs (2)
get_account_compression_authority(105-107)get_registered_program_pda(101-103)programs/system/src/invoke_cpi/instruction_small.rs (2)
get_account_compression_authority(118-123)get_registered_program_pda(111-116)programs/system/src/utils.rs (1)
get_registered_program_pda(13-15)
program-libs/compressed-account/tests/zero_copy_set.rs (2)
program-libs/compressed-account/src/instruction_data/data.rs (1)
assigned_account_index(72-78)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
with_cpi_context(245-247)
program-libs/account-checks/src/account_iterator.rs (1)
program-libs/account-checks/src/checks.rs (3)
check_mut(121-126)check_non_mut(43-48)check_signer(114-119)
programs/system/src/processor/create_inputs_cpi_data.rs (3)
programs/system/src/cpi_context/account.rs (1)
merkle_context(69-71)program-libs/compressed-account/src/instruction_data/traits.rs (1)
merkle_context(49-49)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
merkle_context(86-88)
sdk-libs/sdk-types/src/cpi_accounts_small.rs (2)
sdk-libs/sdk-types/src/cpi_accounts.rs (8)
account_compression_program(174-179)system_program(195-200)to_account_infos(276-283)account_infos(241-243)bump(43-45)bump(219-221)invoking_program(146-151)tree_pubkeys(258-264)sdk-libs/sdk-types/src/cpi_context_write.rs (3)
to_account_infos(22-28)bump(14-16)invoking_program(18-20)
programs/system/tests/invoke_cpi_instruction_small.rs (2)
programs/system/tests/invoke_cpi_instruction.rs (4)
get_account_compression_authority_account_info(79-88)get_account_compression_program_account_info(56-65)get_mut_account_info(103-112)get_system_program_account_info(115-124)sdk-libs/sdk-types/src/cpi_accounts_small.rs (4)
account_compression_program(74-79)system_program(81-86)authority(53-58)account_compression_authority(67-72)
programs/system/tests/cpi_context.rs (3)
programs/system/src/cpi_context/process_cpi_context.rs (1)
process_cpi_context(48-88)anchor-programs/system/src/cpi_context_account.rs (1)
deserialize_cpi_context_account(50-72)programs/system/src/cpi_context/state.rs (1)
deserialize_cpi_context_account_cleared(283-287)
programs/system/src/utils.rs (6)
programs/system/src/accounts/account_traits.rs (2)
get_registered_program_pda(6-6)get_sol_pool_pda(8-8)programs/system/src/invoke/instruction.rs (2)
get_registered_program_pda(98-100)get_sol_pool_pda(106-108)programs/system/src/invoke_cpi/instruction.rs (2)
get_registered_program_pda(101-103)get_sol_pool_pda(109-111)programs/system/src/invoke_cpi/instruction_small.rs (2)
get_registered_program_pda(111-116)get_sol_pool_pda(125-130)program-libs/account-checks/src/account_info/pinocchio.rs (1)
find_program_address(57-76)programs/compressed-token/src/process_transfer.rs (1)
get_cpi_authority_pda(730-732)
programs/system/src/cpi_context/state.rs (3)
program-libs/account-checks/src/checks.rs (1)
check_owner(128-136)programs/system/src/cpi_context/account.rs (5)
data(77-87)data(137-147)address(61-67)address(121-127)merkle_context(69-71)program-libs/compressed-account/src/instruction_data/data.rs (2)
assigned_account_index(72-78)new(61-70)
programs/system/src/accounts/account_checks.rs (4)
programs/system/src/cpi_context/state.rs (6)
std(93-93)std(95-95)std(97-97)std(98-98)std(100-100)std(472-472)program-libs/account-checks/src/checks.rs (9)
check_discriminator(62-62)check_discriminator(78-87)check_mut(121-126)check_non_mut(43-48)check_owner(128-136)check_pda_seeds(151-161)check_pda_seeds_with_bump(163-173)check_program(138-149)check_signer(114-119)programs/system/src/context.rs (2)
cpi_context(351-353)owner(296-298)programs/system/src/cpi_context/account.rs (1)
owner(53-55)
program-libs/compressed-account/tests/pubkey.rs (1)
program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
programs/system/src/processor/create_outputs_cpi_data.rs (5)
programs/system/src/cpi_context/account.rs (4)
address(61-67)address(121-127)owner(53-55)merkle_tree_index(153-155)program-libs/compressed-account/src/instruction_data/traits.rs (7)
address(48-48)address(69-69)owner(13-13)owner(37-39)owner(46-46)owner(73-73)merkle_tree_index(74-74)program-libs/compressed-account/src/instruction_data/with_account_info.rs (6)
address(82-84)address(132-134)owner(71-73)owner(140-142)owner(342-344)merkle_tree_index(144-146)program-libs/compressed-account/src/instruction_data/zero_copy.rs (8)
address(133-135)address(346-348)owner(125-127)owner(340-342)owner(460-472)owner(537-548)owner(582-593)merkle_tree_index(129-131)program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
sdk-libs/sdk-types/src/cpi_accounts.rs (1)
sdk-libs/sdk-types/src/cpi_accounts_small.rs (3)
tree_pubkeys(186-192)to_account_infos(166-174)account_infos(137-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: Test program-libs-slow
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: lint
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: cli-v2
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-libs, light-macros light-sdk light-program-test light-client light-batched-m...
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test"])
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
| AccountError::InvalidProgramId => 12017, | ||
| AccountError::ProgramNotExecutable => 12018, | ||
| AccountError::AccountNotZeroed => 12019, | ||
| AccountError::NotEnoughAccountKeys => 12020, |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm code mapping uniqueness across crates
Ensure 12020 is unused elsewhere and mirrored consistently in any cross‑crate From conversions.
🏁 Script executed:
#!/bin/bash
# Verify there are no conflicting assignments of 12020
rg -nP '\b12020\b' -C2
# Check other error enums mapping to overlapping ranges
rg -nP '->\s*120\d{2}' -C2 program-libsLength of output: 1468
Fix duplicate error code 12020
12020 is assigned in both:
- program-libs/compressed-account/src/lib.rs:98 — CompressedAccountError::ExpectedDiscriminator => 12020
- program-libs/account-checks/src/error.rs:57 — AccountError::NotEnoughAccountKeys => 12020
Action: pick a unique numeric code for one of these mappings (and update any cross‑crate From impls), or centralize shared error codes in a common crate so mappings don't collide.
🤖 Prompt for AI Agents
In program-libs/account-checks/src/error.rs around line 57, the
AccountError::NotEnoughAccountKeys is mapped to numeric code 12020 which
duplicates CompressedAccountError::ExpectedDiscriminator (12020) in
program-libs/compressed-account/src/lib.rs; pick a new unique error code for one
of these mappings (or move shared codes into a central crate and reference them)
and then update any cross-crate From/Into impls or match arms that translate
between these error enums to use the new code constant so there are no
collisions at runtime.
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | ||
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | ||
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | ||
| let cpi_context_account = self.cpi_context().is_some(); | ||
| let write_to_cpi_context = false; // Not used | ||
|
|
||
| Ok(AccountOptions { | ||
| sol_pool_pda, | ||
| decompression_recipient, | ||
| cpi_context_account, | ||
| write_to_cpi_context, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Inconsistent AccountOptions: write_to_cpi_context is always false.
Align with with_account_info/with_readonly: derive write_to_cpi_context from set_context/first_set_context and validate presence.
Apply:
- fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> {
- let sol_pool_pda = self.compress_or_decompress_lamports().is_some();
- let decompression_recipient = sol_pool_pda && !self.is_compress();
- let cpi_context_account = self.cpi_context().is_some();
- let write_to_cpi_context = false; // Not used
-
- Ok(AccountOptions {
- sol_pool_pda,
- decompression_recipient,
- cpi_context_account,
- write_to_cpi_context,
- })
- }
+ fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> {
+ let sol_pool_pda = self.compress_or_decompress_lamports().is_some();
+ let decompression_recipient = sol_pool_pda && !self.is_compress();
+ let ctx = self.cpi_context();
+ let cpi_context_account = ctx.is_some();
+ let write_to_cpi_context = ctx
+ .map(|c| c.first_set_context || c.set_context)
+ .unwrap_or(false);
+ if write_to_cpi_context && !cpi_context_account {
+ return Err(CompressedAccountError::InvalidCpiContext);
+ }
+ Ok(AccountOptions {
+ sol_pool_pda,
+ decompression_recipient,
+ cpi_context_account,
+ write_to_cpi_context,
+ })
+ }📝 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.
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | |
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | |
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | |
| let cpi_context_account = self.cpi_context().is_some(); | |
| let write_to_cpi_context = false; // Not used | |
| Ok(AccountOptions { | |
| sol_pool_pda, | |
| decompression_recipient, | |
| cpi_context_account, | |
| write_to_cpi_context, | |
| }) | |
| } | |
| fn account_option_config(&self) -> Result<AccountOptions, CompressedAccountError> { | |
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | |
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | |
| let ctx = self.cpi_context(); | |
| let cpi_context_account = ctx.is_some(); | |
| let write_to_cpi_context = ctx | |
| .map(|c| c.first_set_context || c.set_context) | |
| .unwrap_or(false); | |
| if write_to_cpi_context && !cpi_context_account { | |
| return Err(CompressedAccountError::InvalidCpiContext); | |
| } | |
| Ok(AccountOptions { | |
| sol_pool_pda, | |
| decompression_recipient, | |
| cpi_context_account, | |
| write_to_cpi_context, | |
| }) | |
| } |
🤖 Prompt for AI Agents
In program-libs/compressed-account/src/instruction_data/zero_copy.rs around
lines 560–572, write_to_cpi_context is hardcoded to false; change it to be
derived from whether the account is being written to via
set_context/first_set_context and validate that a cpi_context exists when it is
set. Specifically, compute write_to_cpi_context = self.set_context().is_some()
|| self.first_set_context().is_some(), and if that value is true but
self.cpi_context().is_none() return an appropriate CompressedAccountError;
otherwise construct AccountOptions with the computed write_to_cpi_context so it
accurately reflects intent.
| // SAFETY: owner() returns a valid pointer to a 32-byte aligned Pubkey | ||
| solana_pubkey::Pubkey::new_from_array(unsafe { *account_info.owner() }), |
There was a problem hiding this comment.
Unsafe pointer dereference needs additional validation.
The comment states "SAFETY: owner() returns a valid pointer to a 32-byte aligned Pubkey" but there's no runtime validation of this assumption before the unsafe dereference.
Consider adding a debug assertion or validation before the unsafe operation:
+ // Validate pointer alignment in debug builds
+ debug_assert!(account_info.owner() as *const _ as usize % std::mem::align_of::<[u8; 32]>() == 0);
// SAFETY: owner() returns a valid pointer to a 32-byte aligned Pubkey
solana_pubkey::Pubkey::new_from_array(unsafe { *account_info.owner() }),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In programs/system/src/accounts/account_checks.rs around lines 145 to 146, the
unsafe dereference of account_info.owner() assumes the pointer is valid and
32-byte aligned without runtime checks; add a lightweight validation before the
unsafe block: obtain the raw pointer, assert it's not null and that its address
is aligned to std::mem::align_of::<Pubkey>() (or 32) using debug_assert! (or
return an error in non-debug builds), then perform the unsafe dereference to
create the Pubkey; alternatively replace the unsafe deref with a safe conversion
path such as using owner().as_ref().copied() or equivalent safe helper that
handles null/invalid pointers.
| #[profile] | ||
| pub fn reinit_cpi_context_account(accounts: &[AccountInfo]) -> Result<()> { | ||
| if accounts.is_empty() { | ||
| return Err(ProgramError::NotEnoughAccountKeys); | ||
| } | ||
| let cpi_context_account = &accounts[0]; | ||
| let associated_merkle_tree = { | ||
| let data = cpi_context_account.try_borrow_data()?; | ||
| CpiContextAccount::deserialize(&mut &data[8..]) | ||
| .map_err(|_| ProgramError::BorshIoError)? | ||
| .associated_merkle_tree | ||
| }; | ||
| // Initialize account with data. | ||
| cpi_context_account | ||
| .serialize(&mut &mut cpi_context_account_data[8..]) | ||
| .unwrap(); | ||
| let params: CpiContextAccountInitParams = | ||
| CpiContextAccountInitParams::new(associated_merkle_tree); | ||
| cpi_context_account_new::<true>(cpi_context_account, params)?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Critical security validation needed for reinit function.
The reinit_cpi_context_account function deserializes existing account data without proper owner and discriminator validation. This could allow unauthorized reinit operations.
Add proper validation before deserialization:
pub fn reinit_cpi_context_account(accounts: &[AccountInfo]) -> Result<()> {
if accounts.is_empty() {
return Err(ProgramError::NotEnoughAccountKeys);
}
let cpi_context_account = &accounts[0];
+
+ // Validate owner before attempting deserialization
+ check_owner(&ID, cpi_context_account)?;
+
+ // Validate discriminator before deserialization
+ let data = cpi_context_account.try_borrow_data()?;
+ if data.len() < 8 || &data[0..8] != CPI_CONTEXT_ACCOUNT_1_DISCRIMINATOR {
+ return Err(SystemProgramError::InvalidCpiContextDiscriminator.into());
+ }
+ drop(data);
+
let associated_merkle_tree = {
let data = cpi_context_account.try_borrow_data()?;
CpiContextAccount::deserialize(&mut &data[8..])
.map_err(|_| ProgramError::BorshIoError)?
.associated_merkle_tree
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In programs/system/src/accounts/init_context_account.rs around lines 68 to 85,
before calling try_borrow_data and deserializing the account, validate the
account owner and the Borsh/discriminator: ensure accounts[0].owner equals the
expected program id (the program that owns CpiContextAccount) and verify the
data length is at least the discriminator + payload size, then check the first 8
bytes (or the crate's defined discriminator) match CpiContextAccount's
discriminator; only after those checks proceed to deserialize &data[8..] and
extract associated_merkle_tree, otherwise return an appropriate ProgramError
(e.g., IllegalOwner or InvalidAccountData).
| #[cfg(feature = "reinit")] | ||
| RE_INIT_CPI_CONTEXT_ACCOUNT_INSTRUCTION => { | ||
| Ok(InstructionDiscriminator::ReInitCpiContextAccount) | ||
| } |
There was a problem hiding this comment.
Consider security implications of feature-gated instruction.
The RE_INIT_CPI_CONTEXT_ACCOUNT_INSTRUCTION discriminator is gated behind cfg(feature = "reinit"), which means it can be conditionally compiled out. This could lead to runtime errors if the feature flag state differs between compilation and deployment environments.
Consider these potential issues:
- If the feature is disabled in production but enabled during testing, the discriminator won't be recognized
- This could lead to
SystemProgramError::InvalidInstructionDataDiscriminatorerrors in production - Runtime feature detection would be safer than compile-time gating
// Consider adding runtime validation instead of compile-time gating
match array {
// ... other discriminators
RE_INIT_CPI_CONTEXT_ACCOUNT_INSTRUCTION => {
if !is_reinit_feature_enabled() {
return Err(SystemProgramError::FeatureNotEnabled.into());
}
Ok(InstructionDiscriminator::ReInitCpiContextAccount)
}
// ...
}🤖 Prompt for AI Agents
In programs/system/src/constants.rs around lines 39 to 42, the
RE_INIT_CPI_CONTEXT_ACCOUNT_INSTRUCTION discriminator is feature-gated with
cfg(feature = "reinit"), which can cause mismatches between compile-time
features and runtime expectations; remove the compile-time gating for this
discriminator and instead keep the discriminator match arm always present,
adding a runtime check that calls a feature-enabled validator (e.g.
is_reinit_feature_enabled()) before returning
InstructionDiscriminator::ReInitCpiContextAccount, and return a clear
SystemProgramError::FeatureNotEnabled when the runtime check fails so the
instruction is rejected safely at runtime.
| #[profile] | ||
| fn check_tree_height<const IS_STATE_TREE: bool>( | ||
| tree_height: &mut u8, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same here: gate the attribute; consider skipping tiny leafs
check_tree_height is trivial; profiling it adds noise with little value. Gate it, and consider removing profiling from this leaf entirely.
Apply:
-#[profile]
+#[cfg_attr(feature = "profile-program", light_program_profiler::profile)]📝 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.
| #[profile] | |
| fn check_tree_height<const IS_STATE_TREE: bool>( | |
| tree_height: &mut u8, | |
| #[cfg_attr(feature = "profile-program", light_program_profiler::profile)] | |
| fn check_tree_height<const IS_STATE_TREE: bool>( | |
| tree_height: &mut u8, |
🤖 Prompt for AI Agents
In programs/system/src/processor/verify_proof.rs around lines 64 to 66, the
#[profile] attribute on check_tree_height is unnecessary noise for a trivial
function; either remove the attribute entirely or gate it behind a compile
feature so profiling is opt-in. Change the file so the attribute is wrapped with
a conditional compilation guard (e.g. only applied when a "profiling" feature or
cfg is enabled) or delete the #[profile] line to skip profiling for this tiny
leaf function.
| #[profile] | ||
| pub fn read_address_roots<'a, 'b: 'a>( | ||
| remaining_accounts: &[AcpAccount<'_>], | ||
| new_address_params: impl Iterator<Item = &'a (dyn NewAddress<'b> + 'a)>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate profiling for read_address_roots
Hot path; ensure profiling is feature‑toggled.
Apply:
-#[profile]
+#[cfg_attr(feature = "profile-program", light_program_profiler::profile)]📝 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.
| #[profile] | |
| pub fn read_address_roots<'a, 'b: 'a>( | |
| remaining_accounts: &[AcpAccount<'_>], | |
| new_address_params: impl Iterator<Item = &'a (dyn NewAddress<'b> + 'a)>, | |
| #[cfg_attr(feature = "profile-program", light_program_profiler::profile)] | |
| pub fn read_address_roots<'a, 'b: 'a>( | |
| remaining_accounts: &[AcpAccount<'_>], | |
| new_address_params: impl Iterator<Item = &'a (dyn NewAddress<'b> + 'a)>, |
🤖 Prompt for AI Agents
In programs/system/src/processor/verify_proof.rs around lines 87–90, the
#[profile] attribute is on a hot path and must be feature‑gated; replace the
bare attribute with a conditional one so profiling is only enabled when a cargo
feature is set (e.g. change to #[cfg_attr(feature = "profiling", profile)] on
the function) and add the "profiling" feature to the crate's Cargo.toml if not
present so the attribute is compiled in only when explicitly enabled.
| #[profile] | ||
| fn read_root<const IS_READ_ONLY: bool, const IS_STATE: bool>( | ||
| merkle_tree_account: &AcpAccount<'_>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate profiling for read_root; this is a very hot helper
Profiling this tight helper can significantly skew CU usage; enable only when needed. Optionally profile just the legacy branches if that’s what you’re targeting.
Apply:
-#[profile]
+#[cfg_attr(feature = "profile-program", light_program_profiler::profile)]📝 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.
| #[profile] | |
| fn read_root<const IS_READ_ONLY: bool, const IS_STATE: bool>( | |
| merkle_tree_account: &AcpAccount<'_>, | |
| #[cfg_attr(feature = "profile-program", light_program_profiler::profile)] | |
| fn read_root<const IS_READ_ONLY: bool, const IS_STATE: bool>( | |
| merkle_tree_account: &AcpAccount<'_>, |
🤖 Prompt for AI Agents
In programs/system/src/processor/verify_proof.rs around lines 116–118, the
#[profile] attribute is applied unconditionally to the very hot read_root helper
which skews CU usage; gate the profiling so it only runs when explicitly
enabled: replace the unconditional attribute with a cfg-gated form (e.g., use
cfg_attr(feature = "profile_read_root", profile) or cfg(feature =
"profile_read_root") to enable profiling only when the feature is enabled), and
if you only want to profile legacy branches consider applying the
cfg_attr/profile only to those specific code paths instead of the whole
function.
| #[allow(clippy::too_many_arguments)] | ||
| #[profile] | ||
| pub fn verify_proof( | ||
| roots: &[[u8; 32]], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate profiling for verify_proof
Top‑level is a good place to keep profiling; still, feature‑gate it.
Apply:
-#[profile]
+#[cfg_attr(feature = "profile-program", light_program_profiler::profile)]📝 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.
| #[allow(clippy::too_many_arguments)] | |
| #[profile] | |
| pub fn verify_proof( | |
| roots: &[[u8; 32]], | |
| #[allow(clippy::too_many_arguments)] | |
| #[cfg_attr(feature = "profile-program", light_program_profiler::profile)] | |
| pub fn verify_proof( | |
| roots: &[[u8; 32]], |
🤖 Prompt for AI Agents
In programs/system/src/processor/verify_proof.rs around lines 161 to 164, the
#[profile] attribute on verify_proof should be feature-gated; replace it with a
cfg_attr so profiling is only enabled when the profiling feature is turned on
(for example: #[cfg_attr(feature = "gate_profiling", profile)]), keeping the
attribute semantics but preventing it from compiling in when the feature is not
enabled.
| pub fn to_account_infos(&self) -> Vec<T> { | ||
| // Skip system light program | ||
| let refs = &self.account_infos()[1..]; | ||
| let mut account_infos = Vec::with_capacity(1 + refs.len()); | ||
| account_infos.push(self.fee_payer().clone()); | ||
| account_infos.extend_from_slice(refs); | ||
| account_infos | ||
| } |
There was a problem hiding this comment.
Breaking change: to_account_infos now returns owned Vec instead of Vec<&'a T>.
The change from returning references to owned values is a breaking API change. The method now requires T: Clone and returns cloned accounts. This change should be clearly documented in migration guides as it will require updates in all calling code.
Ensure that:
- All consumers of this API are updated
- Performance implications of cloning are acceptable
- Migration documentation is provided
🤖 Prompt for AI Agents
In sdk-libs/sdk-types/src/cpi_accounts.rs around lines 276 to 283, the method
was changed to return an owned Vec<T> (requiring T: Clone and cloning account
entries) which is a breaking API change; restore the previous non-owning API by
returning Vec<&'a T> (or add a separate owned-returning method if ownership
semantics are desired). Specifically, change the signature back to Vec<&'a T>,
build the vector of references (skip index 0 as before), reserve appropriate
capacity, push a reference to fee_payer and extend with the slice of refs as
references, and remove any Clone bounds; alternatively, if you must keep the
owned-returning variant, add a new method name (e.g., to_owned_account_infos)
and document the new behavior and performance implications in the migration
notes so callers can opt-in.
… compression program nullify create output order
ananas-block
force-pushed
the
jorrit/refactor-cpi-context
branch
from
281bf48 to
686213b
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs (1)
10-68: Remove or gate the commented-out public API in sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rsto_account_metas_small is commented out in the pinocchio crate but the symbol is still present/used elsewhere; either delete the commented block, restore it behind a cfg/feature, or update callers to the active implementation.
- Commented block: sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs (lines ~10–68)
- Active implementation: sdk-libs/sdk/src/cpi/accounts_small_ix.rs:131
- Call sites: sdk-libs/sdk/src/cpi/invoke.rs:137; program-tests/create-address-test-program/src/lib.rs:79
- Test-only manual impl: program-tests/create-address-test-program/src/lib.rs:258
programs/system/src/processor/create_outputs_cpi_data.rs (1)
101-107: Don’t unwrap merkle context; return a program error insteadUnwrapping can panic at runtime. Propagate a concrete error.
Apply:
- let merkle_context = context - .get_legacy_merkle_context(current_index as u8) - .unwrap(); + let merkle_context = context + .get_legacy_merkle_context(current_index as u8) + .ok_or(SystemProgramError::InputMerkleTreeIndexOutOfBounds)?;
🧹 Nitpick comments (27)
program-tests/utils/src/lib.rs (1)
31-31: Clarify legacy/test-only intent with a module-level doc comment (or make it private if not needed).If this is consumed only within this crate, prefer
modoverpub mod. Otherwise, add a brief doc to signal its legacy scope.- pub mod legacy_cpi_context_account; + /// Legacy CPI-context account helpers for tests; keep until v2 rollout completes. + pub mod legacy_cpi_context_account;programs/system/src/processor/create_inputs_cpi_data.rs (3)
57-60: Consider extracting the error message to a constant.The match expression handling is correct, but the error handling could be more maintainable.
+const INPUT_MERKLE_TREE_INDEX_ERROR: &str = "Input merkle tree index out of bounds"; + current_hashed_mt = match &accounts .get(current_mt_index as usize) - .ok_or(SystemProgramError::InputMerkleTreeIndexOutOfBounds)? + .ok_or_else(|| { + msg!(INPUT_MERKLE_TREE_INDEX_ERROR); + SystemProgramError::InputMerkleTreeIndexOutOfBounds + })?
85-85: Replace debug print with structured logging.The
msg!call with format string concatenation is not ideal for production code and could impact performance.- msg!(format!("create_inputs_cpi_data {} ", current_mt_index).as_str()); + msg!("Invalid account type for merkle tree index: {}", current_mt_index);
109-109: Remove unnecessary empty line.The blank line after the tree index assignment is unnecessary and affects code consistency.
)?; - cpi_ix_data.nullifiers[j] = InsertNullifierInput {programs/system/src/accounts/account_checks.rs (1)
75-88: Minor: rename local vars for clarity (generic helper)
Variable names reference “decompression_recipient” in a generic helper; rename to avoid confusion.Apply:
pub fn anchor_option_mut_account_info( account_info: Option<&AccountInfo>, ) -> Result<Option<&AccountInfo>> { - let option_decompression_recipient = account_info.ok_or(ProgramError::NotEnoughAccountKeys)?; - let decompression_recipient = if *option_decompression_recipient.key() == crate::ID { + let option_account_info = account_info.ok_or(ProgramError::NotEnoughAccountKeys)?; + let result = if *option_account_info.key() == crate::ID { None } else { - check_mut(option_decompression_recipient).map_err(ProgramError::from)?; - Some(option_decompression_recipient) + check_mut(option_account_info).map_err(ProgramError::from)?; + Some(option_account_info) }; - Ok(decompression_recipient) + Ok(result) }sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs (3)
37-39: Prefer explicit System Program ID overPubkey::default()for clarity.Default happens to equal System Program, but be explicit to avoid ambiguity.
Apply:
- // 6. System Program (readonly) - always default pubkey - account_metas.push(AccountMeta::readonly(&Pubkey::default())); + // 6. System Program (readonly) + account_metas.push(AccountMeta::readonly(&pinocchio::system_program::ID));
51-53: CPI-context account ordering: ensure it matches the new “CPI context first” spec.Right now
cpi_contextis appended among optionals; when re-enabling, push CPI-context accounts first to match the v2 ordering.Would you like me to propose an updated ordering here consistent with the system-wide spec?
55-57: Deterministic tree account order.Ensure
tree_accounts()returns a stable order (or sort by key) to avoid nondeterministic metas across runs.programs/system/src/invoke_cpi/verify_signer.rs (1)
128-129: Test ignore: prefer cfg-based gating over blanket ignore.
Consider cfg_attr to only ignore on non-Solana targets (or behind a feature), so the test still runs where supported.programs/system/src/processor/create_address_cpi_data.rs (1)
53-59: Simplify invoking-program-id selection; avoid shadowing.Use
Option::orto preferowner()and fall back to context, with tighter typing.Apply:
- let invoking_program_id_bytes = if let Some(bytes) = new_address_params.owner() { - Ok(bytes) - } else if let Some(invoking_program_id_clone) = invoking_program_id_clone.as_ref() { - Ok(invoking_program_id_clone) - } else { - Err(SystemProgramError::DeriveAddressError) - }?; + let invoking_program_id_bytes = new_address_params + .owner() + .or(invoking_program_id_clone.as_ref()) + .ok_or(SystemProgramError::DeriveAddressError)?;programs/system/src/invoke_cpi/processor.rs (1)
63-66: Clarify “deserialize to clear” side‑effect.Add a brief comment to make the clear-on-deserialize semantics explicit for maintainers.
Apply:
// 4. clear cpi context account if cpi_context_inputs_len > 0 { - deserialize_cpi_context_account_cleared(accounts.get_cpi_context_account().unwrap())?; + // Reads with `cleared = true` to zero/reset the CPI context. + // Dropping the returned zero-copy guard persists the cleared state. + deserialize_cpi_context_account_cleared(accounts.get_cpi_context_account().unwrap())?; }programs/system/Cargo.toml (1)
29-31: Consider feature-gating optional profiling dependenciesSimilar to the compressed-account crate, the profiling dependencies could be made optional to reduce binary size when profiling is disabled.
-profile-program = ["light-program-profiler/profile-program"] -profile-heap = ["light-program-profiler/profile-heap", "dep:light-heap"] +profile-program = ["dep:light-program-profiler", "light-program-profiler/profile-program", "dep:solana-msg"] +profile-heap = ["dep:light-program-profiler", "light-program-profiler/profile-heap", "dep:light-heap", "dep:solana-msg"] [dependencies] -solana-msg = { workspace = true } -light-program-profiler = { workspace = true } +solana-msg = { workspace = true, optional = true } +light-program-profiler = { workspace = true, optional = true }sdk-libs/sdk/src/cpi/mod.rs (1)
51-53: Clarify CPI module organization and document preferred APIaccounts_cpi_context is compiled unconditionally; accounts_small_ix is gated by feature "v2" and sdk-libs/sdk/src/cpi/accounts.rs still exists. Add module-level docs (sdk-libs/sdk/src/cpi/mod.rs) and top-of-file comments in accounts_cpi_context.rs, accounts_small_ix.rs, and accounts.rs that state:
- which module to use for new code,
- the migration path between them,
- whether accounts_small_ix is deprecated or retained for compatibility (consider #[deprecated] or a feature-note in README).
programs/system/src/processor/create_outputs_cpi_data.rs (1)
182-183: Log content may be noisy; confirm acceptabilityLogging the entire context.addresses vector could be verbose and leak state in logs. Consider trimming or hashing entries.
sdk-libs/sdk-types/src/instruction/tree_info.rs (1)
32-39: Extra Clone bound may be unnecessaryget_tree_pubkey doesn’t use Clone. If not required by callers, drop the Clone bound to keep the API maximally generic.
programs/system/tests/invoke_cpi_instruction_small.rs (1)
115-126: Improve null safety in test assertions.The test uses
unwrap()on Line 123 after checkingis_some()on Lines 115-121, but then usesunwrap()again without the safety check. Consider using consistent null-safe patterns.- assert!(invoke_cpi_instruction_small - .exec_accounts - .as_ref() - .unwrap() - .sol_pool_pda - .is_none()); - assert!(invoke_cpi_instruction_small - .exec_accounts - .unwrap() - .decompression_recipient - .is_none()); + let exec_accounts = invoke_cpi_instruction_small + .exec_accounts + .as_ref() + .unwrap(); + assert!(exec_accounts.sol_pool_pda.is_none()); + assert!(exec_accounts.decompression_recipient.is_none());programs/system/src/cpi_context/process_cpi_context.rs (1)
232-242: Consider using debug-only logging for merkle tree mismatch.The error message includes debug information using
solana_pubkey::Pubkey::new_from_array. Consider wrapping this in a debug-only block to reduce binary size in production.if *cpi_context_account.associated_merkle_tree != first_merkle_tree_pubkey.to_pubkey_bytes() { - msg!(format!( - "first_merkle_tree_pubkey {:?} != associated_merkle_tree {:?}", - solana_pubkey::Pubkey::new_from_array(first_merkle_tree_pubkey), - solana_pubkey::Pubkey::new_from_array( - cpi_context_account.associated_merkle_tree.to_bytes() - ) - ) - .as_str()); + #[cfg(debug_assertions)] + { + msg!(format!( + "first_merkle_tree_pubkey {:?} != associated_merkle_tree {:?}", + solana_pubkey::Pubkey::new_from_array(first_merkle_tree_pubkey), + solana_pubkey::Pubkey::new_from_array( + cpi_context_account.associated_merkle_tree.to_bytes() + ) + ) + .as_str()); + } return Err(SystemProgramError::CpiContextAssociatedMerkleTreeMismatch.into()); }programs/system/tests/cpi_context.rs (1)
134-145: Consider making this helper function public for reuse.The
calculate_vector_offsetsfunction is a useful utility that could benefit other test modules working with zero-copy structures.-#[inline] -fn calculate_vector_offsets( +#[inline] +pub fn calculate_vector_offsets(program-libs/account-checks/src/account_iterator.rs (3)
95-102: Position advances on failed checks; confirm intended consumption semantics.
next_signer_mutconsumes the next account (advancesposition) before validation. On failure, the iterator state cannot be rewound. If callers expect retry/recovery, consider peeking before advancing.Example (preserve position on failure):
pub fn next_signer_mut(&mut self, account_name: &str) -> Result<&'info T, AccountError> { - let account_info = self.next_signer(account_name)?; - check_mut(account_info) + let pos = self.position; + let account_info = self.next_signer(account_name)?; + if let Err(e) = check_mut(account_info) { + // restore position so caller can decide + self.position = pos; + self.print_on_error(&e, account_name, Location::caller()); + return Err(e); + } .inspect_err(|e| self.print_on_error(e, account_name, Location::caller()))?; Ok(account_info) }
140-154: remaining() should not error on zero remaining accounts.Returning
Err(NotEnoughAccountKeys)when none remain is surprising. PreferOk(&[])and reserve errors for out‑of‑bounds consumption.Apply:
pub fn remaining(&self) -> Result<&'info [T], AccountError> { - if self.position >= self.accounts.len() { - let location = Location::caller(); - let account_name = "remaining accounts"; - solana_msg::msg!( - "ERROR: Not enough accounts. Requested '{}' at index {} but only {} accounts available. {}:{}:{}", - account_name, self.position, self.accounts.len(), location.file(), location.line(), location.column() - ); - return Err(AccountError::NotEnoughAccountKeys); - } - Ok(&self.accounts[self.position..]) + Ok(if self.position >= self.accounts.len() { + &[] + } else { + &self.accounts[self.position..] + }) }
190-202: Log string nit: stray period and spacing.Message reads
"ERROR: {}. for account ... {}:{}:{}"; consider removing the extra period and double space for cleaner logs.- "ERROR: {}. for account '{}' at index {} {}:{}:{}", + "ERROR: {} for account '{}' at index {} {}:{}:{}",programs/system/src/cpi_context/state.rs (2)
185-201: Replacereturn Err(...)?with plainreturn Err(...).
return Err(SystemProgramError::Unimplemented)?;is redundant and confusing.- msg!("readonly_addresses are not supported when writing into cpi context account"); - return Err(SystemProgramError::Unimplemented)?; + msg!("readonly_addresses are not supported when writing into cpi context account"); + return Err(SystemProgramError::Unimplemented);Repeat for the read_only_accounts branch.
328-340: Clearing path: consider zeroing only payload, not entire tail.
data.fill(0)wipes the entire remaining region. If subsequent fields are appended after output slices, this may be wasteful. Zero only the vector payload regions if needed.sdk-libs/sdk/src/cpi/invoke.rs (4)
61-72: Constructor naming: clarify “assigned” vs. “packed”.
new_with_assigned_addressparallelsnew_with_addressbut only supports assigned addresses elsewhere. Consider deprecating the generic constructor or documenting when each should be used to avoid runtimeunimplemented!.
103-144: Avoid runtime panics for unsupportednew_addresses.Using
unimplemented!will abort the transaction. Return a typed error for better UX and testability.- if cpi_inputs.new_addresses.is_some() { - unimplemented!("new_addresses must be new assigned addresses."); - } + if cpi_inputs.new_addresses.is_some() { + return Err(LightSdkError::Unsupported("new_addresses must be new assigned addresses").into()); + }Apply similarly in the CPI-context path below.
146-186: Duplicate builder logic between “small” and “cpi_context” variants.The input construction is duplicated. Extract a shared helper to reduce drift.
Example:
fn build_with_account_info_inputs( cpi_inputs: CpiInputs, bump: u8, invoking_program_id: Pubkey, ) -> Result<Vec<u8>> { /* build InstructionDataInvokeCpiWithAccountInfo and serialize */ }
103-106: Typo in function name (“progam”).Public function
create_light_system_progam_instruction_invoke_cpi_smallhas a typo; consider adding a correctly spelled alias and deprecating the old name to avoid breaking changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
.cargo/config.toml(0 hunks)anchor-programs/system/Cargo.toml(1 hunks)anchor-programs/system/src/cpi_context_account.rs(2 hunks)anchor-programs/system/src/lib.rs(1 hunks)cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/epoch_pda_34w7KcLBXabMkHuXE2fY368vFe6kP3v5EJn8nPvQ8SKn.json(1 hunks)cli/accounts/forester_epoch_pda_3FBt1BPQHCQkS8k3wrUXMfB6JBhtMhEqQXueHRw2ojZV.json(1 hunks)cli/accounts/governance_authority_pda_CuEtcKkkbTn6qy2qxqDswq5U2ADsqoipYDAYfRvxPjcp.json(1 hunks)cli/accounts/registered_forester_pda_2KNqEh23Se8AHecuzR1UkxL26euq2qXSpQPTH1jH7VqU.json(1 hunks)program-libs/account-checks/Cargo.toml(1 hunks)program-libs/account-checks/src/account_info/pinocchio.rs(1 hunks)program-libs/account-checks/src/account_iterator.rs(1 hunks)program-libs/account-checks/src/error.rs(2 hunks)program-libs/account-checks/src/lib.rs(1 hunks)program-libs/account-checks/src/packed_accounts.rs(1 hunks)program-libs/account-checks/tests/account_iterator.rs(1 hunks)program-libs/compressed-account/Cargo.toml(1 hunks)program-libs/compressed-account/src/compressed_account.rs(6 hunks)program-libs/compressed-account/src/indexer_event/parse.rs(5 hunks)program-libs/compressed-account/src/instruction_data/compressed_proof.rs(2 hunks)program-libs/compressed-account/src/instruction_data/cpi_context.rs(2 hunks)program-libs/compressed-account/src/instruction_data/data.rs(3 hunks)program-libs/compressed-account/src/instruction_data/invoke_cpi.rs(2 hunks)program-libs/compressed-account/src/instruction_data/mod.rs(1 hunks)program-libs/compressed-account/src/instruction_data/traits.rs(3 hunks)program-libs/compressed-account/src/instruction_data/with_account_info.rs(5 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(5 hunks)program-libs/compressed-account/src/instruction_data/zero_copy.rs(6 hunks)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs(1 hunks)program-libs/compressed-account/src/lib.rs(2 hunks)program-libs/compressed-account/src/pubkey.rs(3 hunks)program-libs/compressed-account/tests/account_option_config.rs(1 hunks)program-libs/compressed-account/tests/pubkey.rs(1 hunks)program-libs/compressed-account/tests/zero_copy_set.rs(1 hunks)program-libs/hasher/Cargo.toml(2 hunks)program-libs/hasher/src/to_byte_array.rs(1 hunks)program-libs/zero-copy/src/vec.rs(2 hunks)program-tests/compressed-token-test/tests/test.rs(1 hunks)program-tests/create-address-test-program/Cargo.toml(1 hunks)program-tests/create-address-test-program/src/lib.rs(5 hunks)program-tests/sdk-anchor-test/package.json(0 hunks)program-tests/system-cpi-test/tests/test.rs(4 hunks)program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs(6 hunks)program-tests/system-test/Cargo.toml(2 hunks)program-tests/system-test/tests/test_re_init_cpi_account.rs(1 hunks)program-tests/utils/Cargo.toml(1 hunks)program-tests/utils/src/assert_rollover.rs(1 hunks)program-tests/utils/src/legacy_cpi_context_account.rs(1 hunks)program-tests/utils/src/lib.rs(1 hunks)programs/account-compression/src/processor/insert_into_queues.rs(1 hunks)programs/compressed-token/src/process_transfer.rs(2 hunks)programs/package.json(1 hunks)programs/system/Cargo.toml(2 hunks)programs/system/README.md(0 hunks)programs/system/src/account_compression_state/address.rs(3 hunks)programs/system/src/accounts/account_checks.rs(7 hunks)programs/system/src/accounts/account_traits.rs(1 hunks)programs/system/src/accounts/init_context_account.rs(3 hunks)programs/system/src/accounts/remaining_account_checks.rs(5 hunks)programs/system/src/constants.rs(2 hunks)programs/system/src/context.rs(13 hunks)programs/system/src/cpi_context/account.rs(1 hunks)programs/system/src/cpi_context/address.rs(1 hunks)programs/system/src/cpi_context/instruction_data_trait.rs(1 hunks)programs/system/src/cpi_context/mod.rs(1 hunks)programs/system/src/cpi_context/process_cpi_context.rs(1 hunks)programs/system/src/cpi_context/state.rs(1 hunks)programs/system/src/errors.rs(2 hunks)programs/system/src/invoke/instruction.rs(1 hunks)programs/system/src/invoke_cpi/account.rs(0 hunks)programs/system/src/invoke_cpi/instruction.rs(1 hunks)programs/system/src/invoke_cpi/instruction_small.rs(2 hunks)programs/system/src/invoke_cpi/mod.rs(0 hunks)programs/system/src/invoke_cpi/processor.rs(3 hunks)programs/system/src/invoke_cpi/verify_signer.rs(6 hunks)programs/system/src/lib.rs(5 hunks)programs/system/src/processor/cpi.rs(5 hunks)programs/system/src/processor/create_address_cpi_data.rs(2 hunks)programs/system/src/processor/create_inputs_cpi_data.rs(5 hunks)programs/system/src/processor/create_outputs_cpi_data.rs(9 hunks)programs/system/src/processor/process.rs(6 hunks)programs/system/src/processor/read_only_account.rs(2 hunks)programs/system/src/processor/read_only_address.rs(1 hunks)programs/system/src/processor/sol_compression.rs(8 hunks)programs/system/src/processor/sum_check.rs(1 hunks)programs/system/src/processor/verify_proof.rs(6 hunks)programs/system/src/utils.rs(2 hunks)programs/system/tests/cpi_context.rs(1 hunks)programs/system/tests/invoke_cpi_instruction_small.rs(21 hunks)programs/system/tests/invoke_instruction.rs(1 hunks)sdk-libs/sdk-pinocchio/Cargo.toml(0 hunks)sdk-libs/sdk-pinocchio/src/cpi/accounts_small.rs(2 hunks)sdk-libs/sdk-pinocchio/src/cpi/mod.rs(1 hunks)sdk-libs/sdk-types/Cargo.toml(0 hunks)sdk-libs/sdk-types/src/constants.rs(1 hunks)sdk-libs/sdk-types/src/cpi_accounts.rs(5 hunks)sdk-libs/sdk-types/src/cpi_accounts_small.rs(4 hunks)sdk-libs/sdk-types/src/cpi_context_write.rs(1 hunks)sdk-libs/sdk-types/src/instruction/tree_info.rs(1 hunks)sdk-libs/sdk-types/src/lib.rs(2 hunks)sdk-libs/sdk/Cargo.toml(1 hunks)sdk-libs/sdk/src/cpi/accounts_cpi_context.rs(1 hunks)sdk-libs/sdk/src/cpi/accounts_small_ix.rs(1 hunks)sdk-libs/sdk/src/cpi/invoke.rs(4 hunks)sdk-libs/sdk/src/cpi/mod.rs(1 hunks)
⛔ Files not processed due to max files limit (6)
- sdk-libs/sdk/src/error.rs
- sdk-libs/sdk/src/instruction/pack_accounts.rs
- sdk-libs/sdk/src/instruction/system_accounts.rs
- sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/test.rs
- sdk-tests/sdk-native-test/tests/test.rs
- sdk-tests/sdk-pinocchio-test/tests/test.rs
💤 Files with no reviewable changes (7)
- programs/system/README.md
- programs/system/src/invoke_cpi/mod.rs
- programs/system/src/invoke_cpi/account.rs
- .cargo/config.toml
- program-tests/sdk-anchor-test/package.json
- sdk-libs/sdk-pinocchio/Cargo.toml
- sdk-libs/sdk-types/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (55)
- cli/accounts/epoch_pda_34w7KcLBXabMkHuXE2fY368vFe6kP3v5EJn8nPvQ8SKn.json
- sdk-libs/sdk-pinocchio/src/cpi/mod.rs
- program-libs/compressed-account/src/instruction_data/invoke_cpi.rs
- program-tests/utils/Cargo.toml
- cli/accounts/governance_authority_pda_CuEtcKkkbTn6qy2qxqDswq5U2ADsqoipYDAYfRvxPjcp.json
- program-tests/utils/src/assert_rollover.rs
- cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json
- programs/system/src/processor/read_only_account.rs
- program-tests/compressed-token-test/tests/test.rs
- cli/accounts/forester_epoch_pda_3FBt1BPQHCQkS8k3wrUXMfB6JBhtMhEqQXueHRw2ojZV.json
- programs/system/src/accounts/remaining_account_checks.rs
- program-tests/utils/src/legacy_cpi_context_account.rs
- programs/system/src/invoke/instruction.rs
- sdk-libs/sdk-types/src/lib.rs
- program-libs/compressed-account/src/lib.rs
- programs/system/src/invoke_cpi/instruction.rs
- sdk-libs/sdk-types/src/cpi_context_write.rs
- program-libs/hasher/Cargo.toml
- programs/system/src/processor/read_only_address.rs
- programs/system/src/cpi_context/instruction_data_trait.rs
- programs/system/src/account_compression_state/address.rs
- program-libs/compressed-account/src/instruction_data/zero_copy_set.rs
- programs/system/src/cpi_context/address.rs
- programs/package.json
- anchor-programs/system/Cargo.toml
- program-libs/account-checks/tests/account_iterator.rs
- programs/system/src/processor/sum_check.rs
- program-libs/compressed-account/src/instruction_data/compressed_proof.rs
- program-libs/compressed-account/src/instruction_data/traits.rs
- sdk-libs/sdk/src/cpi/accounts_cpi_context.rs
- program-libs/zero-copy/src/vec.rs
- program-libs/account-checks/src/error.rs
- program-libs/compressed-account/tests/pubkey.rs
- cli/accounts/registered_forester_pda_2KNqEh23Se8AHecuzR1UkxL26euq2qXSpQPTH1jH7VqU.json
- programs/system/src/processor/sol_compression.rs
- program-libs/account-checks/src/account_info/pinocchio.rs
- program-libs/account-checks/src/lib.rs
- program-libs/hasher/src/to_byte_array.rs
- cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json
- program-libs/compressed-account/src/compressed_account.rs
- programs/system/tests/invoke_instruction.rs
- programs/account-compression/src/processor/insert_into_queues.rs
- programs/system/src/accounts/account_traits.rs
- anchor-programs/system/src/lib.rs
- sdk-libs/sdk-types/src/constants.rs
- program-tests/system-cpi-test/tests/test.rs
- program-tests/create-address-test-program/Cargo.toml
- program-libs/compressed-account/tests/zero_copy_set.rs
- programs/compressed-token/src/process_transfer.rs
- program-libs/account-checks/Cargo.toml
- program-tests/system-test/tests/test_re_init_cpi_account.rs
- sdk-libs/sdk/Cargo.toml
- programs/system/src/utils.rs
- program-tests/system-test/Cargo.toml
- program-libs/compressed-account/src/indexer_event/parse.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T00:36:53.191Z
Learnt from: ananas-block
PR: Lightprotocol/light-protocol#1909
File: program-libs/zero-copy/src/init_mut.rs:241-249
Timestamp: 2025-08-14T00:36:53.191Z
Learning: In the light-protocol zero-copy crate, performance is prioritized over safety checks for edge cases like Vec lengths exceeding u32::MAX, even when there might be wire format inconsistencies.
Applied to files:
program-libs/compressed-account/src/pubkey.rsprogram-libs/compressed-account/src/instruction_data/with_readonly.rs
🧬 Code graph analysis (30)
programs/system/src/cpi_context/mod.rs (6)
programs/system/src/cpi_context/account.rs (2)
address(61-67)address(121-127)program-libs/compressed-account/src/instruction_data/traits.rs (2)
address(48-48)address(69-69)program-libs/compressed-account/src/instruction_data/with_account_info.rs (2)
address(82-84)address(132-134)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
address(84-86)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
address(133-135)address(346-348)programs/system/src/cpi_context/process_cpi_context.rs (1)
process_cpi_context(48-88)
programs/system/src/lib.rs (7)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
cpi_context(22-22)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
cpi_context(354-364)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
cpi_context(312-322)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
cpi_context(487-489)cpi_context(616-624)programs/system/src/context.rs (1)
cpi_context(351-353)programs/system/src/cpi_context/instruction_data_trait.rs (1)
cpi_context(66-68)programs/system/src/accounts/init_context_account.rs (1)
reinit_cpi_context_account(69-85)
program-libs/compressed-account/src/instruction_data/cpi_context.rs (3)
program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
first_set_context(661-663)set_context(657-659)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs (2)
first_set_context(122-122)set_context(121-121)js/stateless.js/src/state/types.ts (1)
CompressedCpiContext(353-368)
program-libs/account-checks/src/packed_accounts.rs (1)
program-libs/account-checks/src/account_iterator.rs (1)
len(175-177)
programs/system/src/invoke_cpi/processor.rs (2)
programs/system/src/invoke_cpi/process_cpi_context.rs (1)
process_cpi_context(37-90)programs/system/src/cpi_context/state.rs (1)
deserialize_cpi_context_account_cleared(283-287)
programs/system/src/invoke_cpi/instruction_small.rs (4)
programs/system/src/accounts/account_checks.rs (3)
check_option_cpi_context_account(134-167)check_option_decompression_recipient(117-130)check_option_sol_pool_pda(170-183)programs/system/src/invoke/instruction.rs (5)
from_account_infos(42-84)get_registered_program_pda(98-100)get_account_compression_authority(102-104)get_sol_pool_pda(106-108)get_decompression_recipient(110-112)program-libs/account-checks/src/account_iterator.rs (1)
new(22-28)programs/system/src/accounts/account_traits.rs (4)
get_registered_program_pda(6-6)get_account_compression_authority(7-7)get_sol_pool_pda(8-8)get_decompression_recipient(9-9)
programs/system/tests/cpi_context.rs (4)
anchor-programs/system/src/cpi_context_account.rs (1)
deserialize_cpi_context_account(50-72)programs/system/src/cpi_context/state.rs (2)
deserialize_cpi_context_account_cleared(283-287)output_data_len(74-76)programs/system/src/errors.rs (2)
from(158-233)from(237-239)programs/system/src/invoke_cpi/process_cpi_context.rs (1)
tests(227-812)
programs/system/src/context.rs (5)
program-libs/compressed-account/src/instruction_data/traits.rs (3)
cpi_context(22-22)output_accounts(16-16)input_accounts(15-15)program-libs/compressed-account/src/instruction_data/zero_copy.rs (6)
cpi_context(487-489)cpi_context(616-624)output_accounts(483-485)output_accounts(607-609)input_accounts(478-481)input_accounts(611-614)programs/system/src/cpi_context/instruction_data_trait.rs (3)
cpi_context(66-68)output_accounts(30-32)input_accounts(26-28)programs/system/src/cpi_context/process_cpi_context.rs (1)
set_cpi_context(91-137)programs/system/src/invoke_cpi/process_cpi_context.rs (1)
set_cpi_context(92-135)
program-libs/compressed-account/src/instruction_data/data.rs (3)
js/stateless.js/src/state/types.ts (1)
OutputCompressedAccountWithPackedContext(170-173)js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)js/stateless.js/src/utils/address.ts (1)
NewAddressParamsPacked(64-83)
program-libs/compressed-account/tests/account_option_config.rs (2)
js/stateless.js/src/state/types.ts (2)
CompressedCpiContext(353-368)InstructionDataInvokeCpi(311-345)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
with_cpi_context(245-247)
program-libs/compressed-account/src/instruction_data/with_account_info.rs (3)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_readonly.rs (6)
account_option_config(265-283)with_cpi_context(245-247)rng(676-685)rng(701-721)rng(722-728)rng(729-740)program-libs/compressed-account/src/instruction_data/zero_copy.rs (3)
account_option_config(442-444)account_option_config(560-572)get_rnd_cpi_context(1014-1020)
sdk-libs/sdk/src/cpi/accounts_small_ix.rs (6)
sdk-libs/sdk-types/src/cpi_accounts.rs (1)
config(223-225)sdk-libs/sdk-types/src/cpi_accounts_small.rs (1)
config(119-121)sdk-libs/sdk/src/error.rs (3)
from(92-94)from(98-125)from(129-170)sdk-libs/sdk/src/instruction/system_accounts.rs (1)
default(48-70)sdk-libs/sdk/src/cpi/accounts.rs (1)
try_from(139-161)program-tests/create-address-test-program/src/lib.rs (1)
to_account_metas_small(258-344)
programs/system/src/processor/cpi.rs (5)
programs/system/src/accounts/account_traits.rs (2)
get_account_compression_authority(7-7)get_registered_program_pda(6-6)programs/system/src/invoke/instruction.rs (2)
get_account_compression_authority(102-104)get_registered_program_pda(98-100)programs/system/src/invoke_cpi/instruction.rs (2)
get_account_compression_authority(105-107)get_registered_program_pda(101-103)programs/system/src/invoke_cpi/instruction_small.rs (2)
get_account_compression_authority(118-123)get_registered_program_pda(111-116)programs/system/src/utils.rs (1)
get_registered_program_pda(13-15)
sdk-libs/sdk-types/src/cpi_accounts.rs (1)
sdk-libs/sdk-types/src/cpi_accounts_small.rs (3)
tree_pubkeys(186-192)to_account_infos(166-174)account_infos(137-139)
programs/system/src/processor/create_outputs_cpi_data.rs (4)
programs/system/src/cpi_context/account.rs (4)
address(61-67)address(121-127)owner(53-55)merkle_tree_index(153-155)program-libs/compressed-account/src/instruction_data/traits.rs (7)
address(48-48)address(69-69)owner(13-13)owner(37-39)owner(46-46)owner(73-73)merkle_tree_index(74-74)program-libs/compressed-account/src/instruction_data/with_account_info.rs (6)
address(82-84)address(132-134)owner(71-73)owner(140-142)owner(342-344)merkle_tree_index(144-146)program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (4)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
account_option_config(308-328)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
account_option_config(442-444)account_option_config(560-572)programs/system/src/cpi_context/instruction_data_trait.rs (1)
account_option_config(75-82)
program-libs/compressed-account/src/instruction_data/zero_copy.rs (6)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
account_option_config(24-24)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
account_option_config(308-328)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
account_option_config(265-283)programs/system/src/cpi_context/instruction_data_trait.rs (1)
account_option_config(75-82)program-libs/compressed-account/src/instruction_data/cpi_context.rs (2)
first_set_context(26-28)first_set_context(36-38)program-libs/compressed-account/src/instruction_data/zero_copy_set.rs (1)
first_set_context(122-122)
programs/system/src/cpi_context/process_cpi_context.rs (5)
program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_account_info(152-159)programs/system/src/cpi_context/account.rs (4)
data(77-87)data(137-147)address(61-67)address(121-127)programs/system/src/cpi_context/state.rs (3)
deserialize_cpi_context_account(276-280)deserialize_cpi_context_account_cleared(283-287)is_empty(63-70)programs/system/src/context.rs (2)
cpi_context(351-353)set_cpi_context(203-222)program-libs/compressed-account/src/pubkey.rs (2)
new_from_array(68-70)to_bytes(189-191)
program-tests/create-address-test-program/src/lib.rs (4)
sdk-libs/sdk/src/cpi/accounts.rs (1)
get_account_metas_from_config(26-134)sdk-libs/sdk-types/src/cpi_accounts.rs (6)
fee_payer(128-130)account_infos(241-243)new_with_config(91-97)config(223-225)tree_accounts(251-256)tree_accounts(259-263)sdk-libs/sdk-types/src/cpi_accounts_small.rs (7)
fee_payer(49-51)account_infos(137-139)new_with_config(41-47)config(119-121)system_accounts_end_offset(123-135)tree_accounts(147-154)tree_accounts(187-191)sdk-libs/sdk/src/cpi/accounts_small_ix.rs (1)
to_account_metas_small(131-134)
programs/system/src/cpi_context/account.rs (2)
program-libs/compressed-account/src/compressed_account.rs (4)
hash_with_hashed_values(260-315)hash_with_hashed_values(320-338)hash_with_hashed_values(359-377)merkle_context(220-231)program-libs/compressed-account/src/instruction_data/traits.rs (19)
hash_with_hashed_values(53-59)hash_with_hashed_values(75-81)owner(13-13)owner(37-39)owner(46-46)owner(73-73)lamports(47-47)lamports(68-68)address(48-48)address(69-69)merkle_context(49-49)has_data(50-50)has_data(70-70)data(51-51)data(72-72)skip(52-52)skip(71-71)root_index(61-61)merkle_tree_index(74-74)
programs/system/src/accounts/init_context_account.rs (2)
program-libs/account-checks/src/checks.rs (2)
check_owner(128-136)check_signer(114-119)programs/system/src/cpi_context/state.rs (2)
cpi_context_account_new(403-495)new(378-389)
programs/system/src/accounts/account_checks.rs (3)
programs/system/src/cpi_context/state.rs (6)
std(93-93)std(95-95)std(97-97)std(98-98)std(100-100)std(472-472)program-libs/account-checks/src/checks.rs (9)
check_discriminator(62-62)check_discriminator(78-87)check_mut(121-126)check_non_mut(43-48)check_owner(128-136)check_pda_seeds(151-161)check_pda_seeds_with_bump(163-173)check_program(138-149)check_signer(114-119)programs/system/src/cpi_context/account.rs (1)
owner(53-55)
sdk-libs/sdk/src/cpi/invoke.rs (5)
sdk-libs/sdk-types/src/cpi_accounts.rs (4)
cpi_context(202-213)account_infos(241-243)bump(43-45)bump(219-221)sdk-libs/sdk-types/src/cpi_accounts_small.rs (3)
cpi_context(102-113)account_infos(137-139)bump(175-177)sdk-libs/sdk/src/cpi/accounts_cpi_context.rs (1)
get_account_metas_from_config_cpi_context(5-13)sdk-libs/sdk/src/cpi/accounts.rs (1)
get_account_metas_from_config(26-134)sdk-libs/sdk/src/cpi/accounts_small_ix.rs (1)
to_account_metas_small(131-134)
programs/system/src/processor/process.rs (2)
programs/system/src/cpi_context/process_cpi_context.rs (2)
process_cpi_context(48-88)copy_cpi_context_outputs(144-190)programs/system/src/invoke_cpi/process_cpi_context.rs (2)
process_cpi_context(37-90)copy_cpi_context_outputs(141-164)
program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs (1)
program-tests/create-address-test-program/src/lib.rs (1)
create_invoke_read_only_account_info_instruction(235-256)
programs/system/src/cpi_context/state.rs (3)
program-libs/account-checks/src/checks.rs (1)
check_owner(128-136)programs/system/src/cpi_context/account.rs (4)
data(77-87)data(137-147)address(61-67)address(121-127)program-libs/compressed-account/src/instruction_data/data.rs (2)
assigned_account_index(72-78)new(61-70)
programs/system/tests/invoke_cpi_instruction_small.rs (2)
programs/system/tests/invoke_cpi_instruction.rs (8)
get_account_compression_authority_account_info(79-88)get_account_compression_program_account_info(56-65)get_authority_account_info(22-31)get_fee_payer_account_info(11-20)get_mut_account_info(103-112)get_registered_program_pda_account_info(34-43)get_self_program_account_info(129-138)get_system_program_account_info(115-124)sdk-libs/sdk-types/src/cpi_accounts_small.rs (6)
account_compression_program(74-79)system_program(81-86)fee_payer(49-51)authority(53-58)registered_program_pda(60-65)account_compression_authority(67-72)
sdk-libs/sdk-types/src/cpi_accounts_small.rs (2)
sdk-libs/sdk-types/src/cpi_accounts.rs (8)
account_compression_program(174-179)system_program(195-200)to_account_infos(276-283)account_infos(241-243)bump(43-45)bump(219-221)invoking_program(146-151)tree_pubkeys(258-264)sdk-libs/sdk-types/src/cpi_context_write.rs (3)
to_account_infos(22-28)bump(14-16)invoking_program(18-20)
programs/system/src/invoke_cpi/verify_signer.rs (6)
program-libs/compressed-account/src/instruction_data/traits.rs (1)
bump(23-23)program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
bump(304-306)program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
bump(289-291)program-libs/compressed-account/src/instruction_data/zero_copy.rs (2)
bump(436-438)bump(552-554)programs/system/src/context.rs (1)
bump(259-261)programs/system/src/cpi_context/instruction_data_trait.rs (1)
bump(70-73)
program-libs/account-checks/src/account_iterator.rs (1)
program-libs/account-checks/src/checks.rs (3)
check_mut(121-126)check_non_mut(43-48)check_signer(114-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: Test program-libs-slow
- GitHub Check: cli-v2
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (sdk-libs, light-macros light-sdk light-program-test light-client light-batched-m...
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: lint
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: cli-v1
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test"])
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v1
| #[aligned_sized(anchor)] | ||
| #[derive(Debug, PartialEq, Default)] | ||
| #[account] | ||
| #[repr(C)] | ||
| pub struct CpiContextAccount2 { | ||
| pub fee_payer: Pubkey, | ||
| pub associated_merkle_tree: Pubkey, | ||
| } |
There was a problem hiding this comment.
Layout/deserialization mismatch between v1 and v2 context accounts
CpiContextAccount2 removes the context Vec, but deserialize_cpi_context_account still reads a length and iterates context items. On v2 data this will read past the struct and return corrupted data.
Parse by discriminator and handle both layouts:
-use anchor_lang::prelude::*;
+use anchor_lang::{prelude::*, Discriminator};
@@
-pub fn deserialize_cpi_context_account<'info, 'a>(
- account_info: &AccountInfo<'info>,
-) -> std::result::Result<ZCpiContextAccount2<'a>, ZeroCopyError> {
- let mut account_data = account_info.try_borrow_mut_data().unwrap();
- let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) };
- let (fee_payer, data) =
- Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(&mut data[8..])?;
- let (associated_merkle_tree, data) =
- Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(data)?;
- let (len, data) = Ref::<&'a mut [u8], U32>::from_prefix(data)?;
- let mut data = &*data;
- let mut context = Vec::new();
- for _ in 0..(u64::from(*len)) as usize {
- let (context_item, new_data) = ZInstructionDataInvokeCpi::zero_copy_at(data)?;
- context.push(context_item);
- data = new_data;
- }
- Ok(ZCpiContextAccount2 { fee_payer, associated_merkle_tree, context })
-}
+pub fn deserialize_cpi_context_account<'info, 'a>(
+ account_info: &AccountInfo<'info>,
+) -> std::result::Result<ZCpiContextAccount2<'a>, ZeroCopyError> {
+ let mut account_data = account_info.try_borrow_mut_data().map_err(|_| ZeroCopyError::BorrowMut)?;
+ let data = unsafe { slice::from_raw_parts_mut(account_data.as_mut_ptr(), account_data.len()) };
+ let discr = &data[..8];
+ // Old layout with context Vec
+ if discr == CpiContextAccount::discriminator() {
+ let (fee_payer, data) =
+ Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(&mut data[8..])?;
+ let (associated_merkle_tree, data) =
+ Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(data)?;
+ let (len, data) = Ref::<&'a mut [u8], U32>::from_prefix(data)?;
+ let mut data = &*data;
+ let mut context = Vec::new();
+ for _ in 0..(u64::from(*len)) as usize {
+ let (context_item, new_data) = ZInstructionDataInvokeCpi::zero_copy_at(data)?;
+ context.push(context_item);
+ data = new_data;
+ }
+ return Ok(ZCpiContextAccount2 { fee_payer, associated_merkle_tree, context });
+ }
+ // New v2 layout without context Vec
+ if discr == CpiContextAccount2::discriminator() {
+ let (fee_payer, data) =
+ Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(&mut data[8..])?;
+ let (associated_merkle_tree, _rest) =
+ Ref::<&'a mut [u8], light_compressed_account::pubkey::Pubkey>::from_prefix(data)?;
+ return Ok(ZCpiContextAccount2 { fee_payer, associated_merkle_tree, context: Vec::new() });
+ }
+ Err(ZeroCopyError::InvalidDiscriminator)
+}Also consider returning a versioned enum if callers need to distinguish v1 vs v2.
Also applies to: 44-72, 50-53
| fn account_option_config( | ||
| &self, | ||
| ) -> Result<super::traits::AccountOptions, CompressedAccountError> { | ||
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | ||
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | ||
| let cpi_context_account = self.cpi_context().is_some(); | ||
| let write_to_cpi_context = | ||
| self.cpi_context.first_set_context() || self.cpi_context.set_context(); | ||
|
|
||
| // Validate: if we want to write to CPI context, we must have a CPI context | ||
| if write_to_cpi_context && !cpi_context_account { | ||
| return Err(CompressedAccountError::InvalidCpiContext); | ||
| } | ||
|
|
||
| Ok(AccountOptions { | ||
| sol_pool_pda, | ||
| decompression_recipient, | ||
| cpi_context_account, | ||
| write_to_cpi_context, | ||
| }) |
There was a problem hiding this comment.
Fix invalid field access and derive write_to_cpi_context from optional context
self.cpi_context is not a field on this type; this will not compile. Compute the flag via self.cpi_context() Option instead.
Apply:
- let write_to_cpi_context =
- self.cpi_context.first_set_context() || self.cpi_context.set_context();
+ let write_to_cpi_context = self
+ .cpi_context()
+ .map(|c| c.first_set_context || c.set_context)
+ .unwrap_or(false);📝 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.
| fn account_option_config( | |
| &self, | |
| ) -> Result<super::traits::AccountOptions, CompressedAccountError> { | |
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | |
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | |
| let cpi_context_account = self.cpi_context().is_some(); | |
| let write_to_cpi_context = | |
| self.cpi_context.first_set_context() || self.cpi_context.set_context(); | |
| // Validate: if we want to write to CPI context, we must have a CPI context | |
| if write_to_cpi_context && !cpi_context_account { | |
| return Err(CompressedAccountError::InvalidCpiContext); | |
| } | |
| Ok(AccountOptions { | |
| sol_pool_pda, | |
| decompression_recipient, | |
| cpi_context_account, | |
| write_to_cpi_context, | |
| }) | |
| fn account_option_config( | |
| &self, | |
| ) -> Result<super::traits::AccountOptions, CompressedAccountError> { | |
| let sol_pool_pda = self.compress_or_decompress_lamports().is_some(); | |
| let decompression_recipient = sol_pool_pda && !self.is_compress(); | |
| let cpi_context_account = self.cpi_context().is_some(); | |
| let write_to_cpi_context = self | |
| .cpi_context() | |
| .map(|c| c.first_set_context || c.set_context) | |
| .unwrap_or(false); | |
| // Validate: if we want to write to CPI context, we must have a CPI context | |
| if write_to_cpi_context && !cpi_context_account { | |
| return Err(CompressedAccountError::InvalidCpiContext); | |
| } | |
| Ok(AccountOptions { | |
| sol_pool_pda, | |
| decompression_recipient, | |
| cpi_context_account, | |
| write_to_cpi_context, | |
| }) |
🤖 Prompt for AI Agents
In program-libs/compressed-account/src/instruction_data/with_account_info.rs
around lines 308 to 327, the code incorrectly accesses self.cpi_context as a
field and tries to call methods on it; replace that with the Option returned by
self.cpi_context(): compute cpi_context_account as self.cpi_context().is_some(),
and derive write_to_cpi_context by matching the Option (e.g., if let Some(ctx) =
self.cpi_context() { ctx.first_set_context() || ctx.set_context() } else { false
}), keep the existing validation that write_to_cpi_context requires
cpi_context_account, and return AccountOptions with those corrected booleans.
| #[profile] | ||
| pub fn cpi_signer_check( | ||
| invoking_program: &Pubkey, | ||
| authority: &Pubkey, | ||
| bump: Option<u8>, | ||
| ) -> Result<()> { | ||
| let derived_signer = if let Some(bump) = bump { | ||
| let seeds = [CPI_AUTHORITY_PDA_SEED, &[bump][..]]; | ||
| solana_pubkey::Pubkey::create_program_address( | ||
| &seeds, | ||
| &solana_pubkey::Pubkey::new_from_array(*invoking_program), | ||
| ) | ||
| .map_err(|_| ProgramError::from(SystemProgramError::CpiSignerCheckFailed))? | ||
| .to_bytes() | ||
| let seeds = [CPI_AUTHORITY_PDA_SEED]; | ||
| pinocchio_pubkey::derive_address(&seeds, Some(bump), invoking_program) | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
Switch to pinocchio_pubkey::derive_address: confirm equivalence to Solana semantics.
Ensure derive_address enforces off-curve program address semantics identical to Pubkey::create_program_address when a bump is provided.
- Optional: de-duplicate seeds binding outside the match for clarity.
- let derived_signer = if let Some(bump) = bump {
- let seeds = [CPI_AUTHORITY_PDA_SEED];
- pinocchio_pubkey::derive_address(&seeds, Some(bump), invoking_program)
- } else {
- // Kept for backwards compatibility with instructions, invoke, and invoke cpi.
- let seeds = [CPI_AUTHORITY_PDA_SEED];
+ let seeds = [CPI_AUTHORITY_PDA_SEED];
+ let derived_signer = if let Some(bump) = bump {
+ pinocchio_pubkey::derive_address(&seeds, Some(bump), invoking_program)
+ } else {
solana_pubkey::Pubkey::try_find_program_address(
&seeds,
&solana_pubkey::Pubkey::new_from_array(*invoking_program),
)
.ok_or(ProgramError::InvalidSeeds)?
.0
.to_bytes()
};To confirm equivalence, please check docs:
🌐 Web query:
Does pinocchio_pubkey::derive_address guarantee off-curve program address derivation compatible with Solana's Pubkey::create_program_address (including bump semantics)?
💡 Result:
Short answer: No.
Why: pinocchio_pubkey::derive_address computes the same hash-based address (and has a const version) without calling Solana's create_program_address syscall, but its docs explicitly say it does NOT validate that the result is off-curve and it does not perform the canonical bump search — it expects seeds and (optional) bump to already be valid. So it can reproduce create_program_address when given the same seeds+bump, but it does not guarantee off-curve/canonical-bump semantics by itself; you must still use create_program_address / find_program_address (or otherwise validate) when you need that guarantee. [1][2][3]
Sources:
- pinocchio_pubkey::derive_address docs (notes that it avoids the syscall and does not validate off-curve). [1]
- pinocchio_pubkey::derive_address_const docs. [2]
- Solana Pubkey create_program_address / bump semantics (find/create vs canonical bump explanation). [3]
Don't rely on pinocchio_pubkey::derive_address for PDA validation — enforce off-curve/canonical-bump
pinocchio_pubkey::derive_address only computes the hash; it does NOT validate the result is off-curve or perform the canonical bump search. Using it when a bump is supplied can accept an invalid/on-curve signer.
- Action (programs/system/src/invoke_cpi/verify_signer.rs — ~lines 37–46): when bump is Some, validate the derived address instead of trusting derive_address alone — either call solana_pubkey::Pubkey::create_program_address with the seeds + bump (handle its error) or verify the derive_address result by comparing/confirming via solana_pubkey::Pubkey::try_find_program_address / create_program_address to ensure off-curve and canonical-bump semantics.
- Optional: hoist seeds out of the branches for clarity (diff below).
- let derived_signer = if let Some(bump) = bump {
- let seeds = [CPI_AUTHORITY_PDA_SEED];
- pinocchio_pubkey::derive_address(&seeds, Some(bump), invoking_program)
- } else {
- // Kept for backwards compatibility with instructions, invoke, and invoke cpi.
- let seeds = [CPI_AUTHORITY_PDA_SEED];
+ let seeds = [CPI_AUTHORITY_PDA_SEED];
+ let derived_signer = if let Some(bump) = bump {
+ pinocchio_pubkey::derive_address(&seeds, Some(bump), invoking_program)
+ } else {
solana_pubkey::Pubkey::try_find_program_address(
&seeds,
&solana_pubkey::Pubkey::new_from_array(*invoking_program),
)
.ok_or(ProgramError::InvalidSeeds)?
.0
.to_bytes()
};Committable suggestion skipped: line range outside the PR's diff.
| cpi_ix_data.addresses[i].tree_index = context.get_index_or_insert( | ||
| new_address_params.address_merkle_tree_account_index(), | ||
| remaining_accounts, | ||
| ); | ||
| "V2 address tree", | ||
| )?; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Confirm V2 path intentionally leaves queue_index unset.
In the V2 branch, only tree_index is set (no queue_index). If downstream uses queue_index only for V1, this is fine; otherwise, populate or gate accordingly.
Run:
Expected: no V2-only consumers require queue_index.
🏁 Script executed:
#!/bin/bash
# Inspect usages of `queue_index` to confirm it's not required for V2.
rg -nP -C3 '\bqueue_index\b'Length of output: 51826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== create_address_cpi_data.rs =="
nl -ba programs/system/src/processor/create_address_cpi_data.rs | sed -n '1,240p'Length of output: 73
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== programs/system/src/processor/create_address_cpi_data.rs =="
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/processor/create_address_cpi_data.rs | sed -n '1,240p' || true
echo
echo "== programs/account-compression/src/processor/insert_addresses.rs =="
awk '{printf "%4d: %s\n", NR, $0}' programs/account-compression/src/processor/insert_addresses.rs | sed -n '1,240p' || true
echo
echo "== program-libs/compressed-account/src/instruction_data/insert_into_queues.rs =="
awk '{printf "%4d: %s\n", NR, $0}' program-libs/compressed-account/src/instruction_data/insert_into_queues.rs | sed -n '1,240p' || true
echo
echo "== programs/system/src/processor/create_inputs_cpi_data.rs =="
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/processor/create_inputs_cpi_data.rs | sed -n '1,240p' || trueLength of output: 29315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for NewAddress trait and address_* methods =="
rg -n --hidden "trait\s+NewAddress|address_queue_index\(|address_merkle_tree_account_index\(" || true
echo
echo "== Show trait definition in light_compressed_account (if present) =="
rg -n "trait\s+NewAddress" || true
rg -n "pub trait NewAddress" || true
rg -n "impl .* NewAddress" || trueLength of output: 3531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== program-libs/compressed-account/src/instruction_data/traits.rs =="
awk '{printf "%4d: %s\n", NR, $0}' program-libs/compressed-account/src/instruction_data/traits.rs | sed -n '1,240p' || true
echo
echo "== program-libs/compressed-account/src/instruction_data/zero_copy.rs (around implementations) =="
awk '{printf "%4d: %s\n", NR, $0}' program-libs/compressed-account/src/instruction_data/zero_copy.rs | sed -n '1,220p' || true
awk '{printf "%4d: %s\n", NR, $0}' program-libs/compressed-account/src/instruction_data/zero_copy.rs | sed -n '760,820p' || true
echo
echo "== programs/system/src/cpi_context/address.rs =="
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/cpi_context/address.rs | sed -n '1,240p' || true
echo
echo "== programs/system/src/cpi_context/state.rs =="
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/cpi_context/state.rs | sed -n '1,300p' || true
echo
echo "== programs/system/src/context.rs (around get_index_or_insert and set_rollover_fee) =="
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/context.rs | sed -n '1,220p' || true
awk '{printf "%4d: %s\n", NR, $0}' programs/system/src/context.rs | sed -n '220,520p' || trueLength of output: 46731
V2 branch must set addresses[i].queue_index (bug — needs fix).
BatchedAddressTree path only sets tree_index but account-compression::insert_addresses groups/looks up by queue_index, so V2 entries must set queue_index to the same context index (e.g. assign cpi_ix_data.addresses[i].queue_index = cpi_ix_data.addresses[i].tree_index or call get_index_or_insert again). Fix in programs/system/src/processor/create_address_cpi_data.rs (BatchedAddressTree branch ~lines 61–66); downstream consumer: programs/account-compression/src/processor/insert_addresses.rs.
🤖 Prompt for AI Agents
In programs/system/src/processor/create_address_cpi_data.rs around lines 61 to
66, the V2 BatchedAddressTree branch sets cpi_ix_data.addresses[i].tree_index
via context.get_index_or_insert but does not set addresses[i].queue_index;
account-compression::insert_addresses groups/looks up by queue_index so you must
set queue_index to the same index. Fix by assigning
cpi_ix_data.addresses[i].queue_index = cpi_ix_data.addresses[i].tree_index (or
by calling get_index_or_insert again to obtain the same context index)
immediately after setting tree_index so both fields are populated for downstream
insert_addresses.
| }; | ||
| use light_hasher::{Hasher, Poseidon}; | ||
| use light_program_profiler::profile; | ||
| use pinocchio::{account_info::AccountInfo, msg, program_error::ProgramError}; |
There was a problem hiding this comment.
Wrong Pubkey type path; will not compile
solana_pubkey::Pubkey is not in scope here. Use pinocchio’s Pubkey (already a dependency in this file).
Apply:
- use pinocchio::{account_info::AccountInfo, msg, program_error::ProgramError};
+ use pinocchio::{account_info::AccountInfo, msg, program_error::ProgramError, pubkey::Pubkey as SolPubkey};and
- solana_pubkey::Pubkey::new_from_array(output_account.owner().into())
+ SolPubkey::new_from_array(output_account.owner().into())Also applies to: 260-263
🤖 Prompt for AI Agents
In programs/system/src/processor/create_outputs_cpi_data.rs around line 11 (and
similarly at lines 260-263), the code references solana_pubkey::Pubkey which is
the wrong path and will not compile; replace those references with pinocchio’s
Pubkey and add an import like use pinocchio::pubkey::Pubkey; (or change
fully-qualified solana_pubkey::Pubkey to pinocchio::pubkey::Pubkey) so all
Pubkey uses come from the pinocchio crate already used in this file.
| /// Create a vector of account info references | ||
| pub fn to_account_infos(&self) -> Vec<&'a T> { | ||
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len() - PROGRAM_ACCOUNTS_LEN); | ||
| account_infos.push(self.fee_payer()); | ||
| self.accounts[PROGRAM_ACCOUNTS_LEN..] | ||
| pub fn to_account_infos(&self) -> Vec<T> { | ||
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len()); | ||
| account_infos.push(self.fee_payer().clone()); | ||
| // Skip system light program | ||
| self.accounts[1..] | ||
| .iter() | ||
| .for_each(|acc| account_infos.push(acc)); | ||
| .for_each(|acc| account_infos.push(acc.clone())); | ||
| account_infos | ||
| } |
There was a problem hiding this comment.
Same as CpiAccounts: cloning variant should be opt-in
Expose a borrowing method to avoid forcing T: Clone and extra allocations.
Add a borrowing variant and keep the owned one as opt-in:
- pub fn to_account_infos(&self) -> Vec<T> {
- let mut account_infos = Vec::with_capacity(1 + self.accounts.len());
- account_infos.push(self.fee_payer().clone());
- // Skip system light program
- self.accounts[1..]
- .iter()
- .for_each(|acc| account_infos.push(acc.clone()));
- account_infos
- }
+ pub fn to_account_infos(&self) -> Vec<&'a T> {
+ let mut account_infos = Vec::with_capacity(1 + self.accounts.len());
+ account_infos.push(self.fee_payer());
+ // Skip system light program
+ account_infos.extend(self.accounts[1..].iter());
+ account_infos
+ }
+
+ #[allow(dead_code)]
+ pub fn to_account_infos_owned(&self) -> Vec<T> {
+ let mut account_infos = Vec::with_capacity(1 + self.accounts.len());
+ account_infos.push(self.fee_payer().clone());
+ self.accounts[1..].iter().for_each(|acc| account_infos.push(acc.clone()));
+ account_infos
+ }📝 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.
| /// Create a vector of account info references | |
| pub fn to_account_infos(&self) -> Vec<&'a T> { | |
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len() - PROGRAM_ACCOUNTS_LEN); | |
| account_infos.push(self.fee_payer()); | |
| self.accounts[PROGRAM_ACCOUNTS_LEN..] | |
| pub fn to_account_infos(&self) -> Vec<T> { | |
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len()); | |
| account_infos.push(self.fee_payer().clone()); | |
| // Skip system light program | |
| self.accounts[1..] | |
| .iter() | |
| .for_each(|acc| account_infos.push(acc)); | |
| .for_each(|acc| account_infos.push(acc.clone())); | |
| account_infos | |
| } | |
| /// Create a vector of account info references | |
| pub fn to_account_infos(&self) -> Vec<&'a T> { | |
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len()); | |
| account_infos.push(self.fee_payer()); | |
| // Skip system light program | |
| account_infos.extend(self.accounts[1..].iter()); | |
| account_infos | |
| } | |
| #[allow(dead_code)] | |
| pub fn to_account_infos_owned(&self) -> Vec<T> { | |
| let mut account_infos = Vec::with_capacity(1 + self.accounts.len()); | |
| account_infos.push(self.fee_payer().clone()); | |
| self.accounts[1..].iter().for_each(|acc| account_infos.push(acc.clone())); | |
| account_infos | |
| } |
|
changes have been merged as part of a3497b1 |
1 participant
State
Changes
Cpi context refactor
executing instruction compressed accounts..cpi context compressed accountscpi context accounts..executing instruction accountsNewAddressParamsPacked->CpiContextNewAddressParamsAssignedPackedCpiContextNewAddressParamsAssignedPackedreadonly_addressesandreadonly_accountsare placeholdersCpiContextInAccountandCpiContextOutAccountare structs without options for efficient zero copyReInitCpiContextAccountinstructionCpiContextAccount->CpiContextAccount2CpiContextAccount::DISCRIMINATOR->CpiContextAccount2::DISCRIMINATORCpiContextAccount2::DISCRIMINATORis anchor derivedsystem program small instruction accounts
system program cpi signer check
solana_pubkey::Pubkey::create_program_addresswithpinocchio_pubkey::derive_addresslight-account-checks
AccountIteratorSummary by CodeRabbit
New Features
SDK
Performance
Refactor
Documentation
Chores