refactor: relax ata decompress signer check, feat: add ata decompress idempotent#2360
Open
ananas-block wants to merge 9 commits intomainfrom
Open
refactor: relax ata decompress signer check, feat: add ata decompress idempotent#2360ananas-block wants to merge 9 commits intomainfrom
ananas-block wants to merge 9 commits intomainfrom
Conversation
ananas-block
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a permissionless, idempotent ATA-decompress path to Transfer2: when decompress mode includes a Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Instruction
participant Processor as Transfer2 Processor
participant TokenInput as Token Input Resolver
participant MerkleTree as BatchedMerkleTreeAccount
Client->>Processor: Transfer2 (Decompress, CompressedOnly is_ata=true)
Processor->>Processor: Validate single-input & single-compression
Processor->>Processor: Compute deterministic account_hash
Processor->>MerkleTree: Load tree & check_input_queue_non_inclusion(account_hash)
alt Bloom indicates already spent
MerkleTree-->>Processor: Bloom hit -> spent
Processor-->>Client: Return Ok() (no-op)
else Not present
MerkleTree-->>Processor: Not found
Processor->>TokenInput: set_input_compressed_account(is_ata_decompress=true)
TokenInput->>TokenInput: Skip verify_owner_or_delegate_signer
TokenInput-->>Processor: Signer resolved (permissionless flow)
Processor->>Processor: Continue normal decompress CPI processing
Processor-->>Client: Return Ok() (decompressed)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
programs/compressed-token/program/src/compressed_token/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs (1)
97-129:⚠️ Potential issue | 🟠 Major
DecompressIdempotentdoes not validate that the ATA is pre-initialized as required by the specification.The TRANSFER2.md documentation states "ATA must be pre-created" for
DecompressIdempotent, but the implementation passes the same validation path to bothDecompressandDecompressIdempotentwithout mode awareness. Thevalidate_and_apply_compressed_only()helper does not receive the compression mode, so it cannot enforce mode-specific initialization requirements. This meansDecompressIdempotenthas no way to verify that the destination ATA already exists and is initialized, as the spec requires.Pass the mode to the helper and add a mode-aware check:
Decompress: destination may be fresh or pre-existing (current behavior)DecompressIdempotent: destination must be pre-initialized (currently unenforced)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@programs/compressed-token/program/src/compressed_token/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs` around lines 97 - 129, The DecompressIdempotent branch doesn't enforce the TRANSFER2.md requirement that the destination ATA be pre-initialized; update the Decompress/DecompressIdempotent handling to pass the current ZCompressionMode into validate_and_apply_compressed_only (or add an extra parameter) and implement a mode-aware check inside validate_and_apply_compressed_only that asserts the destination ATA is already created/initialized when mode == ZCompressionMode::DecompressIdempotent but allows fresh destinations for ZCompressionMode::Decompress; reference validate_and_apply_compressed_only and ZCompressionMode::DecompressIdempotent when adding the validation and adjust callers accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@programs/compressed-token/program/CLAUDE.md`:
- Around line 71-73: The doc statement about permissionless ATA decompress is
too broad; update the line referencing Decompress and DecompressIdempotent to
specify that the permissionless path applies only to CToken-associated token
accounts (CToken-ATA) rather than all SPL token accounts — mention the specific
symbols Decompress, DecompressIdempotent (mode 3), and the is_ata=true flag and
align wording with the implementation in transfer2/compression/spl.rs which
rejects DecompressIdempotent for regular SPL token accounts.
In
`@programs/compressed-token/program/src/compressed_token/transfer2/compression/spl.rs`:
- Around line 81-84: The match arm for ZCompressionMode::DecompressIdempotent
currently returns ProgramError::InvalidInstructionData which hides the real
cause; change it to return a named token error (e.g.,
Err(TokenError::InvalidCompressionMode.into()) or a new TokenError variant
dedicated to unsupported DecompressIdempotent) so callers can distinguish
malformed payloads from unsupported compression modes; update imports/usages in
transfer2::compression::spl.rs to bring TokenError into scope and add the new
TokenError variant if it doesn't exist, ensuring the error maps to ProgramError
via .into().
In `@sdk-libs/compressed-token-sdk/src/compressed_token/v2/account2.rs`:
- Around line 248-271: is_decompress() currently only matches
CompressionMode::Decompress and thus returns false for idempotent flows; update
the is_decompress() implementation to also treat
CompressionMode::DecompressIdempotent as a decompress case so that calls like
decompress_idempotent (which sets Compression::decompress_idempotent) are
recognized as decompress operations; locate is_decompress() and add
DecompressIdempotent to the match/conditional alongside Decompress to return
true.
---
Outside diff comments:
In
`@programs/compressed-token/program/src/compressed_token/transfer2/compression/ctoken/compress_or_decompress_ctokens.rs`:
- Around line 97-129: The DecompressIdempotent branch doesn't enforce the
TRANSFER2.md requirement that the destination ATA be pre-initialized; update the
Decompress/DecompressIdempotent handling to pass the current ZCompressionMode
into validate_and_apply_compressed_only (or add an extra parameter) and
implement a mode-aware check inside validate_and_apply_compressed_only that
asserts the destination ATA is already created/initialized when mode ==
ZCompressionMode::DecompressIdempotent but allows fresh destinations for
ZCompressionMode::Decompress; reference validate_and_apply_compressed_only and
ZCompressionMode::DecompressIdempotent when adding the validation and adjust
callers accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44c31050-3598-4586-ac6f-b42f507cb049
⛔ Files ignored due to path filters (5)
js/compressed-token/src/v3/layout/layout-transfer2.tsis excluded by none and included by noneprogram-tests/compressed-token-test/tests/compress_only/ata_decompress.rsis excluded by none and included by noneprogram-tests/utils/src/actions/legacy/instructions/transfer2.rsis excluded by none and included by noneprogram-tests/utils/src/actions/legacy/transfer2/decompress.rsis excluded by none and included by noneprogram-tests/utils/src/assert_transfer2.rsis excluded by none and included by none
📒 Files selected for processing (11)
program-libs/token-interface/src/error.rsprogram-libs/token-interface/src/instructions/transfer2/compression.rsprograms/compressed-token/program/CLAUDE.mdprograms/compressed-token/program/docs/compressed_token/TRANSFER2.mdprograms/compressed-token/program/src/compressed_token/transfer2/compression/ctoken/compress_or_decompress_ctokens.rsprograms/compressed-token/program/src/compressed_token/transfer2/compression/ctoken/inputs.rsprograms/compressed-token/program/src/compressed_token/transfer2/compression/mod.rsprograms/compressed-token/program/src/compressed_token/transfer2/compression/spl.rsprograms/compressed-token/program/src/compressed_token/transfer2/processor.rsprograms/compressed-token/program/src/shared/token_input.rssdk-libs/compressed-token-sdk/src/compressed_token/v2/account2.rs
programs/compressed-token/program/src/compressed_token/transfer2/compression/spl.rs
Outdated
Show resolved
Hide resolved
sdk-libs/compressed-token-sdk/src/compressed_token/v2/account2.rs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@programs/compressed-token/program/src/compressed_token/transfer2/processor.rs`:
- Around line 316-318: Before deserializing the tree with
BatchedMerkleTreeAccount::state_from_account_info, add an explicit validation
that the provided tree_account refers to a V2 tree by checking the merkle tree
metadata type against STATE_MERKLE_TREE_TYPE_V2; if the metadata type is not V2,
return an error. Concretely, read the merkle-tree metadata/header from
tree_account (using the merkle tree metadata helper or by checking the metadata
type field exposed by the merkle_tree module), compare it to
merkle_tree::STATE_MERKLE_TREE_TYPE_V2, and only then call
BatchedMerkleTreeAccount::state_from_account_info; this mirrors the defensive
pattern around DecompressIdempotent/is_ata and makes the version requirement
explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d04d31c7-2503-4d70-a43a-6a7b5b577369
⛔ Files ignored due to path filters (4)
Cargo.lockis excluded by!**/*.lockand included by noneprogram-tests/compressed-token-test/tests/compress_only/ata_decompress.rsis excluded by none and included by noneprogram-tests/utils/src/actions/legacy/instructions/transfer2.rsis excluded by none and included by noneprogram-tests/utils/src/actions/legacy/transfer2/decompress.rsis excluded by none and included by none
📒 Files selected for processing (4)
program-libs/token-interface/src/instructions/transfer2/compression.rsprograms/compressed-token/program/Cargo.tomlprograms/compressed-token/program/src/compressed_token/transfer2/config.rsprograms/compressed-token/program/src/compressed_token/transfer2/processor.rs
programs/compressed-token/program/src/compressed_token/transfer2/processor.rs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
programs/compressed-token/program/CLAUDE.md (1)
72-72: 🧹 Nitpick | 🔵 TrivialThe permissionless ATA decompress documentation may need scope clarification.
This accurately documents the new feature, but per the earlier review discussion, consider clarifying that this permissionless path applies specifically to CToken-associated token accounts (where
is_ata=truein theCompressedOnlyextension), not to standard SPL token accounts. The implementation intransfer2/compression/spl.rshandles SPL token accounts differently.The referenced
docs/compressed_token/TRANSFER2.mdprovides the detailed breakdown, so you could tighten this line to:- - ATA decompress (is_ata=true) is permissionless and idempotent (bloom filter check) + - CToken ATA decompress (is_ata=true) is permissionless and idempotent (bloom filter check)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@programs/compressed-token/program/CLAUDE.md` at line 72, The sentence about "ATA decompress (is_ata=true) is permissionless and idempotent (bloom filter check)" is ambiguous—update the doc to state that the permissionless ATA decompress path applies only to CToken-associated token accounts that have the CompressedOnly extension with is_ata=true (i.e., CToken-associated ATAs), and not to regular SPL token accounts; reference the implementation in transfer2/compression/spl.rs which treats SPL accounts differently and ensure the wording explicitly distinguishes "CToken-associated token accounts (CompressedOnly.is_ata=true)" from "standard SPL token accounts."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@programs/compressed-token/program/CLAUDE.md`:
- Line 72: The sentence about "ATA decompress (is_ata=true) is permissionless
and idempotent (bloom filter check)" is ambiguous—update the doc to state that
the permissionless ATA decompress path applies only to CToken-associated token
accounts that have the CompressedOnly extension with is_ata=true (i.e.,
CToken-associated ATAs), and not to regular SPL token accounts; reference the
implementation in transfer2/compression/spl.rs which treats SPL accounts
differently and ensure the wording explicitly distinguishes "CToken-associated
token accounts (CompressedOnly.is_ata=true)" from "standard SPL token accounts."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43685556-5eb4-446d-829c-c9268d5b531a
⛔ Files ignored due to path filters (2)
program-tests/compressed-token-test/tests/compress_only/ata_decompress.rsis excluded by none and included by noneprogram-tests/utils/src/actions/legacy/instructions/transfer2.rsis excluded by none and included by none
📒 Files selected for processing (3)
programs/compressed-token/program/CLAUDE.mdprograms/compressed-token/program/docs/compressed_token/TRANSFER2.mdprograms/compressed-token/program/src/compressed_token/transfer2/processor.rs
The is_ata_decompress detection was too broad -- it triggered for any tx containing a Decompress compression and any CompressedOnly TLV with is_ata=true. This broke multi-account decompress batches (e.g., AMM stress test) that contain a mix of ATA and non-ATA accounts. Fix: require exactly 1 input and 1 compression as part of the detection condition, not as a separate validation that errors. Multi-input decompress txs now skip the idempotency path entirely.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/client/src/interface/load_accounts.rs`:
- Line 367: The code currently uses packed.insert_or_get(ctx.wallet_owner) which
marks the wallet_owner account writable; replace that call with
packed.insert_or_get_read_only(ctx.wallet_owner) so wallet_owner remains
read-only (since it’s only used to derive/verify the ATA and never mutated),
preserving permissionless behavior and improving transaction parallelism—update
the call site referencing insert_or_get in the load_accounts logic (where
wallet_owner is inserted and later used to verify the ATA).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6d232d4-fd0c-4aaf-aef1-fe3bdd0f9e03
📒 Files selected for processing (1)
sdk-libs/client/src/interface/load_accounts.rs
…o longer requires owner signature
… and d10_token_accounts_test
ananas-block
force-pushed
the
jorrit/chore-relax-ata-decompress
branch
from
54c9c52 to
4b38472
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Permissionless associated token account decompress
Payments need to load the associated token account balance of the sender.
Currently only the owner or delegate can decompress an associated token account.
Thus, if the recipient has an existing compressed associated token account balance the sender creates a new empty associated token account for the recipient which once compressed creates a second compressed balance.
Many transfers can create a compressed associated token account balance that cannot be decompressed within one Solana transaction.
Permissionless associated token account decompression allows the sender to decompress the balance of the recipient so that the recipient balance accumulates in one compressed balance should the associated token account be compressed.
Idempotent associated token account decompress
Permissionless decompress instructions must be idempotent so that nobody can front run a decompress transaction to fail it. Decompression can only be idempotent for one input compressed account because if a validity proof is computed over multiple inputs it will fail if we omit one.
Changes
token_input.rs -- Signer bypass
processor.rs -- Idempotent decompress
Safety