fix: compress and close hotpath#2059
Conversation
WalkthroughAdded a new error variant and made the compressed-token output optional in inputs; the program now requires that compressed token output be present when the compression authority is a signer and returns the new error if it is missing. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK
participant Program
participant Validator
Client->>SDK: create_ctoken_to_spl_transfer_and_close_instruction(...)
SDK->>Program: submit combined transfer2 instruction (compress_and_close + decompress_spl)
Program->>Validator: is compression_authority signer?
alt compression_authority_is_signer
Validator->>Program: is compressed_token_account present?
alt present
Program->>Validator: validate compressed token account
Program->>Client: success
else missing
Program-->>Client: CompressAndCloseOutputMissing error
end
else
Program->>Client: proceed without requiring compressed_token_account
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
program-tests/compressed-token-test/tests/transfer2/spl_ctoken.rsis excluded by none and included by none
📒 Files selected for processing (5)
programs/compressed-token/anchor/src/lib.rs(1 hunks)programs/compressed-token/program/src/transfer2/compression/ctoken/compress_and_close.rs(1 hunks)programs/compressed-token/program/src/transfer2/compression/ctoken/inputs.rs(2 hunks)programs/compressed-token/program/src/transfer2/compression/spl.rs(0 hunks)sdk-libs/compressed-token-sdk/src/account2.rs(1 hunks)
💤 Files with no reviewable changes (1)
- programs/compressed-token/program/src/transfer2/compression/spl.rs
🧰 Additional context used
🧬 Code graph analysis (3)
programs/compressed-token/program/src/transfer2/compression/ctoken/inputs.rs (1)
sdk-libs/compressed-token-sdk/src/account2.rs (1)
compression(379-381)
programs/compressed-token/program/src/transfer2/compression/ctoken/compress_and_close.rs (1)
sdk-libs/sdk/src/instruction/pack_accounts.rs (1)
packed_accounts(348-351)
sdk-libs/compressed-token-sdk/src/account2.rs (2)
sdk-libs/sdk/src/instruction/pack_accounts.rs (1)
packed_accounts(348-351)sdk-libs/compressed-token-sdk/src/instructions/transfer2/instruction.rs (1)
create_transfer2_instruction(71-141)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v2
- 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 (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
7985b70 to
7620e7b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockand included by noneprogram-tests/compressed-token-test/tests/transfer2/spl_ctoken.rsis excluded by none and included by noneprogram-tests/registry-test/tests/compressible.rsis excluded by none and included by none
📒 Files selected for processing (6)
programs/compressed-token/anchor/src/lib.rs(1 hunks)programs/compressed-token/program/src/transfer2/compression/ctoken/compress_and_close.rs(1 hunks)programs/compressed-token/program/src/transfer2/compression/ctoken/inputs.rs(2 hunks)programs/compressed-token/program/src/transfer2/compression/spl.rs(0 hunks)sdk-libs/compressed-token-sdk/src/instructions/mod.rs(1 hunks)sdk-libs/compressed-token-sdk/src/instructions/transfer_interface.rs(1 hunks)
💤 Files with no reviewable changes (1)
- programs/compressed-token/program/src/transfer2/compression/spl.rs
🧰 Additional context used
🧬 Code graph analysis (4)
programs/compressed-token/program/src/transfer2/compression/ctoken/compress_and_close.rs (1)
sdk-libs/sdk/src/instruction/pack_accounts.rs (1)
packed_accounts(348-351)
sdk-libs/compressed-token-sdk/src/instructions/mod.rs (1)
sdk-libs/compressed-token-sdk/src/instructions/transfer_interface.rs (2)
create_ctoken_to_spl_transfer_and_close_instruction(164-236)transfer_interface(430-513)
sdk-libs/compressed-token-sdk/src/instructions/transfer_interface.rs (3)
sdk-libs/compressed-token-sdk/src/account2.rs (2)
mint(362-364)decompress_spl(250-279)sdk-libs/sdk/src/instruction/pack_accounts.rs (1)
packed_accounts(348-351)sdk-libs/compressed-token-sdk/src/instructions/transfer2/instruction.rs (1)
create_transfer2_instruction(71-141)
programs/compressed-token/program/src/transfer2/compression/ctoken/inputs.rs (1)
sdk-libs/compressed-token-sdk/src/account2.rs (1)
compression(370-372)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test_transf...
- 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: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
- GitHub Check: programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
- GitHub Check: programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_compress...
- GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
- GitHub Check: programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_address", "c...
- 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: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Forester e2e test
- GitHub Check: cli-v2
- GitHub Check: system-programs (token test, ["cargo-test-sbf -p sdk-token-test"])
- GitHub Check: system-programs (native, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v1-native-t...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-compr...
- GitHub Check: system-programs (anchor & pinocchio, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -p sdk...
🔇 Additional comments (3)
programs/compressed-token/program/src/transfer2/compression/ctoken/compress_and_close.rs (1)
51-64: LGTM! Conditional validation correctly implements the hotpath fix.The logic properly enforces that compressed output accounts are only required and validated when the compression authority is the signer. Owner-signed operations bypass this requirement, which aligns with the PR objectives.
programs/compressed-token/program/src/transfer2/compression/ctoken/inputs.rs (1)
2-4: LGTM! Optional field enables conditional validation.The change from mandatory to optional
compressed_token_accountcorrectly implements the fix, allowing error handling to be deferred tocompress_and_close.rswhere it's conditionally enforced based on signer type.Also applies to: 12-12, 60-60
sdk-libs/compressed-token-sdk/src/instructions/mod.rs (1)
57-58: LGTM! Clean re-export of the new instruction builder.The addition follows the module's existing export pattern and appropriately exposes the new functionality.
7620e7b to
742b2b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockand included by noneprogram-tests/compressed-token-test/tests/transfer2/spl_ctoken.rsis excluded by none and included by noneprogram-tests/registry-test/tests/compressible.rsis excluded by none and included by none
📒 Files selected for processing (3)
programs/compressed-token/anchor/src/lib.rs(1 hunks)programs/compressed-token/program/src/transfer2/compression/spl.rs(0 hunks)sdk-libs/compressed-token-sdk/src/instructions/mod.rs(1 hunks)
💤 Files with no reviewable changes (1)
- programs/compressed-token/program/src/transfer2/compression/spl.rs
🧰 Additional context used
🧬 Code graph analysis (1)
sdk-libs/compressed-token-sdk/src/instructions/mod.rs (1)
sdk-libs/compressed-token-sdk/src/instructions/transfer_interface.rs (2)
create_ctoken_to_spl_transfer_and_close_instruction(164-236)transfer_interface(430-513)
🪛 GitHub Actions: lint
programs/compressed-token/anchor/src/lib.rs
[warning] 424-424: Diff in message attribute: changed from #[msg("CompressAndClose: Cannot use the same compressed output account for multiple closures")] to a multi-line #[msg(...)] form for 'CompressAndCloseOutputMissing'. This appears to be a formatting/style change.
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
- 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 (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
- GitHub Check: programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test --test v1", "cargo-...
🔇 Additional comments (2)
programs/compressed-token/anchor/src/lib.rs (1)
427-428: LGTM! Past review feedback addressed.The error message now correctly uses "compression authority" terminology, consistent with the
compression_authority_is_signervalidation in the related code. The message clearly communicates that when the compression authority signs the transaction, the compressed token account must be present in the outputs.sdk-libs/compressed-token-sdk/src/instructions/mod.rs (1)
57-58: LGTM! Re-export properly placed.The new public re-export is correctly positioned with other
transfer_interfaceutilities and follows the existing module structure.
| pub use transfer_interface::{ | ||
| create_transfer_ctoken_to_spl_instruction, create_transfer_spl_to_ctoken_instruction, | ||
| transfer_interface, transfer_interface_signed, | ||
| create_ctoken_to_spl_transfer_and_close_instruction, create_transfer_ctoken_to_spl_instruction, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Address the TODO in the implementation and add documentation.
Based on the relevant code snippet showing the implementation in transfer_interface.rs (lines 163-235), two improvements are needed:
-
TODO for dynamic pool_index: Line 210 contains
pool_index (TODO: make dynamic)currently hardcoded to 0. This may work for single-pool scenarios but could be a limitation for multi-pool support. -
Missing documentation: This new public API function has 9 parameters but no doc comments explaining:
- The purpose and use case (compress-close-decompress flow)
- What each parameter represents
- When to use this vs
create_transfer_ctoken_to_spl_instruction - Any constraints or assumptions (like the hardcoded pool_index)
Would you like me to generate documentation for this function, or should the pool_index TODO be tracked in a separate issue?
🤖 Prompt for AI Agents
In sdk-libs/compressed-token-sdk/src/instructions/mod.rs around line 57,
implement the TODO for the hardcoded pool_index and add doc comments for the
public API: change the implementation so pool_index is not fixed to 0 (either
accept pool_index as an additional parameter to the public function or compute
it dynamically from the provided account metas/merkle data and validate it),
update all call sites (transfer_interface.rs and related helpers) to pass or
derive the correct pool_index, and add a complete doc comment block for the
function that explains its purpose (compress-close-decompress flow), documents
all 9 parameters (types and role), states when to use this function vs
create_transfer_ctoken_to_spl_instruction, and lists constraints/assumptions
including how pool_index is determined and any multisig/multi-pool limitations.
Issue:
Changes:
CompressAndCloseOutputMissingSummary by CodeRabbit
New Features
Bug Fixes
Chores