Skip to content

fix: CToken checked deserialization#1995

Merged
ananas-block merged 1 commit intomainfrom
jorrit/fix-add-initialized-checks
Oct 16, 2025
Merged

fix: CToken checked deserialization#1995
ananas-block merged 1 commit intomainfrom
jorrit/fix-add-initialized-checks

Conversation

@ananas-block
Copy link
Contributor

@ananas-block ananas-block commented Oct 16, 2025

Summary by CodeRabbit

  • New Features

    • Added validated deserialization for token accounts that enforces initialization and state checks.
  • Tests

    • Added tests covering uninitialized, frozen, and undersized account scenarios for the new checked paths.
  • Improvements

    • Updated token operations to use the validated deserialization paths for consistent, centralized error handling.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

Adds checked zero-copy deserializers to CToken that validate minimum buffer length (109 bytes) and an initialized state byte, updates call sites to use these checks, and adjusts tests to expect unified InvalidAccountState errors for uninitialized/frozen accounts.

Changes

Cohort / File(s) Summary
Core Implementation
program-libs/ctoken-types/src/state/ctoken/zero_copy.rs
Adds zero_copy_at_checked and zero_copy_at_mut_checked which validate buffer length (>=109) and that byte at offset 108 == 1 (initialized). Return InvalidAccountData or InvalidAccountState as appropriate, otherwise delegate to existing zero-copy paths.
Unit Tests (ctoken)
program-libs/ctoken-types/tests/ctoken/failing.rs
Adds tests asserting InvalidAccountState for uninitialized (byte 108 = 0) and frozen (byte 108 = 2) buffers, and InvalidAccountData for undersized buffers; covers both checked and mut_checked variants.
Integration Test Expectations
program-tests/compressed-token-test/tests/ctoken/close.rs, program-tests/compressed-token-test/tests/ctoken/compress_and_close.rs
Update expected RPC/error codes: previous UninitializedAccount / AccountFrozen expectations replaced with CTokenError::InvalidAccountState (unified error) in relevant tests.
Program Call Sites
programs/compressed-token/program/src/claim.rs, programs/compressed-token/program/src/close_token_account/processor.rs, programs/compressed-token/program/src/ctoken_transfer.rs, programs/compressed-token/program/src/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs
Replace CToken::zero_copy_at / zero_copy_at_mut calls with zero_copy_at_checked / zero_copy_at_mut_checked. Add ownership check (check_owner) in claim.rs. Remove unused ZeroCopyAt/ZeroCopyAtMut trait imports; errors now propagate via ?.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant CTokenChecked as CToken::zero_copy_at_checked
    participant Validator
    participant CTokenUnchecked as CToken::zero_copy_at

    Caller->>CTokenChecked: call with bytes
    activate CTokenChecked

    CTokenChecked->>Validator: verify len >= 109
    activate Validator
    alt len < 109
        Validator-->>CTokenChecked: InvalidAccountData
    else len OK
        Validator->>Validator: check state_byte == 1 (offset 108)
        alt state != 1
            Validator-->>CTokenChecked: InvalidAccountState
        else state == 1
            Validator-->>CTokenChecked: OK
        end
    end
    deactivate Validator

    opt validation OK
        CTokenChecked->>CTokenUnchecked: delegate to unchecked zero-copy
        activate CTokenUnchecked
        CTokenUnchecked-->>CTokenChecked: (ZCToken, remaining_bytes)
        deactivate CTokenUnchecked
    end

    CTokenChecked-->>Caller: Result or CTokenError
    deactivate CTokenChecked
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • sergeytimoshin
  • SwenSchaeferjohann

Poem

🐰 With whiskers twitch and bytes in paw,

I peek at offset one-oh-eight — a law.
If length is short or state untrue,
I thump and shout InvalidAccountState — boo!
Hoppy checks keep tokens safe for you.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: CToken checked deserialization" accurately reflects the primary change in the changeset. The PR introduces two new public methods—zero_copy_at_checked and zero_copy_at_mut_checked—that add validation checks (minimum length and initialization state verification) to the CToken deserialization process. The title is concise, specific, and clearly communicates the main objective without unnecessary detail. It appropriately captures that the PR is addressing deserialization safety by introducing checked variants throughout the codebase, replacing unchecked methods in multiple files and adding corresponding test coverage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jorrit/fix-add-initialized-checks

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ananas-block ananas-block force-pushed the jorrit/fix-add-initialized-checks branch from 534075a to f059970 Compare October 16, 2025 16:45
@ananas-block ananas-block force-pushed the jorrit/fix-add-initialized-checks branch from f059970 to a8de2cb Compare October 16, 2025 16:58
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
program-tests/compressed-token-test/tests/ctoken/compress_and_close.rs (1)

843-844: Update comment to reflect the actual error.

The error expectation correctly changed to InvalidAccountState (18036), but the comment on line 843 still mentions "AccountFrozen". This could confuse future readers.

Apply this diff to align the comment with the actual error:

-        // Assert that the transaction failed with account frozen error
-        // Error: InvalidAccountState (18036)
+        // Assert that the transaction failed with InvalidAccountState error
+        // Frozen accounts are rejected by zero-copy checked deserialization
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f059970 and a8de2cb.

📒 Files selected for processing (8)
  • program-libs/ctoken-types/src/state/ctoken/zero_copy.rs (1 hunks)
  • program-libs/ctoken-types/tests/ctoken/failing.rs (2 hunks)
  • program-tests/compressed-token-test/tests/ctoken/close.rs (2 hunks)
  • program-tests/compressed-token-test/tests/ctoken/compress_and_close.rs (1 hunks)
  • programs/compressed-token/program/src/claim.rs (2 hunks)
  • programs/compressed-token/program/src/close_token_account/processor.rs (2 hunks)
  • programs/compressed-token/program/src/ctoken_transfer.rs (1 hunks)
  • programs/compressed-token/program/src/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • programs/compressed-token/program/src/claim.rs
  • programs/compressed-token/program/src/ctoken_transfer.rs
  • programs/compressed-token/program/src/close_token_account/processor.rs
  • program-libs/ctoken-types/src/state/ctoken/zero_copy.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T06:33:19.426Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/compressible/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:19.426Z
Learning: Applies to program-libs/compressible/src/config.rs : Provide default initialization for the CToken V1 config in CompressibleConfig

Applied to files:

  • program-libs/ctoken-types/tests/ctoken/failing.rs
📚 Learning: 2025-10-16T06:33:55.362Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: programs/compressed-token/program/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:55.362Z
Learning: Applies to programs/compressed-token/program/program-libs/ctoken-types/** : Define all state and instruction data structures in the light-ctoken-types crate (program-libs/ctoken-types), including state/, instructions/, and state/extensions/

Applied to files:

  • program-libs/ctoken-types/tests/ctoken/failing.rs
🧬 Code graph analysis (3)
program-libs/ctoken-types/tests/ctoken/failing.rs (1)
program-libs/ctoken-types/src/state/ctoken/zero_copy.rs (2)
  • zero_copy_at_checked (440-455)
  • zero_copy_at_mut_checked (460-474)
program-tests/compressed-token-test/tests/ctoken/compress_and_close.rs (1)
sdk-libs/program-test/src/utils/assert.rs (1)
  • assert_rpc_error (7-144)
programs/compressed-token/program/src/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs (1)
program-libs/ctoken-types/src/state/ctoken/zero_copy.rs (1)
  • zero_copy_at_mut_checked (460-474)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
  • GitHub Check: Test batched-merkle-tree-simulate
  • GitHub Check: Test program-libs-fast
  • GitHub Check: Test program-libs-slow
  • GitHub Check: Forester e2e test
  • GitHub Check: programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_compress...
  • GitHub Check: programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
  • GitHub Check: programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test_transf...
  • GitHub Check: programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_address", "c...
  • GitHub Check: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
  • GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
  • GitHub Check: programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test --test v1", "cargo-...
  • GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
  • GitHub Check: cli-v2
  • GitHub Check: stateless-js-v1
  • GitHub Check: stateless-js-v2
  • GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-compr...
  • 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-token-test-program, ["cargo-test-sbf -p sdk-token-test"])
  • GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
  • GitHub Check: lint
  • GitHub Check: cli-v1
🔇 Additional comments (5)
program-libs/ctoken-types/tests/ctoken/failing.rs (1)

24-96: LGTM! Comprehensive test coverage for checked zero-copy deserialization.

The new tests thoroughly validate the safety checks introduced by zero_copy_at_checked and zero_copy_at_mut_checked:

  • Correctly verify that uninitialized accounts (byte 108 = 0) return InvalidAccountState
  • Correctly verify that frozen accounts (byte 108 = 2) return InvalidAccountState
  • Correctly verify that undersized buffers (< 109 bytes) return InvalidAccountData
  • Cover both immutable and mutable variants
program-tests/compressed-token-test/tests/ctoken/close.rs (2)

267-267: LGTM! Error expectation updated for checked deserialization.

The change from UninitializedAccount (10) to InvalidAccountState (18036) correctly reflects that the new checked deserialization validates the state byte and returns InvalidAccountState for uninitialized accounts (byte 108 = 0).


320-320: LGTM! Error expectation updated for checked deserialization.

The change from AccountFrozen (76) to InvalidAccountState (18036) correctly reflects that the new checked deserialization validates the state byte and returns InvalidAccountState for frozen accounts (byte 108 = 2).

programs/compressed-token/program/src/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs (2)

39-39: LGTM! Security improvement with checked deserialization.

Replacing zero_copy_at_mut with zero_copy_at_mut_checked adds important safety validation:

  • Verifies buffer is at least 109 bytes
  • Verifies account is initialized (byte 108 == 1)

This prevents potential buffer overflows and ensures only valid accounts are processed.


55-58: Remove the unreachable frozen account check (lines 55-58).

The zero_copy_at_mut_checked call at line 39 already enforces state == 1 (rejecting frozen accounts with state == 2 via InvalidAccountState). The check at lines 55-58 is therefore unreachable and redundant. The TODO comment (lines 51-54) suggests frozen account support is planned for the future; remove this dead code now and implement the proper handling when that feature is actually developed.

Likely an incorrect or invalid review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants