contractcourt: use confheight instead of rescanning the chain
What changed, and why it matters
This change removes a live blockchain rescan that waited for the commitment transaction to confirm, and instead uses a previously known confirmation height stored in the resolver. The main risk is that if the stored confirmation height is wrong or stale, the resolver may compute the wrong unlock time for locked funds, potentially causing premature or delayed sweeps. There is no direct evidence in the commit message or diff that this fixes an active security bug; it appears to be a simplification/optimization.
Verify that c.confirmHeight is always initialized from a trustworthy, confirmed source and is kept up to date across restarts and reorgs. If confirmHeight can be stale or zero, add validation before using it to compute unlockHeight. Consider adding a regression test for reorg or stale-height scenarios.
Security signals we found
Removal of on-chain confirmation wait before computing maturity height
Trust shift from observed confirmation event to stored confirmHeight field
Potential timing issue if confirmHeight does not match actual confirmation block
No explicit security framing or bug reference in commit message
Evidence from the diff
The commit deletes getCommitTxConfHeight(), which registered a chain notification (RegisterConfirmationsNtfn) to learn the confirmation height of the commitment transaction. Launch() now uses c.confirmHeight directly to compute unlockHeight = confirmHeight + MaturityDelay. Tests are updated to pass a fixed testCommitSweepConfHeight and no longer send a TxConfirmation event. The change reduces code and avoids a rescan, but introduces reliance on the accuracy of confirmHeight. If confirmHeight is not reliably set to the actual confirmation block, CSV/CLTV unlock timing could be off.
Changed components
contractcourt/commit_sweep_resolver.gocontractcourt/commit_sweep_resolver_test.goInspect captured patch +25 / −64
diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go
index ea3b061..d8c8c39 100644
--- a/contractcourt/commit_sweep_resolver.go
+++ b/contractcourt/commit_sweep_resolver.go
@@ -123,37 +123,6 @@ func waitForSpend(op *wire.OutPoint, pkScript []byte, heightHint uint32,
}
}
-// getCommitTxConfHeight waits for confirmation of the commitment tx and
-// returns the confirmation height.
-func (c *commitSweepResolver) getCommitTxConfHeight() (uint32, error) {
- txID := c.commitResolution.SelfOutPoint.Hash
- signDesc := c.commitResolution.SelfOutputSignDesc
- pkScript := signDesc.Output.PkScript
-
- const confDepth = 1
-
- confChan, err := c.Notifier.RegisterConfirmationsNtfn(
- &txID, pkScript, confDepth, c.confirmHeight,
- )
- if err != nil {
- return 0, err
- }
- defer confChan.Cancel()
-
- select {
- case txConfirmation, ok := <-confChan.Confirmed:
- if !ok {
- return 0, fmt.Errorf("cannot get confirmation "+
- "for commit tx %v", txID)
- }
-
- return txConfirmation.BlockHeight, nil
-
- case <-c.quit:
- return 0, errResolverShuttingDown
- }
-}
-
// Resolve instructs the contract resolver to resolve the output on-chain. Once
// the output has been *fully* resolved, the function should return immediately
// with a nil ContractResolver value for the first return value. In the case
@@ -381,14 +350,9 @@ func (c *commitSweepResolver) Launch() error {
return nil
}
- confHeight, err := c.getCommitTxConfHeight()
- if err != nil {
- return err
- }
-
// Wait up until the CSV expires, unless we also have a CLTV that
// expires after.
- unlockHeight := confHeight + c.commitResolution.MaturityDelay
+ unlockHeight := c.confirmHeight + c.commitResolution.MaturityDelay
if c.hasCLTV() {
unlockHeight = max(unlockHeight, c.leaseExpiry)
}
diff --git a/contractcourt/commit_sweep_resolver_test.go b/contractcourt/commit_sweep_resolver_test.go
index 6855fdd..5c660e1 100644
--- a/contractcourt/commit_sweep_resolver_test.go
+++ b/contractcourt/commit_sweep_resolver_test.go
@@ -18,6 +18,10 @@ import (
"github.com/stretchr/testify/require"
)
+const (
+ testCommitSweepConfHeight = 99
+)
+
type commitSweepResolverTestContext struct {
resolver *commitSweepResolver
notifier *mock.ChainNotifier
@@ -27,7 +31,8 @@ type commitSweepResolverTestContext struct {
}
func newCommitSweepResolverTestContext(t *testing.T,
- resolution *lnwallet.CommitOutputResolution) *commitSweepResolverTestContext {
+ resolution *lnwallet.CommitOutputResolution,
+ confirmHeight uint32) *commitSweepResolverTestContext {
notifier := &mock.ChainNotifier{
EpochChan: make(chan *chainntnfs.BlockEpoch),
@@ -68,7 +73,7 @@ func newCommitSweepResolverTestContext(t *testing.T,
}
resolver := newCommitSweepResolver(
- *resolution, 0, wire.OutPoint{}, cfg,
+ *resolution, confirmHeight, wire.OutPoint{}, cfg,
)
return &commitSweepResolverTestContext{
@@ -178,7 +183,9 @@ func TestCommitSweepResolverNoDelay(t *testing.T) {
},
}
- ctx := newCommitSweepResolverTestContext(t, &res)
+ ctx := newCommitSweepResolverTestContext(
+ t, &res, testCommitSweepConfHeight,
+ )
// Replace our checkpoint with one which will push reports into a
// channel for us to consume. We replace this function on the resolver
@@ -197,15 +204,12 @@ func TestCommitSweepResolverNoDelay(t *testing.T) {
ctx.resolve()
- spendTx := &wire.MsgTx{}
- spendHash := spendTx.TxHash()
- ctx.notifier.ConfChan <- &chainntnfs.TxConfirmation{
- Tx: spendTx,
- }
-
// No csv delay, so the input should be swept immediately.
<-ctx.sweeper.sweptInputs
+ spendTx := &wire.MsgTx{}
+ spendHash := spendTx.TxHash()
+
amt := btcutil.Amount(res.SelfOutputSignDesc.Output.Value)
expectedReport := &channeldb.ResolverReport{
OutPoint: wire.OutPoint{},
@@ -242,7 +246,10 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) {
SelfOutPoint: outpoint,
}
- ctx := newCommitSweepResolverTestContext(t, &res)
+ // Use confirmHeight = 99, so maturityHeight = 99 + 3 = 102.
+ ctx := newCommitSweepResolverTestContext(
+ t, &res, testCommitSweepConfHeight,
+ )
// Replace our checkpoint with one which will push reports into a
// channel for us to consume. We replace this function on the resolver
@@ -270,25 +277,18 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) {
Amount: btcutil.Amount(amt),
LimboBalance: btcutil.Amount(amt),
}
- if *report != expectedReport {
- t.Fatalf("unexpected resolver report. want=%v got=%v",
- expectedReport, report)
- }
+ require.Equal(t, expectedReport, *report)
ctx.resolve()
- ctx.notifier.ConfChan <- &chainntnfs.TxConfirmation{
- BlockHeight: testInitialBlockHeight - 1,
- }
-
- // Allow resolver to process confirmation.
+ // Allow resolver to launch and update the report.
time.Sleep(sweepProcessInterval)
// Expect report to be updated.
+ // confirmHeight(99) + maturityDelay(3) = 102.
report = ctx.resolver.report()
- if report.MaturityHeight != testInitialBlockHeight+2 {
- t.Fatal("report maturity height incorrect")
- }
+ expectedMaturity := testCommitSweepConfHeight + res.MaturityDelay
+ require.Equal(t, expectedMaturity, report.MaturityHeight)
// Notify initial block height. Although the csv lock is still in
// effect, we expect the input being sent to the sweeper before the csv
@@ -325,13 +325,10 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) {
Outpoint: outpoint,
Type: ReportOutputUnencumbered,
Amount: btcutil.Amount(amt),
- MaturityHeight: testInitialBlockHeight + 2,
+ MaturityHeight: testCommitSweepConfHeight + res.MaturityDelay,
RecoveredBalance: expectedRecoveredBalance,
}
- if *report != expectedReport {
- t.Fatalf("unexpected resolver report. want=%v got=%v",
- expectedReport, report)
- }
+ require.Equal(t, expectedReport, *report)
}
// TestCommitSweepResolverDelay tests resolution of a direct commitment output
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.