From 6452862390a2e5c8e7beacfc35c2f82c05affc3f Mon Sep 17 00:00:00 2001 From: nksazonov Date: Sun, 14 Jun 2026 12:31:27 +0200 Subject: [PATCH 1/8] MF3-L19: fix(nitronode): add version-monotonicity guards to event handlers --- nitronode/event_handlers/service.go | 64 +++ nitronode/event_handlers/service_test.go | 695 +++++++++++++++++++++++ 2 files changed, 759 insertions(+) diff --git a/nitronode/event_handlers/service.go b/nitronode/event_handlers/service.go index a7b14bfe4..76408afe5 100644 --- a/nitronode/event_handlers/service.go +++ b/nitronode/event_handlers/service.go @@ -30,6 +30,39 @@ func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker c } } +// guardEventVersionMonotonic returns true (drop=true) when the incoming event's +// StateVersion is strictly less than the row's current StateVersion. The caller +// must `return nil` immediately when drop is true; the helper logs a structured +// warning identifying which event intent arrived stale. +// +// Intent is a short stable string describing the on-chain transition the dropped +// event would have applied (e.g. "checkpointed", "closed", "escrow_deposit_initiated"). +// It surfaces in the warn log so operators can correlate drops with reentrancy +// scenarios in MF3-L19 / nitronode-event-monotonicity.md. +// +// Once §B lands, this helper is the single chokepoint where the on-drop +// chain-state refresh will be invoked: every dropped event triggers exactly one +// RefreshChannelFromChain call before the function returns drop=true, so the +// refresh policy stays consistent across all six handlers. +func guardEventVersionMonotonic( + ctx context.Context, + logger log.Logger, + chanID string, + intent string, + eventVersion uint64, + currentVersion uint64, +) (drop bool) { + if eventVersion >= currentVersion { + return false + } + logger.Warn("event state version is less than current channel state version, ignoring", + "channelId", chanID, + "intent", intent, + "currentStateVersion", currentVersion, + "eventStateVersion", eventVersion) + return true +} + // HandleNodeBalanceUpdated processes the NodeBalanceUpdated event emitted when the node's // on-chain liquidity changes. It records the new node liquidity for the (blockchain, asset) // pair via SetNodeBalance; this is observability data only and does not affect user staking @@ -151,6 +184,16 @@ func (s *EventHandlerService) HandleHomeChannelCheckpointed(ctx context.Context, return nil } + // Per protocol the checkpointed version cannot be lower than the last known on-chain + // version. This branch is reachable when contract reentrancy emits an inner + // higher-version event before the outer ChannelCheckpointed (see MF3-L19 / + // nitronode-event-monotonicity.md scenarios 1–3). Drop the event so we do not + // regress channel.StateVersion and, critically, so the wasChallenged branch below + // does not flip a live challenge back to Open based on a stale version. + if guardEventVersionMonotonic(ctx, logger, chanID, "checkpointed", event.StateVersion, channel.StateVersion) { + return nil + } + channel.StateVersion = event.StateVersion // Snapshot the pre-checkpoint status once and derive both transition flags from it, so the @@ -387,6 +430,11 @@ func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx co return nil } + // Drop stale Closed events that would regress state_version (MF3-L19). + if guardEventVersionMonotonic(ctx, logger, chanID, "closed", event.StateVersion, channel.StateVersion) { + return nil + } + wasChallenged := channel.Status == core.ChannelStatusChallenged channel.StateVersion = event.StateVersion @@ -533,6 +581,10 @@ func (s *EventHandlerService) HandleEscrowDepositInitiated(ctx context.Context, return nil } + if guardEventVersionMonotonic(ctx, logger, chanID, "escrow_deposit_initiated", event.StateVersion, channel.StateVersion) { + return nil + } + channel.StateVersion = event.StateVersion channel.Status = core.ChannelStatusOpen @@ -701,6 +753,10 @@ func (s *EventHandlerService) HandleEscrowDepositFinalized(ctx context.Context, return nil } + if guardEventVersionMonotonic(ctx, logger, chanID, "escrow_deposit_finalized", event.StateVersion, channel.StateVersion) { + return nil + } + channel.StateVersion = event.StateVersion channel.Status = core.ChannelStatusClosed // Channel is terminal; any pending challenge deadline is no longer meaningful. @@ -775,6 +831,10 @@ func (s *EventHandlerService) HandleEscrowWithdrawalInitiated(ctx context.Contex return nil } + if guardEventVersionMonotonic(ctx, logger, chanID, "escrow_withdrawal_initiated", event.StateVersion, channel.StateVersion) { + return nil + } + channel.StateVersion = event.StateVersion channel.Status = core.ChannelStatusOpen @@ -869,6 +929,10 @@ func (s *EventHandlerService) HandleEscrowWithdrawalFinalized(ctx context.Contex return nil } + if guardEventVersionMonotonic(ctx, logger, chanID, "escrow_withdrawal_finalized", event.StateVersion, channel.StateVersion) { + return nil + } + channel.StateVersion = event.StateVersion channel.Status = core.ChannelStatusClosed // Channel is terminal; any pending challenge deadline is no longer meaningful. diff --git a/nitronode/event_handlers/service_test.go b/nitronode/event_handlers/service_test.go index d0b062af5..43f219031 100644 --- a/nitronode/event_handlers/service_test.go +++ b/nitronode/event_handlers/service_test.go @@ -2490,3 +2490,698 @@ func TestHandleEscrowDepositsPurged_StoreError_Propagates(t *testing.T) { require.Contains(t, err.Error(), "db error") mockStore.AssertExpectations(t) } + +// MF3-L19 §E.1 — RegressionDropped for HandleHomeChannelCheckpointed. +// A lower-version Checkpointed event arriving after a higher-version event must not +// regress channel.StateVersion, must not flip channel.Status, must not run sig backfill +// at the older version, and must not trigger the wasChallenged/wasVoid balance refresh. +func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 10, // N+M + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, // N < N+M + UserSig: "0xstaleusersig", + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + + require.NoError(t, err) + // Row must be untouched: version, status, and no side effects. + require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") + require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must be unchanged") + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) + mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) +} + +// MF3-L19 §E.2 — scenario-3 regression test (the critical one). +// A lower-version Checkpointed must not silently clear an active challenge by entering +// the wasChallenged branch and flipping Status back to Open / clearing ChallengeExpiresAt. +func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 10, // N+M after a higher-version Challenged + ChallengeExpiresAt: &expiryTime, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, // N < N+M (stale Deposited/Checkpointed) + UserSig: "0xstaleusersig", + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + + require.NoError(t, err) + // Critical assertions: challenge state preserved. + require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Challenged status must be preserved") + require.NotNil(t, channel.ChallengeExpiresAt, "ChallengeExpiresAt must not be cleared by stale Checkpointed") + require.Equal(t, expiryTime.Unix(), channel.ChallengeExpiresAt.Unix(), "ChallengeExpiresAt must be unchanged") + require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) +} + +// MF3-L19 §E.3 — EqualVersionAccepted. The guard is `<`, not `<=`, so the legitimate +// indexer-replay/reorg case where the same (channelID, stateVersion) is re-delivered +// must still run the sig-backfill and balance-refresh idempotently. +func TestHandleHomeChannelCheckpointed_EqualVersionAccepted(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, // equal to current + UserSig: "0xusersig", + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusOpen && + ch.StateVersion == 5 + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "0xusersig", "").Return(nil) + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + + require.NoError(t, err) + mockStore.AssertExpectations(t) + // Not Challenged nor Void → backfill of head node sig is skipped. + mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) +} + +// MF3-L19 §E.4 — HigherVersionAccepted. Sanity that the monotonic forward flow still +// works after the guard is added: a higher-version Checkpointed against a Challenged +// channel still clears the challenge and bumps the version. +func TestHandleHomeChannelCheckpointed_HigherVersionAccepted(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 3, + ChallengeExpiresAt: &expiryTime, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, // strictly greater + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusOpen && + ch.StateVersion == 5 && + ch.ChallengeExpiresAt == nil + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil) + mockStore.On("HasSignedFinalize", channelID).Return(false, nil) + mockStore.On("GetLastStateByChannelID", channelID, false).Return(nil, nil) + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + + require.NoError(t, err) + mockStore.AssertExpectations(t) +} + +// MF3-L19 §E.5 — RegressionDropped for HandleHomeChannelClosed. +// A lower-version Closed event must not regress StateVersion, must not flip Status to +// Closed, and must not issue a challenge rescue. See the plan's §A.2 terminal-status +// note: without §B chain-state refresh this leaves the Node row divergent from chain; +// this test pins the §A guard behaviour only. +func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 10, // N+M + } + + event := &core.HomeChannelClosedEvent{ + ChannelID: channelID, + StateVersion: 5, // N < N+M + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + + err := service.HandleHomeChannelClosed(ctx, mockStore, event) + + require.NoError(t, err) + require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") + require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Status must not be flipped to Closed by stale event") + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + // Critically, no rescue issuance — SumNetTransitionAmountAfterVersion / StoreUserState + // / RecordTransaction must all be skipped. + mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "GetLastUserState", mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) +} + +// MF3-L19 §E.6 — RegressionDropped for HandleEscrowDepositInitiated. +// A lower-version EscrowDepositInitiated must not regress StateVersion, must not flip +// Status, and must not call ScheduleInitiateEscrowDeposit. +func TestHandleEscrowDepositInitiated_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 10, + } + + event := &core.EscrowDepositInitiatedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil) + + err := service.HandleEscrowDepositInitiated(ctx, mockStore, event) + + require.NoError(t, err) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusOpen, channel.Status) + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "GetStateByChannelIDAndVersion", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "ScheduleInitiateEscrowDeposit", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// MF3-L19 §E.7 — RegressionDropped for HandleEscrowDepositFinalized. +func TestHandleEscrowDepositFinalized_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + } + + event := &core.EscrowDepositFinalizedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil) + + err := service.HandleEscrowDepositFinalized(ctx, mockStore, event) + + require.NoError(t, err) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Status must not flip to Closed via stale Finalized") + require.NotNil(t, channel.ChallengeExpiresAt, "Stale Finalized must not clear ChallengeExpiresAt") + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// MF3-L19 §E.8 — RegressionDropped for HandleEscrowWithdrawalInitiated. +func TestHandleEscrowWithdrawalInitiated_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 10, + } + + event := &core.EscrowWithdrawalInitiatedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil) + + err := service.HandleEscrowWithdrawalInitiated(ctx, mockStore, event) + + require.NoError(t, err) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusOpen, channel.Status) + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// MF3-L19 §E.9 — RegressionDropped for HandleEscrowWithdrawalFinalized. +func TestHandleEscrowWithdrawalFinalized_RegressionDropped(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + } + + event := &core.EscrowWithdrawalFinalizedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil) + + err := service.HandleEscrowWithdrawalFinalized(ctx, mockStore, event) + + require.NoError(t, err) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusChallenged, channel.Status) + require.NotNil(t, channel.ChallengeExpiresAt) + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// MF3-L19 §E.10 — OnHomeEscrowPath. +// The reactor's handleEscrowDepositInitiatedOnHome / *FinalizedOnHome / *WithdrawalInitiatedOnHome / +// *WithdrawalFinalizedOnHome funnel into HandleHomeChannelCheckpointed (see +// channel_hub_reactor.go:506-559). The §A.1 guard therefore covers all four *OnHome paths +// automatically — there is no separate handler call path to exercise at the +// EventHandlerService layer. Writing a direct unit test against the reactor would require +// reactor fixtures (channelHubFilterer, types.Log) that don't exist for this test target, +// so we skip with documentation per the spec's E.10 special note. +func TestHandleHomeChannelCheckpointed_OnHomeEscrowPath(t *testing.T) { + t.Skip("reactor-level integration test; the *OnHome paths funnel into " + + "HandleHomeChannelCheckpointed (channel_hub_reactor.go:506-559) which is " + + "already covered by TestHandleHomeChannelCheckpointed_RegressionDropped and " + + "TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge. A " + + "reactor-level test would require channelHubFilterer + types.Log fixtures " + + "that aren't in scope here. See MF3-L19 plan §A.7 and the §E.10 special note.") +} + +// MF3-L19 §E.13 — RescueIdempotentOnEqualVersionReplay. +// Fire HandleHomeChannelClosed twice at the same version against a Challenged channel. +// First call: wasChallenged=true → status flips to Closed and issueChallengeRescue +// records exactly one rescue state + transaction. Second call (equal-version replay, +// admitted by the `<` guard): wasChallenged=false because Status is now Closed → +// rescue branch must NOT be re-entered. Total rescue count remains 1. +// +// This pins the invariant that the rescue idempotency is enforced by the channel's +// status transition (Challenged → Closed), not by the version guard. A future refactor +// moving the wasChallenged snapshot or persisting Challenged across handler invocations +// would break this and should fail this test. +func TestHandleHomeChannelClosed_RescueIdempotentOnEqualVersionReplay(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + asset := "USDC" + tokenAddress := "0xtoken" + blockchainID := uint64(1) + closureVersion := uint64(7) + rescueAmount := decimal.NewFromInt(50) + homeChannelIDPtr := channelID + expiryTime := time.Now().Add(time.Hour) + + // Start Challenged at version=closureVersion-2 < closureVersion (legitimate close + // at higher version). The lock returns the same pointer twice; the handler mutates + // channel.Status to Closed after the first call, so the second call observes Closed. + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: asset, + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: closureVersion - 2, + ChallengeExpiresAt: &expiryTime, + } + + prevState := &core.State{ + ID: core.GetStateID(userWallet, asset, 1, 9), + Asset: asset, + UserWallet: userWallet, + Epoch: 1, + Version: 9, + HomeChannelID: &homeChannelIDPtr, + HomeLedger: core.Ledger{ + TokenAddress: tokenAddress, + BlockchainID: blockchainID, + UserBalance: decimal.NewFromInt(50), + UserNetFlow: decimal.NewFromInt(50), + NodeBalance: decimal.Zero, + NodeNetFlow: decimal.Zero, + }, + } + + event := &core.HomeChannelClosedEvent{ + ChannelID: channelID, + StateVersion: closureVersion, + } + + // LockUserStateForHomeChannel is called twice (once per handler invocation) and returns + // the same mutated channel pointer both times. + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil).Times(2) + // UpdateChannel is called twice (the guard admits equal versions, so the second call + // also writes the row idempotently). + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil).Times(2) + + // Rescue side effects: must be called exactly ONCE across both handler invocations. + mockStore.On("SumNetTransitionAmountAfterVersion", channelID, closureVersion).Return(rescueAmount, nil).Once() + mockStore.On("GetLastUserState", userWallet, asset, false).Return(prevState, nil).Once() + mockStore.On("StoreUserState", mock.Anything, "").Return(nil).Once() + mockStore.On("RecordTransaction", mock.Anything, "").Return(nil).Once() + + // First call: wasChallenged=true → rescue fires. + err := service.HandleHomeChannelClosed(ctx, mockStore, event) + require.NoError(t, err) + require.Equal(t, core.ChannelStatusClosed, channel.Status, "Status must be Closed after first call") + + // Second call: equal-version replay. Status is now Closed → wasChallenged=false → + // rescue branch must not re-enter. The `.Once()` constraints on the rescue mocks + // will fail if any are called a second time. + err = service.HandleHomeChannelClosed(ctx, mockStore, event) + require.NoError(t, err) + require.Equal(t, core.ChannelStatusClosed, channel.Status, "Status remains Closed after replay") + + mockStore.AssertExpectations(t) + // Explicit double-check: AssertNumberOfCalls catches any drift even if mock matching + // somehow accepted an extra call against a more permissive expectation. + mockStore.AssertNumberOfCalls(t, "StoreUserState", 1) + mockStore.AssertNumberOfCalls(t, "RecordTransaction", 1) + mockStore.AssertNumberOfCalls(t, "SumNetTransitionAmountAfterVersion", 1) +} + +// MF3-L19 §E.14 — EqualVersionReplay_NoSideEffects. +// For every guarded handler other than HandleHomeChannelClosed (covered by §E.13), an +// equal-version replay must be safe: no double-credit, no second balance-refresh side +// effect, no duplicate RecordTransaction. The handler-level side effects are idempotent +// because UpdateStateSigsIfMissing / RefreshUserEnforcedBalance / UpdateChannel are all +// idempotent on the same input. +// +// CAVEAT per §C.1: HandleEscrowDepositInitiated calls ScheduleInitiateEscrowDeposit, +// which is NOT idempotent — it unconditionally inserts a new blockchain_actions row. The +// sub-test EscrowDepositInitiated_DuplicateScheduleOnReplay explicitly asserts the +// double-call as a regression target for the §F.6 follow-up scheduler-dedup work. +func TestHandleXxx_EqualVersionReplay_NoSideEffects(t *testing.T) { + t.Run("HomeChannelCheckpointed", func(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerService(t) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, + UserSig: "0xusersig", + } + + // Both calls hit the same mock; idempotent UpdateStateSigsIfMissing is the + // guarantor — but we still need to make sure the wasChallenged/wasVoid branch + // isn't re-armed on replay. Status stays Open across both calls, so the + // backfillOffChainHeadNodeSig path is never entered. + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil).Times(2) + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "0xusersig", "").Return(nil).Times(2) + + require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, event)) + require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, event)) + + mockStore.AssertExpectations(t) + // No head-sig backfill on the Open→Open path, even on replay. + mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) + }) + + t.Run("EscrowDepositFinalized", func(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: "0x1234567890123456789012345678901234567890", + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.EscrowDepositFinalizedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil).Times(2) + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil).Times(2) + + require.NoError(t, service.HandleEscrowDepositFinalized(ctx, mockStore, event)) + require.NoError(t, service.HandleEscrowDepositFinalized(ctx, mockStore, event)) + + mockStore.AssertExpectations(t) + }) + + t.Run("EscrowWithdrawalInitiated", func(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: "0x1234567890123456789012345678901234567890", + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.EscrowWithdrawalInitiatedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil).Times(2) + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil).Times(2) + + require.NoError(t, service.HandleEscrowWithdrawalInitiated(ctx, mockStore, event)) + require.NoError(t, service.HandleEscrowWithdrawalInitiated(ctx, mockStore, event)) + + mockStore.AssertExpectations(t) + }) + + t.Run("EscrowWithdrawalFinalized", func(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: "0x1234567890123456789012345678901234567890", + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.EscrowWithdrawalFinalizedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil).Times(2) + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil).Times(2) + + require.NoError(t, service.HandleEscrowWithdrawalFinalized(ctx, mockStore, event)) + require.NoError(t, service.HandleEscrowWithdrawalFinalized(ctx, mockStore, event)) + + mockStore.AssertExpectations(t) + }) + + // §C.1 caveat: ScheduleInitiateEscrowDeposit is NOT idempotent on same-version + // replay — scheduleStateEnforcement unconditionally INSERTs a new blockchain_actions + // row, so equal-version replay enqueues a duplicate action. This sub-test pins the + // CURRENT (buggy) behaviour as a regression target for §F.6's scheduler-dedup follow-up. + // When that follow-up lands, this assertion should be flipped from .Times(2) to .Once(). + t.Run("EscrowDepositInitiated_DuplicateScheduleOnReplay", func(t *testing.T) { + mockStore := new(MockStore) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service := &EventHandlerService{} + + channelID := "0xEscrowChannel123" + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: "0x1234567890123456789012345678901234567890", + Asset: "usdc", + Type: core.ChannelTypeEscrow, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + state := &core.State{ + ID: "state123", + Version: 5, + HomeLedger: core.Ledger{ + BlockchainID: 1, + }, + } + + event := &core.EscrowDepositInitiatedEvent{ + ChannelID: channelID, + StateVersion: 5, + } + + mockStore.On("GetChannelByID", channelID).Return(channel, nil).Times(2) + mockStore.On("UpdateChannel", mock.Anything).Return(nil).Times(2) + mockStore.On("GetStateByChannelIDAndVersion", channelID, uint64(5)).Return(state, nil).Times(2) + // CAVEAT: schedule is called twice — this is the §C.1 / §F.6 latent issue + // (scheduler-dedup gap). The `<` guard admits the equal-version replay, and + // scheduleStateEnforcement does not dedup on (state_id, action_type). Flagged + // for follow-up; assert the duplicate so the regression target is explicit. + mockStore.On("ScheduleInitiateEscrowDeposit", "state123", uint64(1)).Return(nil).Times(2) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil).Times(2) + + require.NoError(t, service.HandleEscrowDepositInitiated(ctx, mockStore, event)) + require.NoError(t, service.HandleEscrowDepositInitiated(ctx, mockStore, event)) + + mockStore.AssertExpectations(t) + mockStore.AssertNumberOfCalls(t, "ScheduleInitiateEscrowDeposit", 2) + }) +} From 94bba9b9139d7ae7200360daa7aba8442d2692c5 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Sun, 14 Jun 2026 13:35:05 +0200 Subject: [PATCH 2/8] feat(nitronode): chain-state refresh for dropped guard events --- nitronode/event_handlers/service.go | 91 +++- nitronode/event_handlers/service_test.go | 568 ++++++++++++++++++-- nitronode/main.go | 26 +- pkg/blockchain/evm/chain_state_refresher.go | 117 ++++ pkg/core/interface.go | 14 + pkg/core/types.go | 23 +- 6 files changed, 774 insertions(+), 65 deletions(-) create mode 100644 pkg/blockchain/evm/chain_state_refresher.go diff --git a/nitronode/event_handlers/service.go b/nitronode/event_handlers/service.go index 76408afe5..99f8154aa 100644 --- a/nitronode/event_handlers/service.go +++ b/nitronode/event_handlers/service.go @@ -18,15 +18,24 @@ var _ core.ChannelHubEventHandler = &EventHandlerService{} type EventHandlerService struct { nodeSigner *core.ChannelDefaultSigner statePacker core.StatePacker + refresher core.ChainStateRefresher } // NewEventHandlerService creates a new EventHandlerService instance. // nodeSigner and statePacker are used to backfill the node signature on the // checkpointed head state when it is missing from the local record. -func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker core.StatePacker) *EventHandlerService { +// refresher is invoked from the home-channel guard-drop paths to fetch the +// authoritative on-chain snapshot and converge the local row; it is mandatory +// and a nil refresher panics here so a misconfigured wire-up fails loudly +// instead of silently degrading to pre-§B warn-and-skip behaviour. +func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker core.StatePacker, refresher core.ChainStateRefresher) *EventHandlerService { + if refresher == nil { + panic("event_handlers: refresher not wired (§B requires a non-nil ChainStateRefresher)") + } return &EventHandlerService{ nodeSigner: nodeSigner, statePacker: statePacker, + refresher: refresher, } } @@ -38,7 +47,7 @@ func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker c // Intent is a short stable string describing the on-chain transition the dropped // event would have applied (e.g. "checkpointed", "closed", "escrow_deposit_initiated"). // It surfaces in the warn log so operators can correlate drops with reentrancy -// scenarios in MF3-L19 / nitronode-event-monotonicity.md. +// scenarios. // // Once §B lands, this helper is the single chokepoint where the on-drop // chain-state refresh will be invoked: every dropped event triggers exactly one @@ -63,6 +72,63 @@ func guardEventVersionMonotonic( return true } +// refreshAfterDroppedEvent fetches the authoritative on-chain channel snapshot +// via the configured ChainStateRefresher and overwrites the local row's status, +// state version, and challenge expiry, then backfills the user signature on the +// chain-asserted version. Called from the home-channel guard-drop paths to +// close the observability gap described in §B. +// +// Two-mode error contract: +// - refresher returns nil: row converged, returns nil so the dedup ledger advances. +// - refresher returns an error: returns non-nil so the reactor rolls back the +// tx and the listener replays the event on its next tick. See §B.2. +// +// The refresher is mandatory; NewEventHandlerService panics on a nil refresher +// so this helper can rely on `s.refresher` being non-nil. +func (s *EventHandlerService) refreshAfterDroppedEvent( + ctx context.Context, + tx core.ChannelHubEventHandlerStore, + channel *core.Channel, + droppedIntent string, +) error { + logger := log.FromContext(ctx) + refreshed, err := s.refresher.RefreshChannelFromChain(ctx, channel.ChannelID) + if err != nil { + // Surface; if the on-chain read fails we cannot safely converge and must + // retry the event by returning a non-nil error so the listener replays. + return fmt.Errorf("refresh after dropped %s: %w", droppedIntent, err) + } + + channel.Status = refreshed.Status + channel.StateVersion = refreshed.StateVersion + channel.ChallengeExpiresAt = refreshed.ChallengeExpiresAt + + if err := tx.UpdateChannel(*channel); err != nil { + return err + } + if err := tx.RefreshUserEnforcedBalance(channel.UserWallet, channel.Asset); err != nil { + return err + } + // Skip the sig backfill when the chain payload carried no user signature + // (e.g. some terminal states do not echo the candidate sig back into + // ChannelMeta.lastState). UpdateStateSigsIfMissing would otherwise be a + // no-op with an empty userSig but the explicit guard documents intent. + if refreshed.LastStateUserSig != "" { + if err := tx.UpdateStateSigsIfMissing(channel.ChannelID, refreshed.StateVersion, refreshed.LastStateUserSig, ""); err != nil { + return err + } + } + + logger.Info("refreshed channel from chain after dropped event", + "channelId", channel.ChannelID, + "droppedIntent", droppedIntent, + "refreshedStatus", refreshed.Status, + "refreshedStateVersion", refreshed.StateVersion, + "refreshedChallengeExpiresAt", refreshed.ChallengeExpiresAt, + ) + return nil +} + // HandleNodeBalanceUpdated processes the NodeBalanceUpdated event emitted when the node's // on-chain liquidity changes. It records the new node liquidity for the (blockchain, asset) // pair via SetNodeBalance; this is observability data only and does not affect user staking @@ -186,12 +252,11 @@ func (s *EventHandlerService) HandleHomeChannelCheckpointed(ctx context.Context, // Per protocol the checkpointed version cannot be lower than the last known on-chain // version. This branch is reachable when contract reentrancy emits an inner - // higher-version event before the outer ChannelCheckpointed (see MF3-L19 / - // nitronode-event-monotonicity.md scenarios 1–3). Drop the event so we do not - // regress channel.StateVersion and, critically, so the wasChallenged branch below + // higher-version event before the outer ChannelCheckpointed. Drop the event so we do + // not regress channel.StateVersion and, critically, so the wasChallenged branch below // does not flip a live challenge back to Open based on a stale version. if guardEventVersionMonotonic(ctx, logger, chanID, "checkpointed", event.StateVersion, channel.StateVersion) { - return nil + return s.refreshAfterDroppedEvent(ctx, tx, channel, "checkpointed") } channel.StateVersion = event.StateVersion @@ -343,11 +408,11 @@ func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, t return nil } - if event.StateVersion < channel.StateVersion { - // Per protocol the challenged version cannot be lower than the last known on-chain version. - // Treat as an anomaly (replay, indexer mis-order, contract bug): warn and skip persistence. - logger.Warn("challenged state version is less than current channel state version, ignoring", "channelId", chanID, "currentStateVersion", channel.StateVersion, "challengedStateVersion", event.StateVersion) - return nil + // Per protocol the challenged version cannot be lower than the last known on-chain version. + // Treat as an anomaly (replay, indexer mis-order, contract reentrancy): drop the event and + // refresh from chain to converge the local row with the authoritative on-chain status. + if guardEventVersionMonotonic(ctx, logger, chanID, "challenged", event.StateVersion, channel.StateVersion) { + return s.refreshAfterDroppedEvent(ctx, tx, channel, "challenged") } channel.StateVersion = event.StateVersion @@ -430,9 +495,9 @@ func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx co return nil } - // Drop stale Closed events that would regress state_version (MF3-L19). + // Drop stale Closed events that would regress state_version. if guardEventVersionMonotonic(ctx, logger, chanID, "closed", event.StateVersion, channel.StateVersion) { - return nil + return s.refreshAfterDroppedEvent(ctx, tx, channel, "closed") } wasChallenged := channel.Status == core.ChannelStatusChallenged diff --git a/nitronode/event_handlers/service_test.go b/nitronode/event_handlers/service_test.go index 43f219031..4e555af2c 100644 --- a/nitronode/event_handlers/service_test.go +++ b/nitronode/event_handlers/service_test.go @@ -622,11 +622,15 @@ func TestHandleHomeChannelChallenged_PersistsChallenge(t *testing.T) { func TestHandleHomeChannelChallenged_StaleVersionIgnored(t *testing.T) { // Per protocol the challenged version cannot be lower than the last known on-chain version. - // Anomalies (replay, indexer mis-order) must not regress channel state. + // Anomalies (replay, indexer mis-order, reentrancy) must not regress + // channel state. With §B landed, the guard-drop triggers an on-chain refresh: the refresher + // returns the authoritative snapshot and the row converges to the chain view, NEVER to the + // stale event payload. mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service := &EventHandlerService{} + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -646,14 +650,35 @@ func TestHandleHomeChannelChallenged_StaleVersionIgnored(t *testing.T) { ChallengeExpiry: uint64(time.Now().Add(time.Hour).Unix()), } + // Refresher returns a snapshot consistent with the current row (chain hasn't moved). + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusOpen, + StateVersion: 5, + ChallengeExpiresAt: nil, + LastStateUserSig: "", + } + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + // Row must converge to the refreshed (== current) chain view, NOT to the stale event payload. + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusOpen && + ch.StateVersion == 5 && + ch.ChallengeExpiresAt == nil + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) err := service.HandleHomeChannelChallenged(ctx, mockStore, event) require.NoError(t, err) + // Row state must reflect the refreshed chain snapshot, NOT the stale event payload. + require.Equal(t, uint64(5), channel.StateVersion, "StateVersion must not regress to the stale event version") + require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must come from refresh, not the stale event") mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) - mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockRefresher.AssertExpectations(t) + // LastStateUserSig is empty, so UpdateStateSigsIfMissing must be skipped (documented intent). + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } func TestHandleHomeChannelChallenged_ChannelNotFound(t *testing.T) { @@ -1629,7 +1654,38 @@ func newTestEventHandlerService(t *testing.T) (*EventHandlerService, string) { nodeSigner, err := core.NewChannelDefaultSigner(signer) require.NoError(t, err) packer := core.NewStatePackerV1(mockEventHandlerAssetStore{}) - return NewEventHandlerService(nodeSigner, packer), signer.PublicKey().Address().String() + // Default refresher: a MockChainStateRefresher with no expectations set. + // Tests that don't exercise the §B refresh path get a non-nil refresher so + // NewEventHandlerService's wire-up check passes; if a test inadvertently + // triggers RefreshChannelFromChain, testify/mock panics loudly with an + // unexpected-call assertion — the desired loud-failure behaviour. + // Tests asserting refresh interactions use newTestEventHandlerServiceWithRefresher. + return NewEventHandlerService(nodeSigner, packer, new(MockChainStateRefresher)), signer.PublicKey().Address().String() +} + +// MockChainStateRefresher is a testify/mock implementation of core.ChainStateRefresher +// used by §B tests (E.11, E.12, and the §A1/§A2 guard-drop tests that now invoke +// the refresher). +type MockChainStateRefresher struct { + mock.Mock +} + +// RefreshChannelFromChain mocks the on-drop chain-state refresh hook. +func (m *MockChainStateRefresher) RefreshChannelFromChain(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { + args := m.Called(ctx, channelID) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*core.RefreshedChannel), args.Error(1) +} + +// newTestEventHandlerServiceWithRefresher constructs an EventHandlerService with a +// caller-provided ChainStateRefresher for tests that exercise the refresh path. +func newTestEventHandlerServiceWithRefresher(t *testing.T, refresher core.ChainStateRefresher) (*EventHandlerService, string) { + t.Helper() + svc, addr := newTestEventHandlerService(t) + svc.refresher = refresher + return svc, addr } // TestHandleHomeChannelCheckpointed_BackfillsHeadNodeSig covers the case where a @@ -2491,15 +2547,19 @@ func TestHandleEscrowDepositsPurged_StoreError_Propagates(t *testing.T) { mockStore.AssertExpectations(t) } -// MF3-L19 §E.1 — RegressionDropped for HandleHomeChannelCheckpointed. +// §E.1 — RegressionDropped for HandleHomeChannelCheckpointed. // A lower-version Checkpointed event arriving after a higher-version event must not -// regress channel.StateVersion, must not flip channel.Status, must not run sig backfill -// at the older version, and must not trigger the wasChallenged/wasVoid balance refresh. +// regress channel.StateVersion via the event payload. With §B landed the guard-drop +// now triggers an on-chain refresh: the mock refresher returns a snapshot that agrees +// with the local row (chain has not progressed further), so the row converges to its +// existing state via the refresh path. The key invariant is that the older event's +// payload is NOT what drives the write — the chain view is. func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerService(t) + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -2519,30 +2579,54 @@ func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { UserSig: "0xstaleusersig", } + // Refresher returns a snapshot consistent with the current row (chain hasn't moved). + refreshedSig := "0xchainusersig" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusOpen, + StateVersion: 10, + ChallengeExpiresAt: nil, + LastStateUserSig: refreshedSig, + } + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + // Row must converge to the refreshed (== current) chain view, NOT to the stale event payload. + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusOpen && + ch.StateVersion == 10 && + ch.ChallengeExpiresAt == nil + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) require.NoError(t, err) - // Row must be untouched: version, status, and no side effects. - require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") - require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must be unchanged") + // Row state must reflect the refreshed chain snapshot, NOT the stale event payload. + require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress to the stale event version") + require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must come from refresh, not the stale event") mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) - mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockRefresher.AssertExpectations(t) + // The stale event's UserSig at the regressed version must never be written. + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", channelID, uint64(5), mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) } -// MF3-L19 §E.2 — scenario-3 regression test (the critical one). +// §E.2 — scenario-3 regression test (the critical one). // A lower-version Checkpointed must not silently clear an active challenge by entering // the wasChallenged branch and flipping Status back to Open / clearing ChallengeExpiresAt. +// With §B landed, the guard-drop triggers an on-chain refresh: the refresher returns the +// authoritative Challenged snapshot (chain still shows Challenged), so the row converges +// to the chain view — NEVER to the stale Checkpointed event's payload which would have +// cleared the challenge. func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testing.T) { mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerService(t) + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -2564,24 +2648,47 @@ func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testin UserSig: "0xstaleusersig", } + // Chain still asserts Challenged at version 10 with the same expiry — refresh agrees with row. + refreshedSig := "0xchainusersig" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + LastStateUserSig: refreshedSig, + } + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + // Critical: UpdateChannel must persist the Challenged snapshot from chain, NOT the cleared + // snapshot the stale event's wasChallenged branch would have produced. + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusChallenged && + ch.StateVersion == 10 && + ch.ChallengeExpiresAt != nil && + ch.ChallengeExpiresAt.Unix() == expiryTime.Unix() + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) require.NoError(t, err) - // Critical assertions: challenge state preserved. - require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Challenged status must be preserved") + // Challenge state preserved via chain refresh, not via the stale event's payload. + require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Challenged status must be preserved via refresh") require.NotNil(t, channel.ChallengeExpiresAt, "ChallengeExpiresAt must not be cleared by stale Checkpointed") require.Equal(t, expiryTime.Unix(), channel.ChallengeExpiresAt.Unix(), "ChallengeExpiresAt must be unchanged") require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockRefresher.AssertExpectations(t) + // The stale wasChallenged branch must NOT run: no HasSignedFinalize lookup, no sig backfill + // at the stale version, no head-sig backfill via GetLastStateByChannelID. mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) - mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", channelID, uint64(5), mock.Anything, mock.Anything) } -// MF3-L19 §E.3 — EqualVersionAccepted. The guard is `<`, not `<=`, so the legitimate +// §E.3 — EqualVersionAccepted. The guard is `<`, not `<=`, so the legitimate // indexer-replay/reorg case where the same (channelID, stateVersion) is re-delivered // must still run the sig-backfill and balance-refresh idempotently. func TestHandleHomeChannelCheckpointed_EqualVersionAccepted(t *testing.T) { @@ -2626,7 +2733,7 @@ func TestHandleHomeChannelCheckpointed_EqualVersionAccepted(t *testing.T) { mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) } -// MF3-L19 §E.4 — HigherVersionAccepted. Sanity that the monotonic forward flow still +// §E.4 — HigherVersionAccepted. Sanity that the monotonic forward flow still // works after the guard is added: a higher-version Checkpointed against a Challenged // channel still clears the challenge and bumps the version. func TestHandleHomeChannelCheckpointed_HigherVersionAccepted(t *testing.T) { @@ -2672,27 +2779,33 @@ func TestHandleHomeChannelCheckpointed_HigherVersionAccepted(t *testing.T) { mockStore.AssertExpectations(t) } -// MF3-L19 §E.5 — RegressionDropped for HandleHomeChannelClosed. +// §E.5 — RegressionDropped for HandleHomeChannelClosed. // A lower-version Closed event must not regress StateVersion, must not flip Status to -// Closed, and must not issue a challenge rescue. See the plan's §A.2 terminal-status -// note: without §B chain-state refresh this leaves the Node row divergent from chain; -// this test pins the §A guard behaviour only. +// Closed, and must not issue a challenge rescue from the stale event payload. With §B +// landed, the guard-drop triggers an on-chain refresh: the chain still asserts +// Challenged (the chain has NOT actually closed at version 5 — see §A.2 terminal-status +// note), so the row converges to the chain Challenged view. The wasChallenged-driven +// rescue branch is owned by the post-guard happy path, NOT by the refresh path, so no +// rescue is issued. func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerService(t) + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) channel := &core.Channel{ - ChannelID: channelID, - UserWallet: userWallet, - Asset: "usdc", - Type: core.ChannelTypeHome, - Status: core.ChannelStatusChallenged, - StateVersion: 10, // N+M + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 10, // N+M + ChallengeExpiresAt: &expiryTime, } event := &core.HomeChannelClosedEvent{ @@ -2700,7 +2813,25 @@ func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { StateVersion: 5, // N < N+M } + // Chain confirms: still Challenged at version 10. The older Closed event must not drive a close. + refreshedSig := "0xchainusersig" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + LastStateUserSig: refreshedSig, + } + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusChallenged && + ch.StateVersion == 10 && + ch.ChallengeExpiresAt != nil + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) err := service.HandleHomeChannelClosed(ctx, mockStore, event) @@ -2708,18 +2839,17 @@ func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Status must not be flipped to Closed by stale event") mockStore.AssertExpectations(t) - mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) - mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) - mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) - // Critically, no rescue issuance — SumNetTransitionAmountAfterVersion / StoreUserState - // / RecordTransaction must all be skipped. + mockRefresher.AssertExpectations(t) + // Critically, no rescue issuance — the rescue branch belongs to the happy-path close, + // not to the refresh path. SumNetTransitionAmountAfterVersion / StoreUserState / + // RecordTransaction must all be skipped. mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "GetLastUserState", mock.Anything, mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) } -// MF3-L19 §E.6 — RegressionDropped for HandleEscrowDepositInitiated. +// §E.6 — RegressionDropped for HandleEscrowDepositInitiated. // A lower-version EscrowDepositInitiated must not regress StateVersion, must not flip // Status, and must not call ScheduleInitiateEscrowDeposit. func TestHandleEscrowDepositInitiated_RegressionDropped(t *testing.T) { @@ -2759,7 +2889,7 @@ func TestHandleEscrowDepositInitiated_RegressionDropped(t *testing.T) { mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } -// MF3-L19 §E.7 — RegressionDropped for HandleEscrowDepositFinalized. +// §E.7 — RegressionDropped for HandleEscrowDepositFinalized. func TestHandleEscrowDepositFinalized_RegressionDropped(t *testing.T) { mockStore := new(MockStore) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -2798,7 +2928,7 @@ func TestHandleEscrowDepositFinalized_RegressionDropped(t *testing.T) { mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } -// MF3-L19 §E.8 — RegressionDropped for HandleEscrowWithdrawalInitiated. +// §E.8 — RegressionDropped for HandleEscrowWithdrawalInitiated. func TestHandleEscrowWithdrawalInitiated_RegressionDropped(t *testing.T) { mockStore := new(MockStore) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -2834,7 +2964,7 @@ func TestHandleEscrowWithdrawalInitiated_RegressionDropped(t *testing.T) { mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } -// MF3-L19 §E.9 — RegressionDropped for HandleEscrowWithdrawalFinalized. +// §E.9 — RegressionDropped for HandleEscrowWithdrawalFinalized. func TestHandleEscrowWithdrawalFinalized_RegressionDropped(t *testing.T) { mockStore := new(MockStore) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -2873,7 +3003,7 @@ func TestHandleEscrowWithdrawalFinalized_RegressionDropped(t *testing.T) { mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } -// MF3-L19 §E.10 — OnHomeEscrowPath. +// §E.10 — OnHomeEscrowPath. // The reactor's handleEscrowDepositInitiatedOnHome / *FinalizedOnHome / *WithdrawalInitiatedOnHome / // *WithdrawalFinalizedOnHome funnel into HandleHomeChannelCheckpointed (see // channel_hub_reactor.go:506-559). The §A.1 guard therefore covers all four *OnHome paths @@ -2887,10 +3017,10 @@ func TestHandleHomeChannelCheckpointed_OnHomeEscrowPath(t *testing.T) { "already covered by TestHandleHomeChannelCheckpointed_RegressionDropped and " + "TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge. A " + "reactor-level test would require channelHubFilterer + types.Log fixtures " + - "that aren't in scope here. See MF3-L19 plan §A.7 and the §E.10 special note.") + "that aren't in scope here. See plan §A.7 and the §E.10 special note.") } -// MF3-L19 §E.13 — RescueIdempotentOnEqualVersionReplay. +// §E.13 — RescueIdempotentOnEqualVersionReplay. // Fire HandleHomeChannelClosed twice at the same version against a Challenged channel. // First call: wasChallenged=true → status flips to Closed and issueChallengeRescue // records exactly one rescue state + transaction. Second call (equal-version replay, @@ -2987,7 +3117,7 @@ func TestHandleHomeChannelClosed_RescueIdempotentOnEqualVersionReplay(t *testing mockStore.AssertNumberOfCalls(t, "SumNetTransitionAmountAfterVersion", 1) } -// MF3-L19 §E.14 — EqualVersionReplay_NoSideEffects. +// §E.14 — EqualVersionReplay_NoSideEffects. // For every guarded handler other than HandleHomeChannelClosed (covered by §E.13), an // equal-version replay must be safe: no double-credit, no second balance-refresh side // effect, no duplicate RecordTransaction. The handler-level side effects are idempotent @@ -3185,3 +3315,347 @@ func TestHandleXxx_EqualVersionReplay_NoSideEffects(t *testing.T) { mockStore.AssertNumberOfCalls(t, "ScheduleInitiateEscrowDeposit", 2) }) } + +// §E.11 — Scenario-4 sequence test: outer ChallengeChannel dropped because an +// inner higher-version Checkpointed already landed. The guard fires, the chain-state +// refresh runs, and the row converges to the chain's authoritative Challenged view — +// closing the observability gap where the Node would otherwise stay Open and admit the +// channel via CheckActiveChannel despite the chain being DISPUTED. +// +// This is the canonical §B test: the older event is dropped (no payload write), but the +// refresher fetches the authoritative on-chain status (Challenged) and the row is +// updated accordingly. RefreshUserEnforcedBalance and UpdateStateSigsIfMissing run with +// the refreshed sig at the refreshed version. See spec §B.2. +func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + asset := "usdc" + + // Setup matches the spec: inner higher-version Checkpointed already landed, + // row is Open at version 5 with no expiry. The outer Challenged at version 3 + // is about to arrive. + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: asset, + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.HomeChannelChallengedEvent{ + ChannelID: channelID, + StateVersion: 3, // lower than current 5 → guard fires + ChallengeExpiry: uint64(time.Now().Add(time.Hour).Unix()), + } + + // Authoritative on-chain view: Challenged at version 5, with a real expiry. + chainExpiry := time.Now().Add(2 * time.Hour) + chainSig := "0xab1234567890" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusChallenged, + StateVersion: 5, + ChallengeExpiresAt: &chainExpiry, + LastStateUserSig: chainSig, + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.ChannelID == channelID && + ch.Status == core.ChannelStatusChallenged && + ch.StateVersion == 5 && + ch.ChallengeExpiresAt != nil && + ch.ChallengeExpiresAt.Unix() == chainExpiry.Unix() + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), chainSig, "").Return(nil) + + err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + require.NoError(t, err) + + require.Equal(t, core.ChannelStatusChallenged, channel.Status, "row must converge to chain Challenged") + require.Equal(t, uint64(5), channel.StateVersion, "row must keep the chain version, not the stale event's") + require.NotNil(t, channel.ChallengeExpiresAt) + require.Equal(t, chainExpiry.Unix(), channel.ChallengeExpiresAt.Unix()) + + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + mockRefresher.AssertNumberOfCalls(t, "RefreshChannelFromChain", 1) +} + +// §E.11 (Checkpointed-guard-path variant). Fires HandleHomeChannelCheckpointed +// with a regression version where the chain has moved on and is now Challenged. The +// §A.1 guard fires, the refresher returns the chain Challenged view, and the row +// converges. This catches A.1's refresh hook independently from the Challenged handler. +func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + asset := "usdc" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: asset, + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 8, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 4, // regression + UserSig: "0xstaleusersig", + } + + chainExpiry := time.Now().Add(time.Hour) + chainSig := "0xab1234567890" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusChallenged, + StateVersion: 8, + ChallengeExpiresAt: &chainExpiry, + LastStateUserSig: chainSig, + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.Status == core.ChannelStatusChallenged && + ch.StateVersion == 8 && + ch.ChallengeExpiresAt != nil && + ch.ChallengeExpiresAt.Unix() == chainExpiry.Unix() + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(8), chainSig, "").Return(nil) + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + require.NoError(t, err) + + require.Equal(t, core.ChannelStatusChallenged, channel.Status) + require.Equal(t, uint64(8), channel.StateVersion) + require.NotNil(t, channel.ChallengeExpiresAt) + + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + // The wasChallenged branch's stale work must not run. + mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) + mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) +} + +// §E.11 (Closed-guard-path variant). Fires HandleHomeChannelClosed with a +// regression version where the chain has progressed further (Closed at the higher +// version). The §A.2 guard fires, refresher returns the chain Closed snapshot, row +// converges. Per §A.2 the chain MAY have actually closed past the stale event's +// version; the refresh path picks up that authoritative view. +func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + asset := "usdc" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: asset, + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 12, + } + + event := &core.HomeChannelClosedEvent{ + ChannelID: channelID, + StateVersion: 7, // regression — older Closed event + } + + chainSig := "0xab1234567890" + refreshed := &core.RefreshedChannel{ + Status: core.ChannelStatusClosed, + StateVersion: 12, + ChallengeExpiresAt: nil, + LastStateUserSig: chainSig, + } + + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { + return ch.Status == core.ChannelStatusClosed && + ch.StateVersion == 12 && + ch.ChallengeExpiresAt == nil + })).Return(nil) + mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) + mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(12), chainSig, "").Return(nil) + + err := service.HandleHomeChannelClosed(ctx, mockStore, event) + require.NoError(t, err) + + require.Equal(t, core.ChannelStatusClosed, channel.Status) + require.Equal(t, uint64(12), channel.StateVersion) + require.Nil(t, channel.ChallengeExpiresAt) + + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + // Rescue branch belongs to the happy-path close, not the refresh path. + mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) +} + +// §E.12 — Refresher-error replay test (Challenged guard path). +// Per spec §B.2's two-mode error contract: when RefreshChannelFromChain fails, the +// handler must return a non-nil error wrapping the RPC failure so the outer reactor +// transaction rolls back. The dedup ledger does NOT advance and the event will be +// re-delivered on the next listener tick. No partial write to the channel row. +func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusOpen, + StateVersion: 5, + } + + event := &core.HomeChannelChallengedEvent{ + ChannelID: channelID, + StateVersion: 3, // regression + ChallengeExpiry: uint64(time.Now().Add(time.Hour).Unix()), + } + + rpcErr := errors.New("rpc unavailable") + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + + err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + + require.Error(t, err, "refresher error must bubble up so the reactor tx rolls back") + require.ErrorIs(t, err, rpcErr, "wrapped error must preserve the underlying RPC failure") + // Channel row must be unchanged — no partial write. + require.Equal(t, uint64(5), channel.StateVersion, "row must not be mutated on refresh failure") + require.Equal(t, core.ChannelStatusOpen, channel.Status, "row must not be mutated on refresh failure") + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + // No partial write. + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// §E.12 (Checkpointed guard path variant). +func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + } + + event := &core.HomeChannelCheckpointedEvent{ + ChannelID: channelID, + StateVersion: 5, // regression + UserSig: "0xstaleusersig", + } + + rpcErr := errors.New("rpc unavailable") + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + + require.Error(t, err) + require.ErrorIs(t, err, rpcErr) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusChallenged, channel.Status) + require.NotNil(t, channel.ChallengeExpiresAt) + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// §E.12 (Closed guard path variant). +func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { + mockStore := new(MockStore) + mockRefresher := new(MockChainStateRefresher) + ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) + + service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + + channelID := "0xHomeChannel123" + userWallet := "0x1234567890123456789012345678901234567890" + expiryTime := time.Now().Add(time.Hour) + + channel := &core.Channel{ + ChannelID: channelID, + UserWallet: userWallet, + Asset: "usdc", + Type: core.ChannelTypeHome, + Status: core.ChannelStatusChallenged, + StateVersion: 10, + ChallengeExpiresAt: &expiryTime, + } + + event := &core.HomeChannelClosedEvent{ + ChannelID: channelID, + StateVersion: 5, // regression + } + + rpcErr := errors.New("rpc unavailable") + mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) + mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + + err := service.HandleHomeChannelClosed(ctx, mockStore, event) + + require.Error(t, err) + require.ErrorIs(t, err, rpcErr) + require.Equal(t, uint64(10), channel.StateVersion) + require.Equal(t, core.ChannelStatusChallenged, channel.Status) + require.NotNil(t, channel.ChallengeExpiresAt) + mockStore.AssertExpectations(t) + mockRefresher.AssertExpectations(t) + mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) + mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, 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/main.go b/nitronode/main.go index 1735cbe0f..2ad3a20b2 100644 --- a/nitronode/main.go +++ b/nitronode/main.go @@ -78,7 +78,23 @@ func main() { } eventHandlerStatePacker := core.NewStatePackerV1(bb.MemoryStore) - eventHandlerService := event_handlers.NewEventHandlerService(nodeChannelSigner, eventHandlerStatePacker) + // Per-chain ChannelHub callers feed the chain-state refresher used by home-channel + // guard-drop paths. The map is populated below as each blockchain is wired up; + // the entry for chain X is always present before that chain's listener starts, so + // the refresher invariant (refresh runs on the same chain as the handler) holds. + channelHubCallers := make(map[uint64]*evm.ChannelHubCaller) + resolveChannelChain := func(channelID string) (uint64, error) { + ch, err := bb.DbStore.GetChannelByID(channelID) + if err != nil { + return 0, fmt.Errorf("lookup channel %s: %w", channelID, err) + } + if ch == nil { + return 0, fmt.Errorf("channel %s not found in DB", channelID) + } + return ch.BlockchainID, nil + } + chainStateRefresher := evm.NewEVMChainStateRefresher(channelHubCallers, resolveChannelChain) + eventHandlerService := event_handlers.NewEventHandlerService(nodeChannelSigner, eventHandlerStatePacker, chainStateRefresher) for _, b := range blockchains { rpcURL, ok := bb.BlockchainRPCs[b.ID] @@ -106,6 +122,14 @@ func main() { logger.Fatal("failed to create EVM client") } + // Register a read-only ChannelHub caller for this chain so the chain-state + // refresher can issue getChannelData reads from home-channel guard-drop paths. + channelHubCaller, err := evm.NewChannelHubCaller(common.HexToAddress(b.ChannelHubAddress), client) + if err != nil { + logger.Fatal("failed to create ChannelHub caller", "error", err, "blockchainID", b.ID) + } + channelHubCallers[b.ID] = channelHubCaller + sigValidators, err := bb.MemoryStore.GetChannelSigValidators(b.ID) if err != nil { logger.Fatal("failed to get channel signature validators from memory store", "error", err, "blockchainID", b.ID) diff --git a/pkg/blockchain/evm/chain_state_refresher.go b/pkg/blockchain/evm/chain_state_refresher.go new file mode 100644 index 000000000..1fc00d373 --- /dev/null +++ b/pkg/blockchain/evm/chain_state_refresher.go @@ -0,0 +1,117 @@ +package evm + +import ( + "context" + "fmt" + "time" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + + "github.com/layer-3/nitrolite/pkg/core" +) + +// On-chain ChannelStatus enum values (contracts/src/interfaces/Types.sol). +const ( + onchainChannelStatusVoid uint8 = 0 + onchainChannelStatusOperating uint8 = 1 + onchainChannelStatusDisputed uint8 = 2 + onchainChannelStatusClosed uint8 = 3 + onchainChannelStatusMigratingIn uint8 = 4 + onchainChannelStatusMigratedOut uint8 = 5 +) + +// ChannelChainResolver returns the host chain ID for a given channelID. +// The EVM refresher uses it to dispatch the on-chain read to the right +// ChannelHubCaller when more than one chain hosts ChannelHub deployments. +type ChannelChainResolver func(channelID string) (uint64, error) + +// EVMChainStateRefresher implements core.ChainStateRefresher by dispatching +// getChannelData reads to a per-chain ChannelHubCaller. The caller map is +// populated at startup with one entry per blockchain that has a ChannelHub +// contract deployed; see nitronode/main.go for wire-up. +// +// The refresher is read-only and stateless beyond the caller map; it is safe +// for concurrent use from multiple reactor handler goroutines. +type EVMChainStateRefresher struct { + callers map[uint64]*ChannelHubCaller + resolveHome ChannelChainResolver +} + +// NewEVMChainStateRefresher constructs a refresher backed by the supplied +// per-chain ChannelHubCaller map and chain resolver. The map is taken by +// reference; callers must not mutate it after construction. resolveHome must +// not be nil — the refresher relies on it to pick the right caller for the +// channel under refresh. +func NewEVMChainStateRefresher(callers map[uint64]*ChannelHubCaller, resolveHome ChannelChainResolver) *EVMChainStateRefresher { + return &EVMChainStateRefresher{ + callers: callers, + resolveHome: resolveHome, + } +} + +// RefreshChannelFromChain reads the authoritative on-chain snapshot for +// channelID from the ChannelHub on the channel's host chain and returns it +// for the caller to overwrite the local row. See core.ChainStateRefresher +// for semantics. +func (r *EVMChainStateRefresher) RefreshChannelFromChain(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { + if r.resolveHome == nil { + return nil, fmt.Errorf("no channel chain resolver configured") + } + + chainID, err := r.resolveHome(channelID) + if err != nil { + return nil, fmt.Errorf("resolve home chain for channel %s: %w", channelID, err) + } + + caller, ok := r.callers[chainID] + if !ok || caller == nil { + return nil, fmt.Errorf("no ChannelHub caller registered for chain %d (channel %s)", chainID, channelID) + } + + channelIDBytes, err := hexToBytes32(channelID) + if err != nil { + return nil, fmt.Errorf("invalid channel ID %q: %w", channelID, err) + } + + data, err := caller.GetChannelData(&bind.CallOpts{Context: ctx}, channelIDBytes) + if err != nil { + return nil, fmt.Errorf("getChannelData(%s) on chain %d: %w", channelID, chainID, err) + } + + status, err := mapOnchainChannelStatus(data.Status) + if err != nil { + return nil, fmt.Errorf("map on-chain status for channel %s: %w", channelID, err) + } + + var expiry *time.Time + if data.ChallengeExpiry != nil && data.ChallengeExpiry.Sign() > 0 { + t := time.Unix(data.ChallengeExpiry.Int64(), 0) + expiry = &t + } + + return &core.RefreshedChannel{ + Status: status, + StateVersion: data.LastState.Version, + ChallengeExpiresAt: expiry, + LastStateUserSig: encodeSig(data.LastState.UserSig), + }, nil +} + +// mapOnchainChannelStatus translates the contract's ChannelStatus enum to the +// off-chain core.ChannelStatus. MIGRATING_IN is treated as Open because the +// channel is live from this hub's perspective. MIGRATED_OUT is treated as +// Closed because no further transitions can land on this hub. +func mapOnchainChannelStatus(s uint8) (core.ChannelStatus, error) { + switch s { + case onchainChannelStatusVoid: + return core.ChannelStatusVoid, nil + case onchainChannelStatusOperating, onchainChannelStatusMigratingIn: + return core.ChannelStatusOpen, nil + case onchainChannelStatusDisputed: + return core.ChannelStatusChallenged, nil + case onchainChannelStatusClosed, onchainChannelStatusMigratedOut: + return core.ChannelStatusClosed, nil + default: + return 0, fmt.Errorf("unknown on-chain ChannelStatus %d", s) + } +} diff --git a/pkg/core/interface.go b/pkg/core/interface.go index b6e863f98..6c8ea80f5 100644 --- a/pkg/core/interface.go +++ b/pkg/core/interface.go @@ -71,6 +71,20 @@ type AssetStore interface { GetTokenDecimals(blockchainID uint64, tokenAddress string) (uint8, error) } +// ChainStateRefresher reads the authoritative on-chain channel snapshot from +// the ChannelHub contract on the channel's host chain. Used by event handlers +// to converge the Node row with chain after a version-regression guard drops +// an event whose outer transaction has nonetheless committed state on chain +// (e.g. outer ChannelChallenged dropped when an inner higher-version +// ChannelCheckpointed lands first). +// +// Implementations are responsible for resolving the channel's host chain +// internally. The returned snapshot reflects on-chain state at RPC-read time, +// not event-emit time. +type ChainStateRefresher interface { + RefreshChannelFromChain(ctx context.Context, channelID string) (*RefreshedChannel, error) +} + // Channel lifecycle event handlers type ChannelHubEventHandler interface { HandleNodeBalanceUpdated(context.Context, ChannelHubEventHandlerStore, *NodeBalanceUpdatedEvent) error diff --git a/pkg/core/types.go b/pkg/core/types.go index 57f949bae..f4dbd0529 100644 --- a/pkg/core/types.go +++ b/pkg/core/types.go @@ -140,6 +140,21 @@ func NewChannel(channelID, userWallet, asset string, ChType ChannelType, blockch } } +// RefreshedChannel carries the authoritative on-chain channel snapshot used by +// ChainStateRefresher to converge a Node row that has diverged from chain. +// +// The snapshot reflects on-chain state at RPC-read time, not event-emit time: +// the contract may have advanced the channel through additional transitions +// between when the dropped event was emitted and when the refresh RPC ran. The +// Node row may therefore briefly skip an intermediate status it never observed, +// but it will always converge to a status the chain currently asserts. +type RefreshedChannel struct { + Status ChannelStatus // mapped from on-chain ChannelStatus enum + StateVersion uint64 // from ChannelMeta.lastState.version + ChallengeExpiresAt *time.Time // nil if no active challenge (on-chain expiry is zero) + LastStateUserSig string // hex-encoded user signature for UpdateStateSigsIfMissing backfill; empty when chain has no sig populated +} + // ChannelDefinition represents configuration for creating a channel type ChannelDefinition struct { Nonce uint64 `json:"nonce"` // A unique number to prevent replay attacks @@ -1090,10 +1105,10 @@ func (t1 Transition) Equal(t2 Transition) error { // Blockchain represents information about a supported blockchain network type Blockchain struct { - Name string `json:"name"` // Blockchain name - ID uint64 `json:"id"` // Blockchain network ID - ChannelHubAddress string `json:"channel_hub_address"` // Address of the ChannelHub contract on this blockchain - BlockStep uint64 `json:"block_step"` // Number of blocks between each channel update + Name string `json:"name"` // Blockchain name + ID uint64 `json:"id"` // Blockchain network ID + ChannelHubAddress string `json:"channel_hub_address"` // Address of the ChannelHub contract on this blockchain + BlockStep uint64 `json:"block_step"` // Number of blocks between each channel update } // Asset represents information about a supported asset From 2a6a9f0db6fe0a0d1ac62c1447259047a4ed411b Mon Sep 17 00:00:00 2001 From: nksazonov Date: Sun, 14 Jun 2026 14:05:31 +0200 Subject: [PATCH 3/8] MF3-L19: fix(contracts): add nonReentrant to ChannelHub lifecycle entrypoints --- contracts/src/ChannelHub.sol | 48 ++-- contracts/test/ChannelHub_reentrancy.t.sol | 278 +++++++++++++++++++++ contracts/test/mocks/ReentrantERC20.sol | 49 ++++ 3 files changed, 357 insertions(+), 18 deletions(-) create mode 100644 contracts/test/ChannelHub_reentrancy.t.sol create mode 100644 contracts/test/mocks/ReentrantERC20.sol diff --git a/contracts/src/ChannelHub.sol b/contracts/src/ChannelHub.sol index ea448cadb..9126ef3cc 100644 --- a/contracts/src/ChannelHub.sol +++ b/contracts/src/ChannelHub.sol @@ -351,7 +351,7 @@ contract ChannelHub is ReentrancyGuard { // inflate _nodeBalances during ERC777/hook callbacks, enabling read-only reentrancy for // external protocols querying getNodeBalance(). Contrast with withdrawFromNode, which uses // CEI (decrement before push) to prevent re-entrancy drains. - function depositToNode(address token, uint256 amount) external payable { + function depositToNode(address token, uint256 amount) external payable nonReentrant { require(amount > 0, IncorrectAmount()); _pullFunds(msg.sender, token, amount); @@ -363,7 +363,7 @@ contract ChannelHub is ReentrancyGuard { emit NodeBalanceUpdated(token, updatedBalance); } - function withdrawFromNode(address to, address token, uint256 amount) external { + function withdrawFromNode(address to, address token, uint256 amount) external nonReentrant { require(to != address(0), InvalidAddress()); require(amount > 0, IncorrectAmount()); require(msg.sender == NODE, IncorrectMsgSender()); @@ -445,7 +445,7 @@ contract ChannelHub is ReentrancyGuard { } } - function purgeEscrowDeposits(uint256 maxSteps) external { + function purgeEscrowDeposits(uint256 maxSteps) external nonReentrant { _purgeEscrowDeposits(maxSteps); } @@ -549,7 +549,7 @@ contract ChannelHub is ReentrancyGuard { // This enables users who already have off-chain virtual states with non-zero version // to create a channel and perform initial operation simultaneously // NOTE: For native ETH channels with DEPOSIT intent, msg.sender must supply msg.value == deposit amount. - function createChannel(ChannelDefinition calldata def, State calldata initState) external payable { + function createChannel(ChannelDefinition calldata def, State calldata initState) external payable nonReentrant { require( initState.intent == StateIntent.DEPOSIT || initState.intent == StateIntent.WITHDRAW || initState.intent == StateIntent.OPERATE, @@ -587,7 +587,7 @@ contract ChannelHub is ReentrancyGuard { } // NOTE: For native ETH channels, msg.sender must supply msg.value == deposit amount. - function depositToChannel(bytes32 channelId, State calldata candidate) public payable { + function depositToChannel(bytes32 channelId, State calldata candidate) public payable nonReentrant { require(candidate.intent == StateIntent.DEPOSIT, IncorrectStateIntent()); ChannelDefinition memory def = _channels[channelId].definition; @@ -602,7 +602,7 @@ contract ChannelHub is ReentrancyGuard { emit ChannelDeposited(channelId, candidate); } - function withdrawFromChannel(bytes32 channelId, State calldata candidate) public { + function withdrawFromChannel(bytes32 channelId, State calldata candidate) public nonReentrant { require(candidate.intent == StateIntent.WITHDRAW, IncorrectStateIntent()); ChannelDefinition memory def = _channels[channelId].definition; @@ -617,7 +617,7 @@ contract ChannelHub is ReentrancyGuard { emit ChannelWithdrawn(channelId, candidate); } - function checkpointChannel(bytes32 channelId, State calldata candidate) external { + function checkpointChannel(bytes32 channelId, State calldata candidate) external nonReentrant { require(candidate.intent == StateIntent.OPERATE, IncorrectStateIntent()); // Can only checkpoint operate states ChannelDefinition memory def = _channels[channelId].definition; @@ -637,7 +637,7 @@ contract ChannelHub is ReentrancyGuard { State calldata candidate, bytes calldata challengerSig, ParticipantIndex challengerIdx - ) external payable { + ) external payable nonReentrant { ChannelMeta storage meta = _channels[channelId]; ChannelDefinition memory def = meta.definition; ChannelStatus status = meta.status; @@ -686,7 +686,7 @@ contract ChannelHub is ReentrancyGuard { emit ChannelChallenged(channelId, candidate, challengeExpiry); } - function closeChannel(bytes32 channelId, State calldata candidate) external { + function closeChannel(bytes32 channelId, State calldata candidate) external nonReentrant { ChannelMeta storage meta = _channels[channelId]; ChannelDefinition memory def = meta.definition; ChannelStatus status = meta.status; @@ -726,7 +726,11 @@ contract ChannelHub is ReentrancyGuard { // ========= Cross-Chain Functions ========== // NOTE: On non-home chain, user funds are pulled. For native ETH, msg.sender must supply msg.value == deposit amount. - function initiateEscrowDeposit(ChannelDefinition calldata def, State calldata candidate) external payable { + function initiateEscrowDeposit(ChannelDefinition calldata def, State calldata candidate) + external + payable + nonReentrant + { require(candidate.intent == StateIntent.INITIATE_ESCROW_DEPOSIT, IncorrectStateIntent()); _requireValidDefinition(def); @@ -757,6 +761,7 @@ contract ChannelHub is ReentrancyGuard { function challengeEscrowDeposit(bytes32 escrowId, bytes calldata challengerSig, ParticipantIndex challengerIdx) external + nonReentrant { EscrowDepositMeta storage meta = _escrowDeposits[escrowId]; bytes32 channelId = meta.channelId; @@ -777,7 +782,10 @@ contract ChannelHub is ReentrancyGuard { emit EscrowDepositChallenged(escrowId, meta.initState, effects.newChallengeExpiry); } - function finalizeEscrowDeposit(bytes32 channelId, bytes32 escrowId, State calldata candidate) external { + function finalizeEscrowDeposit(bytes32 channelId, bytes32 escrowId, State calldata candidate) + external + nonReentrant + { if (_isEscrowDepositHomeChain(channelId, escrowId)) { // HOME CHAIN: Get user from channel definition require(candidate.intent == StateIntent.FINALIZE_ESCROW_DEPOSIT, IncorrectStateIntent()); @@ -828,7 +836,7 @@ contract ChannelHub is ReentrancyGuard { emit EscrowDepositFinalized(escrowId, channelId, candidate); } - function initiateEscrowWithdrawal(ChannelDefinition calldata def, State calldata candidate) external { + function initiateEscrowWithdrawal(ChannelDefinition calldata def, State calldata candidate) external nonReentrant { require(candidate.intent == StateIntent.INITIATE_ESCROW_WITHDRAWAL, IncorrectStateIntent()); _requireValidDefinition(def); @@ -858,6 +866,7 @@ contract ChannelHub is ReentrancyGuard { function challengeEscrowWithdrawal(bytes32 escrowId, bytes calldata challengerSig, ParticipantIndex challengerIdx) external + nonReentrant { EscrowWithdrawalMeta storage meta = _escrowWithdrawals[escrowId]; bytes32 channelId = meta.channelId; @@ -878,7 +887,10 @@ contract ChannelHub is ReentrancyGuard { emit EscrowWithdrawalChallenged(escrowId, meta.initState, effects.newChallengeExpiry); } - function finalizeEscrowWithdrawal(bytes32 channelId, bytes32 escrowId, State calldata candidate) external { + function finalizeEscrowWithdrawal(bytes32 channelId, bytes32 escrowId, State calldata candidate) + external + nonReentrant + { if (_isEscrowWithdrawalHomeChain(channelId, escrowId)) { // HOME CHAIN: Get user from channel definition require(candidate.intent == StateIntent.FINALIZE_ESCROW_WITHDRAWAL, IncorrectStateIntent()); @@ -933,7 +945,7 @@ contract ChannelHub is ReentrancyGuard { emit EscrowWithdrawalFinalized(escrowId, channelId, candidate); } - function initiateMigration(ChannelDefinition calldata def, State calldata candidate) external { + function initiateMigration(ChannelDefinition calldata def, State calldata candidate) external nonReentrant { require(candidate.intent == StateIntent.INITIATE_MIGRATION, IncorrectStateIntent()); bytes32 channelId = Utils.getChannelId(def, VERSION); @@ -968,7 +980,7 @@ contract ChannelHub is ReentrancyGuard { } } - function finalizeMigration(bytes32 channelId, State calldata candidate) external { + function finalizeMigration(bytes32 channelId, State calldata candidate) external nonReentrant { require(candidate.intent == StateIntent.FINALIZE_MIGRATION, IncorrectStateIntent()); ChannelDefinition memory def = _channels[channelId].definition; @@ -1429,7 +1441,7 @@ contract ChannelHub is ReentrancyGuard { } } - function _pullFunds(address from, address token, uint256 amount) internal nonReentrant { + function _pullFunds(address from, address token, uint256 amount) internal { if (amount == 0) return; _requireMsgValueForPull(token, amount); @@ -1441,7 +1453,7 @@ contract ChannelHub is ReentrancyGuard { /// @dev Reverts if the transfer fails. Used in non-adversarial contexts where atomicity is required /// (e.g. voluntary vault withdrawals where the caller controls the destination). - function _pushFunds(address to, address token, uint256 amount) internal nonReentrant { + function _pushFunds(address to, address token, uint256 amount) internal { if (amount == 0) return; if (token == address(0)) { @@ -1454,7 +1466,7 @@ contract ChannelHub is ReentrancyGuard { /// @dev Never reverts. On failure, accumulates funds in `_reclaims[to]` for later recovery via `claimFunds()`. /// Used in adversarial contexts (e.g. channel settlement) where a reverting recipient must not block progress. - function _nonRevertingPushFunds(address to, address token, uint256 amount) internal nonReentrant { + function _nonRevertingPushFunds(address to, address token, uint256 amount) internal { if (amount == 0) return; if (token == address(0)) { diff --git a/contracts/test/ChannelHub_reentrancy.t.sol b/contracts/test/ChannelHub_reentrancy.t.sol new file mode 100644 index 000000000..7abf115cf --- /dev/null +++ b/contracts/test/ChannelHub_reentrancy.t.sol @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.30; + +import {Test} from "forge-std/Test.sol"; +import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; + +import {ReentrantERC20} from "./mocks/ReentrantERC20.sol"; +import {TestUtils} from "./TestUtils.sol"; + +import {ChannelHub} from "../src/ChannelHub.sol"; +import {ECDSAValidator} from "../src/sigValidators/ECDSAValidator.sol"; +import { + ChannelDefinition, + ChannelStatus, + DEFAULT_SIG_VALIDATOR_ID, + Ledger, + ParticipantIndex, + State, + StateIntent +} from "../src/interfaces/Types.sol"; +import {ISignatureValidator} from "../src/interfaces/ISignatureValidator.sol"; +import {Utils} from "../src/Utils.sol"; + +/** + * @notice Regression tests for MF3-L19 (audit) — reentrancy via hook-bearing ERC20 tokens during + * the inbound `_pullFunds` callback. Each test exercises one of the four scenarios from + * `reentrancy-finding-validation.md`: + * 1. Outer DEPOSIT pull → inner lifecycle call. + * 2. Outer `challengeChannel(newer-version)` pull → inner lifecycle call. + * 3. Outer escrow-deposit pull → inner lifecycle call. + * 4. Outer `createChannel(DEPOSIT)` pull → inner lifecycle call. + * + * The malicious token (`ReentrantERC20`) calls back into `ChannelHub` from inside + * `transferFrom` before the outer's state mutations and event emit. The expected behavior + * after the MF3-L19 remediation is that OpenZeppelin's `nonReentrant` modifier on the + * outer lifecycle function rejects the inner call with `ReentrancyGuardReentrantCall`. + * + * To make these tests target the guard rather than secondary checks, the inner reentry + * payloads are calls that would otherwise reach guarded code paths quickly. We do not + * require the inner call to be fully signed — the guard fires before any signature + * validation. + */ +// forge-lint: disable-next-item(unsafe-typecast) +contract ChannelHubTest_Reentrancy is Test { + ChannelHub public cHub; + ReentrantERC20 public token; + + uint256 constant NODE_PK = 1; + uint256 constant ALICE_PK = 2; + + address node; + address alice; + + ISignatureValidator immutable ECDSA_SIG_VALIDATOR = new ECDSAValidator(); + + uint8 constant CHANNEL_HUB_VERSION = 1; + uint32 constant CHALLENGE_DURATION = 86400; + uint64 constant NONCE = 1; + uint256 constant INITIAL_BALANCE = 10000; + + bytes4 immutable REENTRANCY_GUARD_SELECTOR = ReentrancyGuard.ReentrancyGuardReentrantCall.selector; + + function setUp() public { + node = vm.addr(NODE_PK); + alice = vm.addr(ALICE_PK); + + cHub = new ChannelHub(ECDSA_SIG_VALIDATOR, node); + token = new ReentrantERC20("Reentrant Token", "REENT"); + + token.mint(node, INITIAL_BALANCE); + token.mint(alice, INITIAL_BALANCE); + + vm.startPrank(node); + token.approve(address(cHub), INITIAL_BALANCE); + cHub.depositToNode(address(token), INITIAL_BALANCE); + vm.stopPrank(); + + vm.prank(alice); + token.approve(address(cHub), INITIAL_BALANCE); + } + + // ========== Helpers ========== + + function _buildDef() internal view returns (ChannelDefinition memory) { + return ChannelDefinition({ + challengeDuration: CHALLENGE_DURATION, + user: alice, + node: node, + nonce: NONCE, + approvedSignatureValidators: 1, + metadata: bytes32(0) + }); + } + + function _signMutual(State memory state, bytes32 channelId) internal pure returns (State memory) { + state.userSig = TestUtils.signStateEip191WithEcdsaValidator(vm, channelId, state, ALICE_PK); + state.nodeSig = TestUtils.signStateEip191WithEcdsaValidator(vm, channelId, state, NODE_PK); + return state; + } + + function _initialDepositState(uint256 amount) internal view returns (State memory) { + return State({ + version: 0, + intent: StateIntent.DEPOSIT, + metadata: bytes32(0), + homeLedger: Ledger({ + chainId: uint64(block.chainid), + token: address(token), + decimals: 18, + userAllocation: amount, + userNetFlow: int256(amount), + nodeAllocation: 0, + nodeNetFlow: 0 + }), + nonHomeLedger: Ledger({ + chainId: 0, + token: address(0), + decimals: 0, + userAllocation: 0, + userNetFlow: 0, + nodeAllocation: 0, + nodeNetFlow: 0 + }), + userSig: "", + nodeSig: "" + }); + } + + function _openChannel(uint256 initialAmount) internal returns (bytes32 channelId, State memory state) { + ChannelDefinition memory def = _buildDef(); + channelId = Utils.getChannelId(def, CHANNEL_HUB_VERSION); + state = _initialDepositState(initialAmount); + state = _signMutual(state, channelId); + + vm.prank(alice); + cHub.createChannel(def, state); + } + + // ========== Scenario 1: createChannel(DEPOSIT) outer → inner depositToChannel ========== + + function test_reentrancy_createChannel_rejectsInnerDepositToChannel() public { + ChannelDefinition memory def = _buildDef(); + bytes32 channelId = Utils.getChannelId(def, CHANNEL_HUB_VERSION); + State memory state = _initialDepositState(1000); + state = _signMutual(state, channelId); + + // Inner call: depositToChannel with empty State data. The guard fires before any decoding, + // so the precise inner state shape is irrelevant. + State memory empty; + bytes memory innerCalldata = abi.encodeCall(ChannelHub.depositToChannel, (channelId, empty)); + token.armReentry(address(cHub), innerCalldata); + + vm.prank(alice); + cHub.createChannel(def, state); + + assertFalse(token.lastReentrySucceeded(), "inner depositToChannel must be rejected"); + bytes memory ret = token.lastReentryReturnData(); + assertGe(ret.length, 4, "inner revert returndata must contain a selector"); + bytes4 sel; + assembly { + sel := mload(add(ret, 0x20)) + } + assertEq(sel, REENTRANCY_GUARD_SELECTOR, "inner revert must be ReentrancyGuardReentrantCall"); + } + + // ========== Scenario 2: depositToChannel outer → inner checkpointChannel ========== + + function test_reentrancy_depositToChannel_rejectsInnerCheckpointChannel() public { + (bytes32 channelId, State memory state) = _openChannel(1000); + + // Build the next state — DEPOSIT bump. + State memory next = + TestUtils.nextState(state, StateIntent.DEPOSIT, [uint256(1500), uint256(0)], [int256(1500), int256(0)]); + next = _signMutual(next, channelId); + + State memory empty; + bytes memory innerCalldata = abi.encodeCall(ChannelHub.checkpointChannel, (channelId, empty)); + token.armReentry(address(cHub), innerCalldata); + + vm.prank(alice); + cHub.depositToChannel(channelId, next); + + assertFalse(token.lastReentrySucceeded(), "inner checkpointChannel must be rejected"); + bytes memory ret = token.lastReentryReturnData(); + assertGe(ret.length, 4, "inner revert returndata must contain a selector"); + bytes4 sel; + assembly { + sel := mload(add(ret, 0x20)) + } + assertEq(sel, REENTRANCY_GUARD_SELECTOR, "inner revert must be ReentrancyGuardReentrantCall"); + } + + // ========== Scenario 3: challengeChannel(newer-version) outer → inner closeChannel ========== + + function test_reentrancy_challengeChannel_newVersion_rejectsInnerCloseChannel() public { + (bytes32 channelId, State memory state) = _openChannel(1000); + + // Newer DEPOSIT state to force pull (`challengeChannel` invokes `_applyTransitionEffects` + // for higher-version candidates, which calls `_pullFunds` on a DEPOSIT intent). + State memory newer = + TestUtils.nextState(state, StateIntent.DEPOSIT, [uint256(1500), uint256(0)], [int256(1500), int256(0)]); + newer = _signMutual(newer, channelId); + + // Challenger sig is over `signingData || "challenge"` (see ChannelHub_Base helper). + bytes memory signingData = Utils.toSigningData(newer); + bytes memory challengerSigningData = abi.encodePacked(signingData, "challenge"); + bytes memory challengerSigPayload = + TestUtils.signEip191(vm, ALICE_PK, Utils.pack(channelId, challengerSigningData)); + bytes memory challengerSig = abi.encodePacked(DEFAULT_SIG_VALIDATOR_ID, challengerSigPayload); + + State memory empty; + bytes memory innerCalldata = abi.encodeCall(ChannelHub.closeChannel, (channelId, empty)); + token.armReentry(address(cHub), innerCalldata); + + vm.prank(alice); + cHub.challengeChannel(channelId, newer, challengerSig, ParticipantIndex.USER); + + assertFalse(token.lastReentrySucceeded(), "inner closeChannel must be rejected"); + bytes memory ret = token.lastReentryReturnData(); + assertGe(ret.length, 4, "inner revert returndata must contain a selector"); + bytes4 sel; + assembly { + sel := mload(add(ret, 0x20)) + } + assertEq(sel, REENTRANCY_GUARD_SELECTOR, "inner revert must be ReentrancyGuardReentrantCall"); + } + + // ========== Scenario 4: initiateEscrowDeposit (non-home chain) outer → inner purgeEscrowDeposits ========== + + function test_reentrancy_initiateEscrowDeposit_nonHome_rejectsInnerPurge() public { + // Construct an escrow-deposit candidate where homeLedger is on a different chain id so the + // current chain is the non-home chain (pull path triggered). + ChannelDefinition memory def = _buildDef(); + bytes32 channelId = Utils.getChannelId(def, CHANNEL_HUB_VERSION); + + State memory candidate = State({ + version: 1, + intent: StateIntent.INITIATE_ESCROW_DEPOSIT, + metadata: bytes32(0), + homeLedger: Ledger({ + chainId: uint64(block.chainid) + 1, // home is a different chain + token: address(token), + decimals: 18, + userAllocation: 0, + userNetFlow: 0, + nodeAllocation: 500, + nodeNetFlow: -500 + }), + nonHomeLedger: Ledger({ + chainId: uint64(block.chainid), // non-home is this chain → pull occurs + token: address(token), + decimals: 18, + userAllocation: 500, + userNetFlow: 500, + nodeAllocation: 0, + nodeNetFlow: 0 + }), + userSig: "", + nodeSig: "" + }); + candidate = _signMutual(candidate, channelId); + + bytes memory innerCalldata = abi.encodeCall(ChannelHub.purgeEscrowDeposits, (uint256(1))); + token.armReentry(address(cHub), innerCalldata); + + vm.prank(alice); + cHub.initiateEscrowDeposit(def, candidate); + + assertFalse(token.lastReentrySucceeded(), "inner purgeEscrowDeposits must be rejected"); + bytes memory ret = token.lastReentryReturnData(); + assertGe(ret.length, 4, "inner revert returndata must contain a selector"); + bytes4 sel; + assembly { + sel := mload(add(ret, 0x20)) + } + assertEq(sel, REENTRANCY_GUARD_SELECTOR, "inner revert must be ReentrancyGuardReentrantCall"); + } +} diff --git a/contracts/test/mocks/ReentrantERC20.sol b/contracts/test/mocks/ReentrantERC20.sol new file mode 100644 index 000000000..e26471aa6 --- /dev/null +++ b/contracts/test/mocks/ReentrantERC20.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.30; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +/** + * @title ReentrantERC20 + * @notice Mock ERC20 token whose transferFrom calls back into a configured target with configured + * calldata before completing the transfer. Used to simulate ERC777-style `tokensToSend` + * hooks for reentrancy testing of ChannelHub lifecycle functions. + * @dev The reentry fires exactly once: the contract clears its reentry config on each transferFrom + * so the inner call (which itself triggers transferFrom on guarded paths) does not recurse + * indefinitely. The reentry call's success/return data is captured in + * `lastReentryReturnData` / `lastReentrySucceeded` so tests can assert on the inner outcome. + */ +contract ReentrantERC20 is ERC20 { + address public reentryTarget; + bytes public reentryCalldata; + bool public lastReentrySucceeded; + bytes public lastReentryReturnData; + bool public reentryArmed; + + constructor(string memory name, string memory symbol) ERC20(name, symbol) {} + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } + + /// @notice Arm the token to perform a single reentry call into `target` with `data` on the + /// next `transferFrom` invocation. The reentry hook fires before the underlying ERC20 state + /// is mutated, mirroring an ERC777 `tokensToSend` callback. + function armReentry(address target, bytes calldata data) external { + reentryTarget = target; + reentryCalldata = data; + reentryArmed = true; + } + + function transferFrom(address from, address to, uint256 amount) public override returns (bool) { + if (reentryArmed) { + reentryArmed = false; + address target = reentryTarget; + bytes memory data = reentryCalldata; + (bool ok, bytes memory ret) = target.call(data); + lastReentrySucceeded = ok; + lastReentryReturnData = ret; + } + return super.transferFrom(from, to, amount); + } +} From f8befcf7cddd24f66adb60a629a82fa58c14aeea Mon Sep 17 00:00:00 2001 From: nksazonov Date: Sun, 14 Jun 2026 14:14:55 +0200 Subject: [PATCH 4/8] MF3-L19: docs: document version-monotonicity guards and reentrancy invariant --- contracts/SECURITY.md | 15 +++++++++++++++ docs/protocol/security-and-limitations.md | 12 ++++++++++++ nitronode/README.md | 10 ++++++++++ 3 files changed, 37 insertions(+) diff --git a/contracts/SECURITY.md b/contracts/SECURITY.md index 54542fbbc..26caa700a 100644 --- a/contracts/SECURITY.md +++ b/contracts/SECURITY.md @@ -51,6 +51,13 @@ Invariant: Invariant: > Credits accrued while a channel was in `CHALLENGED` status are preserved — either reintegrated into the channel on challenge clearance, or carried into the next epoch on closure — and cannot be shadowed by a concurrently-opened replacement channel. +--- + +8. The Node processes on-chain channel-lifecycle events with per-channel version monotonicity. An event whose `StateVersion` is strictly less than the row's current `StateVersion` is dropped with a structured warn log (see `nitronode/event_handlers/service.go`). For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers an on-chain `getChannelData` read via the `ChainStateRefresher` (`pkg/blockchain/evm/chain_state_refresher.go`, interface in `pkg/core/interface.go`) that overwrites the local row's `Status`, `StateVersion`, and `ChallengeExpiresAt`. This is defense-in-depth against out-of-order event delivery from indexer mis-order, reorg replay, or any future contract change that re-introduces a same-transaction event-order quirk. Escrow event handlers enforce the guard without the refresh hook; cross-chain RPC plumbing for escrow refresh is a follow-up item. + +Invariant: +> The Node's local `channels.state_version` is monotonic per `channelId`. After any dropped lifecycle event for a home channel, the Node row converges with on-chain authoritative state without manual intervention. + ## Invariants --- @@ -185,6 +192,12 @@ no funds can be permanently locked if it does. --- +### Reentrancy + +26. **Lifecycle reentrancy guard**: Every external/public function in `ChannelHub` that mutates lifecycle state is guarded by `nonReentrant` modifier. This prevents cross-function reentrancy via inbound token hooks (ERC777-style `tokensReceived`, non-standard `transferFrom` callbacks) from interleaving lifecycle operations during a `_pullFunds` call. The outbound side remains additionally protected by the `TRANSFER_GAS_LIMIT = 100_000` cap, which prevents recipient hooks from completing a reentrant lifecycle call within the forwarded gas budget. + +--- + ### ChannelClosed event orientation during abandoned migration `initiateMigration()` on the new home chain swaps `homeLedger` ↔ `nonHomeLedger` before storing the state, so that `homeLedger` always represents the chain where execution happens. A consequence of this swap is that `meta.lastState` on the new home chain is stored in opposite orientation from what both parties signed. @@ -540,6 +553,8 @@ Inbound transfer failures occur during: **Mitigation**: The Nitronode only processes operations after observing successful on-chain events. If a user signs a deposit state but the transfer fails on-chain, the state is never enforced, and the Node does not provide services based on unconfirmed deposits. +**Reentrancy via inbound hooks**: Tokens whose `transferFrom` invokes a recipient hook (ERC777-style `tokensReceived`, ERC1363 callbacks, or non-standard ERC20 implementations) could in principle re-enter `ChannelHub` lifecycle entrypoints during an inbound pull. This is blocked by the `nonReentrant` guard on every lifecycle entrypoint — see invariant 26 above and `contracts/src/ChannelHub.sol`. The guard makes already-deployed contracts that historically operated under an off-chain "no hook-bearing tokens" policy safe against this class of attack at the contract layer; the off-chain policy remains in effect for already-deployed contracts at specific addresses as a defense-in-depth measure pending operator-controlled token onboarding decisions. + --- ### Outbound Transfer Failures (ChannelHub → User) diff --git a/docs/protocol/security-and-limitations.md b/docs/protocol/security-and-limitations.md index feaefea4d..5a570c08f 100644 --- a/docs/protocol/security-and-limitations.md +++ b/docs/protocol/security-and-limitations.md @@ -47,6 +47,18 @@ The blockchain layer provides the following guarantees: - After the challenge period, the enforced state becomes final - Final state allocations determine asset distribution +## Off-Chain Convergence with On-Chain State + +The Node maintains a local view of each channel's state by subscribing to on-chain events emitted by the `ChannelHub` contract. To defend against out-of-order event delivery — which can be caused by contract-level reentrancy, indexer mis-ordering, or reorg replay — every event handler that mutates `channels.state_version` or `channels.status` enforces a strict version-monotonicity guard: an event whose `StateVersion` is lower than the row's current `StateVersion` is dropped with a structured warn log. + +For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers a chain-state refresh: the Node calls `getChannelData` on the home-chain `ChannelHub` contract via the `ChainStateRefresher` and overwrites the local row with the authoritative on-chain status, version, and challenge expiry. This ensures convergence with chain even when single-event delivery is insufficient — for example, when an outer `ChannelChallenged` is delivered after a higher-version inner `ChannelCheckpointed` emitted from the same transaction. + +The refresh runs synchronously inside the event-processing transaction. On RPC failure the transaction rolls back and the listener replays the event, so the convergence opportunity is never silently lost. + +Escrow event handlers ship the version guard without the refresh hook; the cross-chain RPC plumbing required for escrow refresh is a deferred follow-up item. Pending its arrival, escrow rows can remain divergent from chain across an interim window until the next on-chain event arrives. + +The on-chain side complements this guard: every `ChannelHub` lifecycle entrypoint is protected by OpenZeppelin's `nonReentrant` modifier. This prevents inbound token hooks (ERC777 `tokensReceived`, non-standard `transferFrom` callbacks) from interleaving lifecycle operations and producing the out-of-order events that would otherwise force the Node's defense-in-depth path to fire. + ## Node Liquidity and Cross-Chain Trust Each user channel is opened with a node. To maintain cross-chain functionality, the node MUST hold sufficient liquidity on each supported blockchain to satisfy off-chain state allocations. diff --git a/nitronode/README.md b/nitronode/README.md index cb505b00b..c44e2f3e7 100644 --- a/nitronode/README.md +++ b/nitronode/README.md @@ -37,6 +37,16 @@ The WebSocket RPC service exposes several API groups: For detailed API specifications, see [../docs/api.yaml](../docs/api.yaml). +### Event handler version monotonicity + +Channel-lifecycle events from the blockchain listener are applied with per-channel version monotonicity. If an event whose `StateVersion` is lower than the row's current `StateVersion` arrives (possible under contract reentrancy, indexer mis-order, or reorg replay), the handler logs a structured warning and drops the event without mutating the row. Implementation lives in [`event_handlers/service.go`](event_handlers/service.go). + +For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers a chain-state refresh: the Node fetches the authoritative on-chain channel state via `getChannelData` on the bound `ChannelHub` contract and overwrites the row's `Status`, `StateVersion`, and `ChallengeExpiresAt`. The refresher implementation is [`pkg/blockchain/evm/chain_state_refresher.go`](../pkg/blockchain/evm/chain_state_refresher.go), bound through the [`core.ChainStateRefresher`](../pkg/core/interface.go) interface. + +The refresh runs inside the event-processing transaction. On RPC failure the transaction rolls back and the listener replays the event, so convergence is never silently lost. Escrow event handlers enforce the guard without the refresh hook — cross-chain RPC plumbing for escrow refresh is a follow-up item. + +Operators may see `"event state version is less than current channel state version, ignoring"` warn logs during channel-lifecycle events; these indicate the defense-in-depth path fired and the Node converged with chain. + ## Configuration Nitronode uses YAML files for core configuration and environment variables for sensitive data and runtime overrides. From aa879a2c684c7d8266d0c5160d2f0c79140b6c6b Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 15 Jun 2026 10:12:55 +0200 Subject: [PATCH 5/8] MF3-L19: docs: record hook-token non-support on current deployments --- .../deployments/HOOK-TOKEN-COMPATIBILITY.md | 34 +++++++++++++++++++ docs/protocol/security-and-limitations.md | 1 + nitronode/chart/config/prod-v1/assets.yaml | 4 +++ nitronode/chart/config/sandbox-v1/assets.yaml | 4 +++ nitronode/chart/config/stress-v1/assets.yaml | 4 +++ 5 files changed, 47 insertions(+) create mode 100644 contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md diff --git a/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md b/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md new file mode 100644 index 000000000..b7a4d6043 --- /dev/null +++ b/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md @@ -0,0 +1,34 @@ +# ChannelHub Deployments — Hook-Token Support + +The `ChannelHub` deployments listed in the matrix below **do not support +hook-enabled tokens**. The following token classes MUST NOT be onboarded to +any of these deployments: + +- **ERC777** (e.g. `imBTC`, legacy `xDAI`) +- **ERC1363 / ERC677** +- **Non-standard ERC20 with re-entrant `transferFrom`** (some rebasing or + fee-on-transfer tokens with callbacks) + +Enforcement is the responsibility of the Node operator. This constraint may +be lifted on future deployments to new chains; each new deployment must be +added to the matrix below with its support status recorded explicitly. + +Last reviewed: 2026-06-15. + +## Matrix + +| Chain ID | Chain | ChannelHub Address | Deploy Commit | Deploy Tag | +| ---: | --- | --- | --- | --- | +| 1 | Ethereum | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 14 | Flare | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 56 | BNB Smart Chain | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 137 | Polygon | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 480 | World Chain | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 8453 | Base | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 59144 | Linea | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 80002 | Polygon Amoy | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 84532 | Base Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 59141 | Linea Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 1440000 | XRPL EVM | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | +| 1449000 | XRPL EVM Testnet | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 11155111 | Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | diff --git a/docs/protocol/security-and-limitations.md b/docs/protocol/security-and-limitations.md index 5a570c08f..33a2fe3c2 100644 --- a/docs/protocol/security-and-limitations.md +++ b/docs/protocol/security-and-limitations.md @@ -93,6 +93,7 @@ The following capabilities are not yet implemented or have acknowledged design t - Watchtower services for automated enforcement - Support for non-EVM blockchains - Formal verification of protocol rules +- Hook-enabled tokens (ERC777, ERC1363, non-standard ERC20 with re-entrant `transferFrom`) are not supported on currently-deployed `ChannelHub` instances. The Node operator MUST NOT onboard such tokens. Per-deployment status (chain ID, ChannelHub address, deploy commit) is recorded in [`contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md`](../../contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md). This restriction may be lifted on future deployments to new chains; each new deployment records its support status in the same file. - Stored signatures are validated but not canonicalized. The node persists accepted `user_sig` / `node_sig` values verbatim (unbounded `text`) rather than re-encoding them after verification. A signature can therefore carry non-canonical or unused trailing bytes and still be retained once its cryptographic checks pass — most notably session-key signatures, where `SessionKeyValidator.validateSignature()` ABI-decodes a `(SessionKeyAuthorization, bytes)` payload and verifies only the embedded ECDSA signatures, leaving any extra bytes around an otherwise valid payload intact. Signatures learned from on-chain events (hex-encoded from the candidate and written via `UpdateStateSigsIfMissing`) follow the same path and bypass the WebSocket message-size limit enforced on direct RPC submissions. This carries no asset-safety risk — verification still gates acceptance — but consumers MUST NOT assume stored signature bytes are minimal or canonical. - Session key off-chain scope enforcement does not apply to direct receive-state acknowledgement. Session key expiration and asset-scope restrictions are enforced by the Nitronode off-chain only; the `SessionKeyValidator` contract validates cryptographic signatures alone. A party holding a session key — even one that has expired, been revoked, or been retired — can bypass the `acknowledge` endpoint, manually sign a pending node-issued receive state, and submit it directly to the contract. This is accepted: receive states exclusively increase the user's allocation and cannot redirect funds away from the user, so out-of-scope acknowledgement carries no financial risk and preserves a recovery path when the node is unavailable. diff --git a/nitronode/chart/config/prod-v1/assets.yaml b/nitronode/chart/config/prod-v1/assets.yaml index 865e05755..ce45ae948 100644 --- a/nitronode/chart/config/prod-v1/assets.yaml +++ b/nitronode/chart/config/prod-v1/assets.yaml @@ -3,6 +3,10 @@ # these token inventories. Group only economically equivalent (1:1 redeemable) # tokens under one symbol. Equivalence cannot be verified programmatically and is an # operator responsibility. See docs/protocol/security-and-limitations.md. +# +# WARNING: hook-enabled tokens (ERC777, ERC1363, non-standard ERC20 with re-entrant +# transferFrom) are NOT supported on some currently-deployed ChannelHub instances. +# Per-deployment status: contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md. assets: # Alphabetically sorted by symbol (case insensitive) - name: "Ether" diff --git a/nitronode/chart/config/sandbox-v1/assets.yaml b/nitronode/chart/config/sandbox-v1/assets.yaml index f09f82645..61b1c3c8a 100644 --- a/nitronode/chart/config/sandbox-v1/assets.yaml +++ b/nitronode/chart/config/sandbox-v1/assets.yaml @@ -3,6 +3,10 @@ # these token inventories. Group only economically equivalent (1:1 redeemable) # tokens under one symbol. Equivalence cannot be verified programmatically and is an # operator responsibility. See docs/protocol/security-and-limitations.md. +# +# WARNING: hook-enabled tokens (ERC777, ERC1363, non-standard ERC20 with re-entrant +# transferFrom) are NOT supported on some currently-deployed ChannelHub instances. +# Per-deployment status: contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md. assets: - name: "Ether" symbol: "eth" diff --git a/nitronode/chart/config/stress-v1/assets.yaml b/nitronode/chart/config/stress-v1/assets.yaml index cf343c9d9..e870c3b61 100644 --- a/nitronode/chart/config/stress-v1/assets.yaml +++ b/nitronode/chart/config/stress-v1/assets.yaml @@ -3,6 +3,10 @@ # these token inventories. Group only economically equivalent (1:1 redeemable) # tokens under one symbol. Equivalence cannot be verified programmatically and is an # operator responsibility. See docs/protocol/security-and-limitations.md. +# +# WARNING: hook-enabled tokens (ERC777, ERC1363, non-standard ERC20 with re-entrant +# transferFrom) are NOT supported on some currently-deployed ChannelHub instances. +# Per-deployment status: contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md. assets: - name: "Yellow USD" symbol: "yusd" From 1bed006fc56e2f7333557396c0b3c2b5b203befc Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 15 Jun 2026 14:25:05 +0200 Subject: [PATCH 6/8] MF3-L19: refactor(nitronode): per-reactor ReadOnlyChannelHub for chain-state refresh --- nitronode/event_handlers/service.go | 61 +++--- nitronode/event_handlers/service_test.go | 195 ++++++++---------- nitronode/main.go | 26 +-- pkg/blockchain/evm/chain_state_refresher.go | 64 ++---- pkg/blockchain/evm/channel_hub_reactor.go | 36 ++-- .../evm/channel_hub_reactor_test.go | 42 ++-- pkg/core/interface.go | 37 ++-- pkg/core/types.go | 2 +- 8 files changed, 206 insertions(+), 257 deletions(-) diff --git a/nitronode/event_handlers/service.go b/nitronode/event_handlers/service.go index 99f8154aa..a7b5fe18e 100644 --- a/nitronode/event_handlers/service.go +++ b/nitronode/event_handlers/service.go @@ -18,41 +18,31 @@ var _ core.ChannelHubEventHandler = &EventHandlerService{} type EventHandlerService struct { nodeSigner *core.ChannelDefaultSigner statePacker core.StatePacker - refresher core.ChainStateRefresher } // NewEventHandlerService creates a new EventHandlerService instance. // nodeSigner and statePacker are used to backfill the node signature on the -// checkpointed head state when it is missing from the local record. -// refresher is invoked from the home-channel guard-drop paths to fetch the -// authoritative on-chain snapshot and converge the local row; it is mandatory -// and a nil refresher panics here so a misconfigured wire-up fails loudly -// instead of silently degrading to pre-§B warn-and-skip behaviour. -func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker core.StatePacker, refresher core.ChainStateRefresher) *EventHandlerService { - if refresher == nil { - panic("event_handlers: refresher not wired (§B requires a non-nil ChainStateRefresher)") - } +// checkpointed head state when it is missing from the local record. The +// on-chain refresh used by home-channel guard-drop paths is supplied per-call +// as a core.ReadOnlyChannelHub parameter by the reactor that owns the chain, +// not stored on the service. +func NewEventHandlerService(nodeSigner *core.ChannelDefaultSigner, statePacker core.StatePacker) *EventHandlerService { return &EventHandlerService{ nodeSigner: nodeSigner, statePacker: statePacker, - refresher: refresher, } } // guardEventVersionMonotonic returns true (drop=true) when the incoming event's // StateVersion is strictly less than the row's current StateVersion. The caller -// must `return nil` immediately when drop is true; the helper logs a structured -// warning identifying which event intent arrived stale. +// must `return nil` (or, for home-channel handlers, `return s.refreshAfterDroppedEvent(...)`) +// immediately when drop is true; the helper logs a structured warning identifying +// which event intent arrived stale. // // Intent is a short stable string describing the on-chain transition the dropped // event would have applied (e.g. "checkpointed", "closed", "escrow_deposit_initiated"). -// It surfaces in the warn log so operators can correlate drops with reentrancy -// scenarios. -// -// Once §B lands, this helper is the single chokepoint where the on-drop -// chain-state refresh will be invoked: every dropped event triggers exactly one -// RefreshChannelFromChain call before the function returns drop=true, so the -// refresh policy stays consistent across all six handlers. +// It surfaces in the warn log so operators can correlate drops with reentrancy or +// reorg-replay events. func guardEventVersionMonotonic( ctx context.Context, logger log.Logger, @@ -73,26 +63,27 @@ func guardEventVersionMonotonic( } // refreshAfterDroppedEvent fetches the authoritative on-chain channel snapshot -// via the configured ChainStateRefresher and overwrites the local row's status, -// state version, and challenge expiry, then backfills the user signature on the -// chain-asserted version. Called from the home-channel guard-drop paths to +// via the supplied ReadOnlyChannelHub and overwrites the local row's status, +// state version, and challenge expiry, then backfills the user signature on +// the chain-asserted version. Called from the home-channel guard-drop paths to // close the observability gap described in §B. // // Two-mode error contract: -// - refresher returns nil: row converged, returns nil so the dedup ledger advances. -// - refresher returns an error: returns non-nil so the reactor rolls back the -// tx and the listener replays the event on its next tick. See §B.2. +// - hub returns nil: row converged, returns nil so the dedup ledger advances. +// - hub returns an error: returns non-nil so the reactor rolls back the tx +// and the listener replays the event on its next tick. See §B.2. // -// The refresher is mandatory; NewEventHandlerService panics on a nil refresher -// so this helper can rely on `s.refresher` being non-nil. +// hub is supplied by the reactor that owns this channel's chain; it must be +// non-nil on the guard-drop paths. func (s *EventHandlerService) refreshAfterDroppedEvent( ctx context.Context, tx core.ChannelHubEventHandlerStore, + hub core.ReadOnlyChannelHub, channel *core.Channel, droppedIntent string, ) error { logger := log.FromContext(ctx) - refreshed, err := s.refresher.RefreshChannelFromChain(ctx, channel.ChannelID) + refreshed, err := hub.FetchChannel(ctx, channel.ChannelID) if err != nil { // Surface; if the on-chain read fails we cannot safely converge and must // retry the event by returning a non-nil error so the listener replays. @@ -224,7 +215,7 @@ func (s *EventHandlerService) HandleHomeChannelMigrated(ctx context.Context, tx // is restored instead. Without that restore, a Closing → Challenged → Open sequence driven by // on-chain events would erase the fact that the node has already signed a finalized state, and // CheckActiveChannel would let the user submit further transitions past the finalized state. -func (s *EventHandlerService) HandleHomeChannelCheckpointed(ctx context.Context, tx core.ChannelHubEventHandlerStore, event *core.HomeChannelCheckpointedEvent) error { +func (s *EventHandlerService) HandleHomeChannelCheckpointed(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, event *core.HomeChannelCheckpointedEvent) error { logger := log.FromContext(ctx) chanID := event.ChannelID // Acquire the user's balance-row lock and read the channel under it before mutating @@ -256,7 +247,7 @@ func (s *EventHandlerService) HandleHomeChannelCheckpointed(ctx context.Context, // not regress channel.StateVersion and, critically, so the wasChallenged branch below // does not flip a live challenge back to Open based on a stale version. if guardEventVersionMonotonic(ctx, logger, chanID, "checkpointed", event.StateVersion, channel.StateVersion) { - return s.refreshAfterDroppedEvent(ctx, tx, channel, "checkpointed") + return s.refreshAfterDroppedEvent(ctx, tx, hub, channel, "checkpointed") } channel.StateVersion = event.StateVersion @@ -384,7 +375,7 @@ func (s *EventHandlerService) backfillOffChainHeadNodeSig(ctx context.Context, t // be resolved via ScheduleCheckpoint, and silently queueing an impossible transaction risks // letting the challenge expire on a stale state. A warning is emitted so operators submit the // appropriate on-chain action manually before expiry. -func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, tx core.ChannelHubEventHandlerStore, event *core.HomeChannelChallengedEvent) error { +func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, event *core.HomeChannelChallengedEvent) error { logger := log.FromContext(ctx) chanID := event.ChannelID // Acquire the user's balance-row lock and read the channel under it before mutating @@ -412,7 +403,7 @@ func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, t // Treat as an anomaly (replay, indexer mis-order, contract reentrancy): drop the event and // refresh from chain to converge the local row with the authoritative on-chain status. if guardEventVersionMonotonic(ctx, logger, chanID, "challenged", event.StateVersion, channel.StateVersion) { - return s.refreshAfterDroppedEvent(ctx, tx, channel, "challenged") + return s.refreshAfterDroppedEvent(ctx, tx, hub, channel, "challenged") } channel.StateVersion = event.StateVersion @@ -472,7 +463,7 @@ func (s *EventHandlerService) HandleHomeChannelChallenged(ctx context.Context, t // 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 { +func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, event *core.HomeChannelClosedEvent) error { logger := log.FromContext(ctx) chanID := event.ChannelID // Acquire the user's balance-row lock and read the channel under it before mutating @@ -497,7 +488,7 @@ func (s *EventHandlerService) HandleHomeChannelClosed(ctx context.Context, tx co // Drop stale Closed events that would regress state_version. if guardEventVersionMonotonic(ctx, logger, chanID, "closed", event.StateVersion, channel.StateVersion) { - return s.refreshAfterDroppedEvent(ctx, tx, channel, "closed") + return s.refreshAfterDroppedEvent(ctx, tx, hub, channel, "closed") } wasChallenged := channel.Status == core.ChannelStatusChallenged diff --git a/nitronode/event_handlers/service_test.go b/nitronode/event_handlers/service_test.go index 4e555af2c..30a10b222 100644 --- a/nitronode/event_handlers/service_test.go +++ b/nitronode/event_handlers/service_test.go @@ -368,7 +368,7 @@ func TestHandleHomeChannelCheckpointed_Success(t *testing.T) { mockStore.On("GetLastStateByChannelID", channelID, false).Return(nil, nil) // Execute - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) // Assert require.NoError(t, err) @@ -414,7 +414,7 @@ func TestHandleHomeChannelCheckpointed_FromVoidPromotesToOpen(t *testing.T) { // Challenged path and must not be reached here. mockStore.On("GetLastStateByChannelID", channelID, false).Return(nil, nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -503,7 +503,7 @@ func TestHandleHomeChannelCheckpointed_FromVoidBackfillsUnsignedReceiverHead(t * capturedNodeSig = args.String(3) }).Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.NotEmpty(t, capturedNodeSig, "node signature must be populated on backfill") @@ -564,7 +564,7 @@ func TestHandleHomeChannelCheckpointed_DoesNotReopenFinalizedChannel(t *testing. mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "", "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) // Not challenged → no Finalize lookup and no head-sig backfill on this path. @@ -612,7 +612,7 @@ func TestHandleHomeChannelChallenged_PersistsChallenge(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(4), "", "").Return(nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -627,10 +627,10 @@ func TestHandleHomeChannelChallenged_StaleVersionIgnored(t *testing.T) { // returns the authoritative snapshot and the row converges to the chain view, NEVER to the // stale event payload. mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -659,7 +659,7 @@ func TestHandleHomeChannelChallenged_StaleVersionIgnored(t *testing.T) { } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { // Row must converge to the refreshed (== current) chain view, NOT to the stale event payload. return ch.ChannelID == channelID && @@ -669,14 +669,14 @@ func TestHandleHomeChannelChallenged_StaleVersionIgnored(t *testing.T) { })).Return(nil) mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, mockHub, event) require.NoError(t, err) // Row state must reflect the refreshed chain snapshot, NOT the stale event payload. require.Equal(t, uint64(5), channel.StateVersion, "StateVersion must not regress to the stale event version") require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must come from refresh, not the stale event") mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // LastStateUserSig is empty, so UpdateStateSigsIfMissing must be skipped (documented intent). mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } @@ -697,7 +697,7 @@ func TestHandleHomeChannelChallenged_ChannelNotFound(t *testing.T) { mockStore.On("LockUserStateForHomeChannel", channelID).Return(nil, nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -725,7 +725,7 @@ func TestHandleHomeChannelChallenged_TypeMismatch(t *testing.T) { mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -773,7 +773,7 @@ func TestHandleHomeChannelChallenged_FromClosingState(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(4), "", "").Return(nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -820,7 +820,7 @@ func TestHandleHomeChannelChallenged_AcquiresUserLockBeforeMutation(t *testing.T mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(4), "", "").Return(nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) } @@ -861,7 +861,7 @@ func TestHandleHomeChannelClosed_Success(t *testing.T) { mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), "", "").Return(nil) // Execute - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) // Assert require.NoError(t, err) @@ -1592,7 +1592,7 @@ func TestHandleHomeChannelCheckpointed_BackfillsUserSig(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), userSig, "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -1630,7 +1630,7 @@ func TestHandleHomeChannelCheckpointed_BackfillError(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "0xdeadbeef", "").Return(errors.New("db error")) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.Error(t, err) require.Contains(t, err.Error(), "db error") @@ -1654,24 +1654,20 @@ func newTestEventHandlerService(t *testing.T) (*EventHandlerService, string) { nodeSigner, err := core.NewChannelDefaultSigner(signer) require.NoError(t, err) packer := core.NewStatePackerV1(mockEventHandlerAssetStore{}) - // Default refresher: a MockChainStateRefresher with no expectations set. - // Tests that don't exercise the §B refresh path get a non-nil refresher so - // NewEventHandlerService's wire-up check passes; if a test inadvertently - // triggers RefreshChannelFromChain, testify/mock panics loudly with an - // unexpected-call assertion — the desired loud-failure behaviour. - // Tests asserting refresh interactions use newTestEventHandlerServiceWithRefresher. - return NewEventHandlerService(nodeSigner, packer, new(MockChainStateRefresher)), signer.PublicKey().Address().String() -} - -// MockChainStateRefresher is a testify/mock implementation of core.ChainStateRefresher -// used by §B tests (E.11, E.12, and the §A1/§A2 guard-drop tests that now invoke -// the refresher). -type MockChainStateRefresher struct { + return NewEventHandlerService(nodeSigner, packer), signer.PublicKey().Address().String() +} + +// MockReadOnlyChannelHub is a testify/mock implementation of core.ReadOnlyChannelHub +// used by §B tests (E.11, E.12, and the §A1/§A2 guard-drop tests that invoke the +// on-chain refresh). Tests that do not exercise the refresh path can pass a fresh +// MockReadOnlyChannelHub with no expectations set: testify/mock panics loudly with +// an unexpected-call assertion if FetchChannel is invoked inadvertently. +type MockReadOnlyChannelHub struct { mock.Mock } -// RefreshChannelFromChain mocks the on-drop chain-state refresh hook. -func (m *MockChainStateRefresher) RefreshChannelFromChain(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { +// FetchChannel mocks the on-drop chain-state refresh hook. +func (m *MockReadOnlyChannelHub) FetchChannel(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { args := m.Called(ctx, channelID) if args.Get(0) == nil { return nil, args.Error(1) @@ -1679,15 +1675,6 @@ func (m *MockChainStateRefresher) RefreshChannelFromChain(ctx context.Context, c return args.Get(0).(*core.RefreshedChannel), args.Error(1) } -// newTestEventHandlerServiceWithRefresher constructs an EventHandlerService with a -// caller-provided ChainStateRefresher for tests that exercise the refresh path. -func newTestEventHandlerServiceWithRefresher(t *testing.T, refresher core.ChainStateRefresher) (*EventHandlerService, string) { - t.Helper() - svc, addr := newTestEventHandlerService(t) - svc.refresher = refresher - return svc, addr -} - // TestHandleHomeChannelCheckpointed_BackfillsHeadNodeSig covers the case where a // challenge is cleared while the off-chain head sits above the checkpointed onchain // version: a receiver state stored unsigned during the challenge window is now the @@ -1764,7 +1751,7 @@ func TestHandleHomeChannelCheckpointed_BackfillsHeadNodeSig(t *testing.T) { capturedNodeSig = args.String(3) }).Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.NotEmpty(t, capturedNodeSig, "node signature must be populated on backfill") @@ -1845,7 +1832,7 @@ func TestHandleHomeChannelCheckpointed_HeadAlreadySigned_NoBackfill(t *testing.T mockStore.On("HasSignedFinalize", channelID).Return(false, nil) mockStore.On("GetLastStateByChannelID", channelID, false).Return(headState, nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -1922,7 +1909,7 @@ func TestHandleHomeChannelCheckpointed_FromChallengedWithSignedFinalize(t *testi // Backfill path: off-chain head is the same already-signed Finalize state — no-op. mockStore.On("GetLastStateByChannelID", channelID, false).Return(finalizeState, nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -1973,7 +1960,7 @@ func TestHandleHomeChannelCheckpointed_AcquiresUserLockBeforeMutation(t *testing mockStore.On("HasSignedFinalize", channelID).Return(false, nil) mockStore.On("GetLastStateByChannelID", channelID, false).Return(nil, nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) } @@ -2050,7 +2037,7 @@ func TestHandleHomeChannelClosed_ChallengeRescue_Squash(t *testing.T) { return true }), "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) @@ -2148,7 +2135,7 @@ func TestHandleHomeChannelClosed_ChallengeRescue_NoCredits(t *testing.T) { }), "").Return(nil) mockStore.On("RecordTransaction", mock.Anything, "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) @@ -2226,7 +2213,7 @@ func TestHandleHomeChannelClosed_ChallengeRescue_NegativeNet_ClampsToZero(t *tes }), "").Return(nil) mockStore.On("RecordTransaction", mock.Anything, "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) @@ -2318,7 +2305,7 @@ func TestHandleHomeChannelClosed_TimeoutAfterFinalize_AppendsRescue(t *testing.T return true }), "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) @@ -2428,7 +2415,7 @@ func TestHandleHomeChannelClosed_CooperativeCloseAfterChallenge_ZeroRescue(t *te return true }), "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.TransitionTypeChallengeRescue, capturedState.Transition.Type) @@ -2486,7 +2473,7 @@ func TestHandleHomeChannelClosed_OpenChannel_NoRescue(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "USDC").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, closureVersion, "", "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -2556,10 +2543,10 @@ func TestHandleEscrowDepositsPurged_StoreError_Propagates(t *testing.T) { // payload is NOT what drives the write — the chain view is. func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -2589,7 +2576,7 @@ func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { // Row must converge to the refreshed (== current) chain view, NOT to the stale event payload. return ch.ChannelID == channelID && @@ -2600,14 +2587,14 @@ func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, mockHub, event) require.NoError(t, err) // Row state must reflect the refreshed chain snapshot, NOT the stale event payload. require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress to the stale event version") require.Equal(t, core.ChannelStatusOpen, channel.Status, "Status must come from refresh, not the stale event") mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // The stale event's UserSig at the regressed version must never be written. mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", channelID, uint64(5), mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) @@ -2623,10 +2610,10 @@ func TestHandleHomeChannelCheckpointed_RegressionDropped(t *testing.T) { // cleared the challenge. func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -2658,7 +2645,7 @@ func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testin } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() // Critical: UpdateChannel must persist the Challenged snapshot from chain, NOT the cleared // snapshot the stale event's wasChallenged branch would have produced. mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { @@ -2671,7 +2658,7 @@ func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testin mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, mockHub, event) require.NoError(t, err) // Challenge state preserved via chain refresh, not via the stale event's payload. @@ -2680,7 +2667,7 @@ func TestHandleHomeChannelCheckpointed_RegressionDoesNotClearChallenge(t *testin require.Equal(t, expiryTime.Unix(), channel.ChallengeExpiresAt.Unix(), "ChallengeExpiresAt must be unchanged") require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // The stale wasChallenged branch must NOT run: no HasSignedFinalize lookup, no sig backfill // at the stale version, no head-sig backfill via GetLastStateByChannelID. mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) @@ -2724,7 +2711,7 @@ func TestHandleHomeChannelCheckpointed_EqualVersionAccepted(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "0xusersig", "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -2773,7 +2760,7 @@ func TestHandleHomeChannelCheckpointed_HigherVersionAccepted(t *testing.T) { mockStore.On("HasSignedFinalize", channelID).Return(false, nil) mockStore.On("GetLastStateByChannelID", channelID, false).Return(nil, nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) mockStore.AssertExpectations(t) @@ -2789,10 +2776,10 @@ func TestHandleHomeChannelCheckpointed_HigherVersionAccepted(t *testing.T) { // rescue is issued. func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -2823,7 +2810,7 @@ func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { return ch.ChannelID == channelID && ch.Status == core.ChannelStatusChallenged && @@ -2833,13 +2820,13 @@ func TestHandleHomeChannelClosed_RegressionDropped(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(10), refreshedSig, "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, mockHub, event) require.NoError(t, err) require.Equal(t, uint64(10), channel.StateVersion, "StateVersion must not regress") require.Equal(t, core.ChannelStatusChallenged, channel.Status, "Status must not be flipped to Closed by stale event") mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // Critically, no rescue issuance — the rescue branch belongs to the happy-path close, // not to the refresh path. SumNetTransitionAmountAfterVersion / StoreUserState / // RecordTransaction must all be skipped. @@ -3098,14 +3085,14 @@ func TestHandleHomeChannelClosed_RescueIdempotentOnEqualVersionReplay(t *testing mockStore.On("RecordTransaction", mock.Anything, "").Return(nil).Once() // First call: wasChallenged=true → rescue fires. - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.ChannelStatusClosed, channel.Status, "Status must be Closed after first call") // Second call: equal-version replay. Status is now Closed → wasChallenged=false → // rescue branch must not re-enter. The `.Once()` constraints on the rescue mocks // will fail if any are called a second time. - err = service.HandleHomeChannelClosed(ctx, mockStore, event) + err = service.HandleHomeChannelClosed(ctx, mockStore, new(MockReadOnlyChannelHub), event) require.NoError(t, err) require.Equal(t, core.ChannelStatusClosed, channel.Status, "Status remains Closed after replay") @@ -3162,8 +3149,8 @@ func TestHandleXxx_EqualVersionReplay_NoSideEffects(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, "usdc").Return(nil).Times(2) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), "0xusersig", "").Return(nil).Times(2) - require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, event)) - require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, event)) + require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event)) + require.NoError(t, service.HandleHomeChannelCheckpointed(ctx, mockStore, new(MockReadOnlyChannelHub), event)) mockStore.AssertExpectations(t) // No head-sig backfill on the Open→Open path, even on replay. @@ -3328,10 +3315,10 @@ func TestHandleXxx_EqualVersionReplay_NoSideEffects(t *testing.T) { // the refreshed sig at the refreshed version. See spec §B.2. func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3366,7 +3353,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { return ch.ChannelID == channelID && ch.Status == core.ChannelStatusChallenged && @@ -3377,7 +3364,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(5), chainSig, "").Return(nil) - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, mockHub, event) require.NoError(t, err) require.Equal(t, core.ChannelStatusChallenged, channel.Status, "row must converge to chain Challenged") @@ -3386,8 +3373,8 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { require.Equal(t, chainExpiry.Unix(), channel.ChallengeExpiresAt.Unix()) mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) - mockRefresher.AssertNumberOfCalls(t, "RefreshChannelFromChain", 1) + mockHub.AssertExpectations(t) + mockHub.AssertNumberOfCalls(t, "FetchChannel", 1) } // §E.11 (Checkpointed-guard-path variant). Fires HandleHomeChannelCheckpointed @@ -3396,10 +3383,10 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh(t *testing.T) { // converges. This catches A.1's refresh hook independently from the Challenged handler. func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3430,7 +3417,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { return ch.Status == core.ChannelStatusChallenged && ch.StateVersion == 8 && @@ -3440,7 +3427,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(8), chainSig, "").Return(nil) - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, mockHub, event) require.NoError(t, err) require.Equal(t, core.ChannelStatusChallenged, channel.Status) @@ -3448,7 +3435,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t require.NotNil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // The wasChallenged branch's stale work must not run. mockStore.AssertNotCalled(t, "HasSignedFinalize", mock.Anything) mockStore.AssertNotCalled(t, "GetLastStateByChannelID", mock.Anything, mock.Anything) @@ -3461,10 +3448,10 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_CheckpointedGuardPath(t // version; the refresh path picks up that authoritative view. func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3493,7 +3480,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testi } mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(refreshed, nil).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(refreshed, nil).Once() mockStore.On("UpdateChannel", mock.MatchedBy(func(ch core.Channel) bool { return ch.Status == core.ChannelStatusClosed && ch.StateVersion == 12 && @@ -3502,7 +3489,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testi mockStore.On("RefreshUserEnforcedBalance", userWallet, asset).Return(nil) mockStore.On("UpdateStateSigsIfMissing", channelID, uint64(12), chainSig, "").Return(nil) - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, mockHub, event) require.NoError(t, err) require.Equal(t, core.ChannelStatusClosed, channel.Status) @@ -3510,7 +3497,7 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testi require.Nil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // Rescue branch belongs to the happy-path close, not the refresh path. mockStore.AssertNotCalled(t, "SumNetTransitionAmountAfterVersion", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "StoreUserState", mock.Anything, mock.Anything) @@ -3518,16 +3505,16 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testi } // §E.12 — Refresher-error replay test (Challenged guard path). -// Per spec §B.2's two-mode error contract: when RefreshChannelFromChain fails, the +// Per spec §B.2's two-mode error contract: when ReadOnlyChannelHub.FetchChannel fails, the // handler must return a non-nil error wrapping the RPC failure so the outer reactor // transaction rolls back. The dedup ledger does NOT advance and the event will be // re-delivered on the next listener tick. No partial write to the channel row. func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3549,9 +3536,9 @@ func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { rpcErr := errors.New("rpc unavailable") mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(nil, rpcErr).Once() - err := service.HandleHomeChannelChallenged(ctx, mockStore, event) + err := service.HandleHomeChannelChallenged(ctx, mockStore, mockHub, event) require.Error(t, err, "refresher error must bubble up so the reactor tx rolls back") require.ErrorIs(t, err, rpcErr, "wrapped error must preserve the underlying RPC failure") @@ -3559,7 +3546,7 @@ func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { require.Equal(t, uint64(5), channel.StateVersion, "row must not be mutated on refresh failure") require.Equal(t, core.ChannelStatusOpen, channel.Status, "row must not be mutated on refresh failure") mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) // No partial write. mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) @@ -3569,10 +3556,10 @@ func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { // §E.12 (Checkpointed guard path variant). func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3596,9 +3583,9 @@ func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { rpcErr := errors.New("rpc unavailable") mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(nil, rpcErr).Once() - err := service.HandleHomeChannelCheckpointed(ctx, mockStore, event) + err := service.HandleHomeChannelCheckpointed(ctx, mockStore, mockHub, event) require.Error(t, err) require.ErrorIs(t, err, rpcErr) @@ -3606,7 +3593,7 @@ func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { require.Equal(t, core.ChannelStatusChallenged, channel.Status) require.NotNil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) @@ -3615,10 +3602,10 @@ func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { // §E.12 (Closed guard path variant). func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { mockStore := new(MockStore) - mockRefresher := new(MockChainStateRefresher) + mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) - service, _ := newTestEventHandlerServiceWithRefresher(t, mockRefresher) + service, _ := newTestEventHandlerService(t) channelID := "0xHomeChannel123" userWallet := "0x1234567890123456789012345678901234567890" @@ -3641,9 +3628,9 @@ func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { rpcErr := errors.New("rpc unavailable") mockStore.On("LockUserStateForHomeChannel", channelID).Return(channel, nil) - mockRefresher.On("RefreshChannelFromChain", mock.Anything, channelID).Return(nil, rpcErr).Once() + mockHub.On("FetchChannel", mock.Anything, channelID).Return(nil, rpcErr).Once() - err := service.HandleHomeChannelClosed(ctx, mockStore, event) + err := service.HandleHomeChannelClosed(ctx, mockStore, mockHub, event) require.Error(t, err) require.ErrorIs(t, err, rpcErr) @@ -3651,7 +3638,7 @@ func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { require.Equal(t, core.ChannelStatusChallenged, channel.Status) require.NotNil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) - mockRefresher.AssertExpectations(t) + mockHub.AssertExpectations(t) mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) diff --git a/nitronode/main.go b/nitronode/main.go index 2ad3a20b2..137a6cccf 100644 --- a/nitronode/main.go +++ b/nitronode/main.go @@ -78,23 +78,7 @@ func main() { } eventHandlerStatePacker := core.NewStatePackerV1(bb.MemoryStore) - // Per-chain ChannelHub callers feed the chain-state refresher used by home-channel - // guard-drop paths. The map is populated below as each blockchain is wired up; - // the entry for chain X is always present before that chain's listener starts, so - // the refresher invariant (refresh runs on the same chain as the handler) holds. - channelHubCallers := make(map[uint64]*evm.ChannelHubCaller) - resolveChannelChain := func(channelID string) (uint64, error) { - ch, err := bb.DbStore.GetChannelByID(channelID) - if err != nil { - return 0, fmt.Errorf("lookup channel %s: %w", channelID, err) - } - if ch == nil { - return 0, fmt.Errorf("channel %s not found in DB", channelID) - } - return ch.BlockchainID, nil - } - chainStateRefresher := evm.NewEVMChainStateRefresher(channelHubCallers, resolveChannelChain) - eventHandlerService := event_handlers.NewEventHandlerService(nodeChannelSigner, eventHandlerStatePacker, chainStateRefresher) + eventHandlerService := event_handlers.NewEventHandlerService(nodeChannelSigner, eventHandlerStatePacker) for _, b := range blockchains { rpcURL, ok := bb.BlockchainRPCs[b.ID] @@ -122,13 +106,13 @@ func main() { logger.Fatal("failed to create EVM client") } - // Register a read-only ChannelHub caller for this chain so the chain-state - // refresher can issue getChannelData reads from home-channel guard-drop paths. + // Bind a read-only ChannelHub view for this chain so the reactor can issue + // getChannelData reads from home-channel guard-drop paths. channelHubCaller, err := evm.NewChannelHubCaller(common.HexToAddress(b.ChannelHubAddress), client) if err != nil { logger.Fatal("failed to create ChannelHub caller", "error", err, "blockchainID", b.ID) } - channelHubCallers[b.ID] = channelHubCaller + channelHubReader := evm.NewChannelHubReader(channelHubCaller) sigValidators, err := bb.MemoryStore.GetChannelSigValidators(b.ID) if err != nil { @@ -143,7 +127,7 @@ func main() { return wrapInTx(func(s database.DatabaseStore) error { return h(s) }) } - reactor := evm.NewChannelHubReactor(b.ID, bb.StateSigner.PublicKey().Address().String(), eventHandlerService, bb.MemoryStore, useCHRStoreInTx) + reactor := evm.NewChannelHubReactor(b.ID, bb.StateSigner.PublicKey().Address().String(), eventHandlerService, bb.MemoryStore, useCHRStoreInTx, channelHubReader) reactor.SetOnEventProcessed(bb.RuntimeMetrics.IncBlockchainEvent) l := evm.NewListener(common.HexToAddress(b.ChannelHubAddress), client, b.ID, b.BlockStep, logger, reactor.HandleEvent, bb.DbStore) l.Listen(blockchainCtx, func(err error) { diff --git a/pkg/blockchain/evm/chain_state_refresher.go b/pkg/blockchain/evm/chain_state_refresher.go index 1fc00d373..f85b570ff 100644 --- a/pkg/blockchain/evm/chain_state_refresher.go +++ b/pkg/blockchain/evm/chain_state_refresher.go @@ -20,62 +20,36 @@ const ( onchainChannelStatusMigratedOut uint8 = 5 ) -// ChannelChainResolver returns the host chain ID for a given channelID. -// The EVM refresher uses it to dispatch the on-chain read to the right -// ChannelHubCaller when more than one chain hosts ChannelHub deployments. -type ChannelChainResolver func(channelID string) (uint64, error) - -// EVMChainStateRefresher implements core.ChainStateRefresher by dispatching -// getChannelData reads to a per-chain ChannelHubCaller. The caller map is -// populated at startup with one entry per blockchain that has a ChannelHub -// contract deployed; see nitronode/main.go for wire-up. +// EVMChannelHubReader implements core.ReadOnlyChannelHub by reading from a +// single chain's bound ChannelHub contract. Each ChannelHubReactor binds its +// own reader for the chain it listens on, so no chain-resolution dispatcher +// is required. // -// The refresher is read-only and stateless beyond the caller map; it is safe -// for concurrent use from multiple reactor handler goroutines. -type EVMChainStateRefresher struct { - callers map[uint64]*ChannelHubCaller - resolveHome ChannelChainResolver +// The reader is read-only and stateless beyond the caller; it is safe for +// concurrent use from multiple reactor handler goroutines. +type EVMChannelHubReader struct { + caller *ChannelHubCaller } -// NewEVMChainStateRefresher constructs a refresher backed by the supplied -// per-chain ChannelHubCaller map and chain resolver. The map is taken by -// reference; callers must not mutate it after construction. resolveHome must -// not be nil — the refresher relies on it to pick the right caller for the -// channel under refresh. -func NewEVMChainStateRefresher(callers map[uint64]*ChannelHubCaller, resolveHome ChannelChainResolver) *EVMChainStateRefresher { - return &EVMChainStateRefresher{ - callers: callers, - resolveHome: resolveHome, - } +// NewChannelHubReader constructs a reader backed by the supplied bound +// ChannelHub caller. The caller must be non-nil; passing nil panics on the +// first FetchChannel invocation rather than at construction. +func NewChannelHubReader(caller *ChannelHubCaller) *EVMChannelHubReader { + return &EVMChannelHubReader{caller: caller} } -// RefreshChannelFromChain reads the authoritative on-chain snapshot for -// channelID from the ChannelHub on the channel's host chain and returns it -// for the caller to overwrite the local row. See core.ChainStateRefresher -// for semantics. -func (r *EVMChainStateRefresher) RefreshChannelFromChain(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { - if r.resolveHome == nil { - return nil, fmt.Errorf("no channel chain resolver configured") - } - - chainID, err := r.resolveHome(channelID) - if err != nil { - return nil, fmt.Errorf("resolve home chain for channel %s: %w", channelID, err) - } - - caller, ok := r.callers[chainID] - if !ok || caller == nil { - return nil, fmt.Errorf("no ChannelHub caller registered for chain %d (channel %s)", chainID, channelID) - } - +// FetchChannel reads the authoritative on-chain snapshot for channelID from +// the ChannelHub contract bound to this reader and returns it for the caller +// to overwrite the local row. See core.ReadOnlyChannelHub for semantics. +func (r *EVMChannelHubReader) FetchChannel(ctx context.Context, channelID string) (*core.RefreshedChannel, error) { channelIDBytes, err := hexToBytes32(channelID) if err != nil { return nil, fmt.Errorf("invalid channel ID %q: %w", channelID, err) } - data, err := caller.GetChannelData(&bind.CallOpts{Context: ctx}, channelIDBytes) + data, err := r.caller.GetChannelData(&bind.CallOpts{Context: ctx}, channelIDBytes) if err != nil { - return nil, fmt.Errorf("getChannelData(%s) on chain %d: %w", channelID, chainID, err) + return nil, fmt.Errorf("getChannelData(%s): %w", channelID, err) } status, err := mapOnchainChannelStatus(data.Status) diff --git a/pkg/blockchain/evm/channel_hub_reactor.go b/pkg/blockchain/evm/channel_hub_reactor.go index 0c74b9e68..fb73494df 100644 --- a/pkg/blockchain/evm/channel_hub_reactor.go +++ b/pkg/blockchain/evm/channel_hub_reactor.go @@ -149,16 +149,22 @@ type ChannelHubReactor struct { eventHandler core.ChannelHubEventHandler assetStore AssetStore useStoreInTx ChannelHubReactorStoreTxProvider + channelHubReader core.ReadOnlyChannelHub onEventProcessed func(blockchainID uint64, success bool) } -func NewChannelHubReactor(blockchainID uint64, nodeAddress string, eventHandler core.ChannelHubEventHandler, assetStore AssetStore, useStoreInTx ChannelHubReactorStoreTxProvider) *ChannelHubReactor { +// NewChannelHubReactor wires a reactor for a single chain. channelHubReader is +// the read-only ChannelHub view for this reactor's chain; it is threaded into +// the home-channel guard-drop handlers so they can converge the Node row with +// chain when a version-regression guard drops an event. +func NewChannelHubReactor(blockchainID uint64, nodeAddress string, eventHandler core.ChannelHubEventHandler, assetStore AssetStore, useStoreInTx ChannelHubReactorStoreTxProvider, channelHubReader core.ReadOnlyChannelHub) *ChannelHubReactor { return &ChannelHubReactor{ - blockchainID: blockchainID, - nodeAddress: nodeAddress, - eventHandler: eventHandler, - assetStore: assetStore, - useStoreInTx: useStoreInTx, + blockchainID: blockchainID, + nodeAddress: nodeAddress, + eventHandler: eventHandler, + assetStore: assetStore, + useStoreInTx: useStoreInTx, + channelHubReader: channelHubReader, } } @@ -357,7 +363,7 @@ func (r *ChannelHubReactor) handleHomeChannelCheckpointed(ctx context.Context, s StateVersion: event.Candidate.Version, UserSig: encodeSig(event.Candidate.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleChannelDeposited(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -371,7 +377,7 @@ func (r *ChannelHubReactor) handleChannelDeposited(ctx context.Context, store Ch StateVersion: event.Candidate.Version, UserSig: encodeSig(event.Candidate.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleChannelWithdrawn(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -385,7 +391,7 @@ func (r *ChannelHubReactor) handleChannelWithdrawn(ctx context.Context, store Ch StateVersion: event.Candidate.Version, UserSig: encodeSig(event.Candidate.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleHomeChannelChallenged(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -400,7 +406,7 @@ func (r *ChannelHubReactor) handleHomeChannelChallenged(ctx context.Context, sto ChallengeExpiry: event.ChallengeExpireAt, UserSig: encodeSig(event.Candidate.UserSig), } - return r.eventHandler.HandleHomeChannelChallenged(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelChallenged(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleHomeChannelClosed(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -414,7 +420,7 @@ func (r *ChannelHubReactor) handleHomeChannelClosed(ctx context.Context, store C StateVersion: event.FinalState.Version, UserSig: encodeSig(event.FinalState.UserSig), } - return r.eventHandler.HandleHomeChannelClosed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelClosed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleEscrowDepositInitiated(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -514,7 +520,7 @@ func (r *ChannelHubReactor) handleEscrowDepositInitiatedOnHome(ctx context.Conte StateVersion: event.State.Version, UserSig: encodeSig(event.State.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleEscrowDepositFinalizedOnHome(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -528,7 +534,7 @@ func (r *ChannelHubReactor) handleEscrowDepositFinalizedOnHome(ctx context.Conte StateVersion: event.State.Version, UserSig: encodeSig(event.State.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleEscrowWithdrawalInitiatedOnHome(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -542,7 +548,7 @@ func (r *ChannelHubReactor) handleEscrowWithdrawalInitiatedOnHome(ctx context.Co StateVersion: event.State.Version, UserSig: encodeSig(event.State.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } func (r *ChannelHubReactor) handleEscrowWithdrawalFinalizedOnHome(ctx context.Context, store ChannelHubReactorStore, l types.Log) error { @@ -556,7 +562,7 @@ func (r *ChannelHubReactor) handleEscrowWithdrawalFinalizedOnHome(ctx context.Co StateVersion: event.State.Version, UserSig: encodeSig(event.State.UserSig), } - return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, &ev) + return r.eventHandler.HandleHomeChannelCheckpointed(ctx, store, r.channelHubReader, &ev) } // Additional event handlers for events not yet defined in core.BlockchainEventHandler diff --git a/pkg/blockchain/evm/channel_hub_reactor_test.go b/pkg/blockchain/evm/channel_hub_reactor_test.go index 0f6b833cb..74958e0e5 100644 --- a/pkg/blockchain/evm/channel_hub_reactor_test.go +++ b/pkg/blockchain/evm/channel_hub_reactor_test.go @@ -157,18 +157,18 @@ func (m *mockChannelHubEventHandler) HandleHomeChannelMigrated(ctx context.Conte return args.Error(0) } -func (m *mockChannelHubEventHandler) HandleHomeChannelCheckpointed(ctx context.Context, tx core.ChannelHubEventHandlerStore, ev *core.HomeChannelCheckpointedEvent) error { - args := m.Called(ctx, tx, ev) +func (m *mockChannelHubEventHandler) HandleHomeChannelCheckpointed(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, ev *core.HomeChannelCheckpointedEvent) error { + args := m.Called(ctx, tx, hub, ev) return args.Error(0) } -func (m *mockChannelHubEventHandler) HandleHomeChannelChallenged(ctx context.Context, tx core.ChannelHubEventHandlerStore, ev *core.HomeChannelChallengedEvent) error { - args := m.Called(ctx, tx, ev) +func (m *mockChannelHubEventHandler) HandleHomeChannelChallenged(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, ev *core.HomeChannelChallengedEvent) error { + args := m.Called(ctx, tx, hub, ev) return args.Error(0) } -func (m *mockChannelHubEventHandler) HandleHomeChannelClosed(ctx context.Context, tx core.ChannelHubEventHandlerStore, ev *core.HomeChannelClosedEvent) error { - args := m.Called(ctx, tx, ev) +func (m *mockChannelHubEventHandler) HandleHomeChannelClosed(ctx context.Context, tx core.ChannelHubEventHandlerStore, hub core.ReadOnlyChannelHub, ev *core.HomeChannelClosedEvent) error { + args := m.Called(ctx, tx, hub, ev) return args.Error(0) } @@ -248,12 +248,14 @@ func packNonIndexed(t *testing.T, eventName string, args ...interface{}) []byte return data } -// newReactor creates a ChannelHubReactor wired to the provided mocks. +// newReactor creates a ChannelHubReactor wired to the provided mocks. The +// reactor's read-only ChannelHub view is nil here because the mock event +// handler never reads it — the unit tests in this file assert dispatch only. func newReactor(blockchainID uint64, nodeAddress string, handler *mockChannelHubEventHandler, assetStore *MockAssetStore, store *mockChannelHubStore) *ChannelHubReactor { useStoreInTx := func(fn ChannelHubReactorStoreTxHandler) error { return fn(store) } - return NewChannelHubReactor(blockchainID, nodeAddress, handler, assetStore, useStoreInTx) + return NewChannelHubReactor(blockchainID, nodeAddress, handler, assetStore, useStoreInTx, nil) } // expectStoreContractEvent sets up the mock expectation for StoreContractEvent. @@ -472,7 +474,7 @@ func TestChannelHubReactor_HandleHomeChannelCheckpointed(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 5 })).Return(nil) @@ -512,7 +514,7 @@ func TestChannelHubReactor_HandleHomeChannelCheckpointed_ForwardsUserSig(t *test handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.UserSig == hexutil.Encode([]byte{0xde, 0xad, 0xbe, 0xef}) })).Return(nil) @@ -549,7 +551,7 @@ func TestChannelHubReactor_HandleHomeChannelChallenged(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelChallenged", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelChallengedEvent) bool { + handler.On("HandleHomeChannelChallenged", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelChallengedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 4 && ev.ChallengeExpiry == challengeExpiry @@ -587,7 +589,7 @@ func TestChannelHubReactor_HandleHomeChannelClosed(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelClosed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelClosedEvent) bool { + handler.On("HandleHomeChannelClosed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelClosedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 10 })).Return(nil) @@ -624,7 +626,7 @@ func TestChannelHubReactor_HandleChannelDeposited(t *testing.T) { assetStore := new(MockAssetStore) // ChannelDeposited dispatches HandleHomeChannelCheckpointed - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 7 })).Return(nil) @@ -661,7 +663,7 @@ func TestChannelHubReactor_HandleChannelWithdrawn(t *testing.T) { assetStore := new(MockAssetStore) // ChannelWithdrawn dispatches HandleHomeChannelCheckpointed - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 8 })).Return(nil) @@ -930,7 +932,7 @@ func TestChannelHubReactor_HandleEscrowDepositInitiatedOnHome(t *testing.T) { assetStore := new(MockAssetStore) // Dispatches HandleHomeChannelCheckpointed with the channelId topic - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 3 })).Return(nil) @@ -968,7 +970,7 @@ func TestChannelHubReactor_HandleEscrowDepositFinalizedOnHome(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 6 })).Return(nil) @@ -1006,7 +1008,7 @@ func TestChannelHubReactor_HandleEscrowWithdrawalInitiatedOnHome(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 4 })).Return(nil) @@ -1044,7 +1046,7 @@ func TestChannelHubReactor_HandleEscrowWithdrawalFinalizedOnHome(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(ev *core.HomeChannelCheckpointedEvent) bool { return ev.ChannelID == hexutil.Encode(channelID[:]) && ev.StateVersion == 9 })).Return(nil) @@ -1141,7 +1143,7 @@ func TestChannelHubReactor_OnEventProcessedCallback(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything).Return(nil) + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) expectStoreContractEvent(store, "ChannelCheckpointed", 50, blockchainID) reactor := newReactor(blockchainID, nodeAddr, handler, assetStore, store) @@ -1164,7 +1166,7 @@ func TestChannelHubReactor_OnEventProcessedCallback(t *testing.T) { handler := new(mockChannelHubEventHandler) assetStore := new(MockAssetStore) - handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) + handler.On("HandleHomeChannelCheckpointed", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) reactor := newReactor(blockchainID, nodeAddr, handler, assetStore, store) diff --git a/pkg/core/interface.go b/pkg/core/interface.go index 6c8ea80f5..b6dceb800 100644 --- a/pkg/core/interface.go +++ b/pkg/core/interface.go @@ -71,28 +71,33 @@ type AssetStore interface { GetTokenDecimals(blockchainID uint64, tokenAddress string) (uint8, error) } -// ChainStateRefresher reads the authoritative on-chain channel snapshot from -// the ChannelHub contract on the channel's host chain. Used by event handlers -// to converge the Node row with chain after a version-regression guard drops -// an event whose outer transaction has nonetheless committed state on chain -// (e.g. outer ChannelChallenged dropped when an inner higher-version -// ChannelCheckpointed lands first). -// -// Implementations are responsible for resolving the channel's host chain -// internally. The returned snapshot reflects on-chain state at RPC-read time, -// not event-emit time. -type ChainStateRefresher interface { - RefreshChannelFromChain(ctx context.Context, channelID string) (*RefreshedChannel, error) +// ReadOnlyChannelHub is a read-only view of the on-chain ChannelHub contract, +// used by event handlers to converge the Node row with chain after a guard +// drops an event. Each reactor binds a ReadOnlyChannelHub for its own chain +// and threads it into the handler methods that need an authoritative on-chain +// snapshot; no global multi-chain dispatcher is required. +type ReadOnlyChannelHub interface { + // FetchChannel reads the authoritative on-chain channel snapshot for channelID + // and returns a RefreshedChannel ready to overwrite the Node's local row. The + // snapshot reflects on-chain state at RPC-read time, not event-emit time. + FetchChannel(ctx context.Context, channelID string) (*RefreshedChannel, error) } -// Channel lifecycle event handlers +// ChannelHubEventHandler defines the off-chain reactions to ChannelHub +// blockchain events. Only the three home-channel guard-drop handlers +// (HandleHomeChannelChallenged, HandleHomeChannelCheckpointed, +// HandleHomeChannelClosed) take a ReadOnlyChannelHub: they are the entrypoints +// where a version-regression guard may drop an event whose outer transaction +// has nonetheless committed state on chain, and the on-chain refresh is +// required to converge the Node row with chain. Other handlers do not need +// the hub and so do not accept the parameter, keeping the interface narrow. type ChannelHubEventHandler interface { HandleNodeBalanceUpdated(context.Context, ChannelHubEventHandlerStore, *NodeBalanceUpdatedEvent) error HandleHomeChannelCreated(context.Context, ChannelHubEventHandlerStore, *HomeChannelCreatedEvent) error HandleHomeChannelMigrated(context.Context, ChannelHubEventHandlerStore, *HomeChannelMigratedEvent) error - HandleHomeChannelCheckpointed(context.Context, ChannelHubEventHandlerStore, *HomeChannelCheckpointedEvent) error - HandleHomeChannelChallenged(context.Context, ChannelHubEventHandlerStore, *HomeChannelChallengedEvent) error - HandleHomeChannelClosed(context.Context, ChannelHubEventHandlerStore, *HomeChannelClosedEvent) error + HandleHomeChannelCheckpointed(ctx context.Context, tx ChannelHubEventHandlerStore, hub ReadOnlyChannelHub, event *HomeChannelCheckpointedEvent) error + HandleHomeChannelChallenged(ctx context.Context, tx ChannelHubEventHandlerStore, hub ReadOnlyChannelHub, event *HomeChannelChallengedEvent) error + HandleHomeChannelClosed(ctx context.Context, tx ChannelHubEventHandlerStore, hub ReadOnlyChannelHub, event *HomeChannelClosedEvent) error HandleEscrowDepositInitiated(context.Context, ChannelHubEventHandlerStore, *EscrowDepositInitiatedEvent) error HandleEscrowDepositChallenged(context.Context, ChannelHubEventHandlerStore, *EscrowDepositChallengedEvent) error HandleEscrowDepositFinalized(context.Context, ChannelHubEventHandlerStore, *EscrowDepositFinalizedEvent) error diff --git a/pkg/core/types.go b/pkg/core/types.go index f4dbd0529..92a295704 100644 --- a/pkg/core/types.go +++ b/pkg/core/types.go @@ -141,7 +141,7 @@ func NewChannel(channelID, userWallet, asset string, ChType ChannelType, blockch } // RefreshedChannel carries the authoritative on-chain channel snapshot used by -// ChainStateRefresher to converge a Node row that has diverged from chain. +// ReadOnlyChannelHub to converge a Node row that has diverged from chain. // // The snapshot reflects on-chain state at RPC-read time, not event-emit time: // the contract may have advanced the channel through additional transitions From 163a2fdf77e93b7943097f64fb5158cab4553df4 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 15 Jun 2026 14:37:04 +0200 Subject: [PATCH 7/8] MF3-L19: docs: address PR review feedback on deployment notes and consistency --- contracts/SECURITY.md | 4 ++-- contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md | 2 ++ nitronode/README.md | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/contracts/SECURITY.md b/contracts/SECURITY.md index 26caa700a..42e2252bb 100644 --- a/contracts/SECURITY.md +++ b/contracts/SECURITY.md @@ -53,7 +53,7 @@ Invariant: --- -8. The Node processes on-chain channel-lifecycle events with per-channel version monotonicity. An event whose `StateVersion` is strictly less than the row's current `StateVersion` is dropped with a structured warn log (see `nitronode/event_handlers/service.go`). For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers an on-chain `getChannelData` read via the `ChainStateRefresher` (`pkg/blockchain/evm/chain_state_refresher.go`, interface in `pkg/core/interface.go`) that overwrites the local row's `Status`, `StateVersion`, and `ChallengeExpiresAt`. This is defense-in-depth against out-of-order event delivery from indexer mis-order, reorg replay, or any future contract change that re-introduces a same-transaction event-order quirk. Escrow event handlers enforce the guard without the refresh hook; cross-chain RPC plumbing for escrow refresh is a follow-up item. +8. The Node processes on-chain channel-lifecycle events with per-channel version monotonicity. An event whose `StateVersion` is strictly less than the row's current `StateVersion` is dropped with a structured warn log (see `nitronode/event_handlers/service.go`). For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers an on-chain `getChannelData` read via the `ChainStateRefresher` (`pkg/blockchain/evm/chain_state_refresher.go`, interface in `pkg/core/interface.go`) that overwrites the local row's `Status`, `StateVersion`, and `ChallengeExpiresAt`. This is defense-in-depth against out-of-order event delivery from indexer mis-order, reorg replay, or any future contract change that re-introduces a same-transaction event-order quirk. Escrow event handlers enforce the guard without the refresh hook; cross-chain RPC plumbing for escrow refresh is a deferred follow-up item. Pending its arrival, escrow rows can remain divergent from chain across an interim window until the next on-chain event arrives. Invariant: > The Node's local `channels.state_version` is monotonic per `channelId`. After any dropped lifecycle event for a home channel, the Node row converges with on-chain authoritative state without manual intervention. @@ -553,7 +553,7 @@ Inbound transfer failures occur during: **Mitigation**: The Nitronode only processes operations after observing successful on-chain events. If a user signs a deposit state but the transfer fails on-chain, the state is never enforced, and the Node does not provide services based on unconfirmed deposits. -**Reentrancy via inbound hooks**: Tokens whose `transferFrom` invokes a recipient hook (ERC777-style `tokensReceived`, ERC1363 callbacks, or non-standard ERC20 implementations) could in principle re-enter `ChannelHub` lifecycle entrypoints during an inbound pull. This is blocked by the `nonReentrant` guard on every lifecycle entrypoint — see invariant 26 above and `contracts/src/ChannelHub.sol`. The guard makes already-deployed contracts that historically operated under an off-chain "no hook-bearing tokens" policy safe against this class of attack at the contract layer; the off-chain policy remains in effect for already-deployed contracts at specific addresses as a defense-in-depth measure pending operator-controlled token onboarding decisions. +**Reentrancy via inbound hooks**: Tokens whose `transferFrom` invokes a recipient hook (ERC777-style `tokensReceived`, ERC1363 callbacks, or non-standard ERC20 implementations) could in principle re-enter `ChannelHub` lifecycle entrypoints during an inbound pull. The `nonReentrant` guard on every lifecycle entrypoint blocks this class of attack at the contract layer — see invariant 26 above and `contracts/src/ChannelHub.sol`. Coverage splits by deployment vintage: future deployments built from commit `2a6a9f0d` or later carry the guard and are protected at the contract layer; currently-deployed contracts at the addresses listed in `contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md` predate the guard and are protected only by the off-chain "no hook-bearing tokens" onboarding policy enforced by the Node operator. --- diff --git a/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md b/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md index b7a4d6043..7ba71d48e 100644 --- a/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md +++ b/contracts/deployments/HOOK-TOKEN-COMPATIBILITY.md @@ -28,7 +28,9 @@ Last reviewed: 2026-06-15. | 59144 | Linea | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | | 80002 | Polygon Amoy | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | | 84532 | Base Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 84532 | Base Sepolia | `0x61b9e0767f2eca7e33802e82f9c64b1ebe72ba31` | `9110ba06` | stress v1.3.0 | | 59141 | Linea Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | | 1440000 | XRPL EVM | `0x1a2f750170474d4c54f8d318d9d4343588b4c4d1` | `e07ad9c2` | prod v1.3.0 | | 1449000 | XRPL EVM Testnet | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | | 11155111 | Sepolia | `0x5dba8515af063db0c243c15ece7b99f91459c7c3` | `b88d511c` | sandbox v1.3.0 | +| 11155111 | Sepolia | `0x7d61ec428cfae560f43647af567ea7c6e2cc5527` | `104c13df` | stress v1.3.0 | diff --git a/nitronode/README.md b/nitronode/README.md index c44e2f3e7..b5e5296c3 100644 --- a/nitronode/README.md +++ b/nitronode/README.md @@ -43,7 +43,7 @@ Channel-lifecycle events from the blockchain listener are applied with per-chann For home-channel events (`ChannelChallenged`, `ChannelCheckpointed`, `ChannelClosed`), a dropped event additionally triggers a chain-state refresh: the Node fetches the authoritative on-chain channel state via `getChannelData` on the bound `ChannelHub` contract and overwrites the row's `Status`, `StateVersion`, and `ChallengeExpiresAt`. The refresher implementation is [`pkg/blockchain/evm/chain_state_refresher.go`](../pkg/blockchain/evm/chain_state_refresher.go), bound through the [`core.ChainStateRefresher`](../pkg/core/interface.go) interface. -The refresh runs inside the event-processing transaction. On RPC failure the transaction rolls back and the listener replays the event, so convergence is never silently lost. Escrow event handlers enforce the guard without the refresh hook — cross-chain RPC plumbing for escrow refresh is a follow-up item. +The refresh runs inside the event-processing transaction. On RPC failure the transaction rolls back and the listener replays the event, so convergence is never silently lost. Escrow event handlers enforce the guard without the refresh hook — cross-chain RPC plumbing for escrow refresh is a deferred follow-up item. Pending its arrival, escrow rows can remain divergent from chain across an interim window until the next on-chain event arrives. Operators may see `"event state version is less than current channel state version, ignoring"` warn logs during channel-lifecycle events; these indicate the defense-in-depth path fired and the Node converged with chain. From 92299e39fc1f5dcf7eeae0a1fea18a77a8d3924d Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 15 Jun 2026 14:55:54 +0200 Subject: [PATCH 8/8] MF3-L19: fix(nitronode): bound refresh RPC and log-and-continue on failure --- nitronode/event_handlers/service.go | 20 ++++++++---- nitronode/event_handlers/service_test.go | 35 +++++++++++---------- pkg/blockchain/evm/chain_state_refresher.go | 9 ++++++ 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/nitronode/event_handlers/service.go b/nitronode/event_handlers/service.go index a7b5fe18e..7ae77d2cd 100644 --- a/nitronode/event_handlers/service.go +++ b/nitronode/event_handlers/service.go @@ -68,10 +68,16 @@ func guardEventVersionMonotonic( // the chain-asserted version. Called from the home-channel guard-drop paths to // close the observability gap described in §B. // -// Two-mode error contract: +// Log-and-continue error contract: // - hub returns nil: row converged, returns nil so the dedup ledger advances. -// - hub returns an error: returns non-nil so the reactor rolls back the tx -// and the listener replays the event on its next tick. See §B.2. +// - hub returns an error: logs at Error level and returns nil. The outer tx +// still commits — the dedup row is recorded and the listener moves on, +// but the local channel row stays at whatever the inner higher-version +// event already set it to. No retry, no replay. The row may stay divergent +// from chain (especially for terminal Closed states where no future event +// will arrive). This is an accepted trade-off: strictly better than +// `logger.Fatal`-ing the node on a transient RPC blip, and the failure +// mode requires a guard drop coinciding with a sustained RPC outage. // // hub is supplied by the reactor that owns this channel's chain; it must be // non-nil on the guard-drop paths. @@ -85,9 +91,11 @@ func (s *EventHandlerService) refreshAfterDroppedEvent( logger := log.FromContext(ctx) refreshed, err := hub.FetchChannel(ctx, channel.ChannelID) if err != nil { - // Surface; if the on-chain read fails we cannot safely converge and must - // retry the event by returning a non-nil error so the listener replays. - return fmt.Errorf("refresh after dropped %s: %w", droppedIntent, err) + logger.Error("refresh after dropped event failed, leaving row possibly divergent from chain", + "channelId", channel.ChannelID, + "droppedIntent", droppedIntent, + "error", err) + return nil } channel.Status = refreshed.Status diff --git a/nitronode/event_handlers/service_test.go b/nitronode/event_handlers/service_test.go index 30a10b222..9fa0be221 100644 --- a/nitronode/event_handlers/service_test.go +++ b/nitronode/event_handlers/service_test.go @@ -3504,12 +3504,14 @@ func TestScenario4_OuterChallengeDroppedTriggersRefresh_ClosedGuardPath(t *testi mockStore.AssertNotCalled(t, "RecordTransaction", mock.Anything, mock.Anything) } -// §E.12 — Refresher-error replay test (Challenged guard path). -// Per spec §B.2's two-mode error contract: when ReadOnlyChannelHub.FetchChannel fails, the -// handler must return a non-nil error wrapping the RPC failure so the outer reactor -// transaction rolls back. The dedup ledger does NOT advance and the event will be -// re-delivered on the next listener tick. No partial write to the channel row. -func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { +// §E.12 — Refresher-error log-and-continue test (Challenged guard path). +// Per the Hybrid log-and-continue error contract: when ReadOnlyChannelHub.FetchChannel +// fails, the handler logs at Error level and returns nil so the outer reactor +// transaction commits (dedup row recorded, listener advances). The local channel +// row stays at whatever the inner higher-version event already set it to — no +// convergence happens. There is no retry; this trades transient divergence for +// not killing the node on a transient RPC blip. +func TestGuardDrop_RefresherErrorLoggedAndIgnored(t *testing.T) { mockStore := new(MockStore) mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -3540,21 +3542,22 @@ func TestGuardDrop_RefresherErrorBubblesUp(t *testing.T) { err := service.HandleHomeChannelChallenged(ctx, mockStore, mockHub, event) - require.Error(t, err, "refresher error must bubble up so the reactor tx rolls back") - require.ErrorIs(t, err, rpcErr, "wrapped error must preserve the underlying RPC failure") - // Channel row must be unchanged — no partial write. + require.NoError(t, err, "refresher error must be logged and swallowed so the reactor tx commits") + // Channel row must be unchanged — no convergence happened. require.Equal(t, uint64(5), channel.StateVersion, "row must not be mutated on refresh failure") require.Equal(t, core.ChannelStatusOpen, channel.Status, "row must not be mutated on refresh failure") mockStore.AssertExpectations(t) mockHub.AssertExpectations(t) - // No partial write. + // No retry: FetchChannel called exactly once. + mockHub.AssertNumberOfCalls(t, "FetchChannel", 1) + // No convergence write. mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } // §E.12 (Checkpointed guard path variant). -func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { +func TestGuardDrop_RefresherErrorLoggedAndIgnored_CheckpointedGuardPath(t *testing.T) { mockStore := new(MockStore) mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -3587,20 +3590,20 @@ func TestGuardDrop_RefresherErrorBubblesUp_CheckpointedGuardPath(t *testing.T) { err := service.HandleHomeChannelCheckpointed(ctx, mockStore, mockHub, event) - require.Error(t, err) - require.ErrorIs(t, err, rpcErr) + require.NoError(t, err) require.Equal(t, uint64(10), channel.StateVersion) require.Equal(t, core.ChannelStatusChallenged, channel.Status) require.NotNil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) mockHub.AssertExpectations(t) + mockHub.AssertNumberOfCalls(t, "FetchChannel", 1) mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } // §E.12 (Closed guard path variant). -func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { +func TestGuardDrop_RefresherErrorLoggedAndIgnored_ClosedGuardPath(t *testing.T) { mockStore := new(MockStore) mockHub := new(MockReadOnlyChannelHub) ctx := log.SetContextLogger(context.Background(), log.NewNoopLogger()) @@ -3632,13 +3635,13 @@ func TestGuardDrop_RefresherErrorBubblesUp_ClosedGuardPath(t *testing.T) { err := service.HandleHomeChannelClosed(ctx, mockStore, mockHub, event) - require.Error(t, err) - require.ErrorIs(t, err, rpcErr) + require.NoError(t, err) require.Equal(t, uint64(10), channel.StateVersion) require.Equal(t, core.ChannelStatusChallenged, channel.Status) require.NotNil(t, channel.ChallengeExpiresAt) mockStore.AssertExpectations(t) mockHub.AssertExpectations(t) + mockHub.AssertNumberOfCalls(t, "FetchChannel", 1) mockStore.AssertNotCalled(t, "UpdateChannel", mock.Anything) mockStore.AssertNotCalled(t, "RefreshUserEnforcedBalance", mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "UpdateStateSigsIfMissing", mock.Anything, mock.Anything, mock.Anything, mock.Anything) diff --git a/pkg/blockchain/evm/chain_state_refresher.go b/pkg/blockchain/evm/chain_state_refresher.go index f85b570ff..aae9c6952 100644 --- a/pkg/blockchain/evm/chain_state_refresher.go +++ b/pkg/blockchain/evm/chain_state_refresher.go @@ -20,6 +20,12 @@ const ( onchainChannelStatusMigratedOut uint8 = 5 ) +// refreshRPCTimeout bounds the time `FetchChannel` will wait for a +// `getChannelData` RPC response. Set to 5s to match other per-operation RPC +// timeouts in the codebase (`nitronode/main.go`, `nitronode/runtime.go`) and to +// stay well above realistic call latency on slow public RPCs. +const refreshRPCTimeout = 5 * time.Second + // EVMChannelHubReader implements core.ReadOnlyChannelHub by reading from a // single chain's bound ChannelHub contract. Each ChannelHubReactor binds its // own reader for the chain it listens on, so no chain-resolution dispatcher @@ -47,6 +53,9 @@ func (r *EVMChannelHubReader) FetchChannel(ctx context.Context, channelID string return nil, fmt.Errorf("invalid channel ID %q: %w", channelID, err) } + ctx, cancel := context.WithTimeout(ctx, refreshRPCTimeout) + defer cancel() + data, err := r.caller.GetChannelData(&bind.CallOpts{Context: ctx}, channelIDBytes) if err != nil { return nil, fmt.Errorf("getChannelData(%s): %w", channelID, err)