YNU-864: Build front door rewrite#147
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR completes the Nitrolite v1 documentation migration: replaces v0.5.x terminology with v1 concepts, rewrites prerequisites and quickstart guides, adds a comprehensive runnable lifecycle example, and updates site navigation accordingly. ChangesNitrolite v1 Documentation & Lifecycle Example
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
09b8a89 to
69b0bbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
examples/nitrolite-v1-lifecycle/package.json (1)
8-9: ⚡ Quick winAlign Node typings with the declared runtime floor.
With
engines.nodeat>=20.0.0, using@types/node24 can allow Node 24–specific APIs to type-check successfully on the Node 20 runtime. Prefer@types/node@^20to match the minimum supported version (or raiseengines.nodeif Node 24 is the intent).Suggested change
"devDependencies": { - "@types/node": "^24.12.2", + "@types/node": "^20.17.0", "typescript": "~6.0.2" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nitrolite-v1-lifecycle/package.json` around lines 8 - 9, The engines.node entry is set to ">=20.0.0" but the package should not pull `@types/node` that target newer Node major versions; update package.json to pin the devDependency "@types/node" to a ^20 range (or alternatively raise "engines.node" to ^24 if Node 24 is intended) so type definitions align with the declared runtime floor—look for the "engines.node" field and the "@types/node" entry in package.json to make this change.docs/nitrolite/build/getting-started/quickstart.mdx (1)
117-130: 💤 Low valueHardcoded
version: 2nmay confuse readers.The snippet uses
version: 2nliterally, while the runnable example computes the next version withnextVersion(session). Readers copying this fragment without referring back to the example may submit a stale version after the first iteration. Consider replacing withnextVersion(session)(orsession.version + 1n) and a brief note that allocations must reflect the previous session version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/nitrolite/build/getting-started/quickstart.mdx` around lines 117 - 130, Replace the hardcoded version value in the AppStateUpdateV1 object with a computed next version (e.g., use nextVersion(session) or session.version + 1n) so the update uses the correct incremented version; update the example where the update object (named update) is created and submitted via client.submitAppSessionDeposit to compute version dynamically and ensure the allocations array reflects the balances from the previous session version as noted.examples/nitrolite-v1-lifecycle/src/index.ts (2)
73-80: 💤 Low value
CHANNEL_DEPOSIT_AMOUNT=0passes validation but is semantically invalid.
decimalEnvaccepts non-negative values, soCHANNEL_DEPOSIT_AMOUNT=0would passloadConfig. InprepareHomeChannel,depositAmount = max(channelDepositAmount, deficit)would then fall back todeficit, which is fine when there is a deficit but produces a 0 deposit when the home channel already has enough — currently harmless, but easy to regress later. Adding a> 0invariant forCHANNEL_DEPOSIT_AMOUNT(mirroringAPP_DEPOSIT_AMOUNT) makes the contract explicit.♻️ Suggested validation
const appDepositAmount = decimalEnv('APP_DEPOSIT_AMOUNT', '0.005'); const operateAmount = decimalEnv('OPERATE_AMOUNT', '0.001'); const withdrawAmount = decimalEnv('WITHDRAW_AMOUNT', '0.002'); + const channelDepositAmount = decimalEnv('CHANNEL_DEPOSIT_AMOUNT', '0.01'); if (!appDepositAmount.isPositive()) { throw new Error('APP_DEPOSIT_AMOUNT must be greater than zero'); } + if (!channelDepositAmount.isPositive()) { + throw new Error('CHANNEL_DEPOSIT_AMOUNT must be greater than zero'); + } if (operateAmount.plus(withdrawAmount).greaterThan(appDepositAmount)) { throw new Error('OPERATE_AMOUNT + WITHDRAW_AMOUNT must be <= APP_DEPOSIT_AMOUNT'); } return { userPrivateKey, appPrivateKey, wsURL: requireEnv('NITRONODE_WS_URL'), rpcURL: requireEnv('RPC_URL'), chainId, asset, - channelDepositAmount: decimalEnv('CHANNEL_DEPOSIT_AMOUNT', '0.01'), + channelDepositAmount, appDepositAmount,Also applies to: 110-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nitrolite-v1-lifecycle/src/index.ts` around lines 73 - 80, The decimalEnv function currently allows zero which makes CHANNEL_DEPOSIT_AMOUNT=0 pass; update validation so decimalEnv enforces a strictly positive value (>0) for deposit amounts used for CHANNEL_DEPOSIT_AMOUNT (and mirror the same check applied to APP_DEPOSIT_AMOUNT at the other site), i.e., after creating amount in decimalEnv (or a new helper), reject values that are zero or negative by throwing an Error like `${name} must be a positive decimal amount` so callers such as prepareHomeChannel never receive a zero depositAmount.
142-145: 💤 Low valueString-matched error handling is fragile across SDK versions.
Several recovery paths key off substrings of
error.message(allowance|approval|approve,home blockchain is already set,already acknowledged,apps.v1 group is disabled,does not require a blockchain operation). If the SDK localizes, refactors, or capitalizes any of these strings, the example will start re-throwing on previously-handled paths. Consider:
- Preferring typed errors / error codes from
@yellow-org/sdkif exposed.- Adding a brief comment noting the SDK version this was validated against (1.2.1) so readers know to revisit on upgrade.
Also applies to: 243-251, 269-276, 371-378
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nitrolite-v1-lifecycle/src/index.ts` around lines 142 - 145, The current error handling in isAllowanceError (and other message-regex checks around the repo) relies on fragile substring matching of Error.message; replace these with checks for SDK-specific error classes or error codes exposed by `@yellow-org/sdk` (e.g., inspect error.name, a code property, or use instanceof <SDKErrorClass> if available) and fallback to the existing regex only if typed/code checks are absent; also add a single-line comment near isAllowanceError noting the SDK version validated (1.2.1) so future upgraders revisit these guards.docs/nitrolite/build/getting-started/prerequisites.mdx (1)
180-184: 💤 Low valueFaucet link freshness.
sepoliafaucet.com(operated by Alchemy) has historically rotated/required signup; consider listing the Google Cloud Sepolia faucet or PoW Sepolia faucet as a backup so readers without an Alchemy account aren't blocked. Optional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/nitrolite/build/getting-started/prerequisites.mdx` around lines 180 - 184, Update the "Sepolia" table row that currently points to https://sepoliafaucet.com/ (the Sepolia faucet) to include one or two alternative backup faucets so users who must sign up with Alchemy aren't blocked; e.g., append or add links for the Google Cloud Sepolia faucet and the PoW Sepolia faucet alongside the existing Alchemy link in the Sepolia row so all three are shown (keep the existing link, add the new URLs and brief labels).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/nitrolite-v1-lifecycle/src/index.ts`:
- Around line 197-221: The function enableNodeLocalAccountTransactions currently
silently returns when the internal createEVMClients factory is missing; update
it to log a visible warning instead of silently returning: inside
enableNodeLocalAccountTransactions, where originalFactory is checked (the
early-return branch), emit a console.warn with a clear message that
createEVMClients could not be patched and LocalAccount preservation may not work
(use the suggested message text), then return; keep all other behavior (the
double-cast to ClientWithEVMFactory, the wrapping of createEVMClients, and the
writeContract override) unchanged.
---
Nitpick comments:
In `@docs/nitrolite/build/getting-started/prerequisites.mdx`:
- Around line 180-184: Update the "Sepolia" table row that currently points to
https://sepoliafaucet.com/ (the Sepolia faucet) to include one or two
alternative backup faucets so users who must sign up with Alchemy aren't
blocked; e.g., append or add links for the Google Cloud Sepolia faucet and the
PoW Sepolia faucet alongside the existing Alchemy link in the Sepolia row so all
three are shown (keep the existing link, add the new URLs and brief labels).
In `@docs/nitrolite/build/getting-started/quickstart.mdx`:
- Around line 117-130: Replace the hardcoded version value in the
AppStateUpdateV1 object with a computed next version (e.g., use
nextVersion(session) or session.version + 1n) so the update uses the correct
incremented version; update the example where the update object (named update)
is created and submitted via client.submitAppSessionDeposit to compute version
dynamically and ensure the allocations array reflects the balances from the
previous session version as noted.
In `@examples/nitrolite-v1-lifecycle/package.json`:
- Around line 8-9: The engines.node entry is set to ">=20.0.0" but the package
should not pull `@types/node` that target newer Node major versions; update
package.json to pin the devDependency "@types/node" to a ^20 range (or
alternatively raise "engines.node" to ^24 if Node 24 is intended) so type
definitions align with the declared runtime floor—look for the "engines.node"
field and the "@types/node" entry in package.json to make this change.
In `@examples/nitrolite-v1-lifecycle/src/index.ts`:
- Around line 73-80: The decimalEnv function currently allows zero which makes
CHANNEL_DEPOSIT_AMOUNT=0 pass; update validation so decimalEnv enforces a
strictly positive value (>0) for deposit amounts used for CHANNEL_DEPOSIT_AMOUNT
(and mirror the same check applied to APP_DEPOSIT_AMOUNT at the other site),
i.e., after creating amount in decimalEnv (or a new helper), reject values that
are zero or negative by throwing an Error like `${name} must be a positive
decimal amount` so callers such as prepareHomeChannel never receive a zero
depositAmount.
- Around line 142-145: The current error handling in isAllowanceError (and other
message-regex checks around the repo) relies on fragile substring matching of
Error.message; replace these with checks for SDK-specific error classes or error
codes exposed by `@yellow-org/sdk` (e.g., inspect error.name, a code property, or
use instanceof <SDKErrorClass> if available) and fallback to the existing regex
only if typed/code checks are absent; also add a single-line comment near
isAllowanceError noting the SDK version validated (1.2.1) so future upgraders
revisit these guards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb08eef3-5a82-471f-b44c-ccfca0f141f8
⛔ Files ignored due to path filters (1)
examples/nitrolite-v1-lifecycle/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
docs/nitrolite/build/getting-started/key-terms.mdxdocs/nitrolite/build/getting-started/prerequisites.mdxdocs/nitrolite/build/getting-started/quickstart.mdxdocs/nitrolite/build/quick-start/index.mddocs/nitrolite/learn/index.mdxdocs/nitrolite/learn/introduction/supported-chains.mdxdocusaurus.config.tsexamples/nitrolite-v1-lifecycle/.env.exampleexamples/nitrolite-v1-lifecycle/.gitignoreexamples/nitrolite-v1-lifecycle/README.mdexamples/nitrolite-v1-lifecycle/package.jsonexamples/nitrolite-v1-lifecycle/src/index.tsexamples/nitrolite-v1-lifecycle/tsconfig.jsonsrc/pages/nitrolite.tsx
💤 Files with no reviewable changes (1)
- docs/nitrolite/build/quick-start/index.md
| function enableNodeLocalAccountTransactions(client: Client): void { | ||
| const sdkClient = client as unknown as ClientWithEVMFactory; | ||
| const originalFactory = sdkClient.createEVMClients?.bind(client); | ||
| if (!originalFactory) { | ||
| return; | ||
| } | ||
|
|
||
| sdkClient.createEVMClients = (chainId: bigint, rpcURL: string): EVMClientFactoryResult => { | ||
| const result = originalFactory(chainId, rpcURL); | ||
| const walletClient = result.walletClient; | ||
| if (!walletClient?.account || !walletClient.writeContract) { | ||
| return result; | ||
| } | ||
|
|
||
| // Node.js public RPCs need the LocalAccount object when viem sends writes. | ||
| // Keep using SDK methods, but preserve the local account after simulation. | ||
| const localAccount = walletClient.account; | ||
| const originalWriteContract = walletClient.writeContract.bind(walletClient); | ||
| walletClient.writeContract = (request: Record<string, unknown>): Promise<Hex> => { | ||
| return originalWriteContract({ ...request, account: localAccount }); | ||
| }; | ||
|
|
||
| return result; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
@yellow-org/sdk 1.2.1 Client createEVMClients public API documentation
💡 Result:
No public documentation found for a createEVMClients public API method in @yellow-org/sdk version 1.2.1. The official Yellow Network documentation at docs.yellow.org references the package '@yellow-org/sdk' in examples, but the primary client creation uses Client.create(endpoint, stateSigner, txSigner, ...) as shown in the TypeScript examples [1][2]. The API reference page exists but lacks detailed method listings for Client constructors in the available snippets [3]. No mentions of createEVMClients appear across searches of docs, npm, GitHub, or related sources. Version 1.2.1 specifics or changelog not found; docs indicate "Version: 1.x" generally [3]. Example usage for Client creation [1]: import { Client, createSigners, withBlockchainRPC } from '@yellow-org/sdk'; const { stateSigner, txSigner } = createSigners(process.env.PRIVATE_KEY!); const client = await Client.create( 'wss://clearnet-sandbox.yellow.com/ws', stateSigner, txSigner, withBlockchainRPC(80002n, process.env.RPC_URL!) ); Official docs: https://docs.yellow.org/docs/api-reference/ [3], examples: https://docs.yellow.org/docs/build/sdk/typescript/examples/ [1]. If this is an internal or unreleased method, check private repos or contact Yellow Network support.
Citations:
- 1: https://docs.yellow.org/docs/build/sdk/typescript/examples/
- 2: https://github.com/layer-3/docs/blob/master/docs/build/sdk/typescript/examples.mdx
- 3: https://docs.yellow.org/docs/api-reference/
🏁 Script executed:
cat -n examples/nitrolite-v1-lifecycle/src/index.ts | sed -n '39,50p;197,221p'Repository: layer-3/docs
Length of output: 1667
🏁 Script executed:
# Also check the ClientWithEVMFactory type definition more broadly
rg -A 10 "ClientWithEVMFactory" examples/nitrolite-v1-lifecycle/src/index.tsRepository: layer-3/docs
Length of output: 824
🏁 Script executed:
# Check where enableNodeLocalAccountTransactions is called
rg -n "enableNodeLocalAccountTransactions" examples/nitrolite-v1-lifecycle/src/index.ts
# Check if there's any logging in the function or surrounding area
cat -n examples/nitrolite-v1-lifecycle/src/index.ts | sed -n '195,225p'Repository: layer-3/docs
Length of output: 1671
🏁 Script executed:
# Check imports to see if console logging or logger is available
head -30 examples/nitrolite-v1-lifecycle/src/index.tsRepository: layer-3/docs
Length of output: 735
Monkey-patching createEVMClients is fragile and silently fails.
enableNodeLocalAccountTransactions mutates an undocumented internal of Client (createEVMClients). The double cast through unknown as ClientWithEVMFactory bypasses TypeScript, already indicating this bypasses the public surface. The method is not listed in @yellow-org/sdk's official documentation.
The current implementation silently returns early if the factory is missing (line 200–202) with no warning. Any SDK refactor—rename, return-shape change, or upstream account-propagation fix—will cause this example to fail silently, with no indication that the patch is no longer working.
Add a warning when the patch fails to apply, so regressions surface in the example output:
if (!originalFactory) {
console.warn(
'Warning: enableNodeLocalAccountTransactions could not patch createEVMClients. ' +
'The SDK shape may have changed. LocalAccount preservation may not work on Node.js public RPCs.'
);
return;
}
Additionally, consider opening an upstream issue with the SDK to fix this in @yellow-org/sdk itself, so the account preservation is handled upstream and this workaround is no longer needed.
[recommend_refactoring]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/nitrolite-v1-lifecycle/src/index.ts` around lines 197 - 221, The
function enableNodeLocalAccountTransactions currently silently returns when the
internal createEVMClients factory is missing; update it to log a visible warning
instead of silently returning: inside enableNodeLocalAccountTransactions, where
originalFactory is checked (the early-return branch), emit a console.warn with a
clear message that createEVMClients could not be patched and LocalAccount
preservation may not work (use the suggested message text), then return; keep
all other behavior (the double-cast to ClientWithEVMFactory, the wrapping of
createEVMClients, and the writeContract override) unchanged.
Rewrite the Nitrolite Build entrypoint around the current v1 TypeScript SDK. - Replace the old quickstart, prerequisites, and key terms pages with v1 setup guidance for @yellow-org/sdk. - Remove the duplicate legacy quick-start route. - Retarget Build navigation and existing Build links to the new getting-started quickstart. - Keep the sandbox WebSocket URL as a clear placeholder until the public endpoint is finalized. - Preserve faucet examples as hidden comments so they can be restored when the endpoint is ready. This makes the first Build path match the current SDK while giving 0.5.x users a clear pointer to the compat package.
e3e3706 to
bd7a17d
Compare
Summary
This updates the Nitrolite Build entrypoint so new builders land on the native v1
@yellow-org/sdkpath instead of the older 0.5.x quickstart.What Changed
@yellow-org/sdk@1.2.1, Node 20,Client.create(),createSigners(),Decimal, and Nitronode terminology.examples/nitrolite-v1-lifecycle.CLOSE_HOME_CHANNEL=true; the default run closes the app session but leaves the home channel open.build/quick-startpage.build/getting-started/quickstart.Why
The previous Build front door looked like v1 documentation but still taught the older 0.5.x package, APIs, endpoint names, and mental model. A new builder following it would hit stale imports and outdated setup steps.
This PR makes the first Build path match the current SDK and gives migrating users an explicit off-ramp before they move to native v1 APIs. It also backs the quickstart with code that was actually run against the v1 SDK and live Nitronode flow before being documented.
Scope Notes
docs/nitrolite/learn/index.mdxanddocs/nitrolite/learn/introduction/supported-chains.mdxare included only for link rewires caused by deleting the old Build quickstart.contracts/deployments, belongs with the Learn introduction refresh and is not part of this Build front-door change.Docs Pages To Review
Build front door:
docs/nitrolite/build/getting-started/quickstart.mdxdocs/nitrolite/build/getting-started/prerequisites.mdxdocs/nitrolite/build/getting-started/key-terms.mdxdocusaurus.config.tsRunnable example:
examples/nitrolite-v1-lifecycle/README.mdexamples/nitrolite-v1-lifecycle/src/index.tsLink-only ripple:
docs/nitrolite/learn/index.mdxdocs/nitrolite/learn/introduction/supported-chains.mdxValidation
git diff --checknpm run buildcd examples/nitrolite-v1-lifecycle && npm installcd examples/nitrolite-v1-lifecycle && npm run build@yellow-org/sdk@1.2.1on Sepoliayellow: home-channel prepare, app-session create, deposit, operate, withdraw, and app-session close all completed successfully.The docs build still reports existing warnings from versioned 0.5.x and Clearnet contract docs, but this PR does not add new broken-link or broken-anchor warnings.