chore: light-ctoken-sdk small refactors and inline docs #2102
chore: light-ctoken-sdk small refactors and inline docs #2102SwenSchaeferjohann merged 17 commits intomainfrom
Conversation
WalkthroughThis PR renames many CPI-facing types from Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Focus review on:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)sdk-libs/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (65)📓 Common learnings📚 Learning: 2025-12-06T00:49:00.181ZApplied to files:
📚 Learning: 2025-11-24T17:59:54.233ZApplied to files:
📚 Learning: 2025-11-24T18:01:54.689ZApplied to files:
📚 Learning: 2025-12-06T00:49:00.181ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-11-24T17:55:17.323ZApplied to files:
📚 Learning: 2025-12-06T00:49:00.181ZApplied to files:
📚 Learning: 2025-11-24T17:59:13.714ZApplied to files:
📚 Learning: 2025-11-24T18:02:15.670ZApplied to files:
📚 Learning: 2025-12-06T00:49:00.181ZApplied to files:
📚 Learning: 2025-11-24T17:59:03.485ZApplied to files:
📚 Learning: 2025-11-24T17:55:17.323ZApplied to files:
📚 Learning: 2025-11-24T17:58:50.237ZApplied to files:
📚 Learning: 2025-11-24T17:56:20.711ZApplied to files:
📚 Learning: 2025-11-24T18:00:13.178ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-12-06T00:49:21.983ZApplied to files:
📚 Learning: 2025-12-06T00:49:21.983ZApplied to files:
📚 Learning: 2025-11-24T18:00:13.178ZApplied to files:
📚 Learning: 2025-11-24T17:55:17.323ZApplied to files:
📚 Learning: 2025-11-24T17:55:17.323ZApplied to files:
📚 Learning: 2025-12-06T00:49:00.181ZApplied to files:
📚 Learning: 2025-11-24T17:56:20.711ZApplied to files:
📚 Learning: 2025-11-24T17:54:38.537ZApplied to files:
📚 Learning: 2025-11-24T17:55:17.323ZApplied to files:
📚 Learning: 2025-11-24T17:54:33.614ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-12-06T00:49:37.752ZApplied to files:
📚 Learning: 2025-11-24T17:58:50.237ZApplied to files:
📚 Learning: 2025-11-24T17:56:50.011ZApplied to files:
📚 Learning: 2025-11-24T17:59:46.693ZApplied to files:
📚 Learning: 2025-11-24T17:57:39.230ZApplied to files:
📚 Learning: 2025-11-24T18:01:54.689ZApplied to files:
📚 Learning: 2025-11-24T17:59:03.485ZApplied to files:
📚 Learning: 2025-11-24T17:59:03.485ZApplied to files:
📚 Learning: 2025-12-06T00:49:13.007ZApplied to files:
📚 Learning: 2025-12-06T00:49:13.007ZApplied to files:
📚 Learning: 2025-11-24T17:58:50.237ZApplied to files:
📚 Learning: 2025-11-24T17:56:20.711ZApplied to files:
📚 Learning: 2025-11-24T18:02:15.670ZApplied to files:
📚 Learning: 2025-11-24T17:56:00.229ZApplied to files:
📚 Learning: 2025-11-24T17:59:23.357ZApplied to files:
📚 Learning: 2025-12-06T00:49:13.007ZApplied to files:
📚 Learning: 2025-11-24T17:59:13.714ZApplied to files:
📚 Learning: 2025-11-24T17:59:23.357ZApplied to files:
📚 Learning: 2025-11-24T17:59:54.233ZApplied to files:
📚 Learning: 2025-12-06T00:49:13.007ZApplied to files:
📚 Learning: 2025-11-24T17:59:46.693ZApplied to files:
📚 Learning: 2025-11-24T17:59:46.693ZApplied to files:
📚 Learning: 2025-11-24T17:58:50.237ZApplied to files:
📚 Learning: 2025-11-24T17:59:03.485ZApplied to files:
📚 Learning: 2025-11-24T17:57:53.312ZApplied to files:
📚 Learning: 2025-12-06T00:49:13.007ZApplied to files:
📚 Learning: 2025-11-24T18:02:15.670ZApplied to files:
📚 Learning: 2025-11-24T18:02:15.670ZApplied to files:
📚 Learning: 2025-12-06T00:49:21.983ZApplied to files:
📚 Learning: 2025-11-24T17:56:50.011ZApplied to files:
📚 Learning: 2025-11-24T17:56:20.711ZApplied to files:
📚 Learning: 2025-11-24T17:56:00.229ZApplied to files:
📚 Learning: 2025-11-24T17:57:53.312ZApplied to files:
📚 Learning: 2025-11-24T17:57:39.230ZApplied to files:
📚 Learning: 2025-11-24T17:56:00.229ZApplied to files:
🧬 Code graph analysis (1)sdk-libs/ctoken-sdk/src/ctoken/create_cmint.rs (2)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (8)
sdk-libs/sdk/src/lib.rs (2)
3-6: Minor grammar issue in documentation.Line 4 begins with "and unique addresses..." which creates an awkward sentence fragment. Consider combining into a single coherent statement or restructuring for clarity.
//! Compressed Accounts store state as account hashes in State Merkle trees. //! and unique addresses in Address Merkle trees. +//! Compressed Accounts store state as account hashes in State Merkle trees +//! and unique addresses in Address Merkle trees.
147-147: Add documentation comments to the new public modules.The
errorandutilsmodules at lines 147 and 154 are properly defined and exported, but they lack documentation comments. Other modules in the same file have documentation (e.g.,cpi,instruction,transfer). Add doc comments to both modules for consistency:/// Error types for the light SDK. pub mod error; /// Utility functions for the light SDK. pub mod utils;sdk-libs/ctoken-sdk/src/ctoken/compressible.rs (1)
48-75: Align CPI default token_account_version with CompressibleParams::default()
CompressibleParamsCpi::newusesCompressibleParams::default()for most fields but hardcodestoken_account_version: TokenDataVersion::ShaFlatinstead ofdefaults.token_account_version. That couples the CPI helper to a specific version and can silently diverge from the core default if it changes.Prefer sourcing from
defaults.token_account_versionto keep both in sync:- Self { + Self { compressible_config, rent_sponsor, system_program, pre_pay_num_epochs: defaults.pre_pay_num_epochs, lamports_per_write: defaults.lamports_per_write, compress_to_account_pubkey: None, - token_account_version: TokenDataVersion::ShaFlat, + token_account_version: defaults.token_account_version, }sdk-libs/ctoken-sdk/src/ctoken/transfer_ctoken.rs (1)
18-55: CPI helper is fine; consider clarifying max_top_up semanticsThe new
TransferCtokenCpiwrapper and itsinvoke/invoke_signedhelpers are wired correctly: they build aTransferCtokenviaFromand pass[source, destination, authority]toinvoke(_signed).One small nit: the field comment on
max_top_upsays "0 = no limit", but the field type isOption<u16>, whereNonealready encodes "no limit". IfSome(0)is intended to be equivalent toNone, it may be worth tightening the comment or enforcing that invariant at construction.sdk-libs/ctoken-sdk/src/ctoken/mod.rs (1)
147-191: New ID/config helper fns look good; minor naming consistency nit
CTOKEN_PROGRAM_ID,CTOKEN_CPI_AUTHORITY,COMPRESSIBLE_CONFIG_V1,RENT_SPONSOR, and the helper functionsid(),cpi_authority(),config_pda(), andrent_sponsor_pda()are simple, correct wrappers and match the documented addresses.If you want maximal API consistency, consider naming the helpers either all with
_pdasuffix or none (e.g.program_id(),cpi_authority_pda(),compressible_config_pda(),rent_sponsor_pda()), but that’s purely stylistic.sdk-libs/ctoken-sdk/src/ctoken/create_cmint.rs (1)
306-354: Consider extracting duplicated account_infos construction.The
invokeandinvoke_signedmethods share nearly identical account ordering logic (lines 310-326 vs 335-351). This duplication increases maintenance burden and risk of divergence.Consider extracting a helper method:
+ fn build_account_infos(self) -> (Instruction, Vec<AccountInfo<'info>>) { + let instruction = self.instruction()?; + let mut account_infos = vec![ + self.system_accounts.light_system_program, + self.mint_signer, + self.authority, + self.payer, + self.system_accounts.cpi_authority_pda, + self.system_accounts.registered_program_pda, + self.system_accounts.account_compression_authority, + self.system_accounts.account_compression_program, + self.system_accounts.system_program, + self.output_queue, + self.address_tree, + ]; + if let Some(cpi_context_account) = self.cpi_context_account { + account_infos.push(cpi_context_account); + } + (instruction, account_infos) + }sdk-libs/ctoken-sdk/src/ctoken/mint_to.rs (2)
275-327: Significant code duplication between invoke and invoke_signed.The account_infos construction (lines 279-297 vs 306-324) is nearly identical between the two methods. This 20-line duplication is more substantial than in other CPI types.
Extract the account ordering into a helper:
+ fn collect_account_infos(self) -> (Instruction, Vec<AccountInfo<'info>>) { + let instruction = self.instruction()?; + let mut account_infos = vec![ + self.system_accounts.light_system_program, + self.authority, + self.payer, + self.system_accounts.cpi_authority_pda, + self.system_accounts.registered_program_pda, + self.system_accounts.account_compression_authority, + self.system_accounts.account_compression_program, + self.system_accounts.system_program, + ]; + if let Some(cpi_context_account) = self.cpi_context_account { + account_infos.push(cpi_context_account); + } + account_infos.push(self.output_queue); + account_infos.push(self.state_tree); + account_infos.push(self.input_queue); + account_infos.extend(self.ctoken_accounts); + (instruction, account_infos) + }
35-37: TODO: Make account_index dynamic.This hardcoded
account_index: 0at line 36 (and line 163) limits flexibility. Theadd_mint_to_actionmethod allows adding more actions with custom indices, but the initial action is always index 0.Would you like me to open an issue to track making the initial
account_indexconfigurable inMintToCTokenParams::newandMintToCTokenCpiWriteParams::new?
- derive_compressed_mint_address -> derive_cmint_compressed_address - find_spl_mint_address -> find_cmint_address - CreateCMint::mint_signer -> mint_seed_pubkey - CreateCMintCpi::mint_signer -> mint_seed - CreateCMintCpi::new_with_address -> new
There was a problem hiding this comment.
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 (6)
sdk-libs/macros/src/compressible/instructions.rs (1)
582-590: Error information is lost for non-custom errors.When
LocalProvider::get_seedsreturns a non-custom error, the mapping defaults toCustom(0), discarding the original error variant (e.g.,InvalidAccountData,BorshIoError). This complicates debugging.Consider either preserving error type information via distinct custom codes or documenting why code
0is acceptable here.sdk-libs/ctoken-sdk/src/compressed_token/v2/create_compressed_mint/instruction.rs (1)
186-211: Consolidate duplicated mint address derivation functions to prevent maintenance drift.These three functions—
derive_cmint_compressed_address,derive_cmint_from_spl_mint, andfind_cmint_address—are duplicated identically in bothsdk-libs/ctoken-sdk/src/ctoken/create_cmint.rsandsdk-libs/ctoken-sdk/src/compressed_token/v2/create_compressed_mint/instruction.rs. The functions share the same signatures and implementations, using identical seeds and program IDs. This duplication risks unintended divergence if one copy is updated without the other. Move these functions to a shared utility module (e.g.,sdk-libs/ctoken-sdk/src/utils/mint_address.rsor similar) and re-export from both current locations to maintain a single source of truth.sdk-libs/ctoken-sdk/src/ctoken/transfer_spl_ctoken.rs (1)
94-130: Account ordering duplicated betweeninvokeandinvoke_signed— consider extracting.Both methods construct identical
account_infosarrays. While this works correctly, you could reduce duplication by extracting the array construction into a helper method. The comments documenting the account order are helpful for maintenance.+ fn build_account_infos(self) -> ([AccountInfo<'info>; 8], Instruction) { + let instruction = TransferSplToCtoken::from(&self).instruction().unwrap(); + let account_infos = [ + self.compressed_token_program_authority, + self.payer, + self.mint, + self.destination_ctoken_account, + self.authority, + self.source_spl_token_account, + self.spl_interface_pda, + self.spl_token_program, + ]; + (account_infos, instruction) + }sdk-libs/ctoken-sdk/src/ctoken/transfer_ctoken_spl.rs (2)
93-129: Same duplication pattern as transfer_spl_ctoken.rs.The
invokeandinvoke_signedmethods have identical account array construction. Consider extracting to a helper if you want consistency across the codebase. The account ordering comments are valuable documentation.
147-209: Instruction construction with profiling looks good.The
#[profile]macro enables performance tracking. The two-phase operation (compress ctoken → decompress to SPL) correctly uses indices 1 and 2 for source/destination respectively, matching thepacked_accountsorder.Note the TODO comment on line 188 (
pool_index (TODO: make dynamic)) — this may need addressing in a follow-up.Would you like me to open an issue to track making the
pool_indexparameter dynamic?sdk-libs/ctoken-sdk/src/ctoken/create_cmint.rs (1)
220-301: Naming inconsistency:CreateCompressedMintCpiWritestill usesmint_signerwhileCreateCMintusesmint_seed_pubkey.The
CreateCMintstruct renamed its field tomint_seed_pubkeyto clarify its purpose, butCreateCompressedMintCpiWrite(lines 224-230) andCreateCompressedMintCpiWriteCpi(lines 455-462) still usemint_signer. This inconsistency could confuse SDK users.If the semantics are the same, consider aligning the naming:
pub struct CreateCompressedMintCpiWrite { - pub mint_signer: Pubkey, + pub mint_seed: Pubkey, pub payer: Pubkey, ... }Please verify whether
mint_signerin the CPI write path serves the same purpose asmint_seed_pubkeyinCreateCMint. If so, the naming should be consistent.
♻️ Duplicate comments (1)
sdk-libs/macros/src/compressible/instructions.rs (1)
606-614: Error information is lost for non-custom errors.Same issue as
get_seeds: non-custom errors are mapped toCustom(0), losing the original error type. Consider preserving error information or documenting this behavior.
ananas-block
force-pushed
the
jorrit/chore-prepare-ctoken-sdk-release
branch
from
3c78cd3 to
eb576ab
Compare
ananas-block
force-pushed
the
jorrit/chore-prepare-ctoken-sdk-release
branch
from
eb576ab to
5a398a6
Compare
ananas-block
force-pushed
the
jorrit/chore-prepare-ctoken-sdk-release
branch
from
445bcdf to
ba715c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (20)
sdk-libs/ctoken-sdk/src/compressed_token/v2/compress_and_close.rs (1)
306-368: Consider simplifying the rent_sponsor_pubkey logic.The
rent_sponsor_pubkeyvariable is initialized asNoneon line 306, then potentially set inside a nested loop (lines 351-353), then checked again on line 360. This control flow is a bit convoluted.The variable
actual_rent_sponsoris determined on lines 343-368, but the logic duplicates the extension-searching pattern that already happened forauthority(lines 322-340).Consider extracting the extension lookup into a small helper to reduce duplication:
+fn find_compressible_extension(ctoken: &CToken) -> Option<&Compressible> { + ctoken.extensions.as_ref()?.iter().find_map(|ext| { + if let ZExtensionStruct::Compressible(e) = ext { Some(e) } else { None } + }) +}Then use this helper for both authority and rent_sponsor lookups.
sdk-libs/token-client/src/actions/transfer2/compress.rs (1)
26-35: Fix return type incompressdoc comment to match the signatureThe doc says
Result<Instruction, CTokenSdkError>, but the function returnsResult<Signature, RpcError>. This will mislead users and any generated docs; the comment should describe the actual API.-/// `Result<Instruction, CTokenSdkError>` - The compression instruction +/// `Result<Signature, RpcError>` - The transaction signature for the compression operationsdk-libs/ctoken-sdk/src/compressed_token/v2/transfer2/instruction.rs (1)
149-184: Consider removing or uncommenting the dead code block.This commented-out
transfer2function contains useful validation logic (checkingmethod_usedflag) that could be valuable. If this is work-in-progress, consider adding a TODO comment explaining when it will be enabled. If it's obsolete, removing it would improve maintainability.sdk-libs/ctoken-sdk/src/compressed_token/v2/mint_action/cpi_accounts.rs (1)
115-125: Usecheck_mut()function and returnAccountError::AccountNotMutableinstead ofAccountError::AccountMutable.The code checks
!account_info.is_writable()and returnsAccountError::AccountMutable("Account is mutable"), which is semantically inverted. When an account is NOT writable, the correct error isAccountError::AccountNotMutable(error code 20002). Better yet, use thecheck_mut()function from theaccount_checksmodule which returns the correct error automatically:check_mut(&out_output_queue)?;instead of manual writable checks. This aligns with the established validation pattern used throughout the codebase.
sdk-libs/token-client/src/instructions/transfer2.rs (3)
188-192: Unwrapped RPC call can panic; consider propagating the error.The
.unwrap()onget_validity_proofresult will panic if the RPC call fails. Since this is client-side code that interacts with external services, propagate the error instead.let rpc_proof_result = rpc .get_validity_proof(hashes, vec![], None) .await - .unwrap() + .map_err(|_| CTokenSdkError::InvalidAccountData)? .value;
199-212: Unwrapped RPC call can panic; propagate error for resilience.The
.unwrap()onget_random_state_tree_info()(line 201) will panic if no tree info is available. Consider usingok_orto convert to an appropriate error.let shared_output_queue = if packed_tree_infos.address_trees.is_empty() { let shared_output_queue = rpc .get_random_state_tree_info() - .unwrap() + .ok_or(CTokenSdkError::InvalidAccountData)? .get_output_pubkey() - .unwrap(); + .ok_or(CTokenSdkError::InvalidAccountData)?; packed_tree_accounts.insert_or_get(shared_output_queue)
264-269: Chain of unwraps on RPC account fetch can panic.The double
.unwrap()onget_account(lines 267-268) will panic if the account doesn't exist or the RPC fails. This pattern repeats at lines 331-336. Propagate errors for production resilience.let source_account_owner = rpc .get_account(input.solana_token_account) .await - .unwrap() - .unwrap() + .map_err(|_| CTokenSdkError::InvalidAccountData)? + .ok_or(CTokenSdkError::InvalidAccountData)? .owner;sdk-libs/ctoken-sdk/src/compressed_token/v1/batch_compress/account_metas.rs (1)
29-33:unimplemented!()will panic at runtime; complete or guard this code path.If
with_lamportsistrue, this will panic. Either implement the SOL pool PDA derivation or add a compile-time feature gate / explicit error return to prevent runtime panics.let sol_pool_pda = if with_lamports { - unimplemented!("TODO hardcode sol pool pda") + // Use the SOL_POOL_PDA constant from light_sdk + Some(Pubkey::from(light_sdk::constants::SOL_POOL_PDA)) } else { None };sdk-libs/ctoken-sdk/src/compressed_token/v1/transfer/account_metas.rs (1)
193-193: Remove debugprintln!statement before release.This appears to be leftover debug code that should not be in production. It will pollute logs and may leak configuration details.
- println!("config.with_anchor_none {}", config.with_anchor_none);sdk-libs/ctoken-sdk/src/compressed_token/v2/create_compressed_mint/instruction.rs (2)
154-154: Minor formatting: missing space after comma.- compressed_mint_instruction_data,input.cpi_context + compressed_mint_instruction_data, input.cpi_context
186-211: Extract mint address derivation functions to a shared utility module.Three functions are duplicated identically across
sdk-libs/ctoken-sdk/src/compressed_token/v2/create_compressed_mint/instruction.rsandsdk-libs/ctoken-sdk/src/ctoken/create_cmint.rs:
find_cmint_addressderive_cmint_compressed_addressderive_cmint_from_spl_mintBoth modules export these functions in their public APIs, creating a maintenance burden where any changes must be synchronized in two places. Extract these to a shared utility module (e.g.,
sdk-libs/ctoken-sdk/src/utils/mint_address.rs) and re-export from both locations.sdk-libs/ctoken-sdk/src/ctoken/compressible.rs (2)
86-94: Consider addingDebugderive toCompressibleParamsCpi.
CompressibleParamsderivesDebug(line 26), butCompressibleParamsCpidoes not. Adding#[derive(Debug)]would be helpful for debugging CPI operations, thoughAccountInfomay require manual implementation.+#[derive(Debug)] pub struct CompressibleParamsCpi<'info> {
96-112: Minor inconsistency innew()constructor.Line 110 hardcodes
TokenDataVersion::ShaFlatinstead of usingdefaults.token_account_version. While functionally equivalent (since the default is alsoShaFlat), using the default value would be more consistent and maintainable if the default ever changes.compress_to_account_pubkey: None, - token_account_version: TokenDataVersion::ShaFlat, + token_account_version: defaults.token_account_version,sdk-libs/ctoken-sdk/src/utils.rs (1)
31-32: Consider using thespl_token::IDconstant instead of hardcoding the address, but addspl-tokenas a dependency first.Line 32 hardcodes the SPL Token program ID as a string, while line 31 uses
spl_token_2022::IDdirectly. For consistency and to avoid potential typos, use the constant from thespl_tokencrate. However,spl-tokenis not currently listed as a dependency in this package'sCargo.toml, so you'll need to add it to the workspace dependencies alongsidespl-token-2022.+spl-token = { workspace = true }Then update the code:
- let spl_token = Pubkey::from_str_const("TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA"); + let spl_token = spl_token::ID;sdk-libs/ctoken-sdk/src/compressed_token/v1/transfer/instruction.rs (2)
121-128: Replace panic with proper error return.This
panic!will abort the entire program ifcpi_context_pubkeyisSomebutcpi_contextisNone. Consider returning an error instead for graceful handling.if let Some(cpi_context_pubkey) = transfer_config.cpi_context_pubkey { if transfer_config.cpi_context.is_some() { account_metas.push(AccountMeta::new(cpi_context_pubkey, false)); } else { - // TODO: throw error - panic!("cpi_context.is_none() but transfer_config.cpi_context_pubkey is some"); + return Err(CTokenSdkError::CpiContextRequired); } }
173-174: Consider propagating error fromtoken_account.compress().The
.unwrap()here will panic on failure. Since this function returnsResult, propagating the error would be safer.let mut token_account = CTokenAccount::new_empty(mint, recipient, output_tree_index); - token_account.compress(amount).unwrap(); + token_account.compress(amount).map_err(|_| CTokenSdkError::InvalidCompressDecompressAmount)?;sdk-libs/ctoken-sdk/src/ctoken/mint_to.rs (1)
238-245: Validation check for CPI context is appropriate.Returning
ProgramError::InvalidAccountDatawhen neitherfirst_set_contextnorset_contextis true prevents invalid CPI writes. Consider using a more specific error variant for clearer debugging.sdk-libs/ctoken-sdk/src/ctoken/transfer_interface.rs (2)
93-157: Consider reducing duplication betweeninvokeandinvoke_signed.Both methods share nearly identical dispatch logic—the only difference is calling
.invoke()vs.invoke_signed(signer_seeds). Consider extracting the common pattern:fn dispatch_transfer( &self, invoke_fn: impl FnOnce(/* variant */) -> Result<(), ProgramError>, ) -> Result<(), ProgramError> { // common logic }Or simpler: have
invoke()delegate toinvoke_signed(&[])if the underlying CPI methods support empty signer seeds.Also applies to: 163-227
100-107:max_top_up: Nonehardcoded—consider exposing as configurable.Both branches hardcode
max_top_up: Nonewith the comment "No limit by default". If users need to set a top-up limit for ctoken→ctoken transfers, they'd need to bypassTransferInterfaceCpientirely. Consider adding a builder method:pub fn with_max_top_up(mut self, max_top_up: Option<u64>) -> Self { self.max_top_up = max_top_up; self }Also applies to: 170-177
sdk-libs/ctoken-sdk/src/compressed_token/v2/account2.rs (1)
363-365: Useself.output.mintfor consistency with the rest of the file.The line uses
self.mintvia Deref coercion, which works but is inconsistent—every other occurrence in this file (lines 125, 158, 183, 210, 241, 270, 303, 337) explicitly usesself.output.mint. Change line 364 to*account_infos[self.output.mint as usize].keyfor clarity and consistency.
sdk-libs/ctoken-sdk/src/compressed_token/v1/batch_compress/account_metas.rs
Outdated
Show resolved
Hide resolved
sdk-libs/ctoken-sdk/src/compressed_token/v2/update_compressed_mint/instruction.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
sdk-libs/ctoken-sdk/src/compressed_token/v2/account2.rs (1)
363-377: Useself.output.mintandself.output.ownerin accessor methodsThe
mint()andowner()methods reference non-existent fields.CTokenAccount2only definesinputs,output,compression,delegate_is_set, andmethod_used(lines 12-19). Themintandownerfields exist only on theoutputfield of typeMultiTokenTransferOutputData. Update the methods to:pub fn mint(&self, account_infos: &[AccountInfo]) -> Pubkey { *account_infos[self.output.mint as usize].key } pub fn owner(&self, account_infos: &[AccountInfo]) -> Pubkey { *account_infos[self.output.owner as usize].key }Note: The commented-out
delegate_accountmethod at line 388 demonstrates the correct pattern withself.output.delegate.sdk-libs/ctoken-sdk/src/compressed_token/v2/transfer2/instruction.rs (1)
149-184: Consider removing commented-out code.A large block of commented code (149-184) remains in the file. If this code is no longer needed, consider removing it to improve maintainability. If it's intended as documentation or future reference, consider moving it to inline documentation or an issue tracker.
sdk-libs/token-client/src/actions/ctoken_transfer.rs (1)
40-40: Address the TODO comment when ready.This TODO indicates the instruction construction should eventually be sourced from the compressed-token-sdk. When that's done, you'll get better type safety and reduce code duplication.
Would you like me to open an issue to track this technical debt?
sdk-libs/ctoken-sdk/tests/mint_action_cpi_accounts_tests.rs (1)
473-475: Duplicate assertion detected.Line 474 duplicates the assertion on line 473. While harmless, consider removing the duplicate for clarity.
let result = MintActionCpiAccounts::<AccountInfo>::try_from_account_infos(&accounts); assert!(result.is_err()); - assert!(result.is_err()); }sdk-libs/token-client/src/instructions/transfer2.rs (3)
300-363: Decompress SPL vs ctoken split is sound; fix misleading comment.The updated owner check using
recipient_account_owner.to_bytes() != CTOKEN_PROGRAM_IDcorrectly routes:
- non-CTOKEN owners through
decompress_splwith SPL interface PDA + pool index, and- CTOKEN owners through
decompress_ctoken.However, the comment in the
elsebranch (“Use the new SPL-specific decompress method”) is now misleading since that branch callsdecompress_ctoken. Consider updating the comment to something like “Use the ctoken-specific decompress method” to avoid confusion.
498-590: Compress-and-close error mapping is reasonable but a bit coarse.The new handling:
- Maps RPC fetch failures, zero-copy parsing failures, and missing compressible extension fields to
CTokenSdkError::InvalidAccountData.- Requires a
Compressibleextension whenis_compressibleis true and uses the owner bytes as rent sponsor whenis_compressibleis false.This is functionally consistent and keeps client-side validation simple. If you want better observability, you might consider introducing more specific
CTokenSdkErrorvariants for “missing compressible extension” vs “invalid layout” in the future, but that’s not a blocker.
5-20: Error type migration to CTokenSdkError is complete and consistent.Both
create_decompress_instructionandcreate_generic_transfer2_instructioncorrectly returnResult<Instruction, CTokenSdkError>, and all internal error handling uses the new error type. Call sites in the action layer (compress.rs, decompress.rs, transfer.rs, etc.) are properly updated to handle theCTokenSdkErrorreturn type. No legacyTokenSdkErrorpatterns remain in the codebase.
♻️ Duplicate comments (2)
sdk-libs/ctoken-sdk/src/ctoken/create.rs (1)
14-26: Doc examples still won’t compile due to?usage in non-Result context.Both examples use
?but are not wrapped in aResult-returning function, so rustdoc will attempt to generatefn main() { ... }and fail compilation. This is the same class of issue that was flagged earlier for this file.Consider either:
- Wrapping the examples in a hidden
fn main() -> Result<(), solana_program_error::ProgramError> { ... }, or- Marking them as
rust,ignoreif you don’t want them compiled.Example fix using a Result-returning function:
-/// ```rust -/// # use solana_pubkey::Pubkey; -/// # use light_ctoken_sdk::ctoken::CreateCTokenAccount; -/// # let payer = Pubkey::new_unique(); -/// # let account = Pubkey::new_unique(); -/// # let mint = Pubkey::new_unique(); -/// # let owner = Pubkey::new_unique(); -/// let instruction = -/// -/// CreateCTokenAccount::new(payer, account, mint, owner) -/// .instruction()?; -/// # Ok::<(), solana_program_error::ProgramError>(()) -/// ``` +/// ```rust +/// # use solana_pubkey::Pubkey; +/// # use light_ctoken_sdk::ctoken::CreateCTokenAccount; +/// # fn example() -> Result<(), solana_program_error::ProgramError> { +/// # let payer = Pubkey::new_unique(); +/// # let account = Pubkey::new_unique(); +/// # let mint = Pubkey::new_unique(); +/// # let owner = Pubkey::new_unique(); +/// let _instruction = +/// CreateCTokenAccount::new(payer, account, mint, owner) +/// .instruction()?; +/// # Ok(()) +/// # } +/// ``` @@ -/// ```rust,no_run +/// ```rust,no_run /// # use light_ctoken_sdk::ctoken::{CreateCTokenAccountCpi, CompressibleParamsCpi}; /// # use solana_account_info::AccountInfo; /// # use solana_pubkey::Pubkey; -/// # let payer: AccountInfo = todo!(); -/// # let account: AccountInfo = todo!(); -/// # let mint: AccountInfo = todo!(); -/// # let owner: Pubkey = todo!(); -/// # let compressible: CompressibleParamsCpi = todo!(); -/// CreateCTokenAccountCpi { +/// # fn example( +/// # payer: AccountInfo, +/// # account: AccountInfo, +/// # mint: AccountInfo, +/// # owner: Pubkey, +/// # compressible: CompressibleParamsCpi, +/// # ) -> Result<(), solana_program_error::ProgramError> { +/// CreateCTokenAccountCpi { /// payer, /// account, /// mint, /// owner, /// compressible: Some(compressible), /// } -/// .invoke()?; -/// # Ok::<(), solana_program_error::ProgramError>(()) +/// .invoke()?; +/// # Ok(()) +/// # } /// ``` Also applies to: 106-125 </blockquote></details> <details> <summary>sdk-libs/ctoken-sdk/src/ctoken/mint_to.rs (1)</summary><blockquote> `54-83`: **Fix doc example import path for `CompressedMintWithContext`.** The doc example (line 57) imports `CompressedMintWithContext` from `light_ctoken_sdk::ctoken`, but this type is not re-exported there. The actual import path is `light_ctoken_interface::instructions::mint_action::CompressedMintWithContext` (as shown in the implementation at line 5). Update the example to use the correct import path or ensure the type is properly re-exported in `light_ctoken_sdk::ctoken::mod.rs`. </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
sdk-libs/ctoken-sdk/src/compressed_token/v2/update_compressed_mint/instruction.rs
Show resolved
Hide resolved
3 participants
Summary by CodeRabbit
Refactor
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.