docs: new protocol proposal #454
Conversation
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 a comprehensive proposal for a new cross-chain architecture for the Nitrolite protocol. It details a system for managing state channels across multiple EVM chains, emphasizing a broker-coordinated approach. The proposal outlines new core concepts, participant roles, enhanced fund management with a two-tier system, and a unified mechanism for state updates, aiming to improve scalability and flexibility for multi-asset, cross-chain operations while maintaining security guarantees. 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
|
WalkthroughAdds comprehensive Nitrolite Cross-Chain Protocol documentation detailing multi-chain state channel architecture, channel lifecycle, state transitions, fund management, security considerations, and integration guidelines. Defines core concepts and protocol workflows for channel operations across multiple blockchains. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/protocol_v1_suggestion.md (3)
78-81: Clarify allocation mechanics with a concrete example.The note explaining how
available + lockedmay differ fromnetValueis technically accurate but somewhat abstract. Consider adding a concrete example to illustrate the scenario.Expand the explanation with an example:
**Note**: `available + locked` may differ from `netValue` due to cross-chain transfers: - After sending funds: `available` decreases but `netValue` remains unchanged - After receiving funds: `available` increases but `netValue` remains unchanged - Only deposit/withdraw operations affect `netValue` + + **Example**: A user deposits 100 tokens (`available=100, locked=0, netValue=100, isNetPositive=true`). After sending 40 tokens cross-chain, the state becomes `available=60, locked=0, netValue=100` (the net deposit hasn't changed, but available funds decreased). Upon receiving those 40 tokens on another chain, `available=100, locked=0, netValue=100` is restored.
315-322: Add token address parameter to integration example.The integration guideline example on line 318 calls
custody.deposit(account, token, amount)but doesn't show how to handle the token parameter (e.g.,address(0)for native tokens). A brief note would improve clarity.Add a clarifying comment:
1. **Opening Channels**: ```solidity // 1. Deposit funds - custody.deposit(account, token, amount); + // Use address(0) for native tokens, otherwise the ERC20 token address + custody.deposit(account, token, amount);
345-350: Clarify "smart allocations" terminology and validation mechanics.The term "smart allocations" is introduced without prior definition. Consider either defining it upfront or linking this section to earlier allocation explanations for better flow.
Expand the explanation:
### Smart Allocations - The protocol uses "smart allocations" that enable: + The allocation structure enables validation and automatic fund rebalancing: - Validation of non-consecutive state versions (e.g., version 5 → 10) - Mathematical verification of state transitions - Automatic fund rebalancing between ledger and liquidity
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/protocol_v1_suggestion.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/protocol_v1_suggestion.md
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
123-123: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
194-194: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
211-211: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
223-223: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
252-252: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
🔇 Additional comments (1)
docs/protocol_v1_suggestion.md (1)
1-15: Well-structured protocol documentation with clear objectives.The overview section effectively communicates the key differences from the current protocol and establishes context for readers. The progression from overview → core concepts → workflows → security is logical and accessible.
| - **Flow**: User submits to broker for validation | ||
| - **Broker Action**: Signs if valid | ||
|
|
||
| ### 4. Checkpointing |
There was a problem hiding this comment.
Fix section numbering: missing "### 3" before Checkpointing.
The protocol workflows sections jump from "### 2. State Transitions" (line 135) to "### 4. Checkpointing" (line 177), skipping "### 3". Either add the missing section or renumber accordingly.
Apply this diff to fix the numbering:
- ### 4. Checkpointing
+ ### 3. CheckpointingThen update subsequent sections:
- ### 5. Dispute Resolution
+ ### 4. Dispute Resolution- ### 6. Cooperative Closing
+ ### 5. Cooperative Closing- ### 7. Emergency Closing
+ ### 6. Emergency Closing📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### 4. Checkpointing | |
| ### 3. Checkpointing |
| ### 4. Checkpointing | |
| ### 4. Dispute Resolution |
| ### 4. Checkpointing | |
| ### 5. Cooperative Closing |
| ### 4. Checkpointing | |
| ### 6. Emergency Closing |
🤖 Prompt for AI Agents
In docs/protocol_v1_suggestion.md around line 177, the heading numbering jumps
from "### 2. State Transitions" to "### 4. Checkpointing", so add or rename the
missing "### 3" heading: either insert a new "### 3. Checkpointing" heading if a
new section is intended, or renumber the existing "### 4. Checkpointing" to "###
3. Checkpointing", then increment or adjust all subsequent section numbers
accordingly to maintain a sequential numeric order.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a detailed proposal for a new cross-chain protocol. The document is well-structured, but there are a few critical ambiguities and minor issues that need to be addressed for clarity and correctness. My review focuses on clarifying the fund management rules, the structure of allocations, and improving the document's structure and completeness. The most critical points concern the state update rules, which appear contradictory, and the allocation structure, which is unclear on how funds are split between participants.
| ```solidity | ||
| struct Allocation { | ||
| address token; // Token address (address(0) for native tokens) | ||
| uint256 available; // Amount available for use | ||
| uint256 locked; // Amount locked/reserved | ||
| uint256 netValue; // Absolute value of (deposits - withdrawals) | ||
| bool isNetPositive; // True if (deposits - withdrawals) >= 0 | ||
| } |
There was a problem hiding this comment.
The Allocation struct and its usage within the State are ambiguous about how funds are divided between the User and the Broker. The State struct contains an allocations array, which is described as being per-token. However, the Allocation struct itself (available, locked, netValue) seems to track funds for a single party and lacks a field (like participant or destination) to specify which party it belongs to.
This makes it unclear how the broker's funds are tracked. Please clarify if the Allocation struct is only for the user, and if so, how the broker's balance is represented in the state. If it's intended to cover both, the structure needs to be revised to show the split. For example, the old protocol's Allocation struct included a destination field, which made this clear.
There was a problem hiding this comment.
Well, that's because broker is just a coordinator here, he doesn't have any funds:)
There was a problem hiding this comment.
A destination field still may be fruitful here to allow channel participants to allocate funds to other addresses.
| 2. **Available Balance Changes**: | ||
| - If `availableDiff = newAvailable - previousAvailable > 0`: Subtract `availableDiff` from custody liquidity | ||
| - If `availableDiff = newAvailable - previousAvailable < 0`: Add `|availableDiff|` to custody liquidity | ||
|
|
||
| 3. **Locked Balance Changes**: | ||
| - If `lockedDiff = newLocked - previousLocked > 0`: Add `lockedDiff` to custody liquidity | ||
| - If `lockedDiff = newLocked - previousLocked < 0`: Subtract `lockedDiff` from custody liquidity |
There was a problem hiding this comment.
The state update rules for Available Balance Changes and Locked Balance Changes appear to be incorrect as they can lead to flawed liquidity calculations. For instance, during a Lock operation (availableDiff = -amount, lockedDiff = amount), the total funds in the channel should remain constant. However, the current rules would incorrectly increase the custody liquidity by 2 * amount. A similar issue exists for Unlock, Deposit, and Withdraw operations.
The rules should likely be conditional on whether netValue changes, or be based on the net change of available + locked funds.
Consider revising these rules to distinguish between on-chain deposits/withdrawals and off-chain transfers.
| 2. **Available Balance Changes**: | |
| - If `availableDiff = newAvailable - previousAvailable > 0`: Subtract `availableDiff` from custody liquidity | |
| - If `availableDiff = newAvailable - previousAvailable < 0`: Add `|availableDiff|` to custody liquidity | |
| 3. **Locked Balance Changes**: | |
| - If `lockedDiff = newLocked - previousLocked > 0`: Add `lockedDiff` to custody liquidity | |
| - If `lockedDiff = newLocked - previousLocked < 0`: Subtract `lockedDiff` from custody liquidity | |
| 2. **Intra-channel Fund Movements**: | |
| Let `totalValueDiff = (new.available + new.locked) - (previous.available + previous.locked)`. | |
| This rule applies when `netValue` is unchanged (`netValueDiff == 0`). | |
| - If `totalValueDiff > 0` (e.g., Receive): Subtract `totalValueDiff` from custody liquidity. | |
| - If `totalValueDiff < 0` (e.g., Send): Add `|totalValueDiff|` to custody liquidity. | |
| - If `totalValueDiff == 0` (e.g., Lock/Unlock): There is no change to custody liquidity. |
There was a problem hiding this comment.
Hmmm, you're correct
There was a problem hiding this comment.
Maybe even custody liquidity should exchange only with locked balances, but it should be thoroughly tested with asynchronous state checkpoints
| - **Flow**: User submits to broker for validation | ||
| - **Broker Action**: Signs if valid | ||
|
|
||
| ### 4. Checkpointing |
There was a problem hiding this comment.
The section numbering under "Protocol Workflows" is incorrect. It jumps from section 2. State Transitions to 4. Checkpointing, skipping section 3. Please renumber the sections sequentially for clarity.
| ### 4. Checkpointing | |
| ### 3. Checkpointing |
| The protocol uses "smart allocations" that enable: | ||
| - Validation of non-consecutive state versions (e.g., version 5 → 10) |
There was a problem hiding this comment.
The "Smart Allocations" feature is mentioned to enable "Validation of non-consecutive state versions (e.g., version 5 -> 10)". This is a powerful feature for state channels, but the document doesn't explain how this is achieved. Please provide more details on the mechanism for this mathematical verification, as it's a crucial aspect of the protocol's efficiency and design.
There was a problem hiding this comment.
Yeah, it actually doesn't validate it, but rather plays as dumb judge by accepting what user and broker agreed to
There was a problem hiding this comment.
Off-chain validators of state history would be crucial to show broker's reputation
|
|
||
| ## Overview | ||
|
|
||
| The Nitrolite protocol implements a cross-chain state channel system where contracts are deployed on multiple EVM chains. A broker server facilitates users' channel state updates in a coordinated manner across these chains. The protocol enables most operations to happen off-chain, with the ability to submit states on-chain when needed to reflect the latest agreements. The broker acts as a coordinator ensuring correct state progression across the multi-chain ecosystem. |
There was a problem hiding this comment.
We may try to abstract from EVM chains here as we describe "Protocol" and we intend to implement on various blockchains
There was a problem hiding this comment.
The same goes for using "Clearnode" or "Node" instead of "Broker"
| ### Key Differences from Current Protocol | ||
|
|
||
| This document describes a new cross-chain protocol that differs from the current implementation in several key ways: | ||
| - **Participants**: Changed from generic CLIENT/SERVER to specific User/Broker roles |
There was a problem hiding this comment.
There is a chance that at some point in the future we may include app sessions as channel participants. And App session accounts would be governed by a separate contract similar to adjudicator. This way no user nor broker would have access to these funds, and the funds would be retrievable by providing signature quorum to the contract governing app sessions (it would have same app session methods as off chain, but in case clearnode is down. the participants would be able to propagate and finalize app state on-chain).
There was a problem hiding this comment.
To add an "app session" as participant, we either need to change participant type from address to bytes32 or "app session" id from bytes32 to address
|
|
||
| The allocation structure maintains: | ||
| - **available**: Funds that can be freely used in state transitions | ||
| - **locked**: Funds that are reserved/locked for specific purposes |
There was a problem hiding this comment.
Who defines the rule to unlock the funds? And how is this tracked? For example, Neodax wants to lock funds to create a limit order, user agrees but what happens next?
|
|
||
| ### Future Enhancements | ||
|
|
||
| 1. **Non-EVM Support**: Update encoding, hashing and signatures for cross-chain compatibility with non-EVM chains |
There was a problem hiding this comment.
Mention account address synchronization here
| 1. **Non-EVM Support**: Update encoding, hashing and signatures for cross-chain compatibility with non-EVM chains | ||
| 2. **Batch Checkpoint**: Add `batchCheckpoint` method to allow workers to submit multiple channel updates in a single transaction, reducing gas costs | ||
| 3. **Additional State Transitions**: Easily extensible to support new transition types beyond the current six | ||
|
|
There was a problem hiding this comment.
Would be good to mention atomicity in bridging
| - Keep signed states for dispute resolution | ||
| - Verify counterparty signatures before accepting | ||
|
|
||
| 2. **Fund Safety**: |
There was a problem hiding this comment.
We may rethink fund management. As this has been confusing people. We need to think if we can implement methods for direct depositing into and withdrawing from channels without intermediate account funding
| - Automatic unlocking on channel closure | ||
|
|
||
| **Safety Features:** | ||
| - Minimum 1-hour challenge period |
There was a problem hiding this comment.
It should be acceptable if we set challenge to 7 and in the future increase to 30 days. This is resolution method in case of disaster. Disaster and resolution is not the process expected to happen everyday. Like any legal process it must allow participants enough time to respond. This also decreases number of participants that may call challenge for "no reason"
| The protocol supports multiple signature types: | ||
| - Raw ECDSA | ||
| - EIP-191 (Ethereum Signed Message) | ||
| - EIP-712 (Structured Data) |
There was a problem hiding this comment.
Alongside EIPs would be good to mention the sig algo. This will help when developing for cross chain
There was a problem hiding this comment.
Mentioned EIPs declare ECDSA themselves, so EIP-XX is sufficient
|
|
||
| ### 2. State Transitions | ||
|
|
||
| Each state update must include transition data explaining the change from the previous state: |
There was a problem hiding this comment.
Does this require keeping all chain of changes (state transitions) until checkpoint?
There was a problem hiding this comment.
Also, what about signatures? Each state must be signed by both user and the Node?
If so, this means that to Receive funds, you must sign the update yourself. This creates an issue of receiving while being offline. And what if when a party comes back online the Node is down?
Also, what is the flow of receiving multiple transfers when still being offline?
There was a problem hiding this comment.
Moreover, this also means that to perform any funds-related operation with an app session (create, deposit, withdraw, close) a user must submit a transaction.
There was a problem hiding this comment.
No, only this transition
| address brokerAddress; // The broker's address (facilitator/coordinator) | ||
| address adjudicator; // Contract that validates state transitions | ||
| uint64 challenge; // Dispute resolution period in seconds (min 1 hour) | ||
| uint64 nonce; // Unique identifier for channels with same participants |
There was a problem hiding this comment.
Also, it has uneven indentation for nonce comment :)
|
|
||
| A channel is a cryptographic construct that establishes a relationship between a user and the broker for coordinated state management across chains. Each channel consists of: | ||
|
|
||
| ```solidity |
There was a problem hiding this comment.
We can do protocol definition abstracted from a specific language like solidity. Thats why we call it protocol, not protocol implementation
There was a problem hiding this comment.
I somewhat agree. On the other hand, in ERCs almost all code examples use Solidity, and here we are talking about blockchain contracts, with Solidity being the most popular language.
There was a problem hiding this comment.
Yes, of course, but as document covers only EVM right now, and that's a just to show draft I didn't do any refactoring
|
|
||
| Track fund availability and net balance per token: | ||
| ```solidity | ||
| struct Allocation { |
There was a problem hiding this comment.
As now we have multiple tokens in a channel, how do we know which allocation is in ownership of user and which is on the broker side? Previously we were using index in array, now it is unclear
There was a problem hiding this comment.
Broker doesn't have any funds, its purpose is coordination, all allocations are for user
| 1. **Custody Ledger**: User's deposited funds that can be withdrawn directly | ||
| 2. **Custody Liquidity**: Contract's liquidity pool that changes only during state updates (cannot be withdrawn directly) |
There was a problem hiding this comment.
So the "Custody Ledger" in your proposal is the same to what "Custody Ledger" is now?
And could you explain "Custody Liquidity" in more details, please?
I don't understand why the whole SC's liquidity pool changes during state updates. Are those updates cross-chain (sending to / receiving from the other chain)?
|
|
||
| **Cross-Chain Architecture**: | ||
| - Each channel is unique to its chain (channelId includes chainId) | ||
| - Broker coordinates related channels across different chains |
There was a problem hiding this comment.
What do you mean by "related channels"? Do you mean "channels of the same user"?
|
|
||
| 1. **Trust Model**: Similar to current protocol - broker (like SERVER) can block progress but cannot steal funds | ||
| 2. **Dispute Resolution**: Users can always challenge and force close after timeout | ||
| 3. **Cross-chain Risks**: Each chain operates independently; broker coordinates but cannot guarantee atomicity |
There was a problem hiding this comment.
I would say that this is a huge downside both for security and implementation.
There was a problem hiding this comment.
What protocol version can guarantee atomicity?
| - Used for ALL state updates between open and close | ||
| - Validates state transition rules | ||
| - Updates on-chain state record | ||
| - Required transitions: deposit, withdraw, send, receive, lock, unlock |
There was a problem hiding this comment.
Does it mean that a user MUST perform a checkpoint(...):
- for each state?
- for all deposit, withdraw, send, receive, lock, unlock ?
There was a problem hiding this comment.
No, checkpoint is required by broker only in case of deposit, other state updates can be checkpointed any time, but are not required to happen
| ### Future Enhancements | ||
|
|
||
| 1. **Non-EVM Support**: Update encoding, hashing and signatures for cross-chain compatibility with non-EVM chains | ||
| 2. **Batch Checkpoint**: Add `batchCheckpoint` method to allow workers to submit multiple channel updates in a single transaction, reducing gas costs |
There was a problem hiding this comment.
What are "workers"? Who should submit either non-batched or batched checkpoints: users or 3rd parties?
There was a problem hiding this comment.
Workers are just runtimes deployed by community to track off-chain states and can be rewarded for checkpointing states on-chain
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.