From f4035ade05d0c44b441f2fe26af89584a76a55d6 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 19 Mar 2024 11:29:12 +0800 Subject: [PATCH] contractcourt: calculate value left when searching for commit deadline This commit changes `findCommitmentDeadline` to `findCommitmentDeadlineAndValue` to calculate the value left from all the time-sensitive HTLCs after subtracting their budgets. This value is then used to calculate the budget to be used when sweeping the anchor output. --- contractcourt/channel_arbitrator.go | 65 +++++++++++----- contractcourt/channel_arbitrator_test.go | 95 +++++++++++++++--------- 2 files changed, 108 insertions(+), 52 deletions(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 72817c23c..2a966629a 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -1321,11 +1321,18 @@ func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions, htlcs htlcSet, anchorPath string) error { // Find the deadline for this specific anchor. - deadline, err := c.findCommitmentDeadline(heightHint, htlcs) + deadlineOpt, _, err := c.findCommitmentDeadlineAndValue( + heightHint, htlcs, + ) if err != nil { return err } + deadline := uint32(1) + deadlineOpt.WhenSome(func(d int32) { + deadline = uint32(d) + }) + // Create a force flag that's used to indicate whether we // should force sweeping this anchor. var force bool @@ -1426,20 +1433,26 @@ func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions, return nil } -// findCommitmentDeadline finds the deadline (relative block height) for a -// commitment transaction by extracting the minimum CLTV from its HTLCs. From -// our PoV, the deadline is defined to be the smaller of, +// findCommitmentDeadlineAndValue finds the deadline (relative block height) +// for a commitment transaction by extracting the minimum CLTV from its HTLCs. +// From our PoV, the deadline is defined to be the smaller of, // - the least CLTV from outgoing HTLCs, or, // - the least CLTV from incoming HTLCs if the preimage is available. // -// Note: when the deadline turns out to be 0 blocks, we will replace it with 1 +// It also finds the total value that are time-sensitive, which is the sum of +// all the outgoing HTLCs plus incoming HTLCs whose preimages are known. It +// then returns the value left after subtracting the budget used for sweeping +// the time-sensitive HTLCs. +// +// NOTE: when the deadline turns out to be 0 blocks, we will replace it with 1 // block because our fee estimator doesn't allow a 0 conf target. This also // means we've left behind and should increase our fee to make the transaction // confirmed asap. -func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32, - htlcs htlcSet) (uint32, error) { +func (c *ChannelArbitrator) findCommitmentDeadlineAndValue(heightHint uint32, + htlcs htlcSet) (fn.Option[int32], btcutil.Amount, error) { deadlineMinHeight := uint32(math.MaxUint32) + totalValue := btcutil.Amount(0) // First, iterate through the outgoingHTLCs to find the lowest CLTV // value. @@ -1453,11 +1466,15 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32, continue } + value := htlc.Amt.ToSatoshis() + totalValue += value + if htlc.RefundTimeout < deadlineMinHeight { deadlineMinHeight = htlc.RefundTimeout + log.Tracef("ChannelArbitrator(%v): outgoing HTLC has "+ - "deadline: %v", c.cfg.ChanPoint, - deadlineMinHeight) + "deadline=%v, value=%v", c.cfg.ChanPoint, + deadlineMinHeight, value) } } @@ -1477,18 +1494,22 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32, // this HTLC. preimageAvailable, err := c.isPreimageAvailable(htlc.RHash) if err != nil { - return 0, err + return fn.None[int32](), 0, err } if !preimageAvailable { continue } + value := htlc.Amt.ToSatoshis() + totalValue += value + if htlc.RefundTimeout < deadlineMinHeight { deadlineMinHeight = htlc.RefundTimeout + log.Tracef("ChannelArbitrator(%v): incoming HTLC has "+ - "deadline: %v", c.cfg.ChanPoint, - deadlineMinHeight) + "deadline=%v, amt=%v", c.cfg.ChanPoint, + deadlineMinHeight, value) } } @@ -1502,9 +1523,9 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32, deadline := deadlineMinHeight - heightHint switch { // When we couldn't find a deadline height from our HTLCs, we will fall - // back to the default value. + // back to the default value as there's no time pressure here. case deadlineMinHeight == math.MaxUint32: - deadline = anchorSweepConfTarget + return fn.None[int32](), 0, nil // When the deadline is passed, we will fall back to the smallest conf // target (1 block). @@ -1515,11 +1536,19 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32, deadline = 1 } - log.Debugf("ChannelArbitrator(%v): calculated deadline: %d, "+ - "using deadlineMinHeight=%d, heightHint=%d", - c.cfg.ChanPoint, deadline, deadlineMinHeight, heightHint) + // Calculate the value left after subtracting the budget used for + // sweeping the time-sensitive HTLCs. + valueLeft := totalValue - calculateBudget( + totalValue, c.cfg.Budget.DeadlineHTLCRatio, + c.cfg.Budget.DeadlineHTLC, + ) - return deadline, nil + log.Debugf("ChannelArbitrator(%v): calculated valueLeft=%v, "+ + "deadline=%d, using deadlineMinHeight=%d, heightHint=%d", + c.cfg.ChanPoint, valueLeft, deadline, deadlineMinHeight, + heightHint) + + return fn.Some(int32(deadline)), valueLeft, nil } // launchResolvers updates the activeResolvers list and starts the resolvers. diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 34f3ff702..96593a9d6 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -2226,9 +2226,10 @@ func TestRemoteCloseInitiator(t *testing.T) { } } -// TestFindCommitmentDeadline tests the logic used to determine confirmation -// deadline is implemented as expected. -func TestFindCommitmentDeadline(t *testing.T) { +// TestFindCommitmentDeadlineAndValue tests the logic used to determine +// confirmation deadline and total time-sensitive value is implemented as +// expected. +func TestFindCommitmentDeadlineAndValue(t *testing.T) { // Create a testing channel arbitrator. log := &mockArbitratorLog{ state: StateDefault, @@ -2251,29 +2252,36 @@ func TestFindCommitmentDeadline(t *testing.T) { heightHint := uint32(1000) htlcExpiryBase := heightHint + uint32(10) + htlcAmt := lnwire.MilliSatoshi(1_000_000) + // Create four testing HTLCs. htlcDust := channeldb.HTLC{ HtlcIndex: htlcIndexBase + 1, RefundTimeout: htlcExpiryBase + 1, OutputIndex: -1, + Amt: htlcAmt, } htlcSmallExipry := channeldb.HTLC{ HtlcIndex: htlcIndexBase + 2, RefundTimeout: htlcExpiryBase + 2, + Amt: htlcAmt, } htlcPreimage := channeldb.HTLC{ HtlcIndex: htlcIndexBase + 3, RefundTimeout: htlcExpiryBase + 3, RHash: rHash, + Amt: htlcAmt, } htlcLargeExpiry := channeldb.HTLC{ HtlcIndex: htlcIndexBase + 4, RefundTimeout: htlcExpiryBase + 100, + Amt: htlcAmt, } htlcExpired := channeldb.HTLC{ HtlcIndex: htlcIndexBase + 5, RefundTimeout: heightHint, + Amt: htlcAmt, } makeHTLCSet := func(incoming, outgoing channeldb.HTLC) htlcSet { @@ -2288,51 +2296,68 @@ func TestFindCommitmentDeadline(t *testing.T) { } testCases := []struct { - name string - htlcs htlcSet - err error - deadline uint32 + name string + htlcs htlcSet + err error + deadline fn.Option[int32] + expectedBudget btcutil.Amount }{ { // When we have no HTLCs, the default value should be // used. - name: "use default conf target", - htlcs: htlcSet{}, - err: nil, - deadline: anchorSweepConfTarget, + name: "use default conf target", + htlcs: htlcSet{}, + err: nil, + deadline: fn.None[int32](), + expectedBudget: 0, }, { // When we have a preimage available in the local HTLC - // set, its CLTV should be used. - name: "use htlc with preimage available", - htlcs: makeHTLCSet(htlcPreimage, htlcLargeExpiry), - err: nil, - deadline: htlcPreimage.RefundTimeout - heightHint, + // set, its CLTV should be used. And the value left + // should be the sum of the HTLCs minus their budgets, + // which is exactly htlcAmt. + name: "use htlc with preimage available", + htlcs: makeHTLCSet(htlcPreimage, htlcLargeExpiry), + err: nil, + deadline: fn.Some(int32( + htlcPreimage.RefundTimeout - heightHint, + )), + expectedBudget: htlcAmt.ToSatoshis(), }, { // When the HTLC in the local set is not preimage // available, we should not use its CLTV even its value - // is smaller. - name: "use htlc with no preimage available", - htlcs: makeHTLCSet(htlcSmallExipry, htlcLargeExpiry), - err: nil, - deadline: htlcLargeExpiry.RefundTimeout - heightHint, + // is smaller. And the value left should be half of + // htlcAmt. + name: "use htlc with no preimage available", + htlcs: makeHTLCSet(htlcSmallExipry, htlcLargeExpiry), + err: nil, + deadline: fn.Some(int32( + htlcLargeExpiry.RefundTimeout - heightHint, + )), + expectedBudget: htlcAmt.ToSatoshis() / 2, }, { // When we have dust HTLCs, their CLTVs should NOT be - // used even the values are smaller. - name: "ignore dust HTLCs", - htlcs: makeHTLCSet(htlcPreimage, htlcDust), - err: nil, - deadline: htlcPreimage.RefundTimeout - heightHint, + // used even the values are smaller. And the value left + // should be half of htlcAmt. + name: "ignore dust HTLCs", + htlcs: makeHTLCSet(htlcPreimage, htlcDust), + err: nil, + deadline: fn.Some(int32( + htlcPreimage.RefundTimeout - heightHint, + )), + expectedBudget: htlcAmt.ToSatoshis() / 2, }, { // When we've reached our deadline, use conf target of - // 1 as our deadline. - name: "use conf target 1", - htlcs: makeHTLCSet(htlcPreimage, htlcExpired), - err: nil, - deadline: 1, + // 1 as our deadline. And the value left should be + // htlcAmt. + name: "use conf target 1", + htlcs: makeHTLCSet(htlcPreimage, htlcExpired), + err: nil, + deadline: fn.Some(int32(1)), + expectedBudget: htlcAmt.ToSatoshis(), }, } @@ -2340,12 +2365,14 @@ func TestFindCommitmentDeadline(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - deadline, err := chanArb.findCommitmentDeadline( - heightHint, tc.htlcs, - ) + deadline, budget, err := chanArb. + findCommitmentDeadlineAndValue( + heightHint, tc.htlcs, + ) require.Equal(t, tc.err, err) require.Equal(t, tc.deadline, deadline) + require.Equal(t, tc.expectedBudget, budget) }) } }