refactor: remove bump from create ata instruction data#2049
Closed
ananas-block wants to merge 2 commits intomainfrom
Closed
refactor: remove bump from create ata instruction data#2049ananas-block wants to merge 2 commits intomainfrom
ananas-block wants to merge 2 commits intomainfrom
Conversation
Contributor
WalkthroughThis PR removes the Changes
Sequence DiagramsequenceDiagram
participant SDK as SDK Builder
participant Program as Compressed Token Program
participant PDA as PDA Derivation
Note over SDK,Program: New flow (bump removed)
SDK->>SDK: derive_ctoken_ata(owner,mint) -> (ata_pubkey, _bump)
SDK->>Program: invoke create ATA instruction (no bump)
Program->>PDA: find_program_address(seeds)
PDA-->>Program: (expected_ata, bump_seed)
Program->>Program: compare expected_ata == provided ATA account
alt match
Program->>Program: continue create flow (possibly compressible)
else mismatch
Program-->>Program: return InvalidSeeds
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
programs/compressed-token/program/src/shared/mod.rs(0 hunks)programs/compressed-token/program/src/shared/validate_ata_derivation.rs(0 hunks)sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs(1 hunks)
💤 Files with no reviewable changes (2)
- programs/compressed-token/program/src/shared/mod.rs
- programs/compressed-token/program/src/shared/validate_ata_derivation.rs
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/account_compression_cpi/**/*.rs : CPI processing functions must derive PDA signer seeds as [CPI_AUTHORITY_PDA_SEED, bump] and use CpiContext::new_with_signer with cpi_authority as the authority account and mapped target accounts.
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/{solana.rs,pinocchio.rs,test_account_info.rs} : Gate SDK-specific implementations with #[cfg(feature = "solana"|"pinocchio"|"test-only")]
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-16T06:33:55.362Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/compressed-token/program/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:55.362Z
Learning: Applies to programs/compressed-token/program/src/create_associated_token_account.rs : Create Associated Token Account instruction must validate that the config state is ACTIVE only
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-16T06:33:19.426Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 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 : Ensure serialization compatibility across Anchor, Pinocchio, and Borsh for core account types used by dependent programs
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/{solana.rs,pinocchio.rs,test_account_info.rs} : Provide SDK-specific AccountInfoTrait implementations in account_info/{solana.rs,pinocchio.rs,test_account_info.rs}
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-15T03:46:43.242Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: sdk-tests/sdk-token-test/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:43.242Z
Learning: Applies to sdk-tests/sdk-token-test/**/tests/**/*.rs : Every test should only contain functional integration tests
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/test_account_info.rs : Use the mock AccountInfo implementation under the test-only feature for unit tests
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/account_info_trait.rs : Ensure the crate compiles with no features enabled by keeping trait definitions free of SDK-specific dependencies
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/**/*.rs : Validate account type with 8-byte discriminators using check_discriminator before deserialization
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
📚 Learning: 2025-10-16T06:33:55.362Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/compressed-token/program/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:55.362Z
Learning: Applies to programs/compressed-token/program/src/create_token_account.rs : Create Token Account instruction must validate that the config state is ACTIVE only
Applied to files:
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs
🧬 Code graph analysis (1)
sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs (2)
program-libs/compressed-account/src/pubkey.rs (1)
new_unique(200-202)sdk-libs/compressed-token-sdk/src/instructions/create_associated_token_account.rs (1)
derive_ctoken_ata(178-187)
Comment on lines
+86
to
113
| fn test_ata_address_derivation() { | ||
| let owner = Pubkey::new_unique(); | ||
| let mint = Pubkey::new_unique(); | ||
| let (ata_pubkey, bump) = derive_ctoken_ata(&owner, &mint); | ||
|
|
||
| // Test with_bump variant (non-idempotent by default) | ||
| let ix_with_bump = | ||
| create_associated_token_account_with_bump(payer, owner, mint, ata_pubkey, bump).unwrap(); | ||
| assert_eq!(ix_with_bump.data[0], CREATE_ATA_DISCRIMINATOR); | ||
|
|
||
| // Test with_bump_and_mode variants | ||
| let ix_with_bump_false = create_associated_token_account_with_bump_and_mode::<false>( | ||
| payer, owner, mint, ata_pubkey, bump, | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(ix_with_bump_false.data[0], CREATE_ATA_DISCRIMINATOR); | ||
|
|
||
| let ix_with_bump_true = create_associated_token_account_with_bump_and_mode::<true>( | ||
| payer, owner, mint, ata_pubkey, bump, | ||
| ) | ||
| .unwrap(); | ||
| let (ata_pubkey, _bump) = derive_ctoken_ata(&owner, &mint); | ||
|
|
||
| // Verify that the derived ATA address is consistent | ||
| let (ata_pubkey2, _bump2) = derive_ctoken_ata(&owner, &mint); | ||
| assert_eq!( | ||
| ix_with_bump_true.data[0], | ||
| CREATE_ATA_IDEMPOTENT_DISCRIMINATOR | ||
| ata_pubkey, ata_pubkey2, | ||
| "ATA derivation should be deterministic" | ||
| ); | ||
|
|
||
| // Verify that different owners produce different ATAs | ||
| let owner2 = Pubkey::new_unique(); | ||
| let (ata_pubkey3, _bump3) = derive_ctoken_ata(&owner2, &mint); | ||
| assert_ne!( | ||
| ata_pubkey, ata_pubkey3, | ||
| "Different owners should have different ATAs" | ||
| ); | ||
|
|
||
| // Verify that different mints produce different ATAs | ||
| let mint2 = Pubkey::new_unique(); | ||
| let (ata_pubkey4, _bump4) = derive_ctoken_ata(&owner, &mint2); | ||
| assert_ne!( | ||
| ata_pubkey, ata_pubkey4, | ||
| "Different mints should have different ATAs" | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Test refactoring looks good; verify if derive_ctoken_ata should still return the bump.
The test logic correctly verifies deterministic ATA derivation and uniqueness across different owners and mints. The rename from test_with_bump_functions to test_ata_address_derivation accurately reflects the new focus.
However, derive_ctoken_ata still returns (Pubkey, u8) but the bump is now completely unused across all tests (and presumably throughout the codebase per the PR objective). Consider whether the function signature should be simplified to return only Pubkey since the bump is no longer propagated to instruction data or used elsewhere.
Run the following script to check if the bump returned by derive_ctoken_ata is used anywhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Search for usages of derive_ctoken_ata where the bump (second tuple element) is actually used.
# Search for patterns where derive_ctoken_ata's bump is captured and used (not ignored with _)
rg -nP --type=rust -C3 'derive_ctoken_ata\([^)]+\)' | rg -v '_bump'
# Also search for destructuring patterns that capture the bump
rg -nP --type=rust -C3 'let \([^,]+,\s*(?!_)[a-z_][a-z0-9_]*\)\s*=\s*derive_ctoken_ata'Length of output: 6573
Refactor derive_ctoken_ata to remove the unused bump return value.
The bump returned by derive_ctoken_ata is dead code—it's discarded at every call site throughout the codebase. I verified all usages:
sdk-libs/token-client/src/actions/mint_action.rs:109– bump ignored- All test call sites – bump ignored
Required changes:
- Update
derive_ctoken_atasignature to return onlyPubkey(not(Pubkey, u8)) - Update call site in
sdk-libs/token-client/src/actions/mint_action.rs:109 - Update all call sites in the test file (lines 89, 92, 100, 108)
The test logic itself is solid—it correctly validates deterministic derivation and address uniqueness—but clean up the signature to eliminate the unused return value.
🤖 Prompt for AI Agents
In sdk-libs/compressed-token-sdk/tests/create_associated_token_account.rs around
lines 86 to 113, the test (and other call sites) call derive_ctoken_ata and
ignore the returned bump; change derive_ctoken_ata to return only Pubkey instead
of (Pubkey, u8), then update all call sites to accept a single Pubkey return
(remove unpacking of the bump). Specifically update
sdk-libs/token-client/src/actions/mint_action.rs at line 109 and this test file
at the call sites on lines ~89, ~92, ~100, and ~108 to stop destructuring the
(pubkey, bump) tuple and just assign the returned Pubkey; no behavior changes
beyond removing the unused bump.
SwenSchaeferjohann
approved these changes
Nov 13, 2025
Contributor
Author
|
too CU intensive |
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
2 participants
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.
Issue:
Changes:
Summary by CodeRabbit
Release Notes
Refactor
Tests