Conversation
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (5)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds comprehensive unit test coverage across five modules including token state validation, indexer batch operations, configuration defaults, proof packing logic, and transaction size handling. No functional runtime code changes are introduced; all modifications are test-only additions with improved assertion coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@CodeRabbit review this pr pls |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
forester/tests/test_indexer_interface.rs (1)
518-599:⚠️ Potential issue | 🟡 MinorBatch test no longer covers the "not-found" path
The previous version of Test 3 apparently included accounts that would return
None(e.g.,compressed_mint_pdawhich is fully compressed and has no on-chain representation). The updated test uses only hot accounts, so the code path wheregetMultipleAccountInterfacesreturns aNoneentry for a missing/cold account is no longer exercised anywhere in this test.Consider keeping at least one entry that is expected to be
None(e.g.,&compressed_mint_pda) to ensure the batch endpoint handles mixed hot/missing results correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/tests/test_indexer_interface.rs` around lines 518 - 599, The batch test no longer checks the "not-found" path; modify the batch_addresses passed to photon_indexer.get_multiple_account_interfaces to include &compressed_mint_pda (e.g., [ &decompressed_mint_pda, &compressible_token_account, &compressed_mint_pda, &bob_ata, &charlie_ata ] or swap into the existing 4-item vector), then update the assertions on batch_response.value to expect a None (i.e., .as_ref() should be None) for the compressed_mint_pda slot and shift the subsequent assertions for bob_ata/charlie_ata to their new indices so the test verifies a mixed hot/missing result set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forester/tests/test_compressible_ctoken.rs`:
- Around line 328-346: The current assert_eq! uses extensions:
account_state.account.extensions.clone(), which tautologically makes the
extensions field always match; update the test to make a real assertion: either
remove the extensions field from the expected Token literal and keep the
existing assert!(account_state.account.extensions.is_some()) check, or replace
that field with the concrete expected extensions value (e.g., the known
CompressibleToken extension data) and assert equality against it; locate the
Token struct construction in test_compressible_ctoken.rs (the assert_eq!
comparing account_state.account) and modify the extensions handling accordingly.
In `@forester/tests/test_indexer_interface.rs`:
- Around line 520-527: The comment above the batch_addresses vector is stale and
mentions "compressed mint PDA (not found)" and "compressed mint seed pubkey"
which no longer match the actual items; update the comment to accurately
describe the current contents: decompressed_mint_pda,
compressible_token_account, bob_ata, and charlie_ata (both on-chain hot
accounts). Locate the comment near batch_addresses and replace the old
descriptions with a brief, correct list referencing decompressed_mint_pda,
compressible_token_account, bob_ata, and charlie_ata.
In `@sdk-libs/client/src/interface/tx_size.rs`:
- Around line 196-199: The comment next to max_size in tx_size.rs miscounts
instruction accounts: each instruction adds a third key created via
AccountMeta::new(...), so replace the misleading "accounts(compact+32*2)" part
with "accounts(compact+32*3)" and update the byte math to reflect header(3) +
accounts(compact+32*3) + blockhash(32) + ixs + sigs ≈ 212 bytes for
one-instruction tx (hence max_size = 250 still allows one but not two); update
the inline numbers to show ~212 for one and ~290 for two so the comment matches
the actual test logic using max_size.
---
Outside diff comments:
In `@forester/tests/test_indexer_interface.rs`:
- Around line 518-599: The batch test no longer checks the "not-found" path;
modify the batch_addresses passed to
photon_indexer.get_multiple_account_interfaces to include &compressed_mint_pda
(e.g., [ &decompressed_mint_pda, &compressible_token_account,
&compressed_mint_pda, &bob_ata, &charlie_ata ] or swap into the existing 4-item
vector), then update the assertions on batch_response.value to expect a None
(i.e., .as_ref() should be None) for the compressed_mint_pda slot and shift the
subsequent assertions for bob_ata/charlie_ata to their new indices so the test
verifies a mixed hot/missing result set.
48198d2 to
666e7ba
Compare
Summary by CodeRabbit