chore: update light sdk docs, upstream fixes #1954
Conversation
ananas-block
requested review from
SwenSchaeferjohann and
sergeytimoshin
as code owners
WalkthroughExpands V2 state-tree support (1→5), adds many serialized account fixtures, migrates TokenAccount → CompressedTokenAccount across indexer/RPC/program-test, introduces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CI as CI
participant Test as LightProgramTest
participant RPC as LightClient/RPC
participant Indexer as Indexer
participant Store as LocalAccountStore
Dev->>CI: push PR
Note right of CI: CI runs sdk-tests including client-test (v2)
CI->>Test: run client-test (v2)
Test->>Store: read/write v1 entries (with tree_type)
Test->>Store: for i = 1..5 create merkle_tree / output_queue / cpi_context
Test->>RPC: expose state tree infos (5 V2 entries)
Dev->>RPC: request compressed token accounts
RPC->>Indexer: forward request
Note right of Indexer: map photon_api models -> CompressedTokenAccount
Indexer-->>RPC: return ItemsWithCursor<CompressedTokenAccount>
RPC-->>Dev: return results
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-update-light-sdk-docs
branch
from
76354ed to
9f71ab2
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: 1
🧹 Nitpick comments (5)
sdk-libs/program-test/src/utils/setup_light_programs.rs (1)
70-76: LGTM with a suggestion for consistency.The error mapping is correct and consistent with the other two program insertions.
Consider applying the same explicit error mapping to the NOOP program (lines 77-82) and light_system_program (lines 84-92) for consistency, as they currently only use
inspect_errwithout wrapping the error:For NOOP program:
program_test .add_program_from_file(NOOP_PROGRAM_ID, path.clone()) .inspect_err(|_| { println!("Program spl_noop bin not found in {}", path); - })?; + }) + .map_err(|e| { + RpcError::CustomError(format!("Failed to add spl_noop program: {}", e)) + })?;For light_system_program:
program_test .add_program_from_file(light_sdk::constants::LIGHT_SYSTEM_PROGRAM_ID, path.clone()) .inspect_err(|_| { println!( "Program light_system_program_pinocchio bin not found in {}", path ); - })?; + }) + .map_err(|e| { + RpcError::CustomError(format!("Failed to add light_system_program_pinocchio program: {}", e)) + })?;This would ensure all program loading failures produce consistent, structured error messages.
js/stateless.js/src/constants.ts (1)
168-201: Reduce duplication: derive V2 entries from arraysNice expansion to five V2 trees. To avoid drift and cut repetition, centralize the 1..5 constants into arrays and build localTestActiveStateTreeInfos with a loop.
Apply these additions to define arrays:
+// Group V2 triples +export const batchMerkleTrees = [batchMerkleTree1, batchMerkleTree2, batchMerkleTree3, batchMerkleTree4, batchMerkleTree5] as const; +export const batchQueues = [batchQueue1, batchQueue2, batchQueue3, batchQueue4, batchQueue5] as const; +export const batchCpiContexts = [batchCpiContext1, batchCpiContext2, batchCpiContext3, batchCpiContext4, batchCpiContext5] as const;Then replace the explicit five V2 objects in localTestActiveStateTreeInfos with:
- { - tree: new PublicKey(batchMerkleTree1), - queue: new PublicKey(batchQueue1), - cpiContext: new PublicKey(batchCpiContext1), - treeType: TreeType.StateV2, - nextTreeInfo: null, - }, - { ... 2..5 ... } + ...batchMerkleTrees.map((tree, i) => ({ + tree: new PublicKey(tree), + queue: new PublicKey(batchQueues[i]), + cpiContext: new PublicKey(batchCpiContexts[i]), + treeType: TreeType.StateV2, + nextTreeInfo: null, + })),Also, consider marking the legacy names with JSDoc deprecation:
-// Deprecated: Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead +/** @deprecated Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead */ export const batchMerkleTree = batchMerkleTree1; +/** @deprecated Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead */ export const batchQueue = batchQueue1;Also applies to: 282-312
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
250-260: Avoid cloning TokenDataWithMerkleContext (optional)This maps via CompressedTokenAccount::try_from(acc.clone()). Consider implementing TryFrom<&TokenDataWithMerkleContext> and using references to skip clones.
If you add this impl, you can switch to:
- .map(|acc| CompressedTokenAccount::try_from(acc.clone())) + .map(|acc| CompressedTokenAccount::try_from(acc))sdk-libs/client/src/indexer/photon_indexer.rs (1)
552-585: PhotonIndexer token account paths now return CompressedTokenAccount — OK; consider dedupType updates and conversions are consistent across v2/non‑v2 branches. You can reduce duplication by extracting a helper that maps API response items to Vec and returns ItemsWithCursor.
Also applies to: 624-629, 652-653, 685-686, 736-737
sdk-libs/client/src/indexer/types.rs (1)
718-724: CompressedTokenAccount type and conversions look correct; minor efficiency tweaks
- Struct and TryFrom implementations align with API models (v1/v2). Good.
- Into<Vec> currently clones account; destructuring avoids that copy.
Apply this diff to avoid cloning in the Into<Vec<...>> impl:
- fn into(self) -> Vec<light_sdk::token::TokenDataWithMerkleContext> { - self.value - .items - .into_iter() - .map( - |token_account| light_sdk::token::TokenDataWithMerkleContext { - token_data: token_account.token, - compressed_account: CompressedAccountWithMerkleContext::from( - token_account.account.clone(), - ), - }, - ) - .collect::<Vec<light_sdk::token::TokenDataWithMerkleContext>>() - } + fn into(self) -> Vec<light_sdk::token::TokenDataWithMerkleContext> { + self.value.items.into_iter().map(|CompressedTokenAccount { token, account }| { + light_sdk::token::TokenDataWithMerkleContext { + token_data: token, + compressed_account: CompressedAccountWithMerkleContext::from(account), + } + }).collect() + }Optionally add a reference-based conversion to support zero‑copy callers:
+impl TryFrom<&light_sdk::token::TokenDataWithMerkleContext> for CompressedTokenAccount { + type Error = IndexerError; + fn try_from(src: &light_sdk::token::TokenDataWithMerkleContext) -> Result<Self, Self::Error> { + let account = CompressedAccount::try_from(src.compressed_account.clone())?; + Ok(CompressedTokenAccount { token: src.token_data, account }) + } +}Also applies to: 725-760, 762-797, 800-809, 813-829, 831-844
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.github/workflows/sdk-tests.yml(2 hunks)cli/accounts/batch_state_merkle_tree_HLKs5NJ8FXkJg8BrzJt56adFYYuwg5etzDtBbQYTsixu.json(0 hunks)cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json(0 hunks)cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json(1 hunks)cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json(1 hunks)cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json(1 hunks)cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json(1 hunks)cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json(1 hunks)js/stateless.js/src/constants.ts(2 hunks)sdk-libs/client/src/constants.rs(1 hunks)sdk-libs/client/src/indexer/indexer_trait.rs(2 hunks)sdk-libs/client/src/indexer/mod.rs(1 hunks)sdk-libs/client/src/indexer/photon_indexer.rs(7 hunks)sdk-libs/client/src/indexer/tree_info.rs(1 hunks)sdk-libs/client/src/indexer/types.rs(6 hunks)sdk-libs/client/src/rpc/client.rs(1 hunks)sdk-libs/client/src/rpc/indexer.rs(3 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(2 hunks)sdk-libs/program-test/src/indexer/test_indexer.rs(3 hunks)sdk-libs/program-test/src/logging/formatter.rs(1 hunks)sdk-libs/program-test/src/program_test/indexer.rs(3 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(1 hunks)sdk-libs/program-test/src/utils/setup_light_programs.rs(1 hunks)sdk-libs/sdk/src/lib.rs(1 hunks)sdk-libs/sdk/src/utils.rs(2 hunks)sdk-tests/client-test/Cargo.toml(1 hunks)sdk-tests/client-test/tests/bmt_program_test.rs(1 hunks)
💤 Files with no reviewable changes (2)
- cli/accounts/batched_output_queue_6L7SzhYB3anwEQ9cphpJ1U7Scwj57bx2xueReg7R9cKU.json
- cli/accounts/batch_state_merkle_tree_HLKs5NJ8FXkJg8BrzJt56adFYYuwg5etzDtBbQYTsixu.json
🧰 Additional context used
🧬 Code graph analysis (10)
sdk-libs/client/src/indexer/indexer_trait.rs (4)
sdk-libs/client/src/indexer/photon_indexer.rs (1)
get_compressed_token_accounts_by_owner(647-753)sdk-libs/client/src/rpc/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)sdk-libs/program-test/src/indexer/test_indexer.rs (1)
get_compressed_token_accounts_by_owner(245-280)sdk-libs/program-test/src/program_test/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)
sdk-libs/client/src/rpc/indexer.rs (1)
js/stateless.js/src/rpc-interface.ts (5)
GetCompressedAccountsByOwnerConfig(45-50)GetCompressedTokenAccountsByOwnerOrDelegateOptions(205-209)PaginatedOptions(217-220)SignatureWithMetadata(69-73)TokenBalance(210-210)
sdk-libs/client/src/indexer/photon_indexer.rs (1)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(728-759)try_from(765-796)try_from(834-843)try_from(855-860)try_from(873-879)try_from(891-896)
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(728-759)try_from(765-796)try_from(834-843)try_from(855-860)try_from(873-879)try_from(891-896)
sdk-libs/client/src/rpc/client.rs (1)
js/stateless.js/src/state/types.ts (1)
TreeInfo(77-99)
sdk-tests/client-test/tests/bmt_program_test.rs (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (2)
new(50-170)test_accounts(180-182)
sdk-libs/client/src/indexer/tree_info.rs (1)
js/stateless.js/src/state/types.ts (1)
TreeInfo(77-99)
sdk-libs/program-test/src/program_test/indexer.rs (1)
js/stateless.js/src/rpc-interface.ts (6)
GetCompressedAccountsByOwnerConfig(45-50)GetCompressedTokenAccountsByOwnerOrDelegateOptions(205-209)PaginatedOptions(217-220)SignatureWithMetadata(69-73)TokenBalance(210-210)ValidityProofWithContext(161-190)
sdk-libs/client/src/indexer/mod.rs (3)
js/stateless.js/src/rpc-interface.ts (4)
AddressWithTree(109-113)SignatureWithMetadata(69-73)TokenBalance(210-210)ValidityProofWithContext(161-190)js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)js/stateless.js/src/state/types.ts (1)
TreeInfo(77-99)
sdk-libs/client/src/indexer/types.rs (1)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)
⏰ 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). (12)
- GitHub Check: stateless-js-v1
- GitHub Check: lint
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-batch...
- 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-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
- GitHub Check: cli-v1
🔇 Additional comments (23)
sdk-libs/program-test/src/utils/setup_light_programs.rs (2)
52-55: LGTM! Clear error propagation.The explicit error mapping improves debuggability by wrapping program loading failures with context about which program failed. The pattern is consistent with existing error handling in this function.
61-64: LGTM! Consistent error handling.Error mapping follows the same pattern as the light_registry program, maintaining consistency.
sdk-libs/program-test/src/logging/formatter.rs (1)
1229-1244: LGTM! Expansion of V2 state tree mappings is consistent.The additions correctly expand the V2 state tree account mappings from one to five triples (merkle_tree, output_queue, and cpi_context for each). The naming convention is consistent and the hardcoded pubkeys align with the test accounts defined elsewhere in the PR.
cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json (1)
1-1: LGTM! CPI context data artifact added for V2 state tree triple 3.This JSON artifact correctly represents a Solana account snapshot for the CPI context account
cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf, which is referenced in the test accounts configuration. The structure is consistent with the other CPI context artifacts added in this PR..github/workflows/sdk-tests.yml (1)
18-18: LGTM! Improved YAML formatting.The change from multi-line to single-line array syntax for
pull_request.typesis a formatting improvement with no functional impact.sdk-libs/program-test/src/accounts/test_accounts.rs (2)
83-109: LGTM! V2 state tree expansion in local test validator accounts.The v2_state_trees vector has been correctly expanded from one to five StateMerkleTreeAccountsV2 entries. Each entry includes merkle_tree, output_queue, and cpi_context pubkeys that are consistent with the formatter mappings and follow a clear naming pattern.
152-178: LGTM! V2 state tree expansion in program test accounts.The v2_state_trees vector has been correctly expanded from one to five StateMerkleTreeAccountsV2 entries in the program test accounts configuration. The structure and pubkeys are consistent with the local test validator accounts.
cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json (1)
1-1: LGTM! CPI context data artifact added for V2 state tree triple 5.This JSON artifact correctly represents a Solana account snapshot for the CPI context account
cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6, which is referenced in the test accounts configuration. The structure is consistent with the other CPI context artifacts added in this PR.cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json (1)
1-1: LGTM! CPI context data artifact added for V2 state tree triple 1.This JSON artifact correctly represents a Solana account snapshot for the CPI context account
cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y, which is referenced in the test accounts configuration. The structure is consistent with the other CPI context artifacts added in this PR.cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json (1)
1-1: LGTM! CPI context data artifact added for V2 state tree triple 2.This JSON artifact correctly represents a Solana account snapshot for the CPI context account
cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B, which is referenced in the test accounts configuration. The structure is consistent with the other CPI context artifacts added in this PR.cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json (1)
1-1: LGTM! CPI context data artifact added for V2 state tree triple 4.This JSON artifact correctly represents a Solana account snapshot for the CPI context account
cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc, which is referenced in the test accounts configuration. The structure is consistent with the other CPI context artifacts added in this PR.sdk-libs/sdk/src/utils.rs (1)
12-21: LGTM! Clean helper for CPI signer seeds.The function correctly derives the PDA and constructs signer seeds with the bump byte, which is the standard pattern for CPI signing. The return type provides both the seeds array and the derived PDA, which is useful for callers.
sdk-tests/client-test/tests/bmt_program_test.rs (2)
4-74: Verify the test intent: why assert accounts don't exist?The first test
test_v2_batched_merkle_tree_accountsusesProgramTestConfig::new(false, None)and then asserts that all v2 state tree accounts do not exist. This appears to be testing that without v2 initialization, these accounts are absent.However, the test name and comments suggest it's about testing account accessibility. Consider:
- Clarifying the test name to reflect that this is a negative test (e.g.,
test_v2_accounts_not_created_without_v2_config)- Adding a comment explaining that this verifies accounts aren't created when v2 is disabled
Can you confirm this test's purpose? If it's meant to verify that v2 accounts are only created with the v2 config, the negative assertion makes sense but could use clearer naming.
76-176: LGTM! Comprehensive positive test for v2 state trees.The test properly:
- Uses
ProgramTestConfig::new_v2(false, None)to enable v2- Verifies exactly 5 v2 state trees exist
- Validates each triple's pubkeys match expected values
- Confirms on-chain accounts exist for all components
The assertions are thorough and the print statements aid debugging.
sdk-libs/client/src/constants.rs (1)
3-21: LGTM! Well-documented lookup table constants.The new constants provide lookup-table pubkeys for both mainnet-beta and devnet, with clear documentation explaining their purpose (reducing transaction size via lookup table indices). The naming convention is consistent and the distinction between state trees and nullifier queues is clear.
sdk-libs/client/src/indexer/mod.rs (1)
16-22: LGTM! Export updated for TokenAccount → CompressedTokenAccount rename.The change correctly replaces
TokenAccountwithCompressedTokenAccountin the public exports, aligning with the broader rename across the indexer surface as described in the PR objectives.sdk-libs/client/src/rpc/client.rs (1)
693-730: LGTM! Localnet v2 defaults expanded to 5 state trees.The change expands the default v2 state trees from a single entry to five, matching the devnet configuration. Each entry:
- Uses distinct, well-defined pubkeys (bmt1..5, oq1..5, cpi1..5)
- Includes a CPI context pubkey
- Sets tree_type to StateV2
- Maintains consistent structure
This aligns with the PR's objective to add devnet batched Merkle trees to the test infrastructure.
sdk-libs/sdk/src/lib.rs (1)
3-6: LGTM! Documentation clarity improvements.The updated crate documentation provides a clearer, more precise description of Compressed Accounts:
- Removed marketing claims about cost reduction
- Focused on technical description (state as account hashes, addresses in Merkle trees)
- Clarified that validity proofs verify state existence and address non-existence
These changes improve technical accuracy without altering any code behavior.
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
125-149: LGTM! Batched state tree account setup for v2 tests.The new block correctly:
- Fetches the batched state merkle tree, output queue, and CPI context accounts
- Uses safe optional matching to proceed only if all accounts exist
- Clones these accounts to each of the 5 v2_state_trees locations
- Mirrors the pattern used for address tree setup (lines 116-124)
This setup enables the program-test infrastructure to support multiple v2 state trees, as verified by the test in
bmt_program_test.rs.sdk-libs/client/src/indexer/tree_info.rs (1)
262-313: LGTM! V2 state trees expanded to 5 triples.The change correctly:
- Defines 5 v2 state tree triples (tree, queue, cpi_context) as an array
- Iterates over the array to insert HashMap entries
- Creates two entries per tree (keyed by queue and tree pubkeys)
- Sets tree_type to StateV2 and next_tree_info to None for all
The pubkeys match those used in:
sdk-libs/client/src/rpc/client.rs(localnet defaults)sdk-tests/client-test/tests/bmt_program_test.rs(test expectations)sdk-libs/program-test/src/program_test/light_program_test.rs(test setup)This maintains consistency across the codebase for v2 state tree configuration.
sdk-libs/client/src/rpc/indexer.rs (1)
98-105: Rename applied correctly for token account endpointsReturn types updated to ItemsWithCursor and delegation preserved. Looks good.
Also applies to: 272-279
sdk-libs/program-test/src/program_test/indexer.rs (1)
98-105: Program-test indexer surface updated — OKReturn types now use ItemsWithCursor. Delegation to inner indexer unchanged.
Also applies to: 269-276
sdk-libs/client/src/indexer/indexer_trait.rs (1)
78-79: No lingeringTokenAccountreferences outsidephoton_apimodels — all remaining matches are insdk-libs/photon-apior non-SDK crates.
ananas-block
force-pushed
the
jorrit/chore-update-light-sdk-docs
branch
2 times, most recently
from
0cfd84b to
d5909fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/client/src/indexer/types.rs (1)
719-844: Add migration guide or changelog entry for TokenAccount → CompressedTokenAccount
NoTokenAccountexports remain insdk-libs/client, but there is no migration guide, deprecation notice, or changelog update in that crate. Add documentation (e.g.sdk-libs/client/CHANGELOG.mdordocs/MIGRATION.md) describing this breaking change and how to migrate.
🧹 Nitpick comments (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
194-194: Consider usingv2_state_trees.len()instead of hardcoded 5.While the hardcoded
5matches the current number of v2 state trees, usingcontext.test_accounts.v2_state_trees.len()would be more maintainable and prevent potential bugs if the number of trees changes.Apply this diff:
- for i in 0..5 { + for i in 0..context.test_accounts.v2_state_trees.len() {
📜 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 (30)
cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json(1 hunks)cli/accounts/batched_output_queue_oq1na8gojfdUhsfCpyjNt6h4JaDWtHf1yQj4koBWfto.json(1 hunks)cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json(1 hunks)cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json(1 hunks)cli/accounts/batched_output_queue_oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq.json(1 hunks)cli/accounts/batched_output_queue_oq5oh5ZR3yGomuQgFduNDzjtGvVWfDRGLuDVjv9a96P.json(1 hunks)cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json(1 hunks)cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json(1 hunks)cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json(1 hunks)cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json(1 hunks)cli/accounts/group_pda_24rt4RgeyjUCWGS2eF7L7gyNMuz6JWdqYpAvb1KRoHxs.json(1 hunks)forester-utils/src/registry.rs(1 hunks)forester/src/rollover/operations.rs(1 hunks)program-libs/compressed-account/src/compressed_account.rs(1 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(0 hunks)program-tests/system-cpi-test/tests/test.rs(2 hunks)program-tests/system-test/tests/test.rs(5 hunks)program-tests/utils/src/e2e_test_env.rs(2 hunks)sdk-libs/client/src/indexer/types.rs(8 hunks)sdk-libs/program-test/src/accounts/initialize.rs(3 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(2 hunks)sdk-libs/program-test/src/indexer/test_indexer.rs(6 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(3 hunks)sdk-tests/client-test/Cargo.toml(2 hunks)
💤 Files with no reviewable changes (1)
- program-libs/compressed-account/src/instruction_data/with_readonly.rs
✅ Files skipped from review due to trivial changes (1)
- program-libs/compressed-account/src/compressed_account.rs
🚧 Files skipped from review as they are similar to previous changes (16)
- cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json
- cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json
- cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json
- cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json
- cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json
- cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json
- program-tests/utils/src/e2e_test_env.rs
- cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json
- cli/accounts/group_pda_24rt4RgeyjUCWGS2eF7L7gyNMuz6JWdqYpAvb1KRoHxs.json
- sdk-tests/client-test/Cargo.toml
- cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json
- forester-utils/src/registry.rs
- cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json
- cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json
- cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json
- cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json
🧰 Additional context used
🧬 Code graph analysis (4)
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)program-libs/batched-merkle-tree/src/merkle_tree.rs (4)
BatchedMerkleTreeAccount(1080-1080)BatchedMerkleTreeAccount(1090-1090)BatchedMerkleTreeAccount(1100-1100)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_bytes(181-186)
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(729-760)try_from(766-797)try_from(835-844)try_from(856-861)try_from(874-880)try_from(892-897)
sdk-libs/client/src/indexer/types.rs (4)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)sdk-libs/sdk/src/account.rs (2)
account(450-452)account(490-492)program-libs/batched-merkle-tree/src/queue.rs (3)
Self(155-155)Self(174-174)Self(185-185)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
Self(123-123)Self(138-138)Self(150-150)Self(166-166)Self(177-177)
program-tests/system-test/tests/test.rs (2)
sdk-libs/program-test/src/indexer/test_indexer.rs (2)
accounts(1905-1939)pubkeys(1073-1082)sdk-libs/program-test/src/program_test/light_program_test.rs (1)
test_accounts(264-266)
⏰ 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: lint
- GitHub Check: stateless-js-v1
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v2
- GitHub Check: cli-v2
- 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-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-batch...
- GitHub Check: cli-v1
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-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: 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 (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
🔇 Additional comments (13)
program-tests/system-cpi-test/tests/test.rs (1)
1486-1486: LGTM! Correct tree_type assignment for v1 state trees.The addition of
tree_type: TreeType::StateV1correctly identifies these as v1 state trees in the test setup.Also applies to: 1544-1544
sdk-libs/program-test/src/accounts/test_accounts.rs (1)
71-84: LGTM! Consistent expansion of test account structures.The expansion from 1 to 2 v1 state trees and 1 to 5 v2 state trees is correctly implemented with:
- Consistent hard-coded pubkeys across both
get_local_test_validator_accounts()andget_program_test_test_accounts()- Proper
tree_type: TreeType::StateV1assignment for v1 entries- Well-structured v2 entries with merkle_tree, output_queue, and cpi_context
Also applies to: 92-118, 149-162, 167-193
program-tests/system-test/tests/test.rs (1)
1675-1720: LGTM! Dynamic account collection improves maintainability.The change from hard-coded pubkeys to dynamically building the pubkeys list from
test_accountsis a good improvement. It correctly iterates over all tree types (v1_state_trees, v1_address_trees, v2_state_trees, v2_address_trees) and extracts their respective pubkeys.sdk-libs/program-test/src/program_test/light_program_test.rs (1)
117-165: LGTM! Account synchronization correctly implemented.The synchronization blocks properly:
- Update v1 state merkle tree accounts with associated queues and CPI contexts
- Update v2 batched merkle tree accounts with associated queues, CPI contexts, and hashed pubkeys
- Use
bytemuckfor safe deserialization- Correctly apply
hash_to_bn254_field_size_befor hashed pubkey computationAlso applies to: 175-233
sdk-libs/program-test/src/accounts/initialize.rs (1)
205-249: LGTM! Initialization correctly expanded with hard-coded pubkeys.The initialization of v1 and v2 state trees is properly implemented:
- v1 entries correctly include
tree_type: TreeType::StateV1- Pubkeys match those in test_accounts.rs for consistency
- Use of
pubkey!macro ensures compile-time validationsdk-libs/client/src/indexer/types.rs (2)
696-696: Breaking change: tree_type field added to StateMerkleTreeAccounts.The addition of the
tree_typefield toStateMerkleTreeAccountsis a breaking change to the public API. TheInto<TreeInfo>implementation correctly usesself.tree_typeinstead of a hardcoded value.Based on the PR objectives mentioning "add devnet batched Merkle trees," this breaking change appears intentional. Please ensure:
- Any public documentation is updated to reflect the new field
- Migration notes are provided if this affects external consumers
Also applies to: 706-706
301-301: Improved cpi_context handling for v1 trees.Setting
cpi_contextfromtree_info.cpi_contextinstead ofNonecorrectly propagates the CPI context information for v1 state trees in validity proof construction.sdk-libs/program-test/src/indexer/test_indexer.rs (6)
19-25: LGTM: Import changes align with type renaming.The imports correctly reflect the migration from
TokenAccounttoCompressedTokenAccountand include the necessary config/options types for the updated method signatures.
244-279: LGTM: Type migration correctly applied.The method signature and implementation have been properly updated to use
CompressedTokenAccountinstead ofTokenAccount. The filtering, mapping, and limiting logic remains sound.
958-965: LGTM: Stub method updated correctly.The return type has been properly updated to use
CompressedTokenAccount. Thetodo!()is appropriate for a test indexer that doesn't require full delegate query functionality.
1248-1285: LGTM: V2 state tree initialization with tree_type.The method correctly sets
tree_type: TreeType::StateV2when converting v2 state trees toStateMerkleTreeAccounts. This aligns with the PR objective to add devnet batched Merkle trees.
1287-1349: LGTM: Improved tree_type derivation logic.The refactor to derive
tree_typefromstate_merkle_tree_account.tree_typerather than relying on hard-coded keypairs is a solid improvement. The logic correctly configures V1 and V2 trees with appropriate parameters (height, root history, batch size).
1461-1553: LGTM: Consistent tree_type propagation.The
tree_typefield is correctly added toStateMerkleTreeAccountsconstruction (line 1533), ensuring the tree type is properly propagated when creating new state Merkle trees. The logic correctly handles both V1 and V2 tree types with appropriate parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json(1 hunks)cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json(1 hunks)cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json(1 hunks)cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json(1 hunks)program-tests/system-cpi-test/tests/test_program_owned_trees.rs(2 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json
- cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json
🚧 Files skipped from review as they are similar to previous changes (3)
- cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json
- cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json
- cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json
🧰 Additional context used
🧬 Code graph analysis (2)
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)program-libs/batched-merkle-tree/src/merkle_tree.rs (4)
BatchedMerkleTreeAccount(1080-1080)BatchedMerkleTreeAccount(1090-1090)BatchedMerkleTreeAccount(1100-1100)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_bytes(181-186)
program-tests/system-cpi-test/tests/test_program_owned_trees.rs (2)
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
accounts(1905-1939)program-tests/merkle-tree/src/lib.rs (1)
root(190-195)
⏰ 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: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- 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 light-batch...
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p 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 (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 (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: cli-v1
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Forester e2e test
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v2
🔇 Additional comments (2)
program-tests/system-cpi-test/tests/test_program_owned_trees.rs (2)
1-1: Verify the intent of disabling the feature gate.The
cfg(feature = "test-sbf")attribute has been commented out, which will cause this test to compile unconditionally. Ensure this change is intentional and that the test functions correctly without the feature gate restriction.If the test-sbf feature gate is no longer needed, consider removing the commented line entirely for clarity. If it should remain feature-gated, restore the attribute.
127-136: LGTM! Improved robustness with dynamic lookup.The change from static array indexing (
state_merkle_trees[2]) to dynamic pubkey-based lookup is a solid improvement. This makes the test more resilient to changes in tree ordering and aligns with the PR's broader move toward data-driven tree discovery.The assertion correctly locates the tree by matching
e.accounts.merkle_tree == program_owned_merkle_tree_pubkeyand then compares roots.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/sdk-tests.yml (1)
51-51: Revert to the host test runner unlessclient-testis now a BPF test crate.Running
cargo sbf-test -p client-teststill assumes this crate contains SBF/BPF integration tests, but the crate lacks Solana BPF dependencies or harness code. Stick withcargo test -p client-testor add the BPF scaffolding socargo sbf-testis justified.
🧹 Nitpick comments (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
165-174: Encapsulate CPI context writesThe two blocks hand-roll identical byte-slice writes (with magic offsets) into the CPI context account. Consider extracting a small helper that takes the account + target pubkeys (and maybe hashed values) so we keep the offsets in one place and avoid drift the next time the layout evolves.
Also applies to: 245-253
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/sdk-tests.yml(2 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(3 hunks)sdk-libs/program-test/src/program_test/rpc.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
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)program-libs/batched-merkle-tree/src/merkle_tree.rs (4)
BatchedMerkleTreeAccount(1080-1080)BatchedMerkleTreeAccount(1090-1090)BatchedMerkleTreeAccount(1100-1100)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_bytes(181-186)
⏰ 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: cli-v2
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: cli-v1
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-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 (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- 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: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: lint
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: stateless-js-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-sdk-macros light-sdk light-program-test light-client light-batch...
- GitHub Check: stateless-js-v1
🔇 Additional comments (1)
sdk-libs/program-test/src/program_test/rpc.rs (1)
230-233: Nice zero-allocation iteration tweakSwitching to
.iter().copied()keeps the old semantics while dropping an extra allocation—good call.Also applies to: 249-252
ananas-block
force-pushed
the
jorrit/chore-update-light-sdk-docs
branch
from
849cc7c to
bc95a70
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
sdk-libs/program-test/src/accounts/test_accounts.rs (1)
71-84: Deduplicate hard-coded V1 state-tree lists to avoid driftThe v1_state_trees arrays are duplicated with identical values. Extract into a shared const/helper to keep them in sync and reduce maintenance risk.
Also applies to: 149-162
sdk-libs/client/src/indexer/photon_indexer.rs (1)
652-653: Reduce duplication in v2/non-v2 mappingBoth branches map items via CompressedTokenAccount::try_from. Consider extracting a small helper to DRY this.
Also applies to: 685-686, 736-737
sdk-libs/client/src/indexer/types.rs (1)
719-725: Rename to CompressedTokenAccount is cohesive; consider a ref to avoid clonesAll TryFrom/Into impls look consistent. Optionally add TryFrom<&light_sdk::token::TokenDataWithMerkleContext> to reduce cloning at call sites.
Example additional impl (outside this hunk):
impl TryFrom<&light_sdk::token::TokenDataWithMerkleContext> for CompressedTokenAccount { type Error = IndexerError; fn try_from(td: &light_sdk::token::TokenDataWithMerkleContext) -> Result<Self, Self::Error> { let account = CompressedAccount::try_from(td.compressed_account.clone())?; Ok(CompressedTokenAccount { token: td.token_data, account }) } }Also applies to: 726-761, 763-798, 801-810, 813-830, 832-845
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
1296-1318: Avoid shadowing output_queue_batch_size for clarityThe tuple binds output_queue_batch_size, shadowing the function parameter. Rename to make intent explicit.
- let (tree_type, merkle_tree, output_queue_batch_size) = + let (tree_type, merkle_tree, output_queue_batch_size_opt) = if state_merkle_tree_account.tree_type == TreeType::StateV2 { let merkle_tree = Box::new(MerkleTree::<Poseidon>::new_with_history( DEFAULT_BATCH_STATE_TREE_HEIGHT as usize, 0, 0, DEFAULT_BATCH_ROOT_HISTORY_LEN as usize, )); - ( + ( TreeType::StateV2, - merkle_tree, - Some(output_queue_batch_size), + merkle_tree, + Some(output_queue_batch_size), ) } else { let merkle_tree = Box::new(MerkleTree::<Poseidon>::new_with_history( account_compression::utils::constants::STATE_MERKLE_TREE_HEIGHT as usize, account_compression::utils::constants::STATE_MERKLE_TREE_CANOPY_DEPTH as usize, 0, account_compression::utils::constants::STATE_MERKLE_TREE_ROOTS as usize, )); - (TreeType::StateV1, merkle_tree, None) + (TreeType::StateV1, merkle_tree, None) }; @@ - output_queue_batch_size, + output_queue_batch_size: output_queue_batch_size_opt,
📜 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 (47)
.github/workflows/sdk-tests.yml(2 hunks)cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json(1 hunks)cli/accounts/batched_output_queue_oq1na8gojfdUhsfCpyjNt6h4JaDWtHf1yQj4koBWfto.json(1 hunks)cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json(1 hunks)cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json(1 hunks)cli/accounts/batched_output_queue_oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq.json(1 hunks)cli/accounts/batched_output_queue_oq5oh5ZR3yGomuQgFduNDzjtGvVWfDRGLuDVjv9a96P.json(1 hunks)cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json(1 hunks)cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json(1 hunks)cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json(1 hunks)cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json(1 hunks)cli/accounts/group_pda_24rt4RgeyjUCWGS2eF7L7gyNMuz6JWdqYpAvb1KRoHxs.json(1 hunks)forester-utils/src/registry.rs(1 hunks)forester/src/rollover/operations.rs(1 hunks)js/stateless.js/src/constants.ts(2 hunks)program-libs/compressed-account/src/compressed_account.rs(1 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(0 hunks)program-tests/registry-test/tests/tests.rs(1 hunks)program-tests/system-cpi-test/tests/test.rs(2 hunks)program-tests/system-cpi-test/tests/test_program_owned_trees.rs(2 hunks)program-tests/system-test/tests/test.rs(5 hunks)program-tests/utils/src/e2e_test_env.rs(2 hunks)sdk-libs/client/src/constants.rs(1 hunks)sdk-libs/client/src/indexer/indexer_trait.rs(2 hunks)sdk-libs/client/src/indexer/mod.rs(1 hunks)sdk-libs/client/src/indexer/photon_indexer.rs(7 hunks)sdk-libs/client/src/indexer/tree_info.rs(1 hunks)sdk-libs/client/src/indexer/types.rs(8 hunks)sdk-libs/client/src/rpc/client.rs(1 hunks)sdk-libs/client/src/rpc/indexer.rs(3 hunks)sdk-libs/program-test/src/accounts/initialize.rs(3 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(2 hunks)sdk-libs/program-test/src/indexer/test_indexer.rs(6 hunks)sdk-libs/program-test/src/logging/formatter.rs(1 hunks)sdk-libs/program-test/src/program_test/indexer.rs(3 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(3 hunks)sdk-libs/program-test/src/program_test/rpc.rs(2 hunks)sdk-libs/sdk/src/lib.rs(1 hunks)sdk-libs/sdk/src/utils.rs(2 hunks)sdk-tests/client-test/Cargo.toml(2 hunks)sdk-tests/client-test/tests/bmt_program_test.rs(1 hunks)
💤 Files with no reviewable changes (1)
- program-libs/compressed-account/src/instruction_data/with_readonly.rs
✅ Files skipped from review due to trivial changes (3)
- cli/accounts/batched_output_queue_oq5oh5ZR3yGomuQgFduNDzjtGvVWfDRGLuDVjv9a96P.json
- cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json
- cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json
🚧 Files skipped from review as they are similar to previous changes (24)
- sdk-libs/sdk/src/lib.rs
- .github/workflows/sdk-tests.yml
- sdk-libs/client/src/indexer/mod.rs
- forester/src/rollover/operations.rs
- sdk-libs/program-test/src/program_test/rpc.rs
- cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json
- cli/accounts/batched_output_queue_oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq.json
- cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json
- program-tests/registry-test/tests/tests.rs
- cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json
- program-tests/system-cpi-test/tests/test_program_owned_trees.rs
- program-libs/compressed-account/src/compressed_account.rs
- cli/accounts/batched_output_queue_oq1na8gojfdUhsfCpyjNt6h4JaDWtHf1yQj4koBWfto.json
- cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json
- sdk-libs/program-test/src/program_test/indexer.rs
- program-tests/utils/src/e2e_test_env.rs
- sdk-libs/program-test/src/logging/formatter.rs
- cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json
- cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json
- program-tests/system-cpi-test/tests/test.rs
- cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json
- sdk-libs/client/src/rpc/indexer.rs
- program-tests/system-test/tests/test.rs
- cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json
🧰 Additional context used
🧬 Code graph analysis (7)
sdk-libs/client/src/indexer/tree_info.rs (1)
js/stateless.js/src/state/types.ts (1)
TreeInfo(77-99)
sdk-libs/client/src/indexer/photon_indexer.rs (1)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(729-760)try_from(766-797)try_from(835-844)try_from(856-861)try_from(874-880)try_from(892-897)
sdk-libs/client/src/indexer/indexer_trait.rs (5)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)sdk-libs/program-test/src/indexer/test_indexer.rs (1)
get_compressed_token_accounts_by_owner(244-279)sdk-libs/client/src/indexer/photon_indexer.rs (1)
get_compressed_token_accounts_by_owner(647-753)sdk-libs/client/src/rpc/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)sdk-libs/program-test/src/program_test/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)
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)program-libs/batched-merkle-tree/src/merkle_tree.rs (4)
BatchedMerkleTreeAccount(1080-1080)BatchedMerkleTreeAccount(1090-1090)BatchedMerkleTreeAccount(1100-1100)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_bytes(181-186)
sdk-tests/client-test/tests/bmt_program_test.rs (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (2)
new(51-278)test_accounts(288-290)
sdk-libs/client/src/indexer/types.rs (4)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)sdk-libs/sdk/src/account.rs (2)
account(450-452)account(490-492)program-libs/batched-merkle-tree/src/queue.rs (3)
Self(155-155)Self(174-174)Self(185-185)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
Self(123-123)Self(138-138)Self(150-150)Self(166-166)Self(177-177)
sdk-libs/program-test/src/indexer/test_indexer.rs (2)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(729-760)try_from(766-797)try_from(835-844)try_from(856-861)try_from(874-880)try_from(892-897)program-tests/merkle-tree/src/lib.rs (1)
new_with_history(62-81)
⏰ 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: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-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 (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-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: cli-v1
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-batch...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: cli-v2
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Forester e2e test
🔇 Additional comments (12)
sdk-tests/client-test/Cargo.toml (1)
9-14: SBF test configuration alignment confirmed.The new
test-sbffeature andcdylibcrate type line up with the SBF + v2 harness changes and look correct.js/stateless.js/src/constants.ts (3)
168-201: LGTM! Consistent integration of V2 state trees.The five new V2 state-tree entries are correctly structured with:
- Proper PublicKey construction
- Correct TreeType.StateV2 assignment
- Appropriate nextTreeInfo: null values
The additions follow the established pattern and maintain consistency with the existing entries.
308-311: LGTM! Good backward compatibility handling.The deprecation aliases for
batchMerkleTreeandbatchQueueproperly maintain backward compatibility by pointing to the new*1variants. The deprecation comment clearly guides users to the new naming convention.
282-301: Verify new V2 public keys
All 15 addresses are unique and within the expected base58 length; confirm they’re deployed and correct on devnet/testnet.sdk-libs/program-test/src/accounts/test_accounts.rs (1)
92-118: Confirm hard-coded pubkeys match validator/test fixturesThese V2 tree/queue/context pubkeys must match what light-test-validator/program-test initialize. Otherwise tests will query non-existent accounts.
Also applies to: 167-193
sdk-libs/client/src/indexer/photon_indexer.rs (1)
14-17: Switchover to CompressedTokenAccount is correctConversions/return types align with the new public type. Good.
Also applies to: 552-553, 584-585
sdk-libs/client/src/indexer/types.rs (1)
691-697: tree_type propagation in StateMerkleTreeAccounts -> TreeInfo is correctUsing self.tree_type avoids hard-coding and matches V1/V2 contexts. LGTM.
Also applies to: 701-709
sdk-libs/program-test/src/indexer/test_indexer.rs (4)
19-26: Updated imports to new public typesImports align with the new trait/API types.
249-259: Owner query returns CompressedTokenAccount correctlyFilter and TryFrom conversion look correct. LGTM.
1257-1263: V2 tree_type propagation during env conversion is correctSetting tree_type: StateV2 here ensures accurate Merkle context downstream.
1530-1534: Include tree_type in StateMerkleTreeAccountsThis aligns account metadata with V1/V2 behavior. Good.
sdk-libs/client/src/indexer/indexer_trait.rs (1)
8-10: No remainingTokenAccountreturns in indexer traitGlobal search confirms all indexer methods now use
CompressedTokenAccount. The only-> TokenAccountoccurrence is the model constructor inphoton-api, which isn’t part of the trait API.
Add utility function to derive CPI authority PDA and seeds for a given program ID.
Add detailed error messages when program loading fails for light_registry, account_compression, and light_compressed_token programs.
Replaces single test tree with 5 production tree triples from devnet: - bmt1/oq1/cpi15, bmt2/oq2/cpi2y, bmt3/oq3/cpi3m, bmt4/oq4/cpi4y, bmt5/oq5/cpi5Z Changes: - Add 15 CLI account JSON files (5 trees × 3 accounts each) - Update client default_trees and tree_info.rs QUEUE_TREE_MAPPING - Update TestAccounts in test_accounts.rs for both test configs - Add account copying in light_program_test.rs for all 5 trees - Add logging names for all 15 pubkeys in formatter.rs - Export all 5 tree triples in js/stateless.js constants - Add bmt_program_test.rs to verify tree setup - Fix CI: use cargo sbf-test for client-test Critical: tree_info.rs now includes all 5 trees with proper CPI context
- types.rs: Use tree_info.cpi_context from QUEUE_TREE_MAPPING instead of None in V1 API compatibility layer. This ensures CPI context is properly propagated. - constants.rs: Add documentation for address lookup table constants explaining their purpose in reducing transaction size.
ananas-block
force-pushed
the
jorrit/chore-update-light-sdk-docs
branch
from
1a0b000 to
76b8a2b
Compare
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 (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
92-115: Guard against panic when v1 trees are skippedUnconditional
.unwrap()will panic ifconfig.skip_v1_treesis true (no address tree/queue created). Gate this block or handle Options.Apply this diff:
- { - let address_mt = context.test_accounts.v1_address_trees[0].merkle_tree; - let address_queue_pubkey = context.test_accounts.v1_address_trees[0].queue; - let mut account = context - .context - .get_account(&keypairs.address_merkle_tree.pubkey()) - .unwrap(); - let merkle_tree_account = bytemuck::from_bytes_mut::<AddressMerkleTreeAccount>( - &mut account.data_as_mut_slice()[8..AddressMerkleTreeAccount::LEN], - ); - merkle_tree_account.metadata.associated_queue = address_queue_pubkey.into(); - context.set_account(address_mt, account); - - let mut account = context - .context - .get_account(&keypairs.address_merkle_tree_queue.pubkey()) - .unwrap(); - let queue_account = bytemuck::from_bytes_mut::<QueueAccount>( - &mut account.data_as_mut_slice()[8..QueueAccount::LEN], - ); - queue_account.metadata.associated_merkle_tree = address_mt.into(); - context.set_account(address_queue_pubkey, account); - } + { + let address_mt = context.test_accounts.v1_address_trees[0].merkle_tree; + let address_queue_pubkey = context.test_accounts.v1_address_trees[0].queue; + if let (Some(mut tree_acc), Some(mut queue_acc)) = ( + context + .context + .get_account(&keypairs.address_merkle_tree.pubkey()), + context + .context + .get_account(&keypairs.address_merkle_tree_queue.pubkey()), + ) { + let merkle_tree_account = bytemuck::from_bytes_mut::<AddressMerkleTreeAccount>( + &mut tree_acc.data_as_mut_slice()[8..AddressMerkleTreeAccount::LEN], + ); + merkle_tree_account.metadata.associated_queue = address_queue_pubkey.into(); + context.set_account(address_mt, tree_acc); + + let queue_account = bytemuck::from_bytes_mut::<QueueAccount>( + &mut queue_acc.data_as_mut_slice()[8..QueueAccount::LEN], + ); + queue_account.metadata.associated_merkle_tree = address_mt.into(); + context.set_account(address_queue_pubkey, queue_acc); + } + }
♻️ Duplicate comments (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
206-210: Loop bound fix looks goodNow iterates over
v2_state_trees.len()instead of a hard-coded count. Resolves the previous concern.
🧹 Nitpick comments (9)
sdk-libs/program-test/src/logging/formatter.rs (1)
1229-1244: LGTM! V2 state tree expansion is correctly implemented.The addition of five V2 state tree triples (merkle tree, output queue, CPI context) follows a consistent naming pattern and correctly expands test account coverage for devnet.
The 15 match arms exhibit high repetition. Consider refactoring with a helper function to reduce duplication and maintenance burden:
fn parse_v2_test_account(pubkey_str: &str) -> Option<String> { let v2_accounts = [ ("bmt1LryLZUMmF7ZtqESaw7wifBXLfXHQYoE4GAmrahU", "v2 state merkle tree 1"), ("oq1na8gojfdUhsfCpyjNt6h4JaDWtHf1yQj4koBWfto", "v2 state output queue 1"), ("cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y", "v2 cpi context 1"), ("bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi", "v2 state merkle tree 2"), ("oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg", "v2 state output queue 2"), ("cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B", "v2 cpi context 2"), ("bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb", "v2 state merkle tree 3"), ("oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ", "v2 state output queue 3"), ("cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf", "v2 cpi context 3"), ("bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8", "v2 state merkle tree 4"), ("oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq", "v2 state output queue 4"), ("cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc", "v2 cpi context 4"), ("bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2", "v2 state merkle tree 5"), ("oq5oh5ZR3yGomuQgFduNDzjtGvVWfDRGLuDVjv9a96P", "v2 state output queue 5"), ("cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6", "v2 cpi context 5"), ]; v2_accounts.iter() .find(|(pk, _)| *pk == pubkey_str) .map(|(_, name)| name.to_string()) }Then use it in the match:
// V2 State Trees and Queues (5 tree triples) id if parse_v2_test_account(id).is_some() => parse_v2_test_account(id).unwrap(),This approach centralizes the mappings and makes future additions/modifications easier.
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
165-173: Avoid magic offsets when writing CPI context fieldsRaw offsets
8 + 32,8 + 32 + 32are brittle and can break if layout changes. Prefer typed parsing or named constants colocated with the account definition.
- Confirm these offsets match the current on-chain struct for the CPI context account.
- Consider exposing constants or an accessor from the crate that defines the CPI context account to avoid hand-maintained offsets.
Also applies to: 245-253
sdk-libs/program-test/src/accounts/test_accounts.rs (1)
71-118: Reduce duplication of hard-coded pubkeys across modulesThe same v1/v2 pubkeys appear here and in initialize.rs and tests. Centralize in a small constants module or a single getter to avoid drift.
Also applies to: 167-193
sdk-libs/program-test/src/accounts/initialize.rs (1)
195-251: Avoid duplicating the same pubkey triples here and in TestAccountsDefine once and reuse to prevent accidental divergence.
program-tests/system-test/tests/test.rs (1)
1730-1730: Consider removing debug println.The println statement appears to be debug output that may have been left during development. Since this is in a helper function for regenerating accounts, it might be intentional for diagnostic purposes. Verify if this output is needed or can be removed.
sdk-libs/program-test/src/indexer/test_indexer.rs (4)
19-26: Avoid duplicate import of RootIndexRootIndex is imported here and again inside get_validity_proof (Line 489). Drop the inner use or rely on this top-level import to avoid shadowing.
249-269: Apply limit before conversion and use MSRV-safe filter
- Use map_or instead of is_none_or to avoid MSRV surprises.
- Apply take(limit) before conversion to avoid unnecessary allocations/conversions.
Apply this diff:
- let mint = options.as_ref().and_then(|opts| opts.mint); - let token_accounts: Result<Vec<CompressedTokenAccount>, IndexerError> = self - .token_compressed_accounts - .iter() - .filter(|acc| { - acc.token_data.owner == *owner && mint.is_none_or(|m| acc.token_data.mint == m) - }) - .map(|acc| CompressedTokenAccount::try_from(acc.clone())) - .collect(); - let token_accounts = token_accounts?; - let token_accounts = if let Some(options) = options { - if let Some(limit) = options.limit { - token_accounts.into_iter().take(limit as usize).collect() - } else { - token_accounts - } - } else { - token_accounts - }; + let mint = options.as_ref().and_then(|opts| opts.mint); + let limit = options.as_ref().and_then(|opts| opts.limit).map(|l| l as usize); + + let iter = self + .token_compressed_accounts + .iter() + .filter(|acc| { + acc.token_data.owner == *owner + && mint.map_or(true, |m| acc.token_data.mint == m) + }); + + let token_accounts: Result<Vec<CompressedTokenAccount>, IndexerError> = match limit { + Some(limit) => iter + .take(limit) + .cloned() + .map(CompressedTokenAccount::try_from) + .collect(), + None => iter.cloned().map(CompressedTokenAccount::try_from).collect(), + }?;Also check your Rust toolchain version supports Option::is_none_or; if not, prefer map_or as above.
1296-1318: Align V2 fee config with add_state_merkle_treeIn new(), V2 tree creation is correct, but rollover_fee later (Line 1323) uses FeeConfig::default(). In add_state_merkle_tree, V2 uses FeeConfig::test_batched(). Align these to avoid inconsistent fees in tests.
For example, set rollover_fee based on tree_type before pushing the bundle:
let rollover_fee = if tree_type == TreeType::StateV2 { FeeConfig::test_batched().state_merkle_tree_rollover as i64 } else { FeeConfig::default().state_merkle_tree_rollover as i64 };
958-965: Implement delegate query mirroring owner logicProvide same filtering, mint option, and limit handling as owner-based method, filtering on token_data.delegate.
Apply this diff:
- async fn get_compressed_token_accounts_by_delegate( - &self, - _delegate: &Pubkey, - _options: Option<GetCompressedTokenAccountsByOwnerOrDelegateOptions>, - _config: Option<IndexerRpcConfig>, - ) -> Result<Response<ItemsWithCursor<CompressedTokenAccount>>, IndexerError> { - todo!("get_compressed_token_accounts_by_delegate not implemented") - } + async fn get_compressed_token_accounts_by_delegate( + &self, + delegate: &Pubkey, + options: Option<GetCompressedTokenAccountsByOwnerOrDelegateOptions>, + _config: Option<IndexerRpcConfig>, + ) -> Result<Response<ItemsWithCursor<CompressedTokenAccount>>, IndexerError> { + let mint = options.as_ref().and_then(|opts| opts.mint); + let limit = options.as_ref().and_then(|opts| opts.limit).map(|l| l as usize); + + let iter = self + .token_compressed_accounts + .iter() + .filter(|acc| { + acc.token_data.delegate == Some(*delegate) + && mint.map_or(true, |m| acc.token_data.mint == m) + }); + + let token_accounts: Result<Vec<CompressedTokenAccount>, IndexerError> = match limit { + Some(limit) => iter + .take(limit) + .cloned() + .map(CompressedTokenAccount::try_from) + .collect(), + None => iter.cloned().map(CompressedTokenAccount::try_from).collect(), + }; + + Ok(Response { + context: Context { + slot: self.get_current_slot(), + }, + value: ItemsWithCursor { + items: token_accounts?, + cursor: None, + }, + }) + }
📜 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 (49)
.github/workflows/sdk-tests.yml(2 hunks)cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json(1 hunks)cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json(1 hunks)cli/accounts/batched_output_queue_oq1na8gojfdUhsfCpyjNt6h4JaDWtHf1yQj4koBWfto.json(1 hunks)cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json(1 hunks)cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json(1 hunks)cli/accounts/batched_output_queue_oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq.json(1 hunks)cli/accounts/batched_output_queue_oq5oh5ZR3yGomuQgFduNDzjtGvVWfDRGLuDVjv9a96P.json(1 hunks)cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json(1 hunks)cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json(1 hunks)cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json(1 hunks)cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json(1 hunks)cli/accounts/cpi_context_cpi3mbwMpSX8FAGMZVP85AwxqCaQMfEk9Em1v8QK9Rf.json(1 hunks)cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json(1 hunks)cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json(1 hunks)cli/accounts/group_pda_24rt4RgeyjUCWGS2eF7L7gyNMuz6JWdqYpAvb1KRoHxs.json(1 hunks)cli/src/utils/constants.ts(1 hunks)forester-utils/src/registry.rs(1 hunks)forester/src/rollover/operations.rs(1 hunks)js/stateless.js/src/constants.ts(2 hunks)program-libs/compressed-account/src/compressed_account.rs(1 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(0 hunks)program-tests/e2e-test/tests/test.rs(1 hunks)program-tests/registry-test/tests/tests.rs(1 hunks)program-tests/system-cpi-test/tests/test.rs(2 hunks)program-tests/system-cpi-test/tests/test_program_owned_trees.rs(2 hunks)program-tests/system-test/tests/test.rs(5 hunks)program-tests/utils/src/e2e_test_env.rs(2 hunks)sdk-libs/client/src/constants.rs(1 hunks)sdk-libs/client/src/indexer/indexer_trait.rs(2 hunks)sdk-libs/client/src/indexer/mod.rs(1 hunks)sdk-libs/client/src/indexer/photon_indexer.rs(7 hunks)sdk-libs/client/src/indexer/tree_info.rs(1 hunks)sdk-libs/client/src/indexer/types.rs(8 hunks)sdk-libs/client/src/rpc/client.rs(1 hunks)sdk-libs/client/src/rpc/indexer.rs(3 hunks)sdk-libs/program-test/src/accounts/initialize.rs(3 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(2 hunks)sdk-libs/program-test/src/indexer/test_indexer.rs(6 hunks)sdk-libs/program-test/src/logging/formatter.rs(1 hunks)sdk-libs/program-test/src/program_test/indexer.rs(3 hunks)sdk-libs/program-test/src/program_test/light_program_test.rs(3 hunks)sdk-libs/program-test/src/program_test/rpc.rs(2 hunks)sdk-libs/sdk/src/lib.rs(1 hunks)sdk-libs/sdk/src/utils.rs(2 hunks)sdk-tests/client-test/Cargo.toml(2 hunks)sdk-tests/client-test/tests/bmt_program_test.rs(1 hunks)
💤 Files with no reviewable changes (1)
- program-libs/compressed-account/src/instruction_data/with_readonly.rs
🚧 Files skipped from review as they are similar to previous changes (24)
- cli/accounts/batched_output_queue_oq2UkeMsJLfXt2QHzim242SUi3nvjJs8Pn7Eac9H9vg.json
- cli/accounts/cpi_context_cpi5ZTjdgYpZ1Xr7B1cMLLUE81oTtJbNNAyKary2nV6.json
- cli/accounts/group_pda_24rt4RgeyjUCWGS2eF7L7gyNMuz6JWdqYpAvb1KRoHxs.json
- cli/accounts/batch_state_merkle_tree_bmt4d3p1a4YQgk9PeZv5s4DBUmbF5NxqYpk9HGjQsd8.json
- cli/accounts/batched_output_queue_oq4ypwvVGzCUMoiKKHWh4S1SgZJ9vCvKpcz6RT6A8dq.json
- cli/accounts/batched_output_queue_oq3AxjekBWgo64gpauB6QtuZNesuv19xrhaC1ZM1THQ.json
- sdk-libs/client/src/indexer/tree_info.rs
- sdk-libs/program-test/src/program_test/rpc.rs
- cli/accounts/batch_state_merkle_tree_bmt3ccLd4bqSVZVeCJnH1F6C8jNygAhaDfxDwePyyGb.json
- forester-utils/src/registry.rs
- .github/workflows/sdk-tests.yml
- sdk-libs/sdk/src/lib.rs
- forester/src/rollover/operations.rs
- cli/accounts/batch_state_merkle_tree_bmt2UxoBxB9xWev4BkLvkGdapsz6sZGkzViPNph7VFi.json
- program-tests/registry-test/tests/tests.rs
- cli/accounts/cpi_context_cpi4yyPDc4bCgHAnsenunGA8Y77j3XEDyjgfyCKgcoc.json
- cli/accounts/cpi_context_cpi1uHzrEhBG733DoEJNgHCyRS3XmmyVNZx5fonubE4.json
- cli/accounts/cpi_context_cpi2yGapXUR3As5SjnHBAVvmApNiLsbeZpF3euWnW6B.json
- sdk-tests/client-test/Cargo.toml
- program-libs/compressed-account/src/compressed_account.rs
- cli/accounts/batch_state_merkle_tree_bmt5yU97jC88YXTuSukYHa8Z5Bi2ZDUtmzfkDTA2mG2.json
- sdk-libs/sdk/src/utils.rs
- cli/accounts/cpi_context_cpi15BoVPKgEPw5o8wc2T816GE7b378nMXnhH3Xbq4y.json
- cli/accounts/cpi_context_cpi2cdhkH5roePvcudTgUL8ppEBfTay1desGh8G8QxK.json
🧰 Additional context used
🧬 Code graph analysis (13)
sdk-libs/program-test/src/accounts/test_accounts.rs (1)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
cpi_context(458-468)
program-tests/system-cpi-test/tests/test_program_owned_trees.rs (1)
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
accounts(1905-1939)
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)program-libs/batched-merkle-tree/src/merkle_tree.rs (4)
BatchedMerkleTreeAccount(1080-1080)BatchedMerkleTreeAccount(1090-1090)BatchedMerkleTreeAccount(1100-1100)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
BatchedQueueAccount(662-662)output_from_bytes(181-186)
sdk-libs/client/src/indexer/indexer_trait.rs (4)
sdk-libs/program-test/src/indexer/test_indexer.rs (1)
get_compressed_token_accounts_by_owner(244-279)sdk-libs/client/src/indexer/photon_indexer.rs (1)
get_compressed_token_accounts_by_owner(647-753)sdk-libs/client/src/rpc/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)sdk-libs/program-test/src/program_test/indexer.rs (1)
get_compressed_token_accounts_by_owner(93-105)
sdk-libs/client/src/indexer/types.rs (3)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)program-libs/batched-merkle-tree/src/queue.rs (3)
Self(155-155)Self(174-174)Self(185-185)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
Self(123-123)Self(138-138)Self(150-150)Self(166-166)Self(177-177)
sdk-tests/client-test/tests/bmt_program_test.rs (1)
sdk-libs/program-test/src/program_test/light_program_test.rs (2)
new(51-278)test_accounts(288-290)
sdk-libs/program-test/src/indexer/test_indexer.rs (2)
sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(729-760)try_from(766-797)try_from(835-844)try_from(856-861)try_from(874-880)try_from(892-897)program-tests/merkle-tree/src/lib.rs (1)
new_with_history(62-81)
sdk-libs/client/src/rpc/indexer.rs (1)
js/stateless.js/src/rpc-interface.ts (2)
GetCompressedTokenAccountsByOwnerOrDelegateOptions(205-209)TokenBalance(210-210)
sdk-libs/client/src/indexer/photon_indexer.rs (3)
js/stateless.js/src/state/compressed-account.ts (1)
CompressedAccount(29-51)js/stateless.js/src/rpc-interface.ts (2)
SignatureWithMetadata(69-73)TokenBalance(210-210)sdk-libs/client/src/indexer/types.rs (10)
try_from(425-432)try_from(518-545)try_from(572-621)try_from(627-674)try_from(729-760)try_from(766-797)try_from(835-844)try_from(856-861)try_from(874-880)try_from(892-897)
program-tests/system-test/tests/test.rs (2)
sdk-libs/program-test/src/indexer/test_indexer.rs (2)
accounts(1905-1939)pubkeys(1073-1082)sdk-libs/program-test/src/program_test/light_program_test.rs (1)
test_accounts(288-290)
sdk-libs/program-test/src/program_test/indexer.rs (1)
js/stateless.js/src/rpc-interface.ts (6)
GetCompressedAccountsByOwnerConfig(45-50)GetCompressedTokenAccountsByOwnerOrDelegateOptions(205-209)PaginatedOptions(217-220)SignatureWithMetadata(69-73)TokenBalance(210-210)ValidityProofWithContext(161-190)
sdk-libs/client/src/rpc/client.rs (1)
js/stateless.js/src/state/types.ts (1)
TreeInfo(77-99)
sdk-libs/program-test/src/accounts/initialize.rs (1)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
cpi_context(458-468)
⏰ 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). (17)
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- 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 (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: lint
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- 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: Forester e2e test
- GitHub Check: stateless-js-v1
🔇 Additional comments (18)
cli/src/utils/constants.ts (1)
26-27: Photon repository and commit validated The new Git repo and commit hash exist and are accessible.program-tests/system-cpi-test/tests/test_program_owned_trees.rs (2)
1-1: Verify the intention of commenting out thecfgattribute.The
test-sbffeature flag has been commented out rather than removed, which disables conditional compilation. This means the test will now always compile regardless of whether BPF-specific features are available.Ensure this change is intentional and that the test doesn't rely on BPF-specific functionality that would cause it to fail in non-BPF environments.
If this change is temporary or under review, consider either:
- Removing the line entirely if the feature flag is no longer needed
- Keeping it active if BPF-specific compilation is still required
129-136: LGTM! Dynamic search improves robustness.The refactor from fixed index access to dynamic search by
merkle_treepubkey is more maintainable and resilient to changes in tree ordering. The pattern is consistent with the codebase (as seen intest_indexer.rs).Based on relevant code snippets.
sdk-libs/client/src/constants.rs (2)
3-4: LGTM! Clear documentation.The documentation for the state tree lookup table constants is clear, consistent, and effectively explains their purpose in reducing transaction sizes.
Also applies to: 13-14
8-9: Align terminology for nullifier queue vs state tree: update the doc comments or constant names so they use the same term (both mainnet and devnet).sdk-libs/program-test/src/accounts/test_accounts.rs (2)
29-46: Good: V2 wrapper with TreeInfo conversionClear struct for V2 triples and
Fromimpl settingtree_type: StateV2. Looks consistent.
71-84: Explicitly tagging V1 withtree_type: StateV1is correctAdds clarity and aligns with the new public surface.
Also applies to: 149-162
sdk-libs/program-test/src/accounts/initialize.rs (2)
205-218: OK: V1 entries includetree_type: StateV1Matches updated API and downstream expectations.
223-249: OK: Provisioning five V2 triples in initializerConsistent with TestAccounts.
sdk-tests/client-test/tests/bmt_program_test.rs (2)
4-74: Test 1 behavior matches configWith
ProgramTestConfig::new, v2 accounts are not created; asserting non-existence is valid.Ensure CI enables/omits the
v2feature flags as intended for this test suite so expectations remain stable.
76-176: Test 2 validates five V2 triples and on-chain presenceCovers structure and existence well.
sdk-libs/program-test/src/program_test/light_program_test.rs (1)
8-8: ```bash
#!/bin/bashVerify the crate name and re-export for hash_to_bn254_field_size_be in the compressed-account crate.
1. Show the package name in Cargo.toml to confirm it's published as
light_compressed_account.sed -n '1,20p' program-libs/compressed-account/Cargo.toml
2. Inspect the root lib.rs for
pub usestatements that re-exporthash_to_bn254_field_size_be.sed -n '1,200p' program-libs/compressed-account/src/lib.rs
</blockquote></details> <details> <summary>sdk-libs/program-test/src/program_test/indexer.rs (1)</summary><blockquote> `3-9`: **No stale `TokenAccount` references in indexer** Verified that `sdk-libs/program-test/src/program_test/indexer.rs` only imports and uses `CompressedTokenAccount`, with no remaining `TokenAccount` occurrences. </blockquote></details> <details> <summary>sdk-libs/client/src/indexer/types.rs (4)</summary><blockquote> `696-696`: **LGTM: tree_type field addition enables V1/V2 tree support.** Adding the `tree_type` field to `StateMerkleTreeAccounts` allows proper differentiation between V1 and V2 state trees, which aligns with the PR objective of adding devnet batched Merkle trees. --- `301-301`: **LGTM: Corrected cpi_context handling.** The change from `cpi_context: None` to `cpi_context: tree_info.cpi_context` ensures the correct CPI context is used from the tree mapping instead of being hardcoded to None. This improves data accuracy in the ValidityProofWithContext. --- `706-706`: **LGTM: Dynamic tree_type from field.** Using `tree_type: self.tree_type` instead of hardcoding `TreeType::StateV1` properly leverages the new field and supports both V1 and V2 trees dynamically. --- `719-845`: **LGTM: Consistent TokenAccount → CompressedTokenAccount renaming.** The renaming from `TokenAccount` to `CompressedTokenAccount` is consistently applied across all struct definitions and trait implementations (TryFrom and Into), aligning with the PR objective. </blockquote></details> <details> <summary>sdk-libs/program-test/src/indexer/test_indexer.rs (1)</summary><blockquote> `1534-1534`: **Good: propagate tree_type into StateMerkleTreeAccounts** This aligns runtime metadata with V2 trees across the surface. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| // Deprecated: Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead | ||
| export const batchMerkleTree = batchMerkleTree1; | ||
| export const batchQueue = batchQueue1; | ||
|
|
There was a problem hiding this comment.
Restore backwards-compatible batchCpiContext export
We kept deprecated aliases for batchMerkleTree and batchQueue, but the legacy batchCpiContext export disappeared. Any downstream import still using batchCpiContext now fails at compile-time. Please add the alias to preserve backwards compatibility.
export const batchMerkleTree = batchMerkleTree1;
export const batchQueue = batchQueue1;
+export const batchCpiContext = batchCpiContext1;📝 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.
| // Deprecated: Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead | |
| export const batchMerkleTree = batchMerkleTree1; | |
| export const batchQueue = batchQueue1; | |
| // Deprecated: Use batchMerkleTree1, batchQueue1, batchCpiContext1 instead | |
| export const batchMerkleTree = batchMerkleTree1; | |
| export const batchQueue = batchQueue1; | |
| export const batchCpiContext = batchCpiContext1; |
🤖 Prompt for AI Agents
In js/stateless.js/src/constants.ts around lines 308 to 311, the deprecated
alias for batchCpiContext was removed causing downstream imports to break;
restore backwards compatibility by re-exporting the existing batchCpiContext1
(or the correct internal identifier) as batchCpiContext alongside the existing
batchMerkleTree and batchQueue aliases, ensuring the name matches prior export
and keeping a deprecation comment consistent with the other aliases.
| merkle_tree: new_merkle_tree_keypair.pubkey(), | ||
| nullifier_queue: new_nullifier_queue_keypair.pubkey(), | ||
| cpi_context: new_cpi_signature_keypair.pubkey(), | ||
| tree_type: TreeType::StateV1, | ||
| }, | ||
| tree_type: TreeType::StateV1, | ||
| merkle_tree: Box::new(light_merkle_tree_reference::MerkleTree::<Poseidon>::new( |
There was a problem hiding this comment.
Preserve the original tree_type during rollover
When we clone a state tree during rollover we now hardcode both the accounts and bundle to TreeType::StateV1. Any V2 tree that rolls over is immediately misclassified as V1, so all V2-specific handling (batched queues, CPI context expectations, etc.) stops working. Please carry the original tree_type forward.
accounts: StateMerkleTreeAccounts {
merkle_tree: new_merkle_tree_keypair.pubkey(),
nullifier_queue: new_nullifier_queue_keypair.pubkey(),
cpi_context: new_cpi_signature_keypair.pubkey(),
- tree_type: TreeType::StateV1,
+ tree_type: bundle.tree_type,
},
- tree_type: TreeType::StateV1,
+ tree_type: bundle.tree_type,📝 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.
| merkle_tree: new_merkle_tree_keypair.pubkey(), | |
| nullifier_queue: new_nullifier_queue_keypair.pubkey(), | |
| cpi_context: new_cpi_signature_keypair.pubkey(), | |
| tree_type: TreeType::StateV1, | |
| }, | |
| tree_type: TreeType::StateV1, | |
| merkle_tree: Box::new(light_merkle_tree_reference::MerkleTree::<Poseidon>::new( | |
| accounts: StateMerkleTreeAccounts { | |
| merkle_tree: new_merkle_tree_keypair.pubkey(), | |
| nullifier_queue: new_nullifier_queue_keypair.pubkey(), | |
| cpi_context: new_cpi_signature_keypair.pubkey(), | |
| tree_type: bundle.tree_type, | |
| }, | |
| tree_type: bundle.tree_type, | |
| merkle_tree: Box::new(light_merkle_tree_reference::MerkleTree::<Poseidon>::new( | |
| // … | |
| )), |
🤖 Prompt for AI Agents
In program-tests/utils/src/e2e_test_env.rs around lines 2382–2388, the rollover
clone hardcodes tree_type = TreeType::StateV1 in the created accounts/bundle;
change those assignments to use the source tree's original type (e.g., use
original_tree.tree_type or the variable holding the existing tree_type) so the
new accounts and bundle carry forward the same tree_type (both in the inner
accounts struct and the outer tree_type field) instead of always setting
StateV1.
76b8a2b to
246fa45
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
program-tests/utils/src/e2e_test_env.rs (1)
2382-2390: Carry over the original tree_type when rolling over a state treeWe still overwrite any rolled-over bundle with
StateV1, so every V2 state tree gets downgraded the moment it rolls. That breaks the batched queue invariants you just added (forester eligibility, CPI expectations, etc.). Please propagate the source bundle’stree_typeinto both the inner accounts and the outer bundle when cloning.- let bundle = self.indexer.get_state_merkle_trees()[index].accounts; + let bundle = self.indexer.get_state_merkle_trees()[index].accounts; + let bundle_tree_type = self.indexer.get_state_merkle_trees()[index].tree_type; … accounts: StateMerkleTreeAccounts { merkle_tree: new_merkle_tree_keypair.pubkey(), nullifier_queue: new_nullifier_queue_keypair.pubkey(), cpi_context: new_cpi_signature_keypair.pubkey(), - tree_type: TreeType::StateV1, + tree_type: bundle.tree_type, }, - tree_type: TreeType::StateV1, + tree_type: bundle_tree_type,
🧹 Nitpick comments (2)
program-tests/e2e-test/tests/test.rs (2)
118-119: Add bounds checking before draining elements.The
drain(..2)operation will panic ifstate_merkle_treescontains fewer than 2 elements. Consider adding a bounds check or an assertion to ensure the test environment is correctly initialized with at least 2 state trees.// remove the two concurrent Merkle trees +assert!( + env.indexer.state_merkle_trees.len() >= 2, + "Expected at least 2 state Merkle trees, found {}", + env.indexer.state_merkle_trees.len() +); env.indexer.state_merkle_trees.drain(..2);
121-128: Consider adding assertions to verify expected tree counts.The test prints the tree counts but doesn't assert the expected values. Adding assertions would make the test more robust and clearly document the expected behavior after tree removal.
println!( "address_merkle_trees {:?}", env.indexer.address_merkle_trees.len() ); println!( "state_merkle_trees {:?}", env.indexer.state_merkle_trees.len() ); +// Verify expected tree counts after removal +assert_eq!( + env.indexer.address_merkle_trees.len(), + expected_address_tree_count, + "Unexpected address tree count after removal" +); +assert_eq!( + env.indexer.state_merkle_trees.len(), + expected_state_tree_count, + "Unexpected state tree count after removal" +);Replace
expected_address_tree_countandexpected_state_tree_countwith the actual expected values based on the test configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
program-tests/e2e-test/tests/test.rs(1 hunks)program-tests/utils/src/e2e_test_env.rs(4 hunks)sdk-libs/program-test/src/logging/decoder.rs(6 hunks)sdk-libs/program-test/src/logging/formatter.rs(2 hunks)sdk-libs/program-test/src/logging/types.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk-libs/program-test/src/logging/formatter.rs
🧰 Additional context used
🧬 Code graph analysis (1)
program-tests/utils/src/e2e_test_env.rs (4)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (2)
output_accounts(478-480)new(245-253)programs/system/src/context.rs (2)
output_accounts(328-337)new(178-200)program-tests/utils/src/system_program.rs (3)
remaining_accounts(663-666)remaining_accounts(669-672)new_address_params(559-567)program-tests/utils/src/pack.rs (2)
new_address_params(88-96)new_address_params(131-141)
⏰ 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: cli-v2
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: lint
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- 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 (light-system-program, ["cargo-test-sbf -p system-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-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- 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 (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v2
🔇 Additional comments (5)
program-tests/e2e-test/tests/test.rs (1)
118-120: Verify the inconsistency in tree removal counts.The code now drains 2 state Merkle trees but still removes only 1 address Merkle tree. Is this intentional, or should
address_merkle_treesalso drain 2 elements for consistency with the expanded V2 state-tree architecture?If the asymmetry is intentional, consider adding a comment explaining why state trees require removing 2 concurrent trees while address trees only need 1.
If both should remove 2, apply this diff:
// remove the two concurrent Merkle trees env.indexer.state_merkle_trees.drain(..2); -env.indexer.address_merkle_trees.remove(0); +env.indexer.address_merkle_trees.drain(..2);sdk-libs/program-test/src/logging/types.rs (1)
109-128: LGTM! Clean infrastructure addition.The new
AddressAssignmentenum andassigned_account_indexfield provide clear tracking of address assignment state across different instruction versions. The three variants appropriately cover legacy behavior (V1), unassigned addresses (None), and assigned addresses (AssignedIndex).sdk-libs/program-test/src/logging/decoder.rs (3)
258-258: LGTM! Correct handling of legacy instructions.Using
AddressAssignment::V1forparse_invoke_instructionandparse_invoke_cpi_instructionis appropriate, as these older instruction types don't support address assignment tracking.Also applies to: 403-403
543-547: LGTM! Proper conditional assignment tracking.The conditional logic correctly populates
assigned_account_indexbased onparam.assigned_to_account:
- Uses
AssignedIndex(param.assigned_account_index)when the address is assigned to an output account- Uses
Nonewhen the address is not assignedThis pattern is consistently applied in both
parse_invoke_cpi_readonly_instructionandparse_invoke_cpi_account_info_instruction.Also applies to: 695-699
559-559: LGTM! Correct handling of readonly addresses.Setting
assigned_account_indextoNonefor readonly addresses is correct, as these addresses are not being assigned to output accounts.Also applies to: 711-711
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
js/stateless.js/src/rpc-interface.ts (1)
280-289: Consider stricter validation for numeric booleans.The helper accepts any
numberand relies on JavaScript's truthiness rules, where only0maps tofalseand all other numbers (including negative numbers, decimals, etc.) map totrue. If the RPC contract guarantees only0or1, consider adding explicit validation to reject unexpected numeric values and improve robustness.Apply this diff to add stricter validation:
const BooleanFromNumberOrBoolean = coerce( boolean(), union([number(), boolean()]), - value => Boolean(value), + value => { + if (typeof value === 'number' && value !== 0 && value !== 1) { + throw new Error(`Expected 0 or 1, got ${value}`); + } + return Boolean(value); + }, );js/stateless.js/tests/e2e/rpc-multi-trees.test.ts (2)
224-229: Consider removing or conditionalizing debug logs.These console.log statements appear to be debug artifacts that will generate significant output on every test run (15 iterations × multiple accounts). While helpful for debugging test failures, they clutter the test output unnecessarily.
Consider one of these approaches:
- Remove the logs if they were only needed during development
- Put them behind an environment variable (e.g.,
if (process.env.DEBUG_TREES) { ... })- Use vitest's built-in debug capabilities if available
Apply this diff to remove the debug logs:
- console.log(`Iteration ${round + 1}, Account ${index}:`); - console.log(` Expected tree (from getCompressedAccountsByOwner): ${expectedTree.toBase58()}`); - console.log(` Actual tree (from getMultipleCompressedAccountProofs): ${actualTree.toBase58()}`); - console.log(` Expected queue: ${expectedQueue.toBase58()}`); - console.log(` Actual queue: ${actualQueue.toBase58()}`); - assert.isTrue(
241-241: Consider removing or conditionalizing this debug log.Similar to the earlier logging, this console.log will execute 15 times per test run. While it may be useful for debugging tree selection, it adds unnecessary noise to the test output.
Apply this diff to remove the debug log:
const tree = selectStateTreeInfo(await rpc.getStateTreeInfos()); - console.log(`Selected tree for transfer in round ${round + 1}: ${tree.tree.toBase58()}`); await transfer(rpc, payer, transferAmount, payer, bob.publicKey);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
js/stateless.js/src/rpc-interface.ts(4 hunks)js/stateless.js/tests/e2e/rpc-multi-trees.test.ts(2 hunks)scripts/install.sh(1 hunks)sdk-libs/program-test/src/logging/formatter.rs(2 hunks)
⏰ 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: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p 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 (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- 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: stateless-js-v2
- GitHub Check: lint
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: cli-v1
- GitHub Check: Forester e2e test
- GitHub Check: cli-v2
- 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-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: stateless-js-v1
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
🔇 Additional comments (3)
sdk-libs/program-test/src/logging/formatter.rs (2)
925-940: LGTM!The AddressAssignment display logic correctly handles all three enum variants with appropriate formatting and color coding. The implementation is consistent with the existing code structure and provides clear information about address-to-output-account assignments.
1245-1260: LGTM!The V2 test account mappings are well-organized with a clear and consistent naming pattern. The five triples (state merkle tree, output queue, cpi context) follow a logical structure that mirrors the V1 accounts and aligns with the PR objective to add devnet batched Merkle trees.
js/stateless.js/src/rpc-interface.ts (1)
434-434: Consistent application of boolean coercion.The
BooleanFromNumberOrBooleanhelper is consistently applied to allproveByIndexfields across the V2 result structures, correctly handling RPC responses that may return numeric (0/1) or boolean values.Also applies to: 565-565, 598-598
ananas-block
force-pushed
the
jorrit/chore-update-light-sdk-docs
branch
2 times, most recently
from
6c11a9b to
48a6c83
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
js/stateless.js/tests/e2e/rpc-multi-trees.test.ts (2)
215-226: Consider specifying an explicit base for hash stringification.The hash-based mapping correctly handles out-of-order proof responses, which is more robust than index-based matching. However,
bn(...).toString()defaults to base 10. For hash values, consider using a more explicit base like hex for better readability and consistency:const accountByHash = new Map( prePayerAccounts.items.map(acc => [ - bn(acc.hash).toString(), + bn(acc.hash).toString('hex'), acc, ]), );And update the matching logic:
- const matchingAccount = accountByHash.get( - bn(proof.hash).toString(), - ); + const matchingAccount = accountByHash.get( + bn(proof.hash).toString('hex'), + );Also applies to: 228-238
243-252: Remove debugging console.log statements.The console.log statements appear to be debugging artifacts that will clutter test output. Consider removing them before merging, or replace them with a debug logger that can be conditionally enabled.
Apply this diff to remove the logging:
- console.log(`.hash: ${bn(proof.hash).toString()}`); - console.log(`Iteration ${round + 1}, Proof ${index}:`); - console.log( - ` Expected tree (from getCompressedAccountsByOwner): ${expectedTree.toBase58()}`, - ); - console.log( - ` Actual tree (from getMultipleCompressedAccountProofs): ${actualTree.toBase58()}`, - ); - console.log(` Expected queue: ${expectedQueue.toBase58()}`); - console.log(` Actual queue: ${actualQueue.toBase58()}`);And:
const tree = selectStateTreeInfo(await rpc.getStateTreeInfos()); - console.log( - `Selected tree for transfer in round ${round + 1}: ${tree.tree.toBase58()}`, - ); await transfer(rpc, payer, transferAmount, payer, bob.publicKey);If you need this for debugging, consider using a debug logger or environment variable to control logging verbosity.
Also applies to: 265-267
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
js/stateless.js/tests/e2e/rpc-multi-trees.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
js/stateless.js/tests/e2e/rpc-multi-trees.test.ts (1)
js/stateless.js/src/state/bn.ts (1)
bn(3-12)
⏰ 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: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- 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-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-slow
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- 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 (light-system-program, ["cargo-test-sbf -p system-test"])
- 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-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v2
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v2
- GitHub Check: lint
🔇 Additional comments (1)
js/stateless.js/tests/e2e/rpc-multi-trees.test.ts (1)
208-210: LGTM! Good defensive practice.Waiting for the indexer to catch up before fetching accounts prevents race conditions and ensures test stability.
ananas-block
force-pushed
the
jorrit/chore-update-light-sdk-docs
branch
from
48a6c83 to
994d4f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
js/stateless.js/tests/e2e/rpc-multi-trees.test.ts (2)
224-232: Remove debug logging or make it conditional.These console.log statements will execute on every test run (15 iterations), cluttering the output. Since the PR objectives don't mention debugging efforts, this appears to be temporary debug code.
Consider one of the following:
Option 1 (Recommended): Remove the debug logs entirely
- console.log(`Iteration ${round + 1}, Account ${index}:`); - console.log( - ` Expected tree (from getCompressedAccountsByOwner): ${expectedTree.toBase58()}`, - ); - console.log( - ` Actual tree (from getMultipleCompressedAccountProofs): ${actualTree.toBase58()}`, - ); - console.log(` Expected queue: ${expectedQueue.toBase58()}`); - console.log(` Actual queue: ${actualQueue.toBase58()}`); -Option 2: Make logging conditional on an environment variable
+ if (process.env.DEBUG_MULTI_TREE_TESTS) { console.log(`Iteration ${round + 1}, Account ${index}:`); console.log( ` Expected tree (from getCompressedAccountsByOwner): ${expectedTree.toBase58()}`, ); console.log( ` Actual tree (from getMultipleCompressedAccountProofs): ${actualTree.toBase58()}`, ); console.log(` Expected queue: ${expectedQueue.toBase58()}`); console.log(` Actual queue: ${actualQueue.toBase58()}`); + }
245-247: Remove debug logging or make it conditional.This console.log will execute 15 times per test run. Similar to the logging above, this appears to be temporary debug code that should be cleaned up.
Apply this diff to remove the log:
- console.log( - `Selected tree for transfer in round ${round + 1}: ${tree.tree.toBase58()}`, - );Or make it conditional on a debug flag if needed for troubleshooting:
+ if (process.env.DEBUG_MULTI_TREE_TESTS) { console.log( `Selected tree for transfer in round ${round + 1}: ${tree.tree.toBase58()}`, ); + }js/stateless.js/tests/e2e/rpc-interop.test.ts (1)
401-484: Add spec clarification for V2 zero-root behavior and plan V2-specific test
The conditional skip is appropriate given current V2 behavior, but returning a zero root for missing leaves isn’t documented in the spec. Please open an issue in the Light Protocol repo to clarify intended behavior for absent elements. Once defined, replace the skip with a V2-specific test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
js/stateless.js/tests/e2e/rpc-interop.test.ts(2 hunks)js/stateless.js/tests/e2e/rpc-multi-trees.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
js/stateless.js/tests/e2e/rpc-interop.test.ts (3)
js/stateless.js/src/constants.ts (1)
featureFlags(16-35)js/stateless.js/src/state/bn.ts (1)
bn(3-12)js/stateless.js/src/actions/transfer.ts (1)
transfer(30-102)
⏰ 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: cli-v1
- 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-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: cli-v2
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: lint
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- 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 (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
🔇 Additional comments (1)
js/stateless.js/tests/e2e/rpc-interop.test.ts (1)
14-14: LGTM!The
featureFlagsimport is correctly added to enable version-specific test skipping.
Changes:
TokenAccount->CompressedTokenAccountSummary by CodeRabbit
New Features
Breaking Changes
Improvements
Tests
Chores / CI