Conversation
Rename the traits/ directory to account/ to better reflect that these macros are for account data structs (like UserRecord), not generic traits. - src/rentfree/traits/ -> src/rentfree/account/ - docs/traits/ -> docs/account/ - Update all import paths and documentation references
📝 WalkthroughWalkthroughReorganizes macro modules from Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Program
participant CompressibleCPI as Compressed Program
participant TokenProgram
participant SystemProgram
Client->>Program: request decompress (PDAs + optional tokens)
Program->>Program: collect PDAs, token accounts, seed params (adds placeholders for optional token fields)
Program->>CompressibleCPI: invoke InstructionDataInvokeCpiWithAccountInfo (CompressedCpiContext, account_infos, read_only_accounts)
Note right of Program: uses fixed-size seed array (MAX_SEEDS = 16) to avoid allocations
CompressibleCPI->>TokenProgram: decompress token accounts/mints (when present)
CompressibleCPI->>SystemProgram: perform lamport/account prep writes as needed
CompressibleCPI-->>Program: return result / updated account infos
Program-->>Client: return decompress outcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/token-sdk/src/token/create.rs (1)
202-218: Silent fallback to bump=0 could mask seed configuration errors.The bump extraction logic on line 206 uses
unwrap_or(0), which silently defaults to zero if:
seedsis empty- The last seed is an empty slice
This could hide misconfiguration bugs where the caller forgets to include the bump or provides seeds in the wrong order. Consider returning an error or using
expect()with a descriptive message if malformed seeds should be treated as a programming error.- let bump = seeds.last().and_then(|s| s.first()).copied().unwrap_or(0); + let bump = seeds + .last() + .and_then(|s| s.first()) + .copied() + .ok_or(ProgramError::InvalidSeeds)?;Alternatively, if this is a deliberate design choice (e.g., supporting non-PDA accounts), a brief comment explaining the fallback behavior would help future maintainers.
sdk-libs/macros/docs/rentfree_program/architecture.md (1)
149-167: Use real headings and add a language to the fenced block.Markdownlint flags MD036/MD040 here. Promote the bolded “Decompress …” lines to headings and annotate the code block with a language (e.g.,
text).✏️ Suggested doc fix
-**Decompress PDAs (Read/Modify)** +### Decompress PDAs (Read/Modify) ... -**Decompress Token Accounts and Mints** +### Decompress Token Accounts and Mints ... -``` +```text
🤖 Fix all issues with AI agents
In `@sdk-libs/macros/docs/CLAUDE.md`:
- Around line 65-71: Update the fenced code block in
sdk-libs/macros/docs/CLAUDE.md that contains the tree listing starting with
"sdk-libs/macros/src/rentfree/" to include a language specifier (e.g., change
the opening ``` to ```text) so the block renders with proper syntax
highlighting; ensure only the opening fence is modified and no other content is
changed.
In `@sdk-libs/macros/docs/rentfree_program/architecture.md`:
- Around line 163-183: The documentation is inverted: update the "Decompress
Token Accounts and Mints" section to state that clients call the generated
decompress_accounts_idempotent instruction for both PDAs and token accounts (not
that token accounts are decompressed directly by the ctoken program); explain
that decompress_accounts_idempotent handles PDA decompression itself and
delegates token decompression to the ctoken program via CPI by invoking
light_token_sdk::compressible::process_decompress_tokens_runtime, and mention
that the generated instruction therefore includes ctoken-related accounts
(light_token_program, ctoken_rent_sponsor, ctoken_cpi_authority, ctoken_config).
In `@sdk-libs/macros/src/rentfree/account/decompress_context.rs`:
- Around line 163-191: The loop in collect_pda_and_token emits three
per-iteration solana_msg logs which add linear compute cost; remove the two
less-useful per-iteration logs inside the for-loop (the "collect_pda_and_token:
processing account {}" log and the "collect_pda_and_token: token {} done" log)
and keep the entry ("collect_pda_and_token: start..."), exit ("loop done...")
and the token-variant log that shows when a PackedCTokenData branch is hit;
update any tests or comments accordingly.
In `@sdk-libs/macros/src/rentfree/account/pack_unpack.rs`:
- Around line 52-58: The macro currently treats every Pubkey field as read-only
by calling insert_or_get_read_only() in pack_unpack.rs (see is_pubkey_type
branch), which breaks cases like token-sdk's owner; change the Pubkey handling
to call remaining_accounts.insert_or_get(self.#field_name) by default (restore
insert_or_get) and add optional field-level attributes to override behavior
(e.g., #[readonly] to force insert_or_get_read_only or #[writable] to force
insert_or_get); update the macro attribute parsing logic where field attributes
are read and apply the chosen behavior for the is_pubkey_type branch (refer to
symbols is_pubkey_type, insert_or_get_read_only, insert_or_get, and the
field_name/field_type handling in the PackedAccounts code).
In `@sdk-libs/macros/src/rentfree/program/crate_context.rs`:
- Around line 74-99: get_struct_fields currently throws away module
qualification by using only the last path segment of type_name; instead, when
matching syn::Type::Path build the full qualified path string from
type_path.path.segments (e.g., join all segment.idents with "::") and use that
full path to match against your stored struct definitions rather than only
comparing item_struct.ident; update the match logic in get_struct_fields to
compare either the fully-qualified name (when available) to the struct's
module-qualified name or fall back to the plain ident match, using the same
helpers that populate self.structs() to obtain the struct's full module path so
duplicate type names in different modules resolve correctly.
In `@sdk-libs/macros/src/rentfree/program/decompress.rs`:
- Around line 201-209: The current skip in is_params_only_seed during
decompression leaves the PDA seed list incomplete and breaks PDA verification;
update the decompression logic in decompress.rs so that either (a) you collect
and include params-only seeds in the seed vector used for re-deriving the PDA
(so the PDA derivation function receives the full seed list), or (b) skip
re-derivation entirely for accounts with params-only seeds and use
compression_info.address directly for verification; locate the branch that
checks is_params_only_seed and implement one of these two behaviors consistently
(ensure the chosen path updates the decompression context/seed collection and
any PDA verification call sites to reference the full seeds or
compression_info.address accordingly).
In `@sdk-libs/macros/src/rentfree/program/variant_enum.rs`:
- Around line 19-20: The struct PdaCtxSeedInfo currently uses a fully-qualified
type for state_field_names; add `use std::collections::HashSet;` at the top of
the file and change the field type in PdaCtxSeedInfo from
`std::collections::HashSet<String>` to `HashSet<String>` (symbol:
PdaCtxSeedInfo, field: state_field_names) so it matches the project's
import/style conventions used elsewhere.
In `@sdk-libs/sdk/Cargo.toml`:
- Around line 68-73: The current configuration sets lints.rust.unexpected_cfgs
level = "allow" which silences missed cfg typos; change the value of level in
the [lints.rust.unexpected_cfgs] table from "allow" to "warn" so the check-cfg
entries (the check-cfg array containing 'cfg(target_os, values("solana"))' and
'cfg(feature, values("frozen-abi", "no-entrypoint"))') still apply but will emit
warnings instead of being fully suppressed; update the level field only and
leave the check-cfg array intact.
In `@sdk-libs/sdk/src/compressible/decompress_runtime.rs`:
- Around line 286-301: The literal mode: 1 in the
InstructionDataInvokeCpiWithAccountInfo construction is a magic number—add a
short clarifying comment immediately above or beside the
InstructionDataInvokeCpiWithAccountInfo creation explaining what mode=1
represents (e.g., "mode=1 = invoke CPI with account info / PDA-only
decompression" or similar), and preferably replace the literal with a
self-documenting constant or enum (e.g., MODE_INVOKE_ACCOUNT_INFO or
InvokeMode::AccountInfo) used in this site (update the reference where
InstructionDataInvokeCpiWithAccountInfo is constructed and any related usages
like where instruction_data.invoke(cpi_accounts.clone())? is called) so future
readers see the meaning without guessing.
- Around line 178-184: The current code silently truncates seeds when
seeds_vec.len() > MAX_SEEDS (MAX_SEEDS = 16) by using .min(MAX_SEEDS); change
this to enforce the invariant instead: before creating seed_refs, check
seeds_vec.len() against MAX_SEEDS (using debug_assert!(seeds_vec.len() <=
MAX_SEEDS) for debug builds and/or return an error (e.g., Err(...)) or panic in
release if the invariant is violated), and then use the actual length to
populate seed_refs; update the logic around seed_refs and len so you do not
silently drop seeds (refer to MAX_SEEDS, seed_refs, and seeds_vec).
| ``` | ||
| sdk-libs/macros/src/rentfree/ | ||
| ├── account/ # Trait derive macros for account data structs | ||
| ├── accounts/ # #[derive(RentFree)] implementation | ||
| ├── program/ # #[rentfree_program] implementation | ||
| ├── traits/ # Trait derive macros | ||
| ├── shared_utils.rs # Common utilities | ||
| └── mod.rs # Module exports |
There was a problem hiding this comment.
Add language specifier to fenced code block.
Per static analysis, the code block at line 65 should have a language specifier for proper rendering and syntax highlighting.
📝 Suggested fix
-```
+```text
sdk-libs/macros/src/rentfree/
├── account/ # Trait derive macros for account data structs
├── accounts/ # #[derive(RentFree)] implementation
├── program/ # #[rentfree_program] implementation
├── shared_utils.rs # Common utilities
└── mod.rs # Module exports🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/CLAUDE.md` around lines 65 - 71, Update the fenced code
block in sdk-libs/macros/docs/CLAUDE.md that contains the tree listing starting
with "sdk-libs/macros/src/rentfree/" to include a language specifier (e.g.,
change the opening ``` to ```text) so the block renders with proper syntax
highlighting; ensure only the opening fence is modified and no other content is
changed.
| **Decompress Token Accounts and Mints** | ||
|
|
||
| Token accounts (ATAs) and mints are decompressed directly via the ctoken program, not through the generated `decompress_accounts_idempotent` instruction: | ||
|
|
||
| ``` | ||
| Client fetches compressed token account/mint from indexer | ||
| | | ||
| v | ||
| Client calls ctoken program's decompress instruction directly | ||
| | | ||
| v | ||
| Token account or mint recreated on-chain | ||
| | | ||
| v | ||
| User interacts with decompressed ctoken account/mint | ||
| ``` | ||
|
|
||
| This separation exists because: | ||
| - **PDAs**: Program-specific, seeds defined by your program, decompressed via generated instruction | ||
| - **Token accounts/mints**: Standard ctoken format, decompressed via ctoken program | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the full architecture.md file to understand complete context
cat -n sdk-libs/macros/docs/rentfree_program/architecture.md | head -200Repository: Lightprotocol/light-protocol
Length of output: 9452
🏁 Script executed:
# Search for decompress_accounts_idempotent implementation
rg -n "decompress_accounts_idempotent" --type=rust -A 5 -B 2Repository: Lightprotocol/light-protocol
Length of output: 10714
🏁 Script executed:
# Search for ctoken decompression handling
rg -n "decompress.*ctoken|ctoken.*decompress" --type=rust -C 3Repository: Lightprotocol/light-protocol
Length of output: 27960
🏁 Script executed:
# Look for instruction generation related to decompression in macros
fd ".*\.rs$" sdk-libs/macros/src | head -20Repository: Lightprotocol/light-protocol
Length of output: 969
🏁 Script executed:
# Look at the decompress context implementation to see how CToken accounts are handled
cat -n sdk-libs/macros/src/rentfree/account/decompress_context.rs | head -150Repository: Lightprotocol/light-protocol
Length of output: 8227
🏁 Script executed:
# Search for how decompress_accounts_idempotent handles token accounts
rg -n "ctoken|token.*account|account.*token" sdk-libs/macros/src/rentfree/program/decompress.rs -A 3 -B 2Repository: Lightprotocol/light-protocol
Length of output: 697
🏁 Script executed:
# Check the actual processor to understand if it handles ctokens or delegates to ctoken program
cat -n sdk-libs/sdk/src/compressible/decompress_runtime.rs | head -250Repository: Lightprotocol/light-protocol
Length of output: 10719
🏁 Script executed:
# Search for CPI calls to ctoken program within decompress functions
rg -n "invoke.*ctoken|ctoken.*invoke" sdk-libs/sdk/src/compressible/ -A 2 -B 2Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Look at the implementation of process_tokens to see if it calls ctoken program
rg -n "process_tokens" sdk-libs/sdk/src/compressible/decompress_runtime.rs -A 50Repository: Lightprotocol/light-protocol
Length of output: 2549
🏁 Script executed:
# Search for ctoken program invocation patterns in decompress functions
rg -n "invoke_cpi|ctoken.*invoke|light_token" sdk-libs/sdk/src/compressible/decompress_runtime.rs -B 2 -A 3Repository: Lightprotocol/light-protocol
Length of output: 562
🏁 Script executed:
# Check what the token variant actually contains and how it's handled
rg -n "PackedTokenData|PackedCTokenData" sdk-libs/sdk/src/compressible/ -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 1973
🏁 Script executed:
# Find the actual implementation of process_tokens for DecompressContext
rg -n "impl.*DecompressContext" sdk-libs/macros/src/ -A 100 | grep -A 100 "process_tokens"Repository: Lightprotocol/light-protocol
Length of output: 2794
🏁 Script executed:
# Look for the actual process_tokens implementation in the macros
fd ".*\.rs$" sdk-libs/macros/src/rentfree/ -exec grep -l "process_tokens" {} \;Repository: Lightprotocol/light-protocol
Length of output: 280
🏁 Script executed:
# Check for token handling in the macro-generated code
cat -n sdk-libs/macros/src/rentfree/program/decompress.rs | head -200Repository: Lightprotocol/light-protocol
Length of output: 8894
🏁 Script executed:
# Look for light_token_sdk decompression imports and usage
rg -n "light_token_sdk|decompress.*token" sdk-libs/macros/src/rentfree/ -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 7317
Correct the token account decompression flow documentation—it's inverted.
The documentation states token accounts bypass decompress_accounts_idempotent and are decompressed "directly via the ctoken program," but the implementation shows the opposite: token decompression is handled by decompress_accounts_idempotent, which internally delegates to the ctoken program via CPI (line 212 in sdk-libs/macros/src/rentfree/account/decompress_context.rs calls light_token_sdk::compressible::process_decompress_tokens_runtime).
The generated instruction includes the ctoken program accounts (light_token_program, ctoken_rent_sponsor, ctoken_cpi_authority, ctoken_config) precisely because token decompression is processed within the instruction. Rewrite this section to clarify: clients call decompress_accounts_idempotent for both PDAs and token accounts; the instruction handles both types, delegating token operations to the ctoken program via CPI.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
163-163: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
167-167: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/rentfree_program/architecture.md` around lines 163 -
183, The documentation is inverted: update the "Decompress Token Accounts and
Mints" section to state that clients call the generated
decompress_accounts_idempotent instruction for both PDAs and token accounts (not
that token accounts are decompressed directly by the ctoken program); explain
that decompress_accounts_idempotent handles PDA decompression itself and
delegates token decompression to the ctoken program via CPI by invoking
light_token_sdk::compressible::process_decompress_tokens_runtime, and mention
that the generated instruction therefore includes ctoken-related accounts
(light_token_program, ctoken_rent_sponsor, ctoken_cpi_authority, ctoken_config).
| solana_msg::msg!("collect_pda_and_token: start, {} accounts", compressed_accounts.len()); | ||
| let post_system_offset = cpi_accounts.system_accounts_end_offset(); | ||
| let all_infos = cpi_accounts.account_infos(); | ||
| let post_system_accounts = &all_infos[post_system_offset..]; | ||
| let program_id = &crate::ID; | ||
|
|
||
| solana_msg::msg!("collect_pda_and_token: allocating vecs"); | ||
| let mut compressed_pda_infos = Vec::with_capacity(compressed_accounts.len()); | ||
| let mut compressed_token_accounts = Vec::with_capacity(compressed_accounts.len()); | ||
|
|
||
| solana_msg::msg!("collect_pda_and_token: starting loop"); | ||
| for (i, compressed_data) in compressed_accounts.into_iter().enumerate() { | ||
| solana_msg::msg!("collect_pda_and_token: processing account {}", i); | ||
| let meta = compressed_data.meta; | ||
| match compressed_data.data { | ||
| #(#pda_match_arms)* | ||
| RentFreeAccountVariant::PackedCTokenData(mut data) => { | ||
| solana_msg::msg!("collect_pda_and_token: token variant {}", i); | ||
| data.token_data.version = 3; | ||
| compressed_token_accounts.push((data, meta)); | ||
| solana_msg::msg!("collect_pda_and_token: token {} done", i); | ||
| } | ||
| RentFreeAccountVariant::CTokenData(_) => { | ||
| unreachable!(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| solana_msg::msg!("collect_pda_and_token: loop done, pdas={} tokens={}", compressed_pda_infos.len(), compressed_token_accounts.len()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
fd "decompress_context.rs" --type fRepository: Lightprotocol/light-protocol
Length of output: 130
🏁 Script executed:
cat -n sdk-libs/macros/src/rentfree/account/decompress_context.rs | head -200 | tail -50Repository: Lightprotocol/light-protocol
Length of output: 3290
🏁 Script executed:
# Look for feature flags or conditional compilation around logging
rg "solana_msg::msg!" sdk-libs/macros/src/rentfree/account/decompress_context.rs -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 1924
🏁 Script executed:
# Check if there's a pattern of logging elsewhere in the codebase
rg "solana_msg::msg!" sdk-libs/ --type rs | wc -lRepository: Lightprotocol/light-protocol
Length of output: 117
🏁 Script executed:
# Look for CLAUDE.md or docs about logging guidelines
fd "CLAUDE.md" sdk-libs/ --type fRepository: Lightprotocol/light-protocol
Length of output: 128
🌐 Web query:
Solana compute unit cost solana_msg::msg! logging overhead
💡 Result:
Short answer: a plain solana_program::msg! that logs small text uses the runtime's log syscall costing ~100 CU per log (log_64_units). However formatted logs (base58, string concatenation, printing pubkeys, or long messages) are significantly more expensive — examples from Solana docs show a base58+concat log ~11,962 CU vs a simple concat log ~357 CU. So use minimal msg! calls; heavy formatting or encoding (like base58 pubkey prints) drives CU into the thousands. [1][2][3]
Sources
- Solana developer guide — logging is expensive; examples and measured CU for base58/concat vs simple concat. [1]
- Solana runtime / compute-units parameter list (log_64_units ≈ 100, log_pubkey_units ≈ 100) and common syscall costs. [2]
- Community CU-optimization repo / examples and notes on logging/syscall CU costs. [3]
References
[1] Solana “How to optimize compute” (logging examples).
[2] Compute-units parameter list / blog describing log_64_units and other syscall costs.
[3] cu_optimizations GitHub (logging/compute examples).
🏁 Script executed:
cat sdk-libs/macros/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 2985
🏁 Script executed:
cat sdk-libs/macros/docs/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 3167
🏁 Script executed:
# Check if there are similar logging patterns in other files in sdk-libs
rg "solana_msg::msg!" sdk-libs/macros/src --type rust | head -20Repository: Lightprotocol/light-protocol
Length of output: 1160
🏁 Script executed:
# Look for any cfg flags or feature gates around these specific logs
cat -n sdk-libs/macros/src/rentfree/account/decompress_context.rs | head -165 | tail -20Repository: Lightprotocol/light-protocol
Length of output: 1336
🏁 Script executed:
# Check the surrounding context to understand if this is a macro-generated code
rg -B 10 "collect_pda_and_token<" sdk-libs/macros/src/rentfree/account/decompress_context.rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 500
On-chain logging trades observability for compute cost—consider optimizing for typical batch sizes.
These solana_msg::msg! calls are intentional framework logging in macro-generated decompression infrastructure. While each log consumes compute units (~100 CU per simple log), the actual cost depends on typical batch sizes for compressed accounts.
The three per-iteration logs (lines 175, 180, 183) scale linearly with account count. If most decompression operations handle small batches (e.g., < 10 accounts), the overhead is minimal. But for larger batches, they compound quickly.
Consider:
- Measure first: Profile a typical transaction to see actual CU impact
- Reduce per-iteration logs: The loop-interior logs are less critical than entry/exit logs—removing lines 175 and 180 would retain observability while cutting loop-proportional costs
- Keep high-level logs: Entry (163), exit (191), and the token variant log (183) provide useful debugging context
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/account/decompress_context.rs` around lines 163
- 191, The loop in collect_pda_and_token emits three per-iteration solana_msg
logs which add linear compute cost; remove the two less-useful per-iteration
logs inside the for-loop (the "collect_pda_and_token: processing account {}" log
and the "collect_pda_and_token: token {} done" log) and keep the entry
("collect_pda_and_token: start..."), exit ("loop done...") and the token-variant
log that shows when a PackedCTokenData branch is hit; update any tests or
comments accordingly.
| if *field_name == "compression_info" { | ||
| quote! { #field_name: None } | ||
| } else if is_pubkey_type(field_type) { | ||
| quote! { #field_name: remaining_accounts.insert_or_get(self.#field_name) } | ||
| // Use read-only since pubkey fields are references (owner, authority, etc.) | ||
| // not accounts that need to be modified | ||
| quote! { #field_name: remaining_accounts.insert_or_get_read_only(self.#field_name) } | ||
| } else if is_copy_type(field_type) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the specific file and context around line 52-58
find . -name "pack_unpack.rs" -path "*/macros/*" | head -5Repository: Lightprotocol/light-protocol
Length of output: 125
🏁 Script executed:
# Get the actual content of the file
if [ -f "sdk-libs/macros/src/rentfree/account/pack_unpack.rs" ]; then
cat -n sdk-libs/macros/src/rentfree/account/pack_unpack.rs | sed -n '40,70p'
fiRepository: Lightprotocol/light-protocol
Length of output: 1508
🏁 Script executed:
# Search for all structs deriving CompressiblePack or LightCompressible
rg -n --type=rust '#\[derive.*(?:CompressiblePack|LightCompressible)' -A 10Repository: Lightprotocol/light-protocol
Length of output: 3656
🏁 Script executed:
# Look for CLAUDE.md files that might provide guidance on field handling
find . -name "CLAUDE.md" -type f | xargs grep -l "Pubkey\|field\|mutab" 2>/dev/null | head -5Repository: Lightprotocol/light-protocol
Length of output: 144
🏁 Script executed:
# Search for examples or tests showing Pubkey field usage in account operations
rg -n --type=rust 'insert_or_get' -B 3 -A 3 | head -40Repository: Lightprotocol/light-protocol
Length of output: 3447
🏁 Script executed:
cat sdk-libs/macros/CLAUDE.md | grep -A 10 -B 2 -i "pubkey\|field\|mutab"Repository: Lightprotocol/light-protocol
Length of output: 1489
🏁 Script executed:
cat CLAUDE.md | grep -A 10 -B 2 -i "compressible\|pubkey\|field"Repository: Lightprotocol/light-protocol
Length of output: 2191
🏁 Script executed:
# Look for any attribute-based field configuration in the macro documentation
rg -n --type=rust -i "field.*attribute\|#\[" sdk-libs/macros/src/ | grep -i "pubkey\|skip\|hash" | head -20Repository: Lightprotocol/light-protocol
Length of output: 54
Fix blanket read-only enforcement for Pubkey fields in PackedAccounts.
The macro forces all Pubkey fields to read-only via insert_or_get_read_only(), but this breaks real-world usage. The token-sdk (sdk-libs/token-sdk/src/pack.rs line 155) explicitly uses insert_or_get() for the owner field to allow mutability—accounts representing owners or authorities that participate in writes cannot be forced read-only. Remove the blanket read-only assumption and either restore insert_or_get() or add a field-level #[readonly] / #[writable] attribute so developers can opt-in to read-only semantics where appropriate.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/account/pack_unpack.rs` around lines 52 - 58,
The macro currently treats every Pubkey field as read-only by calling
insert_or_get_read_only() in pack_unpack.rs (see is_pubkey_type branch), which
breaks cases like token-sdk's owner; change the Pubkey handling to call
remaining_accounts.insert_or_get(self.#field_name) by default (restore
insert_or_get) and add optional field-level attributes to override behavior
(e.g., #[readonly] to force insert_or_get_read_only or #[writable] to force
insert_or_get); update the macro attribute parsing logic where field attributes
are read and apply the chosen behavior for the is_pubkey_type branch (refer to
symbols is_pubkey_type, insert_or_get_read_only, insert_or_get, and the
field_name/field_type handling in the PackedAccounts code).
| /// Get the field names of a struct by its type. | ||
| /// | ||
| /// The type can be a simple identifier (e.g., "SinglePubkeyRecord") or | ||
| /// a qualified path. Returns None if the struct is not found. | ||
| pub fn get_struct_fields(&self, type_name: &syn::Type) -> Option<Vec<String>> { | ||
| // Extract the struct name from the type path | ||
| let struct_name = match type_name { | ||
| syn::Type::Path(type_path) => type_path.path.segments.last()?.ident.to_string(), | ||
| _ => return None, | ||
| }; | ||
|
|
||
| // Find the struct by name | ||
| for item_struct in self.structs() { | ||
| if item_struct.ident == struct_name { | ||
| // Extract field names from the struct | ||
| if let syn::Fields::Named(named_fields) = &item_struct.fields { | ||
| let field_names: Vec<String> = named_fields | ||
| .named | ||
| .iter() | ||
| .filter_map(|f| f.ident.as_ref().map(|i| i.to_string())) | ||
| .collect(); | ||
| return Some(field_names); | ||
| } | ||
| } | ||
| } | ||
| None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of get_struct_fields and surrounding context
rg -n --type=rust -B3 -A3 'get_struct_fields' sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 1632
🏁 Script executed:
#!/bin/bash
# Examine the context around the call site to understand what inner_type is
cat -n sdk-libs/macros/src/rentfree/program/instructions.rs | sed -n '80,110p'Repository: Lightprotocol/light-protocol
Length of output: 1501
🏁 Script executed:
#!/bin/bash
# Understand what inner_type contains and what ident_to_type does
cat -n sdk-libs/macros/src/rentfree/program/instructions.rs | sed -n '1,50p'Repository: Lightprotocol/light-protocol
Length of output: 2333
🏁 Script executed:
#!/bin/bash
# Find the ident_to_type function definition
rg -n --type=rust -A5 'fn ident_to_type' sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 588
🏁 Script executed:
#!/bin/bash
# Examine the TokenSeedSpec structure and how inner_type is populated
rg -n --type=rust -B2 -A10 'struct TokenSeedSpec' sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 1307
🏁 Script executed:
#!/bin/bash
# Find where inner_type is populated in the #[rentfree] field parsing
rg -n --type=rust -B5 -A5 'inner_type.*=' sdk-libs/macros/src/rentfree/program/parsing.rs | head -60Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for where TokenSeedSpec is constructed with inner_type
rg -n --type=rust 'TokenSeedSpec' sdk-libs/macros/src/rentfree/program/parsing.rs | head -20Repository: Lightprotocol/light-protocol
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Look at the Parse implementation to see how inner_type is set
cat -n sdk-libs/macros/src/rentfree/program/parsing.rs | sed -n '68,160p'Repository: Lightprotocol/light-protocol
Length of output: 4153
🏁 Script executed:
#!/bin/bash
# Find where inner_type is populated (set by caller for #[rentfree] fields)
rg -n --type=rust 'inner_type.*Some' sdk-libs/macros/src/rentfree/program | head -20Repository: Lightprotocol/light-protocol
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Look at the context around line 520 to understand what pda.inner_type is
cat -n sdk-libs/macros/src/rentfree/program/instructions.rs | sed -n '505,535p'Repository: Lightprotocol/light-protocol
Length of output: 1420
🏁 Script executed:
#!/bin/bash
# Find the PDA struct definition and what inner_type contains
rg -n --type=rust -B3 'struct.*Pda' sdk-libs/macros/src/rentfree/program/parsing.rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Find where pda_specs are extracted and what pda.inner_type represents
rg -n --type=rust 'pda_specs' sdk-libs/macros/src/rentfree/program/instructions.rs | head -20Repository: Lightprotocol/light-protocol
Length of output: 282
🏁 Script executed:
#!/bin/bash
# Find ExtractedSeedSpec definition to understand pda.inner_type
rg -n --type=rust -B3 -A10 'struct ExtractedSeedSpec' sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 1561
🏁 Script executed:
#!/bin/bash
# Find how inner_type is extracted from Account<'info, T>
rg -n --type=rust -B10 -A5 'extract_inner_type\|get_inner_type\|Account.*inner' sdk-libs/macros/src/rentfree | head -80Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Find where ExtractedSeedSpec is constructed
rg -n --type=rust 'ExtractedSeedSpec {' sdk-libs/macros/src/rentfreeRepository: Lightprotocol/light-protocol
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Find where ExtractedSeedSpec is constructed
rg -n --type=rust 'ExtractedSeedSpec' sdk-libs/macros/src/rentfree/account/seed_extraction.rs | grep -v 'struct\|pub\|//'Repository: Lightprotocol/light-protocol
Length of output: 123
Struct matching by name only can mis-resolve qualified types with duplicate names.
get_struct_fields extracts only the final path segment (line 81: .last()?.ident.to_string()) and discards module qualification. When the inner_type is a fully qualified path like crate::state::UserRecord, this approach loses that qualification and will match any struct named UserRecord across all modules. If the codebase contains structs with the same name in different modules, this risks returning the wrong field list.
The wrapper type concern (e.g., Account<'info, T>) appears to be handled before reaching this function, as inner_type is extracted from the generic argument prior to the call.
To address the ambiguity risk with qualified paths, match against the full module path as suggested:
🛠️ Suggested fix (match full qualified path)
- pub fn get_struct_fields(&self, type_name: &syn::Type) -> Option<Vec<String>> {
- // Extract the struct name from the type path
- let struct_name = match type_name {
- syn::Type::Path(type_path) => type_path.path.segments.last()?.ident.to_string(),
- _ => return None,
- };
-
- // Find the struct by name
- for item_struct in self.structs() {
- if item_struct.ident == struct_name {
- // Extract field names from the struct
- if let syn::Fields::Named(named_fields) = &item_struct.fields {
- let field_names: Vec<String> = named_fields
- .named
- .iter()
- .filter_map(|f| f.ident.as_ref().map(|i| i.to_string()))
- .collect();
- return Some(field_names);
- }
- }
- }
- None
- }
+ pub fn get_struct_fields(&self, type_name: &syn::Type) -> Option<Vec<String>> {
+ let type_path = match type_name {
+ syn::Type::Path(type_path) => type_path,
+ _ => return None,
+ };
+
+ let type_path_str = type_path
+ .path
+ .segments
+ .iter()
+ .map(|seg| seg.ident.to_string())
+ .collect::<Vec<_>>()
+ .join("::");
+
+ let mut matches = Vec::new();
+ for module in self.modules.values() {
+ for item_struct in module.structs() {
+ let full = format!("{}::{}", module.path, item_struct.ident);
+ if full.ends_with(&type_path_str) {
+ if let syn::Fields::Named(named_fields) = &item_struct.fields {
+ matches.push(
+ named_fields
+ .named
+ .iter()
+ .filter_map(|f| f.ident.as_ref().map(|i| i.to_string()))
+ .collect(),
+ );
+ } else {
+ return None;
+ }
+ }
+ }
+ }
+
+ if matches.len() == 1 { Some(matches.remove(0)) } else { None }
+ }🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/program/crate_context.rs` around lines 74 - 99,
get_struct_fields currently throws away module qualification by using only the
last path segment of type_name; instead, when matching syn::Type::Path build the
full qualified path string from type_path.path.segments (e.g., join all
segment.idents with "::") and use that full path to match against your stored
struct definitions rather than only comparing item_struct.ident; update the
match logic in get_struct_fields to compare either the fully-qualified name
(when available) to the struct's module-qualified name or fall back to the plain
ident match, using the same helpers that populate self.structs() to obtain the
struct's full module path so duplicate type names in different modules resolve
correctly.
| /// Field names that exist on the state struct (for filtering data.* seeds) | ||
| pub state_field_names: std::collections::HashSet<String>, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
State-field awareness enables safer seed validation.
Adding state_field_names to PdaCtxSeedInfo allows the codegen to verify that data.field references in seeds actually exist on the state struct—a good defensive measure that will catch misconfigurations at compile time.
Consider adding a use std::collections::HashSet; import at the top of the file to avoid the fully-qualified path in the struct definition, matching the style used elsewhere (e.g., line 392).
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/program/variant_enum.rs` around lines 19 - 20,
The struct PdaCtxSeedInfo currently uses a fully-qualified type for
state_field_names; add `use std::collections::HashSet;` at the top of the file
and change the field type in PdaCtxSeedInfo from
`std::collections::HashSet<String>` to `HashSet<String>` (symbol:
PdaCtxSeedInfo, field: state_field_names) so it matches the project's
import/style conventions used elsewhere.
| [lints.rust.unexpected_cfgs] | ||
| level = "allow" | ||
| check-cfg = [ | ||
| 'cfg(target_os, values("solana"))', | ||
| 'cfg(feature, values("frozen-abi", "no-entrypoint"))', | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider warn for unexpected_cfgs so typos don’t get silenced.
With level = "allow", the whitelist won’t surface mistakes. If the intent is to keep check-cfg while still catching typos, warn is safer.
♻️ Suggested tweak
[lints.rust.unexpected_cfgs]
-level = "allow"
+level = "warn"
check-cfg = [
'cfg(target_os, values("solana"))',
'cfg(feature, values("frozen-abi", "no-entrypoint"))',
]📝 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.
| [lints.rust.unexpected_cfgs] | |
| level = "allow" | |
| check-cfg = [ | |
| 'cfg(target_os, values("solana"))', | |
| 'cfg(feature, values("frozen-abi", "no-entrypoint"))', | |
| ] | |
| [lints.rust.unexpected_cfgs] | |
| level = "warn" | |
| check-cfg = [ | |
| 'cfg(target_os, values("solana"))', | |
| 'cfg(feature, values("frozen-abi", "no-entrypoint"))', | |
| ] |
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/Cargo.toml` around lines 68 - 73, The current configuration sets
lints.rust.unexpected_cfgs level = "allow" which silences missed cfg typos;
change the value of level in the [lints.rust.unexpected_cfgs] table from "allow"
to "warn" so the check-cfg entries (the check-cfg array containing
'cfg(target_os, values("solana"))' and 'cfg(feature, values("frozen-abi",
"no-entrypoint"))') still apply but will emit warnings instead of being fully
suppressed; update the level field only and leave the check-cfg array intact.
| // Use fixed-size array to avoid heap allocation (MAX_SEEDS = 16) | ||
| const MAX_SEEDS: usize = 16; | ||
| let mut seed_refs: [&[u8]; MAX_SEEDS] = [&[]; MAX_SEEDS]; | ||
| let len = seeds_vec.len().min(MAX_SEEDS); | ||
| for i in 0..len { | ||
| seed_refs[i] = seeds_vec[i].as_slice(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent truncation if seeds exceed MAX_SEEDS.
The optimization is solid for avoiding heap allocations. However, if seeds_vec.len() > 16, seeds are silently truncated via .min(MAX_SEEDS). While Solana PDAs are limited to 16 seeds by design, silent truncation could mask bugs if invariants change upstream.
Consider adding a debug assertion or early return with an error:
🔧 Optional: Add defensive check
const MAX_SEEDS: usize = 16;
let mut seed_refs: [&[u8]; MAX_SEEDS] = [&[]; MAX_SEEDS];
let len = seeds_vec.len().min(MAX_SEEDS);
+ debug_assert!(
+ seeds_vec.len() <= MAX_SEEDS,
+ "PDA seeds exceed MAX_SEEDS ({}), truncating",
+ seeds_vec.len()
+ );
for i in 0..len {
seed_refs[i] = seeds_vec[i].as_slice();
}📝 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.
| // Use fixed-size array to avoid heap allocation (MAX_SEEDS = 16) | |
| const MAX_SEEDS: usize = 16; | |
| let mut seed_refs: [&[u8]; MAX_SEEDS] = [&[]; MAX_SEEDS]; | |
| let len = seeds_vec.len().min(MAX_SEEDS); | |
| for i in 0..len { | |
| seed_refs[i] = seeds_vec[i].as_slice(); | |
| } | |
| // Use fixed-size array to avoid heap allocation (MAX_SEEDS = 16) | |
| const MAX_SEEDS: usize = 16; | |
| let mut seed_refs: [&[u8]; MAX_SEEDS] = [&[]; MAX_SEEDS]; | |
| let len = seeds_vec.len().min(MAX_SEEDS); | |
| debug_assert!( | |
| seeds_vec.len() <= MAX_SEEDS, | |
| "PDA seeds exceed MAX_SEEDS ({}), truncating", | |
| seeds_vec.len() | |
| ); | |
| for i in 0..len { | |
| seed_refs[i] = seeds_vec[i].as_slice(); | |
| } |
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/compressible/decompress_runtime.rs` around lines 178 - 184,
The current code silently truncates seeds when seeds_vec.len() > MAX_SEEDS
(MAX_SEEDS = 16) by using .min(MAX_SEEDS); change this to enforce the invariant
instead: before creating seed_refs, check seeds_vec.len() against MAX_SEEDS
(using debug_assert!(seeds_vec.len() <= MAX_SEEDS) for debug builds and/or
return an error (e.g., Err(...)) or panic in release if the invariant is
violated), and then use the actual length to populate seed_refs; update the
logic around seed_refs and len so you do not silently drop seeds (refer to
MAX_SEEDS, seed_refs, and seeds_vec).
| let instruction_data = InstructionDataInvokeCpiWithAccountInfo { | ||
| mode: 1, | ||
| bump: cpi_signer_config.bump, | ||
| invoking_program_id: cpi_signer_config.program_id.into(), | ||
| compress_or_decompress_lamports: 0, | ||
| is_compress: false, | ||
| with_cpi_context: false, | ||
| with_transaction_hash: false, | ||
| cpi_context: CompressedCpiContext::default(), | ||
| proof: proof.0, | ||
| new_address_params: Vec::new(), | ||
| account_infos: compressed_pda_infos, | ||
| read_only_addresses: Vec::new(), | ||
| read_only_accounts: Vec::new(), | ||
| }; | ||
| instruction_data.invoke(cpi_accounts.clone())?; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting the mode: 1 magic number.
The manual instruction construction looks correct for decompression (is_compress: false, with_cpi_context: false for PDAs-only). However, mode: 1 is a magic number without explanation.
A brief comment would help maintainers understand what mode values represent:
📝 Optional: Add clarifying comment
let instruction_data = InstructionDataInvokeCpiWithAccountInfo {
- mode: 1,
+ mode: 1, // CPI invocation mode
bump: cpi_signer_config.bump,📝 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.
| let instruction_data = InstructionDataInvokeCpiWithAccountInfo { | |
| mode: 1, | |
| bump: cpi_signer_config.bump, | |
| invoking_program_id: cpi_signer_config.program_id.into(), | |
| compress_or_decompress_lamports: 0, | |
| is_compress: false, | |
| with_cpi_context: false, | |
| with_transaction_hash: false, | |
| cpi_context: CompressedCpiContext::default(), | |
| proof: proof.0, | |
| new_address_params: Vec::new(), | |
| account_infos: compressed_pda_infos, | |
| read_only_addresses: Vec::new(), | |
| read_only_accounts: Vec::new(), | |
| }; | |
| instruction_data.invoke(cpi_accounts.clone())?; | |
| let instruction_data = InstructionDataInvokeCpiWithAccountInfo { | |
| mode: 1, // CPI invocation mode | |
| bump: cpi_signer_config.bump, | |
| invoking_program_id: cpi_signer_config.program_id.into(), | |
| compress_or_decompress_lamports: 0, | |
| is_compress: false, | |
| with_cpi_context: false, | |
| with_transaction_hash: false, | |
| cpi_context: CompressedCpiContext::default(), | |
| proof: proof.0, | |
| new_address_params: Vec::new(), | |
| account_infos: compressed_pda_infos, | |
| read_only_addresses: Vec::new(), | |
| read_only_accounts: Vec::new(), | |
| }; | |
| instruction_data.invoke(cpi_accounts.clone())?; |
🤖 Prompt for AI Agents
In `@sdk-libs/sdk/src/compressible/decompress_runtime.rs` around lines 286 - 301,
The literal mode: 1 in the InstructionDataInvokeCpiWithAccountInfo construction
is a magic number—add a short clarifying comment immediately above or beside the
InstructionDataInvokeCpiWithAccountInfo creation explaining what mode=1
represents (e.g., "mode=1 = invoke CPI with account info / PDA-only
decompression" or similar), and preferably replace the literal with a
self-documenting constant or enum (e.g., MODE_INVOKE_ACCOUNT_INFO or
InvokeMode::AccountInfo) used in this site (update the reference where
InstructionDataInvokeCpiWithAccountInfo is constructed and any related usages
like where instruction_data.invoke(cpi_accounts.clone())? is called) so future
readers see the meaning without guessing.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/create_accounts_proof.rs`:
- Around line 140-161: The token-only branch currently returns
PackedAddressTreeInfo::default() instead of the actual address tree info
produced by pack_proof; update the return to extract the first address tree info
from packed (e.g., via
packed.packed_tree_infos.address_trees.first().copied().unwrap_or_default()) and
use that as the CreateAccountsProof.address_tree_info so the
CreateAccountsProofResult created after calling pack_proof reflects the same
tree info as the non-empty path; locate this change around the token-only branch
that calls pack_proof and adjust the CreateAccountsProof construction to use the
derived address_tree_info instead of PackedAddressTreeInfo::default().
In `@sdk-libs/macros/docs/accounts/light_mint.md`:
- Line 76: Add a language specifier to the fenced code block that contains the
"Syntax notes:" PDA derivation example so it renders correctly; locate the
fenced block starting with the "Syntax notes:" line in light_mint.md and change
its opening fence to include the specifier (for example use ```text) so the
block becomes a labeled code fence rather than a plain ``` fence.
- Around line 82-90: The docs show inconsistent seed/bump examples which may
confuse readers about when to use self.field versus ctx.bumps.field_name; update
the text around mint_seeds, the Syntax notes, and the three examples to
explicitly state that use self.field.to_account_info().key.as_ref() when
referencing accounts from an accounts struct (e.g.,
self.authority.to_account_info().key.as_ref()), and use ctx.bumps.<name> (e.g.,
ctx.bumps.mint_signer or ctx.bumps.authority) only when the PDA bump was
generated by Anchor via an #[account(seeds=..., bump)] constraint and is
available on the context; add one short clarifying sentence and a small inline
example mapping mint_seeds -> [LP_MINT_SIGNER_SEED,
self.authority.to_account_info().key.as_ref(), &[params.mint_signer_bump]] and
mention that params.mint_signer_bump is for manually passed bumps versus
ctx.bumps.mint_signer for Anchor-derived bumps so readers can see when to use
each form.
- Line 51: The fenced code block that shows the type signature for `mint_seeds`
should include a language specifier for proper syntax highlighting; update the
markdown fence around the example (the block containing the `let (mint_pda,
bump) = light_token_sdk::token::find_mint_address(mint_signer.key);` snippet) to
use ```rust instead of a plain ``` so the example and references to
`mint_seeds`, `mint_signer`, and `find_mint_address` are highlighted correctly.
♻️ Duplicate comments (2)
sdk-libs/macros/src/rentfree/program/decompress.rs (1)
201-209: Skipping params-only seeds breaks PDA verification.The comment at lines 205-207 correctly identifies the problem: "we still need all seeds for the PDA verification." Yet the code skips these seeds with
continue, producing an incomplete seed set. Whenfind_program_addressis called at line 228, it will derive a different PDA than the one stored incompression_info.address.This needs one of these resolutions:
- Use
compression_info.addressdirectly — skip re-derivation entirely for accounts with params-only seeds- Require seed params during decompression — add a mechanism to pass the missing seed values
- Store complete seeds — persist the full seed bytes alongside
compression_infoThe current
continueacts as if the problem is solved, but it's not.#!/bin/bash # Verify how compression_info.address is used in verification vs re-derivation rg -n "compression_info\.address|derive_pda_seeds_with_accounts" --type rust -B 3 -A 8sdk-libs/macros/docs/CLAUDE.md (1)
76-82: Add language specifier to fenced code block.Per static analysis and the past review comment, this code block showing the directory tree should have a language specifier.
📝 Proposed fix
-``` +```text sdk-libs/macros/src/rentfree/ ├── account/ # Trait derive macros for account data structs ├── accounts/ # #[derive(RentFree)] implementation ├── program/ # #[rentfree_program] implementation ├── shared_utils.rs # Common utilities └── mod.rs # Module exports
| | `mint_signer` | Field reference | The AccountInfo that seeds the mint PDA. The mint address is derived from this signer. | | ||
| | `authority` | Field reference | The mint authority. Either a transaction signer or a PDA (if `authority_seeds` is provided). | | ||
| | `decimals` | Expression | Token decimals (e.g., `9` for 9 decimal places). | | ||
| | `mint_seeds` | Slice expression | PDA signer seeds for `mint_signer`. Must be a `&[&[u8]]` expression that matches the `#[account(seeds = ...)]` on `mint_signer`, **including the bump**. | |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block showing the type signature should have a language specifier for proper syntax highlighting.
📝 Proposed fix
-```
+```rust
let (mint_pda, bump) = light_token_sdk::token::find_mint_address(mint_signer.key);🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/light_mint.md` at line 51, The fenced code
block that shows the type signature for `mint_seeds` should include a language
specifier for proper syntax highlighting; update the markdown fence around the
example (the block containing the `let (mint_pda, bump) =
light_token_sdk::token::find_mint_address(mint_signer.key);` snippet) to use
```rust instead of a plain ``` so the example and references to `mint_seeds`,
`mint_signer`, and `find_mint_address` are highlighted correctly.
| ### Signer Seeds (mint_seeds) | ||
|
|
||
| The `mint_seeds` attribute provides the PDA signer seeds used for `invoke_signed` when calling the light token program. These seeds must derive to the `mint_signer` pubkey for the CPI to succeed. | ||
|
|
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block showing PDA derivation should have a language specifier for proper rendering.
📝 Proposed fix
-```
+```text
**Syntax notes:**
- Use `self.field` to reference accounts in the struct
- Use `.to_account_info().key` to get account pubkeys
- The bump must be passed explicitly (typically via instruction params)📝 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.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/light_mint.md` at line 76, Add a language
specifier to the fenced code block that contains the "Syntax notes:" PDA
derivation example so it renders correctly; locate the fenced block starting
with the "Syntax notes:" line in light_mint.md and change its opening fence to
include the specifier (for example use ```text) so the block becomes a labeled
code fence rather than a plain ``` fence.
| mint_seeds = &[LP_MINT_SIGNER_SEED, self.authority.to_account_info().key.as_ref(), &[params.mint_signer_bump]] | ||
| )] | ||
| pub cmint: UncheckedAccount<'info>, | ||
| ``` | ||
|
|
||
| **Syntax notes:** | ||
| - Use `self.field` to reference accounts in the struct | ||
| - Use `.to_account_info().key` to get account pubkeys | ||
| - The bump must be passed explicitly (typically via instruction params) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the inconsistent seed syntax patterns.
The documentation shows three different approaches to specifying mint_seeds:
- Line 82: Uses
self.authority.to_account_info().key.as_ref() - Line 134: Uses
ctx.bumps.mint_signer - Line 166: Uses
ctx.bumps.authority
While these may all be valid, the transition between line 82's example (showing self. prefix) and the syntax notes at lines 87-90 could confuse readers about when to use self.field vs ctx.bumps.field_name. Consider adding a note explaining that ctx.bumps is available for PDA bumps derived by Anchor's #[account(seeds = ..., bump)] constraint.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/light_mint.md` around lines 82 - 90, The docs
show inconsistent seed/bump examples which may confuse readers about when to use
self.field versus ctx.bumps.field_name; update the text around mint_seeds, the
Syntax notes, and the three examples to explicitly state that use
self.field.to_account_info().key.as_ref() when referencing accounts from an
accounts struct (e.g., self.authority.to_account_info().key.as_ref()), and use
ctx.bumps.<name> (e.g., ctx.bumps.mint_signer or ctx.bumps.authority) only when
the PDA bump was generated by Anchor via an #[account(seeds=..., bump)]
constraint and is available on the context; add one short clarifying sentence
and a small inline example mapping mint_seeds -> [LP_MINT_SIGNER_SEED,
self.authority.to_account_info().key.as_ref(), &[params.mint_signer_bump]] and
mention that params.mint_signer_bump is for manually passed bumps versus
ctx.bumps.mint_signer for Anchor-derived bumps so readers can see when to use
each form.
…struct generation, added full account struct tests
ananas-block
force-pushed
the
jorrit/refactor-sdk-macros-3
branch
from
21e730d to
9c46126
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/macros/docs/accounts/architecture.md (1)
14-35: Correcttraits/directory name toaccount/throughout documentation.The documentation references a
traits/directory, but the actual module is namedaccount/in the codebase. The files within it (traits.rs, pack_unpack.rs, light_compressible.rs, seed_extraction.rs, decompress_context.rs, utils.rs, and mod.rs) exist in the correct location, but under the singularaccount/directory, nottraits/. This naming discrepancy appears in both sections 14-35 and 509-567 and should be updated to match the actual structure.sdk-libs/macros/src/rentfree/account/decompress_context.rs (1)
34-96: Preserve caller-providedseed_paramswhen a variant has no params-only fields.Right now every arm passes
Some(&variant_seed_params)even whenparams_only_fieldsis empty, which overrides theseed_paramsargument and can change PDA derivation vs. prior behavior (and makesseed_paramsunused in generated code). Consider passing throughseed_paramsfor variants without params-only fields and only usingvariant_seed_paramswhen you actually populate it.🔧 Suggested adjustment
- let seed_params_update = if params_only_fields.is_empty() { - // No update needed - use the default value declared before match - quote! {} - } else { - let field_inits: Vec<_> = params_only_fields.iter().map(|(field, _, _)| { - quote! { `#field`: std::option::Option::Some(`#field`) } - }).collect(); - quote! { variant_seed_params = SeedParams { #(`#field_inits`,)* ..Default::default() }; } - }; + let (seed_params_update, seed_params_arg) = if params_only_fields.is_empty() { + // No params-only fields; preserve caller-provided seed_params + (quote! {}, quote! { seed_params }) + } else { + let field_inits: Vec<_> = params_only_fields.iter().map(|(field, _, _)| { + quote! { `#field`: std::option::Option::Some(`#field`) } + }).collect(); + ( + quote! { variant_seed_params = SeedParams { #(`#field_inits`,)* ..Default::default() }; }, + quote! { std::option::Option::Some(&variant_seed_params) }, + ) + }; @@ - std::option::Option::Some(&variant_seed_params), + `#seed_params_arg`,
🤖 Fix all issues with AI agents
In `@sdk-libs/macros/docs/accounts/architecture.md`:
- Around line 503-506: Update the doc text to stop implying the hasher
auto-excludes compression_info and instead state that hashing serializes the
entire struct (so compression_info is included unless you explicitly remove it);
explain that exclusion is performed by the compression layer via the
CompressAs::compress_as() implementation (and the Compressible derive which
calls it) which sets compression_info to None, and instruct users to either use
CompressAs/Compressible or manually set compression_info = None before hashing
rather than relying on automatic hasher behavior; replace wording like
"auto-detected" and "set it to None before hashing" with this explicit guidance
and reference the symbols compression_info, CompressAs, and Compressible in the
text.
In `@sdk-libs/macros/src/rentfree/accounts/light_mint.rs`:
- Around line 331-334: The code in light_mint.rs currently takes rent_payment
and write_top_up from mints[0], ignoring per-mint overrides; update
RentFreeBuilder::validate to ensure all entries in the mints collection have
identical rent_payment and write_top_up (or are all None) and return/emit a
clear compile-time error if any differ; specifically iterate over the mints
vector (the same one used by the light mint generation), compare each
mint.rent_payment and mint.write_top_up to the first values, and fail validation
with a descriptive message (or helper function) referencing
rent_payment/write_top_up so the generator never silently drops per-mint
overrides.
In `@sdk-libs/macros/src/rentfree/program/decompress.rs`:
- Around line 221-239: The params-only seed conversion kind is being lost—change
the boolean map params_only_has_conversion (and the storage in
params_only_seed_fields set by extract_data_field_from_expr) to carry an
Option/enum (e.g., ConversionKind::Le | ConversionKind::Be) instead of bool,
update extract_data_field_from_expr to return and store the specific
ConversionKind when it sees to_le_bytes()/to_be_bytes(), and then update the
decompression binding logic in decompress.rs (the block that currently looks up
has_conversion and always calls .to_le_bytes()) to branch on the stored
ConversionKind: call .to_le_bytes() for Le, .to_be_bytes() for Be (or
reject/validate Be if you want to disallow it). Ensure names referenced include
params_only_seed_fields/params_only_has_conversion,
extract_data_field_from_expr, and the bytes_binding logic in decompress.rs so
the conversion kind flows end-to-end.
In `@sdk-libs/macros/src/rentfree/program/instructions.rs`:
- Around line 118-152: When building all_params_only_fields from pda_ctx_seeds,
detect if a field name already exists with a different type and emit a macro
error rather than silently keeping the first type; modify the loop that fills
all_params_only_fields to check existing_ty =
all_params_only_fields.get(&field_str) and if Some(existing_ty) and existing_ty
!= field_type (compare via existing_ty.to_token_stream().to_string() or
syn::Type equality) then produce a syn::Error::new_spanned(...) (or return Err
with that error) that names the field and shows both conflicting types so the
macro user gets a compile-time error; keep the rest of the SeedParams generation
(seed_param_fields/seed_param_defaults) unchanged.
In `@sdk-libs/token-sdk/src/token/create_mints.rs`:
- Around line 589-634: The function create_mints constructs many computed
indices and slices the accounts array without validating its length, which can
panic; add a single bounds check before any slicing that computes the required
minimum length (e.g. compute n = params.mints.len(), mint_pdas_start = n + 12,
required_len = mint_pdas_start + n) and if accounts.len() < required_len return
an appropriate ProgramError (e.g. InvalidArgument); perform this check before
creating the CreateMintsCpi struct so all subsequent uses of mint_signers_start,
mint_pdas_start and the other indexed variables are safe.
- Around line 483-512: Add a short comment above the account collection in the
create_mints flow describing the exact expected account order for the CPI call
(similar to the comment in invoke_cpi_write). Document the sequence: payer,
system_accounts.light_system_program, all mint_seed_accounts,
system_accounts.cpi_authority_pda, system_accounts.registered_program_pda,
system_accounts.account_compression_authority,
system_accounts.account_compression_program, system_accounts.system_program,
cpi_context_account, output_queue, state_merkle_tree, address_tree,
compressible_config, rent_sponsor, then all mints (referencing the variables
self.payer, self.system_accounts.light_system_program, self.mint_seed_accounts,
self.system_accounts.* fields, self.cpi_context_account, self.output_queue,
self.state_merkle_tree, self.address_tree, self.compressible_config,
self.rent_sponsor, and self.mints) so future maintainers can easily map
positions to CPI argument expectations.
- Around line 542-556: Extract the hardcoded MintMetadata version number into a
shared constant (e.g., const MINT_METADATA_VERSION: u8 = 3) and use it instead
of the literal 3 wherever MintMetadata is constructed or validated; update the
construction in create_mints.rs (the MintMetadata instantiation inside
MintInstructionData) to use MINT_METADATA_VERSION and replace other occurrences
across the codebase (including the create_mint validation in
compressed_token/mint_action/actions/create_mint.rs) to reference that constant
so version changes are centralized.
- Around line 385-395: The CpiContext construction for execute_cpi_context
incorrectly sets in_queue_index to self.params.address_tree_index; update the
CpiContext initialization in the execute path (the execute_cpi_context variable)
so that in_queue_index is assigned self.params.output_queue_index (leave
in_tree_index as self.params.address_tree_index), ensuring the execute CPI reads
from the configured queue account rather than the tree account.
| - `compression_info` field is auto-detected and handled specially (no `#[skip]` needed) | ||
| - SHA256 hashes the entire struct, so no `#[hash]` attributes needed | ||
| - SHA256 hashes the entire struct via borsh serialization, so no `#[hash]` attributes needed | ||
| - **Important**: `compression_info` IS included in the hash. Set it to `None` before hashing for consistent results. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find relevant source files in macros
find sdk-libs/macros/src -type f -name "*.rs" | head -20Repository: Lightprotocol/light-protocol
Length of output: 967
🏁 Script executed:
# Search for LightHasherSha and hashing implementations
rg -n "LightHasherSha|struct LightCompressible" sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 1452
🏁 Script executed:
# Search for compression_info handling in traits
rg -n "compression_info" sdk-libs/macros/src/rentfree/traitsRepository: Lightprotocol/light-protocol
Length of output: 147
🏁 Script executed:
# Look for #[hash] attribute usage in derive macros
rg -n "#\[hash\]" sdk-libs/macros/srcRepository: Lightprotocol/light-protocol
Length of output: 1853
🏁 Script executed:
# Check CLAUDE.md files for hashing documentation
find sdk-libs/macros -name "CLAUDE.md" -o -name "*.md" | xargs grep -l "hash" 2>/dev/null | head -10Repository: Lightprotocol/light-protocol
Length of output: 371
🏁 Script executed:
# Read the light_compressible.rs implementation
cat -n sdk-libs/macros/src/rentfree/account/light_compressible.rs | head -180Repository: Lightprotocol/light-protocol
Length of output: 7645
🏁 Script executed:
# Look at the hasher light_hasher.rs to understand DataHasher implementation
cat -n sdk-libs/macros/src/hasher/light_hasher.rs | head -150Repository: Lightprotocol/light-protocol
Length of output: 6395
🏁 Script executed:
# Search for how compression_info is specifically handled in hashing
rg -n "compression_info" sdk-libs/macros/src -A 5 -B 5Repository: Lightprotocol/light-protocol
Length of output: 43176
🏁 Script executed:
# Check the documentation in CLAUDE.md about hashing
cat sdk-libs/macros/docs/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 3588
🏁 Script executed:
# Read the specific section of the documentation being reviewed
sed -n '500,510p' sdk-libs/macros/docs/accounts/architecture.mdRepository: Lightprotocol/light-protocol
Length of output: 422
🏁 Script executed:
# Get more context around lines 503-506
sed -n '495,515p' sdk-libs/macros/docs/accounts/architecture.mdRepository: Lightprotocol/light-protocol
Length of output: 637
🏁 Script executed:
# Check the data_hasher.rs to understand how SHA256 hashing actually works
rg -n "generate_data_hasher_impl_sha" sdk-libs/macros/src/hasher/ -A 30 | head -60Repository: Lightprotocol/light-protocol
Length of output: 4958
🏁 Script executed:
# Check the to_byte_array_impl_sha to see how compression_info is handled
rg -n "generate_to_byte_array_impl_sha" sdk-libs/macros/src/hasher/ -A 40 | head -80Repository: Lightprotocol/light-protocol
Length of output: 6878
🏁 Script executed:
# Look for any special handling of compression_info in the hasher code
rg -n "compression_info" sdk-libs/macros/src/hasher/ -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check the data_hasher implementation for SHA256
cat -n sdk-libs/macros/src/hasher/data_hasher.rs | head -100Repository: Lightprotocol/light-protocol
Length of output: 4668
🏁 Script executed:
# See the rest of the SHA256 DataHasher implementation
sed -n '87,150p' sdk-libs/macros/src/hasher/data_hasher.rsRepository: Lightprotocol/light-protocol
Length of output: 1366
🏁 Script executed:
# Check how compress_as is actually used in the compression workflow
rg -n "compress_as" sdk-libs/macros/src -B 2 -A 2 | head -40Repository: Lightprotocol/light-protocol
Length of output: 2404
🏁 Script executed:
# Look for any actual runtime behavior that shows how compression_info is excluded
rg -n "set_compression_info_none\|compression_info.*None" sdk-libs/macros/src -B 2 -A 2 | head -60Repository: Lightprotocol/light-protocol
Length of output: 54
Clarify that compression_info exclusion requires explicit use of CompressAs, not automatic hasher behavior.
The documentation states that compression_info is "auto-skipped" by the hasher and claims users should "set it to None before hashing." However, the SHA256 hasher implementation (to_byte_array.rs and data_hasher.rs) directly serializes the entire struct via Borsh—there is no automatic filtering of the compression_info field. The actual exclusion happens in the CompressAs trait (traits.rs:147), which explicitly sets compression_info: None when creating a compressed representation.
The documentation should clarify that hashing includes all fields as-is, and that excluding compression_info from the hash requires using the CompressAs::compress_as() method (or Compressible derive, which includes it). The current phrasing conflates the hasher's behavior with the compression workflow's behavior, which may mislead developers about which layer handles the exclusion.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/accounts/architecture.md` around lines 503 - 506, Update
the doc text to stop implying the hasher auto-excludes compression_info and
instead state that hashing serializes the entire struct (so compression_info is
included unless you explicitly remove it); explain that exclusion is performed
by the compression layer via the CompressAs::compress_as() implementation (and
the Compressible derive which calls it) which sets compression_info to None, and
instruct users to either use CompressAs/Compressible or manually set
compression_info = None before hashing rather than relying on automatic hasher
behavior; replace wording like "auto-detected" and "set it to None before
hashing" with this explicit guidance and reference the symbols compression_info,
CompressAs, and Compressible in the text.
| // Get rent_payment and write_top_up from first mint (all mints share same params for now) | ||
| let rent_payment = quote_option_or(&mints[0].rent_payment, quote! { 16u8 }); | ||
| let write_top_up = quote_option_or(&mints[0].write_top_up, quote! { 766u32 }); | ||
|
|
There was a problem hiding this comment.
Multi-mint flow silently ignores per-mint rent_payment / write_top_up.
Only the first mint’s values are applied to the entire batch. If callers specify different values per mint, those overrides are dropped with no warning. Please validate that all mints agree (e.g., in RentFreeBuilder::validate) or surface a compile-time error when they differ. I can help sketch a validation helper if you want.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/accounts/light_mint.rs` around lines 331 - 334,
The code in light_mint.rs currently takes rent_payment and write_top_up from
mints[0], ignoring per-mint overrides; update RentFreeBuilder::validate to
ensure all entries in the mints collection have identical rent_payment and
write_top_up (or are all None) and return/emit a clear compile-time error if any
differ; specifically iterate over the mints vector (the same one used by the
light mint generation), compare each mint.rent_payment and mint.write_top_up to
the first values, and fail validation with a descriptive message (or helper
function) referencing rent_payment/write_top_up so the generator never silently
drops per-mint overrides.
| // Check if this field has a conversion (to_le_bytes, to_be_bytes) | ||
| let has_conversion = params_only_has_conversion | ||
| .get(&field_name) | ||
| .copied() | ||
| .unwrap_or(false); | ||
|
|
||
| if has_conversion { | ||
| // u64 field with to_le_bytes conversion | ||
| // Must bind bytes to a variable to avoid temporary value dropped while borrowed | ||
| let bytes_binding_name = syn::Ident::new( | ||
| &format!("{}_bytes", binding_name), | ||
| proc_macro2::Span::call_site(), | ||
| ); | ||
| bindings.push(quote! { | ||
| let #binding_name = seed_params.#field_ident | ||
| .ok_or(solana_program_error::ProgramError::InvalidAccountData)?; | ||
| let #bytes_binding_name = #binding_name.to_le_bytes(); | ||
| }); | ||
| seed_refs.push(quote! { #bytes_binding_name.as_ref() }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for to_be_bytes usage in the entire codebase
rg -n "to_be_bytes\(\)" --type rustRepository: Lightprotocol/light-protocol
Length of output: 8591
🏁 Script executed:
# Look at the decompress.rs file to understand the context
wc -l sdk-libs/macros/src/rentfree/program/decompress.rsRepository: Lightprotocol/light-protocol
Length of output: 126
🏁 Script executed:
# Search for seed specification patterns in rentfree macros
rg -n "seed" sdk-libs/macros/src/rentfree/ --type rust -A 2 -B 2 | head -100Repository: Lightprotocol/light-protocol
Length of output: 10283
🏁 Script executed:
# Examine seed_extraction.rs to understand how conversions are tracked
cat -n sdk-libs/macros/src/rentfree/account/seed_extraction.rs | head -200Repository: Lightprotocol/light-protocol
Length of output: 8893
🏁 Script executed:
# Read the decompress.rs file around the area in question
cat -n sdk-libs/macros/src/rentfree/program/decompress.rs | sed -n '150,260p'Repository: Lightprotocol/light-protocol
Length of output: 6347
🏁 Script executed:
# Check earlier in decompress.rs to see how params_only_has_conversion is populated
cat -n sdk-libs/macros/src/rentfree/program/decompress.rs | sed -n '1,150p'Repository: Lightprotocol/light-protocol
Length of output: 6345
🏁 Script executed:
# Find where generate_pda_seed_derivation_for_trait_with_ctx_seeds is called
rg -n "generate_pda_seed_derivation_for_trait_with_ctx_seeds" --type rust -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 1440
🏁 Script executed:
# Search for params_only_fields construction to see how the tuple is built
rg -n "params_only_fields" sdk-libs/macros/src/rentfree/program/ --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 11475
🏁 Script executed:
# Look for where to_le_bytes or to_be_bytes is being selected/tracked in seed specs
rg -n "to_le_bytes|to_be_bytes" sdk-libs/macros/src/rentfree/ --type rustRepository: Lightprotocol/light-protocol
Length of output: 1386
🏁 Script executed:
# Look at the seed_extraction.rs to see how conversion is extracted and what gets stored
cat -n sdk-libs/macros/src/rentfree/account/seed_extraction.rs | sed -n '550,700p'Repository: Lightprotocol/light-protocol
Length of output: 6867
🏁 Script executed:
# Search for actual usage of to_be_bytes in seed expressions in test files or examples
fd -e rs . sdk-libs/macros --exec grep -l "to_be_bytes" {} \;Repository: Lightprotocol/light-protocol
Length of output: 178
🏁 Script executed:
# Check what actually gets stored in the params_only_seed_fields - look at variant_enum.rs construction
cat -n sdk-libs/macros/src/rentfree/program/variant_enum.rs | sed -n '1,100p'Repository: Lightprotocol/light-protocol
Length of output: 5204
🏁 Script executed:
# Search for actual seed specifications in test/example files that use to_be_bytes
rg -n "seeds.*to_be_bytes|to_be_bytes.*seeds" --type rust -B 5 -A 5Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Look at the actual ClassifiedSeed enum and how conversions are tracked through the pipeline
cat -n sdk-libs/macros/src/rentfree/account/seed_extraction.rs | sed -n '13,50p'Repository: Lightprotocol/light-protocol
Length of output: 1767
🏁 Script executed:
# Check if conversion type information is preserved anywhere or if only the boolean is stored
rg -n "conversion" sdk-libs/macros/src/rentfree/program/variant_enum.rs --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 486
🏁 Script executed:
# Search for any #[rentfree] seed specs in example code or tests to see actual usage patterns
fd -type f -name "*.rs" sdk-libs/macros/tests | head -5Repository: Lightprotocol/light-protocol
Length of output: 242
🏁 Script executed:
# Check test files to see if there are any examples with conversion in seed specs
find sdk-libs/macros/tests -name "*.rs" -exec grep -l "rentfree\|seeds" {} \;Repository: Lightprotocol/light-protocol
Length of output: 128
🏁 Script executed:
# Look for documentation or examples of rentfree seed usage
find . -name "*.md" -path "*/rentfree/*" -o -name "CLAUDE.md" | xargs grep -l "seed" 2>/dev/null | head -5Repository: Lightprotocol/light-protocol
Length of output: 210
🏁 Script executed:
# Check the rentfree README for seed usage examples
cat sdk-libs/macros/src/rentfree/README.mdRepository: Lightprotocol/light-protocol
Length of output: 2660
🏁 Script executed:
# Check the CLAUDE.md file in macros directory
cat sdk-libs/macros/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 2985
🏁 Script executed:
# Final confirmation: check if conversion type is preserved anywhere in the pipeline
rg -n "to_le_bytes\|to_be_bytes" sdk-libs/macros/src/rentfree/program/ --type rust -A 1 -B 1 | grep -E "(program|decompress|variant)" | head -20Repository: Lightprotocol/light-protocol
Length of output: 54
Conversion type information is lost during params-only seed field extraction.
The vulnerability is real. When extracting params-only seed fields in seed_extraction.rs line 687–691, extract_data_field_from_expr() treats both to_le_bytes() and to_be_bytes() identically, returning only a boolean flag (has_conversion = true) without preserving which conversion method was used. This information loss means that in decompress.rs line 237, the code always uses .to_le_bytes() regardless of the original seed spec.
If a developer writes data.field.to_be_bytes() in a seed specification, the PDA will derive incorrectly. To fix this, preserve the conversion method type through the pipeline—either store Option<ConversionKind> in params_only_seed_fields instead of a boolean, or validate and enforce that only little-endian conversions are permitted in params-only seed specs.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/program/decompress.rs` around lines 221 - 239,
The params-only seed conversion kind is being lost—change the boolean map
params_only_has_conversion (and the storage in params_only_seed_fields set by
extract_data_field_from_expr) to carry an Option/enum (e.g., ConversionKind::Le
| ConversionKind::Be) instead of bool, update extract_data_field_from_expr to
return and store the specific ConversionKind when it sees
to_le_bytes()/to_be_bytes(), and then update the decompression binding logic in
decompress.rs (the block that currently looks up has_conversion and always calls
.to_le_bytes()) to branch on the stored ConversionKind: call .to_le_bytes() for
Le, .to_be_bytes() for Be (or reject/validate Be if you want to disallow it).
Ensure names referenced include
params_only_seed_fields/params_only_has_conversion,
extract_data_field_from_expr, and the bytes_binding logic in decompress.rs so
the conversion kind flows end-to-end.
| // Collect all unique params-only seed fields across all variants for SeedParams struct | ||
| // Use BTreeMap for deterministic ordering | ||
| let mut all_params_only_fields: std::collections::BTreeMap<String, syn::Type> = | ||
| std::collections::BTreeMap::new(); | ||
| for ctx_info in &pda_ctx_seeds { | ||
| for (field_name, field_type, _) in &ctx_info.params_only_seed_fields { | ||
| let field_str = field_name.to_string(); | ||
| all_params_only_fields | ||
| .entry(field_str) | ||
| .or_insert_with(|| field_type.clone()); | ||
| } | ||
| } | ||
|
|
||
| let seed_params_struct = if all_params_only_fields.is_empty() { | ||
| quote! { | ||
| #[derive(anchor_lang::AnchorSerialize, anchor_lang::AnchorDeserialize, Clone, Debug, Default)] | ||
| pub struct SeedParams; | ||
| } | ||
| } else { | ||
| // Collect into Vec for consistent ordering between field declarations and Default impl | ||
| let sorted_fields: Vec<_> = all_params_only_fields.iter().collect(); | ||
| let seed_param_fields: Vec<_> = sorted_fields | ||
| .iter() | ||
| .map(|(name, ty)| { | ||
| let field_ident = format_ident!("{}", name); | ||
| quote! { pub #field_ident: Option<#ty> } | ||
| }) | ||
| .collect(); | ||
| let seed_param_defaults: Vec<_> = sorted_fields | ||
| .iter() | ||
| .map(|(name, _)| { | ||
| let field_ident = format_ident!("{}", name); | ||
| quote! { #field_ident: None } | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
Guard against conflicting params-only field types across variants.
all_params_only_fields keys only by field name. If two variants use the same params-only field name with different conversion/type (e.g., u64 vs Pubkey), the first wins silently, causing type mismatches or incorrect PDA seeds. Please detect conflicts and emit a macro error.
✅ Proposed fix (conflict check)
- let mut all_params_only_fields: std::collections::BTreeMap<String, syn::Type> =
- std::collections::BTreeMap::new();
+ let mut all_params_only_fields: std::collections::BTreeMap<String, (syn::Type, String)> =
+ std::collections::BTreeMap::new();
for ctx_info in &pda_ctx_seeds {
for (field_name, field_type, _) in &ctx_info.params_only_seed_fields {
let field_str = field_name.to_string();
- all_params_only_fields
- .entry(field_str)
- .or_insert_with(|| field_type.clone());
+ let type_str = quote::quote!(`#field_type`).to_string();
+ match all_params_only_fields.get(&field_str) {
+ Some((_, existing_str)) if existing_str != &type_str => {
+ return Err(macro_error!(
+ field_name,
+ "Params-only seed field '{}' has conflicting types across variants",
+ field_str
+ ));
+ }
+ Some(_) => {}
+ None => {
+ all_params_only_fields.insert(field_str, (field_type.clone(), type_str));
+ }
+ }
}
}
@@
- let sorted_fields: Vec<_> = all_params_only_fields.iter().collect();
+ let sorted_fields: Vec<_> = all_params_only_fields.iter().collect();
let seed_param_fields: Vec<_> = sorted_fields
.iter()
- .map(|(name, ty)| {
+ .map(|(name, (ty, _))| {
let field_ident = format_ident!("{}", name);
quote! { pub `#field_ident`: Option<#ty> }
})
.collect();🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/program/instructions.rs` around lines 118 - 152,
When building all_params_only_fields from pda_ctx_seeds, detect if a field name
already exists with a different type and emit a macro error rather than silently
keeping the first type; modify the loop that fills all_params_only_fields to
check existing_ty = all_params_only_fields.get(&field_str) and if
Some(existing_ty) and existing_ty != field_type (compare via
existing_ty.to_token_stream().to_string() or syn::Type equality) then produce a
syn::Error::new_spanned(...) (or return Err with that error) that names the
field and shows both conflicting types so the macro user gets a compile-time
error; keep the rest of the SeedParams generation
(seed_param_fields/seed_param_defaults) unchanged.
| let execute_cpi_context = CpiContext { | ||
| set_context: false, | ||
| first_set_context: false, | ||
| in_tree_index: self.params.address_tree_index, | ||
| in_queue_index: self.params.address_tree_index, // CPI context queue index | ||
| out_queue_index: self.params.output_queue_index, | ||
| token_out_queue_index: 0, | ||
| assigned_account_index: offset + last_idx as u8, | ||
| read_only_address_trees: [0; 4], | ||
| address_tree_pubkey: self.address_tree.key.to_bytes(), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the create_mints.rs file around both line 310 and 389
cd sdk-libs/token-sdk/src/token && wc -l create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 91
🏁 Script executed:
# Check the write path around line 310
cd sdk-libs/token-sdk/src/token && sed -n '300,320p' create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 1102
🏁 Script executed:
# Check the execute path around line 389
cd sdk-libs/token-sdk/src/token && sed -n '375,400p' create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 1161
🏁 Script executed:
# Search for in_queue_index usage patterns across the codebase
rg -n "in_queue_index" --type=rs -B 2 -A 2 | head -100Repository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Look for CpiContext structure definition to understand the field semantics
rg -n "struct CpiContext" --type=rs -A 10Repository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Search for documentation about queue indices
fd -e md . | xargs grep -l "in_queue_index\|output_queue_index" 2>/dev/nullRepository: Lightprotocol/light-protocol
Length of output: 56
🏁 Script executed:
# Try ripgrep without type flag
rg "in_queue_index" -B 2 -A 2 | head -120Repository: Lightprotocol/light-protocol
Length of output: 10232
🏁 Script executed:
# Find CpiContext struct definition
rg "struct CpiContext" -A 15Repository: Lightprotocol/light-protocol
Length of output: 13747
🏁 Script executed:
# Search for Light System Program CPI context handling
fd -e rs | xargs grep -l "CpiContext" | head -10Repository: Lightprotocol/light-protocol
Length of output: 656
🏁 Script executed:
# Look for CLAUDE.md or docs related to queue indices
fd -e md . -path "*/docs/*" | xargs grep -l "queue\|CpiContext" 2>/dev/null | head -10Repository: Lightprotocol/light-protocol
Length of output: 240
🏁 Script executed:
# Check if there's documentation about CPI context semantics
fd "CLAUDE.md" -o "INSERT_ADDRESS_QUEUE.md" -o "UPDATE_FROM_INPUT_QUEUE.md"Repository: Lightprotocol/light-protocol
Length of output: 237
🏁 Script executed:
# Look at invoke_execute more completely to see all context
sed -n '374,430p' sdk-libs/token-sdk/src/token/create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 2265
🏁 Script executed:
# Check what queue indices are available in params
rg "address.*queue" sdk-libs/token-sdk/src/token/create_mints.rs -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 354
🏁 Script executed:
# Look at the params structure to understand available indices
rg "struct.*CreateMint" sdk-libs/token-sdk/src/token/create_mints.rs -A 30 | head -50Repository: Lightprotocol/light-protocol
Length of output: 2177
🏁 Script executed:
# Check the queue_indices.rs test to understand the expected in_queue_index for execute
sed -n '1,100p' programs/compressed-token/program/tests/queue_indices.rsRepository: Lightprotocol/light-protocol
Length of output: 4385
🏁 Script executed:
# Search for what in_queue_index should be in execute mode - look for address_queue or input_queue references
rg "address_queue\|input_queue" sdk-libs/token-sdk/src/token/create_mints.rs -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check the invoke_execute method to see if there's an input_queue account available
sed -n '330,375p' sdk-libs/token-sdk/src/token/create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 1862
🏁 Script executed:
# Look at how decompress_mint.rs handles this (from the earlier ripgrep results)
sed -n '1,100p' sdk-libs/token-sdk/src/token/decompress_mint.rsRepository: Lightprotocol/light-protocol
Length of output: 3414
🏁 Script executed:
# Search for address_queue_index or related queue index handling in create_mints
rg "queue_index" sdk-libs/token-sdk/src/token/create_mints.rs -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 1743
🏁 Script executed:
# Look at the full CreateMintsParams structure and any helper methods
sed -n '1,150p' sdk-libs/token-sdk/src/token/create_mints.rs | grep -A 100 "pub struct CreateMintsParams"Repository: Lightprotocol/light-protocol
Length of output: 3162
🏁 Script executed:
# Check if there are any docs or examples showing the expected queue indices for execute
fd "*.md" | xargs grep -l "execute.*queue\|in_queue_index" 2>/dev/null | head -5Repository: Lightprotocol/light-protocol
Length of output: 355
🏁 Script executed:
# Look at MintActionMetaConfig to see if it reveals queue index patterns
rg "MintActionMetaConfig" sdk-libs/token-sdk/src/token/create_mints.rs -B 5 -A 15Repository: Lightprotocol/light-protocol
Length of output: 4649
Fix in_queue_index in execute CPI context—should use output_queue_index, not address_tree_index.
Line 389 incorrectly sets in_queue_index to self.params.address_tree_index. The in_queue_index and in_tree_index fields are semantically distinct: one identifies the queue account for reading compressed account data, the other identifies the tree account.
The write path (line 310) correctly uses self.params.output_queue_index for in_queue_index. The comprehensive queue indices test confirms this: for execute mode with create_mint=true, the test provides in_tree_index=1 and in_queue_index=6—two different indices for two different account types.
Using the tree index as the queue index will cause the Light System Program to read from the wrong account during CPI execution, likely resulting in account resolution failures or stale data reads.
Change line 389 to:
in_queue_index: self.params.output_queue_index,
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 385 - 395, The
CpiContext construction for execute_cpi_context incorrectly sets in_queue_index
to self.params.address_tree_index; update the CpiContext initialization in the
execute path (the execute_cpi_context variable) so that in_queue_index is
assigned self.params.output_queue_index (leave in_tree_index as
self.params.address_tree_index), ensuring the execute CPI reads from the
configured queue account rather than the tree account.
| // Collect all account infos needed for the CPI | ||
| let mut account_infos = vec![self.payer.clone()]; | ||
|
|
||
| // System accounts | ||
| account_infos.push(self.system_accounts.light_system_program.clone()); | ||
|
|
||
| // Add all mint seeds | ||
| for mint_seed in self.mint_seed_accounts { | ||
| account_infos.push(mint_seed.clone()); | ||
| } | ||
|
|
||
| // More system accounts | ||
| account_infos.push(self.system_accounts.cpi_authority_pda.clone()); | ||
| account_infos.push(self.system_accounts.registered_program_pda.clone()); | ||
| account_infos.push(self.system_accounts.account_compression_authority.clone()); | ||
| account_infos.push(self.system_accounts.account_compression_program.clone()); | ||
| account_infos.push(self.system_accounts.system_program.clone()); | ||
|
|
||
| // CPI context, queues, trees | ||
| account_infos.push(self.cpi_context_account.clone()); | ||
| account_infos.push(self.output_queue.clone()); | ||
| account_infos.push(self.state_merkle_tree.clone()); | ||
| account_infos.push(self.address_tree.clone()); | ||
| account_infos.push(self.compressible_config.clone()); | ||
| account_infos.push(self.rent_sponsor.clone()); | ||
|
|
||
| // Add all mint PDAs | ||
| for mint in self.mints { | ||
| account_infos.push(mint.clone()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Account collection is comprehensive but could use ordering documentation.
The account infos are correctly assembled for the CPI. Consider adding a comment documenting the expected account order (similar to the one in invoke_cpi_write at lines 339-345) to help future maintainers understand the layout.
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 483 - 512, Add a
short comment above the account collection in the create_mints flow describing
the exact expected account order for the CPI call (similar to the comment in
invoke_cpi_write). Document the sequence: payer,
system_accounts.light_system_program, all mint_seed_accounts,
system_accounts.cpi_authority_pda, system_accounts.registered_program_pda,
system_accounts.account_compression_authority,
system_accounts.account_compression_program, system_accounts.system_program,
cpi_context_account, output_queue, state_merkle_tree, address_tree,
compressible_config, rent_sponsor, then all mints (referencing the variables
self.payer, self.system_accounts.light_system_program, self.mint_seed_accounts,
self.system_accounts.* fields, self.cpi_context_account, self.output_queue,
self.state_merkle_tree, self.address_tree, self.compressible_config,
self.rent_sponsor, and self.mints) so future maintainers can easily map
positions to CPI argument expectations.
| MintInstructionData { | ||
| supply: 0, | ||
| decimals: mint_params.decimals, | ||
| metadata: MintMetadata { | ||
| version: 3, | ||
| mint: mint_params.mint.to_bytes().into(), | ||
| mint_decompressed: false, | ||
| mint_signer: mint_signer.to_bytes(), | ||
| bump: mint_params.bump, | ||
| }, | ||
| mint_authority: Some(mint_params.mint_authority.to_bytes().into()), | ||
| freeze_authority: mint_params.freeze_authority.map(|a| a.to_bytes().into()), | ||
| extensions: None, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Find MintMetadata definition
rg -n "struct MintMetadata|enum MintMetadata" --type=rs -A 10Repository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Check version constants and usage
rg -n "version.*3|VERSION.*=|const.*VERSION" --type=rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 98
🏁 Script executed:
# Look at the create_mints.rs file context
cat -n sdk-libs/token-sdk/src/token/create_mints.rs | sed -n '530,560p'Repository: Lightprotocol/light-protocol
Length of output: 1291
🏁 Script executed:
# Check for CLAUDE.md or documentation
find . -name "CLAUDE.md" -o -name "*.md" | head -20Repository: Lightprotocol/light-protocol
Length of output: 649
🏁 Script executed:
# Find MintMetadata definition
rg -n "struct MintMetadata|enum MintMetadata" -A 10 | head -50Repository: Lightprotocol/light-protocol
Length of output: 1279
🏁 Script executed:
# Search for version constants or patterns
rg -n "const.*VERSION|version.*=" | grep -i mint | head -20Repository: Lightprotocol/light-protocol
Length of output: 2546
🏁 Script executed:
# Find where MintMetadata is defined/imported
rg -n "MintMetadata" sdk-libs/token-sdk/src/ | head -30Repository: Lightprotocol/light-protocol
Length of output: 636
🏁 Script executed:
# Read CLAUDE.md for guidance
cat ./CLAUDE.md | head -100Repository: Lightprotocol/light-protocol
Length of output: 5665
🏁 Script executed:
# Look for version constants or const definitions
rg -n "const.*VERSION|const.*MINT.*VERSION" -i | head -20Repository: Lightprotocol/light-protocol
Length of output: 2315
🏁 Script executed:
# Check the imports and dependencies in create_mints.rs
head -50 sdk-libs/token-sdk/src/token/create_mints.rsRepository: Lightprotocol/light-protocol
Length of output: 1878
🏁 Script executed:
# See all places where version = 3 is hardcoded
rg -n "version: 3" | head -20Repository: Lightprotocol/light-protocol
Length of output: 1176
🏁 Script executed:
# Check if there's version validation in token-interface
rg -n "version.*3|VERSION.*3" program-libs/token-interface/ | head -15Repository: Lightprotocol/light-protocol
Length of output: 1406
Consider defining a constant for the metadata version.
The MintMetadata version is hardcoded to 3 here and throughout the codebase. While version 3 is consistently validated as required by the program (see programs/compressed-token/program/src/compressed_token/mint_action/actions/create_mint.rs:106), extracting this to a constant like const MINT_METADATA_VERSION: u8 = 3 in a shared module would centralize this value and make future version migrations clearer. The MintMetadata struct itself has no associated constant, so this would be a small maintainability improvement.
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 542 - 556, Extract
the hardcoded MintMetadata version number into a shared constant (e.g., const
MINT_METADATA_VERSION: u8 = 3) and use it instead of the literal 3 wherever
MintMetadata is constructed or validated; update the construction in
create_mints.rs (the MintMetadata instantiation inside MintInstructionData) to
use MINT_METADATA_VERSION and replace other occurrences across the codebase
(including the create_mint validation in
compressed_token/mint_action/actions/create_mint.rs) to reference that constant
so version changes are centralized.
| pub fn create_mints<'a, 'info>( | ||
| payer: &AccountInfo<'info>, | ||
| accounts: &'info [AccountInfo<'info>], | ||
| params: CreateMintsParams<'a>, | ||
| ) -> Result<(), ProgramError> { | ||
| if params.mints.is_empty() { | ||
| return Err(ProgramError::InvalidArgument); | ||
| } | ||
|
|
||
| let n = params.mints.len(); | ||
| let mint_signers_start = 1; | ||
| let cpi_authority_idx = n + 1; | ||
| let registered_program_idx = n + 2; | ||
| let compression_authority_idx = n + 3; | ||
| let compression_program_idx = n + 4; | ||
| let system_program_idx = n + 5; | ||
| let cpi_context_idx = n + 6; | ||
| let output_queue_idx = n + 7; | ||
| let state_merkle_tree_idx = n + 8; | ||
| let address_tree_idx = n + 9; | ||
| let compressible_config_idx = n + 10; | ||
| let rent_sponsor_idx = n + 11; | ||
| let mint_pdas_start = n + 12; | ||
|
|
||
| // Build named struct from accounts slice | ||
| CreateMintsCpi { | ||
| mint_seed_accounts: &accounts[mint_signers_start..mint_signers_start + n], | ||
| payer: payer.clone(), | ||
| address_tree: accounts[address_tree_idx].clone(), | ||
| output_queue: accounts[output_queue_idx].clone(), | ||
| state_merkle_tree: accounts[state_merkle_tree_idx].clone(), | ||
| compressible_config: accounts[compressible_config_idx].clone(), | ||
| mints: &accounts[mint_pdas_start..mint_pdas_start + n], | ||
| rent_sponsor: accounts[rent_sponsor_idx].clone(), | ||
| system_accounts: SystemAccountInfos { | ||
| light_system_program: accounts[0].clone(), | ||
| cpi_authority_pda: accounts[cpi_authority_idx].clone(), | ||
| registered_program_pda: accounts[registered_program_idx].clone(), | ||
| account_compression_authority: accounts[compression_authority_idx].clone(), | ||
| account_compression_program: accounts[compression_program_idx].clone(), | ||
| system_program: accounts[system_program_idx].clone(), | ||
| }, | ||
| cpi_context_account: accounts[cpi_context_idx].clone(), | ||
| params, | ||
| } | ||
| .invoke() |
There was a problem hiding this comment.
Missing bounds check on accounts slice access.
The create_mints function accesses accounts by computed indices without verifying the slice has sufficient length. If accounts is too short, this will panic at runtime. Consider adding a bounds check:
🛡️ Suggested bounds check
pub fn create_mints<'a, 'info>(
payer: &AccountInfo<'info>,
accounts: &'info [AccountInfo<'info>],
params: CreateMintsParams<'a>,
) -> Result<(), ProgramError> {
if params.mints.is_empty() {
return Err(ProgramError::InvalidArgument);
}
let n = params.mints.len();
+ let required_len = 2 * n + 12; // mint_signers + system + trees + mint_pdas
+ if accounts.len() < required_len {
+ return Err(ProgramError::NotEnoughAccountKeys);
+ }
+
let mint_signers_start = 1;🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 589 - 634, The
function create_mints constructs many computed indices and slices the accounts
array without validating its length, which can panic; add a single bounds check
before any slicing that computes the required minimum length (e.g. compute n =
params.mints.len(), mint_pdas_start = n + 12, required_len = mint_pdas_start +
n) and if accounts.len() < required_len return an appropriate ProgramError (e.g.
InvalidArgument); perform this check before creating the CreateMintsCpi struct
so all subsequent uses of mint_signers_start, mint_pdas_start and the other
indexed variables are safe.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/macros/src/rentfree/accounts/builder.rs (1)
131-136: Inconsistent module paths forCpiAccountsConfig.In
generate_pre_init_pdas_and_mints(line 135):light_sdk_types::cpi_accounts::CpiAccountsConfig::new_with_cpi_context(...)In
generate_pre_init_mints_only(line 201):light_sdk::cpi::CpiAccountsConfig::new_with_cpi_context(...)These reference different module paths for seemingly the same type. This could cause confusion or compilation errors if the type isn't re-exported consistently.
🔧 Suggested fix for consistency
Pick one path and use it consistently. If
light_sdk_types::cpi_accounts::CpiAccountsConfigis the canonical location:- light_sdk::cpi::CpiAccountsConfig::new_with_cpi_context(crate::LIGHT_CPI_SIGNER), + light_sdk_types::cpi_accounts::CpiAccountsConfig::new_with_cpi_context(crate::LIGHT_CPI_SIGNER),Or if
light_sdk::cpire-exports it and is preferred:- light_sdk_types::cpi_accounts::CpiAccountsConfig::new_with_cpi_context(crate::LIGHT_CPI_SIGNER), + light_sdk::cpi::CpiAccountsConfig::new_with_cpi_context(crate::LIGHT_CPI_SIGNER),Also applies to: 196-202
sdk-libs/macros/docs/account/light_compressible.md (1)
9-9: Update source path reference after module reorganization.Line 9 references an outdated path: the file has been reorganized from
traits/toaccount/during the macro refactor. Update to:-**Source**: `sdk-libs/macros/src/rentfree/traits/light_compressible.rs` (lines 56-79) +**Source**: `sdk-libs/macros/src/rentfree/account/light_compressible.rs` (lines 56-79)The line numbers remain valid—they correctly point to the
derive_rentfree_accountfunction that implements theLightCompressiblederive macro.
🤖 Fix all issues with AI agents
In `@sdk-libs/compressible-client/src/pack.rs`:
- Around line 135-149: The comment "must be at index 1" is misleading—update the
comment near include_state_tree and pack_tree_infos to state that the address
tree must be packed before the state tree (i.e., ensure ordering) rather than
asserting a literal index, and mention that the actual index returned by
packed.insert_or_get depends on prior insertions (system accounts, output queue,
etc.); keep references to proof.pack_tree_infos, packed, packed.insert_or_get,
output_tree and state_tree_index so readers know which variables enforce the
ordering.
In `@sdk-libs/macros/docs/account/compressible_pack.md`:
- Around line 323-333: Add a table row documenting Option<Pubkey> handling and
show the generated PackedRecord example: update the Field Handling table to
include a row for `Option<Pubkey>` with "Direct copy (unchanged)" for both Pack
and Unpack, and add a brief example showing the input struct `Record { owner:
Pubkey, delegate: Option<Pubkey> }` alongside the generated `PackedRecord` to
illustrate that `owner` becomes a u8 index while `delegate` remains
`Option<Pubkey>`; reference the documented examples for consistency with naming
and formatting.
In `@sdk-libs/macros/src/rentfree/accounts/light_mint.rs`:
- Around line 331-333: The code currently unconditionally reads rent_payment and
write_top_up from mints[0] (see variables rent_payment and write_top_up) which
can silently hide differing values; update LightMintsBuilder (or the build
function that constructs light mints) to either validate that all entries in
mints have identical rent_payment and write_top_up and return an error/panic if
any differ, or explicitly document in LightMintsBuilder's struct docstring that
only the first mint's rent_payment and write_top_up are used; implement the
former by iterating over mints and comparing each mint.rent_payment and
mint.write_top_up to mints[0], failing fast with a clear message referencing
rent_payment/write_top_up when mismatched.
In `@sdk-libs/macros/src/rentfree/program/decompress.rs`:
- Around line 274-279: The current explicit allocation using
Vec::with_capacity(1) and push to construct a single-element vector for
seeds_vec is correct but verbose; replace the block that creates bump_vec and
pushes bump with a simpler seeds_vec.push(vec![bump]) to achieve the same
efficient single-element allocation while reducing verbosity, or keep the
existing code if you prefer consistency with other allocation patterns (look for
the seeds_vec variable and the bump value in decompressed.rs to apply this
change).
In `@sdk-libs/token-sdk/src/token/create_mints.rs`:
- Around line 637-738: The large commented-out CreateMints client-side builder
(struct CreateMints, impl new, and build_account_metas referencing
CreateMintsParams, build_account_metas, SystemAccounts, etc.) should be removed
or tracked: either delete the commented block from the file, or create/link a
tracking issue and replace the block with a short comment referencing that issue
and why the code is preserved for future work; ensure references to CreateMints,
CreateMintsParams, and build_account_metas are mentioned in the issue so
reviewers can find the rationale.
- Around line 360-371: The signer-seed packing logic in create_mints.rs is
duplicated; extract it into a small helper function (e.g., fn
pack_signer_seeds<'a>(mint_params: &'a SingleMintParams<'a>) -> ([&'a [&'a
[u8]]; 2], usize)) that builds the seeds array and returns (seeds, num_signers),
then replace the in-place blocks in the functions using
mint_params.mint_signer_seeds and mint_params.authority_seeds (the call sites
around the solana_cpi::invoke_signed invocations) with a call to this helper and
pass the returned slice &seeds[..num_signers] into invoke_signed. Ensure the
helper lives in the same module and uses the same array size (2) as the current
code so behavior is unchanged.
- Around line 440-441: The assignment to
MintActionCompressedInstructionData.leaf_index uses base_leaf_index + index as
u32 which can overflow; change the computation to use checked arithmetic (e.g.,
base_leaf_index.checked_add(index as u32) or index.checked_add(base_leaf_index)
after proper casting) and propagate a clear error (or return Result::Err) when
None is returned by checked_add; update the code paths that construct
MintActionCompressedInstructionData (refer to variable names base_leaf_index,
index and the struct MintActionCompressedInstructionData in create_mints.rs) to
handle the error instead of allowing wrapping.
- Around line 306-316: The assigned_account_index in the CpiContext construction
can overflow because it does offset + index as u8; update the code in the
CpiContext creation (and the analogous spot in invoke_execute) to perform
checked arithmetic: convert index to a wider integer (e.g., usize or u16), add
offset, verify the sum fits into u8 (using checked_add or try_from) and return
an error if it doesn't; then assign the validated u8 value to
assigned_account_index to prevent silent wrapping.
- Around line 68-70: The rent_payment field's documented constraint ("must be 0
or >= 2") is not enforced; add validation in the CreateMints::validate() method
(or a dedicated setter) to reject values equal to 1 (and any other invalid u8)
by returning an appropriate error/result; reference the rent_payment field on
the CreateMints struct and update any callers to propagate the validation error
so invalid values cannot pass through.
♻️ Duplicate comments (2)
sdk-libs/macros/src/rentfree/program/variant_enum.rs (1)
19-23: State-field awareness enables safer seed validation.Adding
state_field_namesandparams_only_seed_fieldstoPdaCtxSeedInfoallows the codegen to distinguish between:
- Fields that exist on the state struct (can be derived from unpacked data)
- Params-only fields (must be passed explicitly and stored in the variant)
This is a good defensive measure that prevents misconfigurations at compile time.
As noted in a previous review, consider adding
use std::collections::HashSet;at the top of the file to match the project's style conventions rather than using the fully-qualified path.sdk-libs/macros/src/rentfree/account/decompress_context.rs (1)
152-152: Logging provides observability but consider compute cost trade-offs.These
solana_msg::msg!calls add useful debugging information during decompression. However, as noted in a previous review, each log call consumes compute units (~100 CU for simple logs, more for formatted output).The per-iteration logs (lines 164, 172, 175) scale linearly with account count. For typical small batches this is fine, but for larger batches it compounds.
Consider:
- Keeping entry/exit logs (152, 183) for high-level observability
- Removing or conditionally enabling per-iteration logs if CU becomes a concern
Also applies to: 158-158, 162-165, 172-172, 175-175, 183-183
| // For mint creation: pack address tree first (must be at index 1 per program validation), | ||
| // then state tree. For non-mint: just pack tree infos normally. | ||
| let (client_packed_tree_infos, state_tree_index) = if include_state_tree { | ||
| // Pack tree infos first to ensure address tree is at index 1 | ||
| let tree_infos = proof.pack_tree_infos(&mut packed); | ||
|
|
||
| // Then add state tree (will be after address tree) | ||
| let state_tree = output_tree | ||
| .next_tree_info | ||
| .as_ref() | ||
| .map(|n| n.tree) | ||
| .unwrap_or(output_tree.tree); | ||
| let state_idx = packed.insert_or_get(state_tree); | ||
|
|
||
| (tree_infos, Some(state_idx)) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the "index 1" comment—it's about ordering, not a literal index.
The comment says "address tree must be at index 1" but the actual invariant being enforced is that address trees are packed before the state tree. The actual index depends on prior insertions (system accounts, output queue, etc.).
Consider rewording to avoid confusion:
- // For mint creation: pack address tree first (must be at index 1 per program validation),
- // then state tree. For non-mint: just pack tree infos normally.
+ // For mint creation: pack address trees before the state tree to ensure deterministic
+ // ordering in remaining accounts. For non-mint: just pack tree infos normally.🤖 Prompt for AI Agents
In `@sdk-libs/compressible-client/src/pack.rs` around lines 135 - 149, The comment
"must be at index 1" is misleading—update the comment near include_state_tree
and pack_tree_infos to state that the address tree must be packed before the
state tree (i.e., ensure ordering) rather than asserting a literal index, and
mention that the actual index returned by packed.insert_or_get depends on prior
insertions (system accounts, output queue, etc.); keep references to
proof.pack_tree_infos, packed, packed.insert_or_get, output_tree and
state_tree_index so readers know which variables enforce the ordering.
| ### Limitation: Option<Pubkey> Fields | ||
|
|
||
| Only direct `Pubkey` fields are converted to `u8` indices. `Option<Pubkey>` fields remain as `Option<Pubkey>` in the packed struct because `None` doesn't map cleanly to an index. | ||
|
|
||
| ```rust | ||
| pub struct Record { | ||
| pub owner: Pubkey, // -> u8 in packed struct | ||
| pub delegate: Option<Pubkey>, // -> Option<Pubkey> in packed struct (unchanged) | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find relevant documentation and implementation files
echo "=== Finding compressible_pack.md ==="
find . -name "compressible_pack.md" -type f
echo "=== Finding pack_unpack.rs ==="
find . -name "pack_unpack.rs" -type f
echo "=== Finding CLAUDE.md files in macros ==="
find sdk-libs/macros -name "CLAUDE.md" -type fRepository: Lightprotocol/light-protocol
Length of output: 344
🏁 Script executed:
#!/bin/bash
# Read the documentation file to see the context and Field Handling table
echo "=== compressible_pack.md context (lines 280-340) ==="
sed -n '280,340p' ./sdk-libs/macros/docs/account/compressible_pack.md
echo ""
echo "=== File length and structure ==="
wc -l ./sdk-libs/macros/docs/account/compressible_pack.mdRepository: Lightprotocol/light-protocol
Length of output: 2023
🏁 Script executed:
#!/bin/bash
# Read the pack_unpack.rs implementation to verify Option<Pubkey> handling
echo "=== pack_unpack.rs full contents ==="
wc -l ./sdk-libs/macros/src/rentfree/account/pack_unpack.rs
echo ""
echo "=== Searching for Option handling ==="
rg -n "Option" ./sdk-libs/macros/src/rentfree/account/pack_unpack.rs
echo ""
echo "=== Searching for Pubkey handling ==="
rg -n -A3 -B3 "Pubkey" ./sdk-libs/macros/src/rentfree/account/pack_unpack.rs | head -100Repository: Lightprotocol/light-protocol
Length of output: 244
🏁 Script executed:
#!/bin/bash
# Read the actual implementation file
echo "=== pack_unpack.rs full contents ==="
cat -n ./sdk-libs/macros/src/rentfree/account/pack_unpack.rs
echo ""
echo "=== Check for pack/unpack method implementations ==="
rg -n "fn pack|fn unpack" ./sdk-libs/macros/src/rentfree/account/pack_unpack.rsRepository: Lightprotocol/light-protocol
Length of output: 7834
🏁 Script executed:
#!/bin/bash
# Now check the is_pubkey_type function to see how it handles Option<Pubkey>
echo "=== Finding utils.rs ==="
find ./sdk-libs/macros/src/rentfree/account -name "utils.rs" -type f
echo ""
echo "=== Check utils.rs for is_pubkey_type implementation ==="
cat -n ./sdk-libs/macros/src/rentfree/account/utils.rsRepository: Lightprotocol/light-protocol
Length of output: 4769
🏁 Script executed:
#!/bin/bash
# Verify the logic by checking what happens with a struct containing Option<Pubkey>
echo "=== Checking if there are any test cases or examples with Option<Pubkey> ==="
rg -n "Option.*Pubkey\|Option<Pubkey>" ./sdk-libs/macros/tests/ --type rust
echo ""
echo "=== Check CLAUDE.md for guidance on macro patterns ==="
cat -n ./sdk-libs/macros/CLAUDE.md | head -50Repository: Lightprotocol/light-protocol
Length of output: 2421
🏁 Script executed:
#!/bin/bash
# Let me verify the logic flow one more time to be absolutely certain
echo "=== Simulating what happens with Option<Pubkey> ==="
echo "1. is_pubkey_type for Option<Pubkey>:"
echo " - Checks if last path segment is 'Pubkey'"
echo " - Option<Pubkey> has last segment 'Pubkey'? NO, it's the outer Option"
echo " - Result: FALSE"
echo ""
echo "2. is_copy_type for Option<Pubkey>:"
echo " - Checks if type_name == 'Option'"
echo " - has_copy_inner_type checks inner type (Pubkey)"
echo " - Pubkey is in copy list (line 67)"
echo " - Result: TRUE (because Option<Pubkey> inner is copy)"
echo ""
echo "3. Packed field generation (line 32-35):"
echo " - is_pubkey_type = FALSE"
echo " - packed_type = original type = Option<Pubkey>"
echo ""
echo "4. Pack assignment (line 54-62):"
echo " - is_pubkey_type = FALSE"
echo " - is_copy_type = TRUE"
echo " - Uses: self.#field_name (direct copy, line 59)"
echo ""
echo "5. Unpack assignment (line 109-118):"
echo " - is_pubkey_type = FALSE"
echo " - is_copy_type = TRUE"
echo " - Uses: self.#field_name (direct copy, line 114)"Repository: Lightprotocol/light-protocol
Length of output: 920
Add Option<Pubkey> row to the Field Handling table for documentation completeness.
The new limitation section correctly documents that Option<Pubkey> fields are not converted to indices and remain unchanged. However, the Field Handling table (lines 284-289) omits this case, leaving the documentation incomplete. A single-row addition clarifies the behavior:
| Field Type | Pack Behavior | Unpack Behavior |
|---|---|---|
Option<Pubkey> |
Direct copy (unchanged) | Direct copy (unchanged) |
This brings the table in line with the documented behavior where Option<Pubkey> fields bypass Pubkey-to-index compression.
The example would also benefit from showing the generated PackedRecord struct alongside the input struct, similar to other examples in the documentation, for users to see the transformation clearly.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/docs/account/compressible_pack.md` around lines 323 - 333,
Add a table row documenting Option<Pubkey> handling and show the generated
PackedRecord example: update the Field Handling table to include a row for
`Option<Pubkey>` with "Direct copy (unchanged)" for both Pack and Unpack, and
add a brief example showing the input struct `Record { owner: Pubkey, delegate:
Option<Pubkey> }` alongside the generated `PackedRecord` to illustrate that
`owner` becomes a u8 index while `delegate` remains `Option<Pubkey>`; reference
the documented examples for consistency with naming and formatting.
| // Get rent_payment and write_top_up from first mint (all mints share same params for now) | ||
| let rent_payment = quote_option_or(&mints[0].rent_payment, quote! { 16u8 }); | ||
| let write_top_up = quote_option_or(&mints[0].write_top_up, quote! { 766u32 }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify that all mints truly share the same rent_payment and write_top_up values.
The code takes rent_payment and write_top_up from mints[0] with the comment "all mints share same params for now". If different mints can have different values in the future, this would silently use only the first mint's values. The TODO on line 394 acknowledges this for write_top_up.
Consider either:
- Validating that all mints have matching values (fail-fast if they differ)
- Or accepting this limitation and documenting it in the
LightMintsBuilderstruct's doc comment
♻️ Option: Add validation that all mints have matching params
+ // Validate all mints have matching rent_payment and write_top_up
+ for (i, mint) in mints.iter().enumerate().skip(1) {
+ if mint.rent_payment != mints[0].rent_payment {
+ // Log warning or return error depending on desired behavior
+ }
+ if mint.write_top_up != mints[0].write_top_up {
+ // Log warning or return error depending on desired behavior
+ }
+ }
+
// Get rent_payment and write_top_up from first mint (all mints share same params for now)
let rent_payment = quote_option_or(&mints[0].rent_payment, quote! { 16u8 });
let write_top_up = quote_option_or(&mints[0].write_top_up, quote! { 766u32 });🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/accounts/light_mint.rs` around lines 331 - 333,
The code currently unconditionally reads rent_payment and write_top_up from
mints[0] (see variables rent_payment and write_top_up) which can silently hide
differing values; update LightMintsBuilder (or the build function that
constructs light mints) to either validate that all entries in mints have
identical rent_payment and write_top_up and return an error/panic if any differ,
or explicitly document in LightMintsBuilder's struct docstring that only the
first mint's rent_payment and write_top_up are used; implement the former by
iterating over mints and comparing each mint.rent_payment and mint.write_top_up
to mints[0], failing fast with a clear message referencing
rent_payment/write_top_up when mismatched.
| // Avoid vec![bump] macro which expands to box_new allocation | ||
| { | ||
| let mut bump_vec = Vec::with_capacity(1); | ||
| bump_vec.push(bump); | ||
| seeds_vec.push(bump_vec); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Allocation optimization is correct but consider simplifying.
The refactor from vec![bump] to explicit Vec::with_capacity(1) + push avoids potential box allocation from the vec! macro. However, this is micro-optimization that adds verbosity.
An alternative that's equally efficient:
seeds_vec.push(vec![bump]);The vec![single_value] macro doesn't box for a single known-size element. But the current approach is fine if consistency with other allocation patterns in the codebase is preferred.
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/rentfree/program/decompress.rs` around lines 274 - 279,
The current explicit allocation using Vec::with_capacity(1) and push to
construct a single-element vector for seeds_vec is correct but verbose; replace
the block that creates bump_vec and pushes bump with a simpler
seeds_vec.push(vec![bump]) to achieve the same efficient single-element
allocation while reducing verbosity, or keep the existing code if you prefer
consistency with other allocation patterns (look for the seeds_vec variable and
the bump value in decompressed.rs to apply this change).
| // Collect all unique params-only seed fields across all variants for SeedParams struct | ||
| // Use BTreeMap for deterministic ordering | ||
| let mut all_params_only_fields: std::collections::BTreeMap<String, syn::Type> = | ||
| std::collections::BTreeMap::new(); | ||
| for ctx_info in &pda_ctx_seeds { | ||
| for (field_name, field_type, _) in &ctx_info.params_only_seed_fields { | ||
| let field_str = field_name.to_string(); | ||
| all_params_only_fields | ||
| .entry(field_str) | ||
| .or_insert_with(|| field_type.clone()); | ||
| } | ||
| } | ||
|
|
||
| let seed_params_struct = if all_params_only_fields.is_empty() { | ||
| quote! { | ||
| #[derive(anchor_lang::AnchorSerialize, anchor_lang::AnchorDeserialize, Clone, Debug, Default)] | ||
| pub struct SeedParams; | ||
| } | ||
| } else { | ||
| // Collect into Vec for consistent ordering between field declarations and Default impl | ||
| let sorted_fields: Vec<_> = all_params_only_fields.iter().collect(); | ||
| let seed_param_fields: Vec<_> = sorted_fields | ||
| .iter() | ||
| .map(|(name, ty)| { | ||
| let field_ident = format_ident!("{}", name); | ||
| quote! { pub #field_ident: Option<#ty> } | ||
| }) | ||
| .collect(); | ||
| let seed_param_defaults: Vec<_> = sorted_fields | ||
| .iter() | ||
| .map(|(name, _)| { | ||
| let field_ident = format_ident!("{}", name); | ||
| quote! { #field_ident: None } | ||
| }) | ||
| .collect(); | ||
| quote! { | ||
| #[derive(anchor_lang::AnchorSerialize, anchor_lang::AnchorDeserialize, Clone, Debug)] | ||
| pub struct SeedParams { | ||
| #(#seed_param_fields,)* | ||
| } | ||
| impl Default for SeedParams { | ||
| fn default() -> Self { | ||
| Self { | ||
| #(#seed_param_defaults,)* | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of params_only_seed_fields and related validation
rg "params_only_seed_fields" -t rust -B 5 -A 10Repository: Lightprotocol/light-protocol
Length of output: 20389
🏁 Script executed:
#!/bin/bash
# Check for type validation or conflict detection logic
rg "pda_ctx_seeds|SeedParams" -t rust -B 3 -A 8 | head -200Repository: Lightprotocol/light-protocol
Length of output: 20250
🏁 Script executed:
#!/bin/bash
# Find the struct definition of pda_ctx_seeds to understand its structure
ast-grep --pattern 'struct $_ {
$$$
params_only_seed_fields: $_,
$$$
}'Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Look for any documentation or CLAUDE.md files in the macros directory
find sdk-libs/macros -name "*.md" -type fRepository: Lightprotocol/light-protocol
Length of output: 660
🏁 Script executed:
#!/bin/bash
# Read the CLAUDE.md file for macros
cat -n sdk-libs/macros/CLAUDE.mdRepository: Lightprotocol/light-protocol
Length of output: 3643
🏁 Script executed:
#!/bin/bash
# Search for actual usage of these macros to see if multiple variants with same-named fields exist
rg "#\[rentfree_program\]|pub struct.*PdaCtxSeedInfo|struct.*SeedParams" -A 20 -B 5 | head -200Repository: Lightprotocol/light-protocol
Length of output: 16119
🏁 Script executed:
#!/bin/bash
# Look for test cases or examples that might show multiple variants
fd "\.rs$" sdk-libs/macros --exec grep -l "params_only_seed_fields\|SeedParams" {} \; | head -10Repository: Lightprotocol/light-protocol
Length of output: 343
🏁 Script executed:
#!/bin/bash
# Check the seed extraction logic to understand how params-only fields are determined
cat -n sdk-libs/macros/src/rentfree/account/seed_extraction.rs | head -100Repository: Lightprotocol/light-protocol
Length of output: 4276
🏁 Script executed:
#!/bin/bash
# Read the full seed extraction implementation
cat -n sdk-libs/macros/src/rentfree/account/seed_extraction.rs | tail -150Repository: Lightprotocol/light-protocol
Length of output: 6718
🏁 Script executed:
#!/bin/bash
# Search for type validation or mismatch checking logic
rg "type.*mismatch|Type.*conflict|compatible|inconsistent.*type" -t rust sdk-libs/macros/Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Look for documentation on variant field consistency requirements
find sdk-libs/macros/docs -name "*.md" -exec grep -l "variant.*field\|field.*type\|consistent" {} \;Repository: Lightprotocol/light-protocol
Length of output: 413
🏁 Script executed:
#!/bin/bash
# Check the codegen documentation for variant handling
cat -n sdk-libs/macros/docs/rentfree_program/codegen.md | head -200Repository: Lightprotocol/light-protocol
Length of output: 7717
🏁 Script executed:
#!/bin/bash
# Look at the exact type assignment in seed extraction (lines 658-664)
sed -n '658,664p' sdk-libs/macros/src/rentfree/account/seed_extraction.rsRepository: Lightprotocol/light-protocol
Length of output: 396
🏁 Script executed:
#!/bin/bash
# Search for any validation that checks conversion consistency across variants
rg "has_conversion|conversion.*mismatch|different.*conversion" -t rust sdk-libs/macros/Repository: Lightprotocol/light-protocol
Length of output: 1137
🏁 Script executed:
#!/bin/bash
# Check if there are tests that cover multiple variants with same field names
rg "params_only\|SeedParams" sdk-tests/csdk-anchor-full-derived-test -A 5 -B 5 | grep -A 20 "variant\|Variant" | head -100Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Look for any comments or documentation about field type consistency
rg "consistent|same.*name|same.*field" -t rust sdk-libs/macros/src/rentfree/program/ -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 1142
🏁 Script executed:
#!/bin/bash
# Check the decompress.rs implementation to see how conversion is handled
cat -n sdk-libs/macros/src/rentfree/program/decompress.rs | grep -A 30 "params_only_has_conversion"Repository: Lightprotocol/light-protocol
Length of output: 3562
🏁 Script executed:
#!/bin/bash
# Look for any validation error messages related to field type or conversion conflicts
rg "conversion\|mismatch\|conflict\|inconsistent" -t rust sdk-libs/macros/src/rentfree/program/ -B 3 -A 3Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if there's a semantic constraint documented about same-named fields needing same conversion
cat -n sdk-libs/macros/docs/rentfree_program/architecture.md | head -250Repository: Lightprotocol/light-protocol
Length of output: 10937
🏁 Script executed:
#!/bin/bash
# Look for test cases with multiple variants having same-named params fields
find sdk-tests -name "*.rs" -exec grep -l "params\." {} \; | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A 20 "#\[account(seeds" {} | head -40'Repository: Lightprotocol/light-protocol
Length of output: 1372
🏁 Script executed:
#!/bin/bash
# Search for cases where same field name appears in multiple instruction structs
rg "pub struct.*\(.*info\)" sdk-tests/csdk-anchor-full-derived-test/src/instructions -A 30 | grep -E "pub struct|seeds|params\." | head -100Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if there's any runtime validation or semantic check preventing mixed conversions
rg "Err\(|panic|assert|todo|unimplemented" sdk-libs/macros/src/rentfree/program/instructions.rs | grep -A 2 -B 2 "params\|conversion\|type"Repository: Lightprotocol/light-protocol
Length of output: 54
Add type consistency validation when aggregating params-only seed fields across variants.
The concern is valid. When get_params_only_seed_fields_from_spec() extracts a field like params.id, it derives the type from whether a conversion method (like to_le_bytes()) is applied:
params.id→Pubkey(no conversion)params.id.to_le_bytes()→u64(with conversion)
If two variants define the same field name with different conversion patterns, the aggregation at lines 128-131 silently keeps only the first type via or_insert_with(). Later, the decompress code (lines 173-176) builds a single HashMap of conversion flags per field name, so it also cannot handle conflicting conversion patterns.
This creates a latent type mismatch: the generated SeedParams struct has one type for a field, but at runtime, depending on which variant's conversion flag gets used, the code might expect a different type. The code lacks validation to ensure same-named params fields have consistent conversion status across all variants, and the documentation does not document this as a constraint.
Consider adding a validation pass after collecting all_params_only_fields to check that if a field appears in multiple pda_ctx_seeds, the conversion flag (has_conversion) is consistent across all occurrences.
| /// Rent payment in epochs for the Mint account (must be 0 or >= 2). | ||
| /// Default: 16 (~24 hours) | ||
| pub rent_payment: u8, |
There was a problem hiding this comment.
Missing validation for documented constraint.
The doc comment states rent_payment must be 0 or >= 2, but this isn't enforced anywhere. Invalid values (like 1) would pass through unchecked and potentially cause issues downstream.
Consider adding validation in validate() or a dedicated setter:
Proposed fix
impl<'a, 'info> CreateMintsCpi<'a, 'info> {
/// Validate that the struct is properly constructed.
pub fn validate(&self) -> Result<(), ProgramError> {
let n = self.params.mints.len();
if n == 0 {
return Err(ProgramError::InvalidArgument);
}
if self.mint_seed_accounts.len() != n {
return Err(ProgramError::InvalidArgument);
}
if self.mints.len() != n {
return Err(ProgramError::InvalidArgument);
}
+ // rent_payment must be 0 or >= 2 per protocol requirements
+ if self.params.rent_payment == 1 {
+ return Err(ProgramError::InvalidArgument);
+ }
Ok(())
}🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 68 - 70, The
rent_payment field's documented constraint ("must be 0 or >= 2") is not
enforced; add validation in the CreateMints::validate() method (or a dedicated
setter) to reject values equal to 1 (and any other invalid u8) by returning an
appropriate error/result; reference the rent_payment field on the CreateMints
struct and update any callers to propagate the validation error so invalid
values cannot pass through.
| let cpi_context = CpiContext { | ||
| set_context: index > 0 || offset > 0, | ||
| first_set_context: index == 0 && offset == 0, | ||
| in_tree_index: self.params.address_tree_index, | ||
| in_queue_index: self.params.output_queue_index, | ||
| out_queue_index: self.params.output_queue_index, | ||
| token_out_queue_index: 0, | ||
| assigned_account_index: offset + index as u8, | ||
| read_only_address_trees: [0; 4], | ||
| address_tree_pubkey: self.address_tree.key.to_bytes(), | ||
| }; |
There was a problem hiding this comment.
Potential integer overflow in assigned_account_index calculation.
When offset + index exceeds 255, this wraps silently due to u8 arithmetic. With a large cpi_context_offset and many mints, this could corrupt the CPI context indexing.
Proposed fix using checked arithmetic
- assigned_account_index: offset + index as u8,
+ assigned_account_index: offset
+ .checked_add(index as u8)
+ .ok_or(ProgramError::ArithmeticOverflow)?,The same pattern appears at line 392 in invoke_execute. Both should use checked arithmetic.
📝 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.
| let cpi_context = CpiContext { | |
| set_context: index > 0 || offset > 0, | |
| first_set_context: index == 0 && offset == 0, | |
| in_tree_index: self.params.address_tree_index, | |
| in_queue_index: self.params.output_queue_index, | |
| out_queue_index: self.params.output_queue_index, | |
| token_out_queue_index: 0, | |
| assigned_account_index: offset + index as u8, | |
| read_only_address_trees: [0; 4], | |
| address_tree_pubkey: self.address_tree.key.to_bytes(), | |
| }; | |
| let assigned_account_index = offset | |
| .checked_add(index as u8) | |
| .ok_or(ProgramError::ArithmeticOverflow)?; | |
| let cpi_context = CpiContext { | |
| set_context: index > 0 || offset > 0, | |
| first_set_context: index == 0 && offset == 0, | |
| in_tree_index: self.params.address_tree_index, | |
| in_queue_index: self.params.output_queue_index, | |
| out_queue_index: self.params.output_queue_index, | |
| token_out_queue_index: 0, | |
| assigned_account_index, | |
| read_only_address_trees: [0; 4], | |
| address_tree_pubkey: self.address_tree.key.to_bytes(), | |
| }; |
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 306 - 316, The
assigned_account_index in the CpiContext construction can overflow because it
does offset + index as u8; update the code in the CpiContext creation (and the
analogous spot in invoke_execute) to perform checked arithmetic: convert index
to a wider integer (e.g., usize or u16), add offset, verify the sum fits into u8
(using checked_add or try_from) and return an error if it doesn't; then assign
the validated u8 value to assigned_account_index to prevent silent wrapping.
| // Build signer seeds - pack present seeds at start of array | ||
| let mut seeds: [&[&[u8]]; 2] = [&[], &[]]; | ||
| let mut num_signers = 0; | ||
| if let Some(s) = mint_params.mint_signer_seeds { | ||
| seeds[num_signers] = s; | ||
| num_signers += 1; | ||
| } | ||
| if let Some(s) = mint_params.authority_seeds { | ||
| seeds[num_signers] = s; | ||
| num_signers += 1; | ||
| } | ||
| solana_cpi::invoke_signed(&instruction, &account_infos, &seeds[..num_signers]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting duplicated signer seed packing logic.
The pattern for building signer seeds appears three times:
let mut seeds: [&[&[u8]]; 2] = [&[], &[]];
let mut num_signers = 0;
if let Some(s) = mint_params.mint_signer_seeds {
seeds[num_signers] = s;
num_signers += 1;
}
if let Some(s) = mint_params.authority_seeds {
seeds[num_signers] = s;
num_signers += 1;
}Extracting to a helper would reduce duplication and make future changes easier:
Suggested helper
fn pack_signer_seeds<'a>(
mint_params: &'a SingleMintParams<'a>,
) -> ([&'a [&'a [u8]]; 2], usize) {
let mut seeds: [&[&[u8]]; 2] = [&[], &[]];
let mut num_signers = 0;
if let Some(s) = mint_params.mint_signer_seeds {
seeds[num_signers] = s;
num_signers += 1;
}
if let Some(s) = mint_params.authority_seeds {
seeds[num_signers] = s;
num_signers += 1;
}
(seeds, num_signers)
}Also applies to: 520-531
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 360 - 371, The
signer-seed packing logic in create_mints.rs is duplicated; extract it into a
small helper function (e.g., fn pack_signer_seeds<'a>(mint_params: &'a
SingleMintParams<'a>) -> ([&'a [&'a [u8]]; 2], usize)) that builds the seeds
array and returns (seeds, num_signers), then replace the in-place blocks in the
functions using mint_params.mint_signer_seeds and mint_params.authority_seeds
(the call sites around the solana_cpi::invoke_signed invocations) with a call to
this helper and pass the returned slice &seeds[..num_signers] into
invoke_signed. Ensure the helper lives in the same module and uses the same
array size (2) as the current code so behavior is unchanged.
| let instruction_data = MintActionCompressedInstructionData { | ||
| leaf_index: base_leaf_index + index as u32, |
There was a problem hiding this comment.
Potential integer overflow in leaf_index calculation.
If base_leaf_index is near u32::MAX, adding index will wrap. While unlikely in practice, defensive programming would use checked arithmetic.
Proposed fix
- leaf_index: base_leaf_index + index as u32,
+ leaf_index: base_leaf_index
+ .checked_add(index as u32)
+ .ok_or(ProgramError::ArithmeticOverflow)?,🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 440 - 441, The
assignment to MintActionCompressedInstructionData.leaf_index uses
base_leaf_index + index as u32 which can overflow; change the computation to use
checked arithmetic (e.g., base_leaf_index.checked_add(index as u32) or
index.checked_add(base_leaf_index) after proper casting) and propagate a clear
error (or return Result::Err) when None is returned by checked_add; update the
code paths that construct MintActionCompressedInstructionData (refer to variable
names base_leaf_index, index and the struct MintActionCompressedInstructionData
in create_mints.rs) to handle the error instead of allowing wrapping.
| // // ============================================================================ | ||
| // // Client-side instruction builder | ||
| // // ============================================================================ | ||
|
|
||
| // /// Client-side instruction builder for creating multiple mints. | ||
| // /// | ||
| // /// This struct is used to build instructions for client-side transaction construction. | ||
| // /// For CPI usage within Solana programs, use [`CreateMintsCpi`] instead. | ||
| // /// | ||
| // /// # Example | ||
| // /// | ||
| // /// ```rust,ignore | ||
| // /// use light_token_sdk::token::{CreateMints, CreateMintsParams, SingleMintParams}; | ||
| // /// | ||
| // /// let params = CreateMintsParams::new(vec![mint1_params, mint2_params], proof); | ||
| // /// | ||
| // /// let instructions = CreateMints::new( | ||
| // /// params, | ||
| // /// mint_seed_pubkeys, | ||
| // /// payer, | ||
| // /// address_tree_pubkey, | ||
| // /// output_queue, | ||
| // /// state_merkle_tree, | ||
| // /// cpi_context_pubkey, | ||
| // /// ).instructions()?; | ||
| // /// ``` | ||
| // #[derive(Debug, Clone)] | ||
| // pub struct CreateMints<'a> { | ||
| // pub payer: Pubkey, | ||
| // pub address_tree_pubkey: Pubkey, | ||
| // pub output_queue: Pubkey, | ||
| // pub state_merkle_tree: Pubkey, | ||
| // pub cpi_context_pubkey: Pubkey, | ||
| // pub params: CreateMintsParams<'a>, | ||
| // } | ||
|
|
||
| // impl<'a> CreateMints<'a> { | ||
| // #[allow(clippy::too_many_arguments)] | ||
| // pub fn new( | ||
| // params: CreateMintsParams<'a>, | ||
| // payer: Pubkey, | ||
| // address_tree_pubkey: Pubkey, | ||
| // output_queue: Pubkey, | ||
| // state_merkle_tree: Pubkey, | ||
| // cpi_context_pubkey: Pubkey, | ||
| // ) -> Self { | ||
| // Self { | ||
| // payer, | ||
| // address_tree_pubkey, | ||
| // output_queue, | ||
| // state_merkle_tree, | ||
| // cpi_context_pubkey, | ||
| // params, | ||
| // } | ||
| // } | ||
|
|
||
| // /// Build account metas for the instruction. | ||
| // pub fn build_account_metas(&self) -> Vec<AccountMeta> { | ||
| // let system_accounts = SystemAccounts::default(); | ||
|
|
||
| // let mut accounts = vec![AccountMeta::new_readonly( | ||
| // system_accounts.light_system_program, | ||
| // false, | ||
| // )]; | ||
|
|
||
| // // Add mint signers (from each SingleMintParams) | ||
| // for mint_params in self.params.mints { | ||
| // accounts.push(AccountMeta::new_readonly( | ||
| // mint_params.mint_seed_pubkey, | ||
| // true, | ||
| // )); | ||
| // } | ||
|
|
||
| // // Add system PDAs | ||
| // accounts.extend(vec![ | ||
| // AccountMeta::new_readonly(system_accounts.cpi_authority_pda, false), | ||
| // AccountMeta::new_readonly(system_accounts.registered_program_pda, false), | ||
| // AccountMeta::new_readonly(system_accounts.account_compression_authority, false), | ||
| // AccountMeta::new_readonly(system_accounts.account_compression_program, false), | ||
| // AccountMeta::new_readonly(system_accounts.system_program, false), | ||
| // ]); | ||
|
|
||
| // // CPI context, output queue, address tree | ||
| // accounts.push(AccountMeta::new(self.cpi_context_pubkey, false)); | ||
| // accounts.push(AccountMeta::new(self.output_queue, false)); | ||
| // accounts.push(AccountMeta::new(self.address_tree_pubkey, false)); | ||
|
|
||
| // // Config, rent sponsor | ||
| // accounts.push(AccountMeta::new_readonly(config_pda(), false)); | ||
| // accounts.push(AccountMeta::new(rent_sponsor_pda(), false)); | ||
|
|
||
| // // State merkle tree | ||
| // accounts.push(AccountMeta::new(self.state_merkle_tree, false)); | ||
|
|
||
| // // Add mint PDAs | ||
| // for mint_params in self.params.mints { | ||
| // accounts.push(AccountMeta::new(mint_params.mint, false)); | ||
| // } | ||
|
|
||
| // accounts | ||
| // } | ||
| // } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove or track commented-out code.
This ~100-line commented block for CreateMints client-side builder adds maintenance burden. If it's planned work, track it in an issue and remove the code. If it's needed for reference, a brief comment explaining why it's preserved would help future readers.
🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/token/create_mints.rs` around lines 637 - 738, The
large commented-out CreateMints client-side builder (struct CreateMints, impl
new, and build_account_metas referencing CreateMintsParams, build_account_metas,
SystemAccounts, etc.) should be removed or tracked: either delete the commented
block from the file, or create/link a tracking issue and replace the block with
a short comment referencing that issue and why the code is preserved for future
work; ensure references to CreateMints, CreateMintsParams, and
build_account_metas are mentioned in the issue so reviewers can find the
rationale.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.