sdk/compat: follow-up to #573 and residual review cleanup#582
sdk/compat: follow-up to #573 and residual review cleanup#582
Conversation
📝 WalkthroughWalkthroughReplaces compat-layer auth/RPC stubs with implemented compatibility helpers: structured auth request/verify builders, EIP‑712 signer factory, and active request signing; adds RPC transaction/auth-related types, refactors client asset/error flows, and updates docs/examples to reflect helpers and expanded examples. Changes
Sequence DiagramssequenceDiagram
participant Client
participant Auth as Auth Helper
participant Signer as MessageSigner
participant RPC as NitroliteRPC
Client->>Auth: createAuthRequestMessage(params)
Auth->>Auth: build RPC request (method, params, requestId, timestamp)
Auth-->>Client: return serialized request
Client->>Auth: createAuthVerifyMessage(signer, challenge)
Auth->>Signer: ask to sign msg.req
Signer-->>Auth: signature
Auth->>RPC: signRequestMessage(msg, signer)
RPC->>RPC: compute & attach sig (sig populated)
RPC-->>Auth: signed message
Auth-->>Client: JSON payload
sequenceDiagram
participant Client
participant Factory as EIP712AuthMessageSigner
participant Wallet as WalletClient
participant Validator
Client->>Factory: createEIP712AuthMessageSigner(walletClient, partialMsg, domain)
Factory-->>Client: return MessageSigner
Client->>Factory: invoke MessageSigner(message)
Factory->>Validator: validate method & payload
Validator-->>Factory: validation result
Factory->>Wallet: walletClient.signTypedData(enriched message)
Wallet-->>Factory: signature
Factory-->>Client: signature or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ihsraham, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the compatibility SDK by implementing previously stubbed authentication helpers and cleaning up client-side logic. It also updates documentation to accurately reflect the finalized behavior introduced in a previous PR, ensuring clarity and correctness for developers migrating to the new SDK version. The changes aim to improve the robustness and maintainability of the compatibility layer. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a follow-up to a previous one, primarily focused on replacing auth stubs with real implementations and cleaning up documentation and code. The changes in sdk/compat/src/auth.ts are significant, introducing v0.5.3-compatible authentication logic. Other changes include various cleanups and refactorings in sdk/compat/src/client.ts and updates to RPC helpers and types. My review focuses on potential issues with data types, type safety, and maintainability in the new implementations.
| return JSON.stringify(request, (_, value) => | ||
| typeof value === 'bigint' ? Number(value) : value, | ||
| ); |
There was a problem hiding this comment.
The replacer function in JSON.stringify converts bigint values to Number. This can lead to a loss of precision for large bigint values that exceed Number.MAX_SAFE_INTEGER. While this may be acceptable for timestamps for the near future, it could become a problem later or for other bigint values. To preserve precision, consider converting bigint to a string if the receiving API supports it.
| return JSON.stringify(request, (_, value) => | |
| typeof value === 'bigint' ? Number(value) : value, | |
| ); | |
| return JSON.stringify(request, (_, value) => | |
| typeof value === 'bigint' ? value.toString() : value, | |
| ); |
| _domain: any, | ||
| ): any { | ||
| return async () => '0x'; | ||
| walletClient: any, |
There was a problem hiding this comment.
The walletClient parameter is typed as any, which reduces type safety and makes the code harder to maintain. It appears to be a viem WalletClient. Please provide a specific type for it to improve maintainability and enable static analysis. You will likely need to add import type { WalletClient } from 'viem'; at the top of the file.
| async signRequestMessage(msg: NitroliteRPCMessage, _signer: MessageSigner): Promise<NitroliteRPCMessage> { | ||
| return msg; | ||
| async signRequestMessage(msg: NitroliteRPCMessage, signer: MessageSigner): Promise<NitroliteRPCMessage> { | ||
| const signature = await signer(msg.req as any); |
There was a problem hiding this comment.
The use of msg.req as any to call the signer bypasses a type mismatch. The MessageSigner type expects a Uint8Array, but msg.req is a tuple [number, string, any, number]. This any cast reduces type safety and should be addressed, perhaps by updating the MessageSigner type to reflect its actual usage throughout the compatibility layer.
There was a problem hiding this comment.
Code Review
This pull request is a follow-up that implements feedback from a previous review, primarily by replacing authentication stubs with functional compatibility helpers and cleaning up related documentation and code. The changes are logical and significantly improve the compatibility layer's functionality. I've identified one area for improvement regarding type-safety with MessageSigner that is currently bypassed using a type assertion. Addressing this point will enhance code quality and robustness. Overall, this is a solid contribution.
| async signRequestMessage(msg: NitroliteRPCMessage, _signer: MessageSigner): Promise<NitroliteRPCMessage> { | ||
| return msg; | ||
| async signRequestMessage(msg: NitroliteRPCMessage, signer: MessageSigner): Promise<NitroliteRPCMessage> { | ||
| const signature = await signer(msg.req as any); |
There was a problem hiding this comment.
The as any cast here hides a type mismatch between the MessageSigner type and its usage. MessageSigner is typed as (payload: Uint8Array) => Promise<string>, but it's being called with msg.req, which is of type [number, string, any, number].
For better type safety and to remove the need for this cast, the MessageSigner type definition in sdk/compat/src/types.ts should be updated to reflect its actual usage. For example:
export type MessageSigner = (payload: NitroliteRPCMessage['req']) => Promise<string>;Or a union type could be used if the Uint8Array payload is still required elsewhere.
![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
♻️ Duplicate comments (1)
sdk/compat/src/auth.ts (1)
83-99: Payload is accessed as a tuple despiteMessageSignertyping it asUint8Array.
payload[1]andpayload[2]assume the tuple structure[requestId, method, params, timestamp], confirming theMessageSignertype mismatch flagged inrpc.ts. This signer will only work when called throughNitroliteRPC.signRequestMessage(which passesmsg.req as any), not through any code path that actually passes aUint8Array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 83 - 99, The signer in auth.ts treats the incoming payload as a tuple (accessing payload[1], payload[2]) but MessageSigner is typed as Uint8Array elsewhere; fix by aligning types and parsing: update the MessageSigner typing (or the specific signer signature returned by the async function) to accept the tuple shape [requestId, method, params, timestamp] used by NitroliteRPC.signRequestMessage, or if you must accept a Uint8Array decode/deserialize it into that tuple before accessing indices; ensure the signer function (the returned async (payload: any) => { ... }) and rpc.ts/NitroliteRPC.signRequestMessage agree on the concrete type so payload[1]/payload[2] are valid and keep the existing validation of method and params.challenge.
🧹 Nitpick comments (6)
sdk/compat/src/auth.ts (5)
107-120: Error wrapping re-throws a newError, discarding the original stack trace.On line 119, a new
Erroris thrown with the message concatenated, which loses the original error's stack trace. Consider preserving causality:♻️ Preserve original error as cause
- throw new Error(`EIP-712 signing failed: ${errorMessage}`); + throw new Error(`EIP-712 signing failed: ${errorMessage}`, { cause: err });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 107 - 120, The catch block around walletClient.signTypedData currently throws a new Error which discards the original stack; instead preserve causality by rethrowing the original error or constructing the new Error with the original error as its cause (e.g., new Error(`EIP-712 signing failed: ${errorMessage}`, { cause: err })), so update the catch in auth.ts (the try calling walletClient.signTypedData with EIP712AuthTypes and primaryType 'Policy') to include the original error as the cause or rethrow err after logging.
73-82:walletClient: anybypasses type safety — preferWalletClientfrom viem.The rest of the codebase (e.g.,
client.tsline 13) properly imports and typesWalletClientfrom viem. Usinganyhere loses type checking onwalletClient.account,walletClient.signTypedData, etc.♻️ Suggested fix
+import type { WalletClient } from 'viem'; + export function createEIP712AuthMessageSigner( - walletClient: any, + walletClient: WalletClient, partialMessage: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 73 - 82, Replace the loose walletClient: any in createEIP712AuthMessageSigner with the proper WalletClient type from viem: import WalletClient and change the function signature to walletClient: WalletClient so TypeScript enforces walletClient.account, walletClient.signTypedData, etc.; ensure any uses of account/address and signTypedData in createEIP712AuthMessageSigner match the WalletClient API and adjust types for return values or casts if necessary.
21-23:generateRequestIdmixes timestamp with small random offset — collision-prone under burst traffic.For concurrent requests within the same millisecond window, the random offset (0–9999) added to
Date.now()provides limited uniqueness. This is likely acceptable for auth flows (low frequency), but if this helper is reused for high-throughput RPC calls, collisions are plausible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 21 - 23, The generateRequestId function's current approach (Date.now() + small Math.random() offset) can collide under burst traffic; replace it with a stronger unique-id strategy: use a UUID (crypto.randomUUID()) returning a string, or if UUID isn't available, compose a string from Date.now() plus cryptographically-secure random bytes (via crypto.getRandomValues/crypto.randomBytes) or a process-wide monotonic counter to guarantee uniqueness; update generateRequestId's return type and any callers in auth.ts to accept a string ID if switching from number to string.
25-39:createAuthRequestMessageisasyncbut contains noawait.The function is marked
asyncbut performs no asynchronous operations — it synchronously builds and serializes the message. While this isn't a bug (the return typePromise<string>is still honored), it's slightly misleading. If theasyncis intentional for API consistency with othercreate*functions, that's fine to keep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 25 - 39, The function createAuthRequestMessage is marked async but does no asynchronous work; remove the async keyword and change its return type from Promise<string> to string (update the signature and any callers if they relied on a Promise), so it synchronously builds and returns the serialized request string; alternatively, if API consistency is required, keep async but add a comment explaining why it's intentionally async—prefer removing async and updating the signature in createAuthRequestMessage.
36-38: Align bigint serialization with codebase convention.The
expires_atfield (a Unix timestamp) is safe as aNumbersince it will never exceedNumber.MAX_SAFE_INTEGER. However, the rest of the codebase usesvalue.toString()for bigint serialization (sdk/ts/examples/example-app/src/utils.tsand integration tests). Update this file to match the preferred pattern for consistency:♻️ Align with codebase convention
return JSON.stringify(request, (_, value) => - typeof value === 'bigint' ? Number(value) : value, + typeof value === 'bigint' ? value.toString() : value, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/auth.ts` around lines 36 - 38, Change the bigint-to-primitive serialization used in the JSON.stringify call so it matches the codebase convention: replace the current conversion that uses Number(value) with value.toString() (i.e., when typeof value === 'bigint' return value.toString()). Keep in mind the expires_at Unix timestamp remains safe as a Number elsewhere, but update the JSON.stringify callback in sdk/compat/src/auth.ts (the anonymous replacer passed to JSON.stringify) to use toString() for consistency with other code and tests.sdk/compat/src/types.ts (1)
343-343:LedgerAccountTypeis redundant —Addressis already`0x${string}`.viem's
Addresstype is defined as`0x${string}`, soAddress | \0x${string}`simplifies to justAddress. If the intent is to accept non-checksummed hex strings,Address` already covers that. Consider simplifying:♻️ Simplify type
-export type LedgerAccountType = Address | `0x${string}`; +export type LedgerAccountType = Address;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/src/types.ts` at line 343, The type alias LedgerAccountType is redundant because viem's Address is already the template literal `0x${string}`; replace the current declaration export type LedgerAccountType = Address | `0x${string}`; with export type LedgerAccountType = Address (or remove the alias if unused) and update any usages expecting the union to accept Address; ensure imports for Address remain and run type checks to confirm no regressions in places referencing LedgerAccountType.
🤖 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/compat/src/rpc.ts`:
- Around line 47-50: The MessageSigner type is incorrect for how
signRequestMessage calls it with msg.req (a tuple) and how
createEIP712AuthMessageSigner expects tuple indices; update the MessageSigner
definition in types.ts to accept the actual request shape (e.g., (payload:
Uint8Array | [number, string, any, number]) => Promise<string> or better,
(payload: Uint8Array | NitroliteRPCRequest) => Promise<string>) so
createECDSAMessageSigner and createEIP712AuthMessageSigner conform to the same
contract, and ensure signRequestMessage calls signer with the matching type (or
alternatively serialize msg.req to Uint8Array here and update both signer
implementations to consume bytes).
In `@sdk/compat/src/types.ts`:
- Around line 345-355: The RPCTransaction interface declares createdAt as Date
but JSON RPC responses deliver date fields as strings; update the RPCTransaction
type for createdAt to accept string | Date (or switch to string if you prefer to
enforce explicit parsing elsewhere) and adjust any code that consumes
RPCTransaction (e.g., deserialization/validation paths) to parse the string into
a Date when needed so runtime values match the TypeScript type; reference
RPCTransaction and the createdAt property when making this change.
---
Duplicate comments:
In `@sdk/compat/src/auth.ts`:
- Around line 83-99: The signer in auth.ts treats the incoming payload as a
tuple (accessing payload[1], payload[2]) but MessageSigner is typed as
Uint8Array elsewhere; fix by aligning types and parsing: update the
MessageSigner typing (or the specific signer signature returned by the async
function) to accept the tuple shape [requestId, method, params, timestamp] used
by NitroliteRPC.signRequestMessage, or if you must accept a Uint8Array
decode/deserialize it into that tuple before accessing indices; ensure the
signer function (the returned async (payload: any) => { ... }) and
rpc.ts/NitroliteRPC.signRequestMessage agree on the concrete type so
payload[1]/payload[2] are valid and keep the existing validation of method and
params.challenge.
---
Nitpick comments:
In `@sdk/compat/src/auth.ts`:
- Around line 107-120: The catch block around walletClient.signTypedData
currently throws a new Error which discards the original stack; instead preserve
causality by rethrowing the original error or constructing the new Error with
the original error as its cause (e.g., new Error(`EIP-712 signing failed:
${errorMessage}`, { cause: err })), so update the catch in auth.ts (the try
calling walletClient.signTypedData with EIP712AuthTypes and primaryType
'Policy') to include the original error as the cause or rethrow err after
logging.
- Around line 73-82: Replace the loose walletClient: any in
createEIP712AuthMessageSigner with the proper WalletClient type from viem:
import WalletClient and change the function signature to walletClient:
WalletClient so TypeScript enforces walletClient.account,
walletClient.signTypedData, etc.; ensure any uses of account/address and
signTypedData in createEIP712AuthMessageSigner match the WalletClient API and
adjust types for return values or casts if necessary.
- Around line 21-23: The generateRequestId function's current approach
(Date.now() + small Math.random() offset) can collide under burst traffic;
replace it with a stronger unique-id strategy: use a UUID (crypto.randomUUID())
returning a string, or if UUID isn't available, compose a string from Date.now()
plus cryptographically-secure random bytes (via
crypto.getRandomValues/crypto.randomBytes) or a process-wide monotonic counter
to guarantee uniqueness; update generateRequestId's return type and any callers
in auth.ts to accept a string ID if switching from number to string.
- Around line 25-39: The function createAuthRequestMessage is marked async but
does no asynchronous work; remove the async keyword and change its return type
from Promise<string> to string (update the signature and any callers if they
relied on a Promise), so it synchronously builds and returns the serialized
request string; alternatively, if API consistency is required, keep async but
add a comment explaining why it's intentionally async—prefer removing async and
updating the signature in createAuthRequestMessage.
- Around line 36-38: Change the bigint-to-primitive serialization used in the
JSON.stringify call so it matches the codebase convention: replace the current
conversion that uses Number(value) with value.toString() (i.e., when typeof
value === 'bigint' return value.toString()). Keep in mind the expires_at Unix
timestamp remains safe as a Number elsewhere, but update the JSON.stringify
callback in sdk/compat/src/auth.ts (the anonymous replacer passed to
JSON.stringify) to use toString() for consistency with other code and tests.
In `@sdk/compat/src/types.ts`:
- Line 343: The type alias LedgerAccountType is redundant because viem's Address
is already the template literal `0x${string}`; replace the current declaration
export type LedgerAccountType = Address | `0x${string}`; with export type
LedgerAccountType = Address (or remove the alias if unused) and update any
usages expecting the union to accept Address; ensure imports for Address remain
and run type checks to confirm no regressions in places referencing
LedgerAccountType.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
sdk/compat/docs/migration-offchain.mdsdk/compat/docs/migration-overview.mdsdk/compat/src/auth.tssdk/compat/src/client.tssdk/compat/src/index.tssdk/compat/src/rpc.tssdk/compat/src/types.ts
| async signRequestMessage(msg: NitroliteRPCMessage, signer: MessageSigner): Promise<NitroliteRPCMessage> { | ||
| const signature = await signer(msg.req as any); | ||
| return { ...msg, sig: signature }; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts -C3 'MessageSigner' --glob '!node_modules/**'Repository: erc7824/nitrolite
Length of output: 11305
🏁 Script executed:
# Get the complete implementation of createEIP712AuthMessageSigner
sed -n '73,100p' sdk/compat/src/auth.ts
# Get NitroliteRPCMessage type definition
rg -n 'interface NitroliteRPCMessage|type NitroliteRPCMessage' -A 5 sdk/compat/src/types.ts
# Check what msg.req actually contains and how it's structured
rg -n 'msg\.req' sdk/compat/src/ -B 2 -A 2Repository: erc7824/nitrolite
Length of output: 1558
🏁 Script executed:
# Check how createECDSAMessageSigner is used in tests
rg -n 'createECDSAMessageSigner' -A 5 -B 1 test/integration/
# Check if createECDSAMessageSigner is used with signRequestMessage
rg -n 'signRequestMessage' sdk/compat/src/ -B 5 -A 5
# Check if createECDSAMessageSigner payload parameter is actually invoked
rg -n 'messageSKSigner\|messageWalletSigner' test/integration/common/ -B 2 -A 2Repository: erc7824/nitrolite
Length of output: 3712
🏁 Script executed:
# Search for usage of messageSKSigner and messageWalletSigner
rg -n 'messageSKSigner|messageWalletSigner' test/integration/ -B 2 -A 3
# Find and examine createCleanupSessionKeyCacheMessage
rg -n 'createCleanupSessionKeyCacheMessage|export.*createCleanupSessionKeyCacheMessage' -A 10 sdk/compat/src/Repository: erc7824/nitrolite
Length of output: 34669
🏁 Script executed:
# Get full createECDSAMessageSigner implementation
sed -n '34,50p' sdk/compat/src/signers.tsRepository: erc7824/nitrolite
Length of output: 326
Type mismatch: MessageSigner signature does not match actual usage.
MessageSigner is defined as (payload: Uint8Array) => Promise<string>, but signRequestMessage calls it with msg.req (a [number, string, any, number] tuple), hidden by an as any cast.
This creates a contract violation between implementations:
createECDSAMessageSignercorrectly implements the type as(payload: Uint8Array)and callsaccount.signMessage({ message: { raw: payload } }), expecting bytes.createEIP712AuthMessageSignerviolates the type by declaring(payload: any)and accessingpayload[1]andpayload[2]as tuple indices.
When createECDSAMessageSigner is passed to signRequestMessage (as seen in create_channel.test.ts:82 and other tests), it receives the tuple instead of Uint8Array, leading to incorrect behavior.
Recommended fix: Update MessageSigner to reflect the actual contract:
♻️ Suggested type fix in types.ts
-export type MessageSigner = (payload: Uint8Array) => Promise<string>;
+export type MessageSigner = (payload: any) => Promise<string>;This aligns the type with how both implementations and callers actually use it. Alternatively, serialize msg.req to Uint8Array before signing and update implementations to deserialize.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/compat/src/rpc.ts` around lines 47 - 50, The MessageSigner type is
incorrect for how signRequestMessage calls it with msg.req (a tuple) and how
createEIP712AuthMessageSigner expects tuple indices; update the MessageSigner
definition in types.ts to accept the actual request shape (e.g., (payload:
Uint8Array | [number, string, any, number]) => Promise<string> or better,
(payload: Uint8Array | NitroliteRPCRequest) => Promise<string>) so
createECDSAMessageSigner and createEIP712AuthMessageSigner conform to the same
contract, and ensure signRequestMessage calls signer with the matching type (or
alternatively serialize msg.req to Uint8Array here and update both signer
implementations to consume bytes).
| export interface RPCTransaction { | ||
| id: number; | ||
| txType: RPCTxType; | ||
| fromAccount: LedgerAccountType; | ||
| fromAccountTag?: string; | ||
| toAccount: LedgerAccountType; | ||
| toAccountTag?: string; | ||
| asset: string; | ||
| amount: string; | ||
| createdAt: Date; | ||
| } |
There was a problem hiding this comment.
createdAt: Date may cause issues when deserializing from JSON.
JSON payloads deliver dates as strings, not Date objects. If RPCTransaction is used to type raw RPC responses, consumers will get a string at runtime while TypeScript believes it's a Date. Consider using string | Date or string with a note that consumers should convert, unless this type is only used after explicit deserialization logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/compat/src/types.ts` around lines 345 - 355, The RPCTransaction interface
declares createdAt as Date but JSON RPC responses deliver date fields as
strings; update the RPCTransaction type for createdAt to accept string | Date
(or switch to string if you prefer to enforce explicit parsing elsewhere) and
adjust any code that consumes RPCTransaction (e.g., deserialization/validation
paths) to parse the string into a Date when needed so runtime values match the
TypeScript type; reference RPCTransaction and the createdAt property when making
this change.
- expand examples/lifecycle.ts into a comprehensive compat API walkthrough\n- add examples/tsconfig.json and examples/output.txt for runnable setup + expected output\n- add Build Size metrics to sdk/compat/README.md from build:prod + npm pack --dry-run\n- add tsx dev dependency updates required to run the example workflow
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if (channels.length > 0) { | ||
| try { | ||
| const ch = channels[0]; | ||
| const data = await client.getChannelData(ch.channel_id); |
| console.log('\n-- 10. Signer Helpers --'); | ||
|
|
||
| try { | ||
| const wss = new WalletStateSigner(walletClient); |
|
|
||
| try { | ||
| const sessionKey = generatePrivateKey(); | ||
| const sessionSigner = createECDSAMessageSigner(sessionKey); |
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/compat/examples/lifecycle.ts (1)
102-138:⚠️ Potential issue | 🟡 MinorAlign wallet client chain with
CHAIN_ID(currently fixed to Sepolia).
Line 134 hard-codessepoliawhile Line 104 allows arbitraryCHAIN_ID, so running on non‑Sepolia will build clients against the wrong chain/RPC. Either guard against non‑Sepolia values or mapCHAIN_IDto the correct viem chain.🛠️ Suggested guard to avoid mismatched chains
const CHAIN_ID = parseInt(process.env.CHAIN_ID || '11155111', 10); if (Number.isNaN(CHAIN_ID)) { console.error('CHAIN_ID must be a valid integer'); process.exit(1); } +if (CHAIN_ID !== sepolia.id) { + console.error(`This example currently targets Sepolia only (CHAIN_ID=${sepolia.id}).`); + process.exit(1); +}#!/bin/bash # Verify whether the compat client relies on walletClient.chain or chainId internally. rg -n "walletClient" sdk/compat/src/client.ts -C3 rg -n "chainId" sdk/compat/src/client.ts -C3Also applies to: 395-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/examples/lifecycle.ts` around lines 102 - 138, The code hard-codes the viem chain "sepolia" when calling createWalletClient in main (and similar places), which can mismatch the parsed CHAIN_ID; update the logic to derive the viem chain from CHAIN_ID or fail fast: either map CHAIN_ID to the appropriate viem chain object (e.g., look up by id from known chains or a helper like getChainById) and pass that chain to createWalletClient, or check if CHAIN_ID === sepolia.id and process.exit(1) with an error if not supported; adjust the createWalletClient calls (and any other walletClient creation sites) to use the resolved chain variable instead of the hard-coded sepolia.
🧹 Nitpick comments (2)
sdk/compat/package.json (1)
46-46: LGTM —tsxcorrectly placed as adevDependency.
tsx(TypeScript Execute, Node.js enhanced with esbuild to run TypeScript & ESM files) is at4.21.0on npm, so^4.21.0is pinned to the current latest — valid. The package is excluded from the published artifact sincefilesonly includesdist/andREADME.md.One optional nicety: since
tsxis added specifically to runexamples/lifecycle.ts(per the commit message) but noscriptsentry wraps it, contributors must discover the invocation on their own. Consider adding a convenience script:✨ Optional: add a convenience example script
"scripts": { "build": "tsc", "build:prod": "tsc -p tsconfig.prod.json", "typecheck": "tsc --noEmit", + "example": "tsx examples/lifecycle.ts", "clean": "node -e \"require('fs').rmSync('dist',{recursive:true,force:true})\"" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/package.json` at line 46, Add a convenience npm script to package.json so contributors can run the lifecycle example easily: add a new script key (e.g., "example:lifecycle") that invokes the installed devDependency "tsx" to run "examples/lifecycle.ts" (use the project-local tsx binary so it works for contributors without global installs); update the "scripts" object in package.json to include this new entry.sdk/compat/examples/lifecycle.ts (1)
345-373: Use proper types from@erc7824/nitroliteinstead ofas anyto keep the example aligned with API changes.The inline objects passed to
client.innerClient.createAppSession()andclient.innerClient.submitAppState()matchAppDefinitionV1andAppStateUpdateV1structures respectively. Replace theas anycasts with proper typed objects:import type { AppDefinitionV1, AppStateUpdateV1, AppParticipantV1, AppAllocationV1 } from '@erc7824/nitrolite'; // For createAppSession: const definition: AppDefinitionV1 = { application: appName, participants: appParticipants as AppParticipantV1[], quorum: appQuorum, nonce: appNonce, }; await client.innerClient.createAppSession(definition, appSessionData, [sig]); // For submitAppState: const appUpdate: AppStateUpdateV1 = { appSessionId: createdSessionId, intent: 0, version: 2n, allocations: [], sessionData: '{"round":1}', }; await client.innerClient.submitAppState(appUpdate, [operateSig]);This ensures the example breaks at compile-time if the API contract changes, keeping it in sync with the underlying SDK surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/compat/examples/lifecycle.ts` around lines 345 - 373, Replace the loose any casts around the objects passed to client.innerClient.createAppSession and client.innerClient.submitAppState with proper typed values from `@erc7824/nitrolite`: import AppDefinitionV1, AppStateUpdateV1, AppParticipantV1, AppAllocationV1 and build a const definition: AppDefinitionV1 = { application: appName, participants: appParticipants as AppParticipantV1[], quorum: appQuorum, nonce: appNonce } and a const appUpdate: AppStateUpdateV1 = { appSessionId: createdSessionId as Hex, intent: 0, version: 2n, allocations: [] as AppAllocationV1[], sessionData: '{"round":1}' } (and similarly for the close update), then pass those typed variables into createAppSession(...) and submitAppState(...) instead of using as any.
🤖 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/compat/examples/lifecycle.ts`:
- Around line 102-138: The code hard-codes the viem chain "sepolia" when calling
createWalletClient in main (and similar places), which can mismatch the parsed
CHAIN_ID; update the logic to derive the viem chain from CHAIN_ID or fail fast:
either map CHAIN_ID to the appropriate viem chain object (e.g., look up by id
from known chains or a helper like getChainById) and pass that chain to
createWalletClient, or check if CHAIN_ID === sepolia.id and process.exit(1) with
an error if not supported; adjust the createWalletClient calls (and any other
walletClient creation sites) to use the resolved chain variable instead of the
hard-coded sepolia.
---
Nitpick comments:
In `@sdk/compat/examples/lifecycle.ts`:
- Around line 345-373: Replace the loose any casts around the objects passed to
client.innerClient.createAppSession and client.innerClient.submitAppState with
proper typed values from `@erc7824/nitrolite`: import AppDefinitionV1,
AppStateUpdateV1, AppParticipantV1, AppAllocationV1 and build a const
definition: AppDefinitionV1 = { application: appName, participants:
appParticipants as AppParticipantV1[], quorum: appQuorum, nonce: appNonce } and
a const appUpdate: AppStateUpdateV1 = { appSessionId: createdSessionId as Hex,
intent: 0, version: 2n, allocations: [] as AppAllocationV1[], sessionData:
'{"round":1}' } (and similarly for the close update), then pass those typed
variables into createAppSession(...) and submitAppState(...) instead of using as
any.
In `@sdk/compat/package.json`:
- Line 46: Add a convenience npm script to package.json so contributors can run
the lifecycle example easily: add a new script key (e.g., "example:lifecycle")
that invokes the installed devDependency "tsx" to run "examples/lifecycle.ts"
(use the project-local tsx binary so it works for contributors without global
installs); update the "scripts" object in package.json to include this new
entry.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
sdk/compat/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
sdk/compat/README.mdsdk/compat/examples/lifecycle.tssdk/compat/examples/output.txtsdk/compat/examples/tsconfig.jsonsdk/compat/package.json
✅ Files skipped from review due to trivial changes (2)
- sdk/compat/examples/tsconfig.json
- sdk/compat/examples/output.txt
1 participant
Context
Follow-up to PR #573 (
feat: get_channels RPC, GetAppSessions fix, compat SDK & tests).This PR applies the remaining review feedback/residual comments on the compat SDK and aligns docs/comments with the finalized behavior introduced after #573.
Summary
sdk/compat/src/auth.ts.sdk/compat/src/client.ts.sdk/compat/src/index.tssdk/compat/src/rpc.tssdk/compat/src/types.tsstub/no-oplanguage with accuratecompatibility helperwording in:sdk/compat/src/index.tssdk/compat/src/rpc.tssdk/compat/docs/migration-overview.mdsdk/compat/docs/migration-offchain.mdValidation
cd sdk/compat && npm run typecheckcd appstore && npm run buildcd beatwav && npm run buildcd nexus && npm run buildSummary by CodeRabbit
New Features
Documentation
Improvements
Chores
Additions in this update (February 24, 2026)
Build Sizesection tosdk/compat/README.mdwith measurednpm pack --dry-runanddistbyte stats.sdk/compat/examplesbundle:lifecycle.ts: comprehensive lifecycle walkthrough covering the compat API surface (client lifecycle, assets, balances/channels, transfers, app sessions including multi-party flow,EventPoller, error classification, signers, enums, and inner client access).tsconfig.json: standalone example compiler config with local path aliases.output.txt: expected console output template for quick verification.tsxtosdk/compatdev dependencies so the lifecycle example can be run directly.Validation for this update
cd sdk/compat && npm run build:prodcd sdk/compat && npx tsc -p examples/tsconfig.json --noEmit