MF3-L17, I15, I16: app-session participant docs, Void-checkpoint promotion, signature canonicalization note#828
Conversation
Implementation enforces only the maximum participant count, so a single-participant app session is valid. Update the create_app_session validation docs to state "at least 1 participant required" instead of 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ChannelHub.createChannel can emit ChannelCheckpointed before ChannelCreated for an initial non-deposit/non-withdraw state. If the checkpoint is processed while the local row is still the Void seed, HandleHomeChannelCheckpointed bumped state_version but left the channel Void, violating the protocol invariant until the later ChannelCreated event replayed and repaired it. Treat a checkpoint on a Void home channel as evidence the channel is materialized on-chain and promote it to Open, backfilling any unsigned concurrent receiver head (mirrors HandleHomeChannelCreated). The later ChannelCreated then no-ops via the Status >= Open guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Accepted user_sig/node_sig values are persisted verbatim rather than re-encoded after verification, so they may retain non-canonical or unused trailing bytes (notably around session-key payloads, and for signatures learned from on-chain events that bypass the WebSocket message-size limit). Document this under Known Limitations: no asset-safety impact, but consumers must not assume stored signature bytes are minimal or canonical. Co-Authored-By: Claude Opus 4.8 (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 |
nksazonov
left a comment
nksazonov
left a comment
There was a problem hiding this comment.
Good job — the three audit findings are addressed correctly and the fix logic is sound; two small things to tighten before landing.
There was a problem hiding this comment.
Approving for L-17 / I-15 / I-16 closure. The docs now match the intended single-participant app-session behavior, the checkpoint path promotes only Void home channels to Open, and the stored-signature limitation is documented clearly.
Agree to Nikita's existing non-blocking notes. One small docs cleanup remains: the create-app-session README sample now shows one participant but still has two signature placeholders. That does not reopen L-17, but it is worth tightening before merge.
Resolve conflict in nitronode/event_handlers/service_test.go: keep both the Void-promotion checkpoint test (adapted to the base branch's read-under-lock LockUserStateForHomeChannel API) and the base branch's DoesNotReopenFinalizedChannel test. Also fix two pre-existing breakages inherited from the base branch: - channel_v1/submit_state_test.go referenced the removed MockActionGateway / actionGateway field (action gateway was deleted on base) — drop the stale line. - channel_v1/submit_session_key_state_test.go carried a contradictory AllowsEmptyAssetsWithPastExpiresAt test (version-1 revoke expecting success) alongside the new RevokeFirstSubmit_Rejected rule (version-1 revoke rejected) — remove the obsolete test, superseded by RevokeFirstSubmit_Rejected / RevokeExistingActiveKey. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- HandleHomeChannelCheckpointed: snapshot the pre-checkpoint status once and derive wasVoid/wasChallenged from it, so neither branch depends on the other's mutation order (the Void branch could otherwise flip Status to Open before wasChallenged is read). - Add TestHandleHomeChannelCheckpointed_FromVoidBackfillsUnsignedReceiverHead covering the Void→Open arm of the head-sig backfill (transfer_receive and release), so dropping the `|| wasVoid` clause is caught by CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
Addresses three audit findings.
MF3-L17 — single-participant app sessions (docs)
create_app_sessiondocs claimed "at least 2 participants required", butCreateAppSession()only enforces the max participant count and quorum — a single-participant session withquorum = 1is valid. We decided to keep single-participant app sessions, so the doc was the divergence. Updatedapp_session_v1/README.mdto "at least 1 participant required" to match implementation.MF3-I15 — Void channel left un-promoted on checkpoint (fix)
ChannelHub.createChannel()can emitChannelCheckpointedbeforeChannelCreatedfor an initial non-deposit/non-withdraw state. If the checkpoint was processed while the local row was still theVoidseed,HandleHomeChannelCheckpointed()bumpedstate_versionbut left the channelVoid, violating the protocol invariant until the laterChannelCreatedevent replayed and repaired it.Fix: treat a checkpoint on a
Voidhome channel as evidence it is materialized on-chain and promote it toOpen, backfilling any unsigned concurrent receiver head (mirrorsHandleHomeChannelCreated). The laterChannelCreatedthen no-ops via theStatus >= Openguard. Added a regression test.MF3-I16 — stored signatures not canonicalized (docs)
channel_states.user_sig/node_sigare unboundedtextand accepted signatures are persisted verbatim rather than re-encoded after verification. They may retain non-canonical / unused trailing bytes (notably around session-key payloads, and for signatures learned from on-chain events that bypass the WebSocket message-size limit). No asset-safety impact — verification still gates acceptance. Documented under Known Limitations so consumers don't assume stored signature bytes are minimal/canonical.🤖 Generated with Claude Code