Revert ":sparkles: CPI Authentication using a World PDA (#196)" #213
Conversation
This reverts commit 1463d8d.
WalkthroughThis pull request replaces CPI authentication across multiple codebases, transitioning from a signer-based Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Program as World Program
participant Comp as Component Program
participant Sysvar as Sysvar Instructions
rect rgb(200, 220, 255)
Note over Client,Sysvar: Old Flow (CPI Auth Signer)
Client->>Program: Send instruction with cpi_auth signer
Program->>Program: Validate cpi_auth signer seeds
Program->>Comp: CPI call with signer seeds
end
rect rgb(220, 255, 200)
Note over Client,Sysvar: New Flow (Instruction Sysvar)
Client->>Program: Send instruction with sysvar account
Program->>Sysvar: Read instruction sysvar
Program->>Program: Verify instruction caller via get_instruction_relative
Program->>Comp: CPI call (direct, no signer seeds)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Reasoning: Multi-language refactor spanning 6 distinct files across C#, TypeScript, and Rust with significant structural changes. Includes trait signature modifications, public API surface changes, logic replacement (CPI checker to instruction-caller validation), and cascading updates across account structures and context builders. Requires understanding CPI semantics, lifetime parameters, and cross-cutting authentication flow changes. Changes are largely cohesive (single semantic replacement) but affect multiple interdependent systems, necessitating careful verification of consistency across all codebases. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
clients/typescript/src/generated/idl/world.jsonis excluded by!**/generated/**clients/typescript/src/generated/instructions/apply.tsis excluded by!**/generated/**clients/typescript/src/generated/instructions/initializeComponent.tsis excluded by!**/generated/**clients/typescript/src/generated/types/world.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs(8 hunks)clients/typescript/src/world/transactions.ts(2 hunks)clients/typescript/test/low-level/ecs.ts(0 hunks)clients/typescript/test/low-level/permissioning/component.ts(0 hunks)clients/typescript/test/low-level/permissioning/world.ts(1 hunks)clients/typescript/test/low-level/session.ts(0 hunks)crates/bolt-lang/attribute/bolt-program/src/lib.rs(8 hunks)crates/programs/bolt-component/src/lib.rs(4 hunks)crates/programs/world/src/lib.rs(9 hunks)
💤 Files with no reviewable changes (3)
- clients/typescript/test/low-level/permissioning/component.ts
- clients/typescript/test/low-level/ecs.ts
- clients/typescript/test/low-level/session.ts
🧰 Additional context used
🧬 Code graph analysis (4)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (2)
clients/csharp/Solana.Unity.Bolt.Test/Velocity.cs (6)
Solana(305-317)Solana(319-335)SessionToken(55-89)SessionToken(68-88)Entity(31-53)Entity(38-52)clients/typescript/src/generated/types/world.ts (1)
World(7-467)
crates/bolt-lang/attribute/bolt-program/src/lib.rs (1)
clients/typescript/src/generated/types/world.ts (1)
World(7-467)
crates/programs/world/src/lib.rs (2)
crates/programs/bolt-component/src/lib.rs (2)
initialize(9-11)destroy(13-15)clients/typescript/src/world/transactions.ts (1)
InitializeComponent(393-428)
clients/typescript/src/world/transactions.ts (1)
clients/typescript/src/index.ts (1)
SYSVAR_INSTRUCTIONS_PUBKEY(20-22)
⏰ 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). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: test
🔇 Additional comments (14)
crates/programs/bolt-component/src/lib.rs (2)
33-36: Sysvar wiring looks correctAdding instruction_sysvar_account with the fixed address is appropriate for these handlers. No issues found.
Also applies to: 46-49, 68-71, 90-93
101-107: All verification checks passed—change is consistent and completeThe code shows:
- Both trait implementations (lines 110–118, 121–129) consistently use
CpiContext::new()instead ofnew_with_signer()- Downstream usages in
crates/programs/world/src/lib.rs(lines 721, 743) invoke.build_cpi_context()without signer seed parameters- No lingering references to
new_with_signer,signer_seeds, orcpi_authexist in the codebaseThe migration from signer support to basic CPI contexts is complete and has no unresolved dependencies.
clients/typescript/test/low-level/permissioning/world.ts (1)
2-2: Removed CPI auth import — OKImport cleanup aligns with sysvar-based auth. No concerns.
clients/typescript/src/world/transactions.ts (1)
419-421: InitializeComponent now passes instruction sysvar explicitly — goodExplicitly providing SYSVAR_INSTRUCTIONS_PUBKEY is correct and future‑proof.
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (1)
280-282: C# bindings updated to InstructionSysvarAccount — looks goodDefaults point to Sysvar1nstructions..., and instruction key order includes the sysvar. All consistent with the IDL.
Also applies to: 290-294, 321-323, 337-339, 435-436, 453-454, 485-486, 499-500
crates/bolt-lang/attribute/bolt-program/src/lib.rs (1)
150-156: Instruction-sysvar caller check — pattern verified, runtime testing requiredThe code pattern is correctly and consistently implemented across all four locations (lines 150, 187, 229, 255), and the TypeScript IDL properly includes
instructionSysvarAccount. However, the core concern—thatget_instruction_relative(0)returns the outermost transaction instruction during CPI (not intermediate calls)—remains a runtime semantic that requires empirical testing on your target Solana cluster. You should verify this behavior through integration tests that confirm:
- Direct component calls without World fail with
InvalidCaller- Calls routed through World succeed
- CPI scenarios from other programs are correctly rejected
crates/programs/world/src/lib.rs (8)
260-271: LGTM: CPI calls correctly simplified.The removal of signer seeds from
build()calls is correct for the instruction sysvar authentication approach. These functions now properly delegate authentication to the instruction sysvar verification performed in the account structs.
299-324: LGTM: Instruction sysvar authentication properly implemented.The
instruction_sysvar_accountwith address constraint correctly implements the standard Solana pattern for CPI verification. The address check at line 308 ensures only the legitimate instruction sysvar account can be used.
353-381: LGTM: Session-based apply context correctly updated.The
ApplyWithSessionstruct properly mirrors theApplystruct changes, maintaining consistency in how instruction sysvar authentication is handled across both session and non-session flows.
523-558: LGTM: InitializeComponent correctly refactored.The removal of lifetime parameters and signer seeds from the
build()method is appropriate for the instruction sysvar approach. The method now correctly constructs aCpiContextwithout PDA signing, relying on the instruction sysvar for authentication verification.
560-599: LGTM: DestroyComponent consistently updated.The changes mirror those in
InitializeComponent, ensuring consistency across component lifecycle operations. The instruction sysvar approach is correctly applied.
706-722: LGTM: Update context builder properly refactored.The
build_update_contextfunction correctly transitions to the instruction sysvar approach. The simplified lifetime parameters and the use ofbuild_cpi_context()improve code clarity while maintaining correctness.
724-744: LGTM: Session update context builder consistently updated.The
build_update_context_with_sessionfunction maintains consistency withbuild_update_context, correctly applying the instruction sysvar pattern while preserving session token handling.
1-744: No issues found—migration complete and consistent.All concerns have been verified:
- ✅ No remaining
cpi_authreferences in Rust code- ✅ TypeScript client updated (no
CPI_AUTH_ADDRESSreferences)- ✅ C# client updated (no
CpiAuthreferences)- ✅
instruction_sysvar_accountpattern applied consistently across bolt-component, world program, and bolt-lang attributeThe revert is clean and fully propagated across all client SDKs and program implementations.
| FindComponentProgramDataPda, | ||
| } from "../index"; | ||
| import web3 from "@solana/web3.js"; | ||
| import type web3 from "@solana/web3.js"; |
There was a problem hiding this comment.
Invalid TS type import for @solana/web3.js (breaks types like web3.PublicKey)
There is no default export. Use a type namespace import.
Apply this diff:
-import type web3 from "@solana/web3.js";
+import type * as web3 from "@solana/web3.js";📝 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.
| import type web3 from "@solana/web3.js"; | |
| import type * as web3 from "@solana/web3.js"; |
🤖 Prompt for AI Agents
In clients/typescript/src/world/transactions.ts around line 21, the current line
uses an invalid default type import "import type web3 from '@solana/web3.js'";
replace it with a type namespace import so named exports like web3.PublicKey
work correctly: change to "import type * as web3 from '@solana/web3.js'". Ensure
any subsequent type references remain qualified (web3.PublicKey,
web3.Transaction, etc.).
| #[account(address = anchor_lang::solana_program::sysvar::instructions::id())] | ||
| pub instruction_sysvar_account: AccountInfo<'info>, | ||
| pub system_program: Program<'info, System>, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Account type consistency for instruction_sysvar_account
You mix AccountInfo and UncheckedAccount across structs. It works, but pick one for consistency (prefer AccountInfo for exactness).
Also applies to: 210-212, 272-274, 284-286
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/bolt-program/src/lib.rs around lines 173-175 (and
similarly at 210-212, 272-274, 284-286), the instruction_sysvar_account (and
equivalent fields) are inconsistently typed as a mix of AccountInfo and
UncheckedAccount; pick AccountInfo for consistency and precision: change the
field types declared as UncheckedAccount to AccountInfo<'info> for each listed
location and ensure the appropriate anchor_lang::prelude::AccountInfo is in
scope (adjust imports if needed).
1 participant
Reverts PR #196
Summary by CodeRabbit
CpiAuthwithInstructionSysvarAccountacross SDKs (C#, TypeScript, Rust).CPI_AUTH_ADDRESSconstant from TypeScript SDK.