refactor: light account creation to generic function#2287
Merged
SwenSchaeferjohann merged 10 commits intomainfrom Feb 19, 2026
Merged
refactor: light account creation to generic function#2287SwenSchaeferjohann merged 10 commits intomainfrom
SwenSchaeferjohann merged 10 commits intomainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new feature-gated SDK API Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Macro as Macro (codegen)
participant SDK as SDK::create_accounts
participant CPI as CPI Context / Programs
participant Storage as On-chain Accounts
rect rgba(100,149,237,0.5)
Macro->>SDK: assemble PdaInitParam[], CreateMintsInput, TokenInitParam[], AtaInitParam[] + SharedAccounts
end
rect rgba(34,139,34,0.5)
SDK->>CPI: perform PDAs / mints / token vaults / ATAs creation (CPI calls)
CPI->>Storage: create/initialize accounts (system/token/metadata/rent)
CPI-->>SDK: return results/errors
end
SDK-->>Macro: return success flag (CPI used?) or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sdk-libs/account/src/lib.rs`:
- Around line 226-230: The re-export of
light_sdk_types::interface::accounts::create_accounts is currently gated only by
feature "token" but the symbol requires both "token" and "cpi-context"; update
the cfg on the pub use (the create_accounts, AtaInitParam, CreateMintsInput,
PdaInitParam, SharedAccounts, TokenInitParam re-export) to require both features
(e.g., cfg(all(feature = "token", feature = "cpi-context"))) so consumers
enabling only "token" won't hit a missing-symbol compile error; alternatively,
if you prefer feature wiring, add a Cargo feature dependency ensuring "token"
enables "cpi-context" in this crate's Cargo.toml.
In `@sdk-libs/sdk-types/src/interface/accounts/create_accounts.rs`:
- Around line 141-309: The PDAS to u8 casts can overflow silently: add an
explicit guard that fails when PDAS or the runtime pdas.len() would exceed 255
before any cast; for example, in the main function before calling
create_mints_inner::<AI, MINTS> and before the PDA preparation loop check that
PDAS <= 255 and pdas.len() <= 255 (or that i < 256) and return
Err(LightSdkTypesError::InvalidInstructionData) if not; ensure you remove or
keep casts only after the checks so the uses of PDAS as u8 (the
create_mints_inner::<AI, MINTS>(..., PDAS as u8) call) and i as u8 passed into
prepare_compressed_account_on_init are safe.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@sdk-libs/sdk-types/src/interface/accounts/create_accounts.rs`:
- Around line 168-185: The code constructs a CpiAccounts (via
CpiAccounts::new_with_config and CpiAccountsConfig::new/_with_cpi_context) even
when PDAS == 0 and MINTS == 0 and the resulting cpi_accounts is never used by
create_token_vaults or create_atas; avoid the unnecessary validation/slicing by
guarding construction: check the conditions (e.g., if shared.proof.PDAS > 0 ||
shared.proof.MINTS > 0) before creating CpiAccounts and only call
CpiAccounts::new_with_config when needed, otherwise skip building cpi_accounts
(or make it an Option) and propagate that to callers so unused
allocation/validation is not performed; refer to symbols CpiAccounts,
CpiAccountsConfig, create_token_vaults, create_atas, remaining_accounts, and
shared.proof.system_accounts_offset when applying the change.
- Around line 105-140: In create_accounts, the pda_setup closure parameter is
silently ignored when the const generic PDAS == 0; add a short runtime/debug
assertion and/or an inline comment just before the PDAS > 0 check to document
and assert this behavior so callers aren’t surprised (reference the
create_accounts function, the pda_setup parameter, and the PDAS const generic);
specifically, add a brief debug_assert or explicit comment indicating “pda_setup
is intentionally not called when PDAS == 0” and consume/drop the closure in that
branch if needed to avoid unused-value warnings.
- Around line 396-417: Lift the duplicate CreateTokenAtaCpi construction out of
the conditional: inside the atas loop build a single CreateTokenAtaCpi instance
using shared.fee_payer, ata.owner, ata.mint, ata.ata, then if ata.idempotent
call .idempotent() on that instance else keep the base instance, and finally
call .rent_free(compressible_config, rent_sponsor, system_program).invoke()?;
this preserves the type-state flow of CreateTokenAtaCpi ->
CreateTokenAtaCpiIdempotent while removing duplicated struct construction and
keeping .rent_free()/.invoke() applied to the resulting value.
ananas-block
commented
Feb 18, 2026
ananas-block
commented
Feb 18, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/macros/src/light_pdas/accounts/validation.rs (1)
28-33:⚠️ Potential issue | 🟡 MinorStale doc: rule 8 doesn't mention tokens or ATAs with init.
Line 28 still says "When PDAs or mints exist" but
validate_proof_availability(Line 206) now also requires proof forhas_tokens_with_initandhas_atas_with_init. The doc should match the code.📝 Proposed fix
-//! 8. **CreateAccountsProof availability** - When PDAs or mints exist, -//! `CreateAccountsProof` must be available via either: +//! 8. **CreateAccountsProof availability** - When any init fields exist +//! (PDAs, mints, tokens with init, or ATAs with init), +//! `CreateAccountsProof` must be available via either:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/macros/src/light_pdas/accounts/validation.rs` around lines 28 - 33, Update the documentation block describing rule 8 to match the current validation logic: state that CreateAccountsProof must be available not only when PDAs or mints exist but also when tokens with init or ATAs with init are present; reference the validation function validate_proof_availability and the proof type CreateAccountsProof, and mention the conditions has_tokens_with_init and has_atas_with_init so the doc reflects that proof is required in any of those cases whether provided directly via #[instruction(proof: CreateAccountsProof)] or nested in a params struct field like create_accounts_proof.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/macros/src/light_pdas/accounts/builder.rs`:
- Around line 629-633: Document the parser invariant or make the guard explicit:
update the token metadata handling around has_metadata and
token_metadata_binding in builder.rs so it's clear why
name.unwrap/symbol.unwrap/uri.unwrap are safe—either add a comment noting that
the parser in light_account.rs guarantees mint.name, mint.symbol, and mint.uri
are present or absent together, or change the guard to check all three
(mint.name.is_some() && mint.symbol.is_some() && mint.uri.is_some()) before
performing the as_ref().map(...).unwrap() calls; reference the
mint.name/mint.symbol/mint.uri fields and the
token_metadata_binding/has_metadata variables when making the change.
- Around line 349-356: The code hardcodes self.system_program in builder.rs when
has_tokens_with_init or has_atas_with_init is true instead of resolving it via
the existing InfraRefs/InfraFields mechanism; update the infra handling so
system program names are normalized and validated: add a SystemProgram variant
to InfraFieldType with accepted_names() (e.g., "system_program", "sys_program",
"system"), include it in InfraFields and InfraRefs so the builder uses
InfraRefs::<resolver> to get the account (replace direct self.system_program
usage), and update validate_infra_fields() to error if a system program field is
required but missing when has_tokens_with_init or has_atas_with_init is true.
In `@sdk-libs/macros/src/light_pdas/accounts/validation.rs`:
- Line 206: The line defining needs_proof is over the formatter width; reformat
it to satisfy rustfmt by splitting the expression across multiple lines. Locate
the binding needs_proof in validation.rs (the line with let needs_proof =
ctx.has_pdas || ctx.has_mints || ctx.has_tokens_with_init ||
ctx.has_atas_with_init;) and break the OR chain into multiple lines (e.g., one
operand per line with proper indentation) so rustfmt/linters no longer flag the
long line while preserving the same boolean logic.
In `@sdk-libs/sdk-types/src/interface/accounts/create_accounts.rs`:
- Around line 141-146: The long conditional using PDAS and MINTS causes
formatter to exceed line width; refactor it into shorter expressions (e.g.,
assign u8::MAX as usize to a let max_u8 and/or compute let total =
PDAS.saturating_add(MINTS)) and then use a multi-line if that checks PDAS >
max_u8 || MINTS > max_u8 || total > max_u8 and returns
Err(LightSdkTypesError::InvalidInstructionData) — this keeps the logic identical
while satisfying formatting/linting.
---
Outside diff comments:
In `@sdk-libs/macros/src/light_pdas/accounts/validation.rs`:
- Around line 28-33: Update the documentation block describing rule 8 to match
the current validation logic: state that CreateAccountsProof must be available
not only when PDAs or mints exist but also when tokens with init or ATAs with
init are present; reference the validation function validate_proof_availability
and the proof type CreateAccountsProof, and mention the conditions
has_tokens_with_init and has_atas_with_init so the doc reflects that proof is
required in any of those cases whether provided directly via
#[instruction(proof: CreateAccountsProof)] or nested in a params struct field
like create_accounts_proof.
---
Duplicate comments:
In `@sdk-libs/sdk-types/src/interface/accounts/create_accounts.rs`:
- Around line 396-417: Construct the CreateTokenAtaCpi once inside the loop
(using the same fields payer/shared.fee_payer, owner/ata.owner, mint/ata.mint,
ata/ata.ata), then conditionally call .idempotent() on that instance only when
ata.idempotent is true before chaining .rent_free(compressible_config,
rent_sponsor, system_program). Finally call .invoke()? on the resulting builder;
e.g., create a local variable like let mut cpi = CreateTokenAtaCpi { ... }; if
ata.idempotent { cpi = cpi.idempotent(); } cpi.rent_free(...).invoke()? to
eliminate the duplicated struct construction.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/macros/src/light_pdas/accounts/validation.rs (1)
28-33:⚠️ Potential issue | 🟡 MinorDoc comment is stale — doesn't reflect the expanded proof requirement.
Line 28 says "When PDAs or mints exist" but the implementation at lines 206-207 now also requires
CreateAccountsProofwhenhas_tokens_with_initorhas_atas_with_init. Update the doc to match:Suggested fix
-//! 8. **CreateAccountsProof availability** - When PDAs or mints exist, +//! 8. **CreateAccountsProof availability** - When PDAs, mints, tokens with init, or ATAs with init exist,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/macros/src/light_pdas/accounts/validation.rs` around lines 28 - 33, Update the doc comment for "CreateAccountsProof availability" to reflect the expanded requirement: state that CreateAccountsProof must be provided not only when PDAs or mints exist but also when the flags has_tokens_with_init or has_atas_with_init are true; keep the two valid provision methods (direct instruction argument via #[instruction(proof: CreateAccountsProof)] or nested in params struct with a create_accounts_proof field) and ensure the wording references the symbols CreateAccountsProof, has_tokens_with_init, and has_atas_with_init so readers can correlate the doc with the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/account-pinocchio/Cargo.toml`:
- Around line 14-19: The dependency entry currently lists "light-account =
[\"light-sdk-types/light-account\", \"dep:light-token-interface\"]" but the
"light-sdk-types/light-account" part is redundant because light-sdk-types
already enables the light-account feature unconditionally; remove
"light-sdk-types/light-account" from the features array so the dependency reads
only "light-account = [\"dep:light-token-interface\"]" (or move the entire
"light-account" feature out of the base dependency into an explicit optional
feature), and ensure lib.rs feature-gated re-exports (the create_accounts/token
re-export blocks) still behave as intended by relying on the feature name
"light-account" and the dep:light-token-interface activation.
In `@sdk-libs/account/Cargo.toml`:
- Around line 14-20: The dependency list currently unconditionally adds the
feature "light-account" (the entry light-account =
["light-sdk-types/light-account", "dep:light-token-interface"]) while
light-sdk-types is already built with "light-account" in its base features;
remove the unconditional activation by deleting or changing that light-account
array entry and instead add a small opt-in relay: ensure light-sdk-types' base
features include "cpi-context" and implement a feature table where the
package-local "light-account" feature forward-activates
light-sdk-types/light-account and dep:light-token-interface (so consumers opt
into the feature via the package feature rather than having
light-sdk-types/light-account always enabled). Reference the identifiers
light-account, light-sdk-types, dep:light-token-interface, and cpi-context when
making the change.
In `@sdk-libs/macros/src/light_pdas/accounts/builder.rs`:
- Around line 832-868: The code in generate_ata_init_params currently hardcodes
AtaInitParam { idempotent: true } for every ATA—change this to respect a
configurable flag instead of always true: add a boolean on the AtaField (e.g.,
ata_field.idempotent or allow parsing a new attribute from #[light_account(...
associated_token(..., idempotent = false))]) and use that value when
constructing AtaInitParam in generate_ata_init_params; update any places that
build AtaField (parsing/constructor) to supply the desired default (true for
backward compatibility) or the parsed value, or if the current design
intentionally requires always-idempotent behavior, add a short clarifying
comment above generate_ata_init_params explaining that idempotency is enforced
by design.
In `@sdk-libs/sdk-types/src/interface/accounts/create_accounts.rs`:
- Around line 305-320: Extract the magic literal `1` used for the `mode` field
into a named constant (e.g., MODE_V2) and use that constant when constructing
the InstructionDataInvokeCpiWithAccountInfo instance; specifically replace
`mode: 1` with `mode: MODE_V2` and define `const MODE_V2: <appropriate_type> =
1;` near the top of the file or alongside related constants, keeping the
existing comment (or replacing it with a clearer one) and ensuring the constant
type matches InstructionDataInvokeCpiWithAccountInfo::mode.
- Around line 199-207: Replace the infallible unwrap of cpi_accounts with a
fallible conversion so the program returns a LightSdkTypesError instead of
panicking: in the create_pdas call replace .as_ref().expect("...") with
.as_ref().ok_or(LightSdkTypesError::CpiAccountsMissing)? (or your existing
suitable variant) and propagate the error with ?; do the same change in the
MINTS branch where cpi_accounts is used (the other create_* call) so both
branches return an error instead of calling expect().
---
Outside diff comments:
In `@sdk-libs/macros/src/light_pdas/accounts/validation.rs`:
- Around line 28-33: Update the doc comment for "CreateAccountsProof
availability" to reflect the expanded requirement: state that
CreateAccountsProof must be provided not only when PDAs or mints exist but also
when the flags has_tokens_with_init or has_atas_with_init are true; keep the two
valid provision methods (direct instruction argument via #[instruction(proof:
CreateAccountsProof)] or nested in params struct with a create_accounts_proof
field) and ensure the wording references the symbols CreateAccountsProof,
has_tokens_with_init, and has_atas_with_init so readers can correlate the doc
with the implementation.
---
Duplicate comments:
In `@sdk-libs/macros/src/light_pdas/accounts/builder.rs`:
- Around line 348-355: The generated code currently hardcodes
self.system_program in builder.rs (the let __system_program_info =
self.system_program.to_account_info() path) which fails silently if the user's
infra field has a different name; update the infra handling so system_program is
treated like other infra fields: add a SystemProgram variant to InfraFieldType
and include its possible names in InfraRefs (or extend InfraRefs to resolve
"system_program" alternatives), then use the resolved InfraRef when emitting
code in builder.rs instead of hardcoding self.system_program; additionally, if
you prefer a minimal change, add a clear validation in validate_infra_fields()
that when has_tokens_with_init || has_atas_with_init is true an infra field
resolving to system_program exists and emit a helpful error mentioning the
missing system_program infra field.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/account-pinocchio/Cargo.toml (1)
13-18: 🧹 Nitpick | 🔵 TrivialSame
light-sdk-types/tokenredundancy asaccount/Cargo.toml.The change from
"cpi-context"to"light-account"is correct for the same reason as the sibling crate. And for the same reason,"light-sdk-types/token"in the localtokenfeature (line 13) is already implied by the unconditionallight-account, leavingdep:light-token-interfaceas the only real gate.🧹 Optional cleanup
-token = ["light-sdk-types/token", "dep:light-token-interface"] +token = ["dep:light-token-interface"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/account-pinocchio/Cargo.toml` around lines 13 - 18, The token feature in Cargo.toml redundantly lists "light-sdk-types/token" which is already implied by the unconditional light-account feature; update the token feature declaration so it only gates on dep:light-token-interface (i.e., remove "light-sdk-types/token" from the token feature array), leaving poseidon/sha256 entries unchanged and keeping light-sdk-types dependency with feature "light-account".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@sdk-libs/account-pinocchio/Cargo.toml`:
- Around line 13-18: The token feature in Cargo.toml redundantly lists
"light-sdk-types/token" which is already implied by the unconditional
light-account feature; update the token feature declaration so it only gates on
dep:light-token-interface (i.e., remove "light-sdk-types/token" from the token
feature array), leaving poseidon/sha256 entries unchanged and keeping
light-sdk-types dependency with feature "light-account".
ananas-block
force-pushed
the
jorrit/refactor-light-account-creation-to-generic-function
branch
from
dcad43c to
b69dd22
Compare
Replace manual CPI orchestration in pda, account_loader, and two_mints test modules with the unified create_accounts() generic function, reducing boilerplate across both pinocchio and anchor manual tests.
…ccounts() Replace manual CPI orchestration with the unified create_accounts() function across all 5 migratable processors (pda, account_loader, mint, two_mints, all).
Fail fast if PDAS or MINTS exceed u8::MAX, preventing silent wrapping in downstream `as u8` casts for cpi_context_offset and account indices.
Entire-Checkpoint: 1da6fecc501a
Entire-Checkpoint: c788d31d848b
Entire-Checkpoint: 7bbee35ce40f
Entire-Checkpoint: 5a7704933620
ananas-block
force-pushed
the
jorrit/refactor-light-account-creation-to-generic-function
branch
from
e210875 to
a6f5b44
Compare
SwenSchaeferjohann
approved these changes
Feb 19, 2026
ananas-block
deleted the
jorrit/refactor-light-account-creation-to-generic-function
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors all account creation (PDAs, mints, tokens, ATAs) into a single generic
create_accounts()SDK function, replacing multiple separate code generation paths in the macro with one unified call.Changes
New
create_accounts()generic SDK function - Added unified SDK function insdk-libs/sdk-types/src/interface/accounts/create_accounts.rs(417 lines) for creating PDAs, mints, tokens, and ATAs in a single call with type-safe generic parameters.Export create_accounts in public SDKs - Added re-exports of
create_accounts,PdaInitParam,CreateMintsInput,TokenInitParam,AtaInitParam, andSharedAccountstosdk-libs/account/src/lib.rsandsdk-libs/account-pinocchio/src/lib.rsunder#[cfg(feature = "token")].Macro refactor: Single unified pre_init path - Replaced multiple separate code generation paths (
generate_pre_init_pdas_only(),generate_pre_init_pdas_and_mints(), etc.) with a singlegenerate_pre_init_all()that delegates all account creation to the genericcreate_accounts()function.PDA setup closure now includes serialization - Modified
generate_pda_setup_closure_body()to serialize account data afterset_decompressed()for both boxed and regular Anchor accounts (matching old PDA code pattern).Removed dead token generation code - Deleted
sdk-libs/macros/src/light_pdas/accounts/token.rs(279 lines) as token creation is now handled by genericcreate_accounts().Cleaned up mint field parsing - Removed unused
address_tree_infofield fromLightMintFieldstruct, removeddirect_proof_argparameter frombuild_mint_field(), and simplified mint parsing.Indexed bump slice identifiers - Fixed
__mint_bump_sliceand__auth_bump_sliceto be per-mint indexed (__mint_bump_slice_{idx},__auth_bump_slice_{idx}) to avoid shadowing across multiple mints.Updated manual tests - Simplified test implementations in
anchor-manual-testandpinocchio-manual-testto use newcreate_accounts()function instead of separate mint/PDA/token code paths.Fixed clippy warnings - Collapsed nested
ifstatement increate_accounts.rsand removed unnecessary.clone()calls onAccountInfo(Copy type) in manual tests.Test plan
cargo test -p light-sdk-macros(310 unit tests)cargo test-sbf -p csdk-anchor-full-derived-test(77 tests)cargo test-sbf -p single-pda-testcargo test-sbf -p single-mint-test./scripts/format.sh(lint clean)Summary by CodeRabbit
New Features
Refactor
Tests
Documentation