-
Notifications
You must be signed in to change notification settings - Fork 86
chore: program trait #2178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
chore: program trait #2178
Conversation
📝 WalkthroughWalkthroughThis PR introduces a trait-based compressible-program SDK surface, unifies account interfaces (renames and consolidations to Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AccountInterfaceExt
participant Indexer
participant RPC
participant LoadAccounts
Client->>AccountInterfaceExt: get_multiple_account_interfaces(accounts: &[AccountToFetch])
activate AccountInterfaceExt
AccountInterfaceExt->>RPC: try fetch on-chain account data
RPC-->>AccountInterfaceExt: account data or None
alt on-chain found
AccountInterfaceExt->>AccountInterfaceExt: build AccountInterface::hot
else not found
AccountInterfaceExt->>Indexer: fetch compressed account + proofs by address
Indexer-->>AccountInterfaceExt: CompressedAccount / CompressedTokenAccount
AccountInterfaceExt->>AccountInterfaceExt: build AccountInterface::cold (ColdContext)
end
AccountInterfaceExt-->>Client: Vec<AccountInterface>
deactivate AccountInterfaceExt
Client->>LoadAccounts: create_load_instructions(specs: &[AccountSpec<V>], ...)
activate LoadAccounts
LoadAccounts->>LoadAccounts: group specs -> AllSpecs
par PDA path
LoadAccounts->>Indexer: fetch PDA proofs
Indexer-->>LoadAccounts: proofs
LoadAccounts->>LoadAccounts: create_decompress_from_pda_specs
and ATA path
LoadAccounts->>Indexer: fetch ATA proofs (batched)
Indexer-->>LoadAccounts: proofs
LoadAccounts->>LoadAccounts: create_decompress_from_ata_interfaces
and Mint path
LoadAccounts->>Indexer: fetch mint proofs
Indexer-->>LoadAccounts: proofs
LoadAccounts->>LoadAccounts: create_decompress_from_mint_interface
end
LoadAccounts-->>Client: Vec<Instruction>
deactivate LoadAccounts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/account_interface_ext.rs`:
- Around line 240-268: The get_multiple_account_interfaces function is awaiting
each account fetch sequentially causing N×RTT; change it to gather all
per-account futures and run them in parallel (e.g., build a Vec of futures
calling
get_account_info_interface/get_token_account_interface/get_mint_interface and
use futures::future::try_join_all or a bounded concurrency stream like
FuturesUnordered/StreamExt::buffered) so all RPC calls execute concurrently,
then collect the KeyedAccountInterface results; also map or attach per-account
context to errors (e.g., include AccountToFetch details in map_err) so failures
indicate which account failed, while preserving the existing return type
Result<Vec<KeyedAccountInterface>, RpcError>.
In `@sdk-libs/compressible-client/src/compressible_program.rs`:
- Around line 154-181: In from_mint_interface replace the re-serialization
(mint_data.try_to_vec().unwrap_or_default()) with the raw compressed bytes from
the compressed account so we preserve the original compressed payload and avoid
silently producing empty data; specifically, when matching
crate::MintState::Cold use the bytes field on the compressed value (e.g.,
compressed.data or compressed.bytes as defined in your Compressed type) to set
the struct's data, and only fall back to serializing mint_data or return an
explicit error if the raw compressed bytes are absent — update the
ColdContext::Mint creation accordingly and remove the unwrap_or_default usage.
- Around line 83-89: AccountToFetch::pubkey() currently returns the Mint
variant's signer key which mismatches the keyed account used in responses
(cmint); update the Mint enum variant to store the derived cmint (e.g., change
Mint { signer: Pubkey } to Mint { signer: Pubkey, cmint: Pubkey }) and modify
AccountToFetch::pubkey() to return the stored cmint for Mint, plus update
wherever the Mint variant is constructed (notably where get_mint_interface
derives cmint) to populate that new cmint field so request/response keys and
de-duplication use the same pubkey.
In `@sdk-libs/compressible-client/src/light-program-interface-sdk.md`:
- Around line 13-59: Several fenced code blocks in
light-program-interface-sdk.md (for example the large "CLIENT FLOW" block and
other blocks throughout the document) are missing language identifiers; update
every fenced code block to include an appropriate language tag (e.g., text,
rust, json) so the markdown linter MD040 stops flagging them and code blocks
render with proper syntax highlighting—scan the file for all triple-backtick
sections (including the ones mentioned in the review) and add the correct
language specifier to each fence.
- Around line 526-551: The markdown has fenced code blocks (e.g., the Rust block
shown and other blocks around lines 680-683) that are not surrounded by blank
lines, causing markdownlint MD031; edit light-program-interface-sdk.md and add a
blank line immediately before and after each triple-backtick fenced block
(including the blocks for the functions from_keyed_accounts,
get_accounts_to_update, update, and get_specs_for_operation) so each fenced code
section is separated by an empty line from surrounding text.
- Around line 34-35: The doc example uses the wrong function name; replace the
example call to build_load_instructions_from_specs with the actual exported
function create_load_instructions_from_specs (or alternatively export
build_load_instructions_from_specs if you prefer the old name) so the docs match
the SDK API; update the example invocation and any parameter names in
light-program-interface-sdk.md to match create_load_instructions_from_specs's
signature and ensure the referenced symbols (build_load_instructions_from_specs,
create_load_instructions_from_specs) are consistent.
- Around line 441-446: The MintSpec section in light-program-interface-sdk.md
uses repeated sentence openings that hurt flow; update the prose under the
`MintSpec` heading to rephrase one or two sentences so they don’t start
identically (e.g., vary the subject or use a transitional phrase), ensuring the
meaning stays the same and the Rust code block remains unchanged.
In `@sdk-libs/compressible-client/src/load_accounts.rs`:
- Around line 549-625: create_load_instructions_from_specs currently skips
AtaSpec entries so cold ATAs never get decompressed; add handling analogous to
mints/program-owned: collect cold ATAs from specs.cold_atas() (or similar), map
their load_context to hashes and check for
MissingPdaDecompressionContext/MissingMintHash errors, fetch their proofs
alongside program/mint proofs (use fetch_proof_if_needed or introduce a
fetch_ata_proofs helper), then call a new helper
create_decompress_ata_instructions_from_specs that consumes each
AtaSpec.load_context and its proof (mirroring create_decompress_ata_instructions
semantics) to produce Instruction(s), and push those into out before returning.
- Around line 663-673: The closure building compressed_accounts currently uses
expect("Cold spec must have context") which panics; change it to propagate a
LoadAccountsError::MissingPdaDecompressionContext instead by converting the map
into a fallible iterator (e.g., map -> map(|s| {
s.cold_context.as_ref().ok_or(LoadAccountsError::MissingPdaDecompressionContext).map(|ctx|
(ctx.compressed_account.clone(), s.variant.clone())) }) and then
collect::<Result<Vec<(CompressedAccount, V)>, LoadAccountsError>>() so the
function returns the error to callers; update any surrounding function signature
(in load_accounts.rs where compressed_accounts is built) to return Result and
wire the propagated error through.
| async fn get_multiple_account_interfaces( | ||
| &self, | ||
| accounts: &[AccountToFetch], | ||
| ) -> Result<Vec<KeyedAccountInterface>, RpcError> { | ||
| let mut result = Vec::with_capacity(accounts.len()); | ||
|
|
||
| for account in accounts { | ||
| let keyed = match account { | ||
| AccountToFetch::Pda { | ||
| address, | ||
| program_id, | ||
| } => { | ||
| let iface = self.get_account_info_interface(address, program_id).await?; | ||
| KeyedAccountInterface::from_pda_interface(iface) | ||
| } | ||
| AccountToFetch::Token { address } => { | ||
| let iface = self.get_token_account_interface(address).await?; | ||
| KeyedAccountInterface::from_token_interface(iface) | ||
| } | ||
| AccountToFetch::Mint { signer } => { | ||
| let iface = self.get_mint_interface(signer).await?; | ||
| KeyedAccountInterface::from_mint_interface(iface) | ||
| } | ||
| }; | ||
| result.push(keyed); | ||
| } | ||
|
|
||
| Ok(result) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Parallelize batched fetches to avoid N×RTT latency.
The current loop awaits each fetch sequentially, which negates the benefit of a batch API for larger account sets. Consider try_join_all (or bounded concurrency) and optionally add account context to errors for easier debugging.
♻️ Suggested refactor (parallel fetch)
- let mut result = Vec::with_capacity(accounts.len());
-
- for account in accounts {
- let keyed = match account {
- AccountToFetch::Pda { address, program_id } => {
- let iface = self.get_account_info_interface(address, program_id).await?;
- KeyedAccountInterface::from_pda_interface(iface)
- }
- AccountToFetch::Token { address } => {
- let iface = self.get_token_account_interface(address).await?;
- KeyedAccountInterface::from_token_interface(iface)
- }
- AccountToFetch::Mint { signer } => {
- let iface = self.get_mint_interface(signer).await?;
- KeyedAccountInterface::from_mint_interface(iface)
- }
- };
- result.push(keyed);
- }
-
- Ok(result)
+ let futures = accounts.iter().map(|account| async {
+ let keyed = match account {
+ AccountToFetch::Pda { address, program_id } => {
+ let iface = self.get_account_info_interface(address, program_id).await?;
+ KeyedAccountInterface::from_pda_interface(iface)
+ }
+ AccountToFetch::Token { address } => {
+ let iface = self.get_token_account_interface(address).await?;
+ KeyedAccountInterface::from_token_interface(iface)
+ }
+ AccountToFetch::Mint { signer } => {
+ let iface = self.get_mint_interface(signer).await?;
+ KeyedAccountInterface::from_mint_interface(iface)
+ }
+ };
+ Ok::<_, RpcError>(keyed)
+ });
+
+ futures::future::try_join_all(futures).await🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/account_interface_ext.rs` around lines 240 -
268, The get_multiple_account_interfaces function is awaiting each account fetch
sequentially causing N×RTT; change it to gather all per-account futures and run
them in parallel (e.g., build a Vec of futures calling
get_account_info_interface/get_token_account_interface/get_mint_interface and
use futures::future::try_join_all or a bounded concurrency stream like
FuturesUnordered/StreamExt::buffered) so all RPC calls execute concurrently,
then collect the KeyedAccountInterface results; also map or attach per-account
context to errors (e.g., include AccountToFetch details in map_err) so failures
indicate which account failed, while preserving the existing return type
Result<Vec<KeyedAccountInterface>, RpcError>.
| ``` | ||
| CLIENT FLOW | ||
| ================================================ | ||
| [1] Fetch root account (e.g., PoolState) | ||
| | | ||
| v | ||
| [2] AmmSdk::from_keyed_accounts([pool]) | ||
| | | ||
| +-- parses PoolState | ||
| +-- extracts pubkeys (vaults, mints, etc.) | ||
| +-- derives PDAs (authority, mint_signer) | ||
| +-- builds initial spec for pool_state | ||
| | | ||
| v | ||
| [3] sdk.get_accounts_to_update(&Operation) | ||
| | | ||
| +-- returns pubkeys + types needed | ||
| | | ||
| v | ||
| [4] Client fetches accounts by type: | ||
| +-- PDAs: get_account_info_interface() | ||
| +-- Tokens: get_token_account_interface() | ||
| +-- Mints: get_mint_interface() | ||
| | | ||
| v | ||
| [5] sdk.update(&keyed_accounts) | ||
| | | ||
| +-- parses each account | ||
| +-- builds variant with seed values | ||
| +-- caches spec (hot or cold) | ||
| | | ||
| v | ||
| [6] sdk.get_specs_for_operation(&Operation) | ||
| | | ||
| +-- returns AllSpecs with program_owned, atas, mints | ||
| | | ||
| v | ||
| [7] create_load_instructions_from_specs(&specs, ...) | ||
| | | ||
| +-- filters cold accounts | ||
| +-- fetches proofs | ||
| +-- builds decompress instructions | ||
| | | ||
| v | ||
| [8] Execute transactions | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifiers to fenced code blocks (markdownlint MD040).
Several fenced blocks omit a language tag. Please add appropriate identifiers (text, rust, etc.) to satisfy linting and improve readability.
Also applies to: 65-225, 229-272, 276-305, 309-371, 665-699, 702-735
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/light-program-interface-sdk.md` around lines
13 - 59, Several fenced code blocks in light-program-interface-sdk.md (for
example the large "CLIENT FLOW" block and other blocks throughout the document)
are missing language identifiers; update every fenced code block to include an
appropriate language tag (e.g., text, rust, json) so the markdown linter MD040
stops flagging them and code blocks render with proper syntax highlighting—scan
the file for all triple-backtick sections (including the ones mentioned in the
review) and add the correct language specifier to each fence.
sdk-libs/compressible-client/src/light-program-interface-sdk.md
Outdated
Show resolved
Hide resolved
|
|
||
| ### `MintSpec` | ||
|
|
||
| Spec for Light Mints. | ||
|
|
||
| ```rust |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style tweak: repeated sentence starts.
LanguageTool flagged repeated sentence openings around Line 443; consider a small rephrase for smoother flow.
🧰 Tools
🪛 LanguageTool
[style] ~443-~443: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...kenLoadContext>, } ### `MintSpec` Spec for Light Mints. rust pub struct Mi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/light-program-interface-sdk.md` around lines
441 - 446, The MintSpec section in light-program-interface-sdk.md uses repeated
sentence openings that hurt flow; update the prose under the `MintSpec` heading
to rephrase one or two sentences so they don’t start identically (e.g., vary the
subject or use a transitional phrase), ensuring the meaning stays the same and
the Rust code block remains unchanged.
| ```rust | ||
| fn from_keyed_accounts(accounts: &[KeyedAccountInterface]) -> Result<Self, Self::Error> { | ||
| // Parse PoolState discriminator | ||
| // Deserialize PoolState | ||
| // Extract: amm_config, token_0_vault, token_1_vault, lp_mint, etc. | ||
| // Derive: authority PDA, lp_mint_signer PDA | ||
| // Build initial PoolState spec | ||
| } | ||
| ``` | ||
|
|
||
| 2. **`get_accounts_to_update`**: Return pubkeys based on operation: | ||
| ```rust | ||
| AmmOperation::Swap => [token_0_vault, token_1_vault] | ||
| AmmOperation::Deposit => [token_0_vault, token_1_vault, observation, lp_mint] | ||
| ``` | ||
|
|
||
| 3. **`update`**: Parse accounts by discriminator or known pubkey: | ||
| ```rust | ||
| // Check if pubkey matches known vaults -> parse as token | ||
| // Check discriminator -> parse as PoolState/ObservationState | ||
| // Build variant with seed values from SDK cache | ||
| ``` | ||
|
|
||
| 4. **`get_specs_for_operation`**: Filter cached specs: | ||
| ```rust | ||
| AmmOperation::Swap => [pool_state, token_0_vault, token_1_vault] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add blank lines around fenced code blocks (markdownlint MD031).
A few fenced blocks are missing surrounding blank lines; this triggers markdownlint.
Also applies to: 680-683
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
526-526: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
537-537: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
543-543: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
550-550: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/light-program-interface-sdk.md` around lines
526 - 551, The markdown has fenced code blocks (e.g., the Rust block shown and
other blocks around lines 680-683) that are not surrounded by blank lines,
causing markdownlint MD031; edit light-program-interface-sdk.md and add a blank
line immediately before and after each triple-backtick fenced block (including
the blocks for the functions from_keyed_accounts, get_accounts_to_update,
update, and get_specs_for_operation) so each fenced code section is separated by
an empty line from surrounding text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/compressible_program.rs`:
- Around line 14-35: The example docs use outdated signatures — update the
sample to call get_account_info_interface with the program_id argument and
replace build_load_instructions_from_specs with the new create_load_instructions
API; specifically modify the sequence that builds the RPC client call
(get_account_info_interface) to pass program_id, and change the final step to
call create_load_instructions(&specs, /* appropriate args */) instead of
build_load_instructions_from_specs; keep the rest of the flow
(AmmSdk::from_keyed_accounts, sdk.get_accounts_to_update,
fetch_keyed_interfaces, sdk.update, sdk.get_specs_for_operation) the same but
ensure the updated function names and argument lists match the current public
API.
♻️ Duplicate comments (7)
sdk-libs/compressible-client/src/light-program-interface-sdk.md (3)
13-59: Add language identifiers to fenced code blocks (MD040).
markdownlint still flags these fences; please add language tags (text,rust,toml, etc.) to all of them.Also applies to: 65-225, 229-272, 276-305, 309-371, 665-699, 702-735
526-551: Surround fenced blocks with blank lines (MD031).
Add a blank line before and after each fenced block in these ranges to satisfy markdownlint.Also applies to: 680-683
51-52: Docs reference non-publiccreate_load_instructions_from_specs.
The public API exposescreate_load_instructions; referencing the private helper will not compile for users. Update the diagrams/examples (or make the helper public) and convertAllSpecsviainto_account_specs()where needed.📝 Suggested doc fix (example update)
-use light_compressible_client::{ - AccountInterfaceExt, CompressibleProgram, KeyedAccountInterface, - create_load_instructions_from_specs -}; +use light_compressible_client::{ + AccountInterfaceExt, CompressibleProgram, KeyedAccountInterface, + create_load_instructions +}; - let ixs = create_load_instructions_from_specs( - &specs, + let ixs = create_load_instructions( + &specs.into_account_specs(), program_id, fee_payer, compression_config, rent_sponsor, &rpc, ).await?;Also applies to: 559-564, 589-596, 766-768
sdk-libs/compressible-client/src/account_interface_ext.rs (1)
236-264: Avoid N×RTT sequential fetches inget_multiple_account_interfaces.
Awaiting inside the loop serializes all RPCs; use concurrent dispatch (bounded ortry_join_all) to keep batch fetches performant.♻️ Suggested refactor (parallel fetch)
- let mut result = Vec::with_capacity(accounts.len()); - - for account in accounts { - let keyed = match account { - AccountToFetch::Pda { address, program_id } => { - let iface = self.get_account_info_interface(address, program_id).await?; - KeyedAccountInterface::from_pda_interface(iface) - } - AccountToFetch::Token { address } => { - let iface = self.get_token_account_interface(address).await?; - KeyedAccountInterface::from_token_interface(iface) - } - AccountToFetch::Mint { signer } => { - let iface = self.get_mint_interface(signer).await?; - KeyedAccountInterface::from_mint_interface(iface) - } - }; - result.push(keyed); - } - - Ok(result) + let futures = accounts.iter().map(|account| async { + let keyed = match account { + AccountToFetch::Pda { address, program_id } => { + let iface = self.get_account_info_interface(address, program_id).await?; + KeyedAccountInterface::from_pda_interface(iface) + } + AccountToFetch::Token { address } => { + let iface = self.get_token_account_interface(address).await?; + KeyedAccountInterface::from_token_interface(iface) + } + AccountToFetch::Mint { signer } => { + let iface = self.get_mint_interface(signer).await?; + KeyedAccountInterface::from_mint_interface(iface) + } + }; + Ok::<_, RpcError>(keyed) + }); + + futures::future::try_join_all(futures).awaitsdk-libs/compressible-client/src/load_accounts.rs (1)
491-501: Avoid panic when cold context is missing.
expect("Cold spec must have context")will crash the client on malformed specs; returnLoadAccountsError::MissingPdaDecompressionContextinstead.🛠️ Suggested fix (propagate error)
- let compressed_accounts: Vec<(CompressedAccount, V)> = specs - .iter() - .map(|s| { - let compressed_account = s - .cold_context - .as_ref() - .expect("Cold spec must have context") - .compressed_account - .clone(); - (compressed_account, s.variant.clone()) - }) - .collect(); + let compressed_accounts: Vec<(CompressedAccount, V)> = specs + .iter() + .enumerate() + .map(|(i, s)| { + let compressed_account = s + .cold_context + .as_ref() + .ok_or(LoadAccountsError::MissingPdaDecompressionContext { + index: i, + pubkey: s.address, + })? + .compressed_account + .clone(); + Ok::<_, LoadAccountsError>((compressed_account, s.variant.clone())) + }) + .collect::<Result<Vec<_>, _>>()?;sdk-libs/compressible-client/src/compressible_program.rs (2)
52-87: Mint pubkey should becmint, notsigner.
get_mint_interfacereturnsKeyedAccountInterfacekeyed bycmint, so returningsignerfromAccountToFetch::pubkey()breaks dedupe/mapping. Storecmintin theMintvariant and return it; update match sites accordingly (e.g.,AccountInterfaceExt::get_multiple_account_interfaces).🐛 Suggested fix (store derived cmint)
pub enum AccountToFetch { /// PDA account - uses `get_account_info_interface(address, program_id)` Pda { address: Pubkey, program_id: Pubkey }, /// Token account (program-owned or ATA) - uses `get_token_account_interface(address)` /// The address is the owner of the compressed token. Token { address: Pubkey }, /// Light mint - uses `get_mint_interface(signer)` - Mint { signer: Pubkey }, + Mint { signer: Pubkey, cmint: Pubkey }, } @@ /// Create a mint fetch descriptor. pub fn mint(signer: Pubkey) -> Self { - Self::Mint { signer } + let (cmint, _) = light_token_sdk::token::find_mint_address(&signer); + Self::Mint { signer, cmint } } @@ pub fn pubkey(&self) -> Pubkey { match self { Self::Pda { address, .. } => *address, Self::Token { address } => *address, - Self::Mint { signer } => *signer, + Self::Mint { cmint, .. } => *cmint, } }
161-173: Don’t silently drop mint bytes on serialization failure.
try_to_vec().unwrap_or_default()can mask serialization errors and diverge from the compressed payload. Prefer the raw compressed bytes when available, and fail loudly if serialization is required.♻️ Suggested refactor
- // Serialize mint data for the data field - use borsh::BorshSerialize; - let data = mint_data.try_to_vec().unwrap_or_default(); + // Prefer raw compressed bytes to preserve fidelity + let data = compressed + .data + .as_ref() + .map(|d| { + let mut buf = d.discriminator.to_vec(); + buf.extend_from_slice(&d.data); + buf + }) + .unwrap_or_else(|| { + use borsh::BorshSerialize; + mint_data + .try_to_vec() + .expect("mint_data serialization failed") + });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/compressible_program.rs`:
- Around line 18-36: The doc example uses outdated API names/signatures: update
the sample to call get_account_info_interface(&pool_pubkey, &program_id)
(include the program_id argument) and rename build_load_instructions_from_specs
to create_load_instructions_from_specs; keep the rest of the flow
(KeyedAccountInterface::from_pda_interface, AmmSdk::from_keyed_accounts,
sdk.get_accounts_to_update(&AmmOperation::Deposit), fetch_keyed_interfaces,
sdk.update, sdk.get_specs_for_operation(&AmmOperation::Deposit), and then call
create_load_instructions_from_specs(&specs, ...)) so the example matches the
current public API.
♻️ Duplicate comments (7)
sdk-libs/compressible-client/src/account_interface_ext.rs (1)
236-264: Parallelize batched fetches to avoid N×RTT latency.The loop awaits each RPC call serially, which negates the benefit of a batch interface. Consider
try_join_allor bounded concurrency and attach account context to errors for easier debugging.♻️ Suggested refactor (parallel fetch)
- let mut result = Vec::with_capacity(accounts.len()); - - for account in accounts { - let keyed = match account { - AccountToFetch::Pda { - address, - program_id, - } => { - let iface = self.get_account_info_interface(address, program_id).await?; - KeyedAccountInterface::from_pda_interface(iface) - } - AccountToFetch::Token { address } => { - let iface = self.get_token_account_interface(address).await?; - KeyedAccountInterface::from_token_interface(iface) - } - AccountToFetch::Mint { signer } => { - let iface = self.get_mint_interface(signer).await?; - KeyedAccountInterface::from_mint_interface(iface) - } - }; - result.push(keyed); - } - - Ok(result) + let futures = accounts.iter().map(|account| async { + let keyed = match account { + AccountToFetch::Pda { + address, + program_id, + } => { + let iface = self.get_account_info_interface(address, program_id).await?; + KeyedAccountInterface::from_pda_interface(iface) + } + AccountToFetch::Token { address } => { + let iface = self.get_token_account_interface(address).await?; + KeyedAccountInterface::from_token_interface(iface) + } + AccountToFetch::Mint { signer } => { + let iface = self.get_mint_interface(signer).await?; + KeyedAccountInterface::from_mint_interface(iface) + } + }; + Ok::<_, RpcError>(keyed) + }); + + futures::future::try_join_all(futures).awaitsdk-libs/compressible-client/src/load_accounts.rs (1)
491-500: Avoid panic when a cold spec lacks context.
expect("Cold spec must have context")will crash the client if the spec is malformed. ReturnLoadAccountsError::MissingPdaDecompressionContextso callers can handle it gracefully.🛠️ Suggested fix (propagate error)
- let compressed_accounts: Vec<(CompressedAccount, V)> = specs - .iter() - .map(|s| { - let compressed_account = s - .cold_context - .as_ref() - .expect("Cold spec must have context") - .compressed_account - .clone(); - (compressed_account, s.variant.clone()) - }) - .collect(); + let compressed_accounts: Vec<(CompressedAccount, V)> = specs + .iter() + .enumerate() + .map(|(i, s)| { + let compressed_account = s + .cold_context + .as_ref() + .ok_or(LoadAccountsError::MissingPdaDecompressionContext { + index: i, + pubkey: s.address, + })? + .compressed_account + .clone(); + Ok::<_, LoadAccountsError>((compressed_account, s.variant.clone())) + }) + .collect::<Result<Vec<_>, _>>()?;sdk-libs/compressible-client/src/light-program-interface-sdk.md (3)
13-59: Add language identifiers to fenced code blocks (MD040).Several fenced blocks omit a language tag, which breaks markdownlint. Please add appropriate identifiers (e.g.,
text,rust,toml).Also applies to: 65-225, 229-272, 276-305, 309-371, 665-699, 687-699, 702-735
526-551: Add blank lines around fenced code blocks (MD031).A few fences are missing surrounding blank lines, which triggers markdownlint. Add a blank line before and after each fenced block in these sections.
Also applies to: 680-683
441-446: Vary repeated sentence openings for smoother flow.LanguageTool flagged repeated sentence starts in the
MintSpecsection; a small rephrase will improve readability.sdk-libs/compressible-client/src/compressible_program.rs (2)
52-88: Mint fetch descriptors return the wrong pubkey.
AccountToFetch::pubkey()returns the signer forMint, butget_mint_interfacereturns a keyed account for the derivedcmint. This breaks request→response correlation and de-duplication. Storecmintin theMintvariant and return that instead (also update match arms whereMintis destructured).🛠️ Suggested fix (store derived cmint)
pub enum AccountToFetch { /// PDA account - uses `get_account_info_interface(address, program_id)` Pda { address: Pubkey, program_id: Pubkey }, /// Token account (program-owned or ATA) - uses `get_token_account_interface(address)` /// The address is the owner of the compressed token. Token { address: Pubkey }, /// Light mint - uses `get_mint_interface(signer)` - Mint { signer: Pubkey }, + Mint { signer: Pubkey, cmint: Pubkey }, } @@ /// Create a mint fetch descriptor. pub fn mint(signer: Pubkey) -> Self { - Self::Mint { signer } + let (cmint, _) = light_token_sdk::token::find_mint_address(&signer); + Self::Mint { signer, cmint } } @@ pub fn pubkey(&self) -> Pubkey { match self { Self::Pda { address, .. } => *address, Self::Token { address } => *address, - Self::Mint { signer } => *signer, + Self::Mint { cmint, .. } => *cmint, } }
157-174: Preserve compressed mint bytes instead of re-serializing.
try_to_vec().unwrap_or_default()can silently return empty data and may diverge from the original compressed payload. Prefer the raw compressed bytes when available.🛠️ Suggested fix (use compressed bytes)
- // Serialize mint data for the data field - use borsh::BorshSerialize; - let data = mint_data.try_to_vec().unwrap_or_default(); + let data = compressed + .data + .as_ref() + .map(|d| { + let mut buf = d.discriminator.to_vec(); + buf.extend_from_slice(&d.data); + buf + }) + .unwrap_or_default();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/compressible-client/src/decompress_atas.rs (1)
559-643: Tests reference removed types and functions - compilation will fail.The test module uses
AtaInterfaceandbuild_decompress_atas, which don't exist in this file after the refactor. The current public API exposesAtaAccountInterface,TokenAccountInterface,build_decompress_token_accounts, and related functions.Either update these tests to use the new API or remove them if they're superseded by tests elsewhere.
🐛 Proposed fix outline
#[cfg(test)] mod tests { use super::*; #[test] fn test_derive_ata() { // This test is fine - uses derive_token_ata which exists ... } - #[test] - fn test_ata_interface_is_cold() { - // Uses AtaInterface which doesn't exist - let hot_ata = AtaInterface { ... }; - ... - } + #[test] + fn test_token_account_interface_is_cold() { + // Use TokenAccountInterface instead + let parsed = parse_token_account_interface(&ata_account_interface)?; + assert!(!parsed.is_cold); + ... + } - #[test] - fn test_build_decompress_atas_fast_exit() { - // Uses AtaInterface and build_decompress_atas which don't exist - ... - } + #[test] + fn test_build_decompress_token_accounts_fast_exit() { + // Use TokenAccountInterface and build_decompress_token_accounts + ... + } }sdk-libs/compressible-client/src/compressible_program.rs (1)
422-431: Potential panic:expect("Cold spec must have compressed data")in cold path.While the function is only called with cold specs (filtered by caller), a malformed spec could still panic here. Consider returning a
LoadAccountsErrorinstead for defensive programming.This was flagged in a past review for a similar pattern and was marked as addressed, but this instance in
create_decompress_from_pda_specsstill usesexpect.🛠️ Suggested fix
let compressed_accounts: Vec<(CompressedAccount, V)> = specs .iter() + .enumerate() - .map(|s| { + .map(|(i, s)| { let compressed_account = s .compressed() - .expect("Cold spec must have compressed data") + .ok_or(LoadAccountsError::MissingPdaCompressed { + index: i, + pubkey: s.address(), + })? .clone(); - (compressed_account, s.variant.clone()) + Ok((compressed_account, s.variant.clone())) }) - .collect(); + .collect::<Result<Vec<_>, _>>()?;
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/account_interface_ext.rs`:
- Around line 138-146: The current fallback creates and returns an empty hot
account (AccountInterface::hot) when the PDA doesn't exist; instead, change this
to return an explicit not-found error (e.g., Err(AccountNotFound) using the
module's account error type) from the surrounding function in
AccountInterfaceExt so callers cannot silently treat missing accounts as
present; if you need to preserve the fallback behavior for compatibility, make
it opt-in via a boolean parameter (e.g., fallback_to_empty) or a separate method
and document that callers should check account.data.is_empty() when using the
fallback.
In `@sdk-libs/compressible-client/src/compressible_program.rs`:
- Around line 257-267: The current From<TokenAccountInterface> and
From<AccountInterface> impls are fixed to AccountSpec<()> which prevents type
inference for other generic variants; change them to generic impls like impl<V>
From<TokenAccountInterface> for AccountSpec<V> and impl<V>
From<AccountInterface> for AccountSpec<V> so conversions produce
AccountSpec::Ata(TokenAccountInterface) and AccountSpec::Mint(AccountInterface)
for any V, allowing code like let spec: AccountSpec<MyVariant> =
token_interface.into() to work without explicit construction.
♻️ Duplicate comments (2)
sdk-libs/compressible-client/src/account_interface_ext.rs (1)
222-275: Sequential fetches acknowledged via TODO - consider prioritizing parallelization.The TODO on line 226 acknowledges the N×RTT latency issue from the previous review. For larger account sets, this will be a performance bottleneck.
The implementation is correct but would benefit from
futures::try_join_allwhen you have time to address it. For now, this is acceptable for small account sets.sdk-libs/compressible-client/src/load_accounts.rs (1)
422-431: Same issue:expect()in cold path can panic.This is the same pattern I flagged in
compressible_program.rs. Theexpect("Cold spec must have compressed data")could panic if a malformed spec reaches this code. ReturnLoadAccountsError::MissingPdaCompressedinstead.
| // Doesn't exist - return empty hot account | ||
| let account = solana_account::Account { | ||
| lamports: 0, | ||
| data: vec![], | ||
| owner: *program_id, | ||
| executable: false, | ||
| rent_epoch: 0, | ||
| }; | ||
| Ok(AccountInfoInterface::hot(*address, account)) | ||
| Ok(AccountInterface::hot(*address, account)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Intentional fallback: returns empty hot account when PDA doesn't exist.
This is a reasonable default - callers can check account.data.is_empty() to detect non-existent accounts. However, consider whether returning an error (AccountNotFound) might be safer to prevent silent failures.
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/account_interface_ext.rs` around lines 138 -
146, The current fallback creates and returns an empty hot account
(AccountInterface::hot) when the PDA doesn't exist; instead, change this to
return an explicit not-found error (e.g., Err(AccountNotFound) using the
module's account error type) from the surrounding function in
AccountInterfaceExt so callers cannot silently treat missing accounts as
present; if you need to preserve the fallback behavior for compatibility, make
it opt-in via a boolean parameter (e.g., fallback_to_empty) or a separate method
and document that callers should check account.data.is_empty() when using the
fallback.
| impl From<TokenAccountInterface> for AccountSpec<()> { | ||
| fn from(interface: TokenAccountInterface) -> Self { | ||
| Self::Ata(interface) | ||
| } | ||
| } | ||
|
|
||
| impl From<AccountInterface> for AccountSpec<()> { | ||
| fn from(interface: AccountInterface) -> Self { | ||
| Self::Mint(interface) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
From implementations use () for non-generic conversions - intentional but limits type inference.
The From<TokenAccountInterface> for AccountSpec<()> and From<AccountInterface> for AccountSpec<()> implementations fix the variant type to (). This means you can't do let spec: AccountSpec<MyVariant> = token_interface.into() - you'd need explicit construction.
This is fine if these are primarily used in type-erased contexts, but worth documenting the limitation.
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/compressible_program.rs` around lines 257 -
267, The current From<TokenAccountInterface> and From<AccountInterface> impls
are fixed to AccountSpec<()> which prevents type inference for other generic
variants; change them to generic impls like impl<V> From<TokenAccountInterface>
for AccountSpec<V> and impl<V> From<AccountInterface> for AccountSpec<V> so
conversions produce AccountSpec::Ata(TokenAccountInterface) and
AccountSpec::Mint(AccountInterface) for any V, allowing code like let spec:
AccountSpec<MyVariant> = token_interface.into() to work without explicit
construction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/compressible-client/src/decompress_mint.rs (1)
315-320: Minor: Duplicate step number in comment.Line 315 and 320 both use step "5." in the numbered comments. This is a minor documentation nit.
📝 Suggested fix
- // 5. Check if already decompressed flag is set - return empty vec (idempotent) + // 6. Check if already decompressed flag is set - return empty vec (idempotent) if mint_data.metadata.mint_decompressed { return Ok(vec![]); } - // 5. Get validity proof + // 7. Get validity proof
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/load_accounts.rs`:
- Around line 63-100: fetch_individual_proofs and fetch_batched_proofs perform
sequential indexer calls; change them to run requests concurrently by mapping
each hash (or chunk) to a future calling indexer.get_validity_proof and
collecting with a concurrency-limited executor (e.g.
futures::stream::iter(...).map(|h| async move { indexer.get_validity_proof(...)
}).buffer_unordered(N).collect::<Result<Vec<_>, _>>()) so you parallelize IO
while bounding concurrency; update both functions to build the futures from
hashes/chunks, use buffer_unordered with a reasonable cap (configurable or
MAX_CONCURRENCY) and then extract result.value from each completed response to
return Vec<ValidityProofWithContext>.
- Around line 422-431: The code currently calls expect("Cold spec must have
compressed data") inside the specs.iter().map closure when building
compressed_accounts, which can panic; change this to a fallible conversion that
returns an error instead of panicking (use ok_or / map_err) and collect into a
Result<Vec<(CompressedAccount, V)>, E> so callers can handle the error; update
the surrounding function signature to return Result and propagate the error (or
convert into the crate's error type) and reference the compressed_accounts
construction and the compressed() call on each spec to locate and replace the
expect-based unwrap with an ok_or-based error propagation.
♻️ Duplicate comments (1)
sdk-libs/compressible-client/src/account_interface_ext.rs (1)
222-275: Sequential fetch acknowledged with TODO for parallelization.The current implementation fetches accounts sequentially (O(n) RTT). The TODO at line 226 notes the intent to parallelize with
futures::join_all. For high-volume usage, consider implementing this optimization.Note: The
MintState::Nonecase at lines 263-267 usesDefault::default()for the account field, which creates an account with zero lamports and empty data. This is consistent with the hot fallback pattern elsewhere.
| let compressed_accounts: Vec<(CompressedAccount, V)> = specs | ||
| .iter() | ||
| .map(|s| { | ||
| let compressed_account = s | ||
| .compressed() | ||
| .expect("Cold spec must have compressed data") | ||
| .clone(); | ||
| (compressed_account, s.variant.clone()) | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expect could panic if invariant is violated.
Line 427 uses expect("Cold spec must have compressed data"). While the upstream filter guarantees only cold specs reach here, a defensive approach would use ok_or to return an error instead of panicking. This is especially important in library code where callers may not control the input.
🛡️ Suggested defensive fix
let compressed_accounts: Vec<(CompressedAccount, V)> = specs
.iter()
- .map(|s| {
+ .enumerate()
+ .map(|(i, s)| {
let compressed_account = s
.compressed()
- .expect("Cold spec must have compressed data")
+ .ok_or(LoadAccountsError::MissingPdaCompressed {
+ index: i,
+ pubkey: s.address(),
+ })?
.clone();
- (compressed_account, s.variant.clone())
+ Ok((compressed_account, s.variant.clone()))
})
- .collect();
+ .collect::<Result<Vec<_>, _>>()?;🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/load_accounts.rs` around lines 422 - 431,
The code currently calls expect("Cold spec must have compressed data") inside
the specs.iter().map closure when building compressed_accounts, which can panic;
change this to a fallible conversion that returns an error instead of panicking
(use ok_or / map_err) and collect into a Result<Vec<(CompressedAccount, V)>, E>
so callers can handle the error; update the surrounding function signature to
return Result and propagate the error (or convert into the crate's error type)
and reference the compressed_accounts construction and the compressed() call on
each spec to locate and replace the expect-based unwrap with an ok_or-based
error propagation.
79de9b0 to
2cb9b13
Compare
add csdk-anchor-full-derived-test-sdk format rm md, lint cleanup fmt clean, fmt, lint fmt rm deadfiles clean more, fmt clean up more clean lint clean test
2cb9b13 to
ef84449
Compare
|
Hot LP Mint Identification Fails Location: Problem: LP mint identification only checks Impact: Deposit and Withdraw operations fail when the LP mint is on-chain (hot/uncompressed). Silent failure prevents proper error reporting. Root Cause: Asymmetry in identification strategies:
Code: // Line 371-375: Only matches cold mints
if let Some(ColdContext::Mint { signer, compressed_address, .. }) = &account.cold_context {
if *signer == lp_mint_signer {
return self.parse_mint(account, *signer, *compressed_address);
}
}
// Line 383-384: Hot mint silently skipped
Ok(()) // SILENT FAILURERecommended Fix: // Add BEFORE line 369:
if Some(account.pubkey) == self.lp_mint {
let signer = self.lp_mint_signer.ok_or(AmmSdkError::MissingField("lp_mint_signer"))?;
// Handle hot mint case or derive compressed_address
return self.parse_mint_hot(account, signer);
} |
|
compression_index Hardcoded to 0 Location:
Problem: The Impact: Transaction fails with Evidence: Test at
Code: // load_accounts.rs:199 - WRONG
compression_index: 0, // Should be: compression_index: i as u8Recommended Fix: let mut compression_index_tracker: HashMap<Pubkey, u8> = HashMap::new();
// In loop:
let ata = derive_token_ata(&ctx.wallet_owner, &ctx.mint).0;
let compression_index = *compression_index_tracker.entry(ata).or_insert(0);
compression_index_tracker.insert(ata, compression_index + 1); |
|
Unsafe Array Indexing Can Panic Location: Problem: Direct array indexing Impact: Client panics on malformed proof data from corrupted/malicious indexer. Code: // load_accounts.rs:150 - UNSAFE
let tree_info = &tree_infos.packed_tree_infos[i]; // Can panic!
// Compare to lib.rs:328 - SAFE
tree_infos.get(i).copied().ok_or_else(|| LoadAccountsError::BuildInstruction(...))?Recommended Fix: let tree_info = tree_infos.packed_tree_infos
.get(i)
.ok_or_else(|| LoadAccountsError::BuildInstruction("Tree info index out of bounds".into()))?; |
|
Silent SDK Initialization Failure Location: Problem: If Impact: Downstream operations fail with confusing errors (e.g., Fix: Add validation at line 488: self.pool_state_pubkey.ok_or(AmmSdkError::PoolStateNotParsed)?;
Ok(sdk) |
|
Other potential issues: Naming Inconsistencies
Redundant Code
Silent Failures
Missing Validation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
sdk-libs/macros/src/light_pdas/accounts/mint.rs (3)
309-310: Add bounds check before indexing into mints array.Direct indexing
mints[0]will panic during macro expansion if the mints slice is empty. This produces poor error messages for users.🛡️ Proposed fix with bounds check
+ // Validate non-empty mints + if mints.is_empty() { + return quote! { + compile_error!("At least one mint must be specified for mint creation"); + }; + } + // Get rent_payment and write_top_up from first mint (all mints share same params for now) let rent_payment = quote_option_or(&mints[0].rent_payment, quote! { 16u8 }); let write_top_up = quote_option_or(&mints[0].write_top_up, quote! { 766u32 });
174-183: Check for overflow when casting pda_count to u8.The cast
*pda_count as u8on line 176 silently truncates ifpda_count > 255, which would produce incorrect CPI context offsets and cause runtime failures.✅ Proposed fix with overflow validation
let (cpi_context_offset, output_tree_setup) = match &builder.pda_context { Some((pda_count, tree_expr)) => { - let offset = *pda_count as u8; + if *pda_count > u8::MAX as usize { + return quote! { + compile_error!("PDA count exceeds maximum CPI context offset (255)"); + }; + } + let offset = *pda_count as u8; ( quote! { `#offset` }, quote! { let __output_tree_index = `#tree_expr`; }, ) } None => (quote! { 0u8 }, quote! {}), };
331-332: Replaceexpectwithok_orfor graceful error handling.Using
.expect()causes the generated code to panic at runtime if the proof is missing. This is inconsistent with the pattern on line 359 which properly returns an error.♻️ Proposed fix for consistent error handling
// Extract proof from instruction params - let __proof: light_token_sdk::CompressedProof = `#params_ident.create_accounts_proof.proof.0.clone`() - .expect("proof is required for mint creation"); + let __proof: light_token_sdk::CompressedProof = `#params_ident.create_accounts_proof.proof.0.clone`() + .ok_or(anchor_lang::prelude::ProgramError::InvalidArgument)?;sdk-libs/token-sdk/src/token/decompress_mint.rs (1)
235-301: Validate derived Mint PDA matches the compressed mint metadata.
Right now the CPI helper derives the Mint PDA frommint_seed_pubkeybut never cross-checks it againstcompressed_mint_with_context. A mismatch will fail later with a generic error; a local validation gives clearer feedback and avoids confusing failures.🔧 Suggested validation
- let (mint_pda, _cmint_bump) = crate::token::find_mint_address(&self.mint_seed_pubkey); + let (mint_pda, _cmint_bump) = crate::token::find_mint_address(&self.mint_seed_pubkey); + let expected_mint = Pubkey::from( + self.compressed_mint_with_context + .mint + .as_ref() + .ok_or(ProgramError::InvalidInstructionData)? + .metadata + .mint + .to_bytes(), + ); + if mint_pda != expected_mint { + return Err(ProgramError::InvalidInstructionData); + }sdk-libs/macros/docs/accounts/light_mint.md (1)
166-188: Fix the typo in the example struct name.
CreateBasimintlooks unintended and will confuse readers.✏️ Proposed fix
-pub struct CreateBasimint<'info> { +pub struct CreateBasicMint<'info> {sdk-libs/compressible-client/src/decompress_mint.rs (2)
119-176: Guard against empty proof accounts to avoid panic.
proof_result.accounts[0]will panic if the indexer returns an empty accounts list. Return a structured error instead so callers can handle invalid proofs safely.🛠️ Proposed fix (bounds-check proof accounts)
- let account_info = &proof_result.accounts[0]; + let account_info = proof_result + .accounts + .get(0) + .ok_or(DecompressMintError::ProofMissingAccountInfo)?;pub enum DecompressMintError { @@ #[error("Indexer error: {0}")] IndexerError(#[from] light_client::indexer::IndexerError), + + #[error("Validity proof is missing account info")] + ProofMissingAccountInfo, }
238-399: Align compressed address derivation:decompress_mint_idempotentandcreate_mint_interfaceexpect different inputs.
derive_mint_compressed_addressinternally derives the SPL Mint PDA from the seed, then uses that PDA for address derivation. However,decompress_mint_idempotentacceptsmint_seed_pubkey(the seed), whilecreate_mint_interfaceacceptsaddress(the mint PDA). If both receive the same input value, they will compute different compressed addresses:
decompress_mint_idempotent: seed → PDA → compressed address ✓create_mint_interface: seed (used directly) → compressed address ✗Rename
mint_seed_pubkeytomint_addressand update the derivation to matchcreate_mint_interface:🔧 Suggested fix (address-based derivation)
pub struct DecompressMintRequest { - /// The seed pubkey used to derive the Mint PDA. - /// This is the same value passed as `mint_signer` when the mint was created. - pub mint_seed_pubkey: Pubkey, + /// The Mint PDA address. + pub mint_address: Pubkey, @@ - let compressed_address = - derive_mint_compressed_address(&request.mint_seed_pubkey, &address_tree); + let compressed_address = light_compressed_account::address::derive_address( + &request.mint_address.to_bytes(), + &address_tree.to_bytes(), + &light_token_interface::LIGHT_TOKEN_PROGRAM_ID, + ); @@ - address: request.mint_seed_pubkey, + address: request.mint_address,sdk-libs/compressible-client/src/load_accounts.rs (3)
129-155: Don’t silently drop ATA contexts.
filter_mapwill skip entries ifAtaDecompressContext::from_interfacereturnsNone, which can silently omit decompressions and misalign proof indices. Return a structured error instead.🛠️ Proposed fix (fail fast on missing context)
- let contexts: SmallVec<[AtaDecompressContext; 8]> = accounts - .iter() - .filter_map(|a| AtaDecompressContext::from_interface(a)) - .collect(); + let contexts: SmallVec<[AtaDecompressContext; 8]> = accounts + .iter() + .enumerate() + .map(|(i, a)| { + AtaDecompressContext::from_interface(a).ok_or_else(|| { + LoadAccountsError::BuildInstruction(format!( + "Missing ATA decompress context at index {i} (pubkey {})", + a.key + )) + }) + }) + .collect::<Result<_, _>>()?;
176-179: Bounds-check proof tree infos to avoid panic.
tree_infos.packed_tree_infos[i]will panic if the proof doesn’t include enough entries. Return a controlled error instead.🛠️ Proposed fix (safe indexing)
- let tree_info = &tree_infos.packed_tree_infos[i]; + let tree_info = tree_infos + .packed_tree_infos + .get(i) + .ok_or_else(|| { + LoadAccountsError::BuildInstruction(format!( + "Missing packed tree info for ATA index {i}" + )) + })?;
213-233: compression_index must be unique per ATA within an instruction.Hardcoding
compression_index: 0will triggerTokenError::DuplicateCompressionIndexwhen multiple compressed inputs target the same ATA in one decompress instruction. Track and increment per ATA.🛠️ Proposed fix (per-ATA compression index)
- let mut token_accounts = Vec::with_capacity(contexts.len()); + let mut token_accounts = Vec::with_capacity(contexts.len()); + let mut compression_indices: std::collections::HashMap<Pubkey, u8> = + std::collections::HashMap::new(); @@ - let ata_index = - packed_accounts.insert_or_get(derive_token_ata(&ctx.wallet_owner, &ctx.mint).0); + let ata_pubkey = derive_token_ata(&ctx.wallet_owner, &ctx.mint).0; + let ata_index = packed_accounts.insert_or_get(ata_pubkey); @@ - let tlv: Vec<ExtensionInstructionData> = token + let next_index = compression_indices.entry(ata_pubkey).or_insert(0); + let compression_index = *next_index; + *next_index = next_index.saturating_add(1); + + let tlv: Vec<ExtensionInstructionData> = token .tlv .as_ref() .map(|exts| { @@ delegated_amount: co.delegated_amount, withheld_transfer_fee: co.withheld_transfer_fee, is_frozen, - compression_index: 0, + compression_index, is_ata: true, bump: ctx.bump, owner_index,
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/account_interface.rs`:
- Around line 88-122: cold_token currently hardcodes SplAccount.state =
AccountState::Initialized, losing frozen status; update cold_token to derive the
packed account state from compressed.token.state (e.g., if
compressed.token.state indicates Frozen, set
spl_token_2022::state::AccountState::Frozen, otherwise Initialized) so the
SplAccount you build mirrors the compressed token's freeze status; reference the
compressed token via the local token variable and follow the same mapping used
in TokenAccountInterface::cold when assigning SplAccount.state.
♻️ Duplicate comments (5)
sdk-libs/compressible-client/src/load_accounts.rs (2)
63-100: Sequential proof fetching already noted.No new feedback here beyond the prior review.
422-431: Panic on missing compressed data already flagged.This still uses
expect, which can panic if invariants are violated. The prior suggestion to return a structured error remains applicable.sdk-libs/compressible-client/src/compressible_program.rs (1)
207-217: GenericFromlimitation already flagged.This still constrains
From<TokenAccountInterface>/From<AccountInterface>toAccountSpec<()>. The previous recommendation to make these generic remains.sdk-libs/compressible-client/src/account_interface_ext.rs (2)
109-147: Returning an empty hot account on missing PDA was already flagged.This still silently treats missing PDAs as present; prior feedback to return a not-found error (or make fallback explicit) still applies.
222-275: Sequential multi-fetch remains a known perf issue.The prior suggestion to parallelize RPC calls (bounded concurrency) is still applicable.
| /// Create a cold (compressed) account interface for a token account. | ||
| pub fn cold_token( | ||
| key: Pubkey, | ||
| compressed: CompressedTokenAccount, | ||
| wallet_owner: Pubkey, | ||
| ) -> Self { | ||
| use solana_program::program_pack::Pack; | ||
| use spl_token_2022::state::Account as SplAccount; | ||
|
|
||
| let token = &compressed.token; | ||
| let parsed = SplAccount { | ||
| mint: token.mint, | ||
| owner: wallet_owner, | ||
| amount: token.amount, | ||
| delegate: token.delegate.into(), | ||
| state: spl_token_2022::state::AccountState::Initialized, | ||
| is_native: solana_program::program_option::COption::None, | ||
| delegated_amount: 0, | ||
| close_authority: solana_program::program_option::COption::None, | ||
| }; | ||
| let mut data = vec![0u8; SplAccount::LEN]; | ||
| SplAccount::pack(parsed, &mut data).expect("pack should never fail"); | ||
|
|
||
| Self { | ||
| pubkey, | ||
| account, | ||
| is_cold: true, | ||
| load_context: Some(PdaLoadContext { compressed }), | ||
| key, | ||
| account: Account { | ||
| lamports: compressed.account.lamports, | ||
| data, | ||
| owner: light_token_sdk::token::LIGHT_TOKEN_PROGRAM_ID, | ||
| executable: false, | ||
| rent_epoch: 0, | ||
| }, | ||
| cold: Some(ColdContext::Token(compressed)), | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve frozen state in cold_token.
cold_token always sets state = Initialized, which loses Frozen status for compressed tokens. This can affect downstream logic that checks freeze state.
🛠️ Proposed fix (mirror TokenAccountInterface::cold)
- let parsed = SplAccount {
+ let parsed = SplAccount {
mint: token.mint,
owner: wallet_owner,
amount: token.amount,
delegate: token.delegate.into(),
- state: spl_token_2022::state::AccountState::Initialized,
+ state: match token.state {
+ light_token_sdk::compat::AccountState::Frozen => {
+ spl_token_2022::state::AccountState::Frozen
+ }
+ _ => spl_token_2022::state::AccountState::Initialized,
+ },
is_native: solana_program::program_option::COption::None,
delegated_amount: 0,
close_authority: solana_program::program_option::COption::None,
};🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/account_interface.rs` around lines 88 - 122,
cold_token currently hardcodes SplAccount.state = AccountState::Initialized,
losing frozen status; update cold_token to derive the packed account state from
compressed.token.state (e.g., if compressed.token.state indicates Frozen, set
spl_token_2022::state::AccountState::Frozen, otherwise Initialized) so the
SplAccount you build mirrors the compressed token's freeze status; reference the
compressed token via the local token variable and follow the same mapping used
in TokenAccountInterface::cold when assigning SplAccount.state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/compressible-client/src/load_accounts.rs (2)
220-244:compression_index: 0may causeDuplicateCompressionIndexerrors.Per PR comments, when multiple compressed token accounts decompress to the same ATA in one Transfer2 instruction, setting
compression_index: 0for all will triggerTokenError::DuplicateCompressionIndexat runtime.Track the index per ATA destination:
+ let mut compression_indices: std::collections::HashMap<Pubkey, u8> = + std::collections::HashMap::new(); + for (i, ctx) in contexts.iter().enumerate() { let token = &ctx.compressed.token; let tree_info = &tree_infos.packed_tree_infos[i]; + let ata_key = derive_token_ata(&ctx.wallet_owner, &ctx.mint).0; + let compression_index = *compression_indices.entry(ata_key).or_insert(0); + *compression_indices.get_mut(&ata_key).unwrap() += 1; // ... (in TLV building) CompressedOnlyExtensionInstructionData { // ... - compression_index: 0, + compression_index, // ... }
176-178: Unsafe array indexing may panic on malformed proof data.Line 178 directly indexes
tree_infos.packed_tree_infos[i]without bounds checking. If the proof structure doesn't match the expected context count, this panics.for (i, ctx) in contexts.iter().enumerate() { let token = &ctx.compressed.token; - let tree_info = &tree_infos.packed_tree_infos[i]; + let tree_info = tree_infos.packed_tree_infos + .get(i) + .ok_or_else(|| LoadAccountsError::BuildInstruction( + format!("Tree info index {} out of bounds", i) + ))?;
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/account_interface.rs`:
- Around line 88-122: Update the cold_token constructor to accept a
program_owner: Pubkey parameter (signature: pub fn cold_token(key: Pubkey,
compressed: CompressedTokenAccount, wallet_owner: Pubkey, program_owner: Pubkey)
-> Self) and replace the hardcoded
light_token_sdk::token::LIGHT_TOKEN_PROGRAM_ID with the new program_owner when
building Account.owner; keep the rest of the implementation the same, and update
any callers of cold_token to pass the appropriate program owner (use
LIGHT_TOKEN_PROGRAM_ID where the previous behavior is desired); see
TokenAccountInterface::cold() for the intended pattern.
In `@sdk-libs/macros/docs/accounts/light_mint.md`:
- Line 171: There's a typo in the struct name CreateBasimint<'info>: rename the
struct to CreateBasicMint<'info> wherever it's declared and referenced (e.g.,
the struct signature and any uses in handlers/tests/docs) so the name matches
"Basic Mint Creation" and conventional naming.
♻️ Duplicate comments (4)
sdk-libs/compressible-client/src/load_accounts.rs (2)
422-431:expect()can panic—use fallible error propagation instead.Library code should avoid panics. While the upstream filter guarantees only cold specs reach here, a defensive approach protects against invariant violations:
let compressed_accounts: Vec<(CompressedAccount, V)> = specs .iter() - .map(|s| { + .enumerate() + .map(|(i, s)| { let compressed_account = s .compressed() - .expect("Cold spec must have compressed data") + .ok_or(LoadAccountsError::MissingPdaCompressed { + index: i, + pubkey: s.address(), + })? .clone(); - (compressed_account, s.variant.clone()) + Ok((compressed_account, s.variant.clone())) }) - .collect(); + .collect::<Result<Vec<_>, _>>()?;
63-100: Sequential proof fetching—consider parallelization for throughput.Both
fetch_individual_proofsandfetch_batched_proofsawait sequentially. For larger account sets, this incurs N×RTT latency. Considerfutures::future::try_join_allor bounded concurrency when throughput matters.This isn't blocking, but the pattern at line 348-352 (
futures::join!) shows you're already thinking about concurrency at the higher level.sdk-libs/compressible-client/src/account_interface_ext.rs (2)
222-275: Sequential fetching inget_multiple_account_interfaces—TODO noted.The
TODO: parallelize with futures::join_allat line 226 correctly identifies the optimization opportunity. The current sequential loop incurs N×RTT latency.When you parallelize, consider adding per-account context to errors so failures indicate which account failed:
let futures = accounts.iter().map(|account| async move { // ... fetch logic ... result.map_err(|e| (account.pubkey(), e)) });
138-146: Empty hot account fallback is intentional but warrants documentation.Returning an empty
Accountwhen neither on-chain nor compressed state exists supports idempotent decompression patterns. Callers should checkaccount.data.is_empty()to detect non-existent accounts.Consider adding a doc comment explaining this behavior and how to detect it.
| /// Create a cold (compressed) account interface for a token account. | ||
| pub fn cold_token( | ||
| key: Pubkey, | ||
| compressed: CompressedTokenAccount, | ||
| wallet_owner: Pubkey, | ||
| ) -> Self { | ||
| use solana_program::program_pack::Pack; | ||
| use spl_token_2022::state::Account as SplAccount; | ||
|
|
||
| let token = &compressed.token; | ||
| let parsed = SplAccount { | ||
| mint: token.mint, | ||
| owner: wallet_owner, | ||
| amount: token.amount, | ||
| delegate: token.delegate.into(), | ||
| state: spl_token_2022::state::AccountState::Initialized, | ||
| is_native: solana_program::program_option::COption::None, | ||
| delegated_amount: 0, | ||
| close_authority: solana_program::program_option::COption::None, | ||
| }; | ||
| let mut data = vec![0u8; SplAccount::LEN]; | ||
| SplAccount::pack(parsed, &mut data).expect("pack should never fail"); | ||
|
|
||
| Self { | ||
| pubkey, | ||
| account, | ||
| is_cold: true, | ||
| load_context: Some(PdaLoadContext { compressed }), | ||
| key, | ||
| account: Account { | ||
| lamports: compressed.account.lamports, | ||
| data, | ||
| owner: light_token_sdk::token::LIGHT_TOKEN_PROGRAM_ID, | ||
| executable: false, | ||
| rent_epoch: 0, | ||
| }, | ||
| cold: Some(ColdContext::Token(compressed)), | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for cold_token usage to understand how it's called
rg -n "cold_token\(" --type rust -A 5 -B 2Repository: Lightprotocol/light-protocol
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Find the TokenAccountInterface and its cold() method
rg -n "impl.*TokenAccountInterface" --type rust -A 50 | head -200Repository: Lightprotocol/light-protocol
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Look for the cold() method definition with program_owner parameter
rg -n "fn cold\(" --type rust -A 10 -B 2Repository: Lightprotocol/light-protocol
Length of output: 2483
🏁 Script executed:
#!/bin/bash
# Search for any documentation files related to this
find . -name "*.md" -path "*compressible*" -o -name "CLAUDE.md" | xargs grep -l "cold_token\|token_account" 2>/dev/null || echo "No matches found"Repository: Lightprotocol/light-protocol
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Search for actual calls to cold_token() method
rg -n "\.cold_token\(" --type rust -A 3 -B 3Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Read the CLAUDE.md file for token account guidance
cat ./programs/compressed-token/program/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 14739
🏁 Script executed:
#!/bin/bash
# Also check if there are test files or examples using cold_token
rg -n "cold_token" --type rust | head -30Repository: Lightprotocol/light-protocol
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for tests or examples using cold_token
rg -n "cold_token" --type rust -C 5Repository: Lightprotocol/light-protocol
Length of output: 1029
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or docs in the file explaining the design
head -n 150 sdk-libs/compressible-client/src/account_interface.rs | tail -n 100Repository: Lightprotocol/light-protocol
Length of output: 3012
🏁 Script executed:
#!/bin/bash
# Look at the broader context around cold_token definition
sed -n '75,135p' sdk-libs/compressible-client/src/account_interface.rsRepository: Lightprotocol/light-protocol
Length of output: 1969
Accept program_owner parameter in cold_token() for consistency with TokenAccountInterface::cold().
The cold_token() method hardcodes light_token_sdk::token::LIGHT_TOKEN_PROGRAM_ID as the owner, while the parallel TokenAccountInterface::cold() method (line 263) accepts a flexible program_owner parameter with the comment "usually LIGHT_TOKEN_PROGRAM_ID"—implying other scenarios exist.
For an SDK library, this inconsistency limits flexibility. While CompressedTokenAccount is typically from the Light Token Program, the method should mirror the design pattern of its counterpart to support program-owned vaults or other variations. Consider adding program_owner: Pubkey parameter and using it instead of the hardcoded constant, matching the signature:
pub fn cold_token(
key: Pubkey,
compressed: CompressedTokenAccount,
wallet_owner: Pubkey,
program_owner: Pubkey,
) -> SelfThis preserves the default use case while supporting flexibility, consistent with SDK library design principles.
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/account_interface.rs` around lines 88 - 122,
Update the cold_token constructor to accept a program_owner: Pubkey parameter
(signature: pub fn cold_token(key: Pubkey, compressed: CompressedTokenAccount,
wallet_owner: Pubkey, program_owner: Pubkey) -> Self) and replace the hardcoded
light_token_sdk::token::LIGHT_TOKEN_PROGRAM_ID with the new program_owner when
building Account.owner; keep the rest of the implementation the same, and update
any callers of cold_token to pass the appropriate program owner (use
LIGHT_TOKEN_PROGRAM_ID where the previous behavior is desired); see
TokenAccountInterface::cold() for the intended pattern.
| #[derive(Accounts, LightAccounts)] | ||
| #[instruction(params: CreateParams)] | ||
| pub struct CreateBasicMint<'info> { | ||
| pub struct CreateBasimint<'info> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in struct name: CreateBasimint should likely be CreateBasicMint.
The example struct is named CreateBasimint<'info> but based on the section title "Basic Mint Creation" and conventional naming, this appears to be a typo. It should probably be CreateBasicMint<'info>.
Proposed fix
-pub struct CreateBasimint<'info> {
+pub struct CreateBasicMint<'info> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub struct CreateBasimint<'info> { | |
| pub struct CreateBasicMint<'info> { |
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/light_mint.md` at line 171, There's a typo in
the struct name CreateBasimint<'info>: rename the struct to
CreateBasicMint<'info> wherever it's declared and referenced (e.g., the struct
signature and any uses in handlers/tests/docs) so the name matches "Basic Mint
Creation" and conventional naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
sdk-libs/compressible-client/src/load_accounts.rs (1)
178-178: Unsafe array indexing can panic on malformed proof data.Direct indexing
tree_infos.packed_tree_infos[i]assumes the proof structure length matches the contexts length. If the proof has fewer entries than expected (malformed or malicious input), this will panic.This was flagged in PR objectives as "Unsafe Array Indexing Can Panic" at a different location, but the same pattern appears here.
🛡️ Defensive fix using bounds checking
for (i, ctx) in contexts.iter().enumerate() { let token = &ctx.compressed.token; - let tree_info = &tree_infos.packed_tree_infos[i]; + let tree_info = tree_infos.packed_tree_infos.get(i).ok_or_else(|| { + LoadAccountsError::BuildInstruction(format!( + "Tree info index {} out of bounds (have {})", + i, + tree_infos.packed_tree_infos.len() + )) + })?;
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/account_interface.rs`:
- Around line 3-5: Update the module-level documentation to accurately reflect
AccountInterface's capabilities by including "tokens" alongside "PDAs, mints"
(to match the later comment and the cold_token constructor behavior); locate the
module doc comment describing "Core types" and adjust the line listing for
`AccountInterface` so it reads something like "PDAs, mints, tokens", and ensure
any other summary (e.g., the section comment that currently says "PDAs, mints,
tokens") is consistent with the `cold_token` constructor and the
`TokenAccountInterface` descriptions.
In `@sdk-libs/compressible-client/src/decompress_mint.rs`:
- Around line 23-24: The MintNotFound error is being constructed with the seed
pubkey (request.mint_seed_pubkey) but its message refers to the derived mint
address; update the MintNotFound construction to pass the actual derived mint
Pubkey (the variable holding the derived mint address—e.g., minted/derived_mint,
mint_address, or the result of the PDA derivation you compute earlier) instead
of request.mint_seed_pubkey so the error.address reflects the real mint address;
search for MintNotFound { address: request.mint_seed_pubkey } and replace the
argument with the derived mint Pubkey variable used in that scope.
In `@sdk-libs/macros/docs/accounts/light_mint.md`:
- Line 171: The struct name CreateBasimint contains a typo; rename the struct to
CreateBasicMint and update all references/usages/comments in this doc and any
example code so they match the corrected identifier (e.g., change struct
declaration CreateBasimint -> CreateBasicMint and adjust any instantiations,
type annotations, or mentions in surrounding examples). Ensure exported names
and links in the documentation are updated consistently to avoid broken
references.
♻️ Duplicate comments (4)
sdk-libs/compressible-client/src/account_interface_ext.rs (2)
222-275: Sequential fetching acknowledged with TODO for parallelization.The current implementation awaits each fetch sequentially, which adds N×RTT latency. The TODO at line 226 acknowledges this. For high-throughput scenarios, consider
futures::try_join_allor bounded concurrency withFuturesUnordered. This was flagged in past reviews.
138-146: Intentional fallback to empty hot account when PDA doesn't exist.This design choice silently returns an empty account rather than erroring when a PDA is not found on-chain or compressed. While documented in past reviews as acceptable (callers can check
data.is_empty()), this could mask genuine lookup failures. Consider whether a dedicated "not found" state would be safer than an empty hot account.sdk-libs/compressible-client/src/load_accounts.rs (1)
422-431: Avoid panic with.expect()- use.ok_or()for graceful error handling.Line 427 uses
.expect("Cold spec must have compressed data")which will panic if the invariant is violated. This was flagged in past reviews. Use.ok_or()to return a proper error instead.🛡️ Defensive fix
let compressed_accounts: Vec<(CompressedAccount, V)> = specs .iter() - .map(|s| { + .enumerate() + .map(|(i, s)| { let compressed_account = s .compressed() - .expect("Cold spec must have compressed data") + .ok_or(LoadAccountsError::MissingPdaCompressed { + index: i, + pubkey: s.address(), + })? .clone(); - (compressed_account, s.variant.clone()) + Ok((compressed_account, s.variant.clone())) }) - .collect(); + .collect::<Result<Vec<_>, _>>()?;sdk-libs/compressible-client/src/compressible_program.rs (1)
207-217:Fromimplementations fixed to()- intentional limitation.The
From<TokenAccountInterface>andFrom<AccountInterface>impls produceAccountSpec<()>, meaning you can't dolet spec: AccountSpec<MyVariant> = token_interface.into(). This is fine for type-erased contexts but worth documenting in the crate docs if users expect generic conversions.
| //! Core types: | ||
| //! - `AccountInterface` - Generic compressible account (PDAs, mints) | ||
| //! - `TokenAccountInterface` - Token accounts (ATAs, program-owned vaults) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Documentation inconsistency with actual capabilities.
Line 4 states AccountInterface handles "PDAs, mints", but the section comment at line 35 says "PDAs, mints, tokens" — and the cold_token constructor (lines 88-122) indeed creates an AccountInterface from a CompressedTokenAccount. Consider aligning the module-level docs:
📝 Suggested fix
//! Core types:
-//! - `AccountInterface` - Generic compressible account (PDAs, mints)
+//! - `AccountInterface` - Generic compressible account (PDAs, mints, tokens)
//! - `TokenAccountInterface` - Token accounts (ATAs, program-owned vaults)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! Core types: | |
| //! - `AccountInterface` - Generic compressible account (PDAs, mints) | |
| //! - `TokenAccountInterface` - Token accounts (ATAs, program-owned vaults) | |
| //! Core types: | |
| //! - `AccountInterface` - Generic compressible account (PDAs, mints, tokens) | |
| //! - `TokenAccountInterface` - Token accounts (ATAs, program-owned vaults) |
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/account_interface.rs` around lines 3 - 5,
Update the module-level documentation to accurately reflect AccountInterface's
capabilities by including "tokens" alongside "PDAs, mints" (to match the later
comment and the cold_token constructor behavior); locate the module doc comment
describing "Core types" and adjust the line listing for `AccountInterface` so it
reads something like "PDAs, mints, tokens", and ensure any other summary (e.g.,
the section comment that currently says "PDAs, mints, tokens") is consistent
with the `cold_token` constructor and the `TokenAccountInterface` descriptions.
| #[error("Compressed mint not found for address {address:?}")] | ||
| MintNotFound { address: Pubkey }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error field address holds the seed pubkey, not the derived mint address.
At line 301, address: request.mint_seed_pubkey passes the seed pubkey to MintNotFound, but the error message says "Compressed mint not found for address {address:?}". The address field should contain the derived mint address for consistency.
🔧 Suggested fix
+ // Derive the actual mint address for the error
+ let (mint_address, _) = light_token_sdk::token::find_mint_address(&request.mint_seed_pubkey);
+
// 2. Fetch compressed mint account from indexer
let compressed_account = indexer
.get_compressed_account(compressed_address, None)
.await?
.value
.ok_or(DecompressMintError::MintNotFound {
- address: request.mint_seed_pubkey,
+ address: mint_address,
})?;Also applies to: 300-302
🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/decompress_mint.rs` around lines 23 - 24,
The MintNotFound error is being constructed with the seed pubkey
(request.mint_seed_pubkey) but its message refers to the derived mint address;
update the MintNotFound construction to pass the actual derived mint Pubkey (the
variable holding the derived mint address—e.g., minted/derived_mint,
mint_address, or the result of the PDA derivation you compute earlier) instead
of request.mint_seed_pubkey so the error.address reflects the real mint address;
search for MintNotFound { address: request.mint_seed_pubkey } and replace the
argument with the derived mint Pubkey variable used in that scope.
| #[derive(Accounts, LightAccounts)] | ||
| #[instruction(params: CreateParams)] | ||
| pub struct CreateBasicMint<'info> { | ||
| pub struct CreateBasimint<'info> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in struct name: CreateBasimint should be CreateBasicMint.
The example struct name appears to have a typo - missing the 'c' in "Basic".
📝 Suggested fix
-pub struct CreateBasimint<'info> {
+pub struct CreateBasicMint<'info> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub struct CreateBasimint<'info> { | |
| pub struct CreateBasicMint<'info> { |
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/light_mint.md` at line 171, The struct name
CreateBasimint contains a typo; rename the struct to CreateBasicMint and update
all references/usages/comments in this doc and any example code so they match
the corrected identifier (e.g., change struct declaration CreateBasimint ->
CreateBasicMint and adjust any instantiations, type annotations, or mentions in
surrounding examples). Ensure exported names and links in the documentation are
updated consistently to avoid broken references.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.