Conversation
WalkthroughRenames “small_ix/Small” to “v2_ix/V2” across features, types, functions, and modules. Updates CPI account types and builders to V2 variants, adjusts AccountMode mapping, switches imports/exports, modifies tests accordingly, and changes the default feature in devenv.sh and Cargo manifests. No new algorithms introduced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test/Client
participant SDK as SDK (v2_ix)
participant Prog as System Program
participant CPI as CPI Handler
Test->>SDK: build instruction (v2_ix = true)
SDK->>Prog: invoke_cpi (mode = V2)
Prog->>CPI: process_invoke_cpi::<V2>(InvokeCpiInstructionV2)
CPI-->>Prog: result
Prog-->>SDK: instruction result
SDK-->>Test: transaction outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ananas-block
force-pushed
the
jorrit/chore-rename-small-to-v
branch
from
5bce9bf to
376ba20
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (1)
76-82: Fix tree account writability
Use each account’s actual writability instead of hard-codingtrue:tree_accounts.iter().for_each(|acc| { account_metas.push(AccountMeta { pubkey: *acc.key, is_signer: false, - is_writable: true, + is_writable: acc.is_writable, }); });
🧹 Nitpick comments (11)
sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (2)
9-13: Minor: preallocation underestimates pushes.You push 4 fixed metas up-front; capacity uses 1 + len - PROGRAM_ACCOUNTS_LEN. Bump to 4 to avoid a realloc.
- let mut account_metas = - Vec::with_capacity(1 + cpi_accounts.account_infos().len() - PROGRAM_ACCOUNTS_LEN); + let mut account_metas = + Vec::with_capacity(4 + cpi_accounts.account_infos().len() - PROGRAM_ACCOUNTS_LEN);
68-68: Prefer returning an error over panicking on offset mismatch.Panics in library code are brittle. Return a typed error instead.
- assert_eq!(cpi_accounts.system_accounts_end_offset(), index); + if cpi_accounts.system_accounts_end_offset() != index { + return Err(crate::error::LightSdkError::CpiAccountsIndexOutOfBounds(index).into()); + }programs/system/src/accounts/mode.rs (1)
10-10: Rename Small -> V2 with stable discriminant (1): LGTM.On-wire compatibility is preserved (Anchor=0, V2=1). Consider a tiny polish:
Add Eq derive for completeness:
-#[derive(Debug, PartialEq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)]Confirm no external code still constructs/expects AccountMode::Small.
Also applies to: 19-19, 29-29
programs/system/tests/invoke_cpi_instruction_v2.rs (2)
389-396: Catching panic for insufficient accounts—consider returning an error instead.from_account_infos panics via split_at when under-provisioned. Prefer explicit error returns for better ergonomics and fuzzing.
If feasible, update implementation to validate length and return ProgramError; then replace catch_unwind with assert_err in tests.
205-205: Use assert_eq! for clearer diffs on error assertions.Improves failure messages.
- assert!(result == Err(ProgramError::from(AccountError::AccountOwnedByWrongProgram))); + assert_eq!(result, Err(ProgramError::from(AccountError::AccountOwnedByWrongProgram))); @@ - assert!(result == Err(ProgramError::from(AccountError::InvalidDiscriminator))); + assert_eq!(result, Err(ProgramError::from(AccountError::InvalidDiscriminator)));Also applies to: 223-224
programs/system/src/lib.rs (1)
172-173: Avoid heap allocation in log message.Use msg! formatting directly instead of allocating a String.
- msg!(format!("mode {:?}", mode).as_str()); + msg!("mode {:?}", mode);programs/system/src/invoke_cpi/instruction_v2.rs (1)
31-69: Prefer error return over assert when extra accounts are present.assert!(accounts.next().is_none()) will abort the program on-chain. Returning a ProgramError is safer and improves diagnosability.
Consider replacing the assert with a checked branch that returns a descriptive error (e.g., InvalidInstructionData or NotEnoughAccountKeys/TooManyAccountKeys).
sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs (2)
41-46: Add a debug assertion to validate optional-account indexing.This catches mismatches between CompressionCpiAccountIndexV2 and CpiAccountsConfig at dev time.
if cpi_accounts.config().cpi_context { let account = cpi_accounts.get_account_info(index)?; account_metas.push(AccountMeta::writable(account.key())); index += 1; } + + // Sanity check: computed index should match the expected end of system accounts. + debug_assert_eq!( + cpi_accounts.system_accounts_end_offset(), + index, + "system_accounts_end_offset mismatch; verify CompressionCpiAccountIndexV2 and CpiAccountsConfig" + );
11-13: Minor: guard vector capacity assumptions.Optionally add a debug_assert that cpi_accounts.account_infos().len() >= PROGRAM_ACCOUNTS_LEN to avoid accidental underflow if the input is malformed in tests.
sdk-libs/sdk-types/src/cpi_accounts_v2.rs (2)
102-114: Offset computation looks right; consider documenting invariants.Consider a brief comment noting that V2_SYSTEM_ACCOUNTS_LEN counts 3 program IDs + 3 mandatory system accounts and that flags remove up to 3 optionals. Helps future maintainers keep indices in sync.
144-152: Optional: add a debug assert before capacity math.to_account_infos pre-alloc size assumes accounts.len() >= PROGRAM_ACCOUNTS_LEN. A debug_assert here helps catch invalid caller inputs during testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
program-tests/create-address-test-program/Cargo.toml(1 hunks)program-tests/create-address-test-program/src/lib.rs(3 hunks)program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs(60 hunks)programs/system/src/accounts/mode.rs(3 hunks)programs/system/src/invoke_cpi/instruction_v2.rs(4 hunks)programs/system/src/invoke_cpi/mod.rs(1 hunks)programs/system/src/lib.rs(2 hunks)programs/system/tests/invoke_cpi_instruction_v2.rs(14 hunks)scripts/devenv.sh(1 hunks)sdk-libs/sdk-pinocchio/Cargo.toml(1 hunks)sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs(2 hunks)sdk-libs/sdk-pinocchio/src/cpi/mod.rs(1 hunks)sdk-libs/sdk-types/Cargo.toml(1 hunks)sdk-libs/sdk-types/src/cpi_accounts_v2.rs(4 hunks)sdk-libs/sdk-types/src/lib.rs(2 hunks)sdk-libs/sdk/Cargo.toml(1 hunks)sdk-libs/sdk/src/cpi/accounts_v2_ix.rs(2 hunks)sdk-libs/sdk/src/cpi/mod.rs(1 hunks)sdk-libs/sdk/src/instruction/system_accounts.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
programs/system/src/lib.rs (2)
programs/system/src/invoke_cpi/instruction_v2.rs (1)
from_account_infos(32-69)programs/system/src/invoke_cpi/instruction.rs (1)
from_account_infos(41-85)
programs/system/tests/invoke_cpi_instruction_v2.rs (2)
programs/system/src/invoke_cpi/instruction_v2.rs (1)
from_account_infos(32-69)sdk-libs/sdk-types/src/cpi_accounts_v2.rs (6)
fee_payer(48-50)authority(52-57)registered_program_pda(59-64)account_compression_authority(66-71)sol_pool_pda(73-78)decompression_recipient(80-85)
sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (1)
sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs (1)
to_account_metas_v2(10-64)
sdk-libs/sdk-pinocchio/src/cpi/mod.rs (1)
programs/system/src/lib.rs (1)
invoke(87-111)
sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs (1)
sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (1)
to_account_metas_v2(9-84)
program-tests/create-address-test-program/src/lib.rs (3)
sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs (1)
to_account_metas_v2(10-64)sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (1)
to_account_metas_v2(9-84)sdk-libs/sdk-types/src/cpi_accounts_v2.rs (4)
fee_payer(48-50)account_infos(116-118)new_with_config(40-46)config(98-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: cli-v2
- GitHub Check: lint
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test"])
- GitHub Check: system-programs (sdk-libs, light-macros light-sdk light-program-test light-client light-batched-m...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: Forester e2e test
🔇 Additional comments (34)
sdk-libs/sdk/src/instruction/system_accounts.rs (1)
127-134: Account meta order/content for V2 looks correct.Noop removed and ordering aligns with the doc comment.
sdk-libs/sdk/src/cpi/accounts_v2_ix.rs (3)
2-3: Correct alias/import migration to V2.Imports and type alias map to the V2 indices and generics as expected.
7-8: Type alias surface — OK.Public alias aligns with GenericCpiAccountsV2.
37-37: Index base for optional accounts — OK.Starting at CompressionCpiAccountIndexV2::SolPoolPda is consistent with the V2 layout.
sdk-libs/sdk/src/cpi/mod.rs (1)
51-58: Module gating switched to v2_ix — looks good.Clean re-export under the new feature flag.
sdk-libs/sdk-types/Cargo.toml (1)
12-12: Feature flag v2_ix added — OK.Aligns with code gated on v2_ix.
sdk-libs/sdk-pinocchio/src/cpi/mod.rs (2)
2-3: Expose accounts_v2 behind v2_ix — OK.
7-8: Public re-export under v2_ix — OK.program-tests/create-address-test-program/Cargo.toml (1)
27-27: Test program now opts into v2_ix — OK.Consistent with the feature migration.
scripts/devenv.sh (1)
95-96: Approve devenv.sh default feature switch to v2_ix
Default feature switched to v2_ix — matches the repo-wide rename and no lingering small_ix references remain.programs/system/src/invoke_cpi/mod.rs (1)
4-4: No staleinstruction_smallreferences found — the rename toinstruction_v2is complete.sdk-libs/sdk-types/src/lib.rs (2)
4-5: Feature-gated cpi_accounts_v2 module: LGTM.The gate aligns with the new v2_ix feature.
16-19: No naming conflict for PROGRAM_ACCOUNTS_LEN
cpi_accountsdoesn’t export that constant, so the conditional re-export won’t collide—aliasing isn’t needed.Likely an incorrect or invalid review comment.
sdk-libs/sdk-pinocchio/Cargo.toml (1)
12-12: Pinocchio feature set updated to v2_ix: LGTM.Aligns Pinocchio crate with the V2 rename.
programs/system/tests/invoke_cpi_instruction_v2.rs (1)
10-11: Import switch to instruction_v2::InvokeCpiInstructionV2: LGTM.Matches the V2 surface.
sdk-libs/sdk/Cargo.toml (1)
22-22: Allsmall_ixreferences removed;v2_ixdeclared in all Cargo.toml files.programs/system/src/lib.rs (2)
15-17: Public re-export aligned with V2 rename.Import switch to instruction_v2::InvokeCpiInstructionV2 is consistent with the API migration.
166-197: Switch to AccountMode::V2 + new context constructor looks correct
TryFrom for AccountMode maps 1 → AccountMode::V2.programs/system/src/invoke_cpi/instruction_v2.rs (2)
17-29: Type rename and fields LGTM.Struct name and field set match the V2 accounts shape and trait impls below.
72-104: Trait impls wired correctly.SignerAccounts, CpiContextAccountTrait, and InvokeAccounts impls correctly expose the V2 fields.
sdk-libs/sdk-pinocchio/src/cpi/accounts_v2.rs (2)
8-16: New alias and signature look good.CpiAccountsV2<'a> and to_account_metas_v2(&CpiAccountsV2<'a>) align with the pinocchio AccountMeta<'a> style.
26-33: Index start matches V2 layout.Starting at CompressionCpiAccountIndexV2::SolPoolPda is correct given 3 program accounts + 3 static system accounts preceding the optionals.
program-tests/create-address-test-program/src/lib.rs (3)
21-23: V2 CPI helpers wired in correctly.Imports of to_account_metas_v2 and CpiAccountsV2 reflect the new surface.
217-229: API flag rename propagated.Parameter v2_ix and its serialization in InvokeWithReadOnly are consistent with the new toggle.
61-79: Re-export confirmed:cpi::to_account_metas_v2returnsVec<AccountMeta>fromsolana_program, not the pinocchio variant; no action needed.sdk-libs/sdk-types/src/cpi_accounts_v2.rs (2)
8-19: Index enum correctly enumerates V2 layout.Ordering matches 3 program accounts + core system accounts followed by optionals.
23-29: Constants and struct rename align with V2.V2_SYSTEM_ACCOUNTS_LEN = 9 (6 system + 3 program IDs) is consistent with the enum.
program-tests/system-cpi-v2-test/tests/invoke_cpi_with_read_only.rs (7)
51-51: LGTM! Consistent renaming of boolean parameter.The parameter has been properly renamed from
small_ixtois_v2_ixfollowing Rust naming conventions for boolean variables. The "is_" prefix clearly indicates this is a boolean value, improving code readability.
117-117: Consistent parameter usage throughout test functions.All calls to
local_sdk::perform_test_transactionhave been updated to useis_v2_ixinstead of the previoussmall_ixparameter. The function signature change aligns with the broader refactoring and maintains consistency across all test cases.Also applies to: 317-317, 414-414, 622-622, 665-665, 791-791, 818-818, 853-853, 880-880, 909-909, 936-936, 1015-1015, 1071-1071, 1159-1159, 1225-1225, 1268-1268, 1383-1383, 1411-1411, 1447-1447, 1475-1475, 1505-1505, 1533-1533, 1613-1613, 1670-1670, 1755-1755, 1818-1818, 1840-1840, 1894-1894, 1921-1921, 1972-1972, 1999-1999, 2026-2026, 2094-2094, 2184-2184, 2227-2227, 2270-2270, 2316-2316, 2390-2390, 2480-2480, 2537-2537, 2588-2588, 2634-2634, 2687-2687, 2714-2714, 2762-2762, 2789-2789, 2910-2910, 2998-2998
350-352: Loop iterator variable consistently renamed.The iterator variable has been consistently renamed from
small_ixtois_v2_ixacross all test functions. This maintains the semantic meaning of the boolean flag while following the established naming pattern.Also applies to: 665-667, 1265-1267, 2316-2316, 2634-2634, 2026-2026
1268-1268: Debug message updated to reflect new naming.The debug print statement has been updated to use "v2 ix" instead of the previous term, maintaining consistency with the parameter renaming throughout the codebase.
3051-3051: Function parameter signature updated consistently.The
perform_test_transactionfunction signature has been updated to usev2_ixparameter name, and the mode selection logic correctly mapsv2_ix: boolto the appropriate mode value (1 for true, 0 for false).Also applies to: 3081-3081
3120-3120: SystemAccountMetaConfig structure and methods updated.The configuration structure and its associated methods have been properly updated to use
v2_ixfield instead ofsmall_ix. All constructor methods maintain consistency with the new field name, ensuring proper initialization across different usage patterns.Also applies to: 3142-3142, 3170-3170, 3180-3180, 3189-3189, 3199-3199
3329-3329: Account metadata selection logic preserved.The conditional logic for selecting different account metadata based on the
v2_ixflag has been maintained correctly. The branching logic ensures that the appropriate system accounts are included based on whether V2 instruction handling is enabled.Also applies to: 3368-3368
| #[cfg(feature = "v2")] | ||
| pub fn get_light_system_account_metas_small(config: SystemAccountMetaConfig) -> Vec<AccountMeta> { | ||
| pub fn get_light_system_account_metas_v2(config: SystemAccountMetaConfig) -> Vec<AccountMeta> { | ||
| let cpi_signer = find_cpi_signer_macro!(&config.self_program).0; |
There was a problem hiding this comment.
🧩 Analysis chain
Fix feature gate: use v2_ix, not v2.
This function is gated behind feature="v2", while the repo standard is "v2_ix". As-is, enabling v2_ix won’t compile this function.
Apply:
-#[cfg(feature = "v2")]
+#[cfg(feature = "v2_ix")]
pub fn get_light_system_account_metas_v2(config: SystemAccountMetaConfig) -> Vec<AccountMeta> {Also scan for stray v2 gates:
🏁 Script executed:
#!/bin/bash
rg -nP '\#\s*\[cfg\s*\(\s*feature\s*=\s*"v2"\s*\)\s*\]'Length of output: 1983
Fix feature gate: use v2_ix, not v2, repository-wide.
Replace every
#[cfg(feature = "v2")]with
#[cfg(feature = "v2_ix")](25 occurrences across sdk-libs/client, sdk-libs/sdk, sdk-libs/program-test).
Example in sdk-libs/sdk/src/instruction/system_accounts.rs:
-#[cfg(feature = "v2")]
+#[cfg(feature = "v2_ix")]🤖 Prompt for AI Agents
In sdk-libs/sdk/src/instruction/system_accounts.rs around lines 122 to 124, the
feature gate used is #[cfg(feature = "v2")] but the repository standard is to
use #[cfg(feature = "v2_ix")]; update this occurrence (and any similar
occurrences in the file) to #[cfg(feature = "v2_ix")] so the code compiles
consistently with the rest of the repo.
2 participants
Summary by CodeRabbit