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. WalkthroughCreate-channel no longer accepts an initial deposit: the RPC/SDK amount parameter was removed, the backend forces the first allocation to zero, and channel funding is performed later via a resize/top-up flow. Tests, docs, SDK, and integration helpers updated to the two-step open+resize flow. Changes
Sequence Diagram(s)sequenceDiagram
participant SDK as SDK / Test Client
participant Service as Clearnode Service
participant Chain as Blockchain (Nitro)
rect rgba(220,235,255,0.9)
Note over SDK,Service: Create channel (no initial user deposit)
SDK->>Service: CreateChannel (no amount param)
Service->>Service: Overwrite first allocation -> 0
Service->>Chain: Open channel (allocations: user=0, broker=0)
Chain-->>Service: Open state
Service-->>SDK: Create response (params + initial state)
end
rect rgba(235,220,255,0.9)
Note over SDK,Service: Top-up via ResizeChannel
SDK->>Service: ResizeChannel (desired allocation)
Service->>Chain: Submit resize tx (allocate funds)
Chain-->>Service: Resize confirmed (new Open state)
Service-->>SDK: { resizeState, txHash }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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 @nksazonov, 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 significant change to how state channels are initiated, mandating that all new channels are created with a zero initial deposit. This streamlines the channel creation process by separating it from the funding mechanism. Users will now first establish a channel with no funds, and then subsequently use the 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
|
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.
Code Review
This pull request implements zero-deposit channel creation by hardcoding the initial deposit amount to zero on the backend and removing the amount parameter from the corresponding API endpoints. The changes are consistently applied across the codebase, including updates to the SDK, API documentation, and extensive modifications to integration tests to reflect the new channel creation flow (create with zero deposit, then top up via resize). The test coverage for this new behavior is thorough. My only suggestion is to clarify a comment in the Go backend code that currently describes this change as a temporary fix, which seems to contradict the otherwise permanent nature of this feature implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.work.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
clearnode/channel_service.go(1 hunks)clearnode/channel_service_test.go(8 hunks)clearnode/docs/API.md(4 hunks)clearnode/rpc_router_private.go(1 hunks)clearnode/rpc_router_private_test.go(1 hunks)integration/common/blockchainUtils.ts(2 hunks)integration/common/nitroliteClient.ts(4 hunks)integration/common/testHelpers.ts(3 hunks)integration/tests/challenge_channel.test.ts(1 hunks)integration/tests/create_channel.test.ts(5 hunks)integration/tests/lifecycle_nitrorpc_v02.test.ts(5 hunks)integration/tests/lifecycle_nitrorpc_v04.test.ts(5 hunks)integration/tests/nitrorpc_v04.test.ts(3 hunks)integration/tests/resize_channel.test.ts(9 hunks)sdk/src/client/index.ts(1 hunks)sdk/src/client/state.ts(1 hunks)sdk/src/client/types.ts(1 hunks)sdk/src/rpc/types/request.ts(0 hunks)sdk/test/unit/client/state.test.ts(3 hunks)
💤 Files with no reviewable changes (1)
- sdk/src/rpc/types/request.ts
🧰 Additional context used
🧬 Code graph analysis (13)
sdk/src/client/state.ts (1)
sdk/src/errors.ts (1)
Errors(359-385)
integration/tests/lifecycle_nitrorpc_v02.test.ts (1)
integration/common/testHelpers.ts (3)
createTestChannels(28-49)toRaw(21-23)composeResizeChannelParams(175-190)
integration/tests/lifecycle_nitrorpc_v04.test.ts (1)
integration/common/testHelpers.ts (3)
createTestChannels(28-49)toRaw(21-23)composeResizeChannelParams(175-190)
integration/common/testHelpers.ts (3)
sdk/src/rpc/types/common.ts (1)
RPCChannelOperation(331-338)clearnode/rpc_router_private.go (1)
ResizeChannelParams(89-94)clearnode/testing/client.go (1)
ResizeChannelParams(85-90)
integration/tests/challenge_channel.test.ts (1)
integration/common/setup.ts (1)
CONFIG(4-44)
integration/tests/resize_channel.test.ts (2)
integration/common/setup.ts (1)
CONFIG(4-44)integration/common/testHelpers.ts (1)
composeResizeChannelParams(175-190)
sdk/test/unit/client/state.test.ts (2)
sdk/src/client/state.ts (1)
_prepareAndSignInitialState(32-75)sdk/src/errors.ts (1)
Errors(359-385)
integration/common/nitroliteClient.ts (5)
integration/common/ws.ts (3)
getCreateChannelPredicate(193-197)getChannelUpdatePredicateWithStatus(181-191)TestWebSocket(9-139)sdk/src/rpc/parse/parse.ts (3)
parseCreateChannelResponse(107-107)parseChannelUpdateResponse(137-137)parseResizeChannelResponse(110-110)sdk/src/client/types.ts (1)
State(82-84)sdk/src/rpc/api.ts (1)
createResizeChannelMessage(579-594)integration/common/testHelpers.ts (1)
composeResizeChannelParams(175-190)
sdk/src/client/index.ts (2)
sdk/src/client/types.ts (2)
ResizeChannelParams(156-159)State(82-84)sdk/src/client/state.ts (1)
_prepareAndSignResizeState(107-138)
clearnode/channel_service_test.go (1)
clearnode/rpc_router_private.go (1)
CreateChannelParams(84-87)
integration/tests/create_channel.test.ts (8)
sdk/src/rpc/utils.ts (2)
generateRequestId(18-20)getCurrentTimestamp(9-11)clearnode/testing/client.go (1)
RPCData(39-44)sdk/src/rpc/types/index.ts (2)
RPCData(19-19)NitroliteRPCMessage(36-43)sdk/src/rpc/nitrolite.ts (1)
NitroliteRPC(17-157)integration/common/ws.ts (2)
getCreateChannelPredicate(193-197)getResizeChannelPredicate(205-209)sdk/src/rpc/parse/parse.ts (2)
parseCreateChannelResponse(107-107)parseResizeChannelResponse(110-110)sdk/src/rpc/api.ts (1)
createResizeChannelMessage(579-594)integration/common/testHelpers.ts (1)
composeResizeChannelParams(175-190)
integration/tests/nitrorpc_v04.test.ts (1)
integration/common/testHelpers.ts (2)
createTestChannels(28-49)toRaw(21-23)
integration/common/blockchainUtils.ts (1)
sdk/src/client/services/NitroliteService.ts (1)
custodyAddress(92-94)
⏰ 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 (9)
sdk/src/client/state.ts (1)
50-52: LGTM! Zero-allocation validation correctly enforced.The validation properly enforces the zero-deposit requirement for channel creation at the client side, with a clear error message. The check is positioned correctly before signing operations.
sdk/test/unit/client/state.test.ts (1)
56-57: LGTM! Test coverage properly updated for zero-allocation requirement.The tests correctly reflect the new validation rule with updated fixtures (lines 56-57, 80) and comprehensive negative test case (lines 123-133) that verifies the error path when a non-zero first allocation is provided.
Also applies to: 80-80, 123-133
integration/common/blockchainUtils.ts (1)
20-20: LGTM! Well-structured helper method.The new
getChannelBalancemethod follows the existing pattern fromgetErc20Balanceand correctly reads channel balances from the custody contract with proper decimal handling and error wrapping.Also applies to: 104-136
integration/common/testHelpers.ts (2)
34-34: LGTM! Helper correctly captures initial channel states.The updated signature returning both
channelIdsandstatesis necessary to support the new flow where tests need access to initial states for subsequent resize and challenge operations. Note this is a breaking change for any code using this helper.Also applies to: 36-36, 39-39, 45-45, 48-48
175-190: LGTM! Helpful test utility for resize operations.The
composeResizeChannelParamshelper cleanly constructsResizeChannelParamsfrom RPC response data, properly mapping fields and including the previous state as proof. This reduces test boilerplate and ensures consistent parameter construction.clearnode/channel_service.go (1)
50-50: Clarify TODO comment: Is zero-allocation the final solution?The TODO suggests this is a temporary fix requiring a future solution. However, since
Amountwas removed fromCreateChannelParams(inrpc_router_private.go), there's no parameter to use instead of the hardcoded zero. Either:
- This IS the final solution and the TODO should be removed, or
- There's a planned future approach that should be documented
Please clarify the intent and either remove the TODO or document the planned solution.
integration/tests/challenge_channel.test.ts (1)
48-48: LGTM! Test correctly updated for new channel creation flow.The test properly destructures both
paramsandstatefrom the channel creation response. The comment at line 54 helpfully explains that version 1 results from the resize operation following the zero-deposit channel creation.Also applies to: 54-54
clearnode/rpc_router_private.go (1)
85-86: LGTM! API change correctly enforces zero-deposit requirement.Removing the
Amountfield fromCreateChannelParamsis the key change that enforces zero-deposit channel creation at the protocol level. Clients must now use the separateresize_channeloperation to add funds.integration/tests/nitrorpc_v04.test.ts (1)
6-6: LGTM! Test correctly updated to use channel states.The test properly imports
State, destructures states fromcreateTestChannels, and uses the state version to initializecurrentVersion. Changes are minimal and correctly aligned with the updated helper signature.Also applies to: 72-73, 87-87
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
clearnode/channel_service.go(1 hunks)sdk/src/client/types.ts(1 hunks)sdk/src/rpc/types/request.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- clearnode/channel_service.go
- sdk/src/rpc/types/request.ts
⏰ 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)
4be9be3 to
09bfb4b
Compare
Summary by CodeRabbit
New Features
Changes
Documentation
Removals