MF2-I03: reject reused home channel ID explicitly#766
Conversation
After a challenged channel close, issueChallengeRescue() emits a fresh-epoch state with HomeChannelID == nil. The same-home-channel-id check in request_creation() was gated on currentState.HomeChannelID != nil, so the invariant was enforced indirectly by the channels.channel_id primary key on CreateChannel rather than by handler validation. That produced opaque storage errors instead of a deterministic rejection. Look up the channel by ID before CreateChannel and reject explicitly, regardless of prior-state shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
… fix/nitronode-mf2-i03 # Conflicts: # nitronode/api/channel_v1/request_creation_test.go
nksazonov
left a comment
nksazonov
left a comment
There was a problem hiding this comment.
Good job! The audit finding is fully addressed with a clean, well-tested fix.
…hannel-lookup error - Rename the error wrapping GetChannelByID to "failed to look up channel by computed id" so it does not collide in logs with the GetLastUserState wrap. - Add TestRequestCreation_ChannelAlreadyInitialized covering the prior-state-non-final guard when the computed home channel ID is not yet stored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ihsraham
left a comment
ihsraham
left a comment
There was a problem hiding this comment.
Thanks for tightening this up. The latest head closes I-03 for me: reused computed home channel IDs are rejected before CreateChannel, including the detached-state case, and the added regression coverage covers the closure path. Approving.
- MF2-I02: fix: add SESSION_KEY_AUTH_TYPEHASH to session key authorization payload (#767) - MF2-M02: accept pre-finalize escrow version in ongoing check (#765) - MF2-L03: docs: document home chain migration as not yet active off-chain (#769) - MF2-I03: reject reused home channel ID explicitly (#766) - MF2-H03: guard challenge_rescue against post-Finalize close (#768) - MF2-C02: bound ledger values to Solidity uint256/int256 ranges (#764) - MF2-C01: preserve post-Finalize closing marker across challenge cycle (#762) - MF2-M01: fix(sdk): use keccak256(utf8) for session key application ID hashing (#761) - MF2-H01: harden receiver-state issuance and dispute resolution (#759) - MF2-L01: gate transfer_send during channel creation (#763)
3 participants
Summary
issueChallengeRescue()emits a fresh-epoch state withHomeChannelID == nil. The same-home-channel-id check inrequest_creation()was gated oncurrentState.HomeChannelID != nil, so when the ledger head was a post-rescue state the explicit reject was skipped and the invariant fell back to thechannels.channel_idprimary key onCreateChannel.failed to create channel: duplicate key) and the business rule lives in the storage layer instead of the handler.homeChannelIDbeforeCreateChanneland rejects withcannot use same home channel idregardless of prior-state shape. Thechannel is already initializedguard for non-final prior states with a non-nilHomeChannelIDis preserved.Test plan
go test ./nitronode/api/channel_v1/...go vet ./nitronode/api/channel_v1/...TestRequestCreation_RejectReusedHomeChannelIDcovers post-rescue head (HomeChannelID == nil) with a prior channel record still inchannels— must returncannot use same home channel idbefore reachingCreateChannel/StoreUserState.🤖 Generated with Claude Code