Merged
Conversation
Contributor
📝 WalkthroughWalkthroughReplace many light_sdk usages with a new framework‑agnostic light_account surface; add AccountInfo/AccountMeta traits, generic CPI builders/invoke utilities, LightConfig/state APIs, compression/decompression processors, Pinocchio-specific code paths, and extensive crate/API reorganization and Cargo dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Program
participant LightSys as Light System Program
participant Trees as Merkle Trees/Queues
participant Rent as Rent Sponsor
Client->>Program: submit compress instruction (proof + remaining_accounts)
Program->>Program: validate accounts (ValidatedPdaContext via AccountInfoTrait)
Program->>Program: prepare per‑PDA CompressedAccountInfo
alt any non-compressible found
Program-->>Client: idempotent skip/ok
else all compressible
Program->>LightSys: invoke_light_system_program(CpiAccountsTrait + LightCpi)
LightSys->>Trees: write addresses/state
Trees-->>LightSys: ack
LightSys-->>Program: success
Program->>Rent: close PDAs / transfer lamports (via AccountInfoTrait)
Rent-->>Program: ack
Program-->>Client: success
end
sequenceDiagram
autonumber
participant Client
participant Program
participant LightSys as Light System Program
participant Token as Light Token Program
participant Trees as Merkle Trees/Queues
Client->>Program: submit decompress instruction (proof + remaining_accounts)
Program->>Program: validate accounts (ValidatedPdaContext)
Program->>Program: unpack PDAs, build in_token_data if token present
Program->>LightSys: invoke_light_system_program(write compressed infos)
alt token accounts present
Program->>Token: invoke token CPI (Create ATA / Create Token Account / Transfer2)
Token-->>Program: success
end
LightSys-->>Program: success
Program-->>Client: success
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
sdk-libs/sdk/src/error.rs (1)
241-243: 🧹 Nitpick | 🔵 TrivialMissing error code 16030 creates a gap in the error code sequence.
The error codes jump from 16029 (
MetaCloseInputIsNone) to 16031 (CpiAccountsIndexOutOfBounds). While not a functional issue, this gap suggests either a removed variant or an assignment oversight. Keeping codes contiguous aids documentation and debugging.If the gap is intentional (e.g., a reserved code or removed variant), a brief comment would clarify. Otherwise, new variants could fill this slot.
sdk-libs/macros/src/light_pdas/program/seed_codegen.rs (1)
102-118: 🧹 Nitpick | 🔵 TrivialConsider unifying owner_seeds program_id handling for consistency.
There's an asymmetry in how the program ID is sourced for owner seeds:
Condition Program ID source owner_parameters.is_empty()Caller-provided _program_idparameter!owner_parameters.is_empty()crate::LIGHT_CPI_SIGNER.program_idThis means the generated function signature and behavior differ based on whether other parameters exist. While this may be intentional to allow flexibility when no other inputs are needed, it creates cognitive overhead for consumers who need to understand when they must pass a program ID versus when it's derived automatically.
If the intent is to always use the CPI signer's program ID (matching the PDA and token seed functions above), the empty-params branch could also use
LIGHT_CPI_SIGNER.program_idand simply not expose a_program_idparameter.♻️ Optional: Unify to always use LIGHT_CPI_SIGNER
let (fn_params, fn_body) = if owner_parameters.is_empty() { ( - quote! { _program_id: &solana_pubkey::Pubkey }, + quote! {}, generate_seed_derivation_body( &owner_seed_expressions, - quote! { _program_id }, + quote! { &solana_pubkey::Pubkey::from(crate::LIGHT_CPI_SIGNER.program_id) }, ), ) } else {Note: This would be a breaking change if existing callers rely on passing a custom program ID.
sdk-libs/compressed-token-sdk/src/compressed_token/v2/decompress_full.rs (1)
353-394:⚠️ Potential issue | 🟠 MajorAdd explicit error when
cpi_contextis set without thecpi-contextfeature.
DecompressFullAccountscurrently silently ignorescpi_contextin non-feature builds, risking missing account metas at runtime.CompressAndCloseAccountsalready implements this guard—mirror it here for consistency.🔧 Proposed fix
#[cfg(not(feature = "cpi-context"))] let config = { + if self.cpi_context.is_some() { + return Err(LightSdkTypesError::InvalidInstructionData); + } let mut config = SystemAccountMetaConfig::default(); config.self_program = self.self_program; config };sdk-libs/client/src/interface/initialize_config.rs (1)
83-97: 🧹 Nitpick | 🔵 TrivialConsider renaming
config_bumptoconfig_versionfor clarity.The field
config_bump(Line 38, 83-86) is used as a version seed for PDA derivation (Line 94), not as a PDA bump seed. The actual bump is discarded fromfind_program_address. This naming could confuse future maintainers.The PDA derivation logic itself is correct and aligns with the updated
LIGHT_CONFIG_SEEDpattern used across the codebase.♻️ Suggested naming improvement
pub struct InitializeRentFreeConfig { program_id: Pubkey, fee_payer: Pubkey, program_data_pda: Pubkey, authority: Option<Pubkey>, rent_sponsor: Pubkey, compression_authority: Pubkey, rent_config: light_compressible::rent::RentConfig, write_top_up: u32, address_space: Vec<Pubkey>, - config_bump: u8, + config_version: u8, }And update the builder method and usage accordingly.
forester/src/compressible/pda/compressor.rs (1)
82-100:⚠️ Potential issue | 🟠 MajorGuard against short config data before slicing.
config_account.data[DISCRIMINATOR_LEN..]will panic if the account is uninitialized or has malformed data. Add a length check to return a structured error instead of crashing the process.💡 Suggested fix
- // LightConfig has 8-byte discriminator prefix, skip it for deserialization - let config = LightConfig::try_from_slice(&config_account.data[DISCRIMINATOR_LEN..]) + // LightConfig has 8-byte discriminator prefix, skip it for deserialization + if config_account.data.len() < DISCRIMINATOR_LEN { + return Err(anyhow::anyhow!( + "Config account data too short: {} bytes", + config_account.data.len() + )); + } + let config = LightConfig::try_from_slice(&config_account.data[DISCRIMINATOR_LEN..]) .map_err(|e| anyhow::anyhow!("Failed to deserialize config: {:?}", e))?;
🤖 Fix all issues with AI agents
In `@program-libs/account-checks/src/account_info/pinocchio.rs`:
- Around line 400-402: The hardcoded generic bound
invoke_signed_with_bounds::<64> should be explained or made configurable: either
add a concise comment near the call (next to
pinocchio::cpi::invoke_signed_with_bounds::<64>, instruction, info_refs,
signers) stating why 64 was chosen (e.g., typical token CPI needs and safety vs
Solana's ~120-account limit), or change the API to accept a configurable bound
(e.g., propagate a generic or const parameter named ACCOUNT_BOUND or a function
parameter) and replace the literal 64 with that symbol so callers can override
it; update documentation/comments to reflect the new configurable parameter or
rationale.
In `@program-libs/account-checks/src/error.rs`:
- Around line 37-42: The documented error range (20000-20015) is out of sync
with the enum variants FailedSysvarAccess and ArithmeticOverflow which occupy
20016 and 20017; either expand the documented range in docs/ERRORS.md to include
20016–20017 and list the two new variants (FailedSysvarAccess,
ArithmeticOverflow) with their codes, or keep docs unchanged and refactor the
enum so those variants map into the published range (e.g., renumber or
consolidate with existing codes) and update any match/serialization logic that
relies on numeric discriminants (refer to the enum/error definitions:
FailedSysvarAccess, ArithmeticOverflow, and PinocchioProgramError) so external
consumers validating against the specification remain consistent.
In `@program-libs/account-checks/src/lib.rs`:
- Around line 19-29: The crate-level documentation table is missing entries for
the newly exported items; update the top-level docs to include brief
descriptions for close_account, AccountMetaTrait, and CpiMeta (in addition to
existing entries for AccountInfoTrait, AccountIterator, AccountError, checks,
discriminator, packed_accounts). Edit the crate doc comment to add table rows or
list items that name each export (close_account function, trait
AccountMetaTrait, and type CpiMeta) with one-line purpose/usage descriptions so
the public surface is fully discoverable.
In `@sdk-libs/macros/src/light_pdas/account/derive.rs`:
- Around line 344-348: The method compression_info_mut_opt currently panics for
V2 LightAccount types; update the API docs/comments for the function (and the
impl block containing compression_info_mut_opt) to clearly state it always
panics for V2 types, explain the reason (V2 uses non-Option CompressionInfo),
and indicate callers should use compression_info_mut instead; optionally mark
compression_info_mut_opt as deprecated in the surrounding documentation/comments
to warn users and point them to compression_info_mut for safe usage.
In `@sdk-libs/macros/src/light_pdas/accounts/token.rs`:
- Around line 98-106: Update the module header docs to state that PDAs are
derived using crate::LIGHT_CPI_SIGNER.program_id (not crate::ID); edit the
top-of-file comment to mention the requirement that programs using the Light
Protocol macros must define the Pubkey constant LIGHT_CPI_SIGNER and the program
ID via declare_id!, and note that functions like
solana_pubkey::Pubkey::find_program_address in token.rs now use LIGHT_CPI_SIGNER
for PDA signing.
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 462-483: The pinocchio size-check in
generate_size_validation_pinocchio currently uses a shorter assert! message;
update the compile-time assertion string for COMPRESSED_SIZE to match the
standard path's wording and support contact (include "send a message to
team@lightprotocol.com" and the same phrasing that references the account type)
so both paths produce consistent error text for oversized compressed accounts
(refer to generate_size_validation_pinocchio and how the standard path
constructs the panic message with stringify!(`#qualified_type`) /
COMPRESSED_INIT_SPACE).
In `@sdk-libs/macros/src/light_pdas/program/derive_light_program.rs`:
- Around line 575-578: The function build_intermediate_types currently has an
unused parameter _crate_ctx; either remove it from the signature and all call
sites (e.g., the call from derive_light_program_impl) to avoid dead parameters,
or keep it and add a short comment explaining it's reserved for future use;
update the function signature, all references (including the call in
derive_light_program_impl), and any related tests/uses accordingly so the code
compiles without an unused parameter warning.
In `@sdk-libs/sdk-types/Cargo.toml`:
- Around line 42-47: The allow-list under the table lints.rust.unexpected_cfgs
currently contains unused feature cfg entries 'cfg(feature, values("frozen-abi",
"no-entrypoint"))'; either remove these two feature checks from the check-cfg
array or declare them as real features in this crate (e.g., add them to the
Cargo.toml [features] section) so the allow-list matches actual usage—update the
check-cfg to only include real cfgs or add corresponding feature definitions for
"frozen-abi" and "no-entrypoint".
In `@sdk-libs/sdk-types/src/interface/account/token_seeds.rs`:
- Around line 94-100: The current mapping casts ext.is_ata from bool to u8 with
`as`, which can obscure intent and diverges from
CompressedOnlyExtensionInstructionData.is_ata (bool); update the transformation
in the closure that builds ExtensionStruct::CompressedOnly so that is_ata is
explicitly converted (e.g., set is_ata = if ext.is_ata { 1u8 } else { 0u8 }) or,
alternatively, align types by changing CompressedOnlyExtension.is_ata to bool
and map accordingly; modify the code around the extension closure that
constructs CompressedOnlyExtension to use the chosen explicit conversion so the
intent and type consistency are clear.
In `@sdk-libs/sdk-types/src/interface/cpi/account.rs`:
- Line 140: Replace the panic-causing assertion with a fallible check that
returns a proper error: instead of
assert_eq!(cpi_accounts.system_accounts_end_offset(), index) use an if-check on
cpi_accounts.system_accounts_end_offset() != index and return
Err(LightSdkTypesError::InvalidInstructionData) (or an appropriate variant) from
the surrounding function (ensure the function's signature returns a Result).
This touches the comparison around cpi_accounts.system_accounts_end_offset() and
the call site that currently assumes the assert, so add the conditional
early-return and adjust the function return type if needed.
In `@sdk-libs/sdk-types/src/interface/cpi/create_mints.rs`:
- Around line 777-797: The get_output_queue_next_index function uses a hardcoded
NEXT_INDEX_OFFSET (288), which is brittle; either import and use the canonical
layout constants (e.g., replace the magic 288 with BatchedQueueMetadata::LEN +
QueueBatches::HEADER_LEN or the appropriate constants from
light-batched-merkle-tree) so the offset is derived from types rather than
hardcoded, or, if that dependency cannot be added, add a compile-time/layout
assertion (const_assert! or unit tests) that verifies the expected lengths
(BatchedQueueMetadata::LEN and the six u64 fields / QueueBatches header) match
the values used here and update the NEXT_INDEX_OFFSET to be computed from those
constants instead of a literal 288; update get_output_queue_next_index to use
the computed constant and keep the existing bounds check/error handling.
In `@sdk-libs/sdk-types/src/interface/cpi/create_token_accounts.rs`:
- Around line 29-34: The constants DEFAULT_PRE_PAY_NUM_EPOCHS,
DEFAULT_LAMPORTS_PER_WRITE, and DEFAULT_TOKEN_ACCOUNT_VERSION are
protocol-specific magic numbers; update their declarations to include brief
comments referencing the authoritative source (e.g., the cToken program docs,
protocol spec, or commit/issue that defined them) or add a URL to the exact
doc/section that defines "16 epochs", "766 lamports/write", and "version 3" so
future maintainers can trace and validate these defaults.
- Around line 131-156: The bump extraction in invoke_signed silently defaults to
0 when seeds are missing or malformed; change invoke_signed to validate that
seeds is non-empty and that seeds.last() has at least one byte, and if not
return an explicit error (e.g. a new or existing LightSdkTypesError variant like
InvalidSeeds) instead of unwrap_or(0); update the code paths that build
CompressToPubkey (the bump, program_id, seeds creation) to run only after this
validation so you don't accidentally craft a wrong PDA, and add a brief comment
in invoke_signed documenting the required seed layout.
In `@sdk-libs/sdk-types/src/interface/cpi/impls.rs`:
- Around line 24-32: The three helper methods write_to_cpi_context_first,
write_to_cpi_context_set, and execute_with_cpi_context currently call
self.method(), which causes infinite recursion; change each call to use UFCS so
the intended inherent method is invoked explicitly (e.g., call <Self as
InstructionDataInvokeCpiWithReadOnly>::write_to_cpi_context_first(self) inside
the impl InstructionDataInvokeCpiWithReadOnly block and analogously use <Self as
InstructionDataInvokeCpiWithAccountInfo>::... inside impl
InstructionDataInvokeCpiWithAccountInfo), applying the same UFCS pattern to all
three methods so the compiler will fail fast if the inherent implementation is
missing.
In `@sdk-libs/sdk-types/src/interface/cpi/instruction.rs`:
- Around line 24-63: The trait currently exposes CPI-context methods
unconditionally; gate all CPI-context-related trait
methods—write_to_cpi_context_first, write_to_cpi_context_set,
execute_with_cpi_context, get_with_cpi_context, get_cpi_context, and
has_read_only_accounts—behind #[cfg(feature = "cpi-context")] so they are only
compiled when the feature is enabled, and update Cargo.toml so the cpi-context
dependency requires v2 (cpi-context = ["v2"]) to enforce the documented v2
requirement at compile time.
In `@sdk-libs/sdk-types/src/interface/program/compression/processor.rs`:
- Around line 19-23: The constants FEE_PAYER_INDEX, CONFIG_INDEX, and
RENT_SPONSOR_INDEX define fixed positions in remaining_accounts but may not
match the SDK's documented account layout; update these constants to match the
canonical account order used by the instruction builder (and SDK docs) and add a
doc comment above them referencing the authoritative source (e.g., the
instruction builder function or SDK documentation path) so future maintainers
can verify the mapping; ensure all uses of remaining_accounts in functions that
consume these indices (lookups or slices) remain consistent with the corrected
values.
In
`@sdk-libs/sdk-types/src/interface/program/decompression/create_token_account.rs`:
- Around line 123-137: The bump extraction currently takes the first byte of the
last seed slice which can silently accept multi-byte seeds; update the logic
that produces `bump` (the expression using `signer_seeds.last().and_then(|s|
s.first().copied())`) to explicitly validate the last seed is exactly one byte
long and return `LightSdkTypesError::InvalidSeeds` if not, then proceed to build
`CompressToPubkey { bump, program_id: *program_id, seeds: seeds_without_bump }`
as before; ensure the validation uses the same `signer_seeds` variable and does
not alter how `seeds_without_bump` is constructed.
- Around line 59-187: Both builders (build_create_ata_instruction and
build_create_token_account_instruction) currently hardcode the system program
pubkey as [0u8; 32]; update both function signatures to accept system_program:
&[u8; 32], replace the literal [0u8; 32] CpiMeta entries with *system_program,
and propagate the new parameter to all call sites (e.g., in token.rs) so the
real system program ID is passed through (match the pattern used in
sdk-libs/sdk-types/src/interface/cpi/create_token_accounts.rs).
In `@sdk-libs/sdk-types/src/interface/program/decompression/pda.rs`:
- Around line 61-63: The local type alias Data<const N: usize, P> (which expands
to <<P as PackedLightAccountVariantTrait<N>>::Unpacked as
LightAccountVariantTrait<N>>::Data) should be moved out of the function body to
module scope so it is conventional, documentable, and visible to other items;
create a module-level type alias (adjusting visibility as needed, e.g.,
pub(crate) or pub) named Data<const N: usize, P> with the same expansion and
replace the local alias references in the function with the module-level name,
keeping the same generic parameters and trait bounds
(PackedLightAccountVariantTrait and LightAccountVariantTrait) so callers still
compile.
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs`:
- Around line 555-576: The code incorrectly merges all PDA seeds into one
signer; update the invoke_signed call so each token account PDA is a separate
signer: for the branch handling non-empty token_seeds (symbols: token_seeds,
signer_seed_refs, AI::invoke_cpi, transfer2_data, account_metas), build a Vec of
signer seed slices where each element is the slice of seeds for one PDA (e.g.,
Vec<Vec<&[u8]>> or Vec<&[&[u8]]> depending on types), convert that to a
slice-of-slices (e.g., &[&[&[u8]]]) and pass that to
AI::invoke_cpi/invoke_signed instead of concatenating all seeds into a single
signer_seed_refs.as_slice(). Ensure the outer vector length equals the number of
PDAs so each PDA signs independently.
- Around line 544-547: The loop that sets account_metas[data.owner as usize +
transfer2_packed_start].is_signer = true risks panicking when the computed index
is out of bounds; update the loop over in_token_data to perform bounds checking
(e.g., compute idx = data.owner as usize + transfer2_packed_start and use
account_metas.get_mut(idx) or an explicit range check) and handle the None case
by returning an error or skipping per the module's convention (consistent with
token_seeds.rs/token.rs), referencing the symbols account_metas, in_token_data,
and transfer2_packed_start.
In `@sdk-libs/sdk-types/src/interface/program/decompression/token.rs`:
- Around line 61-69: The idempotency check uses a magic number 108; extract that
magic number into a clearly named constant (e.g., SPL_TOKEN_ACCOUNT_STATE_OFFSET
or TOKEN_STATE_OFFSET) and replace occurrences in the function where
STATE_OFFSET and related logic live (references: STATE_OFFSET,
is_already_initialized, token_account_info) so the intent is explicit and the
offset can be updated centrally; add a short comment documenting the SPL Token
state byte meaning (0=Uninitialized, 1=Initialized, 2=Frozen) next to the new
constant.
In `@sdk-libs/sdk-types/src/pack_accounts.rs`:
- Around line 23-35: PackedAccounts currently uses next_index: u8 and
unconditionally does self.next_index += 1 which will overflow; either document
the 255-account limit in PackedAccounts' struct docstring or, preferably, add
overflow protection: in any method that increments next_index (where
self.next_index += 1 is used) replace the unconditional increment with a checked
add (e.g., self.next_index.checked_add(1)) and return an error (e.g.,
LightSdkTypesError::AccountLimitExceeded) when None is returned; update the
method signatures that perform the increment to return Result<...,
LightSdkTypesError> and adjust callers to propagate the error, keeping map and
other logic unchanged.
In `@sdk-libs/sdk/src/error.rs`:
- Around line 132-157: The From<LightSdkError> for LightSdkTypesError conversion
currently uses a catch-all arm that maps many distinct LightSdkError variants to
LightSdkTypesError::ConstraintViolation, losing context; update the match in the
impl for From<LightSdkError> for LightSdkTypesError to avoid the lossy "_"
arm—either replace it with a fallback that preserves an error code (e.g., map
remaining errors to LightSdkTypesError::ProgramError(u32) by extracting/casting
the original error code similar to the existing LightSdkError::ProgramError(e)
handling) or, if preservation isn't possible, add a clear doc comment on the
impl explaining this is intentionally lossy; target the match in the impl and
the "_" arm to make the change.
- Around line 198-206: The match arm that maps multiple LightSdkTypesError
variants (AccountDataTooSmall, InvalidInstructionData, InvalidSeeds, CpiFailed,
NotEnoughAccountKeys, MissingRequiredSignature) to a single
LightSdkError::ConstraintViolation loses diagnostic detail; update the mapping
in the match over LightSdkTypesError so each distinct variant maps to a specific
LightSdkError variant (add new LightSdkError variants if needed or reuse
existing ones like ExpectedData, DeserializationError, PdaMismatch, CpiError,
MissingAccounts, MissingSignature), adjust the enum LightSdkError accordingly
(add constructors/variants) and update all places that construct or match
LightSdkError to handle the new variants so callers receive precise error
information instead of a generic ConstraintViolation.
In `@sdk-libs/sdk/src/instruction/packed_accounts.rs`:
- Around line 55-62: The methods add_system_accounts and add_system_accounts_v2
currently return crate::error::Result<()> but always return Ok(()) after calling
get_light_system_account_metas / get_light_system_account_metas_v2, so remove
the unnecessary Result wrapper from their signatures and return type (make them
return unit) and update any call sites accordingly, or if you intend future
error propagation keep the Result but add a TODO doc comment explaining the
planned extensibility; reference the functions add_system_accounts,
add_system_accounts_v2, get_light_system_account_metas and
get_light_system_account_metas_v2 when making the change.
In `@sdk-libs/token-sdk/Cargo.toml`:
- Line 38: The anchor feature in the crate's Cargo.toml must propagate the
light-account crate's optional feature; update the "anchor" feature definition
to include "light-account/anchor" alongside the other light-* feature entries so
that enabling the workspace feature "anchor" also activates the
light-account::anchor feature.
Comment on lines
+400
to
+402
| pinocchio::cpi::invoke_signed_with_bounds::<64>(&instruction, &info_refs, &signers) | ||
| .map_err(AccountError::from) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
rg -n "invoke_signed_with_bounds" --type rust -C 2Repository: Lightprotocol/light-protocol
Length of output: 1274
🏁 Script executed:
# Also check for pinocchio::cpi imports and documentation
fd -t f "pinocchio" | head -20Repository: Lightprotocol/light-protocol
Length of output: 129
🏁 Script executed:
# Look at the actual file context
cat -n program-libs/account-checks/src/account_info/pinocchio.rs | sed -n '390,410p'Repository: Lightprotocol/light-protocol
Length of output: 671
🏁 Script executed:
# Search for pinocchio documentation or README
find . -type f -name "*.md" | xargs grep -l "pinocchio" 2>/dev/null | head -10Repository: Lightprotocol/light-protocol
Length of output: 410
🏁 Script executed:
# Look at the full function to understand its purpose
cat -n program-libs/account-checks/src/account_info/pinocchio.rs | sed -n '350,402p'Repository: Lightprotocol/light-protocol
Length of output: 2625
🏁 Script executed:
# Check if pinocchio is documented anywhere in the codebase
rg -n "invoke_signed_with_bounds" --type rust -B10 -A5 | head -50Repository: Lightprotocol/light-protocol
Length of output: 3245
🏁 Script executed:
# Search for any other CPI invocations or bounds constants
rg "invoke_signed" --type rust -C 1Repository: Lightprotocol/light-protocol
Length of output: 50384
🏁 Script executed:
# Check account-checks documentation for pinocchio guidance
cat -n program-libs/account-checks/CLAUDE.md | head -100Repository: Lightprotocol/light-protocol
Length of output: 4879
🏁 Script executed:
# Look for any pinocchio-related documentation or comments about bounds
rg "pinocchio|invoke_signed_with_bounds" --type md -C 2Repository: Lightprotocol/light-protocol
Length of output: 50386
🏁 Script executed:
# Search for any configuration or constants related to account bounds
rg "64|bounds|max.*account" program-libs/account-checks/src/ --type rust -C 1Repository: Lightprotocol/light-protocol
Length of output: 10219
Document why 64 is the chosen bound or make it configurable.
The invoke_signed_with_bounds::<64> constant is hardcoded with no explanation. While 64 accounts is practical for most token operations (well within Solana's ~120-account transaction limit), it lacks documentation of the rationale and is inflexible if future CPIs exceed this limit. Either document why 64 was selected as the bound, or make it configurable via a generic parameter if broader applicability is needed.
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/account_info/pinocchio.rs` around lines 400 -
402, The hardcoded generic bound invoke_signed_with_bounds::<64> should be
explained or made configurable: either add a concise comment near the call (next
to pinocchio::cpi::invoke_signed_with_bounds::<64>, instruction, info_refs,
signers) stating why 64 was chosen (e.g., typical token CPI needs and safety vs
Solana's ~120-account limit), or change the API to accept a configurable bound
(e.g., propagate a generic or const parameter named ACCOUNT_BOUND or a function
parameter) and replace the literal 64 with that symbol so callers can override
it; update documentation/comments to reflect the new configurable parameter or
rationale.
Comment on lines
+37
to
+42
| #[error("Failed to access sysvar.")] | ||
| FailedSysvarAccess, | ||
| #[error("Pinocchio program error with code: {0}")] | ||
| PinocchioProgramError(u32), | ||
| #[error("Arithmetic overflow.")] | ||
| ArithmeticOverflow, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get full error.rs file content
cat -n program-libs/account-checks/src/error.rsRepository: Lightprotocol/light-protocol
Length of output: 5821
🏁 Script executed:
#!/bin/bash
# Search for CLAUDE.md and documentation files in account-checks
find program-libs/account-checks -name "CLAUDE.md" -o -name "ERRORS.md" -o -name "*.md" | head -20Repository: Lightprotocol/light-protocol
Length of output: 494
🏁 Script executed:
#!/bin/bash
# Search for error code definitions in docs
rg -n "20000|20015|20016|12006|12021" program-libs/account-checks/docs/ --type mdRepository: Lightprotocol/light-protocol
Length of output: 1713
🏁 Script executed:
#!/bin/bash
# Check CLAUDE.md files for context on error range flexibility
cat program-libs/account-checks/CLAUDE.md
echo "---"
cat program-libs/account-checks/docs/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 6149
🏁 Script executed:
#!/bin/bash
# Check ERRORS.md for full details about the range
cat program-libs/account-checks/docs/ERRORS.mdRepository: Lightprotocol/light-protocol
Length of output: 6750
Update documentation to reflect extended error code range (20000-20017).
The new FailedSysvarAccess (20016) and ArithmeticOverflow (20017) variants extend beyond the documented range of 20000-20015 in docs/ERRORS.md. Update the documentation to either expand the official range or refactor these variants to fit within the documented boundary, as external consumers may validate error codes against the published specification.
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/error.rs` around lines 37 - 42, The
documented error range (20000-20015) is out of sync with the enum variants
FailedSysvarAccess and ArithmeticOverflow which occupy 20016 and 20017; either
expand the documented range in docs/ERRORS.md to include 20016–20017 and list
the two new variants (FailedSysvarAccess, ArithmeticOverflow) with their codes,
or keep docs unchanged and refactor the enum so those variants map into the
published range (e.g., renumber or consolidate with existing codes) and update
any match/serialization logic that relies on numeric discriminants (refer to the
enum/error definitions: FailedSysvarAccess, ArithmeticOverflow, and
PinocchioProgramError) so external consumers validating against the
specification remain consistent.
Comment on lines
+19
to
+29
| pub mod close_account; | ||
| pub mod discriminator; | ||
| pub mod error; | ||
| pub mod packed_accounts; | ||
|
|
||
| pub use account_info::account_info_trait::AccountInfoTrait; | ||
| pub use account_info::{ | ||
| account_info_trait::{AccountInfoTrait, CpiMeta}, | ||
| account_meta_trait::AccountMetaTrait, | ||
| }; | ||
| pub use account_iterator::AccountIterator; | ||
| pub use close_account::close_account; |
Contributor
There was a problem hiding this comment.
Update crate docs for the new public exports.
Line 19-29 exposes close_account, AccountMetaTrait, and CpiMeta, but the crate-level table doesn’t list them. Please update the table so the public surface stays discoverable.
✏️ Suggested doc update
//! | Module | Description |
//! |--------|-------------|
//! | [`AccountInfoTrait`] | Trait abstraction over Solana account info types |
+//! | [`AccountMetaTrait`] | Trait abstraction over account meta types |
+//! | [`CpiMeta`] | CPI metadata helper for account metas |
//! | [`AccountIterator`] | Iterates over a slice of accounts by index |
//! | [`AccountError`] | Error type for account validation failures |
//! | [`checks`] | Owner, signer, writable, and rent-exempt checks |
//! | [`discriminator`] | Account discriminator constants and validation |
//! | [`packed_accounts`] | Packed account struct deserialization |
+//! | [`close_account`] | Helper for closing accounts via AccountInfoTrait |As per coding guidelines, "Document public modules and traits (AccountInfoTrait, AccountIterator, AccountError, checks, discriminator, packed_accounts) with their purpose and usage".
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/lib.rs` around lines 19 - 29, The crate-level
documentation table is missing entries for the newly exported items; update the
top-level docs to include brief descriptions for close_account,
AccountMetaTrait, and CpiMeta (in addition to existing entries for
AccountInfoTrait, AccountIterator, AccountError, checks, discriminator,
packed_accounts). Edit the crate doc comment to add table rows or list items
that name each export (close_account function, trait AccountMetaTrait, and type
CpiMeta) with one-line purpose/usage descriptions so the public surface is fully
discoverable.
Comment on lines
344
to
348
| fn compression_info_mut_opt(&mut self) -> &mut Option<light_account::CompressionInfo> { | ||
| // V2 types use non-Option CompressionInfo, so this can't return a reference | ||
| // This method is only used by V1 code paths that expect Option<CompressionInfo> | ||
| panic!("compression_info_mut_opt not supported for LightAccount types (use compression_info_mut instead)") | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
compression_info_mut_opt panics - document this clearly.
The generated compression_info_mut_opt method panics because V2 types use non-Option CompressionInfo. This is acceptable for V1 compatibility, but the panic message should be more prominent in documentation to prevent confusion.
Consider adding deprecation warning
+ #[deprecated(since = "2.0.0", note = "V2 types use non-Option CompressionInfo; use compression_info_mut() instead")]
fn compression_info_mut_opt(&mut self) -> &mut Option<light_account::CompressionInfo> {🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/account/derive.rs` around lines 344 - 348, The
method compression_info_mut_opt currently panics for V2 LightAccount types;
update the API docs/comments for the function (and the impl block containing
compression_info_mut_opt) to clearly state it always panics for V2 types,
explain the reason (V2 uses non-Option CompressionInfo), and indicate callers
should use compression_info_mut instead; optionally mark
compression_info_mut_opt as deprecated in the surrounding documentation/comments
to warn users and point them to compression_info_mut for safe usage.
Comment on lines
+98
to
+106
| let (_, bump) = solana_pubkey::Pubkey::find_program_address(&[], &solana_pubkey::Pubkey::from(crate::LIGHT_CPI_SIGNER.program_id)); | ||
| bump | ||
| }; | ||
| } | ||
| } else { | ||
| quote! { | ||
| let __bump: u8 = { | ||
| let seeds: &[&[u8]] = &[#(#seed_refs),*]; | ||
| let (_, bump) = solana_pubkey::Pubkey::find_program_address(seeds, &crate::ID); | ||
| let (_, bump) = solana_pubkey::Pubkey::find_program_address(seeds, &solana_pubkey::Pubkey::from(crate::LIGHT_CPI_SIGNER.program_id)); |
Contributor
There was a problem hiding this comment.
Update module docs to reflect LIGHT_CPI_SIGNER usage.
Line 98-106 now derives bumps with crate::LIGHT_CPI_SIGNER.program_id, but the module docs still say crate::ID is used for PDA signing. Please update the header comment to match the new behavior.
✏️ Suggested doc update
-//! Programs using `#[light_account(init, token, ...)]` must have a `crate::ID`
-//! constant, which is the standard pattern when using Anchor's `declare_id!` macro.
-//! The generated code passes `&crate::ID` to `CreateTokenAccountCpi::rent_free()`
-//! for PDA signing verification.
+//! Programs using `#[light_account(init, token, ...)]` must define both
+//! `crate::ID` (via `declare_id!`) and `crate::LIGHT_CPI_SIGNER`.
+//! The generated code passes `&crate::LIGHT_CPI_SIGNER.program_id` to
+//! `CreateTokenAccountCpi::rent_free()` for PDA signing verification.As per coding guidelines, "Programs using Light Protocol macros must define LIGHT_CPI_SIGNER: Pubkey constant and ID program ID constant via declare_id! macro".
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/accounts/token.rs` around lines 98 - 106,
Update the module header docs to state that PDAs are derived using
crate::LIGHT_CPI_SIGNER.program_id (not crate::ID); edit the top-of-file comment
to mention the requirement that programs using the Light Protocol macros must
define the Pubkey constant LIGHT_CPI_SIGNER and the program ID via declare_id!,
and note that functions like solana_pubkey::Pubkey::find_program_address in
token.rs now use LIGHT_CPI_SIGNER for PDA signing.
Comment on lines
+23
to
+35
| #[derive(Debug)] | ||
| pub struct PackedAccounts<AM: AccountMetaTrait> { | ||
| /// Accounts that must come before system accounts (e.g., signers, fee payer). | ||
| pub pre_accounts: Vec<AM>, | ||
| /// Light system program accounts (authority, programs, trees, queues). | ||
| system_accounts: Vec<AM>, | ||
| /// Next available index for packed accounts. | ||
| next_index: u8, | ||
| /// Map of pubkey bytes to (index, AccountMeta) for deduplication and index tracking. | ||
| map: HashMap<[u8; 32], (u8, AM)>, | ||
| /// Field to sanity check | ||
| system_accounts_set: bool, | ||
| } |
Contributor
There was a problem hiding this comment.
Consider documenting the 255-account limit or adding overflow protection.
The next_index: u8 field limits packed accounts to 255 entries. While this is likely sufficient for most use cases, the current implementation at line 109 (self.next_index += 1) will panic on overflow in debug mode or wrap in release mode.
🛡️ Proposed fix: Add overflow check or document the limit
Option 1: Document the limit in the struct documentation.
Option 2: Add a checked increment:
None => {
let index = self.next_index;
- self.next_index += 1;
+ self.next_index = self.next_index
+ .checked_add(1)
+ .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?;
self.map
.insert(bytes, (index, AM::new(pubkey, is_signer, is_writable)));
index
}This would require changing the return type to Result<u8, LightSdkTypesError>.
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/pack_accounts.rs` around lines 23 - 35, PackedAccounts
currently uses next_index: u8 and unconditionally does self.next_index += 1
which will overflow; either document the 255-account limit in PackedAccounts'
struct docstring or, preferably, add overflow protection: in any method that
increments next_index (where self.next_index += 1 is used) replace the
unconditional increment with a checked add (e.g.,
self.next_index.checked_add(1)) and return an error (e.g.,
LightSdkTypesError::AccountLimitExceeded) when None is returned; update the
method signatures that perform the increment to return Result<...,
LightSdkTypesError> and adjust callers to propagate the error, keeping map and
other logic unchanged.
Comment on lines
+132
to
+157
| /// Convert from SDK's LightSdkError to LightSdkTypesError. | ||
| /// This allows SDK error types to be used where types error types are expected | ||
| /// (e.g., in trait impls for LightPreInit, LightFinalize, AccountMetasVec). | ||
| impl From<LightSdkError> for LightSdkTypesError { | ||
| fn from(e: LightSdkError) -> Self { | ||
| match e { | ||
| LightSdkError::ConstraintViolation => LightSdkTypesError::ConstraintViolation, | ||
| LightSdkError::Borsh => LightSdkTypesError::Borsh, | ||
| LightSdkError::AccountError(e) => LightSdkTypesError::AccountError(e), | ||
| LightSdkError::Hasher(e) => LightSdkTypesError::Hasher(e), | ||
| LightSdkError::MissingCompressionInfo => LightSdkTypesError::MissingCompressionInfo, | ||
| LightSdkError::InvalidRentSponsor => LightSdkTypesError::InvalidRentSponsor, | ||
| LightSdkError::CpiAccountsIndexOutOfBounds(i) => { | ||
| LightSdkTypesError::CpiAccountsIndexOutOfBounds(i) | ||
| } | ||
| LightSdkError::ReadOnlyAccountsNotSupportedInCpiContext => { | ||
| LightSdkTypesError::ReadOnlyAccountsNotSupportedInCpiContext | ||
| } | ||
| LightSdkError::CompressedAccountError(e) => { | ||
| LightSdkTypesError::CompressedAccountError(e) | ||
| } | ||
| LightSdkError::ProgramError(e) => LightSdkTypesError::ProgramError(u64::from(e) as u32), | ||
| _ => LightSdkTypesError::ConstraintViolation, | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Lossy catch-all conversion reduces debuggability.
The catch-all at line 154 maps roughly 30+ distinct LightSdkError variants to a single LightSdkTypesError::ConstraintViolation. When errors flow through this conversion, you lose the original error context, making production debugging significantly harder.
If LightSdkTypesError intentionally has a smaller surface area, consider either:
- Adding a
ProgramError(u32)fallback that preserves the original error code - Documenting explicitly in the doc comment that this is a lossy conversion
💡 Suggested approach to preserve error codes
- _ => LightSdkTypesError::ConstraintViolation,
+ other => {
+ // Preserve the error code even if variant doesn't have a direct mapping
+ let code: u32 = other.into();
+ LightSdkTypesError::ProgramError(code)
+ }🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/error.rs` around lines 132 - 157, The From<LightSdkError>
for LightSdkTypesError conversion currently uses a catch-all arm that maps many
distinct LightSdkError variants to LightSdkTypesError::ConstraintViolation,
losing context; update the match in the impl for From<LightSdkError> for
LightSdkTypesError to avoid the lossy "_" arm—either replace it with a fallback
that preserves an error code (e.g., map remaining errors to
LightSdkTypesError::ProgramError(u32) by extracting/casting the original error
code similar to the existing LightSdkError::ProgramError(e) handling) or, if
preservation isn't possible, add a clear doc comment on the impl explaining this
is intentionally lossy; target the match in the impl and the "_" arm to make the
change.
Comment on lines
+198
to
+206
| LightSdkTypesError::AccountDataTooSmall | ||
| | LightSdkTypesError::InvalidInstructionData | ||
| | LightSdkTypesError::InvalidSeeds | ||
| | LightSdkTypesError::CpiFailed | ||
| | LightSdkTypesError::NotEnoughAccountKeys | ||
| | LightSdkTypesError::MissingRequiredSignature => LightSdkError::ConstraintViolation, | ||
| LightSdkTypesError::ProgramError(code) => { | ||
| LightSdkError::ProgramError(ProgramError::Custom(code)) | ||
| } |
Contributor
There was a problem hiding this comment.
Collapsing six distinct error types into ConstraintViolation loses critical diagnostic information.
These errors have fundamentally different meanings:
AccountDataTooSmall– buffer/allocation issueInvalidInstructionData– deserialization/parsing failureInvalidSeeds– PDA derivation mismatchCpiFailed– cross-program invocation failureNotEnoughAccountKeys– missing accounts in transactionMissingRequiredSignature– authorization failure
When any of these surface as ConstraintViolation, developers will struggle to diagnose the actual problem. Consider adding corresponding variants to LightSdkError if they don't exist, or at minimum map to a more specific existing variant (e.g., AccountDataTooSmall could map to ExpectedData).
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/error.rs` around lines 198 - 206, The match arm that maps
multiple LightSdkTypesError variants (AccountDataTooSmall,
InvalidInstructionData, InvalidSeeds, CpiFailed, NotEnoughAccountKeys,
MissingRequiredSignature) to a single LightSdkError::ConstraintViolation loses
diagnostic detail; update the mapping in the match over LightSdkTypesError so
each distinct variant maps to a specific LightSdkError variant (add new
LightSdkError variants if needed or reuse existing ones like ExpectedData,
DeserializationError, PdaMismatch, CpiError, MissingAccounts, MissingSignature),
adjust the enum LightSdkError accordingly (add constructors/variants) and update
all places that construct or match LightSdkError to handle the new variants so
callers receive precise error information instead of a generic
ConstraintViolation.
Comment on lines
+55
to
+62
| pub fn add_system_accounts( | ||
| &mut self, | ||
| config: SystemAccountMetaConfig, | ||
| ) -> crate::error::Result<()> { | ||
| self.0 | ||
| .add_system_accounts_raw(get_light_system_account_metas(config)); | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing Result wrapper if errors are not possible.
Both add_system_accounts and add_system_accounts_v2 always return Ok(()). If get_light_system_account_metas and its v2 variant cannot fail, the Result return type adds unnecessary ceremony:
// Current
pub fn add_system_accounts(&mut self, config: SystemAccountMetaConfig) -> crate::error::Result<()>
// Could be simpler if no errors are possible
pub fn add_system_accounts(&mut self, config: SystemAccountMetaConfig)However, if this is intentional for future error handling extensibility, documenting that rationale would be helpful.
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/instruction/packed_accounts.rs` around lines 55 - 62, The
methods add_system_accounts and add_system_accounts_v2 currently return
crate::error::Result<()> but always return Ok(()) after calling
get_light_system_account_metas / get_light_system_account_metas_v2, so remove
the unnecessary Result wrapper from their signatures and return type (make them
return unit) and update any call sites accordingly, or if you intend future
error propagation keep the Result but add a TODO doc comment explaining the
planned extensibility; reference the functions add_system_accounts,
add_system_accounts_v2, get_light_system_account_metas and
get_light_system_account_metas_v2 when making the change.
| light-compressible = { workspace = true } | ||
| light-token-interface = { workspace = true } | ||
| light-sdk = { workspace = true, features = ["v2", "cpi-context"] } | ||
| light-account = { workspace = true } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the light-account Cargo.toml to check available features
find . -path "*/light-account/Cargo.toml" -type f | head -5Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for light-account crate definition and its features
rg 'name = "light-account"' -A 50 | head -100Repository: Lightprotocol/light-protocol
Length of output: 4044
🏁 Script executed:
cat sdk-libs/token-sdk/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 2855
🏁 Script executed:
find sdk-libs/account -name "CLAUDE.md" -o -name "README.md" | xargs catRepository: Lightprotocol/light-protocol
Length of output: 54
Enable anchor feature for light-account dependency when the anchor feature is active.
The light-account crate has an anchor feature that enables light-sdk-types/anchor and anchor-lang support. This feature should be propagated in the anchor feature definition on line 12, alongside the other light-* dependencies, to ensure consistent feature enablement across the SDK. Update the anchor feature to include "light-account/anchor".
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/Cargo.toml` at line 38, The anchor feature in the crate's
Cargo.toml must propagate the light-account crate's optional feature; update the
"anchor" feature definition to include "light-account/anchor" alongside the
other light-* feature entries so that enabling the workspace feature "anchor"
also activates the light-account::anchor feature.
cargo build-sbf in Solana 2.x uses SBF_OUT_PATH environment variable to specify the output directory, not SBF_OUT_DIR. Add SBF_OUT_PATH alongside existing SBF_OUT_DIR for backwards compatibility. This fixes the CI error: "Unable to get file metadata for target/sbpf-solana-solana/release/*.so"
- Rename manual-test -> anchor-manual-test (lib: anchor_manual_test) - Rename manual-test-pinocchio -> pinocchio-manual-test (lib: pinocchio_manual_test) - Fix sdk-v1-native-test lib name collision (sdk_native_test -> sdk_v1_native_test) - Fix sdk-light-token-test lib name (native_ctoken_examples -> sdk_light_token_test) - Add light-token-pinocchio crate for Pinocchio token operations
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@sdk-libs/token-pinocchio/Cargo.toml`:
- Around line 27-28: The dev-dependency entry for light-account-checks includes
an unnecessary "solana" feature; update the Cargo.toml dev-dependencies line for
light-account-checks to remove "solana" from the features array so it only
enables ["test-only", "pinocchio", "std"], ensuring token-pinocchio tests don't
pull in Solana-related dependencies.
In `@sdk-libs/token-pinocchio/src/error.rs`:
- Around line 8-33: Add an explicit integer representation to the
LightTokenError enum by annotating it with #[repr(u32)] (place the attribute
directly above the LightTokenError declaration) so the discriminant values used
when casting to u32/ProgramError::Custom are unambiguous; ensure the enum and
its existing variants (SplInterfaceRequired ... MissingSystemProgram) remain
unchanged otherwise.
In `@sdk-libs/token-pinocchio/src/instruction/approve.rs`:
- Around line 43-46: The instruction data currently uses a 1-byte hardcoded
discriminator (data[0] = 4u8) which is incompatible with Anchor's 8-byte
discriminator layout; replace the manual prefix with Anchor's 8-byte
discriminator for the Approve instruction by using Approve::discriminator() (or
anchor_lang::Discriminator::discriminator(&Approve::discriminator())) and append
self.amount.to_le_bytes() to that 8-byte prefix when building the final data
buffer so the instruction data becomes [8-byte discriminator] + [8-byte amount].
In `@sdk-libs/token-pinocchio/src/instruction/close.rs`:
- Around line 76-137: The invoke_with_seeds function currently truncates
signer_seeds to 2 signers and up to 4 seeds each; instead add explicit guards
that validate signer_seeds.len() <= 2 and for each signer seeds.len() <= 4 and
return an appropriate ProgramError (e.g., InvalidArgument or a domain-specific
error) if exceeded, so do this check at the top of invoke_with_seeds before any
seed conversion and only proceed to build seeds_storage_0/1 and call
pinocchio::cpi::slice_invoke_signed when validation passes; reference
signer_seeds, num_signers, and the Signer creation logic when adding the guard
and error return.
- Around line 50-54: The owner account is incorrectly marked writable
(AccountMeta::writable_signer(self.owner.key())) which forces callers to pass it
writable and can cause CPI privilege escalation failures; change the owner meta
to be a readonly signer instead (replace the writable_signer usage for
self.owner with the readonly signer variant such as AccountMeta::readonly_signer
or the equivalent readonly signer constructor used in this codebase) in the
account_metas array inside close.rs so callers only need to pass owner as a
signer unless it truly must be mutated.
In `@sdk-libs/token-pinocchio/src/instruction/create.rs`:
- Around line 6-8: The current re-export of CreateTokenAccountCpi and
CreateTokenAccountRentFreeCpi from
light_sdk_types::interface::cpi::create_token_accounts must be replaced with
imports from the program-libs crate light_token_interface; update the use
statement to import CreateTokenAccountCpi and CreateTokenAccountRentFreeCpi from
the light_token_interface crate (instead of light_sdk_types) so sdk-libs depends
only on program-libs, light-prover-client, and external crates—ensure the
symbols CreateTokenAccountCpi and CreateTokenAccountRentFreeCpi are imported
from the new crate path and remove the old re-export.
In `@sdk-libs/token-pinocchio/src/instruction/decompress_mint.rs`:
- Around line 101-105: Before constructing DecompressMintAction, validate that
self.rent_payment >= 2 and return an error (or propagate a suitable Result) when
it's less; specifically, add a guard in the code path that creates
DecompressMintAction (the block that instantiates DecompressMintAction with
fields rent_payment and write_top_up) to check the rent_payment value and fail
early if rent_payment < 2 to avoid opaque CPI failures at epoch boundaries.
In `@sdk-libs/token-pinocchio/src/instruction/mint_to_checked.rs`:
- Around line 136-180: The code currently silently truncates signer_seeds to 2
signers and 4 seeds each (used when building seeds_storage_0 / seeds_storage_1
and later calling invoke_with_seeds); instead, add explicit validation at the
start of the routine that inspects signer_seeds: if signer_seeds.len() > 2
return InvalidArgument, and for each signer ensure seeds.len() is between 1 and
4 (reject zero-length seed arrays and >4 with InvalidArgument); keep the
existing seed packing logic (seeds_storage_0, seeds_storage_1, num_seeds_0,
num_seeds_1) but only run it after the bounds checks so you fail fast and avoid
silent truncation and invalid PDA signatures.
In `@sdk-libs/token-pinocchio/src/instruction/mint_to.rs`:
- Around line 133-177: The code currently silently clamps signer_seeds to at
most 2 signers and 4 seeds each (using signer_seeds, seeds_storage_0,
seeds_storage_1 and Seed) which can hide caller errors; add explicit validation
at the start of the mint_to/invoke_with_seeds call to reject inputs that exceed
those limits or contain empty seed slices and return an appropriate error (e.g.
InvalidArgument) instead of truncating; specifically check signer_seeds.len() <=
2, each signer_seeds[i].len() <= 4, and that no seed slice is empty, and return
early with InvalidArgument if any check fails before building seeds_storage_0/1
or computing num_seeds_0/1.
In `@sdk-libs/token-pinocchio/src/instruction/revoke.rs`:
- Around line 69-125: Add explicit validation at the start of the signed-invoke
branch to reject excessive signers/seeds: check that signer_seeds.len() <= 2 and
for each entry in signer_seeds ensure seeds.len() <= 4, and if any check fails
return Err(ProgramError::InvalidArgument). Keep these checks before any
truncation or seed-copy logic (the block that builds seeds_storage_0/1 and
computes num_seeds_0/1 and calls Signer::from and slice_invoke_signed) so no
silent truncation occurs.
In `@sdk-libs/token-pinocchio/src/instruction/transfer.rs`:
- Around line 122-185: The invoke_with_seeds helper is duplicated across
instruction modules; extract it into a single shared utility (e.g., a new
module/function) and update callers to use that shared function. Move the
existing invoke_with_seeds implementation (including handling of Seed, Signer,
pinocchio::cpi::slice_invoke and pinocchio::cpi::slice_invoke_signed, and the
2-signer/4-seed logic) into a common utils file, export it, and replace the
duplicated definitions in each instruction module with calls to the shared
invoke_with_seeds to avoid drift and centralize future fixes (retain the same
function signature so callers like transfer.rs need only change the import).
Comment on lines
+27
to
+28
| [dev-dependencies] | ||
| light-account-checks = { workspace = true, features = ["test-only", "pinocchio", "std", "solana"] } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f 'Cargo.toml' program-libs/account-checksRepository: Lightprotocol/light-protocol
Length of output: 110
🏁 Script executed:
sed -n '1,200p' program-libs/account-checks/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 1717
Remove solana feature from light-account-checks dev-dependency
Token-Pinocchio tests only target the Pinocchio SDK; including solana pulls in unused dependencies.
Suggested diff
-[dev-dependencies]
-light-account-checks = { workspace = true, features = ["test-only", "pinocchio", "std", "solana"] }
+light-account-checks = { workspace = true, features = ["test-only", "pinocchio", "std"] }🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/Cargo.toml` around lines 27 - 28, The dev-dependency
entry for light-account-checks includes an unnecessary "solana" feature; update
the Cargo.toml dev-dependencies line for light-account-checks to remove "solana"
from the features array so it only enables ["test-only", "pinocchio", "std"],
ensuring token-pinocchio tests don't pull in Solana-related dependencies.
Comment on lines
+8
to
+33
| /// Errors specific to high-level token operations. | ||
| /// Error codes start at 17500 to avoid conflicts with other Light Protocol errors. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum LightTokenError { | ||
| SplInterfaceRequired = 17500, | ||
| IncompleteSplInterface = 17501, | ||
| UseRegularSplTransfer = 17502, | ||
| CannotDetermineAccountType = 17503, | ||
| MissingMintAccount = 17504, | ||
| MissingSplTokenProgram = 17505, | ||
| MissingSplInterfacePda = 17506, | ||
| MissingSplInterfacePdaBump = 17507, | ||
| SplTokenProgramMismatch = 17508, | ||
| InvalidAccountData = 17509, | ||
| SerializationError = 17510, | ||
| MissingCpiContext = 17511, | ||
| MissingCpiAuthority = 17512, | ||
| MissingOutputQueue = 17513, | ||
| MissingStateMerkleTree = 17514, | ||
| MissingAddressMerkleTree = 17515, | ||
| MissingLightSystemProgram = 17516, | ||
| MissingRegisteredProgramPda = 17517, | ||
| MissingAccountCompressionAuthority = 17518, | ||
| MissingAccountCompressionProgram = 17519, | ||
| MissingSystemProgram = 17520, | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make error code representation explicit (#[repr(u32)]).
Since these values are serialized into ProgramError::Custom and cast to u32, declaring the representation avoids ambiguity and makes the numeric contract explicit.
Proposed change
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(u32)]
pub enum LightTokenError {📝 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.
Suggested change
| /// Errors specific to high-level token operations. | |
| /// Error codes start at 17500 to avoid conflicts with other Light Protocol errors. | |
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| pub enum LightTokenError { | |
| SplInterfaceRequired = 17500, | |
| IncompleteSplInterface = 17501, | |
| UseRegularSplTransfer = 17502, | |
| CannotDetermineAccountType = 17503, | |
| MissingMintAccount = 17504, | |
| MissingSplTokenProgram = 17505, | |
| MissingSplInterfacePda = 17506, | |
| MissingSplInterfacePdaBump = 17507, | |
| SplTokenProgramMismatch = 17508, | |
| InvalidAccountData = 17509, | |
| SerializationError = 17510, | |
| MissingCpiContext = 17511, | |
| MissingCpiAuthority = 17512, | |
| MissingOutputQueue = 17513, | |
| MissingStateMerkleTree = 17514, | |
| MissingAddressMerkleTree = 17515, | |
| MissingLightSystemProgram = 17516, | |
| MissingRegisteredProgramPda = 17517, | |
| MissingAccountCompressionAuthority = 17518, | |
| MissingAccountCompressionProgram = 17519, | |
| MissingSystemProgram = 17520, | |
| } | |
| /// Errors specific to high-level token operations. | |
| /// Error codes start at 17500 to avoid conflicts with other Light Protocol errors. | |
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| #[repr(u32)] | |
| pub enum LightTokenError { | |
| SplInterfaceRequired = 17500, | |
| IncompleteSplInterface = 17501, | |
| UseRegularSplTransfer = 17502, | |
| CannotDetermineAccountType = 17503, | |
| MissingMintAccount = 17504, | |
| MissingSplTokenProgram = 17505, | |
| MissingSplInterfacePda = 17506, | |
| MissingSplInterfacePdaBump = 17507, | |
| SplTokenProgramMismatch = 17508, | |
| InvalidAccountData = 17509, | |
| SerializationError = 17510, | |
| MissingCpiContext = 17511, | |
| MissingCpiAuthority = 17512, | |
| MissingOutputQueue = 17513, | |
| MissingStateMerkleTree = 17514, | |
| MissingAddressMerkleTree = 17515, | |
| MissingLightSystemProgram = 17516, | |
| MissingRegisteredProgramPda = 17517, | |
| MissingAccountCompressionAuthority = 17518, | |
| MissingAccountCompressionProgram = 17519, | |
| MissingSystemProgram = 17520, | |
| } |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/error.rs` around lines 8 - 33, Add an explicit
integer representation to the LightTokenError enum by annotating it with
#[repr(u32)] (place the attribute directly above the LightTokenError
declaration) so the discriminant values used when casting to
u32/ProgramError::Custom are unambiguous; ensure the enum and its existing
variants (SplInterfaceRequired ... MissingSystemProgram) remain unchanged
otherwise.
Comment on lines
+43
to
+46
| // Build instruction data: discriminator(1) + amount(8) | ||
| let mut data = [0u8; 9]; | ||
| data[0] = 4u8; // Approve discriminator | ||
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); |
Contributor
There was a problem hiding this comment.
Generate CPI data with the Anchor 8-byte discriminator
Manual 1-byte prefix (4u8) won’t align with Anchor’s instruction layout. Use anchor_lang::Discriminator on the Approve instruction (e.g. Approve::discriminator()) to build the 8-byte prefix, then append self.amount.to_le_bytes().
sdk-libs/token-pinocchio/src/instruction/approve.rs:43-46
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/approve.rs` around lines 43 - 46,
The instruction data currently uses a 1-byte hardcoded discriminator (data[0] =
4u8) which is incompatible with Anchor's 8-byte discriminator layout; replace
the manual prefix with Anchor's 8-byte discriminator for the Approve instruction
by using Approve::discriminator() (or
anchor_lang::Discriminator::discriminator(&Approve::discriminator())) and append
self.amount.to_le_bytes() to that 8-byte prefix when building the final data
buffer so the instruction data becomes [8-byte discriminator] + [8-byte amount].
Comment on lines
+50
to
+54
| let account_metas = [ | ||
| AccountMeta::writable(self.account.key()), | ||
| AccountMeta::writable(self.destination.key()), | ||
| AccountMeta::writable_signer(self.owner.key()), | ||
| AccountMeta::writable(self.rent_sponsor.key()), |
Contributor
There was a problem hiding this comment.
Owner should be readonly_signer to avoid privilege escalation failures.
Marking owner writable forces callers to pass it as writable; most flows only pass it as signer, so CPI can fail unless it’s truly mutated.
🛠️ Proposed fix
- AccountMeta::writable_signer(self.owner.key()),
+ AccountMeta::readonly_signer(self.owner.key()),📝 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.
Suggested change
| let account_metas = [ | |
| AccountMeta::writable(self.account.key()), | |
| AccountMeta::writable(self.destination.key()), | |
| AccountMeta::writable_signer(self.owner.key()), | |
| AccountMeta::writable(self.rent_sponsor.key()), | |
| let account_metas = [ | |
| AccountMeta::writable(self.account.key()), | |
| AccountMeta::writable(self.destination.key()), | |
| AccountMeta::readonly_signer(self.owner.key()), | |
| AccountMeta::writable(self.rent_sponsor.key()), | |
| ]; |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/close.rs` around lines 50 - 54, The
owner account is incorrectly marked writable
(AccountMeta::writable_signer(self.owner.key())) which forces callers to pass it
writable and can cause CPI privilege escalation failures; change the owner meta
to be a readonly signer instead (replace the writable_signer usage for
self.owner with the readonly signer variant such as AccountMeta::readonly_signer
or the equivalent readonly signer constructor used in this codebase) in the
account_metas array inside close.rs so callers only need to pass owner as a
signer unless it truly must be mutated.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@sdk-libs/token-pinocchio/src/instruction/burn_checked.rs`:
- Around line 17-29: The doc example in burn_checked.rs references a
non-existent field max_top_up on the BurnCheckedCpi struct; update the example
to match the actual struct by removing max_top_up (and any related mention) so
the snippet compiles as-is, or if the feature was intended, add a max_top_up
field to the BurnCheckedCpi struct with an appropriate Option type and update
its constructor/usage accordingly; locate the struct BurnCheckedCpi and the
example block in burn_checked.rs to make the change.
In `@sdk-libs/token-pinocchio/src/instruction/burn.rs`:
- Around line 15-51: The docs and inline payload comment reference a
non-existent max_top_up field on BurnCpi; update the example and comment to
match the struct (BurnCpi with fields source, mint, amount, authority,
system_program, fee_payer) by removing max_top_up from the example and changing
the payload comment ("discriminator(1) + amount(8)") so it accurately reflects
the built instruction in invoke_signed; ensure the example uses fee_payer: None
(or omits it if you prefer) and compiles against the current BurnCpi API.
In `@sdk-libs/token-pinocchio/src/instruction/create_ata.rs`:
- Around line 6-10: The code is re-exporting CreateTokenAtaCpi,
CreateTokenAtaCpiIdempotent, and CreateTokenAtaRentFreeCpi from light_sdk_types
which violates SDK-libs dependencies; remove that direct import and instead
import these CPI builder types from the program-libs crate
light-token-interface. If light-token-interface does not currently re-export
those types, add pub use re-exports there (matching the exact symbols
CreateTokenAtaCpi, CreateTokenAtaCpiIdempotent, CreateTokenAtaRentFreeCpi) and
then update this file to reference light_token_interface::... for those symbols
and delete the light_sdk_types re-export. Ensure any generic visibility comments
are preserved.
In `@sdk-libs/token-pinocchio/src/instruction/mint_to_checked.rs`:
- Around line 17-29: The doc example for MintToCheckedCpi references a
non-existent field max_top_up; update the documentation to match the actual
struct by removing the max_top_up (and fee_payer if applicable) line from the
example, or if the intent was to support an optional top-up, add an Option-typed
field named max_top_up to the MintToCheckedCpi struct and its
initialization/handling so the example is valid; locate the example in
mint_to_checked.rs and make the snippet and the struct signature consistent
(referencing MintToCheckedCpi to find both spots).
In `@sdk-libs/token-pinocchio/src/instruction/mint_to.rs`:
- Around line 15-51: The docs and inline payload comment reference a
non-existent max_top_up field; update the example and comments to match the
actual MintToCpi struct fields. Remove max_top_up from the code example in the
doc block for MintToCpi and change the data construction comment in
invoke_signed (and any related variables like data_len) to reflect only
discriminator(1) + amount(8) (and mention fee_payer is an optional AccountInfo
field, not part of the instruction payload). Ensure all references to max_top_up
are removed or replaced with the correct field names (e.g., fee_payer) around
MintToCpi and invoke_signed.
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs`:
- Around line 11-16: Update the module-level "Transfers" documentation to
include the missing TransferCheckedCpi entry: add a bullet for
`TransferCheckedCpi` alongside `TransferCpi`, `TransferFromSplCpi`,
`TransferToSplCpi`, and `TransferInterfaceCpi` in the top doc comment so the
exported `TransferCheckedCpi` (exported later in this module) is documented;
keep the same brief style/format used for the other transfer items.
In `@sdk-libs/token-pinocchio/src/instruction/revoke.rs`:
- Around line 38-62: The instruction data in RevokeCpi::invoke_signed is using a
single-byte hardcoded discriminator (let data = [5u8]) but must use the 8-byte
REVOKE discriminator from token-types; import the REVOKE constant (e.g. use
crate::constants::REVOKE or the appropriate token-types path) and replace the
hardcoded data with that constant, and apply the same fix pattern to the other
CPI helpers (e.g. FreezeCpi::invoke_signed, ThawCpi::invoke_signed) by replacing
their single-byte literals with the corresponding 8-byte FREEZE/THAW constants.
In `@sdk-libs/token-pinocchio/src/instruction/transfer_checked.rs`:
- Around line 49-55: The inline comment in invoke_signed is inaccurate: it
mentions "optional max_top_up(2)" but the code only writes discriminator
(data[0]), amount (data[1..9]) and decimals (data[9]) and sets data_len = 10;
either remove the "optional max_top_up(2)" text or implement writing and
accounting for max_top_up into the data buffer and update data_len accordingly;
update the comment to exactly describe the actual layout (discriminator(1) +
amount(8) + decimals(1)) if you choose not to implement max_top_up, and ensure
references to data, data_len, self.amount and self.decimals are consistent.
- Around line 56-58: Remove the now-unused local variable authority_writable
introduced near the transfer_checked instruction logic: since you simplified the
condition to rely on self.fee_payer directly, delete the authority_writable
binding and any references to it so the code compiles without unused-variable
warnings; search for the authority_writable declaration in transfer_checked.rs
(near the code that checks self.fee_payer) and remove that line.
In `@sdk-libs/token-pinocchio/src/instruction/transfer.rs`:
- Around line 15-51: The docs and inline payload comment mention a non-existent
field `max_top_up` causing confusion and mismatched data sizing; update the
TransferCpi example and the instruction-data comment to remove any `max_top_up`
references, adjust the data buffer/comment to match the actual payload
(discriminator(1) + amount(8) => 9 bytes) and update the array size and any
accompanying comment in TransferCpi::invoke_signed (and related variables like
`data` and `data_len`) so they reflect the real struct fields (`source`,
`destination`, `amount`, `authority`, optional `fee_payer`) and correct byte
layout.
Comment on lines
+17
to
+29
| /// ```rust,ignore | ||
| /// use light_token_pinocchio::instruction::BurnCheckedCpi; | ||
| /// | ||
| /// BurnCheckedCpi { | ||
| /// source: &ctx.accounts.source, | ||
| /// mint: &ctx.accounts.mint, | ||
| /// amount: 100, | ||
| /// decimals: 9, | ||
| /// authority: &ctx.accounts.authority, | ||
| /// system_program: &ctx.accounts.system_program, | ||
| /// max_top_up: None, | ||
| /// fee_payer: None, | ||
| /// } |
Contributor
There was a problem hiding this comment.
Doc example references max_top_up, but the struct doesn’t define it.
This makes the example unusable as-is. Remove it or add the field.
🛠️ Proposed doc fix
-/// max_top_up: None,📝 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.
Suggested change
| /// ```rust,ignore | |
| /// use light_token_pinocchio::instruction::BurnCheckedCpi; | |
| /// | |
| /// BurnCheckedCpi { | |
| /// source: &ctx.accounts.source, | |
| /// mint: &ctx.accounts.mint, | |
| /// amount: 100, | |
| /// decimals: 9, | |
| /// authority: &ctx.accounts.authority, | |
| /// system_program: &ctx.accounts.system_program, | |
| /// max_top_up: None, | |
| /// fee_payer: None, | |
| /// } | |
| /// |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/burn_checked.rs` around lines 17 -
29, The doc example in burn_checked.rs references a non-existent field
max_top_up on the BurnCheckedCpi struct; update the example to match the actual
struct by removing max_top_up (and any related mention) so the snippet compiles
as-is, or if the feature was intended, add a max_top_up field to the
BurnCheckedCpi struct with an appropriate Option type and update its
constructor/usage accordingly; locate the struct BurnCheckedCpi and the example
block in burn_checked.rs to make the change.
Comment on lines
+15
to
+51
| /// # Example | ||
| /// | ||
| /// ```rust,ignore | ||
| /// use light_token_pinocchio::instruction::BurnCpi; | ||
| /// | ||
| /// BurnCpi { | ||
| /// source: &ctx.accounts.source, | ||
| /// mint: &ctx.accounts.mint, | ||
| /// amount: 100, | ||
| /// authority: &ctx.accounts.authority, | ||
| /// system_program: &ctx.accounts.system_program, | ||
| /// max_top_up: None, | ||
| /// fee_payer: None, | ||
| /// } | ||
| /// .invoke()?; | ||
| /// ``` | ||
| pub struct BurnCpi<'info> { | ||
| pub source: &'info AccountInfo, | ||
| pub mint: &'info AccountInfo, | ||
| pub amount: u64, | ||
| pub authority: &'info AccountInfo, | ||
| pub system_program: &'info AccountInfo, | ||
| /// Optional fee payer for rent top-ups. If not provided, authority pays. | ||
| pub fee_payer: Option<&'info AccountInfo>, | ||
| } | ||
|
|
||
| impl<'info> BurnCpi<'info> { | ||
| pub fn invoke(self) -> Result<(), ProgramError> { | ||
| self.invoke_signed(&[]) | ||
| } | ||
|
|
||
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | ||
| // Build instruction data: discriminator(1) + amount(8) + optional max_top_up(2) | ||
| let mut data = [0u8; 11]; | ||
| data[0] = 8u8; // Burn discriminator | ||
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | ||
| let data_len = 9; |
Contributor
There was a problem hiding this comment.
Example/comment drift: max_top_up isn’t a field on BurnCpi.
The doc example and inline payload comment mention max_top_up, but the struct doesn’t expose it. That will mislead users and the example won’t compile. Please align the docs with the actual struct (or add the field if intended).
📝 Suggested doc/comment fix
-/// max_top_up: None,
/// fee_payer: None,- // Build instruction data: discriminator(1) + amount(8) + optional max_top_up(2)
+ // Build instruction data: discriminator(1) + amount(8)🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/burn.rs` around lines 15 - 51, The
docs and inline payload comment reference a non-existent max_top_up field on
BurnCpi; update the example and comment to match the struct (BurnCpi with fields
source, mint, amount, authority, system_program, fee_payer) by removing
max_top_up from the example and changing the payload comment ("discriminator(1)
+ amount(8)") so it accurately reflects the built instruction in invoke_signed;
ensure the example uses fee_payer: None (or omits it if you prefer) and compiles
against the current BurnCpi API.
Comment on lines
+6
to
+10
| use light_account_checks::AccountInfoTrait; | ||
| // TODO: add types with generics set so that we dont expose the generics | ||
| pub use light_sdk_types::interface::cpi::create_token_accounts::{ | ||
| CreateTokenAtaCpi, CreateTokenAtaCpiIdempotent, CreateTokenAtaRentFreeCpi, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its context
find . -path "*/sdk-libs/token-pinocchio/src/instruction/create_ata.rs" -exec cat {} \;Repository: Lightprotocol/light-protocol
Length of output: 1099
🏁 Script executed:
# Find where CreateTokenAtaCpi is defined
rg -n "struct CreateTokenAtaCpi|pub struct CreateTokenAtaCpi|type CreateTokenAtaCpi" -g "*.rs" -A 2Repository: Lightprotocol/light-protocol
Length of output: 1213
🏁 Script executed:
# Check the Cargo.toml for token-pinocchio
find . -path "*/sdk-libs/token-pinocchio/Cargo.toml" -exec cat {} \;Repository: Lightprotocol/light-protocol
Length of output: 1105
🏁 Script executed:
# Check what's exported from light-token-interface
find . -path "*/light-token-interface/*/lib.rs" -o -path "*/light-token-interface/src/lib.rs" | head -5Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check light_sdk_types to understand what it is
find . -name "light_sdk_types" -type dRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Find light-token-interface crate
fd -t d "light-token-interface" --max-depth 3Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check what's in light-token-interface
find . -path "*/light-token-interface/src/lib.rs" -o -path "*/light-token-interface/src/mod.rs" | xargs cat 2>/dev/null | head -100Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check if CreateTokenAtaCpi is exported from light-token-interface
rg "CreateTokenAtaCpi" -g "light-token-interface/**/*.rs"Repository: Lightprotocol/light-protocol
Length of output: 220
🏁 Script executed:
# Verify where sdk-types is and what it depends on
find . -path "*/sdk-types/Cargo.toml" -exec cat {} \;Repository: Lightprotocol/light-protocol
Length of output: 1665
🏁 Script executed:
# Find all crates to understand structure
find . -maxdepth 3 -name "Cargo.toml" -path "*/light-token-interface/*" -o -name "Cargo.toml" -path "*/program-libs/*" | head -20Repository: Lightprotocol/light-protocol
Length of output: 827
🏁 Script executed:
# Search for program-libs directory structure
ls -la program-libs/ 2>/dev/null || ls -la . | grep -i programRepository: Lightprotocol/light-protocol
Length of output: 1515
🏁 Script executed:
# Check what crates exist and their locations
fd "Cargo.toml" --max-depth 2 | xargs -I {} sh -c 'echo "=== {} ===" && grep "^name = " {} ' | head -80Repository: Lightprotocol/light-protocol
Length of output: 306
🏁 Script executed:
# Look for light-token-interface location and structure
find . -type d -name "light-token-interface" -o -type f -path "*/light-token-interface/*" | head -20Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Search for where CreateTokenAtaCpi might be exported or should be
rg "pub.*CreateTokenAtaCpi|pub.*use.*CreateTokenAtaCpi" --max-count 20Repository: Lightprotocol/light-protocol
Length of output: 767
🏁 Script executed:
# Check program-libs/token-interface structure and exports
cat program-libs/token-interface/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 2349
🏁 Script executed:
# Check what's in token-interface
find program-libs/token-interface -type f -name "*.rs" | head -20Repository: Lightprotocol/light-protocol
Length of output: 1439
🏁 Script executed:
# Check token-interface lib.rs
cat program-libs/token-interface/src/lib.rsRepository: Lightprotocol/light-protocol
Length of output: 1148
🏁 Script executed:
# Check if token-interface exports CPI-related types
rg "cpi|CreateTokenAta" program-libs/token-interface/Repository: Lightprotocol/light-protocol
Length of output: 2818
🏁 Script executed:
# Verify sdk-libs/sdk-types/Cargo.toml dependencies - what does it depend on?
rg "^dependencies|^\[" sdk-libs/sdk-types/Cargo.toml -A 30 | head -50Repository: Lightprotocol/light-protocol
Length of output: 1665
🏁 Script executed:
# Check if CreateTokenAtaCpi or similar CPI types exist in light-token-interface's exports
cat program-libs/token-interface/src/lib.rsRepository: Lightprotocol/light-protocol
Length of output: 1148
🏁 Script executed:
# Search for any CPI wrapper types in light-token-interface
rg "pub struct.*Cpi|pub type.*Cpi" program-libs/token-interface/Repository: Lightprotocol/light-protocol
Length of output: 273
🏁 Script executed:
# Check what's in the interface module structure of light-token-interface
find program-libs/token-interface/src -type f -name "*.rs" | sortRepository: Lightprotocol/light-protocol
Length of output: 3670
🏁 Script executed:
# Verify the actual Cargo dependency path - is light-sdk-types allowed for SDK-libs?
cat sdk-libs/token-pinocchio/Cargo.toml | grep -A 50 "^\[dependencies\]"Repository: Lightprotocol/light-protocol
Length of output: 856
🏁 Script executed:
# Verify the architectural intent - check CLAUDE.md in program-libs
cat program-libs/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 3256
🏁 Script executed:
# Check if there's any SDK-libs <-> SDK-libs dependencies documented or intentional
rg "light-sdk-types" sdk-libs/ --glob="Cargo.toml" -cRepository: Lightprotocol/light-protocol
Length of output: 517
🏁 Script executed:
# Look at what light-sdk-types exports related to CPI for token
cat sdk-libs/sdk-types/src/interface/cpi/create_token_accounts.rs | head -100Repository: Lightprotocol/light-protocol
Length of output: 3548
Remove direct light-sdk-types dependency; these CPI builders should come from light-token-interface.
The re-export from light_sdk_types violates the SDK-libs architecture rule: SDK-libs must depend only on program-libs, light-prover-client, and external crates. Since light-token-interface (a program-libs crate) is already a dependency, the CPI builder types need to be accessible through it.
This likely requires moving or re-exporting the CreateTokenAtaCpi variants through light-token-interface first, then updating this file to import from there.
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/create_ata.rs` around lines 6 - 10,
The code is re-exporting CreateTokenAtaCpi, CreateTokenAtaCpiIdempotent, and
CreateTokenAtaRentFreeCpi from light_sdk_types which violates SDK-libs
dependencies; remove that direct import and instead import these CPI builder
types from the program-libs crate light-token-interface. If
light-token-interface does not currently re-export those types, add pub use
re-exports there (matching the exact symbols CreateTokenAtaCpi,
CreateTokenAtaCpiIdempotent, CreateTokenAtaRentFreeCpi) and then update this
file to reference light_token_interface::... for those symbols and delete the
light_sdk_types re-export. Ensure any generic visibility comments are preserved.
Comment on lines
+17
to
+29
| /// ```rust,ignore | ||
| /// use light_token_pinocchio::instruction::MintToCheckedCpi; | ||
| /// | ||
| /// MintToCheckedCpi { | ||
| /// mint: &ctx.accounts.mint, | ||
| /// destination: &ctx.accounts.destination, | ||
| /// amount: 100, | ||
| /// decimals: 9, | ||
| /// authority: &ctx.accounts.authority, | ||
| /// system_program: &ctx.accounts.system_program, | ||
| /// max_top_up: None, | ||
| /// fee_payer: None, | ||
| /// } |
Contributor
There was a problem hiding this comment.
Doc example references max_top_up, but the struct doesn’t define it.
This will mislead users copying the snippet. Remove it or add the field.
🛠️ Proposed doc fix
-/// max_top_up: None,📝 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.
Suggested change
| /// ```rust,ignore | |
| /// use light_token_pinocchio::instruction::MintToCheckedCpi; | |
| /// | |
| /// MintToCheckedCpi { | |
| /// mint: &ctx.accounts.mint, | |
| /// destination: &ctx.accounts.destination, | |
| /// amount: 100, | |
| /// decimals: 9, | |
| /// authority: &ctx.accounts.authority, | |
| /// system_program: &ctx.accounts.system_program, | |
| /// max_top_up: None, | |
| /// fee_payer: None, | |
| /// } | |
| /// |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mint_to_checked.rs` around lines 17
- 29, The doc example for MintToCheckedCpi references a non-existent field
max_top_up; update the documentation to match the actual struct by removing the
max_top_up (and fee_payer if applicable) line from the example, or if the intent
was to support an optional top-up, add an Option-typed field named max_top_up to
the MintToCheckedCpi struct and its initialization/handling so the example is
valid; locate the example in mint_to_checked.rs and make the snippet and the
struct signature consistent (referencing MintToCheckedCpi to find both spots).
Comment on lines
+15
to
+51
| /// # Example | ||
| /// | ||
| /// ```rust,ignore | ||
| /// use light_token_pinocchio::instruction::MintToCpi; | ||
| /// | ||
| /// MintToCpi { | ||
| /// mint: &ctx.accounts.mint, | ||
| /// destination: &ctx.accounts.destination, | ||
| /// amount: 100, | ||
| /// authority: &ctx.accounts.authority, | ||
| /// system_program: &ctx.accounts.system_program, | ||
| /// max_top_up: None, | ||
| /// fee_payer: None, | ||
| /// } | ||
| /// .invoke()?; | ||
| /// ``` | ||
| pub struct MintToCpi<'info> { | ||
| pub mint: &'info AccountInfo, | ||
| pub destination: &'info AccountInfo, | ||
| pub amount: u64, | ||
| pub authority: &'info AccountInfo, | ||
| pub system_program: &'info AccountInfo, | ||
| /// Optional fee payer for rent top-ups. If not provided, authority pays. | ||
| pub fee_payer: Option<&'info AccountInfo>, | ||
| } | ||
|
|
||
| impl<'info> MintToCpi<'info> { | ||
| pub fn invoke(self) -> Result<(), ProgramError> { | ||
| self.invoke_signed(&[]) | ||
| } | ||
|
|
||
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | ||
| // Build instruction data: discriminator(1) + amount(8) + optional max_top_up(2) | ||
| let mut data = [0u8; 11]; | ||
| data[0] = 7u8; // MintTo discriminator | ||
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | ||
| let data_len = 9; |
Contributor
There was a problem hiding this comment.
Example/comment drift: max_top_up isn’t a field on MintToCpi.
The doc example and payload comment still mention max_top_up, but the struct doesn’t define it. Please update the docs/comments to match the actual inputs.
📝 Suggested doc/comment fix
-/// max_top_up: None,
/// fee_payer: None,- // Build instruction data: discriminator(1) + amount(8) + optional max_top_up(2)
+ // Build instruction data: discriminator(1) + amount(8)🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mint_to.rs` around lines 15 - 51,
The docs and inline payload comment reference a non-existent max_top_up field;
update the example and comments to match the actual MintToCpi struct fields.
Remove max_top_up from the code example in the doc block for MintToCpi and
change the data construction comment in invoke_signed (and any related variables
like data_len) to reflect only discriminator(1) + amount(8) (and mention
fee_payer is an optional AccountInfo field, not part of the instruction
payload). Ensure all references to max_top_up are removed or replaced with the
correct field names (e.g., fee_payer) around MintToCpi and invoke_signed.
Comment on lines
+11
to
+16
| //! ## Transfers | ||
| //! | ||
| //! - [`TransferCpi`] - Transfer between Light Token accounts | ||
| //! - [`TransferFromSplCpi`] - Transfer from SPL token account to Light Token account | ||
| //! - [`TransferToSplCpi`] - Transfer from Light Token account to SPL token account | ||
| //! - [`TransferInterfaceCpi`] - Transfer via CPI, auto-detect source/destination account types |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing TransferCheckedCpi in module documentation.
The "Transfers" section documents TransferCpi, TransferFromSplCpi, TransferToSplCpi, and TransferInterfaceCpi, but TransferCheckedCpi is exported (line 90) without being listed here.
📝 Suggested documentation update
//! ## Transfers
//!
//! - [`TransferCpi`] - Transfer between Light Token accounts
+//! - [`TransferCheckedCpi`] - Transfer between Light Token accounts with decimal validation
//! - [`TransferFromSplCpi`] - Transfer from SPL token account to Light Token account
//! - [`TransferToSplCpi`] - Transfer from Light Token account to SPL token account
//! - [`TransferInterfaceCpi`] - Transfer via CPI, auto-detect source/destination account types🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs` around lines 11 - 16, Update
the module-level "Transfers" documentation to include the missing
TransferCheckedCpi entry: add a bullet for `TransferCheckedCpi` alongside
`TransferCpi`, `TransferFromSplCpi`, `TransferToSplCpi`, and
`TransferInterfaceCpi` in the top doc comment so the exported
`TransferCheckedCpi` (exported later in this module) is documented; keep the
same brief style/format used for the other transfer items.
Comment on lines
+38
to
+62
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | ||
| // Build instruction data: discriminator(1) only | ||
| let data = [5u8]; // Revoke discriminator | ||
|
|
||
| let program_id = Pubkey::from(LIGHT_TOKEN_PROGRAM_ID); | ||
|
|
||
| let account_metas = [ | ||
| AccountMeta::writable(self.token_account.key()), | ||
| AccountMeta::writable_signer(self.owner.key()), | ||
| AccountMeta::readonly(self.system_program.key()), | ||
| ]; | ||
|
|
||
| let instruction = Instruction { | ||
| program_id: &program_id, | ||
| accounts: &account_metas, | ||
| data: &data, | ||
| }; | ||
|
|
||
| let account_infos = [self.token_account, self.owner, self.system_program]; | ||
|
|
||
| if signers.is_empty() { | ||
| slice_invoke(&instruction, &account_infos) | ||
| } else { | ||
| slice_invoke_signed(&instruction, &account_infos, signers) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Light Token program definition
find . -type f -name "*.rs" | xargs rg -l "light.token.*program|light_token.*program" | head -20Repository: Lightprotocol/light-protocol
Length of output: 1215
🏁 Script executed:
#!/bin/bash
# Search for the revoke instruction definition in Light Token program
rg -i "revoke.*instruction|fn revoke" --type rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Look for Light Token program source
fd -t f "lib.rs|mod.rs" | xargs rg -l "revoke" | grep -i "light.*token|token.*program"Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Find the light token program directory
find . -type d -name "*token*" | grep programsRepository: Lightprotocol/light-protocol
Length of output: 449
🏁 Script executed:
#!/bin/bash
# Search for the light token program library
find . -path "*/programs/*token*/src/lib.rs" -o -path "*/programs/*token*/src/instruction*.rs"Repository: Lightprotocol/light-protocol
Length of output: 545
🏁 Script executed:
#!/bin/bash
# Look for revoke instruction definition with discriminator
rg "enum.*Instruction|const.*REVOKE|discriminator.*revoke" --type rust -iRepository: Lightprotocol/light-protocol
Length of output: 3920
🏁 Script executed:
#!/bin/bash
# Check the REVOKE constant definition
cat sdk-libs/token-types/src/constants.rs | grep -A 2 "REVOKE"Repository: Lightprotocol/light-protocol
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Check imports in the revoke.rs file
head -30 sdk-libs/token-pinocchio/src/instruction/revoke.rsRepository: Lightprotocol/light-protocol
Length of output: 835
🏁 Script executed:
#!/bin/bash
# Check other CPI instruction files for discriminator usage pattern
ls -la sdk-libs/token-pinocchio/src/instruction/Repository: Lightprotocol/light-protocol
Length of output: 1529
🏁 Script executed:
#!/bin/bash
# Check how freeze.rs handles the FREEZE discriminator
cat sdk-libs/token-pinocchio/src/instruction/freeze.rsRepository: Lightprotocol/light-protocol
Length of output: 1875
🏁 Script executed:
#!/bin/bash
# Check how thaw.rs handles the THAW discriminator
cat sdk-libs/token-pinocchio/src/instruction/thaw.rsRepository: Lightprotocol/light-protocol
Length of output: 1861
🏁 Script executed:
#!/bin/bash
# Check how other instructions like transfer.rs handle discriminators
grep -A 20 "invoke_signed" sdk-libs/token-pinocchio/src/instruction/transfer.rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 991
Use the 8-byte REVOKE discriminator constant from token-types instead of the hardcoded single byte.
The RevokeCpi::invoke_signed builds instruction data using let data = [5u8];, but the correct discriminator is defined as an 8-byte constant in sdk-libs/token-types/src/constants.rs:
pub const REVOKE: [u8; 8] = [170, 23, 31, 34, 133, 173, 93, 242];
Import and use this constant:
use crate::constants::REVOKE;
// ...
let data = REVOKE;This pattern is systemic across the CPI helpers—freeze.rs (uses 10u8 instead of FREEZE), thaw.rs (uses 11u8 instead of THAW), and others all hardcode single-byte values instead of using the 8-byte discriminator constants. Per the coding guidelines, use the 8-byte discriminators defined in token-types for consistency with the Light Token program's instruction layout.
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/revoke.rs` around lines 38 - 62, The
instruction data in RevokeCpi::invoke_signed is using a single-byte hardcoded
discriminator (let data = [5u8]) but must use the 8-byte REVOKE discriminator
from token-types; import the REVOKE constant (e.g. use crate::constants::REVOKE
or the appropriate token-types path) and replace the hardcoded data with that
constant, and apply the same fix pattern to the other CPI helpers (e.g.
FreezeCpi::invoke_signed, ThawCpi::invoke_signed) by replacing their single-byte
literals with the corresponding 8-byte FREEZE/THAW constants.
Comment on lines
+49
to
+55
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | ||
| // Build instruction data: discriminator(1) + amount(8) + decimals(1) + optional max_top_up(2) | ||
| let mut data = [0u8; 12]; | ||
| data[0] = 12u8; // TransferChecked discriminator | ||
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | ||
| data[9] = self.decimals; | ||
| let data_len = 10; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Misleading comment about data layout.
The comment mentions optional max_top_up(2) but data_len is set to 10, which only covers discriminator (1) + amount (8) + decimals (1). The comment should match the actual data being sent.
📝 Suggested fix
pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> {
- // Build instruction data: discriminator(1) + amount(8) + decimals(1) + optional max_top_up(2)
+ // Build instruction data: discriminator(1) + amount(8) + decimals(1)
let mut data = [0u8; 12];
data[0] = 12u8; // TransferChecked discriminator
data[1..9].copy_from_slice(&self.amount.to_le_bytes());
data[9] = self.decimals;
let data_len = 10;📝 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.
Suggested change
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | |
| // Build instruction data: discriminator(1) + amount(8) + decimals(1) + optional max_top_up(2) | |
| let mut data = [0u8; 12]; | |
| data[0] = 12u8; // TransferChecked discriminator | |
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | |
| data[9] = self.decimals; | |
| let data_len = 10; | |
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | |
| // Build instruction data: discriminator(1) + amount(8) + decimals(1) | |
| let mut data = [0u8; 12]; | |
| data[0] = 12u8; // TransferChecked discriminator | |
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | |
| data[9] = self.decimals; | |
| let data_len = 10; |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/transfer_checked.rs` around lines 49
- 55, The inline comment in invoke_signed is inaccurate: it mentions "optional
max_top_up(2)" but the code only writes discriminator (data[0]), amount
(data[1..9]) and decimals (data[9]) and sets data_len = 10; either remove the
"optional max_top_up(2)" text or implement writing and accounting for max_top_up
into the data buffer and update data_len accordingly; update the comment to
exactly describe the actual layout (discriminator(1) + amount(8) + decimals(1))
if you choose not to implement max_top_up, and ensure references to data,
data_len, self.amount and self.decimals are consistent.
Comment on lines
+56
to
+58
|
|
||
| // Authority is writable only when no fee_payer | ||
| let authority_writable = self.fee_payer.is_none(); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing the unused variable after simplification.
If you apply the simplification above, the authority_writable variable becomes unused and can be removed entirely:
- // Authority is writable only when no fee_payer
- let authority_writable = self.fee_payer.is_none();📝 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.
Suggested change
| // Authority is writable only when no fee_payer | |
| let authority_writable = self.fee_payer.is_none(); |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/transfer_checked.rs` around lines 56
- 58, Remove the now-unused local variable authority_writable introduced near
the transfer_checked instruction logic: since you simplified the condition to
rely on self.fee_payer directly, delete the authority_writable binding and any
references to it so the code compiles without unused-variable warnings; search
for the authority_writable declaration in transfer_checked.rs (near the code
that checks self.fee_payer) and remove that line.
Comment on lines
+15
to
+51
| /// # Example | ||
| /// | ||
| /// ```rust,ignore | ||
| /// use light_token_pinocchio::instruction::TransferCpi; | ||
| /// | ||
| /// TransferCpi { | ||
| /// source: &ctx.accounts.source, | ||
| /// destination: &ctx.accounts.destination, | ||
| /// amount: 100, | ||
| /// authority: &ctx.accounts.authority, | ||
| /// system_program: &ctx.accounts.system_program, | ||
| /// max_top_up: None, | ||
| /// fee_payer: None, | ||
| /// } | ||
| /// .invoke()?; | ||
| /// ``` | ||
| pub struct TransferCpi<'info> { | ||
| pub source: &'info AccountInfo, | ||
| pub destination: &'info AccountInfo, | ||
| pub amount: u64, | ||
| pub authority: &'info AccountInfo, | ||
| pub system_program: &'info AccountInfo, | ||
| /// Optional fee payer for rent top-ups. If not provided, authority pays. | ||
| pub fee_payer: Option<&'info AccountInfo>, | ||
| } | ||
|
|
||
| impl<'info> TransferCpi<'info> { | ||
| pub fn invoke(self) -> Result<(), ProgramError> { | ||
| self.invoke_signed(&[]) | ||
| } | ||
|
|
||
| pub fn invoke_signed(self, signers: &[Signer]) -> Result<(), ProgramError> { | ||
| // Build instruction data | ||
| let mut data = [0u8; 11]; // discriminator(1) + amount(8) + optional max_top_up(2) | ||
| data[0] = 3u8; // Transfer discriminator | ||
| data[1..9].copy_from_slice(&self.amount.to_le_bytes()); | ||
| let data_len = 9; |
Contributor
There was a problem hiding this comment.
Example/comment drift: max_top_up isn’t a field on TransferCpi.
The docs and payload comment still reference max_top_up, but the struct has no such field. Please update the docs/comments to avoid confusion and compile errors for users.
📝 Suggested doc/comment fix
-/// max_top_up: None,
/// fee_payer: None,- // Build instruction data: discriminator(1) + amount(8) + optional max_top_up(2)
+ // Build instruction data: discriminator(1) + amount(8)🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/transfer.rs` around lines 15 - 51,
The docs and inline payload comment mention a non-existent field `max_top_up`
causing confusion and mismatched data sizing; update the TransferCpi example and
the instruction-data comment to remove any `max_top_up` references, adjust the
data buffer/comment to match the actual payload (discriminator(1) + amount(8) =>
9 bytes) and update the array size and any accompanying comment in
TransferCpi::invoke_signed (and related variables like `data` and `data_len`) so
they reflect the real struct fields (`source`, `destination`, `amount`,
`authority`, optional `fee_payer`) and correct byte layout.
ananas-block
force-pushed
the
jorrit/feat-light-pda-pinocchio2
branch
from
5859036 to
df4d842
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs`:
- Around line 220-280: The code must validate that cpi_context and
cpi_context_account are provided together to avoid instruction-data vs
account-list mismatch: before calling instruction_data.data() and before
building account_metas/account_infos (in the function constructing the
MintAction instruction in create_mint.rs where instruction_data, account_metas
and account_infos are created), add a consistency check that returns an
appropriate error (e.g., ProgramError::InvalidArgument or a custom error) if
self.cpi_context.is_some() != self.cpi_context_account.is_some(); only proceed
to append the optional cpi_context_account to account_metas/account_infos when
both are Some.
- Around line 185-218: The builder allows rent_payment values of 1 but the docs
require rent_payment == 0 or >= 2; add a guard before constructing/adding the
DecompressMintAction (check self.params.rent_payment) and return an error (e.g.,
ProgramError::InvalidArgument or a more specific error) if rent_payment == 1, so
that with_decompress_mint(DecompressMintAction { rent_payment:
self.params.rent_payment, ... }) is never called with an invalid value; update
the error message to clearly state "rent_payment must be 0 or >= 2" and
reference rent_payment, with_decompress_mint, and DecompressMintAction in the
change.
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs`:
- Around line 18-32: Update the module docs to include the missing checked
variants: add `MintToCheckedCpi` to the "Mint Operations" list (alongside
`MintToCpi`) with a short note like "checked minting with decimal validation",
and add `BurnCheckedCpi` to the "Other Operations" list (alongside `BurnCpi`)
with a note like "checked burn with decimal validation"; reference the existing
exported symbols `MintToCheckedCpi` and `BurnCheckedCpi` so readers can find
their implementations.
- Line 89: Move the private import use pinocchio::account_info::AccountInfo; out
of the pub re-export block and place it with the other private imports at the
top of the file (after any mod declarations) so it is grouped separately from
public re-exports; update the import order in mod.rs to keep pub use ... lines
together and private use statements (including AccountInfo) in a contiguous
private-import section for clearer visual flow.
Comment on lines
+185
to
+218
| // Validate mint_authority matches authority account | ||
| if self.params.mint_authority != *self.authority.key() { | ||
| return Err(ProgramError::InvalidAccountData); | ||
| } | ||
|
|
||
| // Build MintInstructionData | ||
| let mint_instruction_data = | ||
| light_token_interface::instructions::mint_action::MintInstructionData { | ||
| supply: 0, | ||
| decimals: self.params.decimals, | ||
| metadata: MintMetadata { | ||
| version: 3, | ||
| mint: self.params.mint.into(), | ||
| mint_decompressed: false, | ||
| mint_signer: *self.mint_seed.key(), | ||
| bump: self.params.bump, | ||
| }, | ||
| mint_authority: Some(self.params.mint_authority.into()), | ||
| freeze_authority: self.params.freeze_authority.map(|auth| auth.into()), | ||
| extensions: self.params.extensions.clone(), | ||
| }; | ||
|
|
||
| // Build instruction data | ||
| let mut instruction_data = MintActionCompressedInstructionData::new_mint( | ||
| self.params.address_merkle_tree_root_index, | ||
| self.params.proof, | ||
| mint_instruction_data, | ||
| ); | ||
|
|
||
| // Always add decompress action to create Mint Solana account | ||
| instruction_data = instruction_data.with_decompress_mint(DecompressMintAction { | ||
| rent_payment: self.params.rent_payment, | ||
| write_top_up: self.params.write_top_up, | ||
| }); |
Contributor
There was a problem hiding this comment.
Validate rent_payment against its documented constraints.
The doc comment says “must be 0 or >= 2,” but the builder currently accepts 1 silently. Add a guard to fail fast with a clear error.
💡 Suggested fix
@@
fn build_instruction_inner(
&self,
) -> Result<(Vec<u8>, Vec<AccountMeta<'_>>, Vec<&AccountInfo>), ProgramError> {
+ if self.params.rent_payment == 1 {
+ return Err(ProgramError::InvalidInstructionData);
+ }
// Validate mint_authority matches authority account
if self.params.mint_authority != *self.authority.key() {
return Err(ProgramError::InvalidAccountData);
}🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs` around lines 185 -
218, The builder allows rent_payment values of 1 but the docs require
rent_payment == 0 or >= 2; add a guard before constructing/adding the
DecompressMintAction (check self.params.rent_payment) and return an error (e.g.,
ProgramError::InvalidArgument or a more specific error) if rent_payment == 1, so
that with_decompress_mint(DecompressMintAction { rent_payment:
self.params.rent_payment, ... }) is never called with an invalid value; update
the error message to clearly state "rent_payment must be 0 or >= 2" and
reference rent_payment, with_decompress_mint, and DecompressMintAction in the
change.
Comment on lines
+220
to
+280
| if let Some(ctx) = &self.cpi_context { | ||
| instruction_data = instruction_data.with_cpi_context(ctx.clone()); | ||
| } | ||
|
|
||
| let ix_data = instruction_data | ||
| .data() | ||
| .map_err(|_| ProgramError::BorshIoError)?; | ||
|
|
||
| // Build account metas and account infos in matching order | ||
| // Order matches MintActionMetaConfig::to_account_metas: | ||
| // 1. light_system_program | ||
| // 2. mint_seed (signer) | ||
| // 3. authority (signer) | ||
| // 4. compressible_config | ||
| // 5. mint (writable) | ||
| // 6. rent_sponsor (writable) | ||
| // 7. fee_payer (signer, writable) | ||
| // 8. cpi_authority_pda | ||
| // 9. registered_program_pda | ||
| // 10. account_compression_authority | ||
| // 11. account_compression_program | ||
| // 12. system_program | ||
| // [optional: cpi_context_account] | ||
| // 13. output_queue (writable) | ||
| // 14. address_tree (writable) | ||
|
|
||
| let mut account_metas = vec![ | ||
| AccountMeta::readonly(self.system_accounts.light_system_program.key()), | ||
| AccountMeta::readonly_signer(self.mint_seed.key()), | ||
| AccountMeta::readonly_signer(self.authority.key()), | ||
| AccountMeta::readonly(self.compressible_config.key()), | ||
| AccountMeta::writable(self.mint.key()), | ||
| AccountMeta::writable(self.rent_sponsor.key()), | ||
| AccountMeta::writable_signer(self.payer.key()), | ||
| AccountMeta::readonly(self.system_accounts.cpi_authority_pda.key()), | ||
| AccountMeta::readonly(self.system_accounts.registered_program_pda.key()), | ||
| AccountMeta::readonly(self.system_accounts.account_compression_authority.key()), | ||
| AccountMeta::readonly(self.system_accounts.account_compression_program.key()), | ||
| AccountMeta::readonly(self.system_accounts.system_program.key()), | ||
| ]; | ||
|
|
||
| let mut account_infos = vec![ | ||
| self.system_accounts.light_system_program, | ||
| self.mint_seed, | ||
| self.authority, | ||
| self.compressible_config, | ||
| self.mint, | ||
| self.rent_sponsor, | ||
| 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, | ||
| ]; | ||
|
|
||
| // Add optional cpi_context_account | ||
| if let Some(cpi_ctx_acc) = self.cpi_context_account { | ||
| account_metas.push(AccountMeta::writable(cpi_ctx_acc.key())); | ||
| account_infos.push(cpi_ctx_acc); | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against mismatched cpi_context vs cpi_context_account.
If a caller sets cpi_context without the corresponding account (or vice‑versa), the instruction data and account list get out of sync and the CPI fails at runtime. Add a consistency check.
💡 Suggested fix
@@
- if let Some(ctx) = &self.cpi_context {
+ if self.cpi_context.is_some() != self.cpi_context_account.is_some() {
+ return Err(ProgramError::NotEnoughAccountKeys);
+ }
+ if let Some(ctx) = &self.cpi_context {
instruction_data = instruction_data.with_cpi_context(ctx.clone());
}🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs` around lines 220 -
280, The code must validate that cpi_context and cpi_context_account are
provided together to avoid instruction-data vs account-list mismatch: before
calling instruction_data.data() and before building account_metas/account_infos
(in the function constructing the MintAction instruction in create_mint.rs where
instruction_data, account_metas and account_infos are created), add a
consistency check that returns an appropriate error (e.g.,
ProgramError::InvalidArgument or a custom error) if self.cpi_context.is_some()
!= self.cpi_context_account.is_some(); only proceed to append the optional
cpi_context_account to account_metas/account_infos when both are Some.
Comment on lines
+18
to
+32
| //! ## Mint Operations | ||
| //! | ||
| //! - [`CreateMintCpi`] - Create single compressed mint via CPI | ||
| //! - [`CreateMintsCpi`] - Batch create compressed mints via CPI | ||
| //! - [`DecompressMintCpi`] - Decompress compressed mint to Solana Mint account | ||
| //! - [`MintToCpi`] - Mint tokens to Light Token accounts | ||
| //! | ||
| //! ## Other Operations | ||
| //! | ||
| //! - [`ApproveCpi`] - Approve delegation | ||
| //! - [`RevokeCpi`] - Revoke delegation | ||
| //! - [`FreezeCpi`] - Freeze account | ||
| //! - [`ThawCpi`] - Thaw frozen account | ||
| //! - [`BurnCpi`] - Burn tokens | ||
| //! - [`CloseAccountCpi`] - Close Light Token account |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Documentation omits MintToCheckedCpi and BurnCheckedCpi.
The "Mint Operations" section (lines 18-24) lists MintToCpi but not MintToCheckedCpi (exported at line 88). Similarly, "Other Operations" (lines 25-32) lists BurnCpi but not BurnCheckedCpi (exported at line 57).
Based on learnings, MintToCheckedCpi is used for checked minting operations with decimal validation—worth documenting alongside the unchecked variant.
📝 Suggested documentation update
//! ## Mint Operations
//!
//! - [`CreateMintCpi`] - Create single compressed mint via CPI
//! - [`CreateMintsCpi`] - Batch create compressed mints via CPI
//! - [`DecompressMintCpi`] - Decompress compressed mint to Solana Mint account
//! - [`MintToCpi`] - Mint tokens to Light Token accounts
+//! - [`MintToCheckedCpi`] - Mint tokens with decimal validation
//!
//! ## Other Operations
//!
//! - [`ApproveCpi`] - Approve delegation
//! - [`RevokeCpi`] - Revoke delegation
//! - [`FreezeCpi`] - Freeze account
//! - [`ThawCpi`] - Thaw frozen account
//! - [`BurnCpi`] - Burn tokens
+//! - [`BurnCheckedCpi`] - Burn tokens with decimal validation
//! - [`CloseAccountCpi`] - Close Light Token account📝 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.
Suggested change
| //! ## Mint Operations | |
| //! | |
| //! - [`CreateMintCpi`] - Create single compressed mint via CPI | |
| //! - [`CreateMintsCpi`] - Batch create compressed mints via CPI | |
| //! - [`DecompressMintCpi`] - Decompress compressed mint to Solana Mint account | |
| //! - [`MintToCpi`] - Mint tokens to Light Token accounts | |
| //! | |
| //! ## Other Operations | |
| //! | |
| //! - [`ApproveCpi`] - Approve delegation | |
| //! - [`RevokeCpi`] - Revoke delegation | |
| //! - [`FreezeCpi`] - Freeze account | |
| //! - [`ThawCpi`] - Thaw frozen account | |
| //! - [`BurnCpi`] - Burn tokens | |
| //! - [`CloseAccountCpi`] - Close Light Token account | |
| //! ## Mint Operations | |
| //! | |
| //! - [`CreateMintCpi`] - Create single compressed mint via CPI | |
| //! - [`CreateMintsCpi`] - Batch create compressed mints via CPI | |
| //! - [`DecompressMintCpi`] - Decompress compressed mint to Solana Mint account | |
| //! - [`MintToCpi`] - Mint tokens to Light Token accounts | |
| //! - [`MintToCheckedCpi`] - Mint tokens with decimal validation | |
| //! | |
| //! ## Other Operations | |
| //! | |
| //! - [`ApproveCpi`] - Approve delegation | |
| //! - [`RevokeCpi`] - Revoke delegation | |
| //! - [`FreezeCpi`] - Freeze account | |
| //! - [`ThawCpi`] - Thaw frozen account | |
| //! - [`BurnCpi`] - Burn tokens | |
| //! - [`BurnCheckedCpi`] - Burn tokens with decimal validation | |
| //! - [`CloseAccountCpi`] - Close Light Token account |
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs` around lines 18 - 32, Update
the module docs to include the missing checked variants: add `MintToCheckedCpi`
to the "Mint Operations" list (alongside `MintToCpi`) with a short note like
"checked minting with decimal validation", and add `BurnCheckedCpi` to the
"Other Operations" list (alongside `BurnCpi`) with a note like "checked burn
with decimal validation"; reference the existing exported symbols
`MintToCheckedCpi` and `BurnCheckedCpi` so readers can find their
implementations.
| }; | ||
| pub use mint_to::*; | ||
| pub use mint_to_checked::*; | ||
| use pinocchio::account_info::AccountInfo; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving private import to top of file.
The private use pinocchio::account_info::AccountInfo; statement is sandwiched between pub use re-exports, which breaks the visual flow. Typically, private imports go at the top (after mod declarations) or are grouped separately from public re-exports.
mod transfer_to_spl;
+use pinocchio::account_info::AccountInfo;
+
pub use approve::*;
// ... other pub use statements ...
pub use mint_to_checked::*;
-use pinocchio::account_info::AccountInfo;
pub use revoke::*;🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs` at line 89, Move the private
import use pinocchio::account_info::AccountInfo; out of the pub re-export block
and place it with the other private imports at the top of the file (after any
mod declarations) so it is grouped separately from public re-exports; update the
import order in mod.rs to keep pub use ... lines together and private use
statements (including AccountInfo) in a contiguous private-import section for
clearer visual flow.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs`:
- Around line 299-326: This file duplicates logic for find_mint_address,
derive_mint_compressed_address, and derive_compressed_address instead of using
the canonical implementations; replace the local implementations by delegating
to the canonical functions in light_sdk_types::interface::cpi::create_mints
(e.g. call or re-export
light_sdk_types::interface::cpi::create_mints::find_mint_address and
::derive_mint_compressed_address — and ::derive_compressed_address if available)
so the module matches the sibling modules and avoids divergence.
- Around line 311-318: The function derive_compressed_address is unused and
hardcodes ADDRESS_TREE_V2 which is a different contract than
derive_mint_compressed_address (which accepts an address tree for creating new
mints); either remove derive_compressed_address to eliminate dead code, or keep
it but add a clear doc comment on the function stating it is only for deriving
the compressed address of an existing mint already on ADDRESS_TREE_V2 and show
when SDK consumers should prefer it over derive_mint_compressed_address; if
removing, also delete any re-exports or references to derive_compressed_address
to avoid compile errors.
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs`:
- Around line 89-109: The SystemAccountInfos struct is not re-exported, so
consumers can't construct the type when it's used as a field on exported types
like CreateMintCpi; open instruction/mod.rs and add a pub use for
SystemAccountInfos alongside the other CPI builder exports so the struct is
publicly accessible (i.e. ensure SystemAccountInfos is exported from the
instruction module where CreateMintCpi is exported).
In `@sdk-libs/token-pinocchio/src/instruction/transfer_from_spl.rs`:
- Around line 87-102: Replace the hardcoded numeric account indices passed to
Compression::compress_spl and Compression::decompress with well-named constants
to match the packed_accounts ordering; define constants such as
PACKED_MINT_INDEX, PACKED_DESTINATION_INDEX, PACKED_AUTHORITY_INDEX,
PACKED_SOURCE_INDEX, and PACKED_POOL_INDEX and use them when calling
Compression::compress_spl(self.amount, ..., self.spl_interface_pda_bump,
self.decimals) and Compression::decompress(self.amount, ...), ensuring the same
names/values align with the ordering documented in the packed_accounts section
to prevent future misalignment.
- Around line 18-19: Replace the hardcoded TRANSFER2_DISCRIMINATOR with the
shared constant from light_token_interface: add an import use
light_token_interface::constants::TRANSFER2; remove the local const
TRANSFER2_DISCRIMINATOR and use TRANSFER2 anywhere it is referenced (e.g., in
instruction discriminator construction or matching) to avoid duplication; apply
the same change to the analogous file that defines transfer logic (the
transfer_to_spl.rs counterpart) so both files import and reference
light_token_interface::constants::TRANSFER2 consistently.
Comment on lines
+299
to
+326
| pub fn derive_mint_compressed_address( | ||
| mint_seed: &[u8; 32], | ||
| address_tree_pubkey: &[u8; 32], | ||
| ) -> [u8; 32] { | ||
| let (mint_pda, _) = find_mint_address(mint_seed); | ||
| light_compressed_account::address::derive_address( | ||
| &mint_pda, | ||
| address_tree_pubkey, | ||
| &LIGHT_TOKEN_PROGRAM_ID, | ||
| ) | ||
| } | ||
|
|
||
| /// Derives the compressed address from a Light mint address. | ||
| pub fn derive_compressed_address(mint: &[u8; 32]) -> [u8; 32] { | ||
| light_compressed_account::address::derive_address( | ||
| mint, | ||
| &ADDRESS_TREE_V2, | ||
| &LIGHT_TOKEN_PROGRAM_ID, | ||
| ) | ||
| } | ||
|
|
||
| /// Finds the compressed mint PDA address from a mint seed. | ||
| pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) { | ||
| AccountInfo::find_program_address( | ||
| &[COMPRESSED_MINT_SEED, mint_seed.as_ref()], | ||
| &LIGHT_TOKEN_PROGRAM_ID, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent implementation—delegate to light_sdk_types for consistency.
The sibling module create_mints.rs and account-pinocchio/src/lib.rs both delegate find_mint_address and derive_mint_compressed_address to light_sdk_types::interface::cpi::create_mints::*. This file has its own implementation, which risks divergence if the canonical logic changes.
Consider aligning with the other modules:
♻️ Suggested refactor
-/// Derives the Light Mint address from the mint seed and address tree.
-pub fn derive_mint_compressed_address(
- mint_seed: &[u8; 32],
- address_tree_pubkey: &[u8; 32],
-) -> [u8; 32] {
- let (mint_pda, _) = find_mint_address(mint_seed);
- light_compressed_account::address::derive_address(
- &mint_pda,
- address_tree_pubkey,
- &LIGHT_TOKEN_PROGRAM_ID,
- )
-}
+/// Derives the Light Mint address from the mint seed and address tree.
+pub fn derive_mint_compressed_address(
+ mint_seed: &[u8; 32],
+ address_tree_pubkey: &[u8; 32],
+) -> [u8; 32] {
+ light_sdk_types::interface::cpi::create_mints::derive_mint_compressed_address::<AccountInfo>(
+ mint_seed,
+ address_tree_pubkey,
+ )
+}
-/// Finds the compressed mint PDA address from a mint seed.
-pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) {
- AccountInfo::find_program_address(
- &[COMPRESSED_MINT_SEED, mint_seed.as_ref()],
- &LIGHT_TOKEN_PROGRAM_ID,
- )
-}
+/// Finds the compressed mint PDA address from a mint seed.
+pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) {
+ light_sdk_types::interface::cpi::create_mints::find_mint_address::<AccountInfo>(mint_seed)
+}🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs` around lines 299 -
326, This file duplicates logic for find_mint_address,
derive_mint_compressed_address, and derive_compressed_address instead of using
the canonical implementations; replace the local implementations by delegating
to the canonical functions in light_sdk_types::interface::cpi::create_mints
(e.g. call or re-export
light_sdk_types::interface::cpi::create_mints::find_mint_address and
::derive_mint_compressed_address — and ::derive_compressed_address if available)
so the module matches the sibling modules and avoids divergence.
Comment on lines
+311
to
+318
| /// Derives the compressed address from a Light mint address. | ||
| pub fn derive_compressed_address(mint: &[u8; 32]) -> [u8; 32] { | ||
| light_compressed_account::address::derive_address( | ||
| mint, | ||
| &ADDRESS_TREE_V2, | ||
| &LIGHT_TOKEN_PROGRAM_ID, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, understand the file structure and content
wc -l sdk-libs/token-pinocchio/src/instruction/create_mint.rsRepository: Lightprotocol/light-protocol
Length of output: 131
🏁 Script executed:
# Read the specific section and surrounding context
sed -n '300,330p' sdk-libs/token-pinocchio/src/instruction/create_mint.rsRepository: Lightprotocol/light-protocol
Length of output: 871
🏁 Script executed:
# Check for CLAUDE.md or documentation specific to token-pinocchio
find . -name "CLAUDE.md" -o -name "*.md" | grep -i "token-pinocchio\|token"Repository: Lightprotocol/light-protocol
Length of output: 4984
🏁 Script executed:
# Look for ADDRESS_TREE_V2 definition and usage patterns in token-pinocchio
rg "ADDRESS_TREE_V2" sdk-libs/token-pinocchio/ -n -B2 -A2Repository: Lightprotocol/light-protocol
Length of output: 962
🏁 Script executed:
# Check the derive_mint_compressed_address function mentioned in the comment
rg "derive_mint_compressed_address" sdk-libs/token-pinocchio/ -n -B5 -A5Repository: Lightprotocol/light-protocol
Length of output: 3570
🏁 Script executed:
# Search for usage of derive_compressed_address to understand its intended use case
rg "derive_compressed_address" sdk-libs/token-pinocchio/ -n -B3 -A3 --type rustRepository: Lightprotocol/light-protocol
Length of output: 715
🏁 Script executed:
# Check sdk-tests for guidance on token-pinocchio usage
cat sdk-tests/sdk-light-token-pinocchio/CLAUDE.md 2>/dev/null || echo "No CLAUDE.md found"Repository: Lightprotocol/light-protocol
Length of output: 90
🏁 Script executed:
# Look for any documentation or tests that show when to use derive_compressed_address vs derive_mint_compressed_address
rg "derive_compressed_address|derive_mint_compressed_address" . --type rust --type markdown -B2 -A2 | head -100Repository: Lightprotocol/light-protocol
Length of output: 9165
Clarify the purpose of derive_compressed_address—it's unused and hardcodes the standard address tree.
derive_compressed_address is designed for a distinct use case: deriving the compressed address from an existing mint that is already on ADDRESS_TREE_V2. In contrast, derive_mint_compressed_address accepts the address tree as a parameter because it's used when creating new mints where the caller specifies the tree. This design distinction is intentional.
However, derive_compressed_address is not used anywhere in the codebase, and there's no documentation explaining when SDK consumers should prefer it over the parameterized version. Either document the intended use case, or remove it if it's truly dead code.
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/create_mint.rs` around lines 311 -
318, The function derive_compressed_address is unused and hardcodes
ADDRESS_TREE_V2 which is a different contract than
derive_mint_compressed_address (which accepts an address tree for creating new
mints); either remove derive_compressed_address to eliminate dead code, or keep
it but add a clear doc comment on the function stating it is only for deriving
the compressed address of an existing mint already on ADDRESS_TREE_V2 and show
when SDK consumers should prefer it over derive_mint_compressed_address; if
removing, also delete any re-exports or references to derive_compressed_address
to avoid compile errors.
Comment on lines
+89
to
+109
| /// System accounts required for CPI operations to Light Protocol. | ||
| /// | ||
| /// Pass these accounts when invoking Light Token operations from your program. | ||
| /// | ||
| /// # Fields | ||
| /// | ||
| /// - `light_system_program` - Light System Program | ||
| /// - `cpi_authority_pda` - CPI authority (signs for your program) | ||
| /// - `registered_program_pda` - Your program's registration | ||
| /// - `account_compression_authority` - Compression authority | ||
| /// - `account_compression_program` - Account Compression Program | ||
| /// - `system_program` - Solana System Program | ||
| #[derive(Clone)] | ||
| pub struct SystemAccountInfos<'info> { | ||
| pub light_system_program: &'info AccountInfo, | ||
| pub cpi_authority_pda: &'info AccountInfo, | ||
| pub registered_program_pda: &'info AccountInfo, | ||
| pub account_compression_authority: &'info AccountInfo, | ||
| pub account_compression_program: &'info AccountInfo, | ||
| pub system_program: &'info AccountInfo, | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SystemAccountInfos is exported from lib.rs
echo "=== Checking lib.rs exports ==="
rg "SystemAccountInfos" sdk-libs/token-pinocchio/src/lib.rs
echo ""
echo "=== Checking all usage in token-pinocchio crate ==="
rg "SystemAccountInfos" sdk-libs/token-pinocchio/src --type rust -A 2 -B 2
echo ""
echo "=== Check what's exported from instruction/mod.rs ==="
rg "pub use|pub struct|pub fn" sdk-libs/token-pinocchio/src/instruction/mod.rs | head -20Repository: Lightprotocol/light-protocol
Length of output: 2632
🏁 Script executed:
#!/bin/bash
# Check what create_mint.rs exports and if SystemAccountInfos is public
echo "=== Check create_mint.rs for pub exports ==="
rg "pub (struct|use|fn)" sdk-libs/token-pinocchio/src/instruction/create_mint.rs | head -20
echo ""
echo "=== Check if SystemAccountInfos is in any pub statement ==="
rg "pub.*SystemAccountInfos" sdk-libs/token-pinocchio/src/instruction/create_mint.rsRepository: Lightprotocol/light-protocol
Length of output: 643
🏁 Script executed:
#!/bin/bash
# Check the structure of instruction/mod.rs to understand module organization
echo "=== Check instruction/mod.rs structure (first 50 lines) ==="
head -50 sdk-libs/token-pinocchio/src/instruction/mod.rs
echo ""
echo "=== Check if there are other account info structs exported ==="
rg "pub use.*Accounts|pub use.*AccountInfos" sdk-libs/token-pinocchio/src/instruction/mod.rs
echo ""
echo "=== Check if SystemAccountInfos is used in other modules ==="
rg "SystemAccountInfos" sdk-libs/token-pinocchio/src/instruction --type rust -lRepository: Lightprotocol/light-protocol
Length of output: 1869
Export SystemAccountInfos for API consistency.
The struct cleanly bundles the six system accounts for Light Protocol CPI operations with excellent field documentation. However, SystemAccountInfos is not explicitly exported from the instruction module's public API. Since it's referenced as a field in CreateMintCpi (which is exported), developers using that struct will struggle to construct the required parameter type. Add pub use for SystemAccountInfos in instruction/mod.rs alongside the other CPI builder exports to ensure convenient accessibility for consumers.
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/mod.rs` around lines 89 - 109, The
SystemAccountInfos struct is not re-exported, so consumers can't construct the
type when it's used as a field on exported types like CreateMintCpi; open
instruction/mod.rs and add a pub use for SystemAccountInfos alongside the other
CPI builder exports so the struct is publicly accessible (i.e. ensure
SystemAccountInfos is exported from the instruction module where CreateMintCpi
is exported).
Comment on lines
+18
to
+19
| /// Discriminator for Transfer2 instruction | ||
| const TRANSFER2_DISCRIMINATOR: u8 = 101; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for TRANSFER2 related constants and definitions
rg "TRANSFER2|transfer_2|Transfer2" --type rust -B 2 -A 2 | head -50
# Check if light-token-interface defines discriminators
fd "light.*token.*interface" --type d
# Look for instruction discriminator patterns in the token-pinocchio crate
rg "DISCRIMINATOR|discriminator" sdk-libs/token-pinocchio --type rust -B 1 -A 1Repository: Lightprotocol/light-protocol
Length of output: 12005
🏁 Script executed:
# Check what's in light-token-interface
fd . -path "*/light-token-interface/*" --type f | head -20Repository: Lightprotocol/light-protocol
Length of output: 240
🏁 Script executed:
# Search for anchor_lang::Discriminator usage patterns
rg "anchor_lang::Discriminator|derive.*Discriminator" --type rust | head -20Repository: Lightprotocol/light-protocol
Length of output: 2238
🏁 Script executed:
# Find the light-token program definition
fd "light.*token" --type f -path "*/programs/*" | head -20
# Search for transfer2 instruction definition in programs
rg "transfer2|Transfer2" -t rust programs/ --type-add 'rust:*.rs' -B 3 -A 3 | head -50Repository: Lightprotocol/light-protocol
Length of output: 4102
🏁 Script executed:
# Check if light-token is an Anchor program and how its instructions are defined
find . -path "*/programs/*token*" -name "lib.rs" -o -name "instruction.rs" 2>/dev/null | head -10Repository: Lightprotocol/light-protocol
Length of output: 768
🏁 Script executed:
# Search for where 101 discriminator is defined or referenced in programs
rg "\b101\b" programs/ --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 4090
🏁 Script executed:
# Check light-token-interface for discriminator exports
find . -path "*/light-token-interface*" -type f -name "*.rs" | head -20Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Search for where TRANSFER2_DISCRIMINATOR might be exported
rg "TRANSFER2_DISCRIMINATOR|pub.*101" --type rust | grep -v test | head -20Repository: Lightprotocol/light-protocol
Length of output: 1244
🏁 Script executed:
# Check the compressed-token program's public API
fd "lib.rs" -path "*/light-token-interface*" -o -path "*/token-interface*"Repository: Lightprotocol/light-protocol
Length of output: 240
🏁 Script executed:
# Look at what's actually exported from compressed-token/program/src
head -100 programs/compressed-token/program/src/lib.rsRepository: Lightprotocol/light-protocol
Length of output: 3749
🏁 Script executed:
# Check if token-interface constants are exported and importable
head -50 program-libs/token-interface/src/constants.rs
# Verify the export from token-interface lib.rs
rg "pub.*TRANSFER2|pub use.*constants" program-libs/token-interface/src/lib.rs -A 2 -B 2Repository: Lightprotocol/light-protocol
Length of output: 1956
🏁 Script executed:
# Check if token-pinocchio already imports from token-interface
head -30 sdk-libs/token-pinocchio/src/instruction/transfer_from_spl.rsRepository: Lightprotocol/light-protocol
Length of output: 910
Import TRANSFER2 constant from light_token_interface instead of hardcoding.
The constant TRANSFER2_DISCRIMINATOR duplicates the value defined in program-libs/token-interface/src/constants.rs as pub const TRANSFER2: u8 = 101. Since the file already imports from light_token_interface, import this constant directly:
use light_token_interface::constants::TRANSFER2;Then use TRANSFER2 throughout instead of the local TRANSFER2_DISCRIMINATOR. This same issue exists in transfer_to_spl.rs, so apply the fix consistently.
🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/transfer_from_spl.rs` around lines
18 - 19, Replace the hardcoded TRANSFER2_DISCRIMINATOR with the shared constant
from light_token_interface: add an import use
light_token_interface::constants::TRANSFER2; remove the local const
TRANSFER2_DISCRIMINATOR and use TRANSFER2 anywhere it is referenced (e.g., in
instruction discriminator construction or matching) to avoid duplication; apply
the same change to the analogous file that defines transfer logic (the
transfer_to_spl.rs counterpart) so both files import and reference
light_token_interface::constants::TRANSFER2 consistently.
Comment on lines
+87
to
+102
| let wrap_from_spl = Compression::compress_spl( | ||
| self.amount, | ||
| 0, // mint index | ||
| 3, // source index | ||
| 2, // authority index | ||
| 4, // pool_account_index | ||
| 0, // pool_index | ||
| self.spl_interface_pda_bump, | ||
| self.decimals, | ||
| ); | ||
|
|
||
| let unwrap_to_destination = Compression::decompress( | ||
| self.amount, | ||
| 0, // mint index | ||
| 1, // destination index | ||
| ); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Index alignment looks correct, but consider named constants for clarity.
The hardcoded indices (0, 3, 2, 4) in Compression::compress_spl and Compression::decompress must match the packed_accounts ordering documented in lines 131-141. While correct, using named constants would make maintenance easier and reduce risk of index misalignment during refactors.
const PACKED_MINT_INDEX: u8 = 0;
const PACKED_DESTINATION_INDEX: u8 = 1;
const PACKED_AUTHORITY_INDEX: u8 = 2;
const PACKED_SOURCE_INDEX: u8 = 3;
const PACKED_POOL_INDEX: u8 = 4;🤖 Prompt for AI Agents
In `@sdk-libs/token-pinocchio/src/instruction/transfer_from_spl.rs` around lines
87 - 102, Replace the hardcoded numeric account indices passed to
Compression::compress_spl and Compression::decompress with well-named constants
to match the packed_accounts ordering; define constants such as
PACKED_MINT_INDEX, PACKED_DESTINATION_INDEX, PACKED_AUTHORITY_INDEX,
PACKED_SOURCE_INDEX, and PACKED_POOL_INDEX and use them when calling
Compression::compress_spl(self.amount, ..., self.spl_interface_pda_bump,
self.decimals) and Compression::decompress(self.amount, ...), ensuring the same
names/values align with the ordering documented in the packed_accounts section
to prevent future misalignment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@sdk-libs/account/README.md`:
- Line 112: Add blank lines before and after each fenced Rust code block so
Markdown renders correctly: insert an empty line above and below the ```rust```
blocks referenced by the headings "**Without `init` (manual creation via
`CreateTokenAccountCpi`)**", "**With `init` (Anchor-created)**", and "**Without
`init` (manual creation via `CreateTokenAtaCpi`)**" in the README so each fenced
block is separated from surrounding text.
- Line 53: Update the README entry for the struct field `compression_info:
CompressionInfo` by adding a short paragraph that explains what data
`CompressionInfo` contains (e.g., compression algorithm, level, block/chunk
sizes, and any checksum/meta), how the Light Protocol uses it (e.g., to
negotiate or interpret compressed payloads and validate integrity), and whether
it must be initialized by the user or is populated/managed automatically
(clarify default behavior and any helper constructors or SDK functions that set
it). Mention relevant types/symbols (`CompressionInfo`, `compression_info`,
"Light Protocol") so readers can find the struct and link to
implementation/docs/functions that initialize or update it.
- Around line 1-179: Add missing conceptual and API documentation: expand the
README with a "Core Concepts" section explaining CompressionInfo,
CreateAccountsProof, compression_config and the cold/hot lifecycle mechanics;
add a "Programmatic API" section documenting methods LightAccount::new_init()
and LightAccount::new_mut(); add a "Generated Code" section describing what
#[derive(LightAccount)] and #[derive(LightAccounts)] produce (traits, fields
like compression_info, helper methods, and expected invariants); update examples
to include required imports such as use anchor_lang::prelude::*; and reorder
sections per the suggested structure (Introduction, Core Concepts, Quick Start,
Programmatic API, Advanced).
- Around line 1-179: README is missing programmatic API docs and generated-trait
explanations; add a "Programmatic API" section that documents
LightAccount::new_init() (usage for creating compressed accounts) and
LightAccount::new_mut() (usage for updating existing compressed accounts) and
add a "Generated Code" section that explains that deriving
LightAccounts/LightAccount produces helper traits LightPreInit and LightFinalize
(describe when and how to use those auto-generated traits in instruction
handlers). Reference the symbols LightAccount::new_init, LightAccount::new_mut,
LightPreInit, LightFinalize, and the derive macros
#[derive(LightAccounts)]/#[derive(LightAccount)] so readers can find the
generated APIs and examples of typical handler usage.
- Line 66: Add a new "Core Concepts" section before Quick Start that documents
CreateAccountsProof, compression_config, and pda_rent_sponsor: explain that
CreateAccountsProof is a client-supplied proof object (what it proves and where
to obtain it), describe compression_config (what configuration/account it points
to and how a user can look up or derive the correct address), and describe
pda_rent_sponsor (its role in paying rent, how it is derived and whether it is
the output of derive_light_rent_sponsor_pda!); include concrete usage notes for
clients (expected types/formats, common derivation function names, and where to
find helpers) so callers of CreateAccountsProof, compression_config, and
pda_rent_sponsor in the README understand how to obtain and use them.
- Around line 9-15: Update the "Light Accounts (PDAs)" section in README.md: fix
the duplicate list number (change the second "3." to "4.") and expand the
lifecycle paragraph(s) to clearly state (1) what exactly triggers compression
(e.g., when rent-exempt reserve is depleted or a rent-collection event causes
balance < rent-exempt threshold), (2) how clients load cold state (e.g.,
on-demand decompression APIs or RPC call that fetches compressed bytes and
verifies PDA via Anchor metadata), (3) how accounts transition back to hot
(e.g., an explicit "restore" or write operation that decompresses and funds
rent-exempt balance), and (4) performance/cost implications (latency and compute
cost for decompression vs storage savings, and any rate limits). Reference the
Light Accounts (PDAs) heading and the #[light_account(init)] annotation so
readers can locate the related code and behavior.
In `@sdk-libs/account/src/lib.rs`:
- Around line 419-424: Add a brief inline comment above each function that uses
AccountInfo<'static> (find_mint_address, derive_mint_compressed_address,
derive_associated_token_account) explaining that AccountInfo<'static> is used
purely as a compile-time type witness to satisfy the generic signature of
light_sdk_types::interface::cpi::create_mints::... and that no runtime value is
constructed or used; mention that the 'static lifetime is chosen to avoid
borrowing issues and that this is intentional to clarify why a seemingly unused
concrete type appears in the call.
- Around line 354-371: Add a brief doc comment above the re-export of
AnchorDiscriminator as Discriminator explaining it is the Anchor-compatible
8-byte discriminator and that the LightDiscriminator item is different (used for
Light compressed account discriminators/trait); update the re-export line for
AnchorDiscriminator (Discriminator) to include that comment so consumers can
easily distinguish it from LightDiscriminator.
- Around line 10-14: The module-level doc comment contains a numbered list with
two items labeled "3."; update the sequence in the top-level doc comment (the
lines beginning with "//!") so the list reads 1 through 6 in order (rename the
second "3." to "4." and increment subsequent items accordingly) to restore
correct numbering in the documentation.
sdk-libs/account/README.md
Outdated
Comment on lines
+9
to
+15
| **Light Accounts (PDAs)** | ||
| 1. Create a Solana PDA normally (Anchor `init`) | ||
| 2. Add `#[light_account(init)]` - becomes a Light Account | ||
| 3. Use it as normal Solana account | ||
| 3. When rent runs out, account compresses (cold state) | ||
| 4. State preserved on-chain, client loads when needed (hot state) | ||
| 5. When account is hot, use it as normal Solana account |
Contributor
There was a problem hiding this comment.
Clarify the cold/hot lifecycle mechanics and fix markdown list numbering.
The cold/hot lifecycle is the core value proposition of Light Accounts, but the mechanics are unclear:
- What triggers compression when "rent runs out"?
- How do clients "load" cold state?
- When and how do accounts transition back to hot state?
- What are the performance/cost implications?
Additionally, line 13 has a duplicate list item number "3" (should be "4").
Would you like me to draft a clearer explanation of the lifecycle mechanics?
🔧 Proposed fix for markdown list numbering
2. Add `#[light_account(init)]` - becomes a Light Account
3. Use it as normal Solana account
-3. When rent runs out, account compresses (cold state)
-4. State preserved on-chain, client loads when needed (hot state)
-5. When account is hot, use it as normal Solana account
+4. When rent runs out, account compresses (cold state)
+5. State preserved on-chain, client loads when needed (hot state)
+6. When account is hot, use it as normal Solana account🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 13-13: Ordered list item prefix
Expected: 4; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 14-14: Ordered list item prefix
Expected: 5; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 15-15: Ordered list item prefix
Expected: 6; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
🤖 Prompt for AI Agents
In `@sdk-libs/account/README.md` around lines 9 - 15, Update the "Light Accounts
(PDAs)" section in README.md: fix the duplicate list number (change the second
"3." to "4.") and expand the lifecycle paragraph(s) to clearly state (1) what
exactly triggers compression (e.g., when rent-exempt reserve is depleted or a
rent-collection event causes balance < rent-exempt threshold), (2) how clients
load cold state (e.g., on-demand decompression APIs or RPC call that fetches
compressed bytes and verifies PDA via Anchor metadata), (3) how accounts
transition back to hot (e.g., an explicit "restore" or write operation that
decompresses and funds rent-exempt balance), and (4) performance/cost
implications (latency and compute cost for decompression vs storage savings, and
any rate limits). Reference the Light Accounts (PDAs) heading and the
#[light_account(init)] annotation so readers can locate the related code and
behavior.
| #[derive(Default, LightAccount)] | ||
| #[account] | ||
| pub struct UserRecord { | ||
| pub compression_info: CompressionInfo, // Required field |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explanation for the compression_info field.
The compression_info: CompressionInfo field is marked as required but not explained. Users need to understand:
- What data does
CompressionInfostore? - How is it used by the Light Protocol?
- Should users initialize it manually or is it auto-managed?
🤖 Prompt for AI Agents
In `@sdk-libs/account/README.md` at line 53, Update the README entry for the
struct field `compression_info: CompressionInfo` by adding a short paragraph
that explains what data `CompressionInfo` contains (e.g., compression algorithm,
level, block/chunk sizes, and any checksum/meta), how the Light Protocol uses it
(e.g., to negotiate or interpret compressed payloads and validate integrity),
and whether it must be initialized by the user or is populated/managed
automatically (clarify default behavior and any helper constructors or SDK
functions that set it). Mention relevant types/symbols (`CompressionInfo`,
`compression_info`, "Light Protocol") so readers can find the struct and link to
implementation/docs/functions that initialize or update it.
|
|
||
| #[derive(AnchorSerialize, AnchorDeserialize)] | ||
| pub struct CreateParams { | ||
| pub create_accounts_proof: CreateAccountsProof, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Document CreateAccountsProof, compression_config, and pda_rent_sponsor.
Several critical concepts are used but not explained:
- CreateAccountsProof (line 66): What is it? Where does the client obtain it? What does it prove?
- compression_config (line 77): What does this account configure? How do users obtain the correct address?
- pda_rent_sponsor (line 81): What is its role? How is it derived? Is this the output of
derive_light_rent_sponsor_pda!?
Users need this information to use the API correctly. Consider adding a "Core Concepts" section before Quick Start.
Also applies to: 76-81
🤖 Prompt for AI Agents
In `@sdk-libs/account/README.md` at line 66, Add a new "Core Concepts" section
before Quick Start that documents CreateAccountsProof, compression_config, and
pda_rent_sponsor: explain that CreateAccountsProof is a client-supplied proof
object (what it proves and where to obtain it), describe compression_config
(what configuration/account it points to and how a user can look up or derive
the correct address), and describe pda_rent_sponsor (its role in paying rent,
how it is derived and whether it is the output of
derive_light_rent_sponsor_pda!); include concrete usage notes for clients
(expected types/formats, common derivation function names, and where to find
helpers) so callers of CreateAccountsProof, compression_config, and
pda_rent_sponsor in the README understand how to obtain and use them.
| ### 3. Light Token Account (vault) | ||
|
|
||
| **With `init` (Anchor-created):** | ||
| ```rust |
Contributor
There was a problem hiding this comment.
Fix markdown formatting: add blank lines around fenced code blocks.
Static analysis detected missing blank lines before fenced code blocks at lines 112, 119, 128, and 135. Markdown renderers may not display these correctly.
📝 Proposed fixes
**With `init` (Anchor-created):**
+
```rust
---
**Without `init` (manual creation via `CreateTokenAccountCpi`):**
+
```rust
---
**With `init` (Anchor-created):**
+
```rust
---
**Without `init` (manual creation via `CreateTokenAtaCpi`):**
+
```rustAlso applies to: 119-119, 128-128, 135-135
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 112-112: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@sdk-libs/account/README.md` at line 112, Add blank lines before and after
each fenced Rust code block so Markdown renders correctly: insert an empty line
above and below the ```rust``` blocks referenced by the headings "**Without
`init` (manual creation via `CreateTokenAccountCpi`)**", "**With `init`
(Anchor-created)**", and "**Without `init` (manual creation via
`CreateTokenAtaCpi`)**" in the README so each fenced block is separated from
surrounding text.
Comment on lines
+419
to
+424
| #[cfg(feature = "token")] | ||
| pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) { | ||
| light_sdk_types::interface::cpi::create_mints::find_mint_address::<AccountInfo<'static>>( | ||
| mint_seed, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting the AccountInfo<'static> type witness pattern.
The use of AccountInfo<'static> here is a type witness to satisfy generic constraints—the actual value is never constructed. This is a valid pattern, but a brief comment would help future maintainers understand why 'static is used:
#[cfg(feature = "token")]
pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) {
+ // AccountInfo<'static> serves as a type witness; no value is constructed.
light_sdk_types::interface::cpi::create_mints::find_mint_address::<AccountInfo<'static>>(
mint_seed,
)
}The same applies to derive_mint_compressed_address (line 432) and derive_associated_token_account (line 443).
📝 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.
Suggested change
| #[cfg(feature = "token")] | |
| pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) { | |
| light_sdk_types::interface::cpi::create_mints::find_mint_address::<AccountInfo<'static>>( | |
| mint_seed, | |
| ) | |
| } | |
| #[cfg(feature = "token")] | |
| pub fn find_mint_address(mint_seed: &[u8; 32]) -> ([u8; 32], u8) { | |
| // AccountInfo<'static> serves as a type witness; no value is constructed. | |
| light_sdk_types::interface::cpi::create_mints::find_mint_address::<AccountInfo<'static>>( | |
| mint_seed, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@sdk-libs/account/src/lib.rs` around lines 419 - 424, Add a brief inline
comment above each function that uses AccountInfo<'static> (find_mint_address,
derive_mint_compressed_address, derive_associated_token_account) explaining that
AccountInfo<'static> is used purely as a compile-time type witness to satisfy
the generic signature of light_sdk_types::interface::cpi::create_mints::... and
that no runtime value is constructed or used; mention that the 'static lifetime
is chosen to avoid borrowing issues and that this is intentional to clarify why
a seemingly unused concrete type appears in the call.
SwenSchaeferjohann
approved these changes
Feb 3, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
sdk-libs/client/src/interface/instructions.rs (3)
144-155:⚠️ Potential issue | 🔴 CriticalHardcoded
0u16inupdate_configPDA derivation will fail for non-zero config bumps.The
initialize_configfunction accepts aconfig_bumpparameter (Lines 100-110) and uses it for PDA derivation. However,update_confighardcodes0u16at Line 153. If a config was initialized with a non-zero bump, this function will derive the wrong PDA and the instruction will fail.The fix should accept
config_bumpas a parameter, mirroringinitialize_config:🐛 Proposed fix
pub fn update_config( program_id: &Pubkey, discriminator: &[u8], authority: &Pubkey, new_rent_sponsor: Option<Pubkey>, new_address_space: Option<Vec<Pubkey>>, new_update_authority: Option<Pubkey>, + config_bump: Option<u8>, ) -> Instruction { + let config_bump = config_bump.unwrap_or(0); + let config_bump_u16 = config_bump as u16; let (config_pda, _) = Pubkey::find_program_address( - &[light_account::LIGHT_CONFIG_SEED, &0u16.to_le_bytes()], + &[light_account::LIGHT_CONFIG_SEED, &config_bump_u16.to_le_bytes()], program_id, );
272-280:⚠️ Potential issue | 🟡 MinorPotential u8 overflow on
full_offsetandtoken_accounts_offset.
full_offsetis computed asprogram_account_metas.len() + system_accounts_offsetand cast tou8. With many accounts, this could overflow. Similarly,pda_indices.len()cast tou8could overflow. Consider adding bounds checks or usingu8::try_from()with proper error handling.🛡️ Proposed defensive check
- let full_offset = program_account_metas.len() + system_accounts_offset; - let token_accounts_offset = pda_indices.len() as u8; + let full_offset = u8::try_from(program_account_metas.len() + system_accounts_offset) + .map_err(|_| "system_accounts_offset exceeds u8::MAX")?; + let token_accounts_offset = u8::try_from(pda_indices.len()) + .map_err(|_| "token_accounts_offset exceeds u8::MAX")?; let ix_data = LoadAccountsData { proof: proof.proof, compressed_accounts: typed_accounts, - system_accounts_offset: full_offset as u8, + system_accounts_offset: full_offset, token_accounts_offset, output_queue_index: output_state_tree_index, };
335-341:⚠️ Potential issue | 🟡 MinorSame u8 overflow concern in
build_compress_accounts_idempotent.The
full_offsetcalculation and cast tou8at Line 340 has the same potential overflow issue as in the decompress function.🛡️ Proposed defensive check
- let full_offset = program_account_metas.len() + system_accounts_offset; + let full_offset = u8::try_from(program_account_metas.len() + system_accounts_offset) + .map_err(|_| "system_accounts_offset exceeds u8::MAX")?; let ix_data = SaveAccountsData { proof: proof.proof, compressed_accounts: cold_metas, - system_accounts_offset: full_offset as u8, + system_accounts_offset: full_offset, };sdk-libs/macros/src/light_pdas/program/instructions.rs (1)
112-215:⚠️ Potential issue | 🟠 MajorToken‑only programs lose variant generation when no PDA seeds.
The enum/trait generation falls back to the mint‑only placeholder whenever
pda_ctx_seedsis empty. That also captures token‑only programs (token seeds present, no PDAs), soLightAccountVariantnever includes token variants andis_packed_token()can never return true. This breaks token‑only compression/decompression flows.✅ Proposed fix (apply to both Anchor + Pinocchio generators)
- let enum_and_traits = if pda_ctx_seeds.is_empty() { + let enum_and_traits = if pda_ctx_seeds.is_empty() && !has_token_seeds_early { // mint-only placeholder quote! { /* ... */ } } else { let builder = LightVariantBuilder::new(&pda_ctx_seeds); let builder = if let Some(ref token_seed_specs) = token_seeds { if !token_seed_specs.is_empty() { builder.with_token_seeds(token_seed_specs) } else { builder } } else { builder }; builder.build()? };- let enum_and_traits = if pda_ctx_seeds.is_empty() { + let enum_and_traits = if pda_ctx_seeds.is_empty() && !has_token_seeds_early { quote! { /* ... */ } } else { let builder = LightVariantBuilder::new(&pda_ctx_seeds); let builder = if let Some(ref token_seed_specs) = token_seeds { if !token_seed_specs.is_empty() { builder.with_token_seeds(token_seed_specs) } else { builder } } else { builder }; builder.build_pinocchio()? };Also applies to: 1036-1130
sdk-libs/macros/src/light_pdas/program/decompress.rs (1)
256-365:⚠️ Potential issue | 🟠 MajorCtx seed fields must use SDK-specific Pubkey types—
[u8; 32]for pinocchio,solana_pubkey::Pubkeyfor solana.The hardcoded
solana_pubkey::Pubkeyin ctx field declarations violates the documented SDK-specific type requirement. In pinocchio mode, this creates an unnecessary dependency and mismatches expectations for raw byte keys. Theis_pinocchioflag is already available; use it to conditionally generate the correct type.🛠️ Suggested fix
- let ctx_fields_decl: Vec<_> = ctx_fields - .iter() - .map(|field| { - quote! { pub `#field`: solana_pubkey::Pubkey } - }) - .collect(); + let pubkey_ty = if is_pinocchio { + quote! { [u8; 32] } + } else { + quote! { solana_pubkey::Pubkey } + }; + let ctx_fields_decl: Vec<_> = ctx_fields + .iter() + .map(|field| { + quote! { pub `#field`: `#pubkey_ty` } + }) + .collect();sdk-libs/macros/src/light_pdas/account/derive.rs (1)
744-828: 🧹 Nitpick | 🔵 TrivialAdd a Pinocchio codegen test to lock in the new path.
Current tests only exercise
derive_light_account(Anchor). Adding a small test forderive_light_pinocchio_accountwould guard against regressions in the new framework path.✅ Example test
#[test] fn test_light_account_basic() { let input: DeriveInput = parse_quote! { pub struct UserRecord { pub compression_info: CompressionInfo, pub owner: Pubkey, pub name: String, pub score: u64, } }; @@ } + +#[test] +fn test_light_account_pinocchio_basic() { + let input: DeriveInput = parse_quote! { + pub struct UserRecord { + pub compression_info: CompressionInfo, + pub owner: [u8; 32], + pub name: String, + pub score: u64, + } + }; + let result = derive_light_pinocchio_account(input); + assert!(result.is_ok(), "LightAccount Pinocchio should succeed"); + let output = result.unwrap().to_string(); + assert!( + output.contains("impl light_account_pinocchio :: LightAccount"), + "Should implement Pinocchio LightAccount trait" + ); +}
🤖 Fix all issues with AI agents
In `@sdk-libs/account/README.md`:
- Around line 17-20: Update the "Light Token Accounts" bullets to show the
correct attribute syntax including the required module-path arguments for the
associated_token and token variants: mention the macro attribute name
`light_account` with the `init` argument and show that `associated_token` and
`token` require `::...` style arguments (i.e., update the bullets that currently
show `#[light_account(init, associated_token, ...)]` and `#[light_account(init,
token, ...)]` to the real forms that include the `::`-qualified arguments so
readers see the exact attribute usage).
In `@sdk-libs/client/src/interface/instructions.rs`:
- Around line 103-110: The code converts config_bump (received as Option<u8>) to
u16 before using it as a PDA seed in Pubkey::find_program_address with
light_account::LIGHT_CONFIG_SEED and program_id; confirm whether widening to u16
is intentional — if yes, update the function signature to accept Option<u16>
(and propagate that type through the call sites) or add a comment documenting
the design decision and why a u16 seed is required, otherwise change the
conversion to use the original u8 (or validate and error) so the API type
matches the actual seed width.
- Around line 162-169: The UpdateLightConfigParams is being constructed with
new_compression_authority, new_rent_config, and new_write_top_up hardcoded to
None; either add explicit parameters to the surrounding function signature
(e.g., new_compression_authority: Option<Type>, new_rent_config: Option<Type>,
new_write_top_up: Option<Type>) and wire them into the struct construction
(mapping to bytes or the appropriate representation, similar to
new_update_authority.map(|p| p.to_bytes())), or if these fields must remain
immutable, add a short doc comment in the function explaining that
new_compression_authority, new_rent_config, and new_write_top_up are
intentionally not updateable via this builder so future maintainers aren’t
confused.
In `@sdk-libs/macros/src/light_pdas/account/derive.rs`:
- Around line 279-323: The Pinocchio branch in derive.rs currently treats all
structs as zero-copy by emitting AccountType::PdaZeroCopy and using
core::mem::size_of::<#struct_name>() which undercounts variable-length fields;
change the logic in the Framework::Pinocchio arm of the match to detect
non–fixed-size types and either (a) emit a compile-time error/assert that the
struct is fully fixed-size (no String/Vec/Option<[u8]>/slices) so zero-copy is
safe, or (b) for non-fixed-size structs emit AccountType::Pda and use the anchor
INIT_SPACE calculation (e.g., <Self as anchor_lang::Space>::INIT_SPACE or a
computed serialized-size expression) instead of core::mem::size_of::<Self>();
update the size_assertion, account_type_token, and init_space_token generation
in the Framework::Pinocchio branch accordingly and reference
Framework::Pinocchio, AccountType::PdaZeroCopy,
core::mem::size_of::<#struct_name>(), and <Self as
anchor_lang::Space>::INIT_SPACE when locating the code to change.
In `@sdk-libs/macros/src/light_pdas/light_account_keywords.rs`:
- Around line 25-29: Update the module-level documentation and the docs for
valid_keys_for_namespace to include the new pda:: namespace and its keys;
specifically mention "pda" in the syntax list and document that valid keys for
the pda:: namespace are "seeds" and "zero_copy" (referencing the
PDA_NAMESPACE_KEYS constant) so users of the LightProgram derive and the
#[light_account(pda::...)] attribute see the new namespace; ensure any examples
or lists that previously enumerated namespaces are updated to include pda and
briefly describe what seeds and zero_copy mean.
In `@sdk-libs/macros/src/light_pdas/program/instructions.rs`:
- Around line 1398-1415: The account order is inverted: in process_update_config
you should treat accounts[0] as config and accounts[1] as update_authority to
match Anchor and initialize_config; change the bindings so config = &accounts[0]
and authority = &accounts[1], and build remaining = [*config, *authority] before
calling light_account_pinocchio::process_update_light_config so the CPI receives
(config, update_authority) in the correct order.
In `@sdk-libs/sdk-types/src/interface/cpi/create_mints.rs`:
- Around line 201-216: The validate method in CreateMintsCpi must reject cases
where the CPI context index would overflow the u8 assigned_account_index:
compute the total = (self.cpi_context_offset as
usize).checked_add(self.params.mints.len()). If total is None or total >
(u8::MAX as usize) return Err(LightSdkTypesError::InvalidInstructionData) (or a
more specific error if available); otherwise proceed with the existing length
checks. This prevents the casts in invoke_cpi_write/invoke_execute from wrapping
and corrupting CPI ordering.
- Around line 854-874: The function get_output_queue_next_index currently reads
a u64 and casts to u32 without checking for overflow; update it to detect when
the read u64 is greater than u32::MAX and return a descriptive error (e.g., add
or reuse a LightSdkTypesError variant like QueueIndexOverflow or
IntegerOverflow) instead of silently truncating. Locate
get_output_queue_next_index and the NEXT_INDEX_OFFSET constant, read the u64 as
before, then add a guard such as if next_index > u64::from(u32::MAX) { return
Err(LightSdkTypesError::QueueIndexOverflow) } (or map to an existing error),
otherwise safely cast to u32 and return. Ensure the new error variant is
declared in LightSdkTypesError and used by callers as needed.
In `@sdk-libs/sdk-types/src/interface/program/compression/processor.rs`:
- Around line 77-90: The code indexes remaining_accounts at fixed indices
(CONFIG_INDEX, RENT_SPONSOR_INDEX and later uses at lines ~136-137) without
checking remaining_accounts.len(), which can panic on malformed input; add a
guard near the top of the function that verifies remaining_accounts.len() is >=
max_required_index+1 (where max_required_index is the highest fixed index used,
e.g., CONFIG_INDEX and RENT_SPONSOR_INDEX and the later two indices), and return
Err(LightSdkTypesError::InvalidInstructionData) if the check fails before
calling LightConfig::load_checked or config.validate_rent_sponsor_account::<AI>
and before any other fixed-index accesses of remaining_accounts. Ensure the
check references the exact symbols remaining_accounts, CONFIG_INDEX,
RENT_SPONSOR_INDEX, LightConfig::load_checked and
config.validate_rent_sponsor_account::<AI> so reviewers can find and validate
the change.
- Around line 77-95: The code slices remaining_accounts using
system_accounts_offset but doesn't validate that system_accounts_offset is
within valid bounds, which can panic or misclassify accounts; before doing the
slice (after computing system_accounts_offset and pda_start) check that
system_accounts_offset is >= (RENT_SPONSOR_INDEX + 1) and <= pda_start, and if
not return an appropriate error (e.g., LightSdkTypesError::NotEnoughAccountKeys
or InvalidInstructionData); update the logic around variables
system_accounts_offset, pda_start, remaining_accounts, and constants
RENT_SPONSOR_INDEX so the slice
remaining_accounts[system_accounts_offset..pda_start] is only performed after
this bounds check.
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs`:
- Around line 161-180: The code indexes remaining_accounts at CONFIG_INDEX,
RENT_SPONSOR_INDEX and FEE_PAYER_INDEX without ensuring remaining_accounts has
at least those entries; add an explicit minimum-account-length check (e.g.
ensure remaining_accounts.len() > max(CONFIG_INDEX, RENT_SPONSOR_INDEX,
FEE_PAYER_INDEX)) before calling LightConfig::load_checked or
validate_rent_sponsor_account, and return
LightSdkTypesError::InvalidInstructionData if the check fails so the subsequent
direct indexing of remaining_accounts (used by system_accounts_offset,
CONFIG_INDEX, RENT_SPONSOR_INDEX, FEE_PAYER_INDEX) cannot panic.
- Around line 286-342: Extract the magic indices 3 and 6 into named constants
(e.g., CTOKEN_RENT_SPONSOR_INDEX and CTOKEN_COMPRESSIBLE_CONFIG_INDEX) and use
them when computing ctoken_rent_sponsor and ctoken_compressible_config instead
of get(3)/get(6); also add a pre-check before slicing/indexing that
remaining_accounts.len() meets the minimum required count for the token path (at
least the number of system+hot+token accounts, e.g., 7) so the
.get(...).ok_or(...) won't silently rely on hardcoded literals—update references
in the same function where FEE_PAYER_INDEX, CONFIG_INDEX, RENT_SPONSOR_INDEX,
ctoken_rent_sponsor, ctoken_compressible_config, and
CpiAccounts::new_with_config are used.
Comment on lines
+17
to
+20
| **Light Token Accounts (associated token accounts, Vaults)** | ||
| - Use `#[light_account(init, associated_token, ...)]` for associated token accounts | ||
| - Use `#[light_account(init, token, ...)]` for program-owned vaults | ||
| - Cold/hot lifecycle |
Contributor
There was a problem hiding this comment.
Fix attribute syntax in the “Light Token Accounts” bullets.
The prose shows associated_token / token without the required ::... arguments, which is inconsistent with actual macro usage and could mislead readers. Recommend updating the bullets to reflect the real attribute forms.
✅ Suggested edit
-**Light Token Accounts (associated token accounts, Vaults)**
-- Use `#[light_account(init, associated_token, ...)]` for associated token accounts
-- Use `#[light_account(init, token, ...)]` for program-owned vaults
+**Light Token Accounts (associated token accounts, Vaults)**
+- Use `#[light_account(init, associated_token::...)]` for associated token accounts
+- Use `#[light_account(init, token::...)]` for 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.
Suggested change
| **Light Token Accounts (associated token accounts, Vaults)** | |
| - Use `#[light_account(init, associated_token, ...)]` for associated token accounts | |
| - Use `#[light_account(init, token, ...)]` for program-owned vaults | |
| - Cold/hot lifecycle | |
| **Light Token Accounts (associated token accounts, Vaults)** | |
| - Use `#[light_account(init, associated_token::...)]` for associated token accounts | |
| - Use `#[light_account(init, token::...)]` for program-owned vaults | |
| - Cold/hot lifecycle |
🤖 Prompt for AI Agents
In `@sdk-libs/account/README.md` around lines 17 - 20, Update the "Light Token
Accounts" bullets to show the correct attribute syntax including the required
module-path arguments for the associated_token and token variants: mention the
macro attribute name `light_account` with the `init` argument and show that
`associated_token` and `token` require `::...` style arguments (i.e., update the
bullets that currently show `#[light_account(init, associated_token, ...)]` and
`#[light_account(init, token, ...)]` to the real forms that include the
`::`-qualified arguments so readers see the exact attribute usage).
Comment on lines
+103
to
+110
| let config_bump_u16 = config_bump as u16; | ||
| let (config_pda, _) = Pubkey::find_program_address( | ||
| &[ | ||
| light_account::LIGHT_CONFIG_SEED, | ||
| &config_bump_u16.to_le_bytes(), | ||
| ], | ||
| program_id, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Confirm the u8→u16 conversion for config_bump is intentional.
The config_bump is received as Option<u8> but converted to u16 for the PDA seed. This is presumably to support a larger namespace of config instances per program. If so, consider documenting this design choice or updating the parameter type to Option<u16> to make the intent explicit and avoid the narrowing at the API boundary.
🤖 Prompt for AI Agents
In `@sdk-libs/client/src/interface/instructions.rs` around lines 103 - 110, The
code converts config_bump (received as Option<u8>) to u16 before using it as a
PDA seed in Pubkey::find_program_address with light_account::LIGHT_CONFIG_SEED
and program_id; confirm whether widening to u16 is intentional — if yes, update
the function signature to accept Option<u16> (and propagate that type through
the call sites) or add a comment documenting the design decision and why a u16
seed is required, otherwise change the conversion to use the original u8 (or
validate and error) so the API type matches the actual seed width.
Comment on lines
+162
to
169
| let params = UpdateLightConfigParams { | ||
| new_update_authority: new_update_authority.map(|p| p.to_bytes()), | ||
| new_rent_sponsor: new_rent_sponsor.map(|p| p.to_bytes()), | ||
| new_compression_authority: None, | ||
| new_rent_config: None, | ||
| new_write_top_up: None, | ||
| new_address_space: new_address_space.map(|v| v.iter().map(|p| p.to_bytes()).collect()), | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
new_compression_authority, new_rent_config, new_write_top_up always None.
The UpdateLightConfigParams is built with these fields hardcoded to None. If the intent is to support updating these fields in the future, consider adding corresponding parameters to the function signature now. Otherwise, document that these fields are intentionally not updateable via this builder.
🤖 Prompt for AI Agents
In `@sdk-libs/client/src/interface/instructions.rs` around lines 162 - 169, The
UpdateLightConfigParams is being constructed with new_compression_authority,
new_rent_config, and new_write_top_up hardcoded to None; either add explicit
parameters to the surrounding function signature (e.g.,
new_compression_authority: Option<Type>, new_rent_config: Option<Type>,
new_write_top_up: Option<Type>) and wire them into the struct construction
(mapping to bytes or the appropriate representation, similar to
new_update_authority.map(|p| p.to_bytes())), or if these fields must remain
immutable, add a short doc comment in the function explaining that
new_compression_authority, new_rent_config, and new_write_top_up are
intentionally not updateable via this builder so future maintainers aren’t
confused.
Comment on lines
+279
to
+323
| let (size_assertion, account_type_token, init_space_token) = match framework { | ||
| Framework::Pinocchio => { | ||
| // Pinocchio always uses core::mem::size_of and PdaZeroCopy | ||
| ( | ||
| quote! { | ||
| const _: () = { | ||
| assert!( | ||
| core::mem::size_of::<#struct_name>() <= 800, | ||
| "Compressed account size exceeds 800 byte limit" | ||
| ); | ||
| }; | ||
| }, | ||
| quote! { #on_chain_crate::AccountType::PdaZeroCopy }, | ||
| quote! { core::mem::size_of::<Self>() }, | ||
| ) | ||
| } | ||
| Framework::Anchor => { | ||
| if is_zero_copy { | ||
| ( | ||
| quote! { | ||
| const _: () = { | ||
| assert!( | ||
| core::mem::size_of::<#struct_name>() <= 800, | ||
| "Compressed account size exceeds 800 byte limit" | ||
| ); | ||
| }; | ||
| }, | ||
| quote! { #on_chain_crate::AccountType::PdaZeroCopy }, | ||
| quote! { core::mem::size_of::<Self>() }, | ||
| ) | ||
| } else { | ||
| ( | ||
| quote! { | ||
| const _: () = { | ||
| assert!( | ||
| <#struct_name as anchor_lang::Space>::INIT_SPACE <= 800, | ||
| "Compressed account size exceeds 800 byte limit" | ||
| ); | ||
| }; | ||
| }, | ||
| quote! { #on_chain_crate::AccountType::Pda }, | ||
| quote! { <Self as anchor_lang::Space>::INIT_SPACE }, | ||
| ) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Pinocchio path under-allocates for variable-length fields.
The Pinocchio branch always emits AccountType::PdaZeroCopy and INIT_SPACE = core::mem::size_of::<Self>(). If a struct contains any variable-length fields (e.g., String, Vec, or other non–fixed-size types), size_of will undercount the serialized size and the account will be created too small while still marked zero-copy. That can cause serialization failures or corrupt data. Consider validating that Pinocchio LightAccount structs are fixed-size only, or introduce a size-calculation path for non-zero-copy Pinocchio accounts.
🛠️ One possible safeguard (compile-time error for non–fixed-size fields)
fn generate_light_account_impl(input: &DeriveInput, framework: Framework) -> Result<TokenStream> {
let struct_name = &input.ident;
let packed_struct_name = format_ident!("Packed{}", struct_name);
let fields = extract_fields_from_derive_input(input)?;
+
+ if framework == Framework::Pinocchio {
+ let has_non_fixed = fields.iter().any(|f| {
+ f.ident
+ .as_ref()
+ .is_none_or(|name| name != "compression_info")
+ && !is_copy_type(&f.ty)
+ });
+ if has_non_fixed {
+ return Err(syn::Error::new_spanned(
+ struct_name,
+ "Pinocchio LightAccount requires fixed-size fields; variable-length types are unsupported.",
+ ));
+ }
+ }🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/account/derive.rs` around lines 279 - 323, The
Pinocchio branch in derive.rs currently treats all structs as zero-copy by
emitting AccountType::PdaZeroCopy and using core::mem::size_of::<#struct_name>()
which undercounts variable-length fields; change the logic in the
Framework::Pinocchio arm of the match to detect non–fixed-size types and either
(a) emit a compile-time error/assert that the struct is fully fixed-size (no
String/Vec/Option<[u8]>/slices) so zero-copy is safe, or (b) for non-fixed-size
structs emit AccountType::Pda and use the anchor INIT_SPACE calculation (e.g.,
<Self as anchor_lang::Space>::INIT_SPACE or a computed serialized-size
expression) instead of core::mem::size_of::<Self>(); update the size_assertion,
account_type_token, and init_space_token generation in the Framework::Pinocchio
branch accordingly and reference Framework::Pinocchio, AccountType::PdaZeroCopy,
core::mem::size_of::<#struct_name>(), and <Self as
anchor_lang::Space>::INIT_SPACE when locating the code to change.
Comment on lines
+25
to
+29
| /// Valid keys for `pda::` namespace in `#[light_account(pda::...)]` attributes. | ||
| /// Used by `#[derive(LightProgram)]` enum variants. | ||
| /// - `seeds`: PDA seeds for account derivation | ||
| /// - `zero_copy`: Flag indicating zero-copy deserialization | ||
| pub const PDA_NAMESPACE_KEYS: &[&str] = &["seeds", "zero_copy"]; |
Contributor
There was a problem hiding this comment.
Document the new pda:: namespace in the module docs.
The syntax list and the valid_keys_for_namespace docs still omit pda, which can mislead macro users. Please update them to reflect the new namespace.
📝 Suggested doc update
//! All attribute parameters (except type markers) require a namespace prefix:
+//! - `pda::seeds`, `pda::zero_copy`
//! - `token::seeds`, `token::mint`, `token::owner`, `token::bump`
//! - `associated_token::authority`, `associated_token::mint`, `associated_token::bump`
//! - `mint::signer`, `mint::authority`, `mint::decimals`, `mint::seeds`, etc.
@@
-/// * `namespace` - One of "token", "associated_token", or "mint"
+/// * `namespace` - One of "pda", "token", "associated_token", or "mint"🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/light_account_keywords.rs` around lines 25 -
29, Update the module-level documentation and the docs for
valid_keys_for_namespace to include the new pda:: namespace and its keys;
specifically mention "pda" in the syntax list and document that valid keys for
the pda:: namespace are "seeds" and "zero_copy" (referencing the
PDA_NAMESPACE_KEYS constant) so users of the LightProgram derive and the
#[light_account(pda::...)] attribute see the new namespace; ensure any examples
or lists that previously enumerated namespaces are updated to include pda and
briefly describe what seeds and zero_copy mean.
Comment on lines
+854
to
+874
| /// Read the next_index from a batched output queue account. | ||
| /// | ||
| /// Offset 288 = 8 (discriminator) + 232 (QueueMetadata) + 48 (6 x u64 in QueueBatches). | ||
| /// This reads the raw bytes to avoid depending on `light-batched-merkle-tree`. | ||
| pub fn get_output_queue_next_index<AI: AccountInfoTrait>( | ||
| queue: &AI, | ||
| ) -> Result<u32, LightSdkTypesError> { | ||
| const NEXT_INDEX_OFFSET: usize = 288; | ||
| let data = queue | ||
| .try_borrow_data() | ||
| .map_err(LightSdkTypesError::AccountError)?; | ||
| if data.len() < NEXT_INDEX_OFFSET + 8 { | ||
| return Err(LightSdkTypesError::AccountDataTooSmall); | ||
| } | ||
| let next_index = u64::from_le_bytes( | ||
| data[NEXT_INDEX_OFFSET..NEXT_INDEX_OFFSET + 8] | ||
| .try_into() | ||
| .unwrap(), | ||
| ); | ||
| Ok(next_index as u32) | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid silent truncation of queue next_index.
next_index is read as u64 but cast to u32 without a bounds check. If the queue index can exceed u32::MAX, this will miscompute leaf indices. Add a guard and return an error on overflow.
🛡️ Suggested fix
let next_index = u64::from_le_bytes(
data[NEXT_INDEX_OFFSET..NEXT_INDEX_OFFSET + 8]
.try_into()
.unwrap(),
);
- Ok(next_index as u32)
+ if next_index > u32::MAX as u64 {
+ return Err(LightSdkTypesError::InvalidInstructionData);
+ }
+ Ok(next_index as u32)
}🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/cpi/create_mints.rs` around lines 854 - 874,
The function get_output_queue_next_index currently reads a u64 and casts to u32
without checking for overflow; update it to detect when the read u64 is greater
than u32::MAX and return a descriptive error (e.g., add or reuse a
LightSdkTypesError variant like QueueIndexOverflow or IntegerOverflow) instead
of silently truncating. Locate get_output_queue_next_index and the
NEXT_INDEX_OFFSET constant, read the u64 as before, then add a guard such as if
next_index > u64::from(u32::MAX) { return
Err(LightSdkTypesError::QueueIndexOverflow) } (or map to an existing error),
otherwise safely cast to u32 and return. Ensure the new error variant is
declared in LightSdkTypesError and used by callers as needed.
Comment on lines
+77
to
+90
| let system_accounts_offset = params.system_accounts_offset as usize; | ||
| let num_pdas = params.compressed_accounts.len(); | ||
|
|
||
| if num_pdas == 0 { | ||
| return Err(LightSdkTypesError::InvalidInstructionData); | ||
| } | ||
|
|
||
| // 2. Load and validate config | ||
| let config = LightConfig::load_checked(&remaining_accounts[CONFIG_INDEX], program_id)?; | ||
|
|
||
| // 3. Validate rent_sponsor | ||
| let rent_sponsor = &remaining_accounts[RENT_SPONSOR_INDEX]; | ||
| config.validate_rent_sponsor_account::<AI>(rent_sponsor)?; | ||
|
|
Contributor
There was a problem hiding this comment.
Guard against insufficient remaining_accounts before fixed-index access.
Line 85/88/136 index into remaining_accounts without a length check; malformed input can panic the program. Add a minimal length guard before those reads.
✅ Suggested fix
pub fn process_compress_pda_accounts_idempotent<AI: AccountInfoTrait + Clone>(
remaining_accounts: &[AI],
params: &CompressAndCloseParams,
@@
) -> Result<(), LightSdkTypesError> {
let system_accounts_offset = params.system_accounts_offset as usize;
let num_pdas = params.compressed_accounts.len();
if num_pdas == 0 {
return Err(LightSdkTypesError::InvalidInstructionData);
}
+
+ if remaining_accounts.len() <= RENT_SPONSOR_INDEX {
+ return Err(LightSdkTypesError::NotEnoughAccountKeys);
+ }Also applies to: 136-137
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/compression/processor.rs` around
lines 77 - 90, The code indexes remaining_accounts at fixed indices
(CONFIG_INDEX, RENT_SPONSOR_INDEX and later uses at lines ~136-137) without
checking remaining_accounts.len(), which can panic on malformed input; add a
guard near the top of the function that verifies remaining_accounts.len() is >=
max_required_index+1 (where max_required_index is the highest fixed index used,
e.g., CONFIG_INDEX and RENT_SPONSOR_INDEX and the later two indices), and return
Err(LightSdkTypesError::InvalidInstructionData) if the check fails before
calling LightConfig::load_checked or config.validate_rent_sponsor_account::<AI>
and before any other fixed-index accesses of remaining_accounts. Ensure the
check references the exact symbols remaining_accounts, CONFIG_INDEX,
RENT_SPONSOR_INDEX, LightConfig::load_checked and
config.validate_rent_sponsor_account::<AI> so reviewers can find and validate
the change.
Comment on lines
+77
to
+95
| let system_accounts_offset = params.system_accounts_offset as usize; | ||
| let num_pdas = params.compressed_accounts.len(); | ||
|
|
||
| if num_pdas == 0 { | ||
| return Err(LightSdkTypesError::InvalidInstructionData); | ||
| } | ||
|
|
||
| // 2. Load and validate config | ||
| let config = LightConfig::load_checked(&remaining_accounts[CONFIG_INDEX], program_id)?; | ||
|
|
||
| // 3. Validate rent_sponsor | ||
| let rent_sponsor = &remaining_accounts[RENT_SPONSOR_INDEX]; | ||
| config.validate_rent_sponsor_account::<AI>(rent_sponsor)?; | ||
|
|
||
| // 4. PDA accounts are at the tail of remaining_accounts | ||
| let pda_start = remaining_accounts | ||
| .len() | ||
| .checked_sub(num_pdas) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; |
Contributor
There was a problem hiding this comment.
Validate system_accounts_offset bounds before slicing.
Line 137 slices remaining_accounts[system_accounts_offset..pda_start]. If system_accounts_offset is before the fixed accounts or after pda_start, this panics and/or misclassifies accounts. Validate that it’s within [RENT_SPONSOR_INDEX + 1, pda_start].
✅ Suggested fix
let system_accounts_offset = params.system_accounts_offset as usize;
@@
- // 4. PDA accounts are at the tail of remaining_accounts
+ if system_accounts_offset < (RENT_SPONSOR_INDEX + 1) {
+ return Err(LightSdkTypesError::InvalidInstructionData);
+ }
+
+ // 4. PDA accounts are at the tail of remaining_accounts
let pda_start = remaining_accounts
.len()
.checked_sub(num_pdas)
.ok_or(LightSdkTypesError::NotEnoughAccountKeys)?;
+
+ if system_accounts_offset > pda_start {
+ return Err(LightSdkTypesError::NotEnoughAccountKeys);
+ }Also applies to: 134-138
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/compression/processor.rs` around
lines 77 - 95, The code slices remaining_accounts using system_accounts_offset
but doesn't validate that system_accounts_offset is within valid bounds, which
can panic or misclassify accounts; before doing the slice (after computing
system_accounts_offset and pda_start) check that system_accounts_offset is >=
(RENT_SPONSOR_INDEX + 1) and <= pda_start, and if not return an appropriate
error (e.g., LightSdkTypesError::NotEnoughAccountKeys or
InvalidInstructionData); update the logic around variables
system_accounts_offset, pda_start, remaining_accounts, and constants
RENT_SPONSOR_INDEX so the slice
remaining_accounts[system_accounts_offset..pda_start] is only performed after
this bounds check.
Comment on lines
+161
to
+180
| let system_accounts_offset = params.system_accounts_offset as usize; | ||
| if system_accounts_offset > remaining_accounts.len() { | ||
| return Err(LightSdkTypesError::InvalidInstructionData); | ||
| } | ||
|
|
||
| // PDA accounts: all accounts up to token_accounts_offset | ||
| let num_pda_accounts = params.token_accounts_offset as usize; | ||
| let pda_accounts = params | ||
| .accounts | ||
| .get(..num_pda_accounts) | ||
| .ok_or(LightSdkTypesError::InvalidInstructionData)?; | ||
|
|
||
| if pda_accounts.is_empty() { | ||
| return Err(LightSdkTypesError::InvalidInstructionData); | ||
| } | ||
|
|
||
| // 2. Load and validate config | ||
| let config = LightConfig::load_checked(&remaining_accounts[CONFIG_INDEX], program_id)?; | ||
| let rent_sponsor = &remaining_accounts[RENT_SPONSOR_INDEX]; | ||
| let rent_sponsor_bump = config.validate_rent_sponsor_account::<AI>(rent_sponsor)?; |
Contributor
There was a problem hiding this comment.
Add explicit validation for minimum required accounts before direct indexing.
The check at line 162 validates that system_accounts_offset doesn't exceed the array length, but direct index accesses at lines 178-180 (CONFIG_INDEX=1, RENT_SPONSOR_INDEX=2) and line 195 (FEE_PAYER_INDEX=0) assume remaining_accounts has at least 3 elements. If malformed instruction data provides a small system_accounts_offset with a short remaining_accounts array, these accesses will panic.
🛡️ Proposed fix to add minimum accounts validation
pub fn process_decompress_pda_accounts_idempotent<AI, V>(
remaining_accounts: &[AI],
params: &DecompressIdempotentParams<V>,
cpi_signer: CpiSigner,
program_id: &[u8; 32],
current_slot: u64,
) -> Result<(), LightSdkTypesError>
where
AI: AccountInfoTrait + Clone,
V: DecompressVariant<AI>,
{
let system_accounts_offset = params.system_accounts_offset as usize;
- if system_accounts_offset > remaining_accounts.len() {
+ // Minimum accounts: fee_payer, config, rent_sponsor
+ const MIN_ACCOUNTS: usize = 3;
+ if remaining_accounts.len() < MIN_ACCOUNTS || system_accounts_offset > remaining_accounts.len() {
return Err(LightSdkTypesError::InvalidInstructionData);
}🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs` around
lines 161 - 180, The code indexes remaining_accounts at CONFIG_INDEX,
RENT_SPONSOR_INDEX and FEE_PAYER_INDEX without ensuring remaining_accounts has
at least those entries; add an explicit minimum-account-length check (e.g.
ensure remaining_accounts.len() > max(CONFIG_INDEX, RENT_SPONSOR_INDEX,
FEE_PAYER_INDEX)) before calling LightConfig::load_checked or
validate_rent_sponsor_account, and return
LightSdkTypesError::InvalidInstructionData if the check fails so the subsequent
direct indexing of remaining_accounts (used by system_accounts_offset,
CONFIG_INDEX, RENT_SPONSOR_INDEX, FEE_PAYER_INDEX) cannot panic.
Comment on lines
+286
to
+342
| let system_accounts_offset = params.system_accounts_offset as usize; | ||
| if system_accounts_offset > remaining_accounts.len() { | ||
| return Err(LightSdkTypesError::InvalidInstructionData); | ||
| } | ||
|
|
||
| // 2. Split accounts into PDA and token | ||
| let (pda_accounts, token_accounts) = params | ||
| .accounts | ||
| .split_at_checked(params.token_accounts_offset as usize) | ||
| .ok_or(LightSdkTypesError::InvalidInstructionData)?; | ||
|
|
||
| // 3. Load and validate config | ||
| let config = LightConfig::load_checked(&remaining_accounts[CONFIG_INDEX], program_id)?; | ||
| let rent_sponsor = &remaining_accounts[RENT_SPONSOR_INDEX]; | ||
| let rent_sponsor_bump = config.validate_rent_sponsor_account::<AI>(rent_sponsor)?; | ||
|
|
||
| // 4. Hot accounts at the tail of remaining_accounts | ||
| let num_hot_accounts = params.accounts.len(); | ||
| let hot_accounts_start = remaining_accounts | ||
| .len() | ||
| .checked_sub(num_hot_accounts) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; | ||
| let hot_account_infos = &remaining_accounts[hot_accounts_start..]; | ||
| let (pda_account_infos, token_account_infos) = hot_account_infos | ||
| .split_at_checked(params.token_accounts_offset as usize) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; | ||
|
|
||
| let has_pda_accounts = !pda_accounts.is_empty(); | ||
| let has_token_accounts = !token_accounts.is_empty(); | ||
| let cpi_context = has_pda_accounts && has_token_accounts; | ||
|
|
||
| // 5. Build CpiAccounts | ||
| let cpi_config = CpiAccountsConfig { | ||
| sol_compression_recipient: false, | ||
| sol_pool_pda: false, | ||
| cpi_context, | ||
| cpi_signer, | ||
| }; | ||
| let cpi_accounts = CpiAccounts::new_with_config( | ||
| &remaining_accounts[FEE_PAYER_INDEX], | ||
| &remaining_accounts[system_accounts_offset..hot_accounts_start], | ||
| cpi_config, | ||
| ); | ||
|
|
||
| // Token (ctoken) accounts layout (only required when token accounts are present): | ||
| // [3] ctoken_rent_sponsor, [6] ctoken_compressible_config | ||
| let (ctoken_rent_sponsor, ctoken_compressible_config) = if has_token_accounts { | ||
| let rent_sponsor = remaining_accounts | ||
| .get(3) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; | ||
| let config = remaining_accounts | ||
| .get(6) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; | ||
| (Some(rent_sponsor), Some(config)) | ||
| } else { | ||
| (None, None) | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting token account indices as named constants.
The indices for ctoken_rent_sponsor (3) and ctoken_compressible_config (6) are documented in the function's docstring, but using named constants like the existing FEE_PAYER_INDEX, CONFIG_INDEX, and RENT_SPONSOR_INDEX would improve clarity and reduce the risk of mismatches during refactoring.
Additionally, the same minimum-accounts validation suggestion from the PDA-only processor applies here—the check at line 287 doesn't ensure the minimum required accounts (7 for the token path) exist before direct indexing.
♻️ Suggested constants
const FEE_PAYER_INDEX: usize = 0;
const CONFIG_INDEX: usize = 1;
const RENT_SPONSOR_INDEX: usize = 2;
+// Token-path specific indices
+const CTOKEN_RENT_SPONSOR_INDEX: usize = 3;
+const LIGHT_TOKEN_PROGRAM_INDEX: usize = 4;
+const CPI_AUTHORITY_INDEX: usize = 5;
+const CTOKEN_COMPRESSIBLE_CONFIG_INDEX: usize = 6;🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs` around
lines 286 - 342, Extract the magic indices 3 and 6 into named constants (e.g.,
CTOKEN_RENT_SPONSOR_INDEX and CTOKEN_COMPRESSIBLE_CONFIG_INDEX) and use them
when computing ctoken_rent_sponsor and ctoken_compressible_config instead of
get(3)/get(6); also add a pre-check before slicing/indexing that
remaining_accounts.len() meets the minimum required count for the token path (at
least the number of system+hot+token accounts, e.g., 7) so the
.get(...).ok_or(...) won't silently rely on hardcoded literals—update references
in the same function where FEE_PAYER_INDEX, CONFIG_INDEX, RENT_SPONSOR_INDEX,
ctoken_rent_sponsor, ctoken_compressible_config, and
CpiAccounts::new_with_config are used.
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
LightProgramPinocchioderive macro to derive seed structs, pack, unpack, compress and decompress traits and helpersNew Crates
light-account(sdk-libs/account/)Re-exports macros and generic types specialized with Solana's
AccountInfo<'info>:CpiAccounts<'c, 'info>,CompressCtx<'a, 'info>,DecompressCtx<'a, 'info>,ValidatedPdaContext<'info>,PackedAccountslight-sdk-macros:#[light_program],#[derive(LightAccount)],#[derive(LightAccounts)],#[derive(LightProgram)], etc.light_err(),derive_rent_sponsor_pda(),find_mint_address(),derive_associated_token_account()light-account-pinocchio(sdk-libs/account-pinocchio/)Re-exports macros and generic types specialized with Pinocchio's
AccountInfo(no lifetime):CpiAccounts<'c>,CompressCtx<'a>,DecompressCtx<'a>,ValidatedPdaContext#[derive(LightProgramPinocchio)](replaces#[derive(LightProgram)])[u8; 32]byte arrays instead ofPubkeylight-sdk-typesChangesAdded
interface/directory with generic interface types:Key generic types:
CompressCtx<'a, AI: AccountInfoTrait>- Compression contextDecompressCtx<'a, AI: AccountInfoTrait>- Decompression contextCpiAccounts<'a, T: AccountInfoTrait>- CPI account collectionValidatedPdaContext<AI: AccountInfoTrait>- Validated PDA accountsTest Programs
manual-test-pinocchio(sdk-tests/manual-test-pinocchio/)pinocchio-light-program-test(sdk-tests/pinocchio-light-program-test/)#[derive(LightProgramPinocchio)]on enumanchor-semi-manual-test(sdk-tests/anchor-semi-manual-test/)#[derive(LightProgram)]with standard Anchor#[program]#[derive(Accounts, LightAccounts)]on instruction account structslight-account-checksChangesNew
AccountMetaTraitNew Generic Functions
invoke_cpi()- Generic CPI invocation abstractiontransfer_lamports_cpi()- Transfer via system program CPIcreate_pda_account()- Generic PDA creationclose_account()- Account closuretransfer_lamports()- Direct lamport manipulationPackedAccounts<AM: AccountMetaTrait>Generic instruction account packing with deduplication:
insert_or_get()- Insert or get existing account indexadd_pre_accounts_signer()/add_pre_accounts_meta()- Pre-account managementadd_system_accounts_raw()- System account insertionImplementations
AccountMetaTraitforsolana_instruction::AccountMetaOwnedAccountMetawrapper +AccountMetaTraitimplementationlight-compressibleChangesCreateAccountsProoftolight-sdk-typeslight-sdk-macrosChangesRemoved
#[derive(Compressible)]- unused macro removedUpdated
light_sdk::interface->light_account#[derive(LightProgramPinocchio)]for Pinocchio programsRenamings
COMPRESSIBLE_CONFIG_SEEDLIGHT_CONFIG_SEEDSummary by CodeRabbit
New Features
New Features
Improvements
Documentation
Chores