[Vault SDK] Add Solana operations support #8168
Conversation
🦋 Changeset detectedLatest commit: 0456059 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Solana support to the vault-sdk: new Solana runtime/dev dependencies and a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App (SDK Consumer)
participant SDK as Vault SDK
participant Vault as Vault Service
participant Solana as Solana RPC
rect rgba(230,245,255,0.5)
note over App,SDK: Create / list Solana accounts
App->>SDK: createSolanaAccount(payload)
SDK->>Vault: request { op: "solana:create", data }
Vault-->>SDK: response { accountPubkey }
SDK-->>App: accountPubkey
App->>SDK: listSolanaAccounts(payload)
SDK->>Vault: request { op: "solana:read", filter }
Vault-->>SDK: response { accounts[] }
SDK-->>App: accounts[]
end
rect rgba(255,245,230,0.5)
note over App,Vault: Sign transaction via Vault
App->>Solana: getLatestBlockhash()
Solana-->>App: blockhash
App->>App: build VersionedTransaction (base64)
App->>SDK: signSolanaTransaction({ txBase64, pubkey })
SDK->>Vault: request { op: "solana:signTransaction", tx, pubkey }
Vault-->>SDK: response { signature(base58) }
SDK-->>App: signature(base58)
App->>SDK: reconstructSolanaSignedTransaction(txBase64, signature, pubkey)
SDK-->>App: signedTxBase64
end
rect rgba(255,235,235,0.5)
note over App,Solana: Broadcast and confirm
App->>Solana: sendRawTransaction(signedTxBase64)
Solana-->>App: txSignature
App->>Solana: confirmTransaction(txSignature)
Solana-->>App: confirmation
end
sequenceDiagram
autonumber
participant App as App
participant SDK as Vault SDK
participant Vault as Vault Service
rect rgba(245,245,255,0.5)
note over App,Vault: Sign message (Ed25519)
App->>SDK: signSolanaMessage({ message, pubkey })
SDK->>Vault: request { op: "solana:signMessage", message, pubkey }
Vault-->>SDK: response { signature(base58) }
SDK-->>App: signature(base58)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
Comment |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![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: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/vault-sdk/package.json(1 hunks)packages/vault-sdk/src/exports/thirdweb.ts(3 hunks)packages/vault-sdk/src/sdk.ts(2 hunks)packages/vault-sdk/src/types.ts(3 hunks)packages/vault-sdk/test-solana.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/vault-sdk/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/vault-sdk/src/exports/thirdweb.tspackages/vault-sdk/src/types.tspackages/vault-sdk/test-solana.tspackages/vault-sdk/src/sdk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/vault-sdk/src/exports/thirdweb.tspackages/vault-sdk/src/types.tspackages/vault-sdk/test-solana.tspackages/vault-sdk/src/sdk.ts
**/types.ts
📄 CodeRabbit inference engine (AGENTS.md)
Provide and re‑use local type barrels in a
types.tsfile
Files:
packages/vault-sdk/src/types.ts
🧠 Learnings (1)
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/exports/** : Export everything via `exports/` directory, grouped by feature in the SDK public API
Applied to files:
packages/vault-sdk/src/exports/thirdweb.ts
🧬 Code graph analysis (2)
packages/vault-sdk/test-solana.ts (2)
packages/vault-sdk/src/sdk.ts (7)
VaultClient(157-161)createVaultClient(163-217)createServiceAccount(326-337)createSolanaAccount(504-515)listSolanaAccounts(517-528)signSolanaTransaction(530-541)signSolanaMessage(543-554)packages/vault-sdk/src/types.ts (1)
SolanaVersionedMessage(154-173)
packages/vault-sdk/src/sdk.ts (1)
packages/vault-sdk/src/types.ts (21)
Payload(679-697)EncryptedPayload(14-18)UnencryptedErrorResponse(54-56)PingPayload(540-545)CreateServiceAccountPayload(547-552)GetServiceAccountPayload(554-559)RotateServiceAccountPayload(561-566)CreateEoaPayload(568-573)ListEoaPayload(575-580)SignTransactionPayload(582-587)SignMessagePayload(621-626)CreateAccessTokenPayload(628-633)RevokeAccessTokenPayload(642-647)SignAuthorizationPayload(589-594)SignStructuredMessagePayload(597-602)ListAccessTokensPayload(636-640)CreateSolanaAccountPayload(650-655)ListSolanaAccountsPayload(657-662)SignSolanaTransactionPayload(664-669)SignSolanaMessagePayload(671-676)PolicyComponent(318-388)
🪛 GitHub Actions: CI
packages/vault-sdk/package.json
[error] 1-1: Lockfile specifiers do not match package.json: typescript, @biomejs/biome, @noble/ciphers, @noble/curves, @noble/hashes, @solana/*, abitype, jose, rimraf, tsx, etc. Run 'pnpm install' without --frozen-lockfile or update the lockfile.
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
| export function createSolanaAccount({ | ||
| client, | ||
| request: options, | ||
| }: PayloadParams<CreateSolanaAccountPayload>) { | ||
| return sendRequest<CreateSolanaAccountPayload>({ | ||
| client, | ||
| request: { | ||
| operation: "solana:create", | ||
| ...options, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| export function listSolanaAccounts({ | ||
| client, | ||
| request: options, | ||
| }: PayloadParams<ListSolanaAccountsPayload>) { | ||
| return sendRequest<ListSolanaAccountsPayload>({ | ||
| client, | ||
| request: { | ||
| operation: "solana:list", | ||
| ...options, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Process Solana account payloads before returning
Line 506 currently returns the raw sendRequest result, so the backend’s pubkey byte array is never converted into the publicKey/address strings that the SDK types and test-solana.ts rely on. As soon as the test calls signSolanaTransaction, it sends from: undefined and the vault rejects the request. listSolanaAccounts has the same issue for every entry. Please run _processSolanaAccountData on the responses before returning them.
Apply this diff:
-export function createSolanaAccount({
+export async function createSolanaAccount({
client,
request: options,
}: PayloadParams<CreateSolanaAccountPayload>) {
- return sendRequest<CreateSolanaAccountPayload>({
- client,
- request: {
- operation: "solana:create",
- ...options,
- },
- });
+ const response = await sendRequest<CreateSolanaAccountPayload>({
+ client,
+ request: {
+ operation: "solana:create",
+ ...options,
+ },
+ });
+
+ if (response.success && response.data) {
+ return {
+ ...response,
+ data: _processSolanaAccountData({ ...response.data }),
+ };
+ }
+
+ return response;
}
-export function listSolanaAccounts({
+export async function listSolanaAccounts({
client,
request: options,
}: PayloadParams<ListSolanaAccountsPayload>) {
- return sendRequest<ListSolanaAccountsPayload>({
- client,
- request: {
- operation: "solana:list",
- ...options,
- },
- });
+ const response = await sendRequest<ListSolanaAccountsPayload>({
+ client,
+ request: {
+ operation: "solana:list",
+ ...options,
+ },
+ });
+
+ if (response.success && response.data?.accounts) {
+ return {
+ ...response,
+ data: {
+ ...response.data,
+ accounts: response.data.accounts.map((account) =>
+ _processSolanaAccountData({ ...account }),
+ ),
+ },
+ };
+ }
+
+ return response;
}📝 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.
| export function createSolanaAccount({ | |
| client, | |
| request: options, | |
| }: PayloadParams<CreateSolanaAccountPayload>) { | |
| return sendRequest<CreateSolanaAccountPayload>({ | |
| client, | |
| request: { | |
| operation: "solana:create", | |
| ...options, | |
| }, | |
| }); | |
| } | |
| export function listSolanaAccounts({ | |
| client, | |
| request: options, | |
| }: PayloadParams<ListSolanaAccountsPayload>) { | |
| return sendRequest<ListSolanaAccountsPayload>({ | |
| client, | |
| request: { | |
| operation: "solana:list", | |
| ...options, | |
| }, | |
| }); | |
| } | |
| export async function createSolanaAccount({ | |
| client, | |
| request: options, | |
| }: PayloadParams<CreateSolanaAccountPayload>) { | |
| const response = await sendRequest<CreateSolanaAccountPayload>({ | |
| client, | |
| request: { | |
| operation: "solana:create", | |
| ...options, | |
| }, | |
| }); | |
| if (response.success && response.data) { | |
| return { | |
| ...response, | |
| data: _processSolanaAccountData({ ...response.data }), | |
| }; | |
| } | |
| return response; | |
| } | |
| export async function listSolanaAccounts({ | |
| client, | |
| request: options, | |
| }: PayloadParams<ListSolanaAccountsPayload>) { | |
| const response = await sendRequest<ListSolanaAccountsPayload>({ | |
| client, | |
| request: { | |
| operation: "solana:list", | |
| ...options, | |
| }, | |
| }); | |
| if (response.success && response.data?.accounts) { | |
| return { | |
| ...response, | |
| data: { | |
| ...response.data, | |
| accounts: response.data.accounts.map((account) => | |
| _processSolanaAccountData({ ...account }), | |
| ), | |
| }, | |
| }; | |
| } | |
| return response; | |
| } |
🤖 Prompt for AI Agents
In packages/vault-sdk/src/sdk.ts around lines 504 to 528, the functions
createSolanaAccount and listSolanaAccounts currently return the raw sendRequest
result so the backend pubkey byte array is never converted to SDK-friendly
publicKey/address strings; call and return the result of
_processSolanaAccountData on the response(s) after awaiting sendRequest (for
create: process the single account object; for list: map/process each account in
the returned list), preserving types and errors and ensuring the functions
remain async/return the processed payload to callers.
|
This PR has been inactive for 7 days. It is now marked as stale and will be closed in 2 days if no further activity occurs. |
d4mr
force-pushed
the
pb/solana-vault-ts-sdk
branch
from
e9e20e8 to
409df8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/vault-sdk/src/sdk.ts (1)
463-487: Process Solana account payloads before returningThe response still skips
_processSolanaAccountData, so callers get rawpubkeybytes and downstream Solana flows fail (e.g.,signSolanaTransactionreceivesfrom: undefined). Please await the request and run the processor on the returned account(s) before handing them back.-export function createSolanaAccount({ +export async function createSolanaAccount({ client, request: options, }: PayloadParams<CreateSolanaAccountPayload>) { - return sendRequest<CreateSolanaAccountPayload>({ + const response = await sendRequest<CreateSolanaAccountPayload>({ client, request: { operation: "solana:create", ...options, }, }); + + if (response.success && response.data) { + return { + ...response, + data: _processSolanaAccountData({ ...response.data }), + }; + } + + return response; } -export function listSolanaAccounts({ +export async function listSolanaAccounts({ client, request: options, }: PayloadParams<ListSolanaAccountsPayload>) { - return sendRequest<ListSolanaAccountsPayload>({ + const response = await sendRequest<ListSolanaAccountsPayload>({ client, request: { operation: "solana:list", ...options, }, }); + + if (response.success && response.data?.accounts) { + return { + ...response, + data: { + ...response.data, + accounts: response.data.accounts.map((account) => + _processSolanaAccountData({ ...account }), + ), + }, + }; + } + + return response; }
🧹 Nitpick comments (6)
packages/vault-sdk/vitest.config.ts (1)
3-14: Load test env via dotenv to honor packages/vault-sdk/tests/.env.Without dotenv, overrides must come from the shell. Add a setup file to load tests/.env.
Apply:
export default defineConfig({ test: { globals: true, environment: "node", testTimeout: 60000, // 60 seconds default timeout hookTimeout: 60000, // 60 seconds for beforeAll/afterAll teardownTimeout: 10000, + setupFiles: ["./tests/setup-env.ts"], sequence: { shuffle: false, // Run tests in order }, }, });Add packages/vault-sdk/tests/setup-env.ts:
import { config } from "dotenv"; import { fileURLToPath } from "url"; import path from "node:path"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); config({ path: path.join(__dirname, ".env") });Based on learnings (Vitest 3 best practices).
packages/vault-sdk/src/types.ts (3)
154-173: Prefer JSON-safe types in SolanaVersionedMessage.Uint8Array in instructions.data won’t serialize over JSON. Use base64 (string) for wire compatibility.
export type SolanaVersionedMessage = { version: "legacy" | "v0"; header: { numRequiredSignatures: number; numReadonlySignedAccounts: number; numReadonlyUnsignedAccounts: number; }; staticAccountKeys: string[]; // Use string instead of SolanaAddress for simplicity recentBlockhash: string; instructions: Array<{ programIdIndex: number; accountKeyIndexes: number[]; - data: Uint8Array; + // base64-encoded instruction data for transport + data: string; }>; addressTableLookups?: Array<{ accountKey: string; // Use string instead of SolanaAddress writableIndexes: number[]; readonlyIndexes: number[]; }>; };
362-383: Introduce a SolanaAddress alias for clarity and future validation.Replace plain string with a local alias (type SolanaAddress = string) in policy components to make intent explicit and ease future branding/validation.
Example:
+type SolanaAddress = string; ... | { type: "solana:signTransaction"; - allowlist?: string[]; // Use string for Solana addresses + allowlist?: SolanaAddress[]; metadataPatterns?: MetadataRule[]; payloadPatterns?: Record<string, Rule[]>; - requiredCosigners?: string[]; // Use string for Solana addresses + requiredCosigners?: SolanaAddress[]; } | { type: "solana:signMessage"; - allowlist?: string[]; // Use string for Solana addresses + allowlist?: SolanaAddress[]; metadataPatterns?: MetadataRule[]; messagePattern?: string; }
468-483: Align list response naming with existing EOA list shape.EOA list uses items + totalRecords; Solana uses accounts + totalCount. Consider aligning for consistency and simpler client handling.
-type GetSolanaAccountsData = { - accounts: CreateSolanaAccountData[]; - totalCount: number; +type GetSolanaAccountsData = { + items: CreateSolanaAccountData[]; + totalRecords: number; page: number; pageSize: number; };packages/vault-sdk/tests/solana.test.ts (1)
239-240: Fix misleading lamports comment.100_000_000 lamports = 0.1 SOL, not “1 lamport”. Update comments to avoid confusion.
-const transferAmount = lamports(100_000_000n); // 1 lamport - minimum possible +const transferAmount = lamports(100_000_000n); // 0.1 SOLAlso applies to: 273-274, 329-330, 435-436
packages/vault-sdk/package.json (1)
10-16: Trim Solana deps and align program package
- Move @solana/addresses, @solana/keys, @solana/rpc, @solana/transactions to devDependencies (no direct imports found).
- Verify @solana/kit is used at runtime; if not, move it to devDependencies.
- Add “@solana-program/system@^0.8.0” to dependencies for getTransferSolInstruction (Kit v4 alignment).
- Introduce a size-limit budget to track bundle growth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/vault-sdk/package.json(2 hunks)packages/vault-sdk/src/sdk.ts(2 hunks)packages/vault-sdk/src/types.ts(5 hunks)packages/vault-sdk/tests/.env.example(1 hunks)packages/vault-sdk/tests/solana.test.ts(1 hunks)packages/vault-sdk/vitest.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/vault-sdk/vitest.config.tspackages/vault-sdk/tests/solana.test.tspackages/vault-sdk/src/types.tspackages/vault-sdk/src/sdk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/vault-sdk/vitest.config.tspackages/vault-sdk/tests/solana.test.tspackages/vault-sdk/src/types.tspackages/vault-sdk/src/sdk.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/vault-sdk/tests/solana.test.ts
**/types.ts
📄 CodeRabbit inference engine (AGENTS.md)
Provide and re‑use local type barrels in a
types.tsfile
Files:
packages/vault-sdk/src/types.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/vault-sdk/package.json
🧠 Learnings (1)
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to **/*.test.{ts,tsx} : Keep tests deterministic and side‑effect free; use Vitest
Applied to files:
packages/vault-sdk/package.json
🧬 Code graph analysis (3)
packages/vault-sdk/tests/solana.test.ts (1)
packages/vault-sdk/src/sdk.ts (6)
VaultClient(116-120)createVaultClient(122-176)createServiceAccount(285-296)createSolanaAccount(463-474)signSolanaTransaction(489-500)reconstructSolanaSignedTransaction(505-536)
packages/vault-sdk/src/types.ts (1)
packages/vault-sdk/src/exports/thirdweb.ts (7)
CreateSolanaAccountPayload(38-38)GenericPayload(40-40)Auth(32-32)ListSolanaAccountsPayload(46-46)SignSolanaTransactionPayload(59-59)SignSolanaMessagePayload(58-58)ListAccessTokensPayload(44-44)
packages/vault-sdk/src/sdk.ts (1)
packages/vault-sdk/src/types.ts (21)
Payload(676-694)EncryptedPayload(14-18)UnencryptedErrorResponse(54-56)PingPayload(537-542)CreateServiceAccountPayload(544-549)GetServiceAccountPayload(551-556)RotateServiceAccountPayload(558-563)CreateEoaPayload(565-570)ListEoaPayload(572-577)SignTransactionPayload(579-584)SignMessagePayload(618-623)CreateAccessTokenPayload(625-630)RevokeAccessTokenPayload(639-644)SignAuthorizationPayload(586-591)SignStructuredMessagePayload(594-599)ListAccessTokensPayload(633-637)CreateSolanaAccountPayload(647-652)ListSolanaAccountsPayload(654-659)SignSolanaTransactionPayload(661-666)SignSolanaMessagePayload(668-673)PolicyComponent(318-388)
🪛 dotenv-linter (3.3.0)
packages/vault-sdk/tests/.env.example
[warning] 6-6: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (1)
packages/vault-sdk/tests/solana.test.ts (1)
384-388: Decode base58 signature to raw bytesIn packages/vault-sdk/src/sdk.ts (around line 519), replace:
- // Decode the base58 signature to bytes - const base58Encoder = getBase58Encoder(); - const signatureBytes = base58Encoder.encode(base58Signature); + // Decode the base58 signature to raw bytes + const { getBase58Decoder } = await import("@solana/kit"); + const base58Decoder = getBase58Decoder(); + const signatureBytes = base58Decoder.decode(base58Signature);Ensure the
signaturesmap key matches the expected public-key format when encoding the transaction.Likely an incorrect or invalid review comment.
| SOLANA_RPC_WS_URL=ws://127.0.0.1:8900 | ||
|
|
||
| # Vault Configuration | ||
| VAULT_URL=http://localhost:3000 No newline at end of file |
There was a problem hiding this comment.
Add a trailing newline to satisfy dotenv-linter.
Ends without a newline; add one to appease EndingBlankLine.
Apply:
-VAULT_URL=http://localhost:3000
+VAULT_URL=http://localhost:3000
+📝 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.
| VAULT_URL=http://localhost:3000 | |
| VAULT_URL=http://localhost:3000 | |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 6-6: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
In packages/vault-sdk/tests/.env.example around line 6, the file ends without a
trailing newline which violates dotenv-linter's EndingBlankLine rule; fix by
adding a single newline character at the end of the file so the file ends with a
blank line.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vault-sdk/package.json (1)
6-67: Update the size-limit budget after adding Solana deps.The new runtime dependencies (
@solana/*) will impact the bundle footprint, butpackage.jsonstill lacks/does not update asize-limitentry. Please add or adjust thesize-limitconfiguration so the Solana surface is covered.
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.changeset/soft-cameras-lay.md(1 hunks)packages/vault-sdk/package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md: Each change inpackages/*must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/soft-cameras-lay.md
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/vault-sdk/package.json
🧠 Learnings (1)
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to **/*.test.{ts,tsx} : Keep tests deterministic and side‑effect free; use Vitest
Applied to files:
packages/vault-sdk/package.json
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/vault-sdk/tests/solana.test.ts (2)
44-47: Add explicit return type annotation.The function should have an explicit return type annotation per TypeScript coding guidelines.
Apply this diff:
-function getExplorerUrl(path: string, type: "address" | "tx" = "tx"): string { +function getExplorerUrl(path: string, type: "address" | "tx" = "tx"): string {Wait, the function already has a return type. Let me reconsider...
Actually, looking again at line 44, the function signature is:
function getExplorerUrl(path: string, type: "address" | "tx" = "tx"): string {This already has the return type
: string. So this is correct. Let me review sendSignedTransaction instead.
593-617: Add explicit return type annotation to helper function.The
createTransferTransactionhelper function should have an explicit return type annotation per TypeScript coding guidelines.Apply this diff:
const createTransferTransaction = async ( from: string, to: string, amount: bigint, - ) => { + ): Promise<string> => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/vault-sdk/tests/env-setup.ts(1 hunks)packages/vault-sdk/tests/solana.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/vault-sdk/tests/env-setup.tspackages/vault-sdk/tests/solana.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/vault-sdk/tests/env-setup.tspackages/vault-sdk/tests/solana.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/vault-sdk/tests/solana.test.ts
🧬 Code graph analysis (1)
packages/vault-sdk/tests/solana.test.ts (1)
packages/vault-sdk/src/sdk.ts (7)
VaultClient(116-120)createVaultClient(122-176)createServiceAccount(285-296)createSolanaAccount(463-474)createAccessToken(376-387)signSolanaTransaction(489-500)reconstructSolanaSignedTransaction(505-536)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
packages/vault-sdk/tests/env-setup.ts (1)
1-6: LGTM! File-relative path implementation is correct.The implementation correctly uses
import.meta.urlto derive__dirname, ensuring the.envfile is found regardless of the current working directory. This addresses the concern from the previous review.packages/vault-sdk/tests/solana.test.ts (9)
1-31: LGTM! Imports are well-organized.All imports are necessary and properly organized by source. The imports align with the test requirements for Solana integration testing.
33-39: LGTM! Environment setup is appropriate.Environment variables have sensible fallbacks for local development, and the constant is properly defined. The
.envfile is loaded via the test setup file.
52-69: LGTM! Helper function implementation is sound.The function properly converts the signed transaction bytes to base64 and sends it to the network. The type assertion on line 61 is necessary to satisfy the RPC method's type requirements.
81-161: LGTM! Test setup is comprehensive.The
beforeAllhook properly initializes all required test fixtures: RPC connections, vault client, accounts, and airdrop funding. The console logging aids in debugging and traceability. The 60-second timeout is appropriate for network operations.
163-186: LGTM! Basic validation tests are appropriate.The health check and account validation tests provide essential baseline verification before more complex operations.
188-313: LGTM! Access token tests validate authorization properly.Both positive and negative test cases thoroughly validate that access token permissions are correctly enforced for account creation operations.
315-403: LGTM! Transaction creation and signing tests are comprehensive.The tests properly validate transaction creation, signing flow, and signature format. The signature validation including the 64-byte length check is thorough.
405-589: LGTM! End-to-end tests validate complete transaction flows.The tests thoroughly exercise the full transaction lifecycle including creation, signing, broadcasting, and on-chain verification. The sequential transfer test with fresh blockhashes is particularly valuable for ensuring reliability under repeated operations.
619-1699: LGTM! Comprehensive policy enforcement test suite.The test suite thoroughly validates all transaction pattern policies including program access control, fee payment permissions, instruction limits, writable account restrictions, cosigner requirements, and allowlists. The tests cover both positive and negative cases, and the organization with nested
describeblocks makes the test structure very clear.
d4mr
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/vault-sdk/tests/solana.test.ts (2)
317-317: Consider extracting the transfer amount constant.The value
lamports(100_000_000n)(0.1 SOL) is repeated multiple times throughout the tests. Consider extracting it as a named constant at the top of the file for better maintainability.Apply this diff to add a constant:
const SOLANA_RPC_HTTP_URL = process.env.SOLANA_RPC_HTTP_URL || "http://127.0.0.1:8899"; const SOLANA_RPC_WS_URL = process.env.SOLANA_RPC_WS_URL || "ws://127.0.0.1:8900"; const VAULT_URL = process.env.VAULT_URL || "http://localhost:3000"; const LAMPORTS_PER_SOL = 1_000_000_000n; +const TEST_TRANSFER_AMOUNT = lamports(100_000_000n); // 0.1 SOLThen replace occurrences:
- const transferAmount = lamports(100_000_000n); + const transferAmount = TEST_TRANSFER_AMOUNT;Also applies to: 351-351, 407-407, 513-513
160-160: Consider documenting the purpose of wait times.The hardcoded sleep durations (2000ms, 3000ms) would benefit from brief comments explaining why these specific values were chosen (e.g., "Wait for airdrop confirmation", "Wait for transaction finality").
Example:
- await new Promise((resolve) => setTimeout(resolve, 2000)); + // Wait for airdrop to confirm on-chain + await new Promise((resolve) => setTimeout(resolve, 2000));Also applies to: 484-484, 579-579
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/vault-sdk/tests/env-setup.ts(1 hunks)packages/vault-sdk/tests/solana.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/vault-sdk/tests/env-setup.tspackages/vault-sdk/tests/solana.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/vault-sdk/tests/env-setup.tspackages/vault-sdk/tests/solana.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/vault-sdk/tests/solana.test.ts
🧬 Code graph analysis (1)
packages/vault-sdk/tests/solana.test.ts (1)
packages/vault-sdk/src/sdk.ts (7)
VaultClient(116-120)createVaultClient(122-176)createServiceAccount(285-296)createSolanaAccount(463-474)createAccessToken(376-387)signSolanaTransaction(489-500)reconstructSolanaSignedTransaction(505-536)
⏰ 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). (8)
- GitHub Check: Analyze (javascript)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
🔇 Additional comments (9)
packages/vault-sdk/tests/env-setup.ts (1)
1-6: LGTM! File-relative path resolution implemented correctly.The implementation properly uses
import.meta.urlto resolve the.envfile path relative to the module, addressing the previous feedback aboutcwd()reliability. This ensures the environment file is loaded correctly regardless of the working directory.packages/vault-sdk/tests/solana.test.ts (8)
1-39: LGTM! Imports and configuration are appropriate.The imports are well-organized, environment variables have sensible fallbacks, and the constant definitions are clear.
41-69: LGTM! Helper functions are well-implemented.Both
getExplorerUrlandsendSignedTransactionare correctly implemented with proper encoding and configuration options.
81-161: LGTM! Comprehensive test setup with appropriate resource provisioning.The
beforeAllhook properly initializes the test environment, creates necessary accounts, and provisions sufficient SOL (3 SOL) for all test scenarios. The 60-second timeout is reasonable for network operations.
163-313: LGTM! Access token tests comprehensively validate permission enforcement.The tests properly verify both successful account creation with appropriate permissions and rejection when permissions are missing.
315-403: LGTM! Transaction creation and signing tests are thorough.The tests validate transaction structure, signature format, and byte length expectations.
405-508: LGTM! End-to-end transfer test validates the complete flow.The test properly verifies transaction creation, vault signing, broadcasting, and on-chain confirmation with balance validation.
510-589: LGTM! Multiple transfers test validates sequential transaction handling.The test correctly obtains fresh blockhashes for each transaction and validates all signatures.
591-1699: LGTM! Comprehensive policy enforcement tests cover all transaction pattern controls.The extensive test suite thoroughly validates:
- Program allowlists and denylists
- Fee payment authorization
- Instruction count limits
- Combined policy enforcement
- Writable account controls
- Cosigner requirements
- Account allowlists
Each policy mechanism is tested for both allow and deny scenarios, ensuring robust access control validation.
size-limit report 📦
|
2 participants
PR-Codex overview
This PR focuses on adding support for Vault operations on the Solana blockchain, enhancing the
vault-sdkwith new functionalities related to Solana account management and transaction signing.Detailed summary
package.jsonto include Solana dependencies..env.examplefor Solana RPC configuration.thirdweb.tsfor creating and managing Solana accounts.sdk.tswith Solana-specific transaction and message signing functions.solana.test.ts.types.tsfor Solana account and transaction payloads.vitest.config.tsfor testing environment adjustments.Summary by CodeRabbit
New Features
Tests
Chores
Other