YNU-827: Nitrolite Protocol Documentation#619
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds comprehensive protocol documentation for Nitrolite, including nine new Markdown files that collectively define the channel protocol, state model, cryptography, enforcement rules, interaction patterns, cross-chain functionality, security guarantees, and unified terminology. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/protocol/structure.md (1)
21-21: Consider adding language identifiers to fenced code blocks.The fenced code blocks throughout this document lack language identifiers, which can improve syntax highlighting and readability in markdown viewers. For the directory structure blocks (lines 21 and 27), consider using
text. For the documentation outline blocks (lines 57, 110, 171, 216, 263, 307, 345, 391, 438, 478, and 507), consider usingmarkdownormd.📝 Example fix for documentation outline blocks
-``` +```markdown # Nitrolite Protocol OverviewAnd for directory structure blocks:
-``` +```text docs/protocol/Also applies to: 27-27, 57-57, 110-110, 171-171, 216-216, 263-263, 307-307, 345-345, 391-391, 438-438, 478-478, 507-507
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/structure.md` at line 21, The fenced code blocks in the document lack language identifiers; update the directory-structure blocks (the fences containing "docs/protocol/" and similar tree listings) to use ```text and update the documentation-outline blocks (the fences starting with headings like "# Nitrolite Protocol Overview") to use ```markdown or ```md so Markdown viewers get proper syntax highlighting; search for the code fences that show the directory tree and the outline headings and add the appropriate language tag to each opening ``` fence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/protocol/structure.md`:
- Line 21: The fenced code blocks in the document lack language identifiers;
update the directory-structure blocks (the fences containing "docs/protocol/"
and similar tree listings) to use ```text and update the documentation-outline
blocks (the fences starting with headings like "# Nitrolite Protocol Overview")
to use ```markdown or ```md so Markdown viewers get proper syntax highlighting;
search for the code fences that show the directory tree and the outline headings
and add the appropriate language tag to each opening ``` fence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16a1fc37-b008-46a0-ae15-8991c017e7f8
📒 Files selected for processing (1)
docs/protocol/structure.md
a05d90e to
1176de1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/protocol/structure.md (1)
21-23: Add language identifiers to fenced code blocks.All fenced code blocks in this file should specify a language identifier for better rendering and syntax highlighting. For directory paths and trees, use
textorplaintext. For the markdown document outlines, usemarkdownormd.📝 Example fixes for different block types
For directory path blocks (e.g., lines 21-23):
-``` +```text docs/protocol/For directory tree blocks (e.g., lines 27-41): ```diff -``` +```text docs/protocol/ overview.md ...For markdown outline blocks (e.g., lines 57-96): ```diff -``` +```markdown # Nitrolite Protocol Overview ...</details> Also applies to: 27-41, 57-96, 110-157, 171-202, 216-251, 263-295, 307-333, 345-377, 391-424, 438-464, 478-495, 507-533 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/protocol/structure.mdaround lines 21 - 23, Fenced code blocks in this
file are missing language identifiers; update each triple-backtick block (e.g.,
the blocks containing the literal "docs/protocol/") to include an appropriate
language token—use "text" or "plaintext" for directory paths/trees and
"markdown" or "md" for markdown outlines—so replacewithtext orsimilar blocks in the document).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/protocol/structure.md`:
- Around line 21-23: Fenced code blocks in this file are missing language
identifiers; update each triple-backtick block (e.g., the blocks containing the
literal "docs/protocol/") to include an appropriate language token—use "text" or
"plaintext" for directory paths/trees and "markdown" or "md" for markdown
outlines—so replace ``` with ```text or ```markdown as appropriate for each
block (apply the same change to all other similar blocks in the document).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efe448e2-75b1-4b8c-88ec-f5bc7c189023
📒 Files selected for processing (1)
docs/protocol/structure.md
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/protocol/interactions.md (1)
52-58: Consider documenting the method naming convention.The documentation lists operation names (RequestCreation, SubmitState, etc.) but doesn't specify the actual method identifiers used in messages. Based on the implementation, these follow the format
channels.v1.{operation_name}(e.g.,channels.v1.request_creation). Including this convention would help implementers understand the exact wire format.📝 Proposed addition
Add a note under the Core Operations section:
## Core Operations +Methods are identified using the naming convention `channels.v1.{operation_name}`, where the operation name uses snake_case. + The protocol defines the following core operations:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/interactions.md` around lines 52 - 58, Add a short note under the Core Operations section documenting the method naming convention: explain that wire method identifiers use the pattern channels.v1.{operation_name} (lowercase, underscore or snake-case as in implementation if applicable) and provide 2–3 examples mapping the listed operations—e.g., RequestCreation → channels.v1.request_creation, SubmitState → channels.v1.submit_state, GetLatestState → channels.v1.get_latest_state—so implementers know the exact method strings to use when sending messages.docs/protocol/channel-protocol.md (1)
17-22: Clarify the Asset to Metadata mapping in the channel definition.The table lists "Asset" as a field, but the actual on-chain channel definition struct (as shown in pkg/core/utils.go:84-143) uses a "Metadata" field that is derived from the asset string by taking the first 8 bytes of keccak256(asset) and padding to 32 bytes. Consider adding a note that the Asset identifier is transformed into a 32-byte Metadata field for the on-chain channel definition.
📝 Suggested clarification
| Field | Description | | --------------------------- | -------------------------------------------------------------- | | User | Identifier of the user participant | | Node | Identifier of the node participant | -| Asset | Identifier of the asset operated within the channel | +| Asset | Identifier of the asset operated within the channel (hashed to derive on-chain Metadata field) | | Nonce | Unique nonce to distinguish channels with identical parameters | | ChallengeDuration | Challenge period duration in seconds | | ApprovedSignatureValidators | Bitmask of approved signature validation modes |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/channel-protocol.md` around lines 17 - 22, The "Asset" table entry should clarify that the on-chain channel struct uses a 32-byte "Metadata" field derived from the asset string; update the Asset row in channel-protocol.md to note that Metadata = first 8 bytes of keccak256(asset) padded to 32 bytes (i.e., asset → keccak256(asset) → take first 8 bytes → right-pad to 32 bytes) so readers understand the mapping to the on-chain field "Metadata" (refer to the Metadata field and keccak256 derivation used in the channel construction code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/protocol/channel-protocol.md`:
- Around line 17-22: The "Asset" table entry should clarify that the on-chain
channel struct uses a 32-byte "Metadata" field derived from the asset string;
update the Asset row in channel-protocol.md to note that Metadata = first 8
bytes of keccak256(asset) padded to 32 bytes (i.e., asset → keccak256(asset) →
take first 8 bytes → right-pad to 32 bytes) so readers understand the mapping to
the on-chain field "Metadata" (refer to the Metadata field and keccak256
derivation used in the channel construction code).
In `@docs/protocol/interactions.md`:
- Around line 52-58: Add a short note under the Core Operations section
documenting the method naming convention: explain that wire method identifiers
use the pattern channels.v1.{operation_name} (lowercase, underscore or
snake-case as in implementation if applicable) and provide 2–3 examples mapping
the listed operations—e.g., RequestCreation → channels.v1.request_creation,
SubmitState → channels.v1.submit_state, GetLatestState →
channels.v1.get_latest_state—so implementers know the exact method strings to
use when sending messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8875059e-9bf1-4d10-9ad5-3029b977a3c8
📒 Files selected for processing (11)
docs/protocol/channel-protocol.mddocs/protocol/cross-chain-and-assets.mddocs/protocol/cryptography.mddocs/protocol/enforcement.mddocs/protocol/extensions/app-sessions.mddocs/protocol/extensions/overview.mddocs/protocol/interactions.mddocs/protocol/overview.mddocs/protocol/security-and-limitations.mddocs/protocol/state-model.mddocs/protocol/terminology.md
|
Topics to be covered by second iteration:
UPD: To Include in Terminology: The file is missing "ChannelHub" version concept, which is embedded as a first byte into each channelId. And:
Address remaining comments |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/protocol/cross-chain-and-assets.md`:
- Around line 34-36: The fenced formula block containing "NormalizedAmount =
Amount * 10^(AssetDecimals - ChainDecimals)" needs a language tag to satisfy
markdownlint (MD040); update the fence from ``` to include a language (for
example ```text or ```math) so the block becomes a tagged fenced code block and
retains the same content.
- Around line 35-39: The doc's canonical precision definition is inconsistent
with lossless normalization: instead of saying canonical precision is the
minimum across chains, change the wording so the canonical decimal precision for
a unified asset (referenced by Amount, AssetDecimals, ChainDecimals and
NormalizedAmount = Amount * 10^(AssetDecimals - ChainDecimals)) is the maximum
(i.e., the highest decimal precision among supported chains) or explicitly state
that normalization scales smaller-decimal chain amounts up to the canonical
precision; update the sentence that currently reads "minimum precision across
all supported chains" and any related wording (including the paragraph that uses
NormalizedAmount and the later mention around AssetDecimals/ChainDecimals) to
reflect this lossless scaling rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5795c2d-b296-483c-8991-38bfe049d16f
📒 Files selected for processing (2)
docs/protocol/cross-chain-and-assets.mddocs/protocol/terminology.md
nksazonov
left a comment
There was a problem hiding this comment.
6 of 9 files are reviewed, 3 other are pending
|
|
||
| 1. **Protocol layer** — defines rules for state validity, advancement, and enforcement | ||
| 2. **Off-chain layer** — signed state updates exchange with a node | ||
| 3. **Blockchain layer** — blockchain contracts that hold assets and enforce states | ||
|
|
There was a problem hiding this comment.
I think it is better to describe as:
- Virtual / Extension / App Session layer (the "protocol" name is misleading, because all three layers form a single protocol)
- Off-Chain (Node) layer
- Blockchain layer
There was a problem hiding this comment.
Will be considered in the next docs iteration
nksazonov
left a comment
There was a problem hiding this comment.
6 of 9 files are reviewed, 3 other are pending
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
docs/protocol/cross-chain-and-assets.md (1)
34-36:⚠️ Potential issue | 🟡 MinorAdd language tag to the fenced formula block.
The fenced code block should specify a language (e.g.,
textormath) to satisfy markdownlint MD040.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/cross-chain-and-assets.md` around lines 34 - 36, The fenced formula block containing "NormalizedAmount = Amount * 10^(18 - ChainDecimals)" should include a language tag (e.g., ```text or ```math) to satisfy markdownlint MD040; update the fence surrounding that formula so the opening backticks specify the language while leaving the formula text unchanged so tools and linters recognize it as a language-specific code block.docs/protocol/enforcement.md (1)
154-154:⚠️ Potential issue | 🔴 CriticalFix duplicate and incorrect text in Escrow Deposit Finalize row.
The table entry for "Escrow Deposit Finalize" contains duplicate "On home chain" text and an incorrect non-home chain description. The non-home chain effect for finalization should be releasing escrowed funds to the node vault, not creating an escrow record.
Proposed fix
-| Escrow Deposit Finalize | On home chain: state updated, user allocation increased. On home chain: state updated, node funds adjusted. On non-home chain: escrow record created, user funds locked, automatic release to the Node timer started. | +| Escrow Deposit Finalize | On home chain: state updated, user allocation increased. On non-home chain: escrowed funds released to node vault. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/enforcement.md` at line 154, Update the "Escrow Deposit Finalize" table row to remove the duplicated "On home chain" phrase and correct the non-home chain behavior: keep a single "On home chain" clause describing state update and user allocation increase (and node funds adjustment if intended), and replace the non-home chain clause to state that the escrow is released to the node vault and user funds are unlocked (i.e., automatic release to node vault / transfer to node vault), rather than "escrow record created" or "user funds locked"; edit the table row text for the entry labeled "Escrow Deposit Finalize" accordingly.
🧹 Nitpick comments (3)
docs/protocol/channel-protocol.md (1)
187-193: Consider explicit statement of all non-home ledger field changes.Line 193 describes "UB decreases by Amount, NNF decreases by Amount" but doesn't explicitly state that UNF and NB remain unchanged. While this can be inferred, explicitly listing all field states would improve clarity and make it easier to verify the ledger invariant is maintained.
Suggested clarification
-Non-home ledger effects: UB decreases by Amount, NNF decreases by Amount +Non-home ledger effects: UB decreases by Amount to zero, UNF unchanged, NB unchanged, NNF decreases by Amount🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/channel-protocol.md` around lines 187 - 193, The "Escrow Deposit Finalize" spec currently states non-home ledger effects as "UB decreases by Amount, NNF decreases by Amount" but omits explicit mention of UNF and NB remaining unchanged; update the document so the non-home ledger effects line explicitly lists all field changes (e.g., "UB decreases by Amount, NNF decreases by Amount, UNF unchanged, NB unchanged") to make the state transition unambiguous and ensure the ledger invariant is clearly verifiable; reference the "Escrow Deposit Finalize" section and the field names UB, NNF, UNF, and NB when making the change.docs/protocol/terminology.md (1)
130-131: Consider hyphenating "migrated out" for consistency.For consistency with other channel status names like "migrating-in", consider changing "migrated out" to "migrated-out".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/terminology.md` around lines 130 - 131, Update the phrase "migrated out" to "migrated-out" in the terminology sentence so it matches the hyphenated style used for "migrating-in"; locate the string "migrated out" in the sentence that lists operating, disputed, migrating-in, migrated out and replace it with "migrated-out" to ensure consistent channel status naming.docs/protocol/cryptography.md (1)
67-77: Clarify signature validator governance and lifecycle.The document should explicitly state that:
- User and node must agree on approved signature validators before channel creation
- Nodes control which validators they support, not protocol developers (non-owner-controlled)
- Once agreed in channel definition, the node cannot invalidate validators
- The validation system is extensible
This clarifies the trust model and validator lifecycle. Based on learnings from past review feedback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/cryptography.md` around lines 67 - 77, Add a short "Validator Governance and Lifecycle" subsection under "Signature Validation Modes" that states: (1) the user and node must mutually agree on the channel's approved signature validators prior to channel creation; (2) nodes (not protocol developers) decide which validators they support (i.e., validator support is node-controlled); (3) once the approved validators are recorded in the channel definition they are immutable for that channel and nodes cannot later invalidate them; and (4) the validation system is extensible so new validator types can be added and used if mutually approved by participants; reference the existing terms "Default Mode (0x00)" and "Session Key Mode (0x01)" when noting that these modes are examples of validators.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/protocol/enforcement.md`:
- Line 113: Correct the typo in the sentence "It should be noted that it is NOT
possible the file another challenge on a channel that is already disputed." by
changing "the file" to "to file" so the sentence reads "It should be noted that
it is NOT possible to file another challenge on a channel that is already
disputed. The current challenge must be resolved first." Locate and update this
exact sentence in docs/protocol/enforcement.md.
In `@docs/protocol/state-model.md`:
- Line 113: The fenced code block currently lacks a language tag which triggers
markdownlint MD040; update the block that contains "UserAllocation +
NodeAllocation == UserNetFlow + NodeNetFlow" to include a language specifier
(e.g., add ```text before the line and close with ``` after) so the block
becomes a labeled fenced code block (use the same fence around the existing
content referencing UserAllocation, NodeAllocation, UserNetFlow, NodeNetFlow).
In `@docs/protocol/terminology.md`:
- Around line 113-115: The fenced formula block containing "NormalizedAmount =
Amount * 10^(18 - ChainDecimals)" lacks a language tag and triggers markdownlint
MD040; update that fenced code block (the triple-backtick block wrapping the
formula) to include a language identifier such as text (e.g., change ``` to
```text) so the block is explicitly labeled and the linter warning is resolved.
- Around line 109-117: The WAD Normalization section is ambiguous: update the
wording to state that WAD normalization specifically scales chain-specific
amounts to 18 decimals using the formula NormalizedAmount = Amount * 10^(18 -
ChainDecimals), and explicitly note that this 18-decimal WAD is used for
lossless cross-chain comparisons, while the asset's canonical decimal precision
(e.g., 6 for USDC) remains the canonical display/interaction precision used in
User <> Clearnode interactions (deposits, state submissions, transfers, session
ops); ensure both concepts (WAD = 18-decimal normalization and canonical
precision used for user/clearnode interactions) are clearly separated and
referenced in the paragraph.
---
Duplicate comments:
In `@docs/protocol/cross-chain-and-assets.md`:
- Around line 34-36: The fenced formula block containing "NormalizedAmount =
Amount * 10^(18 - ChainDecimals)" should include a language tag (e.g., ```text
or ```math) to satisfy markdownlint MD040; update the fence surrounding that
formula so the opening backticks specify the language while leaving the formula
text unchanged so tools and linters recognize it as a language-specific code
block.
In `@docs/protocol/enforcement.md`:
- Line 154: Update the "Escrow Deposit Finalize" table row to remove the
duplicated "On home chain" phrase and correct the non-home chain behavior: keep
a single "On home chain" clause describing state update and user allocation
increase (and node funds adjustment if intended), and replace the non-home chain
clause to state that the escrow is released to the node vault and user funds are
unlocked (i.e., automatic release to node vault / transfer to node vault),
rather than "escrow record created" or "user funds locked"; edit the table row
text for the entry labeled "Escrow Deposit Finalize" accordingly.
---
Nitpick comments:
In `@docs/protocol/channel-protocol.md`:
- Around line 187-193: The "Escrow Deposit Finalize" spec currently states
non-home ledger effects as "UB decreases by Amount, NNF decreases by Amount" but
omits explicit mention of UNF and NB remaining unchanged; update the document so
the non-home ledger effects line explicitly lists all field changes (e.g., "UB
decreases by Amount, NNF decreases by Amount, UNF unchanged, NB unchanged") to
make the state transition unambiguous and ensure the ledger invariant is clearly
verifiable; reference the "Escrow Deposit Finalize" section and the field names
UB, NNF, UNF, and NB when making the change.
In `@docs/protocol/cryptography.md`:
- Around line 67-77: Add a short "Validator Governance and Lifecycle" subsection
under "Signature Validation Modes" that states: (1) the user and node must
mutually agree on the channel's approved signature validators prior to channel
creation; (2) nodes (not protocol developers) decide which validators they
support (i.e., validator support is node-controlled); (3) once the approved
validators are recorded in the channel definition they are immutable for that
channel and nodes cannot later invalidate them; and (4) the validation system is
extensible so new validator types can be added and used if mutually approved by
participants; reference the existing terms "Default Mode (0x00)" and "Session
Key Mode (0x01)" when noting that these modes are examples of validators.
In `@docs/protocol/terminology.md`:
- Around line 130-131: Update the phrase "migrated out" to "migrated-out" in the
terminology sentence so it matches the hyphenated style used for "migrating-in";
locate the string "migrated out" in the sentence that lists operating, disputed,
migrating-in, migrated out and replace it with "migrated-out" to ensure
consistent channel status naming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b864752-bd7e-4b52-9b41-caa5b60836c5
⛔ Files ignored due to path filters (1)
docs/protocol/state_ledger_advancement.pngis excluded by!**/*.png
📒 Files selected for processing (7)
docs/protocol/channel-protocol.mddocs/protocol/cross-chain-and-assets.mddocs/protocol/cryptography.mddocs/protocol/enforcement.mddocs/protocol/overview.mddocs/protocol/state-model.mddocs/protocol/terminology.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/protocol/overview.md
|
|
||
| The following fields are preserved exactly from the off-chain representation: | ||
|
|
||
| - Version |
There was a problem hiding this comment.
Add language tag to the fenced code block.
The fenced code block should specify a language (e.g., text) to satisfy markdownlint MD040.
Proposed fix
-```
+```text
UserAllocation + NodeAllocation == UserNetFlow + NodeNetFlow</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/protocol/state-model.md at line 113, The fenced code block currently
lacks a language tag which triggers markdownlint MD040; update the block that
contains "UserAllocation + NodeAllocation == UserNetFlow + NodeNetFlow" to
include a language specifier (e.g., add ```text before the line and close with
around the existing content referencing UserAllocation, NodeAllocation,
UserNetFlow, NodeNetFlow).
| The protocol roadmap includes the following planned improvements: | ||
|
|
||
| - **Validator network** — off-chain state advancement can be independently validated; a validator network would monitor on-chain actions and penalize node misbehaviour that harms the ecosystem | ||
| - **Trustless cross-chain enforcement** — removing the reliance on node liquidity trust through bridging or proof mechanisms |
There was a problem hiding this comment.
This can be removed, and a "Extension layer on-chain enforcement" added instead.
Summary by CodeRabbit