YNU-416: update docs according to release v0.5.0#451
Conversation
|
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 a new Session Keys documentation file, updates the migration guide for v0.5.x protocol changes, and configures Docusaurus code-block magic comments plus CSS for git-diff-like styling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 @philanton, 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 introduces extensive documentation updates to align with the v0.5.0 release, focusing on critical changes to session key functionality, channel lifecycle management, and signature mechanisms across the protocol, Nitrolite SDK, and ClearNode API. It also enhances the visual presentation of code examples in the documentation to clearly illustrate migration steps. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for the v0.5.0 release, including a new comprehensive guide for Session Keys and updating the migration guide with breaking changes. The changes also add Docusaurus configurations to render code diffs, which is a nice improvement for readability. The documentation is generally well-written, but I've identified a few issues in the migration guide, including a syntax error that will break rendering, a grammatical error, and some confusing comments in code examples that could mislead developers. My review provides suggestions to address these points.
--------- Co-authored-by: Maharshi Mishra <maharshi@tor.us>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
erc7824-docs/docs/guides/migration-guide.md (1)
199-199: Consider simplifying wording."PRIOR to" could be simplified to "before" for more concise documentation.
Apply this diff:
-If you plan to work with on-chain channels opened PRIOR to v0.5.0, then on NitroliteClient initialization the `stateSigner` you specify must be based on a Session Key used in the channel as participant. Even if this session key is or will expire, you still need to provide a `stateSigner` based on it. +If you plan to work with on-chain channels opened before v0.5.0, then on NitroliteClient initialization the `stateSigner` you specify must be based on a Session Key used in the channel as participant. Even if this session key is or will expire, you still need to provide a `stateSigner` based on it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
erc7824-docs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
clearnode/docs/SessionKeys.md(1 hunks)erc7824-docs/docs/guides/migration-guide.md(5 hunks)erc7824-docs/docusaurus.config.js(1 hunks)erc7824-docs/src/css/custom.css(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T14:03:35.378Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 419
File: clearnode/rpc_router_private.go:827-834
Timestamp: 2025-11-07T14:03:35.378Z
Learning: For the `revoke_session_key` RPC endpoint in clearnode/rpc_router_private.go: The design intentionally allows non-deterministic behavior when multiple session key signatures are present. Client-side code must ensure correct signatures are provided. The server keeps the RPC protocol flexible rather than implementing deterministic signature selection logic.
Applied to files:
clearnode/docs/SessionKeys.md
📚 Learning: 2025-11-13T10:32:14.541Z
Learnt from: nksazonov
Repo: erc7824/nitrolite PR: 428
File: clearnode/rpc_router_auth.go:233-244
Timestamp: 2025-11-13T10:32:14.541Z
Learning: In clearnode/rpc_router_auth.go, when CheckSessionKeyExists returns ErrSessionKeyExistsAndExpired during authentication, the error should be returned to the client and block re-authentication. This is intentional behavior - expired session keys should not allow automatic re-authentication and replacement in the auth flow.
Applied to files:
clearnode/docs/SessionKeys.md
🪛 Gitleaks (8.29.1)
clearnode/docs/SessionKeys.md
[high] 96-96: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 152-152: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 203-203: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 219-219: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
erc7824-docs/docs/guides/migration-guide.md
[high] 259-259: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 279-279: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 302-302: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 306-306: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 332-332: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 352-352: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 362-362: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 594-594: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 702-702: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
erc7824-docs/docs/guides/migration-guide.md
[style] ~199-~199: ‘PRIOR to’ might be wordy. Consider a shorter alternative.
Context: ...n to work with on-chain channels opened PRIOR to v0.5.0, then on NitroliteClient initial...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ 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 (go)
🔇 Additional comments (3)
erc7824-docs/src/css/custom.css (1)
189-213: LGTM! Git-diff styling implemented correctly.The CSS classes for
git-diff-removeandgit-diff-addare well-structured with appropriate colors (red for removals, green for additions), proper spacing, and dark theme variants for good contrast. This pairs nicely with the Prism magic comments configuration added indocusaurus.config.js.erc7824-docs/docusaurus.config.js (1)
258-274: LGTM! Prism magic comments configured correctly.The magic comments configuration properly defines diff-style annotations (
remove-next-line,add-next-line) with corresponding CSS classes that match the styling added incustom.css. The configuration follows standard Docusaurus patterns and will enable clear visual indication of code changes in the migration guide.clearnode/docs/SessionKeys.md (1)
1-248: LGTM! Comprehensive session keys documentation.The documentation clearly explains session keys concepts including applications, expiration, allowances, and the special "clearnode" root access behavior. Management workflows are well-documented with complete request/response examples. The structure is logical and the content aligns well with the migration guide changes.
Note: Static analysis "Generic API Key" detections on lines 96, 152, 203, and 219 are false positives—these are example hex addresses used in documentation.
0a610e5 to
d34da0a
Compare
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
erc7824-docs/docs/guides/migration-guide.md (1)
57-61: Re-check resize sign conventions and comments; “custody ledger” wording is still confusingThe text says:
resize_amountandallocate_amountshould obeyresize_amount = -allocate_amount.- Negative
allocate_amountwithdraws from the channel to unified balance, positive deposits to the channel.- The examples then label:
// Deposit from custody ledger to unified balance resize_amount: BigInt(50), // Positive = deposit to channel allocate_amount: BigInt(-50) // Negative = deposit to unified balance ... // Withdraw from unified balance to custody ledger resize_amount: BigInt(-100), // Negative = withdraw from channel allocate_amount: BigInt(100) // Positive = withdraw from unified balanceThere are two issues:
- The comments for
resize_amountvsallocate_amountare easy to misread and partially conflict with the earlier bullet that explains semantics only in terms ofallocate_amount.- The “custody ledger” terminology is still unclear; previous reviews already suggested using explicit phrases like “on-chain channel” and “unified balance” instead. This also better matches the actual ledger flow where the channel account is a pass-through between on-chain funds and the unified wallet account. Based on learnings, deposits are modeled as movements between the on-chain channel and the unified wallet balance.
Suggestion:
- Reword comments to consistently describe what happens to the on-chain channel balance and the unified balance, for example:
- // Deposit from custody ledger to unified balance + // Deposit from on-chain channel to unified balance - // Withdraw from unified balance to custody ledger + // Deposit to on-chain channel from unified balance
- Double-check that the “positive/negative” explanations for both
resize_amountandallocate_amountare consistent with the actualhandleResizedsemantics and update the comments accordingly.Based on learnings, deposits flow from the on-chain channel account into the unified wallet account.
Also applies to: 145-183
🧹 Nitpick comments (9)
clearnode/docs/SessionKeys.md (4)
33-44: Clarify behavior when re-authenticating expired session keysThe expiration rules say keys become invalid and cannot be re-registered, which matches the intended behavior where attempts to authenticate with an expired session key are rejected and not automatically replaced. Based on learnings, expired session keys should block re-auth and require creating a new key.
Consider adding an explicit sentence like:
If you try to authenticate with an expired session key, ClearNode returns an error and does not re-register or replace it — you must create a new session key.
This will make the auth behavior around expiration unambiguous for integrators.
Based on learnings, expired session keys must not allow automatic re-authentication.
117-125: Defaultingapplicationto"clearnode"is powerful but risky; recommend stronger guidanceThe note that
applicationis optional and defaults to"clearnode"effectively grants root-like access if callers omit the field. That’s convenient for legacy integrations but easy to misuse in new ones.Consider tightening the wording to:
- Explicitly say the default to
"clearnode"exists only for backward compatibility and may be removed in a future release.- Recommend that all new integrations must set
applicationexplicitly and avoid relying on the default.This keeps the behavior accurate while nudging devs toward safer usage.
126-127: Avoid duplicated “re-auth with existing session key” notesThe behavior for authenticating with an already registered session key (all fields required but config ignored) is explained both in the “General Rules” section and again here.
To keep the doc tighter and reduce maintenance overhead, consider:
- Keeping the more detailed explanation in one place (e.g., “General Rules”), and
- Here, replacing the paragraph with a short cross-reference back to that section.
This preserves the guidance while avoiding future drift between two similar descriptions.
Also applies to: 11-12
193-245: Document non-deterministic behavior forrevoke_session_keywith multiple signaturesThe revoke rules and errors are clear, but one important nuance from the implementation is missing: when multiple session key signatures are provided to
revoke_session_key, the server intentionally does not implement deterministic selection logic — client code must ensure correct signatures are provided. Based on learnings, this non-determinism is by design.Consider adding a short note such as:
If a request carries multiple session-key signatures (for example, in multi-sig setups), ClearNode does not attempt to choose a “best” one. You must ensure that the signatures you send correspond to the wallet that owns the session key being revoked; otherwise behavior is not guaranteed.
This prevents surprising behavior for advanced users building multi-signature flows.
Based on learnings,
revoke_session_keydoes not perform deterministic signature selection.erc7824-docs/docs/guides/migration-guide.md (5)
25-34: Good high-level description of session key fields; consider explicit note on expired re-authThe description of
Application,Allowances, andExpires_atclearly reflects the 0.5.x behavior and highlights the risk of root ("clearnode") keys expiring. To make migration behavior fully explicit (and match the ClearNode implementation), you may want to add a short note here or just below that:Attempts to authenticate with an expired session key are rejected; expired keys are not re-registered, and you must create a new session key.
This mirrors the actual auth flow and aligns with the dedicated Session Keys doc.
Based on learnings, expired session keys must block re-authentication.
65-71: Verify trailing double quote in documented error stringThe error example currently reads:
operation denied: non-zero allocation in <count> channel(s) detected owned by wallet <address>"The trailing
"at the end of the message is unusual and may be a leftover from formatting rather than part of the actual error string.Please confirm the exact error text used by ClearNode; if the live error does not include the trailing quote, consider dropping it here to avoid confusion for users matching on error strings.
81-110: Auth examples accurately show application/root keys; maybe add a short “don’t rely on root” warningThe two
authRequestexamples (application-scoped vs root"clearnode") and the “Important considerations” bullets give a clear picture of how to configure session keys in the SDK.Given the strong impact of root keys, consider adding a one-liner like:
For new integrations, prefer application-scoped session keys and use
"clearnode"root access only when strictly necessary, as this behavior is intended for migration and may be deprecated.This aligns the migration guide with the deprecation note in the dedicated Session Keys doc.
Also applies to: 115-119
611-638: Structuredauth_requestJSON example mixes older/newer field namesThis 0.3.x example uses both
app_nameandapplication, along withexpire:"app_name": "Example App", "allowances": [ "usdc", "100.0" ], "scope": "app.create", "expire": "3600", "application": "0xApp1234567890abcdef..."Given the 0.5.x docs now standardize on
application,allowancesas an array of{ asset, amount }, andexpires_at, this mixing of field names could confuse readers about which shape belongs to which protocol version.Since this section is explicitly about 0.3.x breaking changes, consider adding a short note above the example to clarify that:
app_name/expirereflect the 0.3.x-era API, and- Newer 0.5.x-style fields (like
application/expires_at) are not used here.Alternatively, you could adjust the example to show only the 0.3.x-era shape to avoid mixing terminology.
221-226: Optional: tighten wording (“PRIOR to” → “before”)The phrase:
“opened PRIOR to v0.5.0”
is a bit wordy; “opened before v0.5.0” would be simpler and equally clear.
Not critical, but you may want to update for style consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
erc7824-docs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
clearnode/docs/SessionKeys.md(1 hunks)erc7824-docs/docs/guides/migration-guide.md(5 hunks)erc7824-docs/docusaurus.config.js(1 hunks)erc7824-docs/src/css/custom.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- erc7824-docs/src/css/custom.css
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-05T10:19:06.155Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 411
File: clearnode/custody.go:449-465
Timestamp: 2025-11-05T10:19:06.155Z
Learning: In clearnode/custody.go's handleResized function during positive resize operations (deposits), the ledger flow is: on-chain funds first credit the channel account, are immediately debited from the channel account, then credit the unified wallet account. The channel account acts as a pass-through for deposits. The unified wallet account is where actual deposited funds are held. This is recorded as a deposit transaction from channelAccountID to walletAccountID.
Applied to files:
erc7824-docs/docs/guides/migration-guide.md
📚 Learning: 2025-11-04T19:14:48.960Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 411
File: clearnode/custody.go:542-553
Timestamp: 2025-11-04T19:14:48.960Z
Learning: In clearnode/custody.go's handleClosed function, when unlocking escrow funds on channel close, the code intentionally passes the channelEscrowAccountBalance (which may be negative) directly to ledger.Record for the wallet account. When the escrow balance is negative (e.g., resize confirmation received for a channel not in resizing state), this causes a debit to the unified wallet account, which is the expected behavior to equalize accounting. Temporary negative unified balances in this scenario are acceptable.
Applied to files:
erc7824-docs/docs/guides/migration-guide.md
📚 Learning: 2025-11-07T14:03:35.378Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 419
File: clearnode/rpc_router_private.go:827-834
Timestamp: 2025-11-07T14:03:35.378Z
Learning: For the `revoke_session_key` RPC endpoint in clearnode/rpc_router_private.go: The design intentionally allows non-deterministic behavior when multiple session key signatures are present. Client-side code must ensure correct signatures are provided. The server keeps the RPC protocol flexible rather than implementing deterministic signature selection logic.
Applied to files:
clearnode/docs/SessionKeys.md
📚 Learning: 2025-11-13T10:32:14.541Z
Learnt from: nksazonov
Repo: erc7824/nitrolite PR: 428
File: clearnode/rpc_router_auth.go:233-244
Timestamp: 2025-11-13T10:32:14.541Z
Learning: In clearnode/rpc_router_auth.go, when CheckSessionKeyExists returns ErrSessionKeyExistsAndExpired during authentication, the error should be returned to the client and block re-authentication. This is intentional behavior - expired session keys should not allow automatic re-authentication and replacement in the auth flow.
Applied to files:
clearnode/docs/SessionKeys.md
🪛 Gitleaks (8.29.1)
clearnode/docs/SessionKeys.md
[high] 96-96: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 152-152: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 203-203: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 219-219: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
erc7824-docs/docs/guides/migration-guide.md
[style] ~223-~223: ‘PRIOR to’ might be wordy. Consider a shorter alternative.
Context: ...n to work with on-chain channels opened PRIOR to v0.5.0, then on NitroliteClient initial...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ 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: Test (Integration) / Test Integration
🔇 Additional comments (6)
erc7824-docs/docusaurus.config.js (1)
254-275: Magic comments configuration looks correct and matches intended diff stylesThe
prism.magicCommentsentries forremove-next-line/remove-start–remove-end,add-next-line/add-start–add-end, andhighlight-next-line/highlight-start–highlight-endalign with how they’re used in the docs and with the CSS classes (git-diff-remove,git-diff-add,theme-code-block-highlighted-line).This should give you the desired git-diff-style highlighting in code blocks without impacting existing Prism settings.
erc7824-docs/docs/guides/migration-guide.md (4)
120-141: Nice before/after example for 2-step channel creationThe
createChannelexample using magic comments clearly shows removal of inlineamountandsession_keyand the new “create then fund via resize” pattern. This should make migration straightforward for SDK users.No changes needed here from a correctness perspective.
252-266: EIP-712 amount type change is clear and well-illustratedThe diff-style example for changing
Allowance.amountfromuint256tostringis concise and makes the migration path to string-based values obvious for anyone maintaining typed data definitions.No issues spotted here.
402-427: Diff-style examples for SDK and contracts are helpful and consistentThe added magic-comment-based diffs for:
- Replacing
stateWalletClientwithstateSignerand addingWalletStateSigner,- Switching RPC request creation to object-based parameters,
- Unifying channel operation response types,
- Simplifying
Signatureto aHexalias on the TypeScript side,- Updating Solidity
SignatureandState.sigsdefinitions, and- Migrating adjudicator contracts to
EIP712AdjudicatorBase,are all consistent with the described 0.3.x/0.5.x changes and clearly show before/after states.
These sections look correct and will make it much easier for integrators to update their code.
Also applies to: 481-501, 509-555, 563-582, 797-850, 901-934
188-211: Based on my verification, I've found confirmed inconsistencies in the session key documentation. The review comment is largely correct about the core issue, but contains line number inaccuracies. Let me generate the rewritten comment:
Resolve inconsistent session key response field names and timestamp formats across docs
The ClearNode
get_session_keysresponse is documented inconsistently:
- SessionKeys.md (lines 157-168): Response uses
"allowance"within allowances array and ISO 8601 timestamp strings ("2024-12-31T23:59:59Z")- migration-guide.md (lines 352-366): Same response uses
"amount"within allowances array and numeric millisecond timestamps (1719123456789)Both documents describe the identical ClearNode wire protocol endpoint. Standardize on one representation:
- Decide whether response field should be
allowanceoramount- Clarify timestamp format: numeric milliseconds vs seconds vs ISO 8601 strings, and document conversion if SDK converts during parsing
- Update both files to use consistent field names and types
Also: Lines 231-250 show the SDK TypeScript type definition (not ClearNode response), which uses camelCase
expiresAt/createdAtasbigint—ensure the SDK documentation clearly states how these relate to the wire protocol's snake_case numeric timestamps.⛔ Skipped due to learnings
Learnt from: dimast-x Repo: erc7824/nitrolite PR: 411 File: clearnode/custody.go:449-465 Timestamp: 2025-11-05T10:19:06.155Z Learning: In clearnode/custody.go's handleResized function during positive resize operations (deposits), the ledger flow is: on-chain funds first credit the channel account, are immediately debited from the channel account, then credit the unified wallet account. The channel account acts as a pass-through for deposits. The unified wallet account is where actual deposited funds are held. This is recorded as a deposit transaction from channelAccountID to walletAccountID.Learnt from: dimast-x Repo: erc7824/nitrolite PR: 411 File: clearnode/custody.go:425-427 Timestamp: 2025-11-04T10:39:28.297Z Learning: In clearnode/custody.go's handleResized function, when a resize event is received for a channel with status ChannelStatusOpen (not ChannelStatusResizing), the code intentionally logs an error but continues processing the resize operation normally. This is the expected behavior.Learnt from: dimast-x Repo: erc7824/nitrolite PR: 411 File: clearnode/custody.go:542-553 Timestamp: 2025-11-04T19:14:48.960Z Learning: In clearnode/custody.go's handleClosed function, when unlocking escrow funds on channel close, the code intentionally passes the channelEscrowAccountBalance (which may be negative) directly to ledger.Record for the wallet account. When the escrow balance is negative (e.g., resize confirmation received for a channel not in resizing state), this causes a debit to the unified wallet account, which is the expected behavior to equalize accounting. Temporary negative unified balances in this scenario are acceptable.clearnode/docs/SessionKeys.md (1)
<script> #!/bin/bash # Read the response section around line 358 sed -n '350,375p' erc7824-docs/docs/guides/migration-guide.md </script>
143-189: <function_calls>
| #### Allowance Validation | ||
|
|
||
| - **Supported Assets Only**: All assets specified in allowances must be supported by the system. Unsupported assets cause authentication to fail | ||
| - **Usage Tracking**: The system tracks spending per session key by recording which session key was used for each ledger debit operation | ||
| - **Spending Limits**: Once a session key reaches its spending cap for an asset, further operations requiring that asset are rejected with: `"operation denied: insufficient session key allowance: X required, Y available"` | ||
| - **Empty Allowances**: Providing an empty `allowances` array (`[]`) means zero spending allowed for all assets—any operation attempting to spend funds will be rejected |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the relevant documentation files
echo "=== Finding documentation files ==="
find . -type f -name "SessionKeys.md" -o -name "migration-guide.md" | head -20
# Check if erc7824-docs directory exists
echo -e "\n=== Checking directory structure ==="
ls -la 2>/dev/null | grep -E "erc7824|docs" || echo "Listing top-level dirs..."
find . -maxdepth 2 -type d -name "*docs*" -o -name "*erc7824*" 2>/dev/null | sortRepository: erc7824/nitrolite
Length of output: 402
🏁 Script executed:
#!/bin/bash
echo "=== clearnode/docs/SessionKeys.md (lines 60-75) ==="
sed -n '60,75p' ./clearnode/docs/SessionKeys.md
echo -e "\n=== Searching for get_session_keys response in SessionKeys.md ==="
grep -n -A 10 "get_session_keys" ./clearnode/docs/SessionKeys.md | head -30
echo -e "\n=== erc7824-docs/docs/guides/migration-guide.md - searching for session key responses ==="
rg -n -B 2 -A 8 "get_session_keys|allowance|amount" ./erc7824-docs/docs/guides/migration-guide.md | head -80Repository: erc7824/nitrolite
Length of output: 6818
🏁 Script executed:
#!/bin/bash
echo "=== Searching for allowance/amount field definitions in code ==="
rg -n "allowance|amount" --type rust --type ts -B 2 -A 2 | grep -E "(allowance|amount|struct|interface|type)" | head -60
echo -e "\n=== Looking for get_session_keys implementation ==="
rg -n "get_session_keys" --type rust -A 10 | head -40
echo -e "\n=== Searching SessionKeys.md for all allowance field references ==="
grep -n "allowance\|amount" ./clearnode/docs/SessionKeys.md | head -20
echo -e "\n=== Searching migration-guide.md for all allowance field references ==="
grep -n "allowance\|amount" ./erc7824-docs/docs/guides/migration-guide.md | head -20Repository: erc7824/nitrolite
Length of output: 10644
🏁 Script executed:
#!/bin/bash
echo "=== SessionKeys.md - Create Session Key request (around line 51) ==="
sed -n '40,110p' ./clearnode/docs/SessionKeys.md
echo -e "\n=== SessionKeys.md - Get Session Keys response (around line 150) ==="
sed -n '140,175p' ./clearnode/docs/SessionKeys.mdRepository: erc7824/nitrolite
Length of output: 3429
🏁 Script executed:
#!/bin/bash
echo "=== Checking all docs for allowance field naming patterns ==="
find . -name "*.md" -type f | xargs grep -l "allowance\|get_session_keys" | while read file; do
echo "File: $file"
grep -n "allowance\|amount.*session" "$file" | head -5
echo ""
doneRepository: erc7824/nitrolite
Length of output: 5208
Clarify allowance field naming across docs: request format (amount) vs response format (allowance)
The API uses different field names depending on context:
- Requests (e.g.,
auth_request): use"amount"within allowances objects - Responses (e.g.,
get_session_keys): use"allowance"+"used"fields
SessionKeys.md correctly documents both formats—requests show amount (lines 98-105) and the get_session_keys response shows allowance (lines 157-162).
However, migration-guide.md (lines 89-91) only demonstrates request examples with amount, without clarifying that response objects use allowance instead. This omission creates confusion for API consumers expecting consistent field naming across request and response payloads.
Update migration-guide.md to either: (1) show a get_session_keys response example with the correct allowance field, or (2) explicitly document that request payloads use amount while responses use allowance + used.
🤖 Prompt for AI Agents
In migration-guide.md around lines 89 to 91, the guide shows request examples
using "amount" but omits that responses use "allowance" and "used", causing
confusion; update the section to either add a sample get_session_keys response
that uses "allowance" and "used" (mirroring the SessionKeys.md response format)
or add a concise note immediately after the request example stating: request
payloads use "amount" in allowance objects while API responses use "allowance"
(available cap) and "used" (spent so far), with a short example or JSON snippet
illustrating the response field names.
| <Tabs> | ||
| <TabItem value="before" label="Before"> | ||
| <TabItem value="application" label="Application Auth"> | ||
|
|
||
| ```typescript | ||
| const request = NitroliteRPC.createRequest( | ||
| requestId, | ||
| RPCMethod.GetChannels, | ||
| [participant, status], | ||
| timestamp | ||
| ); | ||
| ```json | ||
| { | ||
| "req": [1, "auth_request", { | ||
| "address": "0x1234567890abcdef...", | ||
| "session_key": "0x9876543210fedcba...", | ||
| "application": "My Trading App", | ||
| "allowances": [ | ||
| { "asset": "usdc", "amount": "1000.0" }, | ||
| { "asset": "eth", "amount": "0.5" } | ||
| ], | ||
| "scope": "app.create", | ||
| "expires_at": 1719123456789 | ||
| }, 1619123456789], | ||
| "sig": ["0x..."] | ||
| } | ||
| ``` | ||
|
|
||
| </TabItem> | ||
| <TabItem value="after" label="After"> | ||
| <TabItem value="root" label="Root Auth (Clearnode)"> | ||
|
|
||
| ```typescript | ||
| const request = NitroliteRPC.createRequest({ | ||
| method: RPCMethod.GetChannels, | ||
| params: { | ||
| participant, | ||
| status, | ||
| }, | ||
| requestId, | ||
| timestamp, | ||
| }); | ||
| ```json | ||
| { | ||
| "req": [1, "auth_request", { | ||
| "address": "0x1234567890abcdef...", | ||
| "session_key": "0x9876543210fedcba...", | ||
| "application": "clearnode", | ||
| "allowances": [], | ||
| "scope": "app.create", | ||
| "expires_at": 1750659456789 | ||
| }, 1619123456789], | ||
| "sig": ["0x..."] | ||
| } | ||
| ``` | ||
|
|
||
| </TabItem> | ||
| </Tabs> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's search for the migration-guide.md file and examine the context around the expires_at field
fd migration-guide.mdRepository: erc7824/nitrolite
Length of output: 104
🏁 Script executed:
# Read the migration guide file to see the full context, especially the spec definition and examples
cat -n erc7824-docs/docs/guides/migration-guide.md | head -400 | tail -200Repository: erc7824/nitrolite
Length of output: 6373
🏁 Script executed:
# Search for all mentions of "expires_at" and "Expires_at" in the migration guide
rg -n "expires_at|Expires_at" erc7824-docs/docs/guides/migration-guide.md -A 2 -B 2Repository: erc7824/nitrolite
Length of output: 1565
🏁 Script executed:
# Search for the specification definition of expires_at (seconds since epoch)
rg -n "seconds since epoch|bigint timestamp" erc7824-docs/docs/guides/migration-guide.md -A 2 -B 2Repository: erc7824/nitrolite
Length of output: 612
🏁 Script executed:
# Look for any other timestamp examples in the document to check for consistency
rg -n "1[0-9]{9,12}" erc7824-docs/docs/guides/migration-guide.md -B 2 -A 1Repository: erc7824/nitrolite
Length of output: 3336
The expires_at examples in the JSON payloads contradict the documented specification
The migration guide explicitly states at line 33 that "Expires_at field: Uses a bigint timestamp (seconds since epoch)." However, the JSON examples consistently use 13-digit values:
- Line 290:
"expires_at": 1719123456789 - Line 307:
"expires_at": 1750659456789 - Line 362:
"expires_at": 1719123456789
These are millisecond timestamps (e.g., 1750659456789 ms ≈ July 2025), not seconds. The TypeScript examples at lines 94 and 108 correctly show BigInt(Math.floor(Date.now() / 1000) + ...), converting milliseconds to seconds, which contradicts these JSON examples.
Correct the JSON examples to use 10-digit second values, or update the specification prose to clarify whether the ClearNode API actually expects milliseconds. This discrepancy will cause client implementations to send incorrect timestamp units.
🤖 Prompt for AI Agents
In erc7824-docs/docs/guides/migration-guide.md around lines 276 to 314, the JSON
examples use millisecond timestamps for "expires_at" but the spec and TypeScript
examples expect seconds; change the two "expires_at" values in these JSON blocks
to 10-digit second values (divide the current 13-digit values by 1000 and
round/floor as needed), and scan nearby examples (and update any others, e.g.,
the one at line ~362) to ensure all "expires_at" fields consistently use
seconds; if you prefer milliseconds instead, instead update the prose at the top
of the guide to state that "expires_at" uses milliseconds and adjust TypeScript
examples accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
erc7824-docs/docs/guides/migration-guide.md (1)
205-205: Simplify wording: replace "PRIOR to" with "before".LanguageTool flagged "PRIOR to" as potentially wordy. Using "before" is more concise and maintains clarity.
-If you plan to work with on-chain channels opened PRIOR to v0.5.0, then on NitroliteClient initialization the `stateSigner` you specify must be based on a Session Key used in the channel as participant. Even if this session key is or will expire, you still need to provide a `stateSigner` based on it. +If you plan to work with on-chain channels opened before v0.5.0, then on NitroliteClient initialization the `stateSigner` you specify must be based on a Session Key used in the channel as participant. Even if this session key is or will expire, you still need to provide a `stateSigner` based on it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
erc7824-docs/docs/guides/migration-guide.md(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-05T10:19:06.155Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 411
File: clearnode/custody.go:449-465
Timestamp: 2025-11-05T10:19:06.155Z
Learning: In clearnode/custody.go's handleResized function during positive resize operations (deposits), the ledger flow is: on-chain funds first credit the channel account, are immediately debited from the channel account, then credit the unified wallet account. The channel account acts as a pass-through for deposits. The unified wallet account is where actual deposited funds are held. This is recorded as a deposit transaction from channelAccountID to walletAccountID.
Applied to files:
erc7824-docs/docs/guides/migration-guide.md
📚 Learning: 2025-11-04T19:14:48.960Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 411
File: clearnode/custody.go:542-553
Timestamp: 2025-11-04T19:14:48.960Z
Learning: In clearnode/custody.go's handleClosed function, when unlocking escrow funds on channel close, the code intentionally passes the channelEscrowAccountBalance (which may be negative) directly to ledger.Record for the wallet account. When the escrow balance is negative (e.g., resize confirmation received for a channel not in resizing state), this causes a debit to the unified wallet account, which is the expected behavior to equalize accounting. Temporary negative unified balances in this scenario are acceptable.
Applied to files:
erc7824-docs/docs/guides/migration-guide.md
📚 Learning: 2025-11-04T10:39:28.297Z
Learnt from: dimast-x
Repo: erc7824/nitrolite PR: 411
File: clearnode/custody.go:425-427
Timestamp: 2025-11-04T10:39:28.297Z
Learning: In clearnode/custody.go's handleResized function, when a resize event is received for a channel with status ChannelStatusOpen (not ChannelStatusResizing), the code intentionally logs an error but continues processing the resize operation normally. This is the expected behavior.
Applied to files:
erc7824-docs/docs/guides/migration-guide.md
🪛 Gitleaks (8.30.0)
erc7824-docs/docs/guides/migration-guide.md
[high] 265-265: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 285-285: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 308-308: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 312-312: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 338-338: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 358-358: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 368-368: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 600-600: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 708-708: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
erc7824-docs/docs/guides/migration-guide.md
[style] ~205-~205: ‘PRIOR to’ might be wordy. Consider a shorter alternative.
Context: ...n to work with on-chain channels opened PRIOR to v0.5.0, then on NitroliteClient initial...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Publish (Clearnode)
- GitHub Check: Test (Integration) / Test Integration
- GitHub Check: Analyze (go)
| const resizeMessage = await createResizeChannelMessage(messageSigner, { | ||
| channel_id: channelId, | ||
| resize_amount: BigInt(50), // Positive = deposit to channel, negative = withdraw from channel to custody ledger | ||
| allocate_amount: BigInt(-50), // Negative = deposit to unified balance, negative = withdraw from unified balance to channel | ||
| funds_destination: walletAddress, | ||
| }); |
There was a problem hiding this comment.
Clarify allocate_amount semantics and terminology in comments.
Line 153's comment is contradictory ("Negative = deposit to unified balance, negative = withdraw from unified balance to channel") and the terminology "custody ledger" at line 152 is vague. According to line 57's explanation, the sign conventions should be: negative allocate_amount withdraws from unified balance, positive deposits. Revise for clarity:
resize_amount: BigInt(50), // Positive = deposit to channel, negative = withdraw from channel to unified balance
allocate_amount: BigInt(-50), // Positive = withdraw from unified balance to channel, negative = deposit to unified balanceCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In erc7824-docs/docs/guides/migration-guide.md around lines 150 to 155, update
the inline comments to remove the contradictory wording and use clear sign
semantics and terminology: change the resize_amount comment to "Positive =
deposit to channel, negative = withdraw from channel to on-chain custody" (or
"custody ledger/account" if preferred), and change the allocate_amount comment
to match line 57's convention: "Positive = deposit from unified balance into the
channel, negative = withdraw from channel into the unified balance." Ensure both
comments use consistent phrasing (deposit vs withdraw) and explicitly reference
"unified balance" and "on-chain custody" to avoid ambiguity.
| { "asset": "eth", "amount": "0.5" } | ||
| ], | ||
| "scope": "app.create", | ||
| "expires_at": 1719123456789 |
There was a problem hiding this comment.
Fix expires_at timestamp values in JSON examples to match specification.
The specification at line 33 clearly states "Expires_at field: Uses a bigint timestamp (seconds since epoch)," and TypeScript examples at lines 94 and 108 correctly convert to seconds using Math.floor(Date.now() / 1000). However, the JSON examples at lines 272, 289, and 344 use 13-digit millisecond values instead of 10-digit second values. This inconsistency will cause client implementations to send incorrect timestamp units to the ClearNode API.
Correct the JSON examples by dividing millisecond values by 1000 and rounding:
"req": [1, "auth_request", {
"address": "0x1234567890abcdef...",
"session_key": "0x9876543210fedcba...",
"application": "My Trading App",
"allowances": [
{ "asset": "usdc", "amount": "1000.0" },
{ "asset": "eth", "amount": "0.5" }
],
"scope": "app.create",
- "expires_at": 1719123456789
+ "expires_at": 1719123456
}, 1619123456789], "req": [1, "auth_request", {
"address": "0x1234567890abcdef...",
"session_key": "0x9876543210fedcba...",
"application": "clearnode",
"allowances": [],
"scope": "app.create",
- "expires_at": 1750659456789
+ "expires_at": 1750659456
}, 1619123456789], "res": [1, "get_session_keys", {
"session_keys": [{
"id": "sk_123",
"session_key": "0x9876543210fedcba...",
"application": "My Trading App",
"allowances": [
{ "asset": "usdc", "amount": "1000.0", "used": "250.0" }
],
"scope": "app.create",
- "expires_at": 1719123456789,
+ "expires_at": 1719123456,Also applies to: 289-289, 344-344
🤖 Prompt for AI Agents
In erc7824-docs/docs/guides/migration-guide.md around lines 272, 289 and 344,
the JSON examples use 13-digit millisecond timestamps for "expires_at" but the
spec requires 10-digit seconds; update each JSON example to convert the
millisecond values to seconds by dividing by 1000 and rounding down (e.g.,
Math.floor(ms/1000)) so the "expires_at" fields contain seconds-since-epoch
integers consistent with the TypeScript samples and the specification.
Summary by CodeRabbit
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.