diff --git a/nitronode/api/app_session_v1/handler.go b/nitronode/api/app_session_v1/handler.go index eed203fe8..ac30997c4 100644 --- a/nitronode/api/app_session_v1/handler.go +++ b/nitronode/api/app_session_v1/handler.go @@ -170,6 +170,16 @@ func (h *Handler) issueReleaseReceiverState(ctx context.Context, tx Store, recei // unsigned so the challenge-rescue squash at close (Challenged path) can pick // it up, and so terminal-status channels never receive a node-signed credit // that won't settle. + // + // MF3-I01 (release path): same reasoning as channel_v1. The listener + // ordering & idempotency invariant (pkg/blockchain/evm/listener.go, + // processEvents doc) guarantees HandleHomeChannelChallenged precedes + // HandleHomeChannelClosed for any Path-1 (challenge-timeout) close, so + // an unsigned release credit either lands before the close handler runs + // (and is squashed into the rescue sum) or after it (and reads the + // rescue row as currentState, with HomeChannelID=nil). The wedge state + // where currentState transitively points at a Closed channel is not + // reachable through the supported event-ingestion path. _, channelStatus, err := tx.CheckActiveChannel(receiverWallet, asset) if err != nil { return rpc.Errorf("failed to check receiver active channel: %v", err) diff --git a/nitronode/api/channel_v1/handler.go b/nitronode/api/channel_v1/handler.go index bda8ab70d..a9153f158 100644 --- a/nitronode/api/channel_v1/handler.go +++ b/nitronode/api/channel_v1/handler.go @@ -123,6 +123,17 @@ func (h *Handler) issueTransferReceiverState(ctx context.Context, tx Store, send // candidate (Challenged) or commit a credit that will never settle (Closed, // Closing). The unsigned row is still persisted so the challenge-rescue // squash at close can pick it up. + // + // MF3-I01: persisting an unsigned row whose HomeChannelID still references + // a now-Closed channel is safe under the listener ordering & idempotency + // invariant (pkg/blockchain/evm/listener.go, processEvents doc). For any + // Path-1 (challenge-timeout) close, HandleHomeChannelChallenged has + // already run before HandleHomeChannelClosed, so the rescue issued from + // HandleHomeChannelClosed has overwritten currentState with a fresh-epoch + // row whose HomeChannelID is nil, and the next call here reads that row + // rather than the wedge state. The unsigned credit issued here can only + // land on a Closed channel if it commits before the close handler runs + // — in which case the close handler's rescue sum picks it up. _, channelStatus, err := tx.CheckActiveChannel(receiverWallet, senderState.Asset) if err != nil { return nil, rpc.Errorf("failed to check receiver active channel: %v", err) diff --git a/nitronode/event_handlers/service.go b/nitronode/event_handlers/service.go index 604ba20e7..732a76621 100644 --- a/nitronode/event_handlers/service.go +++ b/nitronode/event_handlers/service.go @@ -312,16 +312,24 @@ func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, t // finalized and closed on-chain. It updates the channel status to Closed and sets the final state version. // Once closed, no further state updates are possible for this channel. // -// Additionally, when the closing path was a challenge resolution (channel was Challenged -// and no node-signed Finalize state exists for it), the handler issues a single -// ChallengeRescue state for the user that squashes the sum of receiver-state credits -// accrued during the challenge window into an off-channel ledger entry tied to the -// closed channel's ID. When a node-signed Finalize already exists, the cooperative-close -// path has already advanced the user to a fresh epoch via NextState(), and any receiver -// credits accumulated post-Finalize live at (epoch+1, version=0..N) with HomeChannelID=nil -// — issuing a rescue at (epoch+1, version=1) would either overwrite the lone receiver -// credit or collide on deterministic state ID with an existing row, so the rescue is -// skipped entirely in that case. +// Additionally, when the channel was locally Challenged at the time of close, the handler +// issues a ChallengeRescue state crediting the user the net receiver-minus-sender balance +// accrued strictly above the closure version. The rescue runs unconditionally on the +// Challenged → Closed transition: both path-1 (timeout on a stale candidate) and path-2 +// (cooperative close on a signed Finalize) routes pass through this branch, and the +// constructor + prev source pick the correct placement for the rescue row. +// +// MF3-I01 recovery anchor. This handler is the single recovery point for the wedge +// state described in audit finding MF3-I01: a receiver credit issued during the +// challenge window inherits HomeChannelID from currentState via NextState(), so the +// user's latest stored state can transiently point at a channel that closes via path-1 +// before the next receiver-credit issuance reads currentState again. The listener +// ordering & idempotency invariant (pkg/blockchain/evm/listener.go, see processEvents +// doc) guarantees HandleHomeChannelChallenged has already run for any path-1 close, so +// wasChallenged is true here and the rescue advances the user past the closed channel. +// Subsequent receiver-credit issuance reads the rescue row as currentState and no +// longer carries the closed channel reference, so request_creation can reopen on the +// same (wallet, asset) through the normal flow. func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx core.ChannelHubEventHandlerStore, event *core.HomeChannelClosedEvent) error { logger := log.FromContext(ctx) chanID := event.ChannelID @@ -368,24 +376,9 @@ func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx co } if wasChallenged { - // Post-Finalize cooperative close racing with an on-chain challenge: receiver - // credits issued after the Finalize live in a fresh epoch via NextState() and - // are not channel-attached, so the rescue path would either overwrite them or - // collide on deterministic state ID. Skip the rescue branch entirely. - finalized, err := tx.HasSignedFinalize(chanID) - if err != nil { + if err := s.issueChallengeRescue(ctx, tx, channel, event.StateVersion); err != nil { return err } - if finalized { - logger.Debug("skipping challenge_rescue for post-Finalize close", - "channelId", chanID, - "userWallet", channel.UserWallet, - "closureVersion", event.StateVersion) - } else { - if err := s.issueChallengeRescue(ctx, tx, channel, event.StateVersion); err != nil { - return err - } - } } logger.Info("handled HomeChannelClosed event", "channelId", event.ChannelID, "stateVersion", event.StateVersion, "userWallet", channel.UserWallet) @@ -393,37 +386,37 @@ func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx co } // issueChallengeRescue emits a ChallengeRescue state on the user's ledger after a -// challenged-channel close. The state is issued unconditionally so the user's latest -// stored state moves to a fresh epoch with HomeChannelID nil; without it future -// receiver-state issuance and channels.v1.request_creation would stay wedged on the -// closed channel. The rescue amount is the NET effect on the user's home-channel -// balance of transitions stored strictly above closureVersion — receives -// (TransferReceive, Release) credit the user, sends (TransferSend, Commit) debit, -// everything else is excluded because it requires onchain backing the chain didn't -// enforce or belongs to a different ledger. Signed (Open-time) and unsigned -// (during-challenge) rows both contribute. The result is clamped at zero so an -// adversarial close at a version where the user's own balance was higher than the -// off-chain head can't dock the user further. AccountID is the closed channel's ID; -// HomeChannelID is nil — the shape of a credit to a user with no open home channel. +// challenged-channel close. The state advances the user's chain past the closed +// channel so future receiver issuance and channels.v1.request_creation no longer +// wedge on it. +// +// Amount: NET effect on the user's home-channel balance of transitions stored +// strictly above closureVersion — receives (TransferReceive, Release) credit the +// user, sends (TransferSend, Commit) debit; everything else is excluded because it +// requires onchain backing the chain didn't enforce or belongs to a different +// ledger. Signed (Open-time) and unsigned (during-challenge) rows both contribute. +// Clamped at zero so an adversarial close at a version where the user's own balance +// was higher than the off-chain head can't dock the user further. +// +// Placement: prev is the user's latest state (across both channel-attached and +// detached rows). When prev is the in-channel head, the rescue wraps to a fresh +// epoch at (E+1, 0). When prev is a detached tip — the case where a node-signed +// Finalize already advanced the user via NextState() and post-Finalize receiver +// credits live at (E+1, v=0..M) with HomeChannelID nil — the rescue appends at +// (E+1, M+1), inheriting prev's ledger. NewChallengeRescueState picks the branch. +// +// AccountID on the rescue transition is the closed channel's ID; the rescue row +// itself has HomeChannelID nil — the shape of a credit to a user with no open home +// channel, to be folded into a signed state when the user next opens one. func (s *EventHandlerService) issueChallengeRescue(ctx context.Context, tx core.ChannelHubEventHandlerStore, channel *core.Channel, closureVersion uint64) error { logger := log.FromContext(ctx) - prev, err := tx.GetLastStateByChannelID(channel.ChannelID, false) - if err != nil { - return err - } - if prev == nil { - // Should not happen for a channel that reached Challenged → Closed: at least the - // closure state itself must be on file. Surface the inconsistency rather than - // silently dropping the rescue. - return fmt.Errorf("no state found for closed challenged channel %s", channel.ChannelID) - } - // Strict `>` against closureVersion: the row at the closure version itself is the // closing state and must be excluded — only transitions issued strictly after the - // dispute version are unenforced. The epoch filter pins the sum to prev.Epoch (the - // closed channel's epoch) as a defense against any future DB inconsistency. - net, err := tx.SumNetTransitionAmountAfterVersion(channel.ChannelID, closureVersion, prev.Epoch) + // dispute version are unenforced. A channel's in-channel rows live at a single + // epoch; detached post-Finalize rows have HomeChannelID NULL and are already + // excluded by the channel_id predicate, so no epoch filter is needed. + net, err := tx.SumNetTransitionAmountAfterVersion(channel.ChannelID, closureVersion) if err != nil { return err } @@ -439,20 +432,26 @@ func (s *EventHandlerService) issueChallengeRescue(ctx context.Context, tx core. "channelId", channel.ChannelID, "netAmount", net.String(), "closureVersion", closureVersion, - "prevVersion", prev.Version, ) total = decimal.Zero } - // Invariant: any row included in the sum sits strictly above closureVersion, so the - // channel's off-chain head must too. A non-zero net with prev at or below closure - // means the state chain disagrees with itself — surface it before issuing the rescue. - if !total.IsZero() && prev.Version <= closureVersion { - return fmt.Errorf("challenge_rescue: non-zero net (%s) but prev v=%d <= closure v=%d on channel %s", - total.String(), prev.Version, closureVersion, channel.ChannelID) + // prev is the user's latest state across both channel-attached and detached rows. + // When a node-signed Finalize already advanced the user via NextState() at sign + // time, prev is the detached tip and the rescue appends after it. Otherwise prev + // is the channel's own head and the rescue wraps to a fresh epoch. + prev, err := tx.GetLastUserState(channel.UserWallet, channel.Asset, false) + if err != nil { + return err + } + if prev == nil { + // Should not happen for a channel that reached Challenged → Closed: at least the + // closure state itself must be on file. Surface the inconsistency rather than + // silently dropping the rescue. + return fmt.Errorf("no state found for closed challenged channel %s", channel.ChannelID) } - rescue, err := core.NewChallengeRescueState(*prev, total) + rescue, err := core.NewChallengeRescueState(*prev, channel.ChannelID, total) if err != nil { return err } diff --git a/nitronode/event_handlers/service_test.go b/nitronode/event_handlers/service_test.go index a560532ea..def94ca2a 100644 --- a/nitronode/event_handlers/service_test.go +++ b/nitronode/event_handlers/service_test.go @@ -1611,9 +1611,8 @@ func TestHandleHomeChannelClosed_ChallengeRescue_Squash(t *testing.T) { })).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(false, nil) - mockStore.On("GetLastStateByChannelID", channelID, false).Return(prevState, nil) - mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion, prevState.Epoch).Return(rescueAmount, nil) + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(rescueAmount, nil) + mockStore.On("GetLastUserState", userWallet, asset, false).Return(prevState, nil) var capturedState core.State mockStore.On("StoreUserState", mock.MatchedBy(func(state core.State) bool { @@ -1713,9 +1712,8 @@ func TestHandleHomeChannelClosed_ChallengeRescue_NoCredits(t *testing.T) { })).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(false, nil) - mockStore.On("GetLastStateByChannelID", channelID, false).Return(prevState, nil) - mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion, prevState.Epoch).Return(decimal.Zero, nil) + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(decimal.Zero, nil) + mockStore.On("GetLastUserState", userWallet, asset, false).Return(prevState, nil) var capturedState core.State mockStore.On("StoreUserState", mock.MatchedBy(func(state core.State) bool { @@ -1792,9 +1790,8 @@ func TestHandleHomeChannelClosed_ChallengeRescue_NegativeNet_ClampsToZero(t *tes mockStore.On("UpdateChannel", mock.Anything).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(false, nil) - mockStore.On("GetLastStateByChannelID", channelID, false).Return(prevState, nil) - mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion, prevState.Epoch).Return(decimal.NewFromInt(-49), nil) + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(decimal.NewFromInt(-49), nil) + mockStore.On("GetLastUserState", userWallet, asset, false).Return(prevState, nil) var capturedState core.State mockStore.On("StoreUserState", mock.MatchedBy(func(state core.State) bool { @@ -1816,15 +1813,18 @@ func TestHandleHomeChannelClosed_ChallengeRescue_NegativeNet_ClampsToZero(t *tes mockStore.AssertExpectations(t) } -// TestHandleHomeChannelClosed_PostFinalize_SkipsRescue pins behavior for the -// cooperative-close-racing-an-onchain-challenge path: the node has already signed a -// Finalize state for this channel, and post-Finalize receiver credits live in a fresh -// epoch via NextState() with HomeChannelID=nil. Issuing a rescue at (epoch+1, version=0) -// in that case would either overwrite a lone receiver credit (silent balance loss) or -// collide on deterministic state ID with an existing row (StoreUserState fails and rolls -// back the close, leaving the channel stuck Challenged). The handler must short-circuit -// the rescue branch entirely when HasSignedFinalize reports true. -func TestHandleHomeChannelClosed_PostFinalize_SkipsRescue(t *testing.T) { +// TestHandleHomeChannelClosed_TimeoutAfterFinalize_AppendsRescue pins the path-1 +// timeout close arriving after a node-signed Finalize already advanced the user's +// chain. The chain settled at a version Y strictly below the Finalize version F: +// the user's true off-chain claim was userAlloc(F), but on-chain only userAlloc(Y) +// was paid. The shortfall is the net of receiver/sender transitions in (Y, F]. +// +// At sign time, NextState() created a detached fresh-epoch chain at (E+1, v=0..M) +// holding post-Finalize receiver credits with HomeChannelID nil. Placing the rescue +// at (E+1, v=0) would collide on deterministic state ID with the first detached +// row. The handler instead appends after the detached tip at (E+1, M+1), +// inheriting the tip's ledger and adding the shortfall on top. +func TestHandleHomeChannelClosed_TimeoutAfterFinalize_AppendsRescue(t *testing.T) { mockStore := new(MockStore) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -1833,7 +1833,9 @@ func TestHandleHomeChannelClosed_PostFinalize_SkipsRescue(t *testing.T) { channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" asset := "USDC" - closureVersion := uint64(7) + closureVersion := uint64(5) // Y; Finalize was at a higher version F. + rescueAmount := decimal.NewFromInt(80) + priorDetachedBalance := decimal.NewFromInt(20) expiryTime := time.Now().Add(time.Hour) channel := &core.Channel{ @@ -1842,10 +1844,26 @@ func TestHandleHomeChannelClosed_PostFinalize_SkipsRescue(t *testing.T) { Asset: asset, Type: core.ChannelTypeHome, Status: core.ChannelStatusChallenged, - StateVersion: 5, + StateVersion: 4, ChallengeExpiresAt: &expiryTime, } + // Detached tip: post-Finalize receiver at (E+1, v=3) with HomeChannelID nil. + detachedTip := &core.State{ + ID: core.GetStateID(userWallet, asset, 2, 3), + Asset: asset, + UserWallet: userWallet, + Epoch: 2, + Version: 3, + HomeChannelID: nil, + HomeLedger: core.Ledger{ + UserBalance: priorDetachedBalance, + UserNetFlow: decimal.Zero, + NodeBalance: decimal.Zero, + NodeNetFlow: priorDetachedBalance, + }, + } + event := &core.HomeChannelClosedEvent{ ChannelID: channelID, StateVersion: closureVersion, @@ -1860,48 +1878,73 @@ func TestHandleHomeChannelClosed_PostFinalize_SkipsRescue(t *testing.T) { })).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(true, nil) + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(rescueAmount, nil) + mockStore.On("GetLastUserState", userWallet, asset, false).Return(detachedTip, nil) + + var capturedState core.State + mockStore.On("StoreUserState", mock.MatchedBy(func(state core.State) bool { + capturedState = state + return true + }), "").Return(nil) + var capturedTx core.Transaction + mockStore.On("RecordTransaction", mock.MatchedBy(func(tx core.Transaction) bool { + capturedTx = tx + return true + }), "").Return(nil) err := service.HandleHomeChannelClosed(ctx, mockStore, event) require.NoError(t, err) + require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) + require.Equal(t, channelID, capturedState.Transition.AccountID) + require.True(t, rescueAmount.Equal(capturedState.Transition.Amount)) + require.Nil(t, capturedState.HomeChannelID, "rescue stays off-channel") + + // Append at (detachedTip.Epoch, detachedTip.Version+1), inheriting prior balance. + require.Equal(t, detachedTip.Epoch, capturedState.Epoch) + require.Equal(t, detachedTip.Version+1, capturedState.Version) + require.True(t, priorDetachedBalance.Add(rescueAmount).Equal(capturedState.HomeLedger.UserBalance), + "want %s, got %s", priorDetachedBalance.Add(rescueAmount).String(), capturedState.HomeLedger.UserBalance.String()) + require.True(t, priorDetachedBalance.Add(rescueAmount).Equal(capturedState.HomeLedger.NodeNetFlow)) + + require.Equal(t, core.TransactionTypeChallengeRescue, capturedTx.TxType) + require.Equal(t, channelID, capturedTx.FromAccount) + require.Equal(t, userWallet, capturedTx.ToAccount) + require.True(t, rescueAmount.Equal(capturedTx.Amount)) + mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) } -// TestHandleHomeChannelClosed_PostFinalize_CollisionRegression documents the exact -// failure modes the guard prevents. Two post-Finalize receiver credits already exist on -// the user's ledger at (epoch+1, version=0) and (epoch+1, version=1) — produced by the -// normal NextState() path off the Finalize state with HomeChannelID=nil. Without the -// guard the handler would: -// - call GetLastStateByChannelID(channelID), which only matches channel-attached -// rows and therefore returns the Finalize state at the original epoch, -// - call SumNetTransitionAmountAfterVersion(channelID, ...), which filters by -// home_channel_id and returns zero (post-Finalize credits have HomeChannelID=nil), -// - construct a rescue state at (Finalize.Epoch+1, version=0) with amount=0, -// - StoreUserState on that ID, which collides deterministically with the existing -// (epoch+1, version=0) row via GetStateID(wallet, asset, epoch, version) and -// rolls the transaction back, stranding the channel in Challenged. +// TestHandleHomeChannelClosed_CooperativeCloseAfterChallenge_ZeroRescue pins the +// cooperative-close-after-local-challenge race: the operator counter-submitted a +// Finalize at version F while the channel was Challenged, the user then accepted +// cooperative CLOSE at the same version F, and the close event arrives with +// StateVersion == F. NextState() at sign time detached the post-Finalize chain at +// (E+1, v=0..M) with HomeChannelID nil. // -// With the guard, HasSignedFinalize=true short-circuits the entire branch before any -// of those DB calls fire. The mock asserts none of them are reached. +// SumNetTransitionAmountAfterVersion(channelID, F) must collapse to zero by the +// SQL predicate, with no intent gate required: +// - In-channel rows live at versions <= F (closure version is the channel head). +// - Post-Finalize detached rows have home_channel_id NULL and are excluded by +// the channel_id equality predicate. // -// Intentional twin of TestHandleHomeChannelClosed_PostFinalize_SkipsRescue: setup and -// AssertNotCalled set are identical at the mock level; this case exists to document the -// specific collision failure modes the guard prevents. Do not collapse the two. -func TestHandleHomeChannelClosed_PostFinalize_CollisionRegression(t *testing.T) { +// The rescue must therefore emit a zero-amount state appended after the detached +// tip at (E+1, M+1), inheriting the tip's ledger unchanged — chain still advances +// so future receiver issuance and channels.v1.request_creation no longer wedge on +// the closed channel, but no extra balance is credited beyond what the detached +// chain already holds. This is the invariant that lets the unconditional rescue +// branch be safe without forwarding finalState.intent through the reactor. +func TestHandleHomeChannelClosed_CooperativeCloseAfterChallenge_ZeroRescue(t *testing.T) { mockStore := new(MockStore) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) service, _ := newTestEventHandlerService(t) - channelID := "0xHomeChannelCollision" + channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" asset := "USDC" - closureVersion := uint64(10) + closureVersion := uint64(7) // F: cooperative CLOSE settles at the Finalize version. + priorDetachedBalance := decimal.NewFromInt(40) expiryTime := time.Now().Add(time.Hour) channel := &core.Channel{ @@ -1910,10 +1953,26 @@ func TestHandleHomeChannelClosed_PostFinalize_CollisionRegression(t *testing.T) Asset: asset, Type: core.ChannelTypeHome, Status: core.ChannelStatusChallenged, - StateVersion: 9, + StateVersion: 6, ChallengeExpiresAt: &expiryTime, } + // Detached tip: post-Finalize receiver credits at (E+1, v=2) with HomeChannelID nil. + detachedTip := &core.State{ + ID: core.GetStateID(userWallet, asset, 2, 2), + Asset: asset, + UserWallet: userWallet, + Epoch: 2, + Version: 2, + HomeChannelID: nil, + HomeLedger: core.Ledger{ + UserBalance: priorDetachedBalance, + UserNetFlow: decimal.Zero, + NodeBalance: decimal.Zero, + NodeNetFlow: priorDetachedBalance, + }, + } + event := &core.HomeChannelClosedEvent{ ChannelID: channelID, StateVersion: closureVersion, @@ -1928,60 +1987,45 @@ func TestHandleHomeChannelClosed_PostFinalize_CollisionRegression(t *testing.T) })).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(true, nil) - - err := service.HandleHomeChannelClosed(ctx, mockStore, event) - require.NoError(t, err, "post-Finalize close must succeed; rescue branch must be skipped") - - mockStore.AssertExpectations(t) - // The collision-class operations must not be invoked once the guard fires. - mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) -} - -// TestHandleHomeChannelClosed_PostFinalize_HasSignedFinalizeErr ensures errors from the -// HasSignedFinalize lookup are surfaced rather than silently dropping the rescue guard. -func TestHandleHomeChannelClosed_PostFinalize_HasSignedFinalizeErr(t *testing.T) { - mockStore := new(MockStore) - ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - - service, _ := newTestEventHandlerService(t) - - channelID := "0xHomeChannel123" - userWallet := "0x1234567890123456789012345678901234567890" - asset := "USDC" - closureVersion := uint64(7) + // Key invariant: the predicate collapses cooperative CLOSE to a zero net. In-channel + // rows are at versions <= F, detached rows are excluded by home_channel_id = ?. + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(decimal.Zero, nil) + mockStore.On("GetLastUserState", userWallet, asset, false).Return(detachedTip, nil) - channel := &core.Channel{ - ChannelID: channelID, - UserWallet: userWallet, - Asset: asset, - Type: core.ChannelTypeHome, - Status: core.ChannelStatusChallenged, - StateVersion: 5, - } + var capturedState core.State + mockStore.On("StoreUserState", mock.MatchedBy(func(state core.State) bool { + capturedState = state + return true + }), "").Return(nil) + var capturedTx core.Transaction + mockStore.On("RecordTransaction", mock.MatchedBy(func(tx core.Transaction) bool { + capturedTx = tx + return true + }), "").Return(nil) - event := &core.HomeChannelClosedEvent{ - ChannelID: channelID, - StateVersion: closureVersion, - } + err := service.HandleHomeChannelClosed(ctx, mockStore, event) + require.NoError(t, err) - dbErr := errors.New("db down") - mockStore.On("GetChannelByID", channelID).Return(channel, nil) - mockStore.On("LockUserState", userWallet, asset).Return(decimal.Zero, nil) - mockStore.On("UpdateChannel", mock.Anything).Return(nil) - mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) - mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - mockStore.On("HasSignedFinalize", channelID).Return(false, dbErr) + require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) + require.Equal(t, channelID, capturedState.Transition.AccountID) + require.True(t, capturedState.Transition.Amount.IsZero(), + "cooperative CLOSE after challenge must produce zero-amount rescue, got %s", capturedState.Transition.Amount.String()) + require.Nil(t, capturedState.HomeChannelID, "rescue stays off-channel") + + // Append after the detached tip: (E, M+1), inheriting the tip's ledger unchanged. + require.Equal(t, detachedTip.Epoch, capturedState.Epoch) + require.Equal(t, detachedTip.Version+1, capturedState.Version) + require.True(t, priorDetachedBalance.Equal(capturedState.HomeLedger.UserBalance), + "balance must be unchanged from detached tip, want %s, got %s", + priorDetachedBalance.String(), capturedState.HomeLedger.UserBalance.String()) + require.True(t, priorDetachedBalance.Equal(capturedState.HomeLedger.NodeNetFlow)) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) - require.ErrorIs(t, err, dbErr) + require.Equal(t, core.TransactionTypeChallengeRescue, capturedTx.TxType) + require.Equal(t, channelID, capturedTx.FromAccount) + require.Equal(t, userWallet, capturedTx.ToAccount) + require.True(t, capturedTx.Amount.IsZero()) - mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) + mockStore.AssertExpectations(t) } // TestHandleHomeChannelClosed_OpenChannel_NoRescue pins behavior for the normal @@ -2022,7 +2066,7 @@ func TestHandleHomeChannelClosed_OpenChannel_NoRescue(t *testing.T) { require.NoError(t, err) mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) } diff --git a/nitronode/event_handlers/testing.go b/nitronode/event_handlers/testing.go index 1edf76527..dfe5be52f 100644 --- a/nitronode/event_handlers/testing.go +++ b/nitronode/event_handlers/testing.go @@ -21,6 +21,16 @@ func (m *MockStore) GetLastStateByChannelID(channelID string, signed bool) (*cor return args.Get(0).(*core.State), args.Error(1) } +// GetLastUserState mocks retrieving the most recent state for a user's asset across +// all channels and detached chain entries. +func (m *MockStore) GetLastUserState(wallet, asset string, signed bool) (*core.State, error) { + args := m.Called(wallet, asset, signed) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*core.State), args.Error(1) +} + // GetStateByChannelIDAndVersion mocks retrieving a specific state version for a channel func (m *MockStore) GetStateByChannelIDAndVersion(channelID string, version uint64) (*core.State, error) { args := m.Called(channelID, version) @@ -102,8 +112,8 @@ func (m *MockStore) UpdateStateSigsIfMissing(channelID string, version uint64, u // SumNetTransitionAmountAfterVersion mocks the net-change query used to compute // challenge-rescue amounts on a closed channel. -func (m *MockStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) { - args := m.Called(channelID, minVersion, epoch) +func (m *MockStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) { + args := m.Called(channelID, minVersion) return args.Get(0).(decimal.Decimal), args.Error(1) } diff --git a/nitronode/store/database/db_store_test.go b/nitronode/store/database/db_store_test.go index 44fc5ba1d..72125e980 100644 --- a/nitronode/store/database/db_store_test.go +++ b/nitronode/store/database/db_store_test.go @@ -1103,7 +1103,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 2, core.TransitionTypeTransferReceive, decimal.NewFromFloat(0.001), false) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 1, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 1) require.NoError(t, err) assert.True(t, decimal.NewFromFloat(0.001).Equal(net), "want 0.001, got %s", net.String()) }) @@ -1123,7 +1123,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 4, core.TransitionTypeTransferSend, decimal.NewFromInt(1), true) storeState(t, store, 1, 5, core.TransitionTypeTransferReceive, decimal.NewFromInt(1), true) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2) require.NoError(t, err) assert.True(t, decimal.NewFromInt(1).Equal(net), "want 1, got %s", net.String()) }) @@ -1142,7 +1142,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 3, core.TransitionTypeHomeDeposit, decimal.NewFromInt(300000), true) storeState(t, store, 1, 4, core.TransitionTypeTransferReceive, decimal.NewFromInt(1), true) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2) require.NoError(t, err) assert.True(t, decimal.NewFromInt(1).Equal(net), "want 1, got %s", net.String()) }) @@ -1160,7 +1160,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 2, core.TransitionTypeTransferReceive, decimal.NewFromInt(1), true) storeState(t, store, 1, 3, core.TransitionTypeTransferSend, decimal.NewFromInt(50), true) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 1, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 1) require.NoError(t, err) assert.True(t, decimal.NewFromInt(-49).Equal(net), "want -49, got %s", net.String()) }) @@ -1178,7 +1178,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 4, core.TransitionTypeCommit, decimal.NewFromInt(3), true) storeState(t, store, 1, 5, core.TransitionTypeRelease, decimal.NewFromInt(1), false) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2) require.NoError(t, err) assert.True(t, decimal.NewFromInt(3).Equal(net), "want 3, got %s", net.String()) }) @@ -1199,7 +1199,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 7, core.TransitionTypeMigrate, decimal.NewFromInt(5), true) storeState(t, store, 1, 8, core.TransitionTypeFinalize, decimal.NewFromInt(0), true) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 2) require.NoError(t, err) assert.True(t, net.IsZero(), "want 0, got %s", net.String()) }) @@ -1210,7 +1210,7 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { store := NewDBStore(db) setupChannel(t, store) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 0, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 0) require.NoError(t, err) assert.True(t, net.IsZero()) }) @@ -1226,26 +1226,10 @@ func TestDBStore_SumNetTransitionAmountAfterVersion(t *testing.T) { storeState(t, store, 1, 6, core.TransitionTypeTransferReceive, decimal.NewFromInt(99), true) storeState(t, store, 1, 7, core.TransitionTypeTransferReceive, decimal.NewFromInt(3), false) - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 6, 1) + net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 6) require.NoError(t, err) assert.True(t, decimal.NewFromInt(3).Equal(net), "want 3, got %s", net.String()) }) - - t.Run("Excludes rows in other epochs", func(t *testing.T) { - db, cleanup := SetupTestDB(t) - defer cleanup() - store := NewDBStore(db) - setupChannel(t, store) - - // Two rows above cutoff: one in epoch 1, one in epoch 2. Caller asks for epoch 1 - // only; the epoch-2 row must be excluded even though every other predicate matches. - storeState(t, store, 1, 7, core.TransitionTypeTransferReceive, decimal.NewFromInt(30), false) - storeState(t, store, 2, 8, core.TransitionTypeTransferReceive, decimal.NewFromInt(999), false) - - net, err := store.SumNetTransitionAmountAfterVersion(homeChannelID, 6, 1) - require.NoError(t, err) - assert.True(t, decimal.NewFromInt(30).Equal(net), "want 30, got %s", net.String()) - }) } func TestDBStore_HasSignedFinalize(t *testing.T) { diff --git a/nitronode/store/database/interface.go b/nitronode/store/database/interface.go index 1230b4207..98d5fc408 100644 --- a/nitronode/store/database/interface.go +++ b/nitronode/store/database/interface.go @@ -112,11 +112,11 @@ type DatabaseStore interface { // SumNetTransitionAmountAfterVersion returns the net effect on the user's // home-channel balance of transitions stored against channelID strictly above - // minVersion at the supplied epoch. Receiver credits (TransferReceive, Release) - // contribute positively; sender debits (TransferSend, Commit) contribute negatively. - // Other transition kinds are excluded. Used to compute the ChallengeRescue amount - // when a challenged channel is closed. - SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) + // minVersion. Receiver credits (TransferReceive, Release) contribute positively; + // sender debits (TransferSend, Commit) contribute negatively. Other transition + // kinds are excluded. Used to compute the ChallengeRescue amount when a + // challenged channel is closed. + SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) // --- Blockchain Action Operations --- diff --git a/nitronode/store/database/state.go b/nitronode/store/database/state.go index 35655a96c..01357a973 100644 --- a/nitronode/store/database/state.go +++ b/nitronode/store/database/state.go @@ -270,12 +270,12 @@ func (s *DBStore) HasSignedFinalize(channelID string) (bool, error) { // SumNetTransitionAmountAfterVersion returns the net effect on the user's home-channel -// balance of transitions stored against channelID strictly above minVersion at the -// supplied epoch. Receiver credits (TransferReceive, Release) contribute positively; -// sender debits (TransferSend, Commit) contribute negatively. Other transition kinds -// (HomeDeposit, HomeWithdrawal, escrow ops, migrate, finalize, acknowledgement) are -// excluded because they either require onchain backing the chain didn't enforce at -// this closure or belong to a different ledger. +// balance of transitions stored against channelID strictly above minVersion. Receiver +// credits (TransferReceive, Release) contribute positively; sender debits +// (TransferSend, Commit) contribute negatively. Other transition kinds (HomeDeposit, +// HomeWithdrawal, escrow ops, migrate, finalize, acknowledgement) are excluded because +// they either require onchain backing the chain didn't enforce at this closure or +// belong to a different ledger. // // Used to compute the ChallengeRescue amount when a challenged channel is closed: // onchain payout reflects the closure version, anything strictly above didn't enforce, @@ -283,7 +283,11 @@ func (s *DBStore) HasSignedFinalize(channelID string) (bool, error) { // sends in that interval. Signed (Open-time) and unsigned (during-challenge) rows // both contribute — both are real value the state-advancer validated at submit time. // The caller clamps the result at zero before issuing the rescue. -func (s *DBStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) { +// +// No epoch filter: a channel's in-channel rows live at a single epoch; detached +// post-Finalize rows have home_channel_id NULL and are already excluded by the +// channel_id predicate. +func (s *DBStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) { cid := strings.ToLower(channelID) var row struct { Net decimal.Decimal `gorm:"column:net"` @@ -296,14 +300,13 @@ func (s *DBStore) SumNetTransitionAmountAfterVersion(channelID string, minVersio END), 0) AS net FROM channel_states WHERE home_channel_id = ? - AND epoch = ? AND version > ? `, uint8(core.TransitionTypeTransferReceive), uint8(core.TransitionTypeRelease), uint8(core.TransitionTypeTransferSend), uint8(core.TransitionTypeCommit), - cid, epoch, minVersion, + cid, minVersion, ).Scan(&row).Error if err != nil { return decimal.Zero, fmt.Errorf("failed to compute net transition amount: %w", err) diff --git a/pkg/blockchain/evm/channel_hub_reactor.go b/pkg/blockchain/evm/channel_hub_reactor.go index e21b9bd97..e25296e25 100644 --- a/pkg/blockchain/evm/channel_hub_reactor.go +++ b/pkg/blockchain/evm/channel_hub_reactor.go @@ -33,6 +33,10 @@ type ChannelHubReactorStore interface { // Returns nil if no matching state exists. GetLastStateByChannelID(channelID string, signed bool) (*core.State, error) + // GetLastUserState retrieves the most recent state for a user's asset across all + // channels and detached chain entries. Returns nil if no matching state exists. + GetLastUserState(wallet, asset string, signed bool) (*core.State, error) + // GetStateByChannelIDAndVersion retrieves a specific state version for a channel. // Returns nil if the state with the specified version does not exist. GetStateByChannelIDAndVersion(channelID string, version uint64) (*core.State, error) @@ -89,9 +93,9 @@ type ChannelHubReactorStore interface { // SumNetTransitionAmountAfterVersion returns the net effect on the user's // home-channel balance of transitions stored against channelID strictly above - // minVersion at the supplied epoch. Receiver credits (TransferReceive, Release) - // contribute positively; sender debits (TransferSend, Commit) contribute negatively. - SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) + // minVersion. Receiver credits (TransferReceive, Release) contribute positively; + // sender debits (TransferSend, Commit) contribute negatively. + SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) // StoreUserState persists a user state row. Used to record a ChallengeRescue squash // state derived from a closed challenged channel. diff --git a/pkg/blockchain/evm/channel_hub_reactor_test.go b/pkg/blockchain/evm/channel_hub_reactor_test.go index 885b93a78..818ade62e 100644 --- a/pkg/blockchain/evm/channel_hub_reactor_test.go +++ b/pkg/blockchain/evm/channel_hub_reactor_test.go @@ -29,6 +29,14 @@ func (m *mockChannelHubStore) GetLastStateByChannelID(channelID string, signed b return args.Get(0).(*core.State), args.Error(1) } +func (m *mockChannelHubStore) GetLastUserState(wallet, asset string, signed bool) (*core.State, error) { + args := m.Called(wallet, asset, signed) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*core.State), args.Error(1) +} + func (m *mockChannelHubStore) GetStateByChannelIDAndVersion(channelID string, version uint64) (*core.State, error) { args := m.Called(channelID, version) if args.Get(0) == nil { @@ -95,8 +103,8 @@ func (m *mockChannelHubStore) UpdateStateSigsIfMissing(channelID string, version return args.Error(0) } -func (m *mockChannelHubStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) { - args := m.Called(channelID, minVersion, epoch) +func (m *mockChannelHubStore) SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) { + args := m.Called(channelID, minVersion) return args.Get(0).(decimal.Decimal), args.Error(1) } diff --git a/pkg/blockchain/evm/listener.go b/pkg/blockchain/evm/listener.go index e20f37978..1e2a013d7 100644 --- a/pkg/blockchain/evm/listener.go +++ b/pkg/blockchain/evm/listener.go @@ -189,6 +189,43 @@ func (l *Listener) listenEvents(ctx context.Context) error { // events are checked via IsContractEventPresent; once a non-present event is found // the check is skipped for the rest of that phase (events are strictly ordered). // Returns nil on subscription loss (reconnect), non-nil on handler/check failure. +// +// Listener ordering & idempotency invariant +// ----------------------------------------- +// Downstream handlers (and any code reasoning about the relative arrival order +// of on-chain events) may rely on the following guarantees provided by this +// loop. Changes that weaken any of them must update every consumer that cites +// this invariant by name. +// +// 1. Strict per-contract ordering. Within a single Listener, events are +// delivered to handleEvent in ascending (block_number, log_index) order +// across the historical → live transition. Phase 1 drains historicalCh to +// completion before phase 2 reads from currentCh, and the upstream +// reconcileBlockRange + live subscription preserve chain order within each +// phase. +// +// 2. Idempotent resume. On restart, IsContractEventPresent gates the first +// event of each phase: events already persisted in a prior run are skipped +// rather than reprocessed. Once a non-present event is seen the check is +// dropped for the remainder of the phase (safe because of guarantee 1). +// +// 3. Cursor advances only on handler success. lastBlock is updated on each +// live event, but a non-nil return from handleEvent unsubscribes and +// surfaces the error to the caller without persisting any state past the +// failed event; the next Listen invocation re-fetches from the same +// cursor. Transient handler failures retry instead of silently dropping. +// +// 4. Reorged-out logs are discarded. Live deliveries with Removed=true are +// dropped. A reorg that fully removes a ChannelChallenged log also +// removes the matching on-chain status transition to DISPUTED, so the +// contract's Path-1 (challenge-timeout) close cannot subsequently fire +// for the same channel. +// +// A consequence used by the nitronode event handlers: for any channel that +// closes via Path-1 (challenge-timeout, ChannelHub Closed-from-DISPUTED), +// HandleHomeChannelChallenged is guaranteed to run before HandleHomeChannelClosed +// for that channel. See nitronode/event_handlers/service.go (audit finding +// MF3-I01) for the wedge case this rules out. func (l *Listener) processEvents( ctx context.Context, eventSubscription interface { diff --git a/pkg/core/interface.go b/pkg/core/interface.go index 8f59ea1f7..0830b94a0 100644 --- a/pkg/core/interface.go +++ b/pkg/core/interface.go @@ -107,6 +107,11 @@ type ChannelHubEventHandlerStore interface { // Returns nil if no matching state exists. GetLastStateByChannelID(channelID string, signed bool) (*State, error) + // GetLastUserState retrieves the most recent state for a user's asset across all + // channels and detached chain entries (HomeChannelID nil). Returns nil if no + // matching state exists. If signed is true, only fully co-signed states are returned. + GetLastUserState(wallet, asset string, signed bool) (*State, error) + // GetStateByChannelIDAndVersion retrieves a specific state version for a channel. // Returns nil if the state with the specified version does not exist. GetStateByChannelIDAndVersion(channelID string, version uint64) (*State, error) @@ -165,11 +170,11 @@ type ChannelHubEventHandlerStore interface { // SumNetTransitionAmountAfterVersion returns the net effect on the user's // home-channel balance of transitions stored against channelID strictly above - // minVersion at the supplied epoch. Receiver credits (TransferReceive, Release) - // contribute positively; sender debits (TransferSend, Commit) contribute negatively. - // Other transition kinds are excluded. Used to compute the ChallengeRescue amount - // when a challenged channel is closed. - SumNetTransitionAmountAfterVersion(channelID string, minVersion, epoch uint64) (decimal.Decimal, error) + // minVersion. Receiver credits (TransferReceive, Release) contribute positively; + // sender debits (TransferSend, Commit) contribute negatively. Other transition + // kinds are excluded. Used to compute the ChallengeRescue amount when a + // challenged channel is closed. + SumNetTransitionAmountAfterVersion(channelID string, minVersion uint64) (decimal.Decimal, error) // StoreUserState persists a user state row. Used by the event handler to record a // ChallengeRescue squash state derived from a closed challenged channel. diff --git a/pkg/core/state_packer.go b/pkg/core/state_packer.go index 33647dfc9..15e2a80e9 100644 --- a/pkg/core/state_packer.go +++ b/pkg/core/state_packer.go @@ -41,6 +41,14 @@ func (l contractLedger) Validate() error { if err := checkInt256(l.NodeNetFlow); err != nil { return fmt.Errorf("node net flow: %w", err) } + allocSum := new(big.Int).Add(l.UserAllocation, l.NodeAllocation) + if err := checkUint256(allocSum); err != nil { + return fmt.Errorf("allocation sum: %w", err) + } + netFlowSum := new(big.Int).Add(l.UserNetFlow, l.NodeNetFlow) + if err := checkInt256(netFlowSum); err != nil { + return fmt.Errorf("net flow sum: %w", err) + } return nil } diff --git a/pkg/core/state_packer_test.go b/pkg/core/state_packer_test.go index 3119758d6..72f7ba3b8 100644 --- a/pkg/core/state_packer_test.go +++ b/pkg/core/state_packer_test.go @@ -205,3 +205,77 @@ func TestPackState_RejectsOutOfRangeValues(t *testing.T) { assert.Contains(t, err.Error(), "int256") }) } + +// TestContractLedger_Validate covers the ABI-layer guard directly, bypassing +// Ledger.Validate. Solidity computes `userAllocation + nodeAllocation` as +// uint256 and `userNetFlow + nodeNetFlow` as int256; individually-valid scaled +// values can still overflow these aggregates, so the packer's last-chance +// check must reject them before signing. +func TestContractLedger_Validate(t *testing.T) { + t.Parallel() + + maxUint256 := new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1)) + + t.Run("happy_path", func(t *testing.T) { + t.Parallel() + l := contractLedger{ + UserAllocation: big.NewInt(10), + NodeAllocation: big.NewInt(20), + UserNetFlow: big.NewInt(10), + NodeNetFlow: big.NewInt(20), + } + assert.NoError(t, l.Validate()) + }) + + t.Run("net_flow_sum_overflows_int256", func(t *testing.T) { + t.Parallel() + l := contractLedger{ + UserAllocation: big.NewInt(0), + NodeAllocation: big.NewInt(0), + UserNetFlow: new(big.Int).Set(maxInt256), + NodeNetFlow: big.NewInt(1), + } + err := l.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "net flow sum") + }) + + t.Run("net_flow_sum_underflows_int256", func(t *testing.T) { + t.Parallel() + l := contractLedger{ + UserAllocation: big.NewInt(0), + NodeAllocation: big.NewInt(0), + UserNetFlow: new(big.Int).Set(minInt256), + NodeNetFlow: big.NewInt(-1), + } + err := l.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "net flow sum") + }) + + t.Run("allocation_sum_overflows_uint256", func(t *testing.T) { + t.Parallel() + l := contractLedger{ + UserAllocation: new(big.Int).Set(maxUint256), + NodeAllocation: big.NewInt(1), + UserNetFlow: big.NewInt(0), + NodeNetFlow: big.NewInt(0), + } + err := l.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "allocation sum") + }) + + t.Run("net_flow_sum_at_boundary", func(t *testing.T) { + t.Parallel() + half := new(big.Int).Rsh(maxInt256, 1) + other := new(big.Int).Sub(maxInt256, half) + l := contractLedger{ + UserAllocation: big.NewInt(0), + NodeAllocation: big.NewInt(0), + UserNetFlow: half, + NodeNetFlow: other, + } + assert.NoError(t, l.Validate()) + }) +} diff --git a/pkg/core/types.go b/pkg/core/types.go index c38e2f58f..20e58ddbf 100644 --- a/pkg/core/types.go +++ b/pkg/core/types.go @@ -175,44 +175,61 @@ func NewVoidState(asset, userWallet string) *State { } } -// NewChallengeRescueState constructs a fully-formed ChallengeRescue state derived from -// prev — the user's last known state for the asset whose home channel is being closed -// after a challenge. The rescue state opens a fresh epoch at version 0 with no home -// channel and no token/blockchain context (the closed channel is gone); it credits the -// user the rescued amount as a standalone ledger entry referencing the closed channel -// (taken from prev.HomeChannelID) via the transition's AccountID. The rescue state is -// meant to be stored unsigned, like a credit to a user with no open home channel. +// NewChallengeRescueState constructs a ChallengeRescue state crediting amount to the +// user against closedChannelID. Placement depends on prev: // -// Version 0 matches the NextState() post-final convention: any in-protocol fresh-epoch -// state starts at version 0. Callers must guarantee no other (epoch+1, version=0) row -// already exists for this (wallet, asset) — for the rescue path this is enforced by -// only invoking it on non-Finalize challenge closes (see HandleHomeChannelClosed). +// - prev is in-channel (HomeChannelID != nil): the rescue opens a fresh epoch at +// (prev.Epoch+1, version=0) with a clean ledger seeded by amount. Used when no +// node-signed Finalize exists locally for the closed channel — the user's chain +// has not been advanced past prev yet. // -// Not a general-purpose constructor: the sole authorized callsite is -// EventHandlerService.issueChallengeRescue, which gates invocation on -// HasSignedFinalize=false. Adding a second callsite without an equivalent guard will -// cause StoreUserState to fail with a deterministic GetStateID collision against an -// existing (epoch+1, version=0) row. -func NewChallengeRescueState(prev State, amount decimal.Decimal) (*State, error) { - if prev.HomeChannelID == nil { - return nil, fmt.Errorf("prev state has no home channel ID") +// - prev is detached (HomeChannelID == nil): the rescue appends at (prev.Epoch, +// prev.Version+1), inheriting prev's ledger and adding amount on top. Used after +// a path-1 timeout close when a node-signed Finalize is on file: the sign-time +// NextState() has already advanced the user to a fresh epoch and post-Finalize +// receiver credits may already live there. Placing rescue at v=0 would collide +// on deterministic state ID; appending after the detached tip avoids that. +// +// closedChannelID is the on-chain channel whose close triggers the rescue. It is +// recorded as the rescue's transition AccountID so the credit is traceable back to +// the settlement. +func NewChallengeRescueState(prev State, closedChannelID string, amount decimal.Decimal) (*State, error) { + if closedChannelID == "" { + return nil, fmt.Errorf("closed channel ID is empty") } if amount.IsNegative() { return nil, fmt.Errorf("challenge rescue amount must be non-negative") } - closedChannelID := *prev.HomeChannelID - rescue := &State{ - Asset: prev.Asset, - UserWallet: prev.UserWallet, - Epoch: prev.Epoch + 1, - Version: 0, - HomeLedger: Ledger{ - UserBalance: amount, - UserNetFlow: decimal.Zero, - NodeBalance: decimal.Zero, - NodeNetFlow: amount, - }, + var rescue *State + if prev.HomeChannelID == nil { + // Detached tip: append at (prev.Epoch, prev.Version+1), carry ledger forward. + rescue = &State{ + Asset: prev.Asset, + UserWallet: prev.UserWallet, + Epoch: prev.Epoch, + Version: prev.Version + 1, + HomeLedger: Ledger{ + UserBalance: prev.HomeLedger.UserBalance.Add(amount), + UserNetFlow: prev.HomeLedger.UserNetFlow, + NodeBalance: prev.HomeLedger.NodeBalance, + NodeNetFlow: prev.HomeLedger.NodeNetFlow.Add(amount), + }, + } + } else { + // In-channel prev: wrap to a fresh epoch with a clean ledger seeded by amount. + rescue = &State{ + Asset: prev.Asset, + UserWallet: prev.UserWallet, + Epoch: prev.Epoch + 1, + Version: 0, + HomeLedger: Ledger{ + UserBalance: amount, + UserNetFlow: decimal.Zero, + NodeBalance: decimal.Zero, + NodeNetFlow: amount, + }, + } } rescue.ID = GetStateID(rescue.UserWallet, rescue.Asset, rescue.Epoch, rescue.Version) @@ -686,6 +703,17 @@ func (l Ledger) Validate(assetDecimals uint8) error { return fmt.Errorf("node net flow out of int256 range: %w", err) } + // Solidity computes `userAllocation + nodeAllocation` as uint256 and + // `userNetFlow + nodeNetFlow` as int256. Individually-valid scaled values + // can still overflow these aggregates, producing a state the contract will + // reject. + if _, err := DecimalToUint256(sumBalances, assetDecimals); err != nil { + return fmt.Errorf("allocation sum out of uint256 range: %w", err) + } + if _, err := DecimalToInt256(sumNetFlows, assetDecimals); err != nil { + return fmt.Errorf("net flow sum out of int256 range: %w", err) + } + return nil } diff --git a/pkg/core/types_test.go b/pkg/core/types_test.go index 042967a38..304fffccb 100644 --- a/pkg/core/types_test.go +++ b/pkg/core/types_test.go @@ -1,6 +1,7 @@ package core import ( + "math/big" "testing" "github.com/shopspring/decimal" @@ -215,11 +216,11 @@ func TestNewChallengeRescueState(t *testing.T) { } } - t.Run("Success - opens fresh epoch at version 0 with no channel context", func(t *testing.T) { + t.Run("Success - in-channel prev wraps to fresh epoch at version 0", func(t *testing.T) { prev := makePrev() amount := decimal.NewFromInt(42) - rescue, err := NewChallengeRescueState(prev, amount) + rescue, err := NewChallengeRescueState(prev, closedChannelID, amount) require.NoError(t, err) require.NotNil(t, rescue) @@ -244,9 +245,44 @@ func TestNewChallengeRescueState(t *testing.T) { assert.Equal(t, expectedTxID, rescue.Transition.TxID) }) + t.Run("Success - detached prev appends at next version inheriting ledger", func(t *testing.T) { + // Simulates path-1 timeout after a signed Finalize: user's chain already advanced + // via NextState() to a fresh epoch with post-Finalize receiver credits at v=0..M. + prev := State{ + Asset: "USDC", + UserWallet: "0xUser", + Epoch: 4, + Version: 2, + HomeChannelID: nil, + HomeLedger: Ledger{ + UserBalance: decimal.NewFromInt(10), + UserNetFlow: decimal.Zero, + NodeBalance: decimal.Zero, + NodeNetFlow: decimal.NewFromInt(10), + }, + } + amount := decimal.NewFromInt(42) + + rescue, err := NewChallengeRescueState(prev, closedChannelID, amount) + require.NoError(t, err) + require.NotNil(t, rescue) + + assert.Equal(t, prev.Epoch, rescue.Epoch) + assert.Equal(t, prev.Version+1, rescue.Version) + assert.Nil(t, rescue.HomeChannelID) + assert.True(t, decimal.NewFromInt(52).Equal(rescue.HomeLedger.UserBalance), "got %s", rescue.HomeLedger.UserBalance.String()) + assert.True(t, decimal.NewFromInt(52).Equal(rescue.HomeLedger.NodeNetFlow), "got %s", rescue.HomeLedger.NodeNetFlow.String()) + assert.True(t, rescue.HomeLedger.UserNetFlow.IsZero()) + assert.True(t, rescue.HomeLedger.NodeBalance.IsZero()) + + assert.Equal(t, TransitionTypeChallengeRescue, rescue.Transition.Type) + assert.Equal(t, closedChannelID, rescue.Transition.AccountID) + assert.True(t, amount.Equal(rescue.Transition.Amount)) + }) + t.Run("Accepts zero amount", func(t *testing.T) { prev := makePrev() - rescue, err := NewChallengeRescueState(prev, decimal.Zero) + rescue, err := NewChallengeRescueState(prev, closedChannelID, decimal.Zero) require.NoError(t, err) require.NotNil(t, rescue) assert.True(t, rescue.Transition.Amount.IsZero()) @@ -256,14 +292,13 @@ func TestNewChallengeRescueState(t *testing.T) { t.Run("Rejects negative amount", func(t *testing.T) { prev := makePrev() - _, err := NewChallengeRescueState(prev, decimal.NewFromInt(-1)) + _, err := NewChallengeRescueState(prev, closedChannelID, decimal.NewFromInt(-1)) require.Error(t, err) }) - t.Run("Rejects prev state without home channel", func(t *testing.T) { + t.Run("Rejects empty closed channel ID", func(t *testing.T) { prev := makePrev() - prev.HomeChannelID = nil - _, err := NewChallengeRescueState(prev, decimal.NewFromInt(1)) + _, err := NewChallengeRescueState(prev, "", decimal.NewFromInt(1)) require.Error(t, err) }) } @@ -464,6 +499,34 @@ func TestLedger_Equal_Validate(t *testing.T) { lInvalid = l1 lInvalid.UserNetFlow = decimal.NewFromInt(999) // Mismatch assert.Error(t, lInvalid.Validate(6)) + + // Each net-flow field fits int256, but their sum does not. Solidity computes + // `userNetFlow + nodeNetFlow` as int256, so an unchecked sum overflow would + // produce a state the contract panics on. + lSumOverflow := Ledger{ + TokenAddress: "0xT", + BlockchainID: 1, + UserBalance: decimal.NewFromBigInt(maxInt256, 0), + NodeBalance: decimal.NewFromInt(1), + UserNetFlow: decimal.NewFromBigInt(maxInt256, 0), + NodeNetFlow: decimal.NewFromInt(1), + } + err := lSumOverflow.Validate(0) + require.Error(t, err) + assert.Contains(t, err.Error(), "net flow sum out of int256 range") + + // Sum exactly at the int256 boundary must pass. + halfMax := new(big.Int).Rsh(maxInt256, 1) // (2^255-1)/2 + otherHalf := new(big.Int).Sub(maxInt256, halfMax) + lBoundary := Ledger{ + TokenAddress: "0xT", + BlockchainID: 1, + UserBalance: decimal.NewFromBigInt(halfMax, 0), + NodeBalance: decimal.NewFromBigInt(otherHalf, 0), + UserNetFlow: decimal.NewFromBigInt(halfMax, 0), + NodeNetFlow: decimal.NewFromBigInt(otherHalf, 0), + } + assert.NoError(t, lBoundary.Validate(0)) } func TestTransactionType_String(t *testing.T) { diff --git a/sdk/mcp/package-lock.json b/sdk/mcp/package-lock.json index 5cd893191..355310c4d 100644 --- a/sdk/mcp/package-lock.json +++ b/sdk/mcp/package-lock.json @@ -1367,9 +1367,9 @@ } }, "node_modules/qs": { - "version": "6.15.0", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.0.tgz", - "integrity": "sha512-mAZTtNCeetKMH+pSjrb76NAM8V9a05I9aBZOHztWy/UqcJdQYNsf59vrRKWnojAT9Y+GbIvoTBC++CPHqpDBhQ==", + "version": "6.15.2", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.2.tgz", + "integrity": "sha512-Rzq0KEyX/w/tEybncDgdkZrJgVUsUMk3xjh3t5bv3S1HTAtg+uOYt72+ZfwiQwKdysThkTBdL/rTi6HDmX9Ddw==", "license": "BSD-3-Clause", "dependencies": { "side-channel": "^1.1.0"