YNU-829: Update Data Models and Communication Flows diagrams#616
Merged
YNU-829: Update Data Models and Communication Flows diagrams#616
Conversation
dimast-x
requested review from
alessio,
ihsraham,
nksazonov and
philanton
as code owners
Contributor
📝 WalkthroughWalkthroughDocumentation updates refine wording, rename transition enums, replace cycleId with epoch for state IDs, expand Node-side validation into detailed lock/gate/verify sequences, add signer-type byte semantics, restructure multi-phase escrow/home flows with on-chain events, and fully remodel the data model with new entities and enumerations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Node
participant HomeChain
participant DB
User->>Client: Initiate state transition (signed state with signer-type byte)
Client->>Node: SubmitState(userSig, packedState)
Node->>Node: ActionGateway.AllowAction
Node->>Node: LockUserState
Node->>DB: GetLastUserState
DB-->>Node: currentState
Node->>Node: EnsureNoOngoingStateTransitions
Node->>Node: ValidateStateAdvancement(currentState, incomingState)
Node->>Node: PackState
Node->>Node: ExtractSignerType & Verify via ChannelSigValidator
Node->>Node: nodeSigner.Sign(packedState) → nodeSig
Node->>DB: RecordTransaction / StoreUserState
Node-->>Client: Return nodeSig
Client->>HomeChain: CheckpointState(packedState, nodeSig, userSig)
HomeChain-->>Client: TxHash (HomeChannelCheckpointed)
HomeChain->>Node: Emit HomeChannelCheckpointed event
Node->>DB: Update channel.StateVersion / Status
Node-->>Client: ChannelUpdate, BalanceUpdate
Client-->>User: Success / TxHash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
docs/communication_flows/escrow_chan_withdrawal.mmd (1)
55-55: Minor inconsistency: Missing signer type byte note.Line 23 notes that
signState(state)prepends a signer type byte, but line 55 omits this detail. For consistency across the diagram, consider updating this note to match.- Note over Client: signState(state) returns userSig + Note over Client: signState(state) returns userSig (prepends signer type byte)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/communication_flows/escrow_chan_withdrawal.mmd` at line 55, The note at "Note over Client: signState(state) returns userSig" is missing the signer type byte detail; update this note so it matches the earlier description (e.g., "signState(state) prepends signer type byte and returns userSig") so that signState(state) and userSig are described consistently across the diagram; ensure the exact phrasing mirrors the earlier note about prepending the signer type byte.docs/communication_flows/escrow_chan_deposit.mmd (1)
64-64: Minor inconsistency: Missing signer type byte note.For consistency with line 23, the Phase 4
signStatenote should also mention the signer type byte prepending.- Note over Client: signState(state) returns userSig + Note over Client: signState(state) returns userSig (prepends signer type byte)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/communication_flows/escrow_chan_deposit.mmd` at line 64, Update the Phase 4 note for signState(state) so it matches line 23's wording by mentioning that the signer type byte is prepended to the signature; specifically, edit the note "Note over Client: signState(state) returns userSig" to indicate that signState returns a signature with a signer type byte prepended (e.g., signer-type + userSig) so the signState(state) description is consistent with the earlier signer type byte note.
🤖 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/communication_flows/home_chan_creation_from_scratch.mmd`:
- Around line 27-35: The diagram omits passing approvedSigValidators into
CreateChannel even though GetHomeChannelID, the prior validations, and Channel
persistence use it; update the flow so CreateChannel(homeChannelID, userWallet,
asset, Home, blockchainID, token, nonce, challenge, approvedSigValidators)
(i.e., include approvedSigValidators as an input) to match
GetHomeChannelID(...), the IsChannelSignerSupported/ChannelSigValidator.Verify
checks, and the Channel persistence in clearnode/store/database/channel.go;
ensure the CreateChannel node in the mermaid flow includes that parameter name
so the diagram accurately reflects channel identity and signer-policy being
stored.
In `@docs/data_models.mmd`:
- Around line 160-163: The AppSessionV1 diagram currently defines application_id
as plain varchar while the referenced primary key AppV1.id and other FKs use
varchar~66~; update the AppSessionV1.application_id field to use varchar~66~
(matching AppV1.id and sibling FKs) so the foreign key width/type is consistent
across the data model diagram.
- Around line 112-122: The documented column types for the int256-backed fields
are incorrect: change transition_amount and all balance/net_flow columns
(transition_amount, home_user_balance, home_user_net_flow, home_node_balance,
home_node_net_flow, escrow_user_balance, escrow_user_net_flow,
escrow_node_balance, escrow_node_net_flow) from `numeric` to the actual storage
type `varchar(78)` so the data model matches the store implementation that
persists these int256 values as 78-char varchars.
In `@docs/README.md`:
- Around line 32-33: Update the README.md fenced code block to include a
language specifier and fix the typo in the comment: replace the existing line
containing "cerebro/" (the directory entry) where the comment reads "Cerebro
Testing CLient" with "Cerebro Testing Client" and wrap the block with a
language-tagged fence (e.g., ```text) so the block becomes a properly fenced
code sample; ensure the opening fence includes the language and the closing
fence is present.
---
Nitpick comments:
In `@docs/communication_flows/escrow_chan_deposit.mmd`:
- Line 64: Update the Phase 4 note for signState(state) so it matches line 23's
wording by mentioning that the signer type byte is prepended to the signature;
specifically, edit the note "Note over Client: signState(state) returns userSig"
to indicate that signState returns a signature with a signer type byte prepended
(e.g., signer-type + userSig) so the signState(state) description is consistent
with the earlier signer type byte note.
In `@docs/communication_flows/escrow_chan_withdrawal.mmd`:
- Line 55: The note at "Note over Client: signState(state) returns userSig" is
missing the signer type byte detail; update this note so it matches the earlier
description (e.g., "signState(state) prepends signer type byte and returns
userSig") so that signState(state) and userSig are described consistently across
the diagram; ensure the exact phrasing mirrors the earlier note about prepending
the signer type byte.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78b42512-e4ac-40db-b1fa-15eaf850be56
📒 Files selected for processing (10)
docs/README.mddocs/communication_flows/app_session_deposit.mmddocs/communication_flows/escrow_chan_deposit.mmddocs/communication_flows/escrow_chan_withdrawal.mmddocs/communication_flows/home_chan_creation_from_scratch.mmddocs/communication_flows/home_chan_deposit.mmddocs/communication_flows/home_chan_withdraw.mmddocs/communication_flows/home_chan_withdraw_on_create_from_state.mmddocs/communication_flows/transfer.mmddocs/data_models.mmd
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
docs/data_models.mmd (1)
112-121:⚠️ Potential issue | 🟠 MajorUse the persisted column types for the int256-backed fields.
Lines 112, 115, 117, 119, and 121 are still documented as
numeric, but the current store model persiststransition_amount,home_user_balance,home_node_balance,escrow_user_balance, andescrow_node_balanceasvarchar(78). The diagram is still out of sync withclearnode/store/database/state.go:38-85.📝 Suggested doc fix
- +numeric transition_amount + +varchar~78~ transition_amount +char~66~ home_channel_id FK +char~66~ escrow_channel_id FK - +numeric home_user_balance + +varchar~78~ home_user_balance +numeric home_user_net_flow - +numeric home_node_balance + +varchar~78~ home_node_balance +numeric home_node_net_flow - +numeric escrow_user_balance + +varchar~78~ escrow_user_balance +numeric escrow_user_net_flow - +numeric escrow_node_balance + +varchar~78~ escrow_node_balance +numeric escrow_node_net_flow🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data_models.mmd` around lines 112 - 121, Update the documented column types for the int256-backed fields to match the persisted schema: change transition_amount, home_user_balance, home_node_balance, escrow_user_balance, and escrow_node_balance from "numeric" to "varchar(78)" in docs/data_models.mmd so the diagram aligns with the store model (the persisted column type used for those fields).
🧹 Nitpick comments (2)
docs/communication_flows/escrow_chan_deposit.mmd (2)
64-64: InconsistentsignStatenote between phases.Line 23 documents that
signState(state)"prepends signer type byte", but line 64 omits this detail. For consistency across the document, consider adding the same clarification:- Note over Client: signState(state) returns userSig + Note over Client: signState(state) returns userSig (prepends signer type byte)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/communication_flows/escrow_chan_deposit.mmd` at line 64, The "Note over Client: signState(state) returns userSig" is missing the detail that signState(state) "prepends signer type byte"; update this note to match the earlier phase's wording (e.g., "signState(state) returns userSig (prepends signer type byte)") so the signState semantics are consistent across the document and match the description near signState(state).
67-70: Consider expanding the condensed validation flow for consistency.Phase 4's Node validation (lines 67-70) uses a heavily condensed notation compared to Phase 1 (lines 26-33). While this reduces repetition, it may cause readers to miss important details like:
approvedSigValidatorsfromCheckOpenChannel- Signer type extraction and verification
If brevity is intentional for repeated flows, consider adding a note like "Same validation pipeline as Phase 1" to make the relationship explicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/communication_flows/escrow_chan_deposit.mmd` around lines 67 - 70, The condensed Phase 4 Node validation sequence (LockUserState, CheckOpenChannel, GetLastUserState, EnsureNoOngoingStateTransitions, ValidateStateAdvancement, PackState, verify sig, node sign, RecordTransaction, StoreUserState(state)) omits details present in Phase 1 (e.g., approvedSigValidators from CheckOpenChannel and signer type extraction/verification); update the diagram text to either expand Phase 4 to explicitly include those steps (mention approvedSigValidators and signer type extraction/verification where CheckOpenChannel/verify sig appear) or add a clear note like "Same validation pipeline as Phase 1 (includes approvedSigValidators and signer type extraction/verification)" to make the relationship explicit.
🤖 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/communication_flows/app_session_deposit.mmd`:
- Line 39: The flow calls VerifyQuorum(packedAppState, sigQuorum, participants)
but never produces packedAppState; insert a step that serializes/packs
newAppState into packedAppState just before the VerifyQuorum call (e.g.,
"packedAppState = Pack(newAppState)" or a descriptive box labeled "Pack
newAppState into packedAppState") so the sequence explicitly shows creation of
packedAppState from newAppState prior to calling VerifyQuorum; reference the
symbols newAppState, packedAppState, and VerifyQuorum to locate where to add
this packing step.
- Around line 37-38: The sequence is missing the channel-level signer-policy
check: after extracting sigType (the current Note over Node) but before calling
ChannelSigValidator.Verify(userWallet, packedState, userSig), insert an explicit
IsChannelSignerSupported(approvedSigValidators, sigType) gate to enforce the
channel’s approvedSigValidators bitmask; update the diagram so the flow first
evaluates IsChannelSignerSupported(approvedSigValidators, sigType) and only
proceeds to ChannelSigValidator.Verify(...) when that check passes.
In `@docs/communication_flows/escrow_chan_withdrawal.mmd`:
- Around line 37-39: The sequence diagram flips payload ownership:
PackChannelDefinition and PackState are performed on Client but
initiateEscrowWithdrawal is invoked by Node (and similarly Pack on Node but
Client calls submit later). Ensure the actor that packs data also submits it —
move the initiateEscrowWithdrawal call to Client when
PackChannelDefinition/PackState run on Client, and move the submit call to Node
when packing happens on Node (or vice‑versa) so packedChannelDef/packedState
ownership remains consistent; update the sequence lines referencing
PackChannelDefinition, PackState, and initiateEscrowWithdrawal accordingly for
both Phase 2 and Phase 4.
In `@docs/communication_flows/transfer.mmd`:
- Around line 20-23: The flow uses mixed sender-side names (userWallet,
senderWallet, state/incomingState, senderState) which creates ambiguity; pick
and use a single sender-side identifier (e.g., senderWallet for the wallet and
senderState for the state) across all steps including ActionGateway.AllowAction,
LockUserState, CheckOpenChannel, GetLastUserState and the later state
references, and update any occurrences of userWallet, state, or incomingState to
that chosen name so the ownership is consistent throughout the diagram.
- Around line 36-41: The diagram signs packedReceiverState but never constructs
it; update the receiver branch so after
ApplyTransferReceiveTransition(senderWallet, amount, txID) you explicitly pack
the resulting receiverState (e.g., call PackState(receiverState) →
packedReceiverState) before the GetLastSignedState/If-last-signed checks, and
then pass that packedReceiverState into nodeSigner.Sign(packedReceiverState);
ensure the symbol packedReceiverState is introduced and used consistently
alongside GetLastUserState, ApplyTransferReceiveTransition, and nodeSigner.Sign.
---
Duplicate comments:
In `@docs/data_models.mmd`:
- Around line 112-121: Update the documented column types for the int256-backed
fields to match the persisted schema: change transition_amount,
home_user_balance, home_node_balance, escrow_user_balance, and
escrow_node_balance from "numeric" to "varchar(78)" in docs/data_models.mmd so
the diagram aligns with the store model (the persisted column type used for
those fields).
---
Nitpick comments:
In `@docs/communication_flows/escrow_chan_deposit.mmd`:
- Line 64: The "Note over Client: signState(state) returns userSig" is missing
the detail that signState(state) "prepends signer type byte"; update this note
to match the earlier phase's wording (e.g., "signState(state) returns userSig
(prepends signer type byte)") so the signState semantics are consistent across
the document and match the description near signState(state).
- Around line 67-70: The condensed Phase 4 Node validation sequence
(LockUserState, CheckOpenChannel, GetLastUserState,
EnsureNoOngoingStateTransitions, ValidateStateAdvancement, PackState, verify
sig, node sign, RecordTransaction, StoreUserState(state)) omits details present
in Phase 1 (e.g., approvedSigValidators from CheckOpenChannel and signer type
extraction/verification); update the diagram text to either expand Phase 4 to
explicitly include those steps (mention approvedSigValidators and signer type
extraction/verification where CheckOpenChannel/verify sig appear) or add a clear
note like "Same validation pipeline as Phase 1 (includes approvedSigValidators
and signer type extraction/verification)" to make the relationship explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb0da117-22d6-4add-82ae-a4f19b99bc16
📒 Files selected for processing (10)
docs/README.mddocs/communication_flows/app_session_deposit.mmddocs/communication_flows/escrow_chan_deposit.mmddocs/communication_flows/escrow_chan_withdrawal.mmddocs/communication_flows/home_chan_creation_from_scratch.mmddocs/communication_flows/home_chan_deposit.mmddocs/communication_flows/home_chan_withdraw.mmddocs/communication_flows/home_chan_withdraw_on_create_from_state.mmddocs/communication_flows/transfer.mmddocs/data_models.mmd
Comment on lines
+37
to
+38
| Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key) | ||
| Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig) |
Contributor
There was a problem hiding this comment.
Add the signer-policy gate before ChannelSigValidator.Verify.
Line 37 extracts sigType, but Line 38 goes straight to ChannelSigValidator.Verify(...). That validator checks signature ownership; it does not enforce the channel’s approvedSigValidators bitmask. Without an explicit IsChannelSignerSupported(approvedSigValidators, sigType) step, the diagram drops a material authorization check that the other channel flows now document.
📝 Suggested doc fix
Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key)
+ Note over Node: Verify signer type approved: IsChannelSignerSupported(approvedSigValidators, sigType)
Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig)📝 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.
Suggested change
| Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key) | |
| Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig) | |
| Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key) | |
| Note over Node: Verify signer type approved: IsChannelSignerSupported(approvedSigValidators, sigType) | |
| Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/communication_flows/app_session_deposit.mmd` around lines 37 - 38, The
sequence is missing the channel-level signer-policy check: after extracting
sigType (the current Note over Node) but before calling
ChannelSigValidator.Verify(userWallet, packedState, userSig), insert an explicit
IsChannelSignerSupported(approvedSigValidators, sigType) gate to enforce the
channel’s approvedSigValidators bitmask; update the diagram so the flow first
evaluates IsChannelSignerSupported(approvedSigValidators, sigType) and only
proceeds to ChannelSigValidator.Verify(...) when that check passes.
| Note over Node: PackState(incomingState) → packedState | ||
| Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key) | ||
| Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig) | ||
| Note over Node: VerifyQuorum(packedAppState, sigQuorum, participants) |
Contributor
There was a problem hiding this comment.
Define packedAppState before quorum verification.
Line 39 verifies quorum over packedAppState, but the flow never packs or serializes newAppState into that value. As written, the sequence consumes an input that is never produced.
📝 Suggested doc fix
Note over Node: PackState(incomingState) → packedState
+ Note over Node: PackAppState(newAppState) → packedAppState
Note over Node: Extract signer type from userSig (0x00=default, 0x01=session key)
Note over Node: ChannelSigValidator.Verify(userWallet, packedState, userSig)
Note over Node: VerifyQuorum(packedAppState, sigQuorum, participants)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/communication_flows/app_session_deposit.mmd` at line 39, The flow calls
VerifyQuorum(packedAppState, sigQuorum, participants) but never produces
packedAppState; insert a step that serializes/packs newAppState into
packedAppState just before the VerifyQuorum call (e.g., "packedAppState =
Pack(newAppState)" or a descriptive box labeled "Pack newAppState into
packedAppState") so the sequence explicitly shows creation of packedAppState
from newAppState prior to calling VerifyQuorum; reference the symbols
newAppState, packedAppState, and VerifyQuorum to locate where to add this
packing step.
Comment on lines
37
to
39
| Note over Client: PackChannelDefinition(channelDef) | ||
| Note over Client: PackState(channelId, state) | ||
| Node->>EscrowChain: initiateEscrowWithdrawal(packedChannelDef, packedState) |
Contributor
There was a problem hiding this comment.
Keep packing on the actor that submits the chain call.
In Phase 2, Lines 37-38 build packedChannelDef/packedState on Client, but Line 39 has Node submit them. In Phase 4, Line 64 packs on Node, but Line 65 has Client submit packedState. The payload ownership flips in both places without any explicit handoff.
📝 Suggested doc fix
- Note over Client: PackChannelDefinition(channelDef)
- Note over Client: PackState(channelId, state)
+ Note over Node: PackChannelDefinition(channelDef)
+ Note over Node: PackState(channelId, state)
Node->>EscrowChain: initiateEscrowWithdrawal(packedChannelDef, packedState)
…
- Note over Node: PackState(channelId, state)
+ Note over Client: PackState(channelId, state)
Client->>EscrowChain: finalizeEscrowWithdrawal(escrowChannelId, packedState)Also applies to: 64-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/communication_flows/escrow_chan_withdrawal.mmd` around lines 37 - 39,
The sequence diagram flips payload ownership: PackChannelDefinition and
PackState are performed on Client but initiateEscrowWithdrawal is invoked by
Node (and similarly Pack on Node but Client calls submit later). Ensure the
actor that packs data also submits it — move the initiateEscrowWithdrawal call
to Client when PackChannelDefinition/PackState run on Client, and move the
submit call to Node when packing happens on Node (or vice‑versa) so
packedChannelDef/packedState ownership remains consistent; update the sequence
lines referencing PackChannelDefinition, PackState, and initiateEscrowWithdrawal
accordingly for both Phase 2 and Phase 4.
Comment on lines
+20
to
+23
| Note over Node: ActionGateway.AllowAction(userWallet, GatedActionTransfer) | ||
| Note over Node: LockUserState(senderWallet, asset) | ||
| Note over Node: CheckOpenChannel(senderWallet, asset) → approvedSigValidators | ||
| Note over Node: GetLastUserState(senderWallet, asset) → currentState |
Contributor
There was a problem hiding this comment.
Use one sender-side name throughout this node flow.
Line 20 switches to userWallet, Lines 21-29 use senderWallet, and Lines 45-46 switch again to senderState even though the submitted object has been state/incomingState up to that point. In a sender/receiver flow, that ambiguity makes the ownership of each step harder to follow than it needs to be.
📝 Suggested doc fix
- Note over Node: ActionGateway.AllowAction(userWallet, GatedActionTransfer)
+ Note over Node: ActionGateway.AllowAction(senderWallet, GatedActionTransfer)
…
- Note over Node: NewTransactionFromTransition(senderState, receiverState, transition) → RecordTransaction
- Note over Node: StoreUserState(senderState) → also updates sender UserBalance
+ Note over Node: NewTransactionFromTransition(incomingState, receiverState, transition) → RecordTransaction
+ Note over Node: StoreUserState(incomingState) → also updates sender UserBalanceAlso applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/communication_flows/transfer.mmd` around lines 20 - 23, The flow uses
mixed sender-side names (userWallet, senderWallet, state/incomingState,
senderState) which creates ambiguity; pick and use a single sender-side
identifier (e.g., senderWallet for the wallet and senderState for the state)
across all steps including ActionGateway.AllowAction, LockUserState,
CheckOpenChannel, GetLastUserState and the later state references, and update
any occurrences of userWallet, state, or incomingState to that chosen name so
the ownership is consistent throughout the diagram.
Comment on lines
+36
to
+41
| Note over Node: GetLastUserState(receiverWallet, asset) → receiverState (or VoidState) | ||
| Note over Node: receiverState.NextState() → increment version | ||
| Note over Node: ApplyTransferReceiveTransition(senderWallet, amount, txID) | ||
| Note over Node: GetLastSignedState(receiverWallet, asset) | ||
| Note over Node: If last signed was MutualLock or EscrowLock → skip signing | ||
| Note over Node: Else if HomeChannelID exists → nodeSigner.Sign(packedReceiverState) |
Contributor
There was a problem hiding this comment.
Introduce packedReceiverState before signing it.
Line 41 signs packedReceiverState, but the receiver branch never produces that value. Every other signing path in these diagrams explicitly packs the state first, so the payload is currently undefined.
📝 Suggested doc fix
Note over Node: GetLastSignedState(receiverWallet, asset)
Note over Node: If last signed was MutualLock or EscrowLock → skip signing
+ Note over Node: Else if HomeChannelID exists → PackState(receiverState) → packedReceiverState
Note over Node: Else if HomeChannelID exists → nodeSigner.Sign(packedReceiverState)
Note over Node: StoreUserState(receiverState) → also updates receiver UserBalance📝 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.
Suggested change
| Note over Node: GetLastUserState(receiverWallet, asset) → receiverState (or VoidState) | |
| Note over Node: receiverState.NextState() → increment version | |
| Note over Node: ApplyTransferReceiveTransition(senderWallet, amount, txID) | |
| Note over Node: GetLastSignedState(receiverWallet, asset) | |
| Note over Node: If last signed was MutualLock or EscrowLock → skip signing | |
| Note over Node: Else if HomeChannelID exists → nodeSigner.Sign(packedReceiverState) | |
| Note over Node: GetLastUserState(receiverWallet, asset) → receiverState (or VoidState) | |
| Note over Node: receiverState.NextState() → increment version | |
| Note over Node: ApplyTransferReceiveTransition(senderWallet, amount, txID) | |
| Note over Node: GetLastSignedState(receiverWallet, asset) | |
| Note over Node: If last signed was MutualLock or EscrowLock → skip signing | |
| Note over Node: Else if HomeChannelID exists → PackState(receiverState) → packedReceiverState | |
| Note over Node: Else if HomeChannelID exists → nodeSigner.Sign(packedReceiverState) | |
| Note over Node: StoreUserState(receiverState) → also updates receiver UserBalance |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/communication_flows/transfer.mmd` around lines 36 - 41, The diagram
signs packedReceiverState but never constructs it; update the receiver branch so
after ApplyTransferReceiveTransition(senderWallet, amount, txID) you explicitly
pack the resulting receiverState (e.g., call PackState(receiverState) →
packedReceiverState) before the GetLastSignedState/If-last-signed checks, and
then pass that packedReceiverState into nodeSigner.Sign(packedReceiverState);
ensure the symbol packedReceiverState is introduced and used consistently
alongside GetLastUserState, ApplyTransferReceiveTransition, and nodeSigner.Sign.
philanton
approved these changes
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit