feat: more account checks #1982
Conversation
…r reporting - Add next_checked_pubkey method for pubkey validation with detailed errors - Add print_on_error_pubkey helper for pubkey mismatch debugging - Add PartialEq bound to AccountInfoTrait::Pubkey for comparisons - Add InvalidAccount error variant for account validation failures - Add discriminator logging in check_discriminator for debugging - Update test_account_info to match pinocchio 0.9 Account struct layout - Update tests with correct discriminator hash
WalkthroughAdds extensive documentation for account-checks and introduces new public APIs: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Iterator as AccountIterator
participant Checks as checks.rs
participant Errors as AccountError
Caller->>Iterator: new_with_owner(accounts, owner)
Note over Iterator: position = 0
Caller->>Iterator: next_checked_pubkey(name, expected_pubkey)
Iterator->>Iterator: next_account(name)
alt pubkey == expected_pubkey
Iterator-->>Caller: &T
else pubkey != expected_pubkey
Iterator->>Iterator: print_on_error_pubkey(...)
Iterator-->>Caller: Err(Errors::InvalidAccount)
end
Caller->>Checks: check_discriminator(account, T::LIGHT_DISCRIMINATOR)
alt match
Checks-->>Caller: Ok
else mismatch
Note over Checks: logs discriminator mismatch
Checks-->>Caller: Err(Errors::InvalidDiscriminator)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
program-libs/account-checks/src/packed_accounts.rs (1)
27-27: Track the TODO for future work.The TODO comment references adding
get_checked_accountfromPackedAccounts. Consider tracking this in an issue to ensure it's not forgotten.Do you want me to open a new issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
program-libs/account-checks/src/account_info/account_info_trait.rs(1 hunks)program-libs/account-checks/src/account_iterator.rs(2 hunks)program-libs/account-checks/src/checks.rs(1 hunks)program-libs/account-checks/src/error.rs(2 hunks)program-libs/account-checks/src/packed_accounts.rs(3 hunks)program-libs/account-checks/tests/tests.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
program-libs/account-checks/src/account_iterator.rs (2)
program-libs/account-checks/src/account_info/account_info_trait.rs (1)
pubkey(21-21)program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
⏰ 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). (20)
- GitHub Check: Validate PR title
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- 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-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: lint
- GitHub Check: stateless-js-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-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- 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: cli-v2
- GitHub Check: cli-v1
🔇 Additional comments (9)
program-libs/account-checks/src/error.rs (1)
35-36: LGTM!The new
InvalidAccounterror variant is properly integrated with:
- A clear error message
- Sequential error code assignment (12021)
- Consistent conversion trait implementations
Also applies to: 60-60
program-libs/account-checks/src/account_info/account_info_trait.rs (1)
10-10: LGTM!Adding
PartialEqto thePubkeybound enables the newnext_checked_pubkeymethod inaccount_iterator.rsto perform pubkey comparisons. This change is minimal and correctly scoped.program-libs/account-checks/tests/tests.rs (1)
55-55: Verify tests pass with the updated discriminator.The discriminator value changed from a simple sequential pattern to what appears to be a hash-derived value. Ensure that all tests using this discriminator pass correctly and that the change aligns with the discriminator generation strategy.
program-libs/account-checks/src/checks.rs (1)
84-88: LGTM!Adding debug logging for discriminator mismatches significantly improves observability. The log message clearly shows both expected and actual discriminators, which will help diagnose validation failures.
program-libs/account-checks/src/account_iterator.rs (3)
65-86: LGTM!The
next_checked_pubkeymethod provides a clean way to validate account pubkeys with detailed error reporting. The implementation correctly:
- Retrieves the next account
- Compares pubkeys using the raw byte arrays
- Returns the new
InvalidAccounterror on mismatch- Logs detailed diagnostic information via
inspect_err
225-258: LGTM!The
print_on_error_pubkeyhelper provides detailed error diagnostics with conditional formatting:
- Uses
solana_pubkey::Pubkey::new_from_arrayfor pretty-printing when the solana feature is enabled- Falls back to raw byte array display otherwise
- Includes all relevant context (error, account name, pubkeys, location)
213-213: LGTM!Replacing
#[inline(never)]with#[cold]is more idiomatic for error paths. The#[cold]attribute provides the same optimization hint (don't inline, optimize for the cold path) with clearer intent.Also applies to: 225-225
program-libs/account-checks/src/packed_accounts.rs (2)
5-9: LGTM!The new
ProgramPackedAccountsstruct provides a clean abstraction for index-based account access with proper documentation. The public visibility and generic design allow for flexible usage across different account types.
16-16: LGTM!Moving
Location::caller()capture to the top of the function ensures the location is available for all error paths. While the current code only uses it in one place, this refactoring makes the code more maintainable for future enhancements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
program-libs/account-checks/src/packed_accounts.rs (1)
27-27: TODO noted for future work.The TODO comment indicates planned work to add
get_checked_accountfunctionality. Consider tracking this in a GitHub issue to ensure it's not forgotten.Would you like me to create a GitHub issue to track the
get_checked_accountimplementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
program-libs/account-checks/src/account_info/account_info_trait.rs(1 hunks)program-libs/account-checks/src/account_iterator.rs(2 hunks)program-libs/account-checks/src/checks.rs(1 hunks)program-libs/account-checks/src/error.rs(2 hunks)program-libs/account-checks/src/packed_accounts.rs(3 hunks)program-libs/account-checks/tests/tests.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
program-libs/account-checks/src/account_iterator.rs (2)
program-libs/account-checks/src/account_info/account_info_trait.rs (1)
pubkey(21-21)program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
⏰ 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). (19)
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- 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-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: lint
- GitHub Check: stateless-js-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-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- 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: cli-v2
- GitHub Check: cli-v1
🔇 Additional comments (9)
program-libs/account-checks/src/checks.rs (1)
84-88: LGTM! Helpful discriminator mismatch logging.The added logging provides valuable debugging information by showing both expected and actual discriminator values when a mismatch occurs, making it easier to diagnose account type errors.
program-libs/account-checks/src/account_info/account_info_trait.rs (1)
10-10: LGTM! Essential bound for pubkey comparison.Adding
PartialEqto thePubkeyassociated type is necessary to support the newnext_checked_pubkeyvalidation method introduced in this PR. This is a breaking change to the trait, but it's well-justified by the added functionality.program-libs/account-checks/src/error.rs (2)
35-36: LGTM! Clear error variant for account validation.The new
InvalidAccounterror variant provides a clear, general-purpose error for account validation failures, used by the newnext_checked_pubkeymethod.
60-60: LGTM! Error code follows existing pattern.The error code
12021follows the sequential numbering pattern of existing error codes.program-libs/account-checks/src/packed_accounts.rs (2)
5-9: LGTM! Clearer struct documentation.The updated documentation better describes the purpose and contents of
ProgramPackedAccounts.
16-16: LGTM! Improved error location capture.Moving
Location::caller()before the bounds check ensures the caller's actual location is captured for error messages, regardless of which path the error takes. This improves debugging accuracy.program-libs/account-checks/src/account_iterator.rs (2)
65-86: LGTM! Well-structured pubkey validation method.The
next_checked_pubkeymethod provides a clear, reusable pattern for fetching and validating accounts by pubkey. The use ofinspect_errensures detailed error logging without cluttering the success path.
225-258: LGTM! Comprehensive pubkey mismatch logging.The
print_on_error_pubkeyhelper provides detailed debugging information for pubkey mismatches:
- Conditional formatting based on the
solanafeature improves readability- With
solanafeature: formats asPubkeyobjects- Without
solanafeature: shows raw byte arrays- Includes all relevant context (error, account name, both pubkeys, index, location)
The
#[cold]attribute appropriately optimizes for the success path.program-libs/account-checks/tests/tests.rs (1)
55-55: [scratchpad]
[task review_file_2/6]
Reviewtests.rscontext forTestStruct.
[observations]
- Script found
impl Discriminator for TestStructat lines 54-57.- No other production references to
TestStruct.
[pending]- Inspect the struct definition to see if it derives discriminator macro or is manual.
[actions]- Display lines 1-100 of
program-libs/account-checks/tests/tests.rs.
[done]
[/scratchpad]
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (20)
program-libs/account-checks/src/packed_accounts.rs (1)
27-27: TODO comment flagged for future implementation.The TODO suggests adding
get_checked_accountfunctionality fromPackedAccounts. This would enhance type safety by validating accounts at retrieval time.Would you like me to open a new issue to track this enhancement, or provide a draft implementation of the
get_checked_accountmethod?program-libs/account-checks/src/error.rs (1)
35-36: EnrichInvalidAccountwith actionable context.The single
InvalidAccountvariant is used for mismatched pubkeys, borrow failures, size/discriminator checks, etc., making “Invalid Account.” ambiguous. Introduce distinct variants (e.g.AccountKeyMismatch,BorrowDataFailed,InvalidDiscriminator) or include context in the error message to pinpoint the failure.program-libs/account-checks/docs/ERRORS.md (2)
46-50: Use correct Solana ProgramError path in examplesThe module path should be solana_program::program_error::ProgramError.
Apply:
-impl From<AccountError> for solana_program_error::ProgramError { +impl From<AccountError> for solana_program::program_error::ProgramError {And similarly for the Pinocchio example if its path differs in your codebase; ensure it matches actual modules.
Also applies to: 54-58
84-96: Gate both BorrowError and BorrowMutError conversions consistentlyThe first impl is cfg(feature = "solana") but the second isn’t. For clarity and consistency, gate both:
#[cfg(feature = "solana")] impl From<std::cell::BorrowError> for AccountError { ... } -impl From<std::cell::BorrowMutError> for AccountError { +#[cfg(feature = "solana")] +impl From<std::cell::BorrowMutError> for AccountError { fn from(_: std::cell::BorrowMutError) -> Self { AccountError::BorrowAccountDataFailed } }program-libs/account-checks/docs/PACKED_ACCOUNTS.md (2)
112-116: Add language to fenced log block (MD040)Specify a language for the log examples.
-``` +```text ERROR: Not enough accounts. Requested 'mint' at index 5 but only 3 accounts available. src/processor.rs:42:18 ERROR: Not enough accounts. Requested 'merkle_tree' at index 10 but only 8 accounts available. src/processor.rs:55:23--- `209-213`: **Use library’s AccountError in example for consistency** Avoid introducing a CustomError in examples; align with existing AccountError. ```diff - if index >= accounts.len() { - return Err(CustomError::InvalidIndex); - } + if index >= accounts.len() { + return Err(AccountError::NotEnoughAccountKeys); + }program-libs/account-checks/docs/ACCOUNT_ITERATOR.md (1)
237-243: Add language to fenced log block (MD040)-``` +```text ERROR: Not enough accounts. Requested 'mint' at index 3 but only 2 accounts available. src/processor.rs:45:12 ERROR: Invalid Signer. for account 'authority' at index 0 src/processor.rs:42:8 ERROR: Invalid Account. for account 'system_program' address: 11111111111111111111111111111112, expected: 11111111111111111111111111111111, at index 4 src/processor.rs:48:15</blockquote></details> <details> <summary>program-libs/account-checks/src/account_iterator.rs (2)</summary><blockquote> `216-249`: **Improve pubkey formatting in error logs (readability)** Use Display for Pubkey on Solana to print base58 instead of Debug tuple; keep bytes for non-Solana. ```diff #[cfg(feature = "solana")] solana_msg::msg!( - "ERROR: {}. for account '{}' address: {:?}, expected: {:?}, at index {} {}:{}:{}", + "ERROR: {}. for account '{}' address: {}, expected: {}, at index {} {}:{}:{}", error, account_name, - solana_pubkey::Pubkey::new_from_array(pubkey1), - solana_pubkey::Pubkey::new_from_array(pubkey2), + solana_pubkey::Pubkey::new_from_array(pubkey1), + solana_pubkey::Pubkey::new_from_array(pubkey2), self.position.saturating_sub(1), location.file(), location.line(), location.column() );Optionally, for non-Solana builds consider hex formatting for brevity:
- pubkey1, - pubkey2, + format!("0x{}", hex::encode(pubkey1)), + format!("0x{}", hex::encode(pubkey2)),(Requires hex crate if acceptable.)
170-179: API ergonomics: remaining_unchecked never errors; return a slice directlySimplify signature since it always returns Ok.
- pub fn remaining_unchecked(&self) -> Result<&'info [T], AccountError> { - if self.position >= self.accounts.len() { - Ok(&[]) - } else { - Ok(&self.accounts[self.position..]) - } - } + pub fn remaining_unchecked(&self) -> &'info [T] { + if self.position >= self.accounts.len() { + &[] + } else { + &self.accounts[self.position..] + } + }program-libs/account-checks/docs/ACCOUNT_CHECKS.md (11)
14-19: Signature mismatch forcheck_owner.Documentation shows
fn check_owner(...), but the implementation ispub fn check_owner(...). The doc should reflect the public signature.
25-34: Document full signature forcheck_program.Implementation is
pub fn check_program(...)and returnsAccountError::InvalidAccountwhenaccount_info.ownerequalsprogram_id. The doc omits thepuband the additional error variant. Please update the signature and error list to match code.
39-45: Includepuband signer error nuance.Implementation is
pub fn check_signer(...)and also maps borrow failures fromaccount_info.is_signer()intoAccountError::InvalidAccount. Document bothpubvisibility and the extra error path.
48-54: MentionAccountError::InvalidAccountbranch.
check_mutreturnsAccountError::InvalidAccountifis_writablecannot be borrowed, but the doc omits it. Please add this error.
57-63:check_non_mutalso returnsInvalidAccount.The doc lists only
AccountMutable, yet the code returnsAccountError::InvalidAccounton borrow failure. Update error list to match.
68-76: Clarify borrow failure behaviour.
check_discriminatorreturnsAccountError::InvalidAccountif the data borrow fails before size/discriminator checks. Document that extra error path.
79-85:set_discriminatormutability and errors.Implementation requires writable data and can return
AccountError::InvalidAccounton borrow failure. Mirror thepubvisibility and error set.
88-96:account_info_initreturnsAccountError::InvalidAccount.Doc omits this branch; the code returns it when any borrow fails. Please include it and the
pubqualifier.
101-121: Combined validators missingInvalidAccounterrors.Each combined helper forwards
AccountError::InvalidAccountfrom borrow failures incheck_*. Document this, and reflectpub fnsignatures.
138-158: PDA checks return multiple errors.Both PDA helpers propagate
AccountError::InvalidAccounton borrow failures.check_pda_seeds_with_bumpalso mapsPubkeyError::InvalidSeedstoAccountError::InvalidAccount. Incorporate these.
180-186:check_data_is_zeroedreturnsInvalidAccounton borrow failure.Document the additional error case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
program-libs/account-checks/CLAUDE.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_CHECKS.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_INFO_TRAIT.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_ITERATOR.md(1 hunks)program-libs/account-checks/docs/CLAUDE.md(1 hunks)program-libs/account-checks/docs/DISCRIMINATOR.md(1 hunks)program-libs/account-checks/docs/ERRORS.md(1 hunks)program-libs/account-checks/docs/PACKED_ACCOUNTS.md(1 hunks)program-libs/account-checks/src/account_info/account_info_trait.rs(1 hunks)program-libs/account-checks/src/account_info/test_account_info.rs(1 hunks)program-libs/account-checks/src/account_iterator.rs(2 hunks)program-libs/account-checks/src/checks.rs(1 hunks)program-libs/account-checks/src/error.rs(2 hunks)program-libs/account-checks/src/packed_accounts.rs(3 hunks)program-libs/account-checks/tests/tests.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
program-libs/account-checks/src/account_iterator.rs (2)
program-libs/account-checks/src/account_info/account_info_trait.rs (1)
pubkey(21-21)program-libs/compressed-account/src/pubkey.rs (1)
new_from_array(68-70)
program-libs/account-checks/src/account_info/test_account_info.rs (1)
program-libs/account-checks/src/account_info/account_info_trait.rs (2)
is_signer(23-23)is_writable(22-22)
🪛 markdownlint-cli2 (0.18.1)
program-libs/account-checks/docs/ERRORS.md
120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
program-libs/account-checks/docs/PACKED_ACCOUNTS.md
112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
program-libs/account-checks/docs/ACCOUNT_ITERATOR.md
237-237: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (19)
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- 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-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: cli-v1
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-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, cargo test...
- GitHub Check: lint
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
🔇 Additional comments (10)
program-libs/account-checks/docs/CLAUDE.md (1)
1-34: LGTM!The documentation structure is clear, well-organized, and provides excellent navigation guidance for the account-checks crate. The overview effectively introduces the core components, type system, and utilities with appropriate cross-references.
program-libs/account-checks/docs/DISCRIMINATOR.md (1)
1-221: LGTM!Comprehensive documentation that effectively explains the Discriminator trait, its purpose, and usage patterns. The code examples are clear and demonstrate both basic and advanced scenarios. The best practices section provides valuable guidance on avoiding common pitfalls.
program-libs/account-checks/tests/tests.rs (1)
54-57: LGTM!The discriminator value update from sequential bytes
[1,2,3,4,5,6,7,8]to[180, 4, 231, 26, 220, 144, 55, 168]makes the test more realistic by using a proper discriminator value rather than a simple placeholder.program-libs/account-checks/src/checks.rs (1)
84-88: LGTM!The discriminator mismatch logging is a valuable debugging aid that helps developers quickly identify the expected vs. actual discriminator values when validation fails. The message format is clear and the logging is appropriately placed in the error path.
program-libs/account-checks/src/account_info/test_account_info.rs (2)
34-35: LGTM!The type change from
i32tou32fororiginal_data_lenis correct, as data lengths are always non-negative. This aligns with proper type semantics for size fields.
29-29: Ensure borrow_state matches Pinocchio’s Account layout
Local search didn’t reveal theAccountstruct; manually inspect theborrow_statefield in the Pinocchio crate to confirm that unborrowed is represented as0and update tests accordingly.program-libs/account-checks/src/packed_accounts.rs (2)
5-9: LGTM!The
ProgramPackedAccountsstruct provides a clean abstraction for index-based dynamic account access. The generic design overAccountInfoTraitenables cross-SDK compatibility as documented.
16-16: LGTM!Moving
Location::caller()to the start of the function is a good optimization that avoids redundant calls in the error path. The#[track_caller]attribute ensures the location still accurately reflects the call site.program-libs/account-checks/src/account_info/account_info_trait.rs (1)
10-10: Approve PartialEq bound additionBoth
solana_pubkey::Pubkeyand[u8; 32]satisfyPartialEq, so the added bound is safe.program-libs/account-checks/src/account_iterator.rs (1)
65-86: next_checked_pubkey: solid addition; validates key with precise error contextImplementation is correct and uses track_caller + detailed logging. LGTM.
Please confirm InvalidAccount is the intended variant for pubkey mismatch in your error taxonomy.
| fn check_account_balance_is_rent_exempt<A: AccountInfoTrait>( | ||
| account_info: &A, | ||
| expected_size: usize | ||
| ) -> Result<u64, AccountError> | ||
| ``` | ||
| - Verifies account size and rent exemption | ||
| - Returns rent exemption amount | ||
| - **Errors:** | ||
| - `InvalidAccountSize` (12010) - Size mismatch | ||
| - `InvalidAccountBalance` (12013) - Below rent exemption | ||
| - `FailedBorrowRentSysvar` (12014) - Can't access rent | ||
|
|
There was a problem hiding this comment.
check_account_balance_is_rent_exempt signature and errors.
Implementation returns pub fn ... -> Result<(), AccountError> (no u64). It also can emit InvalidAccount when data length/lamports borrow fails. Update doc accordingly.
🤖 Prompt for AI Agents
program-libs/account-checks/docs/ACCOUNT_CHECKS.md around lines 164 to 175: the
docblock for check_account_balance_is_rent_exempt is inaccurate — the
implementation returns Result<(), AccountError> (not Result<u64, AccountError>)
and may emit an InvalidAccount error when borrowing data/lamports fails. Update
the signature line to show -> Result<(), AccountError>, remove the statement
that it "Returns rent exemption amount", and update the Errors list to include:
InvalidAccountSize (12010) - size mismatch; InvalidAccountBalance (12013) -
below rent exemption; FailedBorrowRentSysvar (12014) - can't access rent; and
InvalidAccount (add the correct code used in the codebase) - failure borrowing
account data/lamports. Ensure wording describes that the function only verifies
size and rent-exempt status and returns Ok(()) on success.
| | Function | Error Code | Error Name | Condition | | ||
| |----------|------------|------------|-----------| | ||
| | `check_owner` | 12007 | AccountOwnedByWrongProgram | Owner mismatch | | ||
| | `check_mut` | 12008 | AccountNotMutable | Not writable | | ||
| | `check_discriminator` | 12006 | InvalidDiscriminator | Wrong type | | ||
| | `check_discriminator` | 12010 | InvalidAccountSize | < 8 bytes | | ||
| | `set_discriminator` | 12012 | AlreadyInitialized | Non-zero disc | | ||
| | `check_non_mut` | 12011 | AccountMutable | Is writable | | ||
| | `check_signer` | 12015 | InvalidSigner | Not signer | | ||
| | `check_pda_seeds*` | 12016 | InvalidSeeds | PDA mismatch | | ||
| | `check_program` | 12017 | InvalidProgramId | Key mismatch | | ||
| | `check_program` | 12018 | ProgramNotExecutable | Not executable | | ||
| | `check_data_is_zeroed` | 12019 | AccountNotZeroed | Has data | | ||
| | `check_account_balance_*` | 12010 | InvalidAccountSize | Size mismatch | | ||
| | `check_account_balance_*` | 12013 | InvalidAccountBalance | Low balance | | ||
| | `account_info_init` | 12009 | BorrowAccountDataFailed | Can't borrow | | ||
|
|
There was a problem hiding this comment.
Error table omits InvalidAccount and misstates return values.
Multiple functions in this table can emit AccountError::InvalidAccount, and check_account_balance_is_rent_exempt does not return a rent amount. Please revise the table to match code.
🤖 Prompt for AI Agents
In program-libs/account-checks/docs/ACCOUNT_CHECKS.md around lines 303-319, the
error table omits that many functions can return AccountError::InvalidAccount
and incorrectly describes the behavior of check_account_balance_is_rent_exempt
(it does not return a rent amount) and misstates some error-code mappings;
update the table to (1) add an "InvalidAccount | <correct code>" entry for every
function that can emit AccountError::InvalidAccount (matching the code paths),
(2) remove any claim that check_account_balance_is_rent_exempt returns a rent
amount and instead describe it as checking rent-exemptness (and set the correct
error name/code from the implementation), and (3) fix any duplicated/misassigned
error codes (e.g., the 12010 usage) so each function maps to the exact error
name and numeric code used in the source.
ananas-block
force-pushed
the
jorrit/feat-account-checks-from-pinocchio
branch
from
c07c85d to
2dba902
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
program-libs/account-checks/docs/ACCOUNT_CHECKS.md (2)
162-175: Fix signature/return description forcheck_account_balance_is_rent_exempt.The implementation at
src/checks.rs::check_account_balance_is_rent_exemptreturnsResult<(), AccountError>and only verifies rent exemption; it never yields a rent amount. Please update the signature, drop the “Returns rent exemption amount” bullet, and document theInvalidAccount(12021) path when borrowing lamports/data fails.- expected_size: usize -) -> Result<u64, AccountError> + expected_size: usize +) -> Result<(), AccountError>-- Verifies account size and rent exemption -- Returns rent exemption amount +- Verifies account size and rent exemption, returning `Ok(())` on success- `InvalidAccountSize` (12010) - Size mismatch - `InvalidAccountBalance` (12013) - Below rent exemption - `FailedBorrowRentSysvar` (12014) - Can't access rent + - `InvalidAccount` (12021) - Failed to borrow lamports or data
303-319: Bring the error table back in sync with the code.Functions like
check_account_balance_is_rent_exemptcan emitAccountError::InvalidAccount(12021), and the rent-check functions returnOk(()), not a rent amount. Please add the missingInvalidAccountentries where applicable and correct the description for the rent-check helpers so the table reflects actual behavior.program-libs/account-checks/docs/ERRORS.md (1)
120-134: Correct hex values and add fenced language.The log example still lacks a language specifier, and the hex conversions for 12006/12020/12021 are off by 0x10. Update the block to use ```text and set the hex values to 0x2ee6, 0x2ef4, and 0x2ef5 respectively so the docs line up with the emitted codes.
-``` -Program log: ERROR: Invalid Discriminator. -Program log: Custom program error: 0x2ef6 // 12006 in hex - -Program log: ERROR: Not enough accounts. Requested 'mint' at index 3 but only 2 accounts available. -Program log: Custom program error: 0x2f04 // 12020 in hex -``` +```text +Program log: ERROR: Invalid Discriminator. +Program log: Custom program error: 0x2ee6 // 12006 in hex + +Program log: ERROR: Not enough accounts. Requested 'mint' at index 3 but only 2 accounts available. +Program log: Custom program error: 0x2ef4 // 12020 in hex +``` ```diff - - 12006 = 0x2EF6 - - 12020 = 0x2F04 - - 12021 = 0x2F05 + - 12006 = 0x2EE6 + - 12020 = 0x2EF4 + - 12021 = 0x2EF5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
program-libs/account-checks/CLAUDE.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_CHECKS.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_INFO_TRAIT.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_ITERATOR.md(1 hunks)program-libs/account-checks/docs/CLAUDE.md(1 hunks)program-libs/account-checks/docs/DISCRIMINATOR.md(1 hunks)program-libs/account-checks/docs/ERRORS.md(1 hunks)program-libs/account-checks/docs/PACKED_ACCOUNTS.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- program-libs/account-checks/docs/CLAUDE.md
- program-libs/account-checks/docs/ACCOUNT_ITERATOR.md
- program-libs/account-checks/docs/DISCRIMINATOR.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
program-libs/account-checks/docs/ERRORS.md
120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
program-libs/account-checks/docs/PACKED_ACCOUNTS.md
112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (19)
- 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, cargo test...
- GitHub Check: cli-v2
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
| ## Error Messages | ||
|
|
||
| ProgramPackedAccounts provides detailed error messages with `#[track_caller]`: | ||
|
|
||
| ``` | ||
| ERROR: Not enough accounts. Requested 'mint' at index 5 but only 3 accounts available. src/processor.rs:42:18 | ||
| ERROR: Not enough accounts. Requested 'merkle_tree' at index 10 but only 8 accounts available. src/processor.rs:55:23 | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the log snippet.
markdownlint is flagging this block (MD040). Prefix it with text so the linter passes and the snippet renders consistently.
-```
+```text
ERROR: Not enough accounts. Requested 'mint' at index 5 but only 3 accounts available. src/processor.rs:42:18
ERROR: Not enough accounts. Requested 'merkle_tree' at index 10 but only 8 accounts available. src/processor.rs:55:23🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
program-libs/account-checks/docs/PACKED_ACCOUNTS.md around lines 108 to 116: the
fenced code block with error messages lacks a language identifier and triggers
markdownlint MD040; update the opening fence from ``` to ```text so the snippet
is explicitly marked as plain text (leave the content unchanged and keep the
closing ```).
- Add missing FailedBorrowRentSysvar (12014) to error table - Fix hex code conversions: 12006=0x2EE6, 12020=0x2EF4, 12021=0x2EF5 - Add language identifier 'text' to code fence in log examples Addresses CodeRabbit review feedback.
Migrated all AccountError codes from 12006-12021 to 20000-20015: - InvalidDiscriminator: 12006 → 20000 (0x4E20) - AccountOwnedByWrongProgram: 12007 → 20001 - AccountNotMutable: 12008 → 20002 - BorrowAccountDataFailed: 12009 → 20003 - InvalidAccountSize: 12010 → 20004 - AccountMutable: 12011 → 20005 - AlreadyInitialized: 12012 → 20006 - InvalidAccountBalance: 12013 → 20007 - FailedBorrowRentSysvar: 12014 → 20008 - InvalidSigner: 12015 → 20009 - InvalidSeeds: 12016 → 20010 - InvalidProgramId: 12017 → 20011 - ProgramNotExecutable: 12018 → 20012 - AccountNotZeroed: 12019 → 20013 - NotEnoughAccountKeys: 12020 → 20014 (0x4E2E) - InvalidAccount: 12021 → 20015 (0x4E2F) Updated all documentation with new error codes and hex values. All tests passing.
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 (4)
program-libs/account-checks/docs/ACCOUNT_CHECKS.md(1 hunks)program-libs/account-checks/docs/ACCOUNT_ITERATOR.md(1 hunks)program-libs/account-checks/docs/ERRORS.md(1 hunks)program-libs/account-checks/src/error.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- program-libs/account-checks/docs/ACCOUNT_CHECKS.md
🚧 Files skipped from review as they are similar to previous changes (2)
- program-libs/account-checks/docs/ERRORS.md
- program-libs/account-checks/docs/ACCOUNT_ITERATOR.md
🧰 Additional context used
🧬 Code graph analysis (1)
program-libs/account-checks/src/error.rs (1)
program-libs/compressed-account/src/lib.rs (5)
from(83-107)from(112-114)from(119-121)from(141-150)from(189-197)
⏰ 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). (19)
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v2
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- 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-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- 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, cargo test...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: lint
SwenSchaeferjohann
deleted the
jorrit/feat-account-checks-from-pinocchio
branch
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests