refactor: light program test make anchor programs optional deps#1976
refactor: light program test make anchor programs optional deps#1976ananas-block merged 3 commits intomainfrom
Conversation
WalkthroughFeature-gate refactors and dev-mode wiring across program-test; introduce read-only compressed account support in SDK (v2), with CPI integrations and errors; add utilities for loading accounts and program setup; adjust indexer imports/gating; extend ProgramTestConfig; add anchor tests for read-only paths; minor import/api removals/renames. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Caller
participant SDK as SDK v2 Instruction Builder
participant CPI as Target Program
App->>SDK: Build CPI with LightAccount
alt Account has read_only_account_hash
SDK->>SDK: to_packed_read_only_account()
note right of SDK: Collect read_only_accounts
end
SDK->>CPI: Invoke with read-only accounts (packed)
CPI-->>App: Result
sequenceDiagram
autonumber
participant Caller
participant SDK as inner_invoke_write_to_cpi_context_typed
Caller->>SDK: Invoke write-to-CPI-context
alt has_read_only_accounts() == true
SDK-->>Caller: Err(ReadOnlyAccountsNotSupportedInCpiContext)
else
SDK-->>Caller: Proceed with serialization and invoke
end
sequenceDiagram
autonumber
participant Harness as ProgramTest
participant Files as Accounts on Disk
participant Chain as Runtime/Indexer
alt feature = devenv
Harness->>Harness: Initialize protocol, trees, queues
Harness->>Chain: Set accounts and indexer (batch size configured)
else non-devenv
Harness->>Files: find_accounts_dir()
Files-->>Harness: Path
Harness->>Files: load_all_accounts_from_dir()
Files-->>Harness: HashMap<Pubkey, Account>
Harness->>Harness: Verify batch size from queues
Harness->>Chain: Set accounts and indexer (derived batch size)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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/chore-program-test-optional-deps
branch
from
8a986de to
31c76f7
Compare
ananas-block
force-pushed
the
jorrit/chore-program-test-optional-deps
branch
from
791bf94 to
9f25af8
Compare
ananas-block
force-pushed
the
jorrit/chore-program-test-optional-deps
branch
from
10fa9da to
b9d2471
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/program-test/src/indexer/test_indexer.rs (2)
2283-2314: Fix infinite retry loop on non-success HTTP responsesNon-success HTTP statuses never decrement retries, causing an infinite loop. Decrement and backoff on all non-success cases.
- while retries > 0 { - let response_result = client - .post(format!("{}{}", SERVER_ADDRESS, PROVE_PATH)) - .header("Content-Type", "text/plain; charset=utf-8") - .body(json_payload.clone()) - .send() - .await; - if let Ok(response_result) = response_result { - if response_result.status().is_success() { - let body = response_result.text().await.unwrap(); + while retries > 0 { + match client + .post(format!("{}{}", SERVER_ADDRESS, PROVE_PATH)) + .header("Content-Type", "text/plain; charset=utf-8") + .body(json_payload.clone()) + .send() + .await + { + Ok(resp) if resp.status().is_success() => { + let body = resp.text().await.unwrap(); let proof_json = deserialize_gnark_proof_json(&body).unwrap(); let (proof_a, proof_b, proof_c) = proof_from_json_struct(proof_json); let (proof_a, proof_b, proof_c) = compress_proof(&proof_a, &proof_b, &proof_c); return Ok(ValidityProofWithContext { accounts: account_proof_inputs, addresses: address_proof_inputs, proof: CompressedProof { a: proof_a, b: proof_b, c: proof_c, } .into(), }); - } - } else { - println!("Error: {:#?}", response_result); - tokio::time::sleep(Duration::from_secs(5)).await; - retries -= 1; - } + } + Ok(resp) => { + log::warn!("prove server non-success: {}", resp.status()); + retries -= 1; + tokio::time::sleep(Duration::from_secs(5)).await; + } + Err(err) => { + log::warn!("prove server error: {}", err); + retries -= 1; + tokio::time::sleep(Duration::from_secs(5)).await; + } + } }
1226-1261: Guard underflow and avoid O(n²) removes in finalize_batched_address_tree_updateonchain_next_index - address_tree_index can underflow and slice/remove(0) is O(n²). Use saturating/checked arithmetic and drain.
- let address_tree_index = address_tree.right_most_index(); - let onchain_next_index = onchain_account.next_index; - let diff_onchain_indexer = onchain_next_index - address_tree_index as u64; - let addresses = address_tree.queue_elements[0..diff_onchain_indexer as usize].to_vec(); - for _ in 0..diff_onchain_indexer { - address_tree.queue_elements.remove(0); - } + let address_tree_index = address_tree.right_most_index(); + let onchain_next_index = onchain_account.next_index; + let diff_onchain_indexer = + onchain_next_index.saturating_sub(address_tree_index as u64) as usize; + if diff_onchain_indexer == 0 { + return; + } + let n = std::cmp::min(diff_onchain_indexer, address_tree.queue_elements.len()); + let addresses: Vec<[u8; 32]> = address_tree.queue_elements.drain(0..n).collect();
🧹 Nitpick comments (10)
sdk-libs/sdk-types/src/instruction/account_meta.rs (1)
170-170: Consider adding a deprecation attribute to guide migration.The type alias maintains backward compatibility, which is good. However, consider adding a
#[deprecated]attribute to signal that consumers should migrate toCompressedAccountMetaReadOnly.Apply this diff if deprecation is desired:
+#[deprecated(since = "TBD", note = "Use `CompressedAccountMetaReadOnly` instead")] pub type CompressedAccountMetaBurn = CompressedAccountMetaReadOnly;If the alias is intended to remain indefinitely without deprecation, you can disregard this suggestion.
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/read_only.rs (2)
34-36: Consider preserving error context in tree_pubkeys mapping.The error mapping to
ReadOnlyError::InvalidAccountloses the original error context fromtree_pubkeys(). While acceptable for test code, preserving the original error or providing more specific error variants could aid debugging.Consider either:
- Preserving the original error by using
?directly ifReadOnlyErrorcan wrap it, or- Adding more specific error variants to
ReadOnlyError(e.g.,TreePubkeyError)This pattern is repeated in all four test functions (lines 67-69, 101-103, 134-136).
20-152: Consider extracting common logic to reduce duplication.There's significant code duplication across the four test functions:
- SHA256 vs Poseidon variants differ only in the LightAccount type constructor
- light_system_cpi vs lowlevel variants differ only in the CPI type used
For better maintainability, consider extracting shared logic into helper functions or using generics where possible. However, this is a test module and the explicit implementations may provide clearer test coverage, so this refactor is optional.
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/read_only.rs (1)
107-161: Consider parameterizing account creation helpers to reduce duplication.The
create_compressed_accountandcreate_compressed_account_poseidonhelpers are nearly identical, differing only in the instruction used (lines 149-156 vs 311-318).Consider extracting a parameterized helper function:
async fn create_compressed_account_with_instruction<F>( name: String, rpc: &mut LightProgramTest, payer: &Keypair, address: &[u8; 32], instruction_builder: F, ) -> Result<Signature, RpcError> where F: FnOnce(ValidityProof, PackedAddressTreeInfo, u8, String) -> InstructionHowever, for test code, explicit implementations may provide clearer coverage, so this refactor is optional.
Also applies to: 269-323
sdk-libs/program-test/src/indexer/test_indexer.rs (6)
631-669: Honor start_offset in get_queue_elements (v2)start_offset is ignored; implement windowing to support pagination.
- let end_offset = std::cmp::min( - num_elements as usize, - address_tree_bundle.queue_elements.len(), - ); - let queue_elements = address_tree_bundle.queue_elements[0..end_offset].to_vec(); + let start = _start_offset.unwrap_or(0) as usize; + let len = address_tree_bundle.queue_elements.len(); + if start >= len { + return Ok(Response { + context: Context { slot: self.get_current_slot() }, + value: QueueElementsResult { elements: vec![], first_value_queue_index: None }, + }); + } + let end = std::cmp::min(start + num_elements as usize, len); + let queue_elements = address_tree_bundle.queue_elements[start..end].to_vec();Apply the same start/end windowing in the InputStateV2 and OutputStateV2 branches.
Also applies to: 675-742, 745-820
468-470: Replace println! with structured logging (debug/warn) and/or gate with debug_assertionsUse log::{debug,info,warn} instead of println! to avoid noisy stdout in tests and CI; gate verbose logs under debug.
- println!("state_merkle_tree_pubkeys {:?}", state_merkle_tree_pubkeys); - println!("hashes {:?}", hashes); + log::debug!("state_merkle_tree_pubkeys {:?}", state_merkle_tree_pubkeys); + log::debug!("hashes {:?}", hashes); @@ - println!("queue_element {:?}", queue_element); + log::debug!("queue_element {:?}", queue_element); @@ - println!("index {:?}", index); - println!( - "accounts.output_queue_batch_size {:?}", - accounts.output_queue_batch_size - ); + log::debug!("index {:?}", index); + log::debug!( + "accounts.output_queue_batch_size {:?}", + accounts.output_queue_batch_size + ); @@ - println!( + log::info!( "creating Merkle tree bundle {:?}", self.state_merkle_trees .iter() .map(|x| x.accounts.merkle_tree) .collect::<Vec<_>>() ); @@ - println!("Test indexer didn't find input compressed accounts to nullify"); + log::debug!("Test indexer didn't find input compressed accounts to nullify"); @@ - println!("accounts {:?}", bundle.accounts); + log::debug!("accounts {:?}", bundle.accounts); @@ - println!("Error: {:#?}", response_result); + log::warn!("Error posting to prover: {:#?}", response_result);Also applies to: 491-499, 1570-1575, 1832-1833, 1945-1946, 2310-2313
259-261: Avoid MSRV-fragile Option::is_none_or; use map_or for compatibilitymap_or(true, ...) is widely supported and clearer.
- acc.token_data.owner == *owner && mint.is_none_or(|m| acc.token_data.mint == m) + acc.token_data.owner == *owner && mint.map_or(true, |m| acc.token_data.mint == m)Please confirm MSRV; if is_none_or is acceptable in your toolchain, this is optional.
1662-1673: Centralize token discriminators and owner ID; avoid literals inlineDefine V1/V2 discriminators once and reuse for clarity and consistency.
- const TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR: [u8; 8] = - [2, 0, 0, 0, 0, 0, 0, 0]; - let is_v1_token = data.discriminator == TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR; - let is_v2_token = data.discriminator == [0, 0, 0, 0, 0, 0, 0, 3]; // V2 discriminator + const TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR_V1: [u8; 8] = + [2, 0, 0, 0, 0, 0, 0, 0]; + const TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR_V2: [u8; 8] = + [0, 0, 0, 0, 0, 0, 0, 3]; + let is_v1_token = data.discriminator == TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR_V1; + let is_v2_token = data.discriminator == TOKEN_COMPRESSED_ACCOUNT_DISCRIMINATOR_V2;If canonical discriminators exist in a shared crate, import them instead.
2253-2270: Use constants for heights in CombinedJsonStruct(Legacy)Avoid magic 26; derive from constants to reduce drift.
- CombinedJsonStructLegacy { + CombinedJsonStructLegacy { circuit_type: ProofType::Combined.to_string(), - state_tree_height: 26, - address_tree_height: 26, + state_tree_height: STATE_MERKLE_TREE_HEIGHT as u32, + address_tree_height: STATE_MERKLE_TREE_HEIGHT as u32, // Address V1 height = 26 inclusion: inclusion_payload_legacy.unwrap().inputs, non_inclusion: non_inclusion_payload.inputs, }
1270-1307: Typo: init_from_acounts → init_from_accounts (add forwarder to avoid breakage)Rename for clarity; keep deprecated alias to preserve callers.
-pub async fn init_from_acounts( +#[deprecated(note = "use init_from_accounts")] +pub async fn init_from_acounts( payer: &Keypair, env: &TestAccounts, output_queue_batch_size: usize, ) -> Self { @@ } + +pub async fn init_from_accounts( + payer: &Keypair, + env: &TestAccounts, + output_queue_batch_size: usize, +) -> Self { + Self::init_from_acounts(payer, env, output_queue_batch_size).await +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
program-tests/utils/src/conversions.rs(0 hunks)sdk-libs/client/src/indexer/photon_indexer.rs(2 hunks)sdk-libs/program-test/Cargo.toml(3 hunks)sdk-libs/program-test/src/accounts/mod.rs(1 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(3 hunks)sdk-libs/program-test/src/indexer/address_tree.rs(2 hunks)sdk-libs/program-test/src/indexer/extensions.rs(1 hunks)sdk-libs/program-test/src/indexer/state_tree.rs(1 hunks)sdk-libs/program-test/src/indexer/test_indexer.rs(17 hunks)sdk-libs/program-test/src/program_test/config.rs(5 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(2 hunks)sdk-libs/program-test/src/utils/create_account.rs(1 hunks)sdk-libs/program-test/src/utils/load_accounts.rs(1 hunks)sdk-libs/program-test/src/utils/mod.rs(1 hunks)sdk-libs/program-test/src/utils/setup_light_programs.rs(4 hunks)sdk-libs/sdk-types/src/instruction/account_meta.rs(1 hunks)sdk-libs/sdk/src/account.rs(12 hunks)sdk-libs/sdk/src/cpi/instruction.rs(1 hunks)sdk-libs/sdk/src/cpi/invoke.rs(1 hunks)sdk-libs/sdk/src/cpi/v1/invoke.rs(1 hunks)sdk-libs/sdk/src/cpi/v2/invoke.rs(6 hunks)sdk-libs/sdk/src/error.rs(2 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/Cargo.toml(1 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/lib.rs(3 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/read_only.rs(1 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/read_only.rs(1 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/test.rs(1 hunks)
💤 Files with no reviewable changes (1)
- program-tests/utils/src/conversions.rs
🧰 Additional context used
🧬 Code graph analysis (13)
sdk-libs/sdk/src/cpi/instruction.rs (2)
sdk-libs/sdk/src/cpi/v1/invoke.rs (1)
has_read_only_accounts(352-355)sdk-libs/sdk/src/cpi/v2/invoke.rs (2)
has_read_only_accounts(188-190)has_read_only_accounts(275-277)
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/read_only.rs (4)
sdk-libs/sdk/src/account.rs (4)
account(574-576)account(638-640)new_read_only(484-553)new_read_only(879-953)js/stateless.js/src/state/types.ts (1)
ValidityProof(397-410)sdk-libs/sdk/src/cpi/instruction.rs (1)
new_cpi(15-15)sdk-libs/sdk/src/cpi/v2/invoke.rs (2)
new_cpi(33-41)new_cpi(194-202)
sdk-libs/sdk-types/src/instruction/account_meta.rs (1)
js/stateless.js/src/state/compressed-account.ts (1)
PackedStateTreeInfo(120-141)
sdk-libs/program-test/src/program_test/light_program_test.rs (3)
program-libs/hasher/src/hash_to_field_size.rs (1)
hash_to_bn254_field_size_be(91-93)sdk-libs/program-test/src/accounts/initialize.rs (3)
initialize_accounts(39-252)context(87-88)context(291-292)sdk-libs/program-test/src/utils/load_accounts.rs (1)
load_all_accounts_from_dir(68-140)
sdk-libs/program-test/src/utils/load_accounts.rs (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
new(55-338)
sdk-libs/sdk/src/cpi/v1/invoke.rs (2)
sdk-libs/sdk/src/cpi/instruction.rs (1)
has_read_only_accounts(101-101)sdk-libs/sdk/src/cpi/v2/invoke.rs (2)
has_read_only_accounts(188-190)has_read_only_accounts(275-277)
sdk-libs/program-test/src/program_test/config.rs (3)
program-libs/batched-merkle-tree/src/initialize_address_tree.rs (2)
default(36-51)test_default(194-209)program-libs/batched-merkle-tree/src/initialize_state_tree.rs (2)
default(45-63)test_default(292-310)programs/registry/src/protocol_config/state.rs (1)
default(51-70)
sdk-libs/sdk/src/account.rs (3)
sdk-libs/sdk/src/error.rs (3)
from(109-111)from(115-142)from(146-194)program-libs/compressed-account/src/compressed_account.rs (3)
hash(78-84)hash(339-353)hash(378-390)program-libs/hasher/src/lib.rs (1)
hash(28-28)
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/lib.rs (2)
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/read_only.rs (5)
create_compressed_account_poseidon(269-323)read_sha256_light_system_cpi(163-214)read_poseidon_light_system_cpi(325-376)read_sha256_lowlevel(216-267)read_poseidon_lowlevel(378-429)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/read_only.rs (4)
process_read_sha256_light_system_cpi(21-51)process_read_poseidon_light_system_cpi(54-85)process_read_sha256_lowlevel(88-118)process_read_poseidon_lowlevel(121-152)
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/read_only.rs (2)
js/stateless.js/src/rpc-interface.ts (1)
AddressWithTree(109-113)js/stateless.js/src/utils/instruction.ts (2)
PackedAccounts(5-99)SystemAccountMetaConfig(101-129)
sdk-libs/sdk/src/cpi/v2/invoke.rs (2)
sdk-libs/sdk/src/cpi/instruction.rs (1)
has_read_only_accounts(101-101)sdk-libs/sdk/src/cpi/v1/invoke.rs (1)
has_read_only_accounts(352-355)
sdk-libs/program-test/src/indexer/test_indexer.rs (6)
sdk-libs/program-test/src/program_test/light_program_test.rs (2)
test_accounts(348-350)indexer(340-342)sdk-libs/program-test/src/accounts/address_tree.rs (1)
create_address_merkle_tree_and_queue_account(53-138)sdk-libs/program-test/src/accounts/address_tree_v2.rs (1)
create_batch_address_merkle_tree(11-47)sdk-libs/program-test/src/accounts/state_tree.rs (1)
create_state_merkle_tree_and_queue_account(106-216)sdk-libs/program-test/src/accounts/state_tree_v2.rs (1)
create_batched_state_merkle_tree(17-109)sdk-libs/client/src/indexer/tree_info.rs (1)
height(15-22)
sdk-libs/program-test/src/accounts/test_accounts.rs (3)
programs/registry/src/sdk.rs (1)
create_register_program_instruction(59-89)programs/registry/src/utils.rs (2)
get_forester_pda(34-36)get_protocol_config_pda_address(5-7)sdk-libs/program-test/src/accounts/initialize.rs (1)
get_group_pda(254-260)
⏰ 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). (21)
- GitHub Check: lint
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: cli-v2
- 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 (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: stateless-js-v1
🔇 Additional comments (25)
sdk-libs/sdk-types/src/instruction/account_meta.rs (2)
173-178: LGTM! Rename improves semantic clarity.The rename from
CompressedAccountMetaBurntoCompressedAccountMetaReadOnlybetter reflects the struct's purpose. The type alias at line 170 ensures backward compatibility.
180-204: LGTM! Implementation correctly reflects read-only semantics. AllCompressedAccountMetaBurnreferences alias toCompressedAccountMetaReadOnlyand are covered.sdk-libs/client/src/indexer/photon_indexer.rs (1)
1593-1593: Scoped import keeps v2 path tidy.Pulling
MerkleProofWithContextinto the v2-only branch keeps the symbol hidden from non-v2 builds without changing behavior—looks good.sdk-libs/sdk/src/cpi/v1/invoke.rs (1)
350-355: LGTM: Correct trait implementation for V1.The
has_read_only_accountsmethod correctly returnsfalsesince V1 doesn't support read-only accounts. This aligns with the trait definition and provides a consistent interface across V1 and V2 implementations.sdk-libs/sdk/src/error.rs (1)
84-93: LGTM: Well-defined error variants for read-only account support.The new error variants provide clear, actionable error messages for read-only account scenarios. The numeric mappings (16037-16040) are properly sequential and integrated into the
From<LightSdkError> for u32implementation.sdk-libs/sdk/src/cpi/v2/invoke.rs (6)
47-52: LGTM: Correct early-return path for read-only accounts.The early-return pattern correctly handles read-only accounts by converting them via
to_packed_read_only_account()and bypassing standard account processing. This is consistent with the read-only account design.
104-109: LGTM: Consistent read-only handling in Poseidon variant.The implementation mirrors the SHA256 variant's early-return pattern, ensuring consistent handling across hashers.
187-190: LGTM: Appropriate has_read_only_accounts implementation.The method correctly checks for the presence of read-only accounts by testing if the vector is non-empty, aligning with the trait contract.
208-213: LGTM: Consistent early-return for AccountInfo variant.The read-only account handling is properly implemented for the
InstructionDataInvokeCpiWithAccountInfotype, maintaining consistency across all CPI instruction types.
228-233: LGTM: Complete Poseidon variant coverage.The Poseidon hasher variant for
InstructionDataInvokeCpiWithAccountInfocorrectly implements the read-only early-return pattern.
274-277: LGTM: Consistent trait implementation.The
has_read_only_accountsimplementation forInstructionDataInvokeCpiWithAccountInfomatches the pattern used in other types.sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/read_only.rs (1)
22-105: LGTM: Well-structured integration tests for read-only accounts.The tests provide comprehensive coverage of read-only account functionality across different hashers (SHA256/Poseidon) and CPI methods (LightSystemProgramCpi/lowlevel). The serial execution is appropriate for integration tests that modify on-chain state.
sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/src/lib.rs (3)
4-4: LGTM: Module import properly added.The
mod read_onlyimport correctly exposes the read-only test implementations for use in the program instructions.
183-224: LGTM: Poseidon variant properly implemented.The
create_compressed_account_poseidonfunction correctly uses the Poseidon hasher variant (light_sdk::account::poseidon::LightAccount) and follows the same pattern as the SHA256 variant, with the appropriate call towith_light_account_poseidon.
325-373: LGTM: Clean delegation to read_only module.The wrapper functions provide a clean API surface while delegating to the
read_onlymodule implementations. The separation of concerns between the public API and implementation logic is well-maintained.sdk-libs/sdk/src/cpi/instruction.rs (1)
96-101: LGTM!The trait method addition is well-documented and properly feature-gated, aligning with the existing
cpi-contextAPI surface. The implementations in v1 (returningfalse) and v2 (checkingread_only_accounts.is_empty()) correctly reflect their respective support levels.sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/tests/test.rs (1)
16-16: LGTM!The serial test execution is appropriate for integration tests that interact with shared RPC state and test environments. This prevents race conditions when multiple tests manipulate the same underlying resources concurrently.
Also applies to: 22-22
sdk-libs/sdk/src/cpi/invoke.rs (1)
139-145: LGTM!The runtime guard correctly prevents read-only accounts from being used in CPI context write operations. The early-exit placement is optimal, and the error message clearly directs users to the appropriate alternative (
invoke_execute_cpi_context()).sdk-libs/program-test/src/indexer/state_tree.rs (1)
5-5: Poseidon import alignment looks good.Pointing the tree bundle straight at
light_hasher::Poseidonkeeps this module buildable even when the concurrent tree crate is disabled.sdk-libs/program-test/src/utils/create_account.rs (1)
2-2: Switching tosolana_pubkey::Pubkeylooks right.This keeps the helper usable without the account-compression dependency while staying ABI-compatible with
system_instruction::create_account.sdk-libs/program-test/Cargo.toml (1)
10-32: Feature gating and optional dependency wiring look consistent.Marking the heavy program crates optional and tying them to the
devenvfeature matches the new cfg usage, so non-devenv builds can finally skip them.sdk-libs/program-test/src/indexer/extensions.rs (1)
75-80: Good call gating the batched finalizer.Keeping this async hook behind
devenvmirrors the dependency gating and prevents non-devenv consumers from needing the batched tree stack.sdk-libs/program-test/src/utils/mod.rs (1)
4-6: Module exposure matches the feature split.Making
load_accountsalways available while keepingregister_test_foresterbehinddevenvlines up with the manifest changes.sdk-libs/program-test/src/indexer/address_tree.rs (1)
3-21: Conditional imports are wired correctly.Pulling
DEFAULT_BATCH_ROOT_HISTORY_LENfrom the batched tree crate only whendevenvis on (and otherwise reusing the local constant) keeps non-devenv builds free of the optional dependency while still sharing the logic.sdk-libs/program-test/src/accounts/test_accounts.rs (1)
123-191: The feature-split PDA initialization reads well.Runtime derivation stays in the
devenvblock, while the non-devenv branch reuses fixed PDAs so the module compiles without pulling registry dependencies. Nice balance between flexibility and optional builds.
2 participants
Issue:
Summary by CodeRabbit
New Features
Refactor
Chores