Closed
Conversation
Contributor
📝 WalkthroughWalkthroughA comprehensive architectural refactoring introducing framework-agnostic account handling through Changes
Sequence DiagramsequenceDiagram
participant Client
participant LightProgram as Light Program<br/>(Macro Generated)
participant AccountInfoTrait as AccountInfoTrait<br/>(Framework Agnostic)
participant Framework as Framework Impl<br/>(Solana/Pinocchio)
participant SystemProgram as System Program
Client->>LightProgram: Invoke with remaining_accounts
LightProgram->>AccountInfoTrait: Call trait methods (get_min_rent_balance, invoke_cpi, etc)
AccountInfoTrait->>Framework: Dispatch to framework-specific impl
Framework->>Framework: Use native account info methods
Framework-->>AccountInfoTrait: Return results
alt PDA Creation
AccountInfoTrait->>SystemProgram: invoke_cpi with CreateAccount
SystemProgram-->>AccountInfoTrait: Account created
else Lamport Transfer
AccountInfoTrait->>SystemProgram: invoke_cpi with Transfer
SystemProgram-->>AccountInfoTrait: Lamports transferred
end
AccountInfoTrait-->>LightProgram: Operation complete
LightProgram-->>Client: Result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes This is a fundamental architectural refactoring establishing a new framework-agnostic SDK layer. The complexity arises from:
The changes require understanding:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ananas-block
commented
Feb 2, 2026
sdk-libs/token-sdk/src/anchor.rs
Outdated
Contributor
Author
There was a problem hiding this comment.
remove sdk-libs/token-sdk/src/anchor.rs we should use light-account instead
ananas-block
commented
Feb 2, 2026
Comment on lines
27
to
+32
| #[cfg(feature = "anchor")] | ||
| use anchor_lang::{AnchorDeserialize, AnchorSerialize}; | ||
| pub use anchor_lang::{AnchorDeserialize, AnchorSerialize}; | ||
| #[cfg(not(feature = "anchor"))] | ||
| use borsh::{BorshDeserialize as AnchorDeserialize, BorshSerialize as AnchorSerialize}; | ||
| pub use borsh::{BorshDeserialize as AnchorDeserialize, BorshSerialize as AnchorSerialize}; |
![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: 36
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/Cargo.toml (1)
14-28:⚠️ Potential issue | 🟠 MajorAdd
v2dependency tocpi-contextfeature; consider includinganchorin defaults.The
cpi-contextfeature currently lacks the requiredv2dependency. According to the README,cpi-context"Requires the v2 feature," but the definition only includeslight-sdk-types/cpi-contextwithout declaring thev2feature dependency. This must be corrected:-cpi-context = ["light-sdk-types/cpi-context"] +cpi-context = ["light-sdk-types/cpi-context", "v2"]Additionally, consider whether
anchorshould be in default features. Currently, consumers must explicitly opt into Anchor serialization, whereas thev2feature is enabled by default. If this library primarily targets Anchor programs, includinganchorin defaults would align consumer expectations.sdk-libs/sdk-types/src/cpi_accounts/v2.rs (1)
210-220:⚠️ Potential issue | 🟡 MinorUse iterator skip to prevent potential panic on empty accounts slice.
self.accounts[1..]will panic if the accounts slice is empty. While the constructor's documentation states that accounts must start at system accounts (implying non-empty), usingiter().skip(1)is more defensive and keeps the same intent without risking a panic.✅ Suggested fix
- self.accounts[1..] - .iter() - .for_each(|acc| account_infos.push(acc.clone())); + self.accounts + .iter() + .skip(1) + .for_each(|acc| account_infos.push(acc.clone()));program-libs/account-checks/src/lib.rs (1)
5-12: 🧹 Nitpick | 🔵 TrivialUpdate module documentation table to include
close_account.The module documentation table at the top doesn't include the newly added
close_accountmodule. For consistency with the existing documentation pattern, consider adding it://! | Module | Description | //! |--------|-------------| //! | [`AccountInfoTrait`] | Trait abstraction over Solana account info types | //! | [`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`] | Utility to close accounts and transfer lamports |Based on coding guidelines: "Document public modules and traits (AccountInfoTrait, AccountIterator, AccountError, checks, discriminator, packed_accounts) with their purpose and usage"
sdk-libs/macros/src/light_pdas/program/decompress.rs (1)
553-567: 🧹 Nitpick | 🔵 TrivialConsider eliminating Pubkey round-trip.
The PDA derivation converts
program_id: &[u8; 32]toPubkey, callsfind_program_address, then converts the result back to[u8; 32]viato_bytes(). Sincesolana_pubkey::Pubkey::find_program_addressrequires a&Pubkey, this round-trip is necessary, but you could add a brief comment explaining why:+ // find_program_address requires Pubkey type, convert and convert back let program_id_pubkey = solana_pubkey::Pubkey::from(*program_id); let (pda, bump) = solana_pubkey::Pubkey::find_program_address(seeds, &program_id_pubkey);sdk-libs/macros/src/light_pdas/accounts/variant.rs (1)
490-507: 🧹 Nitpick | 🔵 TrivialPrefer
ProgramPackedAccounts::get_u8for seed index lookups.The packed seed indices come from instruction data; using raw
accounts.get()drops the contextual error message and diverges from the packed-accounts access pattern used elsewhere in this generator. Consider wrappingaccountsonce and usingget_u8with a descriptive label.♻️ Suggested direction
- let `#field` = solana_pubkey::Pubkey::new_from_array( - accounts - .get(self.seeds.#idx_field as usize) - .ok_or(light_account::LightSdkTypesError::NotEnoughAccountKeys)? - .key() - ); + let packed_accounts = light_account::packed_accounts::ProgramPackedAccounts { accounts }; + let account = packed_accounts + .get_u8(self.seeds.#idx_field, `#error_msg`) + .map_err(|_| light_account::LightSdkTypesError::NotEnoughAccountKeys)?; + let `#field` = solana_pubkey::Pubkey::new_from_array(account.key());Based on learnings: Provide descriptive names in ProgramPackedAccounts error messages (e.g., 'token_mint' instead of 'account') and call get()/get_u8() with bounds checking.
🤖 Fix all issues with AI agents
In `@program-libs/account-checks/src/account_info/pinocchio.rs`:
- Around line 321-342: The transfer_lamports_cpi function is creating a Signer
from signer_seeds even when signer_seeds is empty, causing an invalid PDA;
change the logic in transfer_lamports_cpi to mirror create_account_cpi: if
signer_seeds is empty call pinocchio_system::instructions::Transfer { ...
}.invoke_signed(&[]) directly, otherwise build seeds_vec, create
Signer::from(&seeds_vec[..]) and call invoke_signed(&[signer]) so empty-seed
transfers use no signers and seeded CPIs use the constructed signer.
- Around line 200-212: The realloc implementation in pinocchio.rs currently
ignores the zero_init parameter; update fn realloc(&self, new_len: usize,
zero_init: bool) to respect zero_init by detecting when the account grows and,
if zero_init==true, zeroing the newly allocated bytes before returning (preserve
existing error mapping via AccountError::from). Locate the realloc method and
the helper used to change size (resize) and ensure you zero the range between
old_len and new_len when growing; keep the same signature and error behavior so
callers like resize_registered_program_account.rs relying on zeroing get the
promised guarantees.
In `@program-libs/account-checks/src/account_info/solana.rs`:
- Around line 275-326: The transfer invoke includes system_program in the
accounts slice unnecessarily; in create_pda_with_lamports_solana find the
transfer_ix/invoke_signed calls (uses transfer_ix, rent_payer_seeds, rent_payer,
account, system_program) and remove system_program.clone() from the accounts
arrays passed to invoke_signed in both branches so the calls only pass
rent_payer and account (still conditionally using rent_payer_seeds for signed
PDAs); no other behavior changes are needed.
- Around line 135-191: The parameter system_program in create_pda_account is
unused (it's only needed for trait compatibility across implementations); add a
brief doc comment on the create_pda_account method (and/or the trait signature
if more appropriate) clarifying that system_program is accepted for API
consistency but may be unused in this Solana implementation (refer to
create_pda_account and create_pda_with_lamports_solana so reviewers can see the
related helper uses).
In `@sdk-libs/account/Cargo.toml`:
- Around line 9-27: The crate currently forces std by specifying light-sdk-types
with its "std" feature and leaves light-compressed-account with its alloc
default, so toggle of this crate's std/alloc features has no effect; change the
dependency entries to not force features (set default-features = false) and
remove the hardcoded "std" from light-sdk-types so the crate-level features
control them. Concretely, update the light-sdk-types dependency (the entry named
light-sdk-types) to include default-features = false and only the non-std
features (e.g., "v2", "cpi-context"), set light-compressed-account to
default-features = false (so its std/alloc is driven by our features), and
ensure light-hasher keeps default-features = false; keep the features table
entries ("std" -> "light-sdk-types/std","light-compressed-account/std" and
"alloc" -> "light-sdk-types/alloc","light-compressed-account/alloc" if present)
so toggling this crate's std/alloc features will correctly enable downstream
std/alloc flags.
In `@sdk-libs/client/src/interface/load_accounts.rs`:
- Line 3: The imports of Pack and derive_rent_sponsor_pda in load_accounts.rs
create a forbidden sdk-libs → sdk-libs dependency; extract the Pack trait/type
and the derive_rent_sponsor_pda function into a program-lib (e.g.,
token-interface or a new shared interface crate) and update load_accounts.rs to
import them from that program-lib instead of light_account/light_sdk; ensure the
signatures and visibility match (export Pack and derive_rent_sponsor_pda from
the program-lib) and update any references in functions using Pack or
derive_rent_sponsor_pda so compilation succeeds without referencing sibling
sdk-libs crates.
In `@sdk-libs/client/src/interface/mod.rs`:
- Around line 24-29: The module currently re-exports LightConfig and
CreateAccountsProof which pulls sdk-libs/account and sdk-libs/sdk-types into
sdk-libs/client; remove the offending re-exports (the pub use of LightConfig and
CreateAccountsProof) from mod.rs so sdk-libs/client no longer depends on other
sdk-libs crates, or alternatively replace them with re-exports of equivalent
types that live in program-libs if such equivalents exist; after removal, update
any documentation/tests to import LightConfig and CreateAccountsProof directly
from their defining crates instead of from the client interface.
In `@sdk-libs/macros/src/light_pdas/accounts/builder.rs`:
- Around line 364-379: The code constructs
InstructionDataInvokeCpiWithAccountInfo with a magic literal mode: 1; replace
this magic number with a named constant or use the provided helper/constructor
in the invoke helpers to make intent clear. Update the builder where
InstructionDataInvokeCpiWithAccountInfo is created (the struct named
InstructionDataInvokeCpiWithAccountInfo and the call to instruction_data.invoke)
to use a descriptive constant (e.g., INVOKE_WITH_ACCOUNT_INFO_MODE) or the SDK’s
helper constructor from the invoke module instead of hardcoding 1, and add a
short doc comment on the constant explaining what "mode 1" represents.
In `@sdk-libs/macros/src/light_pdas/light_account_keywords.rs`:
- Around line 25-30: The top-level module doc comment listing valid keys must be
updated to include the new pda:: namespace entries; modify the doc comment above
PDA_NAMESPACE_KEYS so the syntax/docs section enumerates "pda::seeds" and
"pda::zero_copy" alongside the existing token/associated_token/mint entries,
ensuring the documentation text clearly states that these are valid keys for the
pda:: namespace used by the #[light_account(pda::...)] attributes and maps to
the PDA_NAMESPACE_KEYS constant.
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 314-369: Both generate_dispatch_fn and
generate_enum_dispatch_method duplicate the match/deserialize arm generation
(the compress_arms logic using info.is_zero_copy, qualify_type_with_crate, and
calling light_account::prepare_account_for_compression). Extract that duplicated
logic into a single helper on the same type (e.g., fn build_compress_arms(&self)
-> Result<Vec<TokenStream>>) that iterates self.accounts and returns the Vec of
arm TokenStreams (reusing the same zero-copy vs deserialize branches), then call
that helper from both generate_dispatch_fn and generate_enum_dispatch_method to
replace the compress_arms local construction.
In `@sdk-libs/macros/src/light_pdas/program/derive_light_program.rs`:
- Around line 644-654: The code currently defaults any ManualSeed::DataField in
variant.seeds to type Pubkey when pushing an InstructionDataSpec into
instruction_data_specs (using seen_data_fields and InstructionDataSpec), which
mis-types non-Pubkey fields; update derive_light_program.rs to query the real
field type via the existing CrateContext for the ident (or the variant/struct
definition) and use that resolved type when constructing InstructionDataSpec,
and if you cannot resolve the type or the type is not Pubkey when a Pubkey is
required, emit a clear compile-time error instead of silently defaulting to
Pubkey; locate the logic around ManualSeed::DataField handling and replace the
hardcoded syn::parse_quote!(Pubkey) with the resolved type or an error path.
- Around line 86-210: The parsing helpers (parse_enum_variants,
parse_variant_attr, extract_inner_type) and related types (VariantAttrContent,
SeenDeriveKeywords, ParsedManualVariant, ManualVariantKind, ManualSeed) should
be extracted into a new module file (light_pdas/program/parse.rs); move their
implementations there, export the necessary functions/types with pub(crate) as
needed, and update derive_light_program.rs to use them via use
crate::light_pdas::program::parse::{parse_enum_variants, parse_variant_attr,
extract_inner_type, VariantAttrContent, SeenDeriveKeywords, ParsedManualVariant,
...}; ensure any syn/tokenstream types still compile by adding the correct use
imports in parse.rs and adjust visibility for fields/functions referenced from
the derive code.
- Around line 351-410: The ATA (ManualVariantKind::Ata) branch must reject any
namespaced keys collected during parsing: after the parse loop and before
finishing validation, add a check in the ManualVariantKind::Ata arm that
inspects the parsed-key tracker (the seen structure used with
seen.add_namespaced_key(...)) and returns a syn::Error if any namespaced keys
were recorded (e.g., "associated_token variant must not have namespaced keys
like `pda::...` or `token::...`"). Keep the existing checks for
seeds/owner_seeds and use the same error construction style
(syn::Error::new(Span::call_site(), ...)); refer to ManualVariantKind::Ata,
seen, parse_namespaced_value, and the parse loop that calls
seen.add_namespaced_key to locate where to add this validation.
In `@sdk-libs/macros/src/light_pdas/program/instructions.rs`:
- Around line 526-545: The initialize_compression_config function currently
passes a hard-coded config_bump = 0 to
light_account::process_initialize_light_config which will break PDA-derived
config accounts; update InitializeCompressionConfig flow to provide the correct
bump by either adding a bump field to InitConfigParams (e.g., init_config_bump)
and pass params.config_bump, or compute the bump via the known seeds used for
CompressibleConfig (seed = [b"compressible_config", version.to_le_bytes()]) and
pass that derived bump into process_initialize_light_config in place of the 0 so
the stored bump matches PDA derivation and subsequent validations succeed.
In `@sdk-libs/macros/src/light_pdas/program/parsing.rs`:
- Around line 685-686: Remove the unused LightFinalize import in the delegation
branch: where you currently have use light_account::{LightPreInit,
LightFinalize}; (the block that calls
`#ctx_name.accounts.light_pre_init`(`#ctx_name.remaining_accounts`,
&#params_ident)), change it to import only LightPreInit so the generated code no
longer emits unused-import warnings; keep the call to
`#ctx_name.accounts.light_pre_init` and the & `#params_ident` usage unchanged.
- Around line 687-688: Replace the verbose error mapping that wraps errors
through solana_program_error::ProgramError with the direct conversion using the
existing From impl by changing the three occurrences of .map_err(|e|
anchor_lang::error::Error::from(solana_program_error::ProgramError::from(e)))?
to .map_err(anchor_lang::error::Error::from)? (these occur in the parsing logic
where LightSdkTypesError is converted); also remove the unused import
LightFinalize from the imports in this module since it is not referenced
anywhere in the delegation branch.
In `@sdk-libs/sdk-types/src/interface/cpi/account.rs`:
- Around line 71-76: The code calls Vec::with_capacity(1 +
cpi_accounts.account_infos().len() - PROGRAM_ACCOUNTS_LEN) which can underflow
when account_infos().len() < PROGRAM_ACCOUNTS_LEN; change to first compute a
safe subtraction using checked_sub (e.g. let extra =
cpi_accounts.account_infos().len().checked_sub(PROGRAM_ACCOUNTS_LEN).ok_or(LightSdkTypesError::InvalidCpiAccountLayout)?;)
and then allocate with Vec::with_capacity(1 + extra) or return an appropriate
LightSdkTypesError from to_cpi_metas; update references to to_cpi_metas,
cpi_accounts.account_infos(), PROGRAM_ACCOUNTS_LEN and the metas allocation
accordingly.
In `@sdk-libs/sdk-types/src/interface/cpi/create_mints.rs`:
- Around line 247-255: The CpiContext construction sets assigned_account_index
to offset + index as u8 which can overflow; update the CpiContext creation (the
assigned_account_index field in CpiContext within create_mints.rs and the
similar block at the other occurrence) to perform a checked addition (e.g.,
offset.checked_add(index as u8)) and return an explicit error if None is
returned, rather than allowing wraparound; ensure the function signature
propagates that error so callers handle it.
- Around line 324-356: The CPI context in invoke_execute sets
CpiContext.in_queue_index incorrectly to self.params.address_tree_index; update
invoke_execute (inside the MintActionCompressedInstructionData.cpi_context
construction) to use self.params.output_queue_index (matching invoke_cpi_write)
so routing is consistent between write and execute phases, and ensure any
necessary cast/typing matches how output_queue_index is used elsewhere.
In `@sdk-libs/sdk-types/src/interface/cpi/instruction.rs`:
- Around line 24-63: The interface exposes CPI-related methods
(write_to_cpi_context_first, write_to_cpi_context_set, execute_with_cpi_context,
get_with_cpi_context, get_cpi_context, has_read_only_accounts) but they lack the
feature gate declared in the docs; add #[cfg(feature = "cpi-context")] to each
of these method declarations in the trait so their availability matches the
documentation and the SDK's LightCpiInstruction trait, ensuring consistency
across implementations (alternatively, if unconditional exposure was intended,
update the doc comments to remove the "Only available with the `cpi-context`
feature enabled" text).
In `@sdk-libs/sdk-types/src/interface/instruction/pack_accounts.rs`:
- Around line 75-115: The functions insert_or_get, insert_or_get_read_only and
insert_or_get_config currently return u8 and silently wrap when next_index
overflows; change their signatures to return Result<u8, YourErrorType> (or a
suitable error enum) and in insert_or_get_config check for overflow before
allocating a new index (e.g., if next_index == u8::MAX return Err(IndexOverflow)
else use the current next_index and increment safely via checked_add or by
setting next_index = next_index.wrapping_add after validating). Update callers
accordingly so map.insert(bytes, (index, AM::new(pubkey, is_signer,
is_writable))) only runs when index allocation succeeds; keep existing branch
behavior for existing entries but return Ok(index) instead of raw u8.
In `@sdk-libs/sdk-types/src/interface/program/compression/pda.rs`:
- Around line 115-122: The two different hashers (Sha256 for output_data_hash
and Sha256BE for input_data_hash) should be explicitly documented: add a concise
comment above the output_data_hash and input_data_hash lines explaining that
output_data_hash is produced with the standard Sha256 (then zeroing the first
byte per protocol) while input_data_hash uses Sha256BE to produce a big-endian
hash of the account_key (as required by the PDA initialization/verification
convention); reference the symbols Sha256, Sha256BE, output_data_hash,
input_data_hash, compressed_data, and account_key so future maintainers
understand the intentional difference and where it matters.
In `@sdk-libs/sdk-types/src/interface/program/compression/processor.rs`:
- Around line 70-155: In process_compress_pda_accounts_idempotent: add explicit
bounds checks before any indexing/slicing of remaining_accounts (check
remaining_accounts.len() > CONFIG_INDEX and > RENT_SPONSOR_INDEX and >
FEE_PAYER_INDEX), validate system_accounts_offset as usize is <= pda_start and
<= remaining_accounts.len(), and ensure
pda_start.checked_add(num_pdas).is_some() and pda_start + num_pdas <=
remaining_accounts.len() so the loop dispatch_fn(&remaining_accounts[pda_index],
...) and the slice &remaining_accounts[system_accounts_offset..pda_start] cannot
panic; return an appropriate LightSdkTypesError (e.g., NotEnoughAccountKeys or
InvalidInstructionData) if any check fails.
In `@sdk-libs/sdk-types/src/interface/program/config/state.rs`:
- Around line 15-76: LightConfig currently has a different field layout than the
on‑chain CompressibleConfig spec (e.g. version is u8, uses Vec for
address_space, missing withdrawal_authority and bumps), so update LightConfig
(and its LEN and size_for_address_space calculations) to match the spec: change
version to u16, add state:u8 and bump:u8, include update_authority,
withdrawal_authority, rent_sponsor, compression_authority as [u8;32] Pubkey
equivalents, add rent_sponsor_bump:u8 and compression_authority_bump:u8, replace
address_space Vec<[u8;32]> with fixed [[u8;32];4], add _place_holder [u8;32],
and recompute sizes using core::mem::size_of and fixed array sizes
(adjust/remove MAX_ADDRESS_TREES_PER_SPACE logic); alternatively, if preserving
the old layout is required, introduce a versioned enum/struct pair (e.g.
LightConfigV1 + LightConfigV2) and implement migration/deserialization that
branches on the version field in Discriminator/size logic so on‑chain
deserialization matches CompressibleConfig.
In
`@sdk-libs/sdk-types/src/interface/program/decompression/create_token_account.rs`:
- Around line 19-23: The doc comment above the create_token_account function
incorrectly states it returns `(data, account_metas, program_id)` while the
implementation returns only `(data, account_metas)`; update the docstring to
match the actual return value by changing the "Returns" line to `(data,
account_metas)` (or, if you intended to return the program_id, adjust the
function signature and return to include program_id) and ensure the comment
references the create_token_account function and its actual tuple shape.
In `@sdk-libs/sdk-types/src/interface/program/decompression/pda.rs`:
- Around line 61-63: The local generic type alias Data<const N: usize, P> = <<P
as PackedLightAccountVariantTrait<N>>::Unpacked as
LightAccountVariantTrait<N>>::Data is correct but non-obvious; add a short
inline comment immediately above the alias explaining its purpose (to reduce
verbosity when referring to the account Data type derived from P’s Unpacked
variant) and why it’s defined inside the function body (scoped convenience for
this function only), and keep references to PackedLightAccountVariantTrait,
Unpacked, and LightAccountVariantTrait in the comment so readers can quickly
locate the related traits.
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs`:
- Around line 545-547: Check for out-of-range owner indices before mutating
account_metas: in the loop over in_token_data, validate that (data.owner as
usize + transfer2_packed_start) is < account_metas.len() and if not return
Err(ProgramError::InvalidInstructionData) (or the crate's
InvalidInstructionData). Update the block that sets account_metas[...].is_signer
= true to perform this bounds check first, referencing in_token_data,
data.owner, transfer2_packed_start and account_metas to locate the code.
- Around line 305-309: In the token-path branch you access
remaining_accounts[CONFIG_INDEX] and [RENT_SPONSOR_INDEX] without checking
remaining_accounts.len(); add the same fixed-index guard used elsewhere to
validate remaining_accounts has at least max(CONFIG_INDEX, RENT_SPONSOR_INDEX)+1
entries before indexing, and if not return the appropriate error (e.g.,
NotEnoughAccountKeys / your function's account-length error); update the
token-path logic where CONFIG_INDEX and RENT_SPONSOR_INDEX are used (and any
call sites like LightConfig::load_checked and
config.validate_rent_sponsor_account::<AI>) to run this guard first.
- Around line 181-185: The code indexes remaining_accounts at CONFIG_INDEX and
RENT_SPONSOR_INDEX without checking length, which can panic; before calling
LightConfig::load_checked and validate_rent_sponsor_account::<AI>, add a guard
that remaining_accounts.len() > max(CONFIG_INDEX, RENT_SPONSOR_INDEX) (or use
remaining_accounts.get(CONFIG_INDEX).and_then/get(RENT_SPONSOR_INDEX)) and
return an appropriate ProgramError (e.g., a custom invalid input / not enough
account error) if accounts are missing, then proceed to call
LightConfig::load_checked and validate_rent_sponsor_account::<AI> using the
safely obtained account refs.
- Around line 485-575: The Transfer2 CPI fails because account_metas is
reordered but invoke_cpi still receives the original remaining_accounts
(breaking positional correspondence), and the system program CpiMeta uses a
zeroed pubkey instead of the real system program ID; update the code to build an
account_infos (or account_infos_vec) vector that mirrors account_metas order
(using the same indices: LIGHT_SYSTEM_PROGRAM_ID entry, fee_payer_key,
CPI_AUTHORITY, REGISTERED_PROGRAM_PDA, ACCOUNT_COMPRESSION_AUTHORITY_PDA,
ACCOUNT_COMPRESSION_PROGRAM_ID, system_program, optional cpi_ctx, then packed
remaining_accounts starting at packed_accounts_offset) and pass that ordered
account_infos to AI::invoke_cpi/ invoke_signed instead of remaining_accounts,
and replace the zeroed system_program CpiMeta pubkey ([0u8;32]) with the real
Solana system program pubkey (solana_sdk::system_program::ID or equivalent) so
metas and AccountInfos align for invoke_signed.
In `@sdk-libs/sdk-types/src/interface/program/decompression/token.rs`:
- Around line 61-112: The ATA creation currently checks only
token_account_info.data_len() == 0 which will skip initialization for
preallocated-but-uninitialized accounts; update the ATA path to use the existing
is_already_initialized check (STATE_OFFSET const and is_already_initialized
variable) so you only skip creation when the account is truly initialized, and
otherwise either call build_create_ata_instruction and invoke the CPI or return
an explicit error when data_len() > 0 but is_already_initialized is false;
adjust the condition guarding the create branch (referencing token_account_info,
is_already_initialized, STATE_OFFSET, ata_info, build_create_ata_instruction,
and AI::invoke_cpi) accordingly.
In `@sdk-libs/sdk-types/src/interface/program/validation.rs`:
- Around line 46-88: In validate_pda_common_accounts_inner<const
EXTRACT_COMPRESSION_AUTHORITY: bool, AI>, stop returning compression_authority
unchecked: when EXTRACT_COMPRESSION_AUTHORITY is true, validate the extracted
account against the loaded LightConfig before constructing ValidatedPdaContext.
Either call a new
LightConfig::validate_compression_authority_account(compression_authority)
method (add it to LightConfig if missing) or directly compare
compression_authority.key() to the expected key stored on light_config and
return a LightSdkTypesError on mismatch; keep the same flow and error mapping as
rent_sponsor validation.
In `@sdk-libs/sdk/Cargo.toml`:
- Around line 29-31: The cpi-context feature is not declaring v2 as a required
feature, so enablement can violate the documented contract; update the features
block so the cpi-context feature includes "v2" in its array (alongside
"light-sdk-types/cpi-context") so enabling cpi-context automatically enables the
v2 feature (i.e., make cpi-context depend on v2).
In `@sdk-libs/sdk/src/error.rs`:
- Around line 132-156: The From<LightSdkError> for LightSdkTypesError impl is
collapsing many distinct LightSdkError variants into generic
ConstraintViolation/Borsh, losing error fidelity; update the match in the impl
(the From<LightSdkError> for LightSdkTypesError conversion) to explicitly map
each concrete variant (e.g., AccountDataTooSmall, InvalidInstructionData,
InvalidSeeds, CpiFailed, NotEnoughAccountKeys, MissingRequiredSignature,
BorshIo, etc.) to the corresponding LightSdkTypesError variant instead of
falling back to ConstraintViolation/Borsh, and make the symmetrical changes in
the reverse conversion impl (the From<LightSdkTypesError> for LightSdkError or
other related conversion function) so both directions preserve the specific
variants rather than collapsing them.
In `@sdk-libs/token-sdk/tests/pack_test.rs`:
- Around line 1-3: Add a "compressible" feature to the token-sdk crate so the
test guarded by #![cfg(feature = "compressible")] can be enabled; update
Cargo.toml to declare a features table entry named compressible that enables the
dependency feature "light-compressible" (i.e. add a compressible feature that
maps to "light-compressible") so importing PackedAccounts from light_account and
running with --features compressible compiles.
Comment on lines
+200
to
+212
| fn assign(&self, new_owner: &[u8; 32]) -> Result<(), AccountError> { | ||
| // SAFETY: We trust the caller to provide a valid owner. | ||
| // This is safe in the Solana runtime context where the runtime | ||
| // validates ownership changes. | ||
| unsafe { | ||
| self.assign(&pinocchio::pubkey::Pubkey::from(*new_owner)); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn realloc(&self, new_len: usize, _zero_init: bool) -> Result<(), AccountError> { | ||
| self.resize(new_len).map_err(AccountError::from) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the AccountInfo trait definition
fd -t f "\.rs$" -x rg -l "trait AccountInfo" {}Repository: Lightprotocol/light-protocol
Length of output: 140
🏁 Script executed:
# Search for the trait definition and understand realloc contract
rg "trait AccountInfo" -A 30 -B 2Repository: Lightprotocol/light-protocol
Length of output: 6527
🏁 Script executed:
# Check pinocchio's resize method behavior/documentation
rg "fn resize" -A 10 --type rsRepository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Look at other AccountInfo implementations to see how they handle realloc
rg "fn realloc" -B 5 -A 10 --type rsRepository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Check if pinocchio is documented in the repo
fd -t f "pinocchio" -o -name "*pinocchio*"Repository: Lightprotocol/light-protocol
Length of output: 302
🏁 Script executed:
# Read the full AccountInfo trait definition
cat program-libs/account-checks/src/account_info/account_info_trait.rsRepository: Lightprotocol/light-protocol
Length of output: 5645
🏁 Script executed:
# Read the pinocchio implementation around the realloc method
sed -n '190,230p' program-libs/account-checks/src/account_info/pinocchio.rsRepository: Lightprotocol/light-protocol
Length of output: 1557
🏁 Script executed:
# Search for all realloc implementations
rg "realloc" -B 3 -A 8Repository: Lightprotocol/light-protocol
Length of output: 24788
🏁 Script executed:
# Check if there's a solana_program implementation for comparison
fd pinocchio.rs -o fd solana_program.rsRepository: Lightprotocol/light-protocol
Length of output: 195
🏁 Script executed:
# Search for pinocchio in Cargo.toml to understand the dependency
rg "pinocchio" Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 721
🏁 Script executed:
# Look for tests of realloc behavior, especially with zero_init
rg "zero_init|realloc.*true" -A 5 -B 5Repository: Lightprotocol/light-protocol
Length of output: 6370
🏁 Script executed:
# Check if there's documentation about pinocchio in the repo
fd -type f -name "*pinocchio*" -o -name "*PINOCCHIO*"Repository: Lightprotocol/light-protocol
Length of output: 242
🏁 Script executed:
# Search for pinocchio resize usage to see if it's documented
rg "\.resize\(" -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 7448
🏁 Script executed:
# Search for pinocchio crate in the SDK to understand how resize works
fd "account-pinocchio" -type dRepository: Lightprotocol/light-protocol
Length of output: 242
🏁 Script executed:
# Look at the pinocchio account implementation
cat sdk-libs/account-pinocchio/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 1625
🏁 Script executed:
# Check if there's any documentation about resize/realloc in pinocchio
rg "resize|realloc" sdk-libs/account-pinocchio/ -A 3 -B 1Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Look for pinocchio usage in tests to see how resize is called
rg "\.resize\(|realloc\(" program-tests/ -B 2 -A 2 | head -100Repository: Lightprotocol/light-protocol
Length of output: 1266
🏁 Script executed:
# Check what pinocchio version 0.9 provides - look at SDK version
rg "pinocchio.*0\.9" -A 2 -B 2Repository: Lightprotocol/light-protocol
Length of output: 1313
🏁 Script executed:
# Look at the pinocchio implementation in light-account-pinocchio
find sdk-libs/account-pinocchio/src -name "*.rs" | head -20Repository: Lightprotocol/light-protocol
Length of output: 109
🏁 Script executed:
# Check if there's any test that verifies realloc zero_init behavior
rg "zero_init.*true|realloc.*true" -B 5 -A 5Repository: Lightprotocol/light-protocol
Length of output: 2934
🏁 Script executed:
# Check AccountInfo implementations for clues about resize behavior
rg "impl.*AccountInfo" -B 2 -A 15 | head -80Repository: Lightprotocol/light-protocol
Length of output: 7106
🏁 Script executed:
# Look at how pinocchio AccountInfo is wrapped in the local implementation
cat sdk-libs/account-pinocchio/src/lib.rsRepository: Lightprotocol/light-protocol
Length of output: 8866
🏁 Script executed:
# Check if there are any tests that verify zero_init behavior
rg "test.*realloc|test.*zero" -B 3 -A 8Repository: Lightprotocol/light-protocol
Length of output: 50387
🏁 Script executed:
# Look at the solana AccountInfo to understand what zero_init does in solana-program
rg "deprecated.*realloc" -B 5 -A 15Repository: Lightprotocol/light-protocol
Length of output: 2471
🏁 Script executed:
# Check if there's documentation about pinocchio AccountInfo in the repo
find . -name "*.md" -type f | xargs grep -l "pinocchio" | head -5Repository: Lightprotocol/light-protocol
Length of output: 215
🏁 Script executed:
# Look at the actual usage of realloc with zero_init to understand the expectation
rg "realloc.*true" -B 5 -A 3Repository: Lightprotocol/light-protocol
Length of output: 2515
Honor the zero_init parameter—pinocchio's implementation violates the trait contract and risks leaking uninitialized data.
The AccountInfoTrait explicitly requires realloc to respect the zero_init parameter. Solana's implementation honors it; pinocchio ignores it (note the _ prefix). Documentation confirms that zero_init=true must zero new bytes when growing. Real code—like resize_registered_program_account.rs—already passes true, expecting this guarantee. Without it, stale memory is exposed.
Fix: Zero new bytes when requested
- fn realloc(&self, new_len: usize, _zero_init: bool) -> Result<(), AccountError> {
- self.resize(new_len).map_err(AccountError::from)
- }
+ fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), AccountError> {
+ let old_len = self.data_len();
+ self.resize(new_len).map_err(AccountError::from)?;
+ if zero_init && new_len > old_len {
+ let mut data = self.try_borrow_mut_data().map_err(AccountError::from)?;
+ data[old_len..].fill(0);
+ }
+ Ok(())
+ }📝 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
| fn assign(&self, new_owner: &[u8; 32]) -> Result<(), AccountError> { | |
| // SAFETY: We trust the caller to provide a valid owner. | |
| // This is safe in the Solana runtime context where the runtime | |
| // validates ownership changes. | |
| unsafe { | |
| self.assign(&pinocchio::pubkey::Pubkey::from(*new_owner)); | |
| } | |
| Ok(()) | |
| } | |
| fn realloc(&self, new_len: usize, _zero_init: bool) -> Result<(), AccountError> { | |
| self.resize(new_len).map_err(AccountError::from) | |
| } | |
| fn assign(&self, new_owner: &[u8; 32]) -> Result<(), AccountError> { | |
| // SAFETY: We trust the caller to provide a valid owner. | |
| // This is safe in the Solana runtime context where the runtime | |
| // validates ownership changes. | |
| unsafe { | |
| self.assign(&pinocchio::pubkey::Pubkey::from(*new_owner)); | |
| } | |
| Ok(()) | |
| } | |
| fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), AccountError> { | |
| let old_len = self.data_len(); | |
| self.resize(new_len).map_err(AccountError::from)?; | |
| if zero_init && new_len > old_len { | |
| let mut data = self.try_borrow_mut_data().map_err(AccountError::from)?; | |
| data[old_len..].fill(0); | |
| } | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/account_info/pinocchio.rs` around lines 200 -
212, The realloc implementation in pinocchio.rs currently ignores the zero_init
parameter; update fn realloc(&self, new_len: usize, zero_init: bool) to respect
zero_init by detecting when the account grows and, if zero_init==true, zeroing
the newly allocated bytes before returning (preserve existing error mapping via
AccountError::from). Locate the realloc method and the helper used to change
size (resize) and ensure you zero the range between old_len and new_len when
growing; keep the same signature and error behavior so callers like
resize_registered_program_account.rs relying on zeroing get the promised
guarantees.
Comment on lines
+321
to
+342
| fn transfer_lamports_cpi( | ||
| &self, | ||
| destination: &Self, | ||
| lamports: u64, | ||
| signer_seeds: &[&[u8]], | ||
| ) -> Result<(), AccountError> { | ||
| extern crate alloc; | ||
| use alloc::vec::Vec; | ||
|
|
||
| use pinocchio::instruction::{Seed, Signer}; | ||
|
|
||
| let seeds_vec: Vec<Seed> = signer_seeds.iter().map(|s| Seed::from(*s)).collect(); | ||
| let signer = Signer::from(&seeds_vec[..]); | ||
|
|
||
| pinocchio_system::instructions::Transfer { | ||
| from: self, | ||
| to: destination, | ||
| lamports, | ||
| } | ||
| .invoke_signed(&[signer]) | ||
| .map_err(AccountError::from) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's search for pinocchio dependency and understand the codebase structure
fd -t f "Cargo.toml" | head -20Repository: Lightprotocol/light-protocol
Length of output: 772
🏁 Script executed:
# Search for the actual file mentioned in the review
fd "pinocchio.rs" program-libs/account-checks/src/Repository: Lightprotocol/light-protocol
Length of output: 129
🏁 Script executed:
# Look for pinocchio crate definition and find documentation
rg "pinocchio" Cargo.toml -A 2 | head -30Repository: Lightprotocol/light-protocol
Length of output: 1131
🏁 Script executed:
# Search for how Signer is used elsewhere in the codebase
rg "Signer::from" --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 8360
🏁 Script executed:
# Look for documentation on transfer_lamports_cpi usage
rg "transfer_lamports_cpi" --type rust -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 3789
🏁 Script executed:
# Check if there are any tests or examples of PDA signer creation with empty seeds
rg "invoke_signed.*\[\]" --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 1766
🏁 Script executed:
# Look at the README or ACCOUNT_CHECKS.md documentation mentioned in learnings
fd "ACCOUNT_CHECKS.md" program-libs/account-checks/Repository: Lightprotocol/light-protocol
Length of output: 122
Handle empty signer seeds by invoking without signers instead of creating invalid PDA.
The transfer_lamports_cpi method unconditionally creates a Signer::from(&seeds_vec[..]) even when signer_seeds is empty. Per the trait documentation and the codebase's own pattern elsewhere (solana.rs, create_account), passing empty seeds to invoke_signed attempts create_program_address(&[], program_id), which is invalid. When the sender is already a signer (indicated by empty seeds), use invoke_signed(&[]) directly.
The same file demonstrates the correct pattern in create_account_cpi: it conditionally builds the signer only when seeds are non-empty, otherwise calls invoke_signed(&[]).
🔧 Proposed fix
- let seeds_vec: Vec<Seed> = signer_seeds.iter().map(|s| Seed::from(*s)).collect();
- let signer = Signer::from(&seeds_vec[..]);
-
- pinocchio_system::instructions::Transfer {
- from: self,
- to: destination,
- lamports,
- }
- .invoke_signed(&[signer])
- .map_err(AccountError::from)
+ if signer_seeds.is_empty() {
+ pinocchio_system::instructions::Transfer {
+ from: self,
+ to: destination,
+ lamports,
+ }
+ .invoke_signed(&[])
+ .map_err(AccountError::from)
+ } else {
+ let seeds_vec: Vec<Seed> = signer_seeds.iter().map(|s| Seed::from(*s)).collect();
+ let signer = Signer::from(&seeds_vec[..]);
+ pinocchio_system::instructions::Transfer {
+ from: self,
+ to: destination,
+ lamports,
+ }
+ .invoke_signed(&[signer])
+ .map_err(AccountError::from)
+ }📝 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
| fn transfer_lamports_cpi( | |
| &self, | |
| destination: &Self, | |
| lamports: u64, | |
| signer_seeds: &[&[u8]], | |
| ) -> Result<(), AccountError> { | |
| extern crate alloc; | |
| use alloc::vec::Vec; | |
| use pinocchio::instruction::{Seed, Signer}; | |
| let seeds_vec: Vec<Seed> = signer_seeds.iter().map(|s| Seed::from(*s)).collect(); | |
| let signer = Signer::from(&seeds_vec[..]); | |
| pinocchio_system::instructions::Transfer { | |
| from: self, | |
| to: destination, | |
| lamports, | |
| } | |
| .invoke_signed(&[signer]) | |
| .map_err(AccountError::from) | |
| } | |
| fn transfer_lamports_cpi( | |
| &self, | |
| destination: &Self, | |
| lamports: u64, | |
| signer_seeds: &[&[u8]], | |
| ) -> Result<(), AccountError> { | |
| extern crate alloc; | |
| use alloc::vec::Vec; | |
| use pinocchio::instruction::{Seed, Signer}; | |
| if signer_seeds.is_empty() { | |
| pinocchio_system::instructions::Transfer { | |
| from: self, | |
| to: destination, | |
| lamports, | |
| } | |
| .invoke_signed(&[]) | |
| .map_err(AccountError::from) | |
| } else { | |
| let seeds_vec: Vec<Seed> = signer_seeds.iter().map(|s| Seed::from(*s)).collect(); | |
| let signer = Signer::from(&seeds_vec[..]); | |
| pinocchio_system::instructions::Transfer { | |
| from: self, | |
| to: destination, | |
| lamports, | |
| } | |
| .invoke_signed(&[signer]) | |
| .map_err(AccountError::from) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/account_info/pinocchio.rs` around lines 321 -
342, The transfer_lamports_cpi function is creating a Signer from signer_seeds
even when signer_seeds is empty, causing an invalid PDA; change the logic in
transfer_lamports_cpi to mirror create_account_cpi: if signer_seeds is empty
call pinocchio_system::instructions::Transfer { ... }.invoke_signed(&[])
directly, otherwise build seeds_vec, create Signer::from(&seeds_vec[..]) and
call invoke_signed(&[signer]) so empty-seed transfers use no signers and seeded
CPIs use the constructed signer.
Comment on lines
+135
to
+191
| #[inline(never)] | ||
| fn create_pda_account( | ||
| &self, | ||
| lamports: u64, | ||
| space: u64, | ||
| owner: &[u8; 32], | ||
| pda_seeds: &[&[u8]], | ||
| rent_payer: &Self, | ||
| rent_payer_seeds: &[&[u8]], | ||
| system_program: &Self, | ||
| ) -> Result<(), AccountError> { | ||
| use solana_cpi::invoke_signed; | ||
| use solana_system_interface::instruction as system_instruction; | ||
|
|
||
| let owner_pubkey = solana_pubkey::Pubkey::from(*owner); | ||
|
|
||
| // Cold path: account already has lamports (e.g., attacker donation). | ||
| // CreateAccount would fail, so use Assign + Allocate + Transfer. | ||
| if self.lamports() > 0 { | ||
| return create_pda_with_lamports_solana( | ||
| self, | ||
| lamports, | ||
| space, | ||
| &owner_pubkey, | ||
| pda_seeds, | ||
| rent_payer, | ||
| rent_payer_seeds, | ||
| system_program, | ||
| ); | ||
| } | ||
|
|
||
| // Normal path: CreateAccount | ||
| let create_ix = system_instruction::create_account( | ||
| rent_payer.key, | ||
| self.key, | ||
| lamports, | ||
| space, | ||
| &owner_pubkey, | ||
| ); | ||
| // Only include rent_payer_seeds when the payer is itself a PDA. | ||
| // Passing empty seeds to invoke_signed causes create_program_address(&[], program_id) | ||
| // which can fail if the result happens to land on the ed25519 curve. | ||
| if rent_payer_seeds.is_empty() { | ||
| invoke_signed( | ||
| &create_ix, | ||
| &[rent_payer.clone(), self.clone()], | ||
| &[pda_seeds], | ||
| ) | ||
| } else { | ||
| invoke_signed( | ||
| &create_ix, | ||
| &[rent_payer.clone(), self.clone()], | ||
| &[rent_payer_seeds, pda_seeds], | ||
| ) | ||
| } | ||
| .map_err(|_| AccountError::InvalidAccount) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused system_program parameter in create_pda_account.
The system_program parameter is passed but never used in the Solana implementation. Looking at the cold path helper create_pda_with_lamports_solana, it also receives system_program but only uses it in the transfer instruction's account list.
However, the standard CreateAccount path (line 167-190) doesn't include system_program in the accounts list, which is correct since invoke_signed automatically resolves the system program. But then the parameter seems vestigial for consistency with the trait signature.
Consider adding a brief doc comment explaining that the parameter exists for trait compatibility but may not be used by all implementations.
🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/account_info/solana.rs` around lines 135 -
191, The parameter system_program in create_pda_account is unused (it's only
needed for trait compatibility across implementations); add a brief doc comment
on the create_pda_account method (and/or the trait signature if more
appropriate) clarifying that system_program is accepted for API consistency but
may be unused in this Solana implementation (refer to create_pda_account and
create_pda_with_lamports_solana so reviewers can see the related helper uses).
Comment on lines
+275
to
326
| /// Cold path for create_pda_account when account already has lamports. | ||
| #[cold] | ||
| #[inline(never)] | ||
| #[allow(clippy::too_many_arguments)] | ||
| fn create_pda_with_lamports_solana<'a>( | ||
| account: &solana_account_info::AccountInfo<'a>, | ||
| lamports: u64, | ||
| space: u64, | ||
| owner: &solana_pubkey::Pubkey, | ||
| pda_seeds: &[&[u8]], | ||
| rent_payer: &solana_account_info::AccountInfo<'a>, | ||
| rent_payer_seeds: &[&[u8]], | ||
| system_program: &solana_account_info::AccountInfo<'a>, | ||
| ) -> Result<(), AccountError> { | ||
| use solana_cpi::invoke_signed; | ||
| use solana_system_interface::instruction as system_instruction; | ||
|
|
||
| let current_lamports = account.lamports(); | ||
|
|
||
| // Assign owner | ||
| let assign_ix = system_instruction::assign(account.key, owner); | ||
| invoke_signed(&assign_ix, std::slice::from_ref(account), &[pda_seeds]) | ||
| .map_err(|_| AccountError::InvalidAccount)?; | ||
|
|
||
| // Allocate space | ||
| let allocate_ix = system_instruction::allocate(account.key, space); | ||
| invoke_signed(&allocate_ix, std::slice::from_ref(account), &[pda_seeds]) | ||
| .map_err(|_| AccountError::InvalidAccount)?; | ||
|
|
||
| // Transfer remaining lamports for rent-exemption if needed | ||
| if lamports > current_lamports { | ||
| let transfer_ix = | ||
| system_instruction::transfer(rent_payer.key, account.key, lamports - current_lamports); | ||
| // Only include rent_payer_seeds when the payer is itself a PDA. | ||
| if rent_payer_seeds.is_empty() { | ||
| invoke_signed( | ||
| &transfer_ix, | ||
| &[rent_payer.clone(), account.clone(), system_program.clone()], | ||
| &[], | ||
| ) | ||
| } else { | ||
| invoke_signed( | ||
| &transfer_ix, | ||
| &[rent_payer.clone(), account.clone(), system_program.clone()], | ||
| &[rent_payer_seeds], | ||
| ) | ||
| } | ||
| .map_err(|_| AccountError::InvalidAccount)?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: system_program included in transfer accounts but not required.
In lines 310-321, the system_program is included in the accounts array for the transfer instruction, but for invoke_signed the system program is automatically resolved by the runtime and doesn't need to be explicitly passed.
This is harmless but slightly verbose. The pinocchio implementation in pinocchio.rs (lines 277-292) doesn't include it in the transfer accounts.
🔧 Optional simplification
let transfer_ix =
system_instruction::transfer(rent_payer.key, account.key, lamports - current_lamports);
// Only include rent_payer_seeds when the payer is itself a PDA.
if rent_payer_seeds.is_empty() {
invoke_signed(
&transfer_ix,
- &[rent_payer.clone(), account.clone(), system_program.clone()],
+ &[rent_payer.clone(), account.clone()],
&[],
)
} else {
invoke_signed(
&transfer_ix,
- &[rent_payer.clone(), account.clone(), system_program.clone()],
+ &[rent_payer.clone(), account.clone()],
&[rent_payer_seeds],
)
}🤖 Prompt for AI Agents
In `@program-libs/account-checks/src/account_info/solana.rs` around lines 275 -
326, The transfer invoke includes system_program in the accounts slice
unnecessarily; in create_pda_with_lamports_solana find the
transfer_ix/invoke_signed calls (uses transfer_ix, rent_payer_seeds, rent_payer,
account, system_program) and remove system_program.clone() from the accounts
arrays passed to invoke_signed in both branches so the calls only pass
rent_payer and account (still conditionally using rent_payer_seeds for signed
PDAs); no other behavior changes are needed.
Comment on lines
+9
to
+27
| [features] | ||
| default = ["std"] | ||
| std = ["light-sdk-types/std", "light-compressed-account/std"] | ||
| alloc = ["light-sdk-types/alloc", "light-compressed-account/alloc"] | ||
| token = ["light-sdk-types/token", "dep:light-token-interface"] | ||
| poseidon = ["light-sdk-types/poseidon", "light-hasher/poseidon"] | ||
| sha256 = ["light-sdk-types/sha256", "light-hasher/sha256"] | ||
| anchor = ["light-sdk-types/anchor", "dep:anchor-lang"] | ||
| anchor-discriminator = ["light-sdk-macros/anchor-discriminator"] | ||
|
|
||
| [dependencies] | ||
| light-sdk-types = { workspace = true, features = ["std", "v2", "cpi-context"] } | ||
| light-sdk-macros = { workspace = true } | ||
| light-macros = { workspace = true } | ||
| light-account-checks = { workspace = true, features = ["solana"] } | ||
| light-hasher = { workspace = true, default-features = false } | ||
| light-compressed-account = { workspace = true } | ||
| light-compressible = { workspace = true } | ||
| light-token-interface = { workspace = true, optional = true } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the Cargo.toml files for the mentioned dependencies
fd "Cargo.toml" -t f | head -20Repository: Lightprotocol/light-protocol
Length of output: 772
🏁 Script executed:
# Find light-sdk-types and light-compressed-account Cargo.toml files
find . -name "Cargo.toml" -type f | grep -E "(light-sdk-types|light-compressed-account)" | head -10Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# List the workspace structure to understand the layout
git ls-files | grep -E "Cargo.toml$" | head -30Repository: Lightprotocol/light-protocol
Length of output: 1197
🏁 Script executed:
# Find sdk-libs directory and related crates
fd "Cargo.toml" -t f | grep -i "sdk"Repository: Lightprotocol/light-protocol
Length of output: 1393
🏁 Script executed:
# Check workspace root to understand member structure
cat Cargo.toml | grep -A 50 "\[workspace\]"Repository: Lightprotocol/light-protocol
Length of output: 1669
🏁 Script executed:
# Examine sdk-libs/sdk-types/Cargo.toml
cat sdk-libs/sdk-types/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 1665
🏁 Script executed:
# Examine program-libs/compressed-account/Cargo.toml
cat program-libs/compressed-account/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 2074
🏁 Script executed:
# Read the full sdk-libs/account/Cargo.toml to see the features definition
cat sdk-libs/account/Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 1565
🏁 Script executed:
# Check if there are any no_std tests or usage patterns
rg "no_std|no-std|#!\[no_std\]" sdk-libs/account/ sdk-tests/ --type rustRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check how light-account is used in tests and other crates
rg "light-account|light_account" --type toml | grep -v "^Binary" | head -20Repository: Lightprotocol/light-protocol
Length of output: 1037
Feature flags don't currently control std/alloc behavior for no_std targets
light-sdk-types is hardcoded with the std feature in the dependency declaration, so disabling the crate's std flag won't actually disable std compilation. Meanwhile, light-compressed-account defaults to alloc only. Since the workspace root declares this crate with default-features = false, the intent is to support feature-gated std behavior—but the hardcoded dependencies prevent that.
🛠️ Suggested adjustment
-light-sdk-types = { workspace = true, features = ["std", "v2", "cpi-context"] }
+light-sdk-types = { workspace = true, default-features = false, features = ["v2", "cpi-context"] }
-light-compressed-account = { workspace = true }
+light-compressed-account = { workspace = true, default-features = false }📝 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
| [features] | |
| default = ["std"] | |
| std = ["light-sdk-types/std", "light-compressed-account/std"] | |
| alloc = ["light-sdk-types/alloc", "light-compressed-account/alloc"] | |
| token = ["light-sdk-types/token", "dep:light-token-interface"] | |
| poseidon = ["light-sdk-types/poseidon", "light-hasher/poseidon"] | |
| sha256 = ["light-sdk-types/sha256", "light-hasher/sha256"] | |
| anchor = ["light-sdk-types/anchor", "dep:anchor-lang"] | |
| anchor-discriminator = ["light-sdk-macros/anchor-discriminator"] | |
| [dependencies] | |
| light-sdk-types = { workspace = true, features = ["std", "v2", "cpi-context"] } | |
| light-sdk-macros = { workspace = true } | |
| light-macros = { workspace = true } | |
| light-account-checks = { workspace = true, features = ["solana"] } | |
| light-hasher = { workspace = true, default-features = false } | |
| light-compressed-account = { workspace = true } | |
| light-compressible = { workspace = true } | |
| light-token-interface = { workspace = true, optional = true } | |
| [features] | |
| default = ["std"] | |
| std = ["light-sdk-types/std", "light-compressed-account/std"] | |
| alloc = ["light-sdk-types/alloc", "light-compressed-account/alloc"] | |
| token = ["light-sdk-types/token", "dep:light-token-interface"] | |
| poseidon = ["light-sdk-types/poseidon", "light-hasher/poseidon"] | |
| sha256 = ["light-sdk-types/sha256", "light-hasher/sha256"] | |
| anchor = ["light-sdk-types/anchor", "dep:anchor-lang"] | |
| anchor-discriminator = ["light-sdk-macros/anchor-discriminator"] | |
| [dependencies] | |
| light-sdk-types = { workspace = true, default-features = false, features = ["v2", "cpi-context"] } | |
| light-sdk-macros = { workspace = true } | |
| light-macros = { workspace = true } | |
| light-account-checks = { workspace = true, features = ["solana"] } | |
| light-hasher = { workspace = true, default-features = false } | |
| light-compressed-account = { workspace = true, default-features = false } | |
| light-compressible = { workspace = true } | |
| light-token-interface = { workspace = true, optional = true } |
🤖 Prompt for AI Agents
In `@sdk-libs/account/Cargo.toml` around lines 9 - 27, The crate currently forces
std by specifying light-sdk-types with its "std" feature and leaves
light-compressed-account with its alloc default, so toggle of this crate's
std/alloc features has no effect; change the dependency entries to not force
features (set default-features = false) and remove the hardcoded "std" from
light-sdk-types so the crate-level features control them. Concretely, update the
light-sdk-types dependency (the entry named light-sdk-types) to include
default-features = false and only the non-std features (e.g., "v2",
"cpi-context"), set light-compressed-account to default-features = false (so its
std/alloc is driven by our features), and ensure light-hasher keeps
default-features = false; keep the features table entries ("std" ->
"light-sdk-types/std","light-compressed-account/std" and "alloc" ->
"light-sdk-types/alloc","light-compressed-account/alloc" if present) so toggling
this crate's std/alloc features will correctly enable downstream std/alloc
flags.
Comment on lines
+61
to
112
| // Idempotency: check if token account is already initialized | ||
| // State byte at offset 108: 0=Uninitialized, 1=Initialized, 2=Frozen | ||
| const STATE_OFFSET: usize = 108; | ||
| let is_already_initialized = !token_account_info.data_is_empty() | ||
| && token_account_info.data_len() > STATE_OFFSET | ||
| && token_account_info.try_borrow_data()?[STATE_OFFSET] != 0; | ||
| let is_already_initialized = token_account_info.data_len() > STATE_OFFSET && { | ||
| let data = token_account_info | ||
| .try_borrow_data() | ||
| .map_err(|_| LightSdkTypesError::ConstraintViolation)?; | ||
| data[STATE_OFFSET] != 0 | ||
| }; | ||
|
|
||
| // Get token-specific references from context | ||
| let ctoken_compressible_config_key = ctx | ||
| .ctoken_compressible_config | ||
| .as_ref() | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)? | ||
| .key(); | ||
| let ctoken_rent_sponsor_key = ctx | ||
| .ctoken_rent_sponsor | ||
| .as_ref() | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)? | ||
| .key(); | ||
|
|
||
| if let Some((ata_bump, wallet_owner_index)) = ata_info { | ||
| // ATA path: use invoke() without signer seeds | ||
| // Resolve wallet owner pubkey from packed index | ||
| let wallet_owner_pubkey = packed_accounts | ||
| let wallet_owner_key = packed_accounts | ||
| .get(wallet_owner_index as usize) | ||
| .ok_or(ProgramError::InvalidAccountData)? | ||
| .key; | ||
|
|
||
| // Idempotency check: only create ATA if it doesn't exist | ||
| // For ATAs, we still continue with decompression even if account exists | ||
| if token_account_info.data_is_empty() { | ||
| let instruction = build_create_ata_instruction( | ||
| wallet_owner_pubkey, | ||
| mint_pubkey, | ||
| fee_payer.key, | ||
| token_account_info.key, | ||
| .ok_or(LightSdkTypesError::InvalidInstructionData)? | ||
| .key(); | ||
|
|
||
| // Idempotency: only create ATA if it doesn't exist | ||
| if token_account_info.data_len() == 0 { | ||
| let (data, account_metas) = build_create_ata_instruction( | ||
| &wallet_owner_key, | ||
| &mint_key, | ||
| &fee_payer_key, | ||
| &token_account_info.key(), | ||
| ata_bump, | ||
| ctx.ctoken_compressible_config.key, | ||
| ctx.ctoken_rent_sponsor.key, | ||
| &ctoken_compressible_config_key, | ||
| &ctoken_rent_sponsor_key, | ||
| ctx.light_config.write_top_up, | ||
| )?; | ||
|
|
||
| // Invoke WITHOUT signer seeds - ATA is derived from light token program, not our program | ||
| anchor_lang::solana_program::program::invoke(&instruction, ctx.remaining_accounts)?; | ||
| // Invoke WITHOUT signer seeds - ATA is derived from light token program | ||
| AI::invoke_cpi( | ||
| &LIGHT_TOKEN_PROGRAM_ID, | ||
| &data, | ||
| &account_metas, | ||
| ctx.remaining_accounts, | ||
| &[], | ||
| ) | ||
| .map_err(|_| LightSdkTypesError::CpiFailed)?; | ||
| } |
Contributor
There was a problem hiding this comment.
ATA idempotency can skip preallocated-but-uninitialized accounts.
The ATA path only creates when data_len == 0. If the account exists with allocated data but state == 0, we’ll skip creation and proceed with an unusable account. Consider reusing is_already_initialized (or explicitly erroring) so we don’t silently skip initialization.
🔧 Suggested fix
- if token_account_info.data_len() == 0 {
+ if !is_already_initialized {
let (data, account_metas) = build_create_ata_instruction(
&wallet_owner_key,
&mint_key,
&fee_payer_key,
&token_account_info.key(),
ata_bump,
&ctoken_compressible_config_key,
&ctoken_rent_sponsor_key,
ctx.light_config.write_top_up,
)?;🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/decompression/token.rs` around lines
61 - 112, The ATA creation currently checks only token_account_info.data_len()
== 0 which will skip initialization for preallocated-but-uninitialized accounts;
update the ATA path to use the existing is_already_initialized check
(STATE_OFFSET const and is_already_initialized variable) so you only skip
creation when the account is truly initialized, and otherwise either call
build_create_ata_instruction and invoke the CPI or return an explicit error when
data_len() > 0 but is_already_initialized is false; adjust the condition
guarding the create branch (referencing token_account_info,
is_already_initialized, STATE_OFFSET, ata_info, build_create_ata_instruction,
and AI::invoke_cpi) accordingly.
Comment on lines
+46
to
+88
| /// Internal function with const generic for optional compression_authority extraction. | ||
| fn validate_pda_common_accounts_inner<const EXTRACT_COMPRESSION_AUTHORITY: bool, AI>( | ||
| remaining_accounts: &[AI], | ||
| program_id: &[u8; 32], | ||
| ) -> Result<ValidatedPdaContext<AI>, LightSdkTypesError> | ||
| where | ||
| AI: AccountInfoTrait + Clone, | ||
| { | ||
| let mut account_iter = AccountIterator::new(remaining_accounts); | ||
|
|
||
| let fee_payer = account_iter | ||
| .next_signer_mut("fee_payer") | ||
| .map_err(LightSdkTypesError::AccountError)?; | ||
| let config = account_iter | ||
| .next_non_mut("config") | ||
| .map_err(LightSdkTypesError::AccountError)?; | ||
| let rent_sponsor = account_iter | ||
| .next_mut("rent_sponsor") | ||
| .map_err(LightSdkTypesError::AccountError)?; | ||
|
|
||
| let compression_authority = if EXTRACT_COMPRESSION_AUTHORITY { | ||
| Some( | ||
| account_iter | ||
| .next_account("compression_authority") | ||
| .map_err(LightSdkTypesError::AccountError)? | ||
| .clone(), | ||
| ) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let light_config = LightConfig::load_checked(config, program_id)?; | ||
|
|
||
| let rent_sponsor_bump = light_config.validate_rent_sponsor_account(rent_sponsor)?; | ||
|
|
||
| Ok(ValidatedPdaContext { | ||
| fee_payer: fee_payer.clone(), | ||
| light_config, | ||
| rent_sponsor: rent_sponsor.clone(), | ||
| rent_sponsor_bump, | ||
| compression_authority, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for validate_pda_common_accounts_inner usage and compression_authority handling
echo "=== Searching for validate_pda_common_accounts_inner calls ==="
rg -n "validate_pda_common_accounts_inner" -g '**/*.rs' -A 3 -B 2
echo ""
echo "=== Searching for how compression_authority is used after extraction ==="
rg -n "compression_authority" sdk-libs/sdk-types/src/interface/program/validation.rs -A 5 -B 2
echo ""
echo "=== Searching for ValidatedPdaContext usage ==="
rg -n "ValidatedPdaContext" -g '**/*.rs' -A 3 | head -100Repository: Lightprotocol/light-protocol
Length of output: 6744
🏁 Script executed:
#!/bin/bash
# Find the full context of the validation.rs file to understand usage patterns
wc -l sdk-libs/sdk-types/src/interface/program/validation.rsRepository: Lightprotocol/light-protocol
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Search for all functions that call validate_pda_common_accounts or related validation
rg -n "validate_pda_common_accounts|EXTRACT_COMPRESSION_AUTHORITY" -g '**/*.rs' -B 2 -A 5Repository: Lightprotocol/light-protocol
Length of output: 3541
🏁 Script executed:
#!/bin/bash
# Look for documentation about compression authority requirements
find . -name "*.md" -o -name "CLAUDE.md" | xargs grep -l "compression.*authority" 2>/dev/null | head -20Repository: Lightprotocol/light-protocol
Length of output: 924
🏁 Script executed:
#!/bin/bash
# Search for where validate_compress_accounts is called and how compression_authority is used
rg -n "validate_compress_accounts" -g '**/*.rs' -A 10 | head -150Repository: Lightprotocol/light-protocol
Length of output: 3319
🏁 Script executed:
#!/bin/bash
# Check the full validation.rs file to see all account validation patterns
cat -n sdk-libs/sdk-types/src/interface/program/validation.rs | head -100Repository: Lightprotocol/light-protocol
Length of output: 4298
🏁 Script executed:
#!/bin/bash
# Search for how compression_authority is validated or checked after being retrieved
rg -n "\.compression_authority" -g '**/*.rs' -A 3 -B 1Repository: Lightprotocol/light-protocol
Length of output: 50384
🏁 Script executed:
#!/bin/bash
# Find the actual compress instruction implementations (not tests)
find programs -name "*.rs" -path "*/src/*" | xargs grep -l "compress.*instruction\|fn.*compress" | head -5Repository: Lightprotocol/light-protocol
Length of output: 432
🏁 Script executed:
#!/bin/bash
# Search for where ValidatedPdaContext is used in actual program logic
rg -n "validated_context\.compression_authority\|.compression_authority\?" programs -g '**/*.rs' --max-count 20Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if there are any signer requirements or validation for compression_authority in program logic
rg -n "is_signer.*compression_authority|compression_authority.*is_signer|assert.*compression_authority|signer.*authority" programs -g '**/*.rs' -A 2 -B 2 | head -80Repository: Lightprotocol/light-protocol
Length of output: 7269
🏁 Script executed:
#!/bin/bash
# Check the full implementation of compress_and_close to see how compression_authority is used
cat -n programs/registry/src/compressible/compress_and_close.rs | head -150Repository: Lightprotocol/light-protocol
Length of output: 3684
🏁 Script executed:
#!/bin/bash
# Check how compression_authority is passed to CPI and if it's marked as signer
rg -n "compression_authority" programs/registry/src/compressible/compress_and_close.rs -A 3 -B 1Repository: Lightprotocol/light-protocol
Length of output: 965
🏁 Script executed:
#!/bin/bash
# Look for the actual accounts struct in registry program for compress operations
rg -n "struct.*Compress|pub struct" programs/registry/src/compressible/ -A 15 | head -100Repository: Lightprotocol/light-protocol
Length of output: 9195
🏁 Script executed:
#!/bin/bash
# Check if there's documentation about the expected validation patterns
find . -name "*.md" -o -name "CLAUDE.md" | xargs grep -l "validate_compress_accounts\|compression_authority" 2>/dev/nullRepository: Lightprotocol/light-protocol
Length of output: 736
🏁 Script executed:
#!/bin/bash
# Check all programs that use validate_compress_accounts to see if they have proper constraints
rg -l "validate_compress_accounts" programs -g '**/*.rs' | while read f; do
echo "=== File: $f ==="
head -50 "$f" | grep -A 10 "struct\|has_one"
doneRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check the AccountIterator and next_account implementation to understand what validation happens
rg -n "fn next_account|impl.*AccountIterator" -A 10 -g '**/*.rs' | head -80Repository: Lightprotocol/light-protocol
Length of output: 2221
🏁 Script executed:
#!/bin/bash
# Check the CONFIG_ACCOUNT.md documentation for compression_authority validation requirements
cat program-libs/compressible/docs/CONFIG_ACCOUNT.md | head -200Repository: Lightprotocol/light-protocol
Length of output: 6151
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about validate_compress_accounts requirements
rg -n "validate_compress_accounts" -B 5 -A 10 sdk-libs/sdk-types/src/interface/program/validation.rsRepository: Lightprotocol/light-protocol
Length of output: 709
🏁 Script executed:
#!/bin/bash
# Check if there are any other SDK callers of validate_compress_accounts (outside programs/)
rg -l "validate_compress_accounts" sdk-libs -g '**/*.rs'Repository: Lightprotocol/light-protocol
Length of output: 232
🏁 Script executed:
#!/bin/bash
# Check how other accounts are validated in the validation function
cat -n sdk-libs/sdk-types/src/interface/program/validation.rs | head -90Repository: Lightprotocol/light-protocol
Length of output: 3844
🏁 Script executed:
#!/bin/bash
# Look for any signer validation or similar patterns in the SDK
rg -n "validate_rent_sponsor_account" sdk-libs/sdk-types -A 5 -B 5Repository: Lightprotocol/light-protocol
Length of output: 5906
🏁 Script executed:
#!/bin/bash
# Check the comment/doc expectations for the function
head -100 sdk-libs/sdk-types/src/interface/program/validation.rsRepository: Lightprotocol/light-protocol
Length of output: 3598
Add compression_authority validation to prevent arbitrary account acceptance.
The function validates other PDA accounts (rent_sponsor via PDA derivation check) but extracts compression_authority without any validation—not even a key match against the loaded config. While downstream Anchor programs enforce this via has_one constraints, the SDK function itself should validate that the provided compression_authority matches the key stored in the loaded light_config, or callers outside Anchor contexts could pass arbitrary accounts.
Consider adding validation similar to rent_sponsor:
light_config.validate_compression_authority_account(compression_authority)?;If no such method exists, either add it to LightConfig or validate the key directly here before returning the context.
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/validation.rs` around lines 46 - 88,
In validate_pda_common_accounts_inner<const EXTRACT_COMPRESSION_AUTHORITY: bool,
AI>, stop returning compression_authority unchecked: when
EXTRACT_COMPRESSION_AUTHORITY is true, validate the extracted account against
the loaded LightConfig before constructing ValidatedPdaContext. Either call a
new LightConfig::validate_compression_authority_account(compression_authority)
method (add it to LightConfig if missing) or directly compare
compression_authority.key() to the expected key stored on light_config and
return a LightSdkTypesError on mismatch; keep the same flow and error mapping as
rent_sponsor validation.
Comment on lines
29
to
31
| v2 = ["light-sdk-types/v2"] | ||
| cpi-context = ["light-sdk-types/cpi-context"] | ||
| devnet = [] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the current state of the Cargo.toml file
cd sdk-libs/sdk && head -50 Cargo.toml | tail -30Repository: Lightprotocol/light-protocol
Length of output: 1130
🏁 Script executed:
# Also check the complete feature section to understand all features
cd sdk-libs/sdk && sed -n '/^\[features\]/,/^\[/p' Cargo.toml | head -20Repository: Lightprotocol/light-protocol
Length of output: 622
🏁 Script executed:
# Look for any documentation about feature requirements in the sdk directory
find sdk-libs/sdk -name "README.md" -o -name "CLAUDE.md" | head -5Repository: Lightprotocol/light-protocol
Length of output: 94
🏁 Script executed:
# Read the README.md to understand feature requirements
cat sdk-libs/sdk/README.mdRepository: Lightprotocol/light-protocol
Length of output: 6672
🏁 Script executed:
# Also search for any documentation about cpi-context and v2 relationship
rg "cpi-context|v2" sdk-libs/sdk/README.md -A 3 -B 3Repository: Lightprotocol/light-protocol
Length of output: 927
Enforce v2 as a dependency of cpi-context in the feature declaration.
The README clearly documents that cpi-context "Requires the v2 feature" since it enables one validity proof across multiple CPIs from different programs. Without this enforced dependency at the feature level, developers can inadvertently enable cpi-context without v2, violating the documented contract.
Suggested fix
-cpi-context = ["light-sdk-types/cpi-context"]
+cpi-context = ["light-sdk-types/cpi-context", "v2"]📝 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
| v2 = ["light-sdk-types/v2"] | |
| cpi-context = ["light-sdk-types/cpi-context"] | |
| devnet = [] | |
| v2 = ["light-sdk-types/v2"] | |
| cpi-context = ["light-sdk-types/cpi-context", "v2"] | |
| devnet = [] |
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/Cargo.toml` around lines 29 - 31, The cpi-context feature is not
declaring v2 as a required feature, so enablement can violate the documented
contract; update the features block so the cpi-context feature includes "v2" in
its array (alongside "light-sdk-types/cpi-context") so enabling cpi-context
automatically enables the v2 feature (i.e., make cpi-context depend on v2).
Comment on lines
+132
to
+156
| /// 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.
Preserve specific error variants in SDK ↔ sdk-types conversions.
The current mapping collapses multiple concrete variants (e.g., AccountDataTooSmall, InvalidInstructionData, InvalidSeeds, CpiFailed, NotEnoughAccountKeys, MissingRequiredSignature, BorshIo) into ConstraintViolation/Borsh. That drops error-code fidelity and makes downstream handling ambiguous. Please map these variants explicitly in both directions.
🔧 Suggested mapping additions
impl From<LightSdkError> for LightSdkTypesError {
fn from(e: LightSdkError) -> Self {
match e {
LightSdkError::ConstraintViolation => LightSdkTypesError::ConstraintViolation,
LightSdkError::Borsh => LightSdkTypesError::Borsh,
+ LightSdkError::BorshIo(s) => LightSdkTypesError::BorshIo(s),
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::AccountDataTooSmall => LightSdkTypesError::AccountDataTooSmall,
+ LightSdkError::InvalidInstructionData => LightSdkTypesError::InvalidInstructionData,
+ LightSdkError::InvalidSeeds => LightSdkTypesError::InvalidSeeds,
+ LightSdkError::CpiFailed => LightSdkTypesError::CpiFailed,
+ LightSdkError::NotEnoughAccountKeys => LightSdkTypesError::NotEnoughAccountKeys,
+ LightSdkError::MissingRequiredSignature => LightSdkTypesError::MissingRequiredSignature,
LightSdkError::ProgramError(e) => LightSdkTypesError::ProgramError(u64::from(e) as u32),
_ => LightSdkTypesError::ConstraintViolation,
}
}
}impl From<LightSdkTypesError> for LightSdkError {
fn from(e: LightSdkTypesError) -> Self {
match e {
...
- LightSdkTypesError::BorshIo(_) => LightSdkError::Borsh,
+ LightSdkTypesError::BorshIo(s) => LightSdkError::BorshIo(s),
+ LightSdkTypesError::AccountDataTooSmall => LightSdkError::AccountDataTooSmall,
+ LightSdkTypesError::InvalidInstructionData => LightSdkError::InvalidInstructionData,
+ LightSdkTypesError::InvalidSeeds => LightSdkError::InvalidSeeds,
+ LightSdkTypesError::CpiFailed => LightSdkError::CpiFailed,
+ LightSdkTypesError::NotEnoughAccountKeys => LightSdkError::NotEnoughAccountKeys,
+ LightSdkTypesError::MissingRequiredSignature => LightSdkError::MissingRequiredSignature,
LightSdkTypesError::ProgramError(code) => {
LightSdkError::ProgramError(ProgramError::Custom(code))
}
}
}
}Also applies to: 159-207
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/error.rs` around lines 132 - 156, The From<LightSdkError>
for LightSdkTypesError impl is collapsing many distinct LightSdkError variants
into generic ConstraintViolation/Borsh, losing error fidelity; update the match
in the impl (the From<LightSdkError> for LightSdkTypesError conversion) to
explicitly map each concrete variant (e.g., AccountDataTooSmall,
InvalidInstructionData, InvalidSeeds, CpiFailed, NotEnoughAccountKeys,
MissingRequiredSignature, BorshIo, etc.) to the corresponding LightSdkTypesError
variant instead of falling back to ConstraintViolation/Borsh, and make the
symmetrical changes in the reverse conversion impl (the From<LightSdkTypesError>
for LightSdkError or other related conversion function) so both directions
preserve the specific variants rather than collapsing them.
Comment on lines
1
to
+3
| #![cfg(feature = "compressible")] | ||
|
|
||
| use light_sdk::instruction::PackedAccounts; | ||
| use light_account::PackedAccounts; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for 'compressible' feature in token-sdk Cargo.toml
echo "=== Checking for 'compressible' feature in sdk-libs/token-sdk/Cargo.toml ==="
cat sdk-libs/token-sdk/Cargo.toml | grep -A 30 '^\[features\]'
echo ""
echo "=== All feature gates in token-sdk workspace ==="
rg 'feature.*=.*"compressible"' sdk-libs/token-sdk/ -t toml
echo ""
echo "=== Check workspace root Cargo.toml for compressible feature ==="
cat Cargo.toml | grep -A 50 '^\[features\]' | head -60Repository: Lightprotocol/light-protocol
Length of output: 1697
🏁 Script executed:
#!/bin/bash
# Verify the light_account crate and PackedAccounts export
echo "=== Check if light_account exports PackedAccounts ==="
rg 'pub.*PackedAccounts' sdk-libs/ -t rs | head -20
echo ""
echo "=== Check light_account Cargo.toml location ==="
find . -name "Cargo.toml" -path "*/light*account*" | head -5Repository: Lightprotocol/light-protocol
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Check how compressible feature is used elsewhere in the codebase
echo "=== Usage of #![cfg(feature = \"compressible\")] in tests ==="
rg '#!\[cfg\(feature = "compressible"\)\]' sdk-libs/ -t rs
echo ""
echo "=== Check other test files in token-sdk ==="
ls -la sdk-libs/token-sdk/tests/Repository: Lightprotocol/light-protocol
Length of output: 638
🏁 Script executed:
#!/bin/bash
# Read the actual pack_test.rs file
echo "=== Content of sdk-libs/token-sdk/tests/pack_test.rs (first 20 lines) ==="
head -20 sdk-libs/token-sdk/tests/pack_test.rs
echo ""
echo "=== Check workspace Cargo.toml for compressible feature ==="
grep -n "compressible" Cargo.toml | head -10Repository: Lightprotocol/light-protocol
Length of output: 945
🏁 Script executed:
#!/bin/bash
# Find light-account and light-compressible crates
echo "=== Find light-account crate ==="
find . -type d -name "light-account" | grep -v target | head -5
echo ""
echo "=== Find light-compressible crate ==="
find . -type d -name "light-compressible" | grep -v target | head -5
echo ""
echo "=== List workspace members ==="
grep -A 30 '^\[workspace\]' Cargo.tomlRepository: Lightprotocol/light-protocol
Length of output: 1105
🏁 Script executed:
#!/bin/bash
# Check if PackedAccounts is in light-compressible instead
echo "=== Search for PackedAccounts definition ==="
rg 'pub.*PackedAccounts' . --glob='*.rs' | head -20
echo ""
echo "=== Search for compressible feature definition ==="
rg 'compressible' Cargo.toml | head -20Repository: Lightprotocol/light-protocol
Length of output: 2027
Add missing compressible feature to token-sdk/Cargo.toml.
The test file is gated by #![cfg(feature = "compressible")] and the PackedAccounts import from light_account is correct. However, the compressible feature is not defined in sdk-libs/token-sdk/Cargo.toml, which means the test won't compile when explicitly enabled with --features compressible. Add the feature to Cargo.toml:
[features]
compressible = ["light-compressible"]🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/tests/pack_test.rs` around lines 1 - 3, Add a
"compressible" feature to the token-sdk crate so the test guarded by
#![cfg(feature = "compressible")] can be enabled; update Cargo.toml to declare a
features table entry named compressible that enables the dependency feature
"light-compressible" (i.e. add a compressible feature that maps to
"light-compressible") so importing PackedAccounts from light_account and running
with --features compressible compiles.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/sdk-types/src/interface/program/decompression/token.rs (1)
115-154:⚠️ Potential issue | 🟠 MajorDon’t return early when the token account already exists.
That skips adding token data/seeds, so Transfer2 won’t decompress into pre-initialized vaults.✅ Suggested fix
- if is_already_initialized { - return Ok(()); - } - let bump = &[packed.bump()]; let seeds = packed .seed_refs_with_bump(packed_accounts, bump) .map_err(|_| LightSdkTypesError::InvalidSeeds)?; @@ - let (data, account_metas) = build_create_token_account_instruction( - &token_account_info.key(), - &mint_key, - &owner, - &fee_payer_key, - &ctoken_compressible_config_key, - &ctoken_rent_sponsor_key, - ctx.light_config.write_top_up, - &signer_seeds, - ctx.program_id, - )?; - - // Invoke with PDA seeds - AI::invoke_cpi( - &LIGHT_TOKEN_PROGRAM_ID, - &data, - &account_metas, - ctx.remaining_accounts, - &[signer_seeds.as_slice()], - ) - .map_err(|e| LightSdkTypesError::ProgramError(e.into()))?; + if !is_already_initialized { + let (data, account_metas) = build_create_token_account_instruction( + &token_account_info.key(), + &mint_key, + &owner, + &fee_payer_key, + &ctoken_compressible_config_key, + &ctoken_rent_sponsor_key, + ctx.light_config.write_top_up, + &signer_seeds, + ctx.program_id, + )?; + AI::invoke_cpi( + &LIGHT_TOKEN_PROGRAM_ID, + &data, + &account_metas, + ctx.remaining_accounts, + &[signer_seeds.as_slice()], + ) + .map_err(|e| LightSdkTypesError::ProgramError(e.into()))?; + }
🤖 Fix all issues with AI agents
In `@sdk-libs/macros/src/lib.rs`:
- Around line 213-264: Add dedicated markdown docs under sdk-libs/macros/docs/
describing the new derives and attributes: create pages for LightProgram (derive
LightProgram), LightProgramPinocchio (derive LightProgramPinocchio),
#[derive(LightAccount)] and #[derive(LightAccounts)] behavior, and the
#[light_account] attribute variants (pda::seeds, token::seeds,
token::owner_seeds, associated_token, zero_copy, etc.). For each page include
short usage examples matching the enum/attribute syntax shown in the code, list
generated behavior (e.g. types/functions produced, differences for pinocchio:
Borsh vs Anchor, light_account_pinocchio:: paths, [u8;32] vs Pubkey,
Config/compress/decompress as associated functions), and cross-reference the
macro entry points light_program_derive and light_program_pinocchio_derive so
readers can locate the implementation.
In `@sdk-libs/macros/src/light_pdas/accounts/variant.rs`:
- Around line 173-196: The packed struct's field type for non-account, non-LE
bytes seeds is wrong: in generate_packed_seeds_struct_pinocchio() the
seed_fields entry for ClassifiedSeed::DataRooted currently uses Pubkey but
generate_pack_seed_fields_pinocchio() assigns self.seeds.#field (a [u8; 32] from
generate_seeds_struct_pinocchio()), causing a type mismatch; update the
packed_field_type for the DataRooted non-LE branch to use [u8; 32] (or the
equivalent array type used in generate_seeds_struct_pinocchio()) so the types
align with generate_pack_seed_fields_pinocchio and seed_fields entries.
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 349-367: The generated compress_dispatch implementation (impl
`#enum_name` :: compress_dispatch) slices account data with data[..8] and
data[8..8+size], which can panic on short buffers; add explicit length checks
before each slice (check data.len() >= 8 for the discriminator and data.len() >=
8 + size for subsequent payload slices) and return
Err(light_account::LightSdkTypesError::InvalidInstructionData) on failure; apply
the same guards in the pinocchio dispatch path and any similar slices in the
same file (the other dispatch block around the 414-433 range).
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs`:
- Around line 165-201: The code currently only checks system_accounts_offset
against remaining_accounts.len(), but later uses it to slice up to
hot_accounts_start (in the CpiAccounts::new call where
remaining_accounts[system_accounts_offset..hot_accounts_start] is passed), which
can panic if system_accounts_offset > hot_accounts_start; update the guard to
compute hot_accounts_start first (or after its computation) and return
Err(LightSdkTypesError::InvalidInstructionData) when system_accounts_offset >
hot_accounts_start so the slice is safe; apply the same defensive check to the
other similar slicing site referenced in the review (the second block that
constructs CPI/Account slices around hot accounts).
Comment on lines
+213
to
+264
| /// Derive macro for manually specifying compressed account variants on an enum. | ||
| /// | ||
| /// Generates equivalent code to `#[light_program]` auto-discovery, but allows | ||
| /// specifying account types and seeds explicitly. Useful for external programs | ||
| /// where you don't own the module. | ||
| /// | ||
| /// ## Example | ||
| /// | ||
| /// ```ignore | ||
| /// #[derive(LightProgram)] | ||
| /// pub enum ProgramAccounts { | ||
| /// #[light_account(pda::seeds = [b"record", ctx.owner])] | ||
| /// Record(MinimalRecord), | ||
| /// | ||
| /// #[light_account(pda::seeds = [RECORD_SEED, ctx.owner], pda::zero_copy)] | ||
| /// ZeroCopyRecord(ZeroCopyRecord), | ||
| /// | ||
| /// #[light_account(token::seeds = [VAULT_SEED, ctx.mint], token::owner_seeds = [AUTH_SEED])] | ||
| /// Vault, | ||
| /// | ||
| /// #[light_account(associated_token)] | ||
| /// Ata, | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// Seed expressions use explicit prefixes: | ||
| /// - `ctx.field` - context account reference | ||
| /// - `data.field` - instruction data parameter | ||
| /// - `b"literal"` or `"literal"` - byte/string literal | ||
| /// - `CONSTANT` or `path::CONSTANT` - constant in SCREAMING_SNAKE_CASE | ||
| #[proc_macro_derive(LightProgram, attributes(light_account))] | ||
| pub fn light_program_derive(input: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(input as DeriveInput); | ||
| into_token_stream(light_pdas::program::derive_light_program_impl(input)) | ||
| } | ||
|
|
||
| /// Pinocchio variant of `#[derive(LightProgram)]`. | ||
| /// | ||
| /// Generates pinocchio-compatible code instead of Anchor: | ||
| /// - `BorshSerialize/BorshDeserialize` instead of `AnchorSerialize/AnchorDeserialize` | ||
| /// - `light_account_pinocchio::` paths instead of `light_account::` | ||
| /// - Config/compress/decompress as enum associated functions | ||
| /// - `[u8; 32]` instead of `Pubkey` in generated params | ||
| /// | ||
| /// See `#[derive(LightProgram)]` for usage syntax (identical attribute syntax). | ||
| #[proc_macro_derive(LightProgramPinocchio, attributes(light_account))] | ||
| pub fn light_program_pinocchio_derive(input: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(input as DeriveInput); | ||
| into_token_stream(light_pdas::program::derive_light_program_pinocchio_impl( | ||
| input, | ||
| )) | ||
| } |
Contributor
There was a problem hiding this comment.
Document the new LightProgram / LightProgramPinocchio derives.
Please add dedicated docs in sdk-libs/macros/docs/ covering usage and generated behavior for these new derives. As per coding guidelines: “Document macro architecture and functionality in dedicated markdown files within docs/ directory, including separate documentation for #[derive(LightAccounts)], #[light_account] variants, #[derive(LightAccount)], and #[light_program] implementation details.”
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/lib.rs` around lines 213 - 264, Add dedicated markdown
docs under sdk-libs/macros/docs/ describing the new derives and attributes:
create pages for LightProgram (derive LightProgram), LightProgramPinocchio
(derive LightProgramPinocchio), #[derive(LightAccount)] and
#[derive(LightAccounts)] behavior, and the #[light_account] attribute variants
(pda::seeds, token::seeds, token::owner_seeds, associated_token, zero_copy,
etc.). For each page include short usage examples matching the enum/attribute
syntax shown in the code, list generated behavior (e.g. types/functions
produced, differences for pinocchio: Borsh vs Anchor, light_account_pinocchio::
paths, [u8;32] vs Pubkey, Config/compress/decompress as associated functions),
and cross-reference the macro entry points light_program_derive and
light_program_pinocchio_derive so readers can locate the implementation.
Comment on lines
+173
to
+196
| fn generate_packed_seeds_struct_pinocchio(&self) -> TokenStream { | ||
| let struct_name = format_ident!("Packed{}Seeds", self.variant_name); | ||
| let fields: Vec<_> = self | ||
| .seed_fields | ||
| .iter() | ||
| .map(|sf| { | ||
| let name = if sf.is_account_seed { | ||
| format_ident!("{}_idx", sf.field_name) | ||
| } else { | ||
| sf.field_name.clone() | ||
| }; | ||
| let ty = &sf.packed_field_type; | ||
| quote! { pub #name: #ty } | ||
| }) | ||
| .collect(); | ||
|
|
||
| quote! { | ||
| #[derive(borsh::BorshSerialize, borsh::BorshDeserialize, Clone, Debug)] | ||
| pub struct #struct_name { | ||
| #(#fields,)* | ||
| pub bump: u8, | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the file and examine the structure
find . -path "*/sdk-libs/macros/src/light_pdas/accounts/variant.rs" -type fRepository: Lightprotocol/light-protocol
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Search for both generate functions and understand the context
rg "generate_seeds_struct_pinocchio|generate_packed_seeds_struct_pinocchio" -A 20 sdk-libs/macros/src/light_pdas/accounts/variant.rsRepository: Lightprotocol/light-protocol
Length of output: 2570
🏁 Script executed:
#!/bin/bash
# Look for the SeedField struct and packed_field_type definition
rg "struct SeedField|packed_field_type" -B 5 -A 10 sdk-libs/macros/src/light_pdas/Repository: Lightprotocol/light-protocol
Length of output: 10304
🏁 Script executed:
#!/bin/bash
# Check for pack/unpack implementations related to seeds
rg "impl.*Pack|fn pack|fn unpack" -B 3 -A 15 sdk-libs/macros/src/light_pdas/ | head -150Repository: Lightprotocol/light-protocol
Length of output: 14987
🏁 Script executed:
#!/bin/bash
# Look for pack_stmts generation and how packed seed fields are populated
rg "pack_stmts|pack_stmt" -B 5 -A 10 sdk-libs/macros/src/light_pdas/Repository: Lightprotocol/light-protocol
Length of output: 6350
🏁 Script executed:
#!/bin/bash
# Search for pinocchio-specific pack/unpack in variant_enum.rs
rg "pinocchio" -B 3 -A 15 sdk-libs/macros/src/light_pdas/program/variant_enum.rs | head -200Repository: Lightprotocol/light-protocol
Length of output: 8799
🏁 Script executed:
#!/bin/bash
# Look at how SeedFieldInfo is constructed and what generates the pack_stmts
rg "generate_pack_impl_pinocchio" -A 50 sdk-libs/macros/src/light_pdas/accounts/variant.rsRepository: Lightprotocol/light-protocol
Length of output: 4108
🏁 Script executed:
#!/bin/bash
# Find generate_pack_seed_fields_pinocchio to see how the packed fields are actually populated
rg "generate_pack_seed_fields_pinocchio" -A 50 sdk-libs/macros/src/light_pdas/accounts/variant.rsRepository: Lightprotocol/light-protocol
Length of output: 4447
🏁 Script executed:
#!/bin/bash
# Also search for how seed_to_packed_seed is being used in the pinocchio context
rg "seed_to_packed_seed|to_packed" -B 3 -A 5 sdk-libs/macros/src/light_pdas/Repository: Lightprotocol/light-protocol
Length of output: 3422
🏁 Script executed:
#!/bin/bash
# Let's look at the entire generate_pack_seed_fields_pinocchio to confirm all branches
rg "fn generate_pack_seed_fields_pinocchio" -A 20 sdk-libs/macros/src/light_pdas/accounts/variant.rsRepository: Lightprotocol/light-protocol
Length of output: 945
🏁 Script executed:
#!/bin/bash
# Let's verify if there are any compilation errors or tests related to this
find . -name "*.rs" -path "*/tests/*" -exec grep -l "pinocchio\|pack_seed\|packed.*seeds" {} \; | head -20Repository: Lightprotocol/light-protocol
Length of output: 2026
🏁 Script executed:
#!/bin/bash
# Let's trace through what values are actually assigned in pack_seed_fields
# by examining the full context around generate_pack_seed_fields_pinocchio
cat sdk-libs/macros/src/light_pdas/accounts/variant.rs | sed -n '330,380p'Repository: Lightprotocol/light-protocol
Length of output: 2263
🏁 Script executed:
#!/bin/bash
# Check if Pubkey is used elsewhere in a way that would suggest why it was set to Pubkey
rg "packed_field_type.*Pubkey" sdk-libs/macros/src/light_pdas/accounts/variant.rs -B 5 -A 2Repository: Lightprotocol/light-protocol
Length of output: 859
🏁 Script executed:
#!/bin/bash
# Verify the full line count of the file to ensure context
wc -l sdk-libs/macros/src/light_pdas/accounts/variant.rsRepository: Lightprotocol/light-protocol
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Let's check the exact context around where packed_field_type is set to Pubkey for DataRooted non-LE cases
rg "ClassifiedSeed::DataRooted" -A 20 sdk-libs/macros/src/light_pdas/accounts/variant.rs | head -50Repository: Lightprotocol/light-protocol
Length of output: 2127
🏁 Script executed:
#!/bin/bash
# Now find the exact location where DataRooted non-LE is being processed in seed_fields creation
rg "ClassifiedSeed::DataRooted.*root.*expr" -B 3 -A 20 sdk-libs/macros/src/light_pdas/accounts/variant.rs | grep -A 20 "} else {"Repository: Lightprotocol/light-protocol
Length of output: 5580
Fix pinocchio packed seed field type for non-account, non-LE bytes seeds.
In generate_packed_seeds_struct_pinocchio(), the packed_field_type for ClassifiedSeed::DataRooted non-LE cases is incorrectly set to Pubkey, but generate_pack_seed_fields_pinocchio() assigns self.seeds.#field (which is [u8; 32] per generate_seeds_struct_pinocchio()) to that field. This type mismatch causes a compilation error.
For consistency with the pinocchio seeds struct, use [u8; 32] for non-account, non-LE bytes seeds:
Fix packed field type construction
Change line in the DataRooted branch where packed_field_type is set:
} else {
fields.push(SeedFieldInfo {
field_name,
field_type: quote! { Pubkey },
- packed_field_type: quote! { Pubkey },
+ packed_field_type: quote! { [u8; 32] },
is_account_seed: false,
has_le_bytes: false,
});
}🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/accounts/variant.rs` around lines 173 - 196,
The packed struct's field type for non-account, non-LE bytes seeds is wrong: in
generate_packed_seeds_struct_pinocchio() the seed_fields entry for
ClassifiedSeed::DataRooted currently uses Pubkey but
generate_pack_seed_fields_pinocchio() assigns self.seeds.#field (a [u8; 32] from
generate_seeds_struct_pinocchio()), causing a type mismatch; update the
packed_field_type for the DataRooted non-LE branch to use [u8; 32] (or the
equivalent array type used in generate_seeds_struct_pinocchio()) so the types
align with generate_pack_seed_fields_pinocchio and seed_fields entries.
Comment on lines
+349
to
+367
| Ok(quote! { | ||
| impl #enum_name { | ||
| pub fn compress_dispatch<'info>( | ||
| account_info: &anchor_lang::prelude::AccountInfo<'info>, | ||
| meta: &light_account::account_meta::CompressedAccountMetaNoLamportsNoAddress, | ||
| index: usize, | ||
| ctx: &mut light_account::CompressCtx<'_, 'info>, | ||
| ) -> std::result::Result<(), light_account::LightSdkTypesError> { | ||
| use light_account::LightDiscriminator; | ||
| use borsh::BorshDeserialize; | ||
| let data = account_info.try_borrow_data()?; | ||
| let discriminator: [u8; 8] = data[..8] | ||
| .try_into() | ||
| .map_err(|_| light_account::LightSdkTypesError::InvalidInstructionData)?; | ||
| match discriminator { | ||
| #(#compress_arms)* | ||
| _ => Ok(()), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add length guards before slicing account data
data[..8] and data[8..8+size] will panic on undersized/malformed accounts, turning a bad input into a hard abort. Add explicit length checks to return InvalidInstructionData instead (for both anchor and pinocchio dispatch paths).
🛡️ Suggested guard in generated dispatch (apply similarly in pinocchio path)
- let data = account_info.try_borrow_data()?;
+ let data = account_info.try_borrow_data()?;
+ if data.len() < 8 {
+ return Err(light_account::LightSdkTypesError::InvalidInstructionData);
+ }
let discriminator: [u8; 8] = data[..8]
.try_into()
.map_err(|_| light_account::LightSdkTypesError::InvalidInstructionData)?;
...
- let pod_bytes = &data[8..8 + core::mem::size_of::<#name>()];
+ let size = core::mem::size_of::<#name>();
+ if data.len() < 8 + size {
+ return Err(light_account::LightSdkTypesError::InvalidInstructionData);
+ }
+ let pod_bytes = &data[8..8 + size];Also applies to: 414-433
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/program/compress.rs` around lines 349 - 367,
The generated compress_dispatch implementation (impl `#enum_name` ::
compress_dispatch) slices account data with data[..8] and data[8..8+size], which
can panic on short buffers; add explicit length checks before each slice (check
data.len() >= 8 for the discriminator and data.len() >= 8 + size for subsequent
payload slices) and return
Err(light_account::LightSdkTypesError::InvalidInstructionData) on failure; apply
the same guards in the pinocchio dispatch path and any similar slices in the
same file (the other dispatch block around the 414-433 range).
Comment on lines
+165
to
+201
| 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)?; | ||
|
|
||
| // 3. Hot accounts (PDAs) 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 = hot_account_infos | ||
| .get(..num_pda_accounts) | ||
| .ok_or(LightSdkTypesError::NotEnoughAccountKeys)?; | ||
|
|
||
| // 4. Build CpiAccounts (system + tree accounts, excluding hot accounts) | ||
| let cpi_accounts = CpiAccounts::new( | ||
| &remaining_accounts[FEE_PAYER_INDEX], | ||
| &remaining_accounts[system_accounts_offset..hot_accounts_start], | ||
| cpi_signer, |
Contributor
There was a problem hiding this comment.
Guard system_accounts_offset against hot_accounts_start.
If the offset exceeds the hot-accounts start, the slice will panic. Return an error instead.
🔒 Suggested guard
let hot_accounts_start = remaining_accounts
.len()
.checked_sub(num_hot_accounts)
.ok_or(LightSdkTypesError::NotEnoughAccountKeys)?;
+ if system_accounts_offset > hot_accounts_start {
+ return Err(LightSdkTypesError::InvalidInstructionData);
+ }
let hot_account_infos = &remaining_accounts[hot_accounts_start..];Also applies to: 294-335
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/interface/program/decompression/processor.rs` around
lines 165 - 201, The code currently only checks system_accounts_offset against
remaining_accounts.len(), but later uses it to slice up to hot_accounts_start
(in the CpiAccounts::new call where
remaining_accounts[system_accounts_offset..hot_accounts_start] is passed), which
can panic if system_accounts_offset > hot_accounts_start; update the guard to
compute hot_accounts_start first (or after its computation) and return
Err(LightSdkTypesError::InvalidInstructionData) when system_accounts_offset >
hot_accounts_start so the slice is safe; apply the same defensive check to the
other similar slicing site referenced in the review (the second block that
constructs CPI/Account slices around hot accounts).
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
1 participant
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.
Diff Description - Detailed
Summary: Major SDK Architecture Refactoring (+23,746 / -5,554 lines, 354 files)
New crates providing framework-specific type aliases for Light Protocol types:
Moved interface definitions from sdk-libs/sdk/ to sdk-libs/sdk-types/:
2.1 Account Module (interface/account/)
2.2 Accounts Module (interface/accounts/)
2.3 CPI Module (interface/cpi/) (NEW)
2.4 Program Module (interface/program/)
2.5 New Files
3.1 New AccountInfoTrait Methods (account_info_trait.rs)
3.2 New AccountMetaTrait (account_meta_trait.rs)
3.3 New close_account Module (close_account.rs)
3.4 New Error Variants (error.rs)
3.5 Enhanced Pinocchio Implementation (pinocchio.rs)
3.6 Enhanced Solana Implementation (solana.rs)
4.1 New derive_light_program.rs (1,770 lines)
4.2 Enhanced variant.rs (457+ lines added)
4.3 Enhanced instructions.rs (794+ lines)
4.4 Enhanced compress.rs / decompress.rs
4.5 Other Changes
5.1 sdk-pinocchio/src/error.rs (+10 error variants)
5.2 sdk-types/src/error.rs
6.1 anchor-semi-manual-test
6.2 manual-test-pinocchio
6.3 pinocchio-light-program-test
10.1 Forester Updates
10.2 Token SDK (sdk-libs/token-sdk/)
10.3 SDK Core (sdk-libs/sdk/)
10.4 Program Test (sdk-libs/program-test/)
10.5 Single-* Test Crates Updates
10.6 csdk-anchor-full-derived-test
Summary by CodeRabbit
Release Notes
New Features
light-accountlibrary providing generic account manipulation utilities compatible with multiple frameworks.light-account-pinocchiolibrary for Pinocchio framework support.Improvements