From ebefe63f8e24d78ba74d479195e1334cd0b85811 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 30 Oct 2025 16:32:57 -0700 Subject: [PATCH] contractcourt: add sync dispatch fast-path for single confirmation closes In this commit, we add a fast-path optimization to the chain watcher's closeObserver that immediately dispatches close events when only a single confirmation is required (numConfs == 1). This addresses a timing issue with integration tests that were designed around the old synchronous blockbeat behavior, where close events were dispatched immediately upon spend detection. The recent async confirmation architecture (introduced in commit f6f716ab7) properly handles reorgs by waiting for N confirmations before dispatching close events. However, this created a race condition in integration tests that mine blocks synchronously and expect immediate close notifications. With the build tag setting numConfs to 1 for itests, the async confirmation notification could arrive after the test already started waiting for the close event, causing timeouts. We introduce a new handleSpendDispatch method that checks if numConfs == 1 and, if so, immediately calls handleCommitSpend to dispatch the close event synchronously, then returns true to skip the async state machine. This preserves the old behavior for integration tests while maintaining the full async reorg protection for production (where numConfs >= 3). The implementation adds the fast-path check in both spend detection paths (blockbeat and spend notification) to ensure consistent behavior regardless of which detects the spend first. We also update the affected unit tests to remove their expectation of confirmation registration, since the fast-path bypasses that step entirely. This approach optimizes for the integration test scenario without compromising production safety, as the fast-path only activates when a single confirmation is sufficient - a configuration that only exists in the controlled test environment. --- contractcourt/chain_watcher.go | 66 +++++++++++++++++++++++++++-- contractcourt/chain_watcher_test.go | 14 +++--- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index c3fc10d87..8516b5fbe 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -705,6 +705,12 @@ func newChainSet(chanState *channeldb.OpenChannel) (*chainSet, error) { // - Pending (confNtfn != nil): Spend detected, waiting for N confirmations // // - Confirmed: Spend confirmed with N blocks, close has been processed +// +// For single-confirmation scenarios (numConfs == 1), we bypass the async state +// machine and immediately dispatch close events upon spend detection. This +// provides synchronous behavior for integration tests which expect immediate +// notifications. For multi-confirmation scenarios (production with numConfs +// >= 3), we use the full async state machine with reorg protection. func (c *chainWatcher) closeObserver() { defer c.wg.Done() @@ -724,7 +730,7 @@ func (c *chainWatcher) closeObserver() { } spendNtfn := c.fundingSpendNtfn - defer spendNtfn.Cancel() + defer func() { spendNtfn.Cancel() }() // We use these variables to implement a state machine to track the // state of the spend confirmation process: @@ -756,6 +762,7 @@ func (c *chainWatcher) closeObserver() { "duplicate spend detection for tx %v", c.cfg.chanState.FundingOutpoint, spend.SpenderTxHash) + return confNtfn, nil } @@ -815,7 +822,22 @@ func (c *chainWatcher) closeObserver() { continue } + // FAST PATH: Check if we should dispatch immediately + // for single-confirmation scenarios. + if c.handleSpendDispatch(spend, "blockbeat") { + if confNtfn != nil { + confNtfn.Cancel() + confNtfn = nil + } + pendingSpend = nil + continue + } + + // ASYNC PATH: Multiple confirmations (production). // STATE TRANSITION: None -> Pending (from blockbeat). + // We've detected a spend, but don't process it yet. + // Instead, register for confirmations to protect + // against shallow reorgs. log.Infof("ChannelPoint(%v): detected spend from "+ "blockbeat, transitioning to %v", c.cfg.chanState.FundingOutpoint, @@ -825,7 +847,7 @@ func (c *chainWatcher) closeObserver() { if err != nil { log.Errorf("Unable to handle spend "+ "detection: %v", err) - return + continue } pendingSpend = spend confNtfn = newConfNtfn @@ -838,6 +860,18 @@ func (c *chainWatcher) closeObserver() { return } + // FAST PATH: Check if we should dispatch immediately + // for single-confirmation scenarios. + if c.handleSpendDispatch(spend, "spend notification") { + if confNtfn != nil { + confNtfn.Cancel() + confNtfn = nil + } + pendingSpend = nil + continue + } + + // ASYNC PATH: Multiple confirmations (production). log.Infof("ChannelPoint(%v): detected spend from "+ "notification, transitioning to %v", c.cfg.chanState.FundingOutpoint, @@ -847,7 +881,7 @@ func (c *chainWatcher) closeObserver() { if err != nil { log.Errorf("Unable to handle spend "+ "detection: %v", err) - return + continue } pendingSpend = spend confNtfn = newConfNtfn @@ -906,6 +940,8 @@ func (c *chainWatcher) closeObserver() { return } + c.fundingSpendNtfn = spendNtfn + log.Infof("ChannelPoint(%v): re-registered for spend "+ "detection", c.cfg.chanState.FundingOutpoint) @@ -1675,6 +1711,30 @@ func deriveFundingPkScript(chanState *channeldb.OpenChannel) ([]byte, error) { return fundingPkScript, nil } +// handleSpendDispatch processes a detected spend. For single-confirmation +// scenarios (numConfs == 1), it immediately dispatches the close event and +// returns true. For multi-confirmation scenarios, it returns false, indicating +// the caller should proceed with the async state machine. +func (c *chainWatcher) handleSpendDispatch(spend *chainntnfs.SpendDetail, + source string) bool { + + numConfs := c.requiredConfsForSpend() + if numConfs == 1 { + log.Infof("ChannelPoint(%v): single confirmation mode, "+ + "dispatching immediately from %s", + c.cfg.chanState.FundingOutpoint, source) + + err := c.handleCommitSpend(spend) + if err != nil { + log.Errorf("Failed to handle commit spend: %v", err) + } + + return true + } + + return false +} + // handleCommitSpend takes a spending tx of the funding output and handles the // channel close based on the closure type. func (c *chainWatcher) handleCommitSpend( diff --git a/contractcourt/chain_watcher_test.go b/contractcourt/chain_watcher_test.go index c57859ca4..8275886a1 100644 --- a/contractcourt/chain_watcher_test.go +++ b/contractcourt/chain_watcher_test.go @@ -94,10 +94,9 @@ func TestChainWatcherRemoteUnilateralClose(t *testing.T) { t.Fatalf("unable to send blockbeat") } - // Wait for the chain watcher to register for confirmations and send - // the confirmation. Since we set chanCloseConfs to 1, one confirmation - // is sufficient. - aliceNotifier.WaitForConfRegistrationAndSend(t) + // With chanCloseConfs set to 1, the fast-path dispatches immediately + // without confirmation registration. The close event should arrive + // directly after processing the blockbeat. // We should get a new spend event over the remote unilateral close // event channel. @@ -231,10 +230,9 @@ func TestChainWatcherRemoteUnilateralClosePendingCommit(t *testing.T) { t.Fatalf("unable to send blockbeat") } - // Wait for the chain watcher to register for confirmations and send - // the confirmation. Since we set chanCloseConfs to 1, one confirmation - // is sufficient. - aliceNotifier.WaitForConfRegistrationAndSend(t) + // With chanCloseConfs set to 1, the fast-path dispatches immediately + // without confirmation registration. The close event should arrive + // directly after processing the blockbeat. // We should get a new spend event over the remote unilateral close // event channel.