From 7aa9ade4c280894921a639c5beca19280f1b4607 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 17 Oct 2024 13:24:24 +0200 Subject: [PATCH] contratcourt: dont consider dust htlc for anchor sweep Now that we cancel dust htlcs prematurely even before the commitment tx is confirmed we don't consider dust htlcs when creating the cpfp transaction. --- contractcourt/channel_arbitrator.go | 323 ++++++++++++++--------- contractcourt/channel_arbitrator_test.go | 116 +++++++- 2 files changed, 312 insertions(+), 127 deletions(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index ce67908cd..a76fcd330 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -1390,133 +1390,22 @@ func (c *ChannelArbitrator) stateStep( func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions, heightHint uint32) error { - // Use the chan id as the exclusive group. This prevents any of the - // anchors from being batched together. - exclusiveGroup := c.cfg.ShortChanID.ToUint64() - - // sweepWithDeadline is a helper closure that takes an anchor - // resolution and sweeps it with its corresponding deadline. - sweepWithDeadline := func(anchor *lnwallet.AnchorResolution, - htlcs htlcSet, anchorPath string) error { - - // Find the deadline for this specific anchor. - deadline, value, err := c.findCommitmentDeadlineAndValue( - heightHint, htlcs, - ) - if err != nil { - return err - } - - // If we cannot find a deadline, it means there's no HTLCs at - // stake, which means we can relax our anchor sweeping - // conditions as we don't have any time sensitive outputs to - // sweep. However we need to register the anchor output with the - // sweeper so we are later able to bump the close fee. - if deadline.IsNone() { - log.Infof("ChannelArbitrator(%v): no HTLCs at stake, "+ - "sweeping anchor with default deadline", - c.cfg.ChanPoint) - } - - witnessType := input.CommitmentAnchor - - // For taproot channels, we need to use the proper witness - // type. - if txscript.IsPayToTaproot( - anchor.AnchorSignDescriptor.Output.PkScript, - ) { - - witnessType = input.TaprootAnchorSweepSpend - } - - // Prepare anchor output for sweeping. - anchorInput := input.MakeBaseInput( - &anchor.CommitAnchor, - witnessType, - &anchor.AnchorSignDescriptor, - heightHint, - &input.TxInfo{ - Fee: anchor.CommitFee, - Weight: anchor.CommitWeight, - }, - ) - - // If we have a deadline, we'll use it to calculate the - // deadline height, otherwise default to none. - deadlineDesc := "None" - deadlineHeight := fn.MapOption(func(d int32) int32 { - deadlineDesc = fmt.Sprintf("%d", d) - - return d + int32(heightHint) - })(deadline) - - // Calculate the budget based on the value under protection, - // which is the sum of all HTLCs on this commitment subtracted - // by their budgets. - // The anchor output in itself has a small output value of 330 - // sats so we also include it in the budget to pay for the - // cpfp transaction. - budget := calculateBudget( - value, c.cfg.Budget.AnchorCPFPRatio, - c.cfg.Budget.AnchorCPFP, - ) + AnchorOutputValue - - log.Infof("ChannelArbitrator(%v): offering anchor from %s "+ - "commitment %v to sweeper with deadline=%v, budget=%v", - c.cfg.ChanPoint, anchorPath, anchor.CommitAnchor, - deadlineDesc, budget) - - // Sweep anchor output with a confirmation target fee - // preference. Because this is a cpfp-operation, the anchor - // will only be attempted to sweep when the current fee - // estimate for the confirmation target exceeds the commit fee - // rate. - _, err = c.cfg.Sweeper.SweepInput( - &anchorInput, - sweep.Params{ - ExclusiveGroup: &exclusiveGroup, - Budget: budget, - DeadlineHeight: deadlineHeight, - }, - ) - if err != nil { - return err - } - - return nil - } - // Update the set of activeHTLCs so that the sweeping routine has an // up-to-date view of the set of commitments. c.updateActiveHTLCs() - // Sweep anchors based on different HTLC sets. Notice the HTLC sets may - // differ across commitments, thus their deadline values could vary. - for htlcSet, htlcs := range c.activeHTLCs { - switch { - case htlcSet == LocalHtlcSet && anchors.Local != nil: - err := sweepWithDeadline(anchors.Local, htlcs, "local") - if err != nil { - return err - } + // Prepare the sweeping requests for all possible versions of + // commitments. + sweepReqs, err := c.prepareAnchorSweeps(heightHint, anchors) + if err != nil { + return err + } - case htlcSet == RemoteHtlcSet && anchors.Remote != nil: - err := sweepWithDeadline( - anchors.Remote, htlcs, "remote", - ) - if err != nil { - return err - } - - case htlcSet == RemotePendingHtlcSet && - anchors.RemotePending != nil: - - err := sweepWithDeadline( - anchors.RemotePending, htlcs, "remote pending", - ) - if err != nil { - return err - } + // Send out the sweeping requests to the sweeper. + for _, req := range sweepReqs { + _, err = c.cfg.Sweeper.SweepInput(req.input, req.params) + if err != nil { + return err } } @@ -2254,7 +2143,7 @@ func (c *ChannelArbitrator) checkRemoteChainActions( } // checkRemoteDiffActions checks the set difference of the HTLCs on the remote -// confirmed commit and remote dangling commit for HTLCS that we need to cancel +// confirmed commit and remote pending commit for HTLCS that we need to cancel // back. If we find any HTLCs on the remote pending but not the remote, then // we'll mark them to be failed immediately. func (c *ChannelArbitrator) checkRemoteDiffActions( @@ -2277,7 +2166,7 @@ func (c *ChannelArbitrator) checkRemoteDiffActions( } // With the remote HTLCs assembled, we'll mark any HTLCs only on the - // remote dangling commitment to be failed asap. + // remote pending commitment to be failed asap. actionMap := make(ChainActionMap) for _, htlc := range danglingHTLCs.outgoingHTLCs { if _, ok := remoteHtlcs[htlc.HtlcIndex]; ok { @@ -3200,6 +3089,192 @@ func (c *ChannelArbitrator) checkLegacyBreach() (ArbitratorState, error) { return StateContractClosed, nil } +// sweepRequest wraps the arguments used when calling `SweepInput`. +type sweepRequest struct { + // input is the input to be swept. + input input.Input + + // params holds the sweeping parameters. + params sweep.Params +} + +// createSweepRequest creates an anchor sweeping request for a particular +// version (local/remote/remote pending) of the commitment. +func (c *ChannelArbitrator) createSweepRequest( + anchor *lnwallet.AnchorResolution, htlcs htlcSet, anchorPath string, + heightHint uint32) (sweepRequest, error) { + + // Use the chan id as the exclusive group. This prevents any of the + // anchors from being batched together. + exclusiveGroup := c.cfg.ShortChanID.ToUint64() + + // Find the deadline for this specific anchor. + deadline, value, err := c.findCommitmentDeadlineAndValue( + heightHint, htlcs, + ) + if err != nil { + return sweepRequest{}, err + } + + // If we cannot find a deadline, it means there's no HTLCs at stake, + // which means we can relax our anchor sweeping conditions as we don't + // have any time sensitive outputs to sweep. However we need to + // register the anchor output with the sweeper so we are later able to + // bump the close fee. + if deadline.IsNone() { + log.Infof("ChannelArbitrator(%v): no HTLCs at stake, "+ + "sweeping anchor with default deadline", + c.cfg.ChanPoint) + } + + witnessType := input.CommitmentAnchor + + // For taproot channels, we need to use the proper witness type. + if txscript.IsPayToTaproot( + anchor.AnchorSignDescriptor.Output.PkScript, + ) { + + witnessType = input.TaprootAnchorSweepSpend + } + + // Prepare anchor output for sweeping. + anchorInput := input.MakeBaseInput( + &anchor.CommitAnchor, + witnessType, + &anchor.AnchorSignDescriptor, + heightHint, + &input.TxInfo{ + Fee: anchor.CommitFee, + Weight: anchor.CommitWeight, + }, + ) + + // If we have a deadline, we'll use it to calculate the deadline + // height, otherwise default to none. + deadlineDesc := "None" + deadlineHeight := fn.MapOption(func(d int32) int32 { + deadlineDesc = fmt.Sprintf("%d", d) + + return d + int32(heightHint) + })(deadline) + + // Calculate the budget based on the value under protection, which is + // the sum of all HTLCs on this commitment subtracted by their budgets. + // The anchor output in itself has a small output value of 330 sats so + // we also include it in the budget to pay for the cpfp transaction. + budget := calculateBudget( + value, c.cfg.Budget.AnchorCPFPRatio, c.cfg.Budget.AnchorCPFP, + ) + AnchorOutputValue + + log.Infof("ChannelArbitrator(%v): offering anchor from %s commitment "+ + "%v to sweeper with deadline=%v, budget=%v", c.cfg.ChanPoint, + anchorPath, anchor.CommitAnchor, deadlineDesc, budget) + + // Sweep anchor output with a confirmation target fee preference. + // Because this is a cpfp-operation, the anchor will only be attempted + // to sweep when the current fee estimate for the confirmation target + // exceeds the commit fee rate. + return sweepRequest{ + input: &anchorInput, + params: sweep.Params{ + ExclusiveGroup: &exclusiveGroup, + Budget: budget, + DeadlineHeight: deadlineHeight, + }, + }, nil +} + +// prepareAnchorSweeps creates a list of requests to be used by the sweeper for +// all possible commitment versions. +func (c *ChannelArbitrator) prepareAnchorSweeps(heightHint uint32, + anchors *lnwallet.AnchorResolutions) ([]sweepRequest, error) { + + // requests holds all the possible anchor sweep requests. We can have + // up to 3 different versions of commitments (local/remote/remote + // dangling) to be CPFPed by the anchors. + requests := make([]sweepRequest, 0, 3) + + // remotePendingReq holds the request for sweeping the anchor output on + // the remote pending commitment. It's only set when there's an actual + // pending remote commitment and it's used to decide whether we need to + // update the fee budget when sweeping the anchor output on the local + // commitment. + remotePendingReq := fn.None[sweepRequest]() + + // First we check on the remote pending commitment and optionally + // create an anchor sweeping request. + htlcs, ok := c.activeHTLCs[RemotePendingHtlcSet] + if ok && anchors.RemotePending != nil { + req, err := c.createSweepRequest( + anchors.RemotePending, htlcs, "remote pending", + heightHint, + ) + if err != nil { + return nil, err + } + + // Save the request. + requests = append(requests, req) + + // Set the optional variable. + remotePendingReq = fn.Some(req) + } + + // Check the local commitment and optionally create an anchor sweeping + // request. The params used in this request will be influenced by the + // anchor sweeping request made from the pending remote commitment. + htlcs, ok = c.activeHTLCs[LocalHtlcSet] + if ok && anchors.Local != nil { + req, err := c.createSweepRequest( + anchors.Local, htlcs, "local", heightHint, + ) + if err != nil { + return nil, err + } + + // If there's an anchor sweeping request from the pending + // remote commitment, we will compare its budget against the + // budget used here and choose the params that has a larger + // budget. The deadline when choosing the remote pending budget + // instead of the local one will always be earlier or equal to + // the local deadline because outgoing HTLCs are resolved on + // the local commitment first before they are removed from the + // remote one. + remotePendingReq.WhenSome(func(s sweepRequest) { + if s.params.Budget <= req.params.Budget { + return + } + + log.Infof("ChannelArbitrator(%v): replaced local "+ + "anchor(%v) sweep params with pending remote "+ + "anchor sweep params, \nold:[%v], \nnew:[%v]", + c.cfg.ChanPoint, anchors.Local.CommitAnchor, + req.params, s.params) + + req.params = s.params + }) + + // Save the request. + requests = append(requests, req) + } + + // Check the remote commitment and create an anchor sweeping request if + // needed. + htlcs, ok = c.activeHTLCs[RemoteHtlcSet] + if ok && anchors.Remote != nil { + req, err := c.createSweepRequest( + anchors.Remote, htlcs, "remote", heightHint, + ) + if err != nil { + return nil, err + } + + requests = append(requests, req) + } + + return requests, nil +} + // failIncomingDust resolves the incoming dust HTLCs because they do not have // an output on the commitment transaction and cannot be resolved onchain. We // mark them as failed here. diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index ba78139ab..441c36907 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -2256,7 +2256,7 @@ func TestFindCommitmentDeadlineAndValue(t *testing.T) { mockPreimageDB := newMockWitnessBeacon() mockPreimageDB.lookupPreimage[rHash] = rHash - // Attack a mock PreimageDB and Registry to channel arbitrator. + // Attach a mock PreimageDB and Registry to channel arbitrator. chanArb := chanArbCtx.chanArb chanArb.cfg.PreimageDB = mockPreimageDB chanArb.cfg.Registry = &mockRegistry{} @@ -2447,7 +2447,7 @@ func TestSweepAnchors(t *testing.T) { mockPreimageDB := newMockWitnessBeacon() mockPreimageDB.lookupPreimage[rHash] = rHash - // Attack a mock PreimageDB and Registry to channel arbitrator. + // Attach a mock PreimageDB and Registry to channel arbitrator. chanArb := chanArbCtx.chanArb chanArb.cfg.PreimageDB = mockPreimageDB chanArb.cfg.Registry = &mockRegistry{} @@ -2598,6 +2598,116 @@ func TestSweepAnchors(t *testing.T) { ) } +// TestSweepLocalAnchor checks the sweep params used for the local anchor will +// be updated optionally based on the pending remote commit. +func TestSweepLocalAnchor(t *testing.T) { + // Create a testing channel arbitrator. + log := &mockArbitratorLog{ + state: StateDefault, + newStates: make(chan ArbitratorState, 5), + } + chanArbCtx, err := createTestChannelArbitrator(t, log) + require.NoError(t, err, "unable to create ChannelArbitrator") + + // Attach a mock PreimageDB and Registry to channel arbitrator. + chanArb := chanArbCtx.chanArb + mockPreimageDB := newMockWitnessBeacon() + chanArb.cfg.PreimageDB = mockPreimageDB + chanArb.cfg.Registry = &mockRegistry{} + + // Set current block height. + heightHint := uint32(1000) + chanArbCtx.chanArb.blocks <- int32(heightHint) + + htlcIndex := uint64(99) + deadlineDelta := uint32(10) + + htlcAmt := lnwire.MilliSatoshi(1_000_000) + + // Create one testing HTLC. + deadlineSmallDelta := deadlineDelta + 4 + htlcSmallExipry := channeldb.HTLC{ + HtlcIndex: htlcIndex, + RefundTimeout: heightHint + deadlineSmallDelta, + Amt: htlcAmt, + } + + // Setup our local HTLC set such that it doesn't have any HTLCs. We + // expect an anchor sweeping request to be made using the params + // created from sweeping the anchor from the pending remote commit. + chanArb.activeHTLCs[LocalHtlcSet] = htlcSet{} + + // Setup our remote HTLC set such that no valid HTLCs can be used, thus + // the anchor sweeping is skipped. + chanArb.activeHTLCs[RemoteHtlcSet] = htlcSet{} + + // Setup out pending remote HTLC set such that we will use the HTLC's + // CLTV from the outgoing HTLC set. + // Only half of the deadline is used since the anchor cpfp sweep. The + // other half of the deadline is used to sweep the HTLCs at stake. + expectedPendingDeadline := heightHint + deadlineSmallDelta/2 + chanArb.activeHTLCs[RemotePendingHtlcSet] = htlcSet{ + outgoingHTLCs: map[uint64]channeldb.HTLC{ + htlcSmallExipry.HtlcIndex: htlcSmallExipry, + }, + } + + // Mock FindOutgoingHTLCDeadline so the pending remote's outgoing HTLC + // returns the small expiry value. + chanArb.cfg.FindOutgoingHTLCDeadline = func( + htlc channeldb.HTLC) fn.Option[int32] { + + if htlc.RHash != htlcSmallExipry.RHash { + return fn.None[int32]() + } + + return fn.Some(int32(htlcSmallExipry.RefundTimeout)) + } + + // Create AnchorResolutions. + anchors := &lnwallet.AnchorResolutions{ + Local: &lnwallet.AnchorResolution{ + AnchorSignDescriptor: input.SignDescriptor{ + Output: &wire.TxOut{Value: 1}, + }, + }, + Remote: &lnwallet.AnchorResolution{ + AnchorSignDescriptor: input.SignDescriptor{ + Output: &wire.TxOut{Value: 1}, + }, + }, + RemotePending: &lnwallet.AnchorResolution{ + AnchorSignDescriptor: input.SignDescriptor{ + Output: &wire.TxOut{Value: 1}, + }, + }, + } + + // Sweep anchors and check there's no error. + err = chanArb.sweepAnchors(anchors, heightHint) + require.NoError(t, err) + + // Verify deadlines are used as expected. + deadlines := chanArbCtx.sweeper.deadlines + + // We should see two `SweepInput` calls - one for sweeping the local + // anchor, the other from the remote pending anchor. + require.Len(t, deadlines, 2) + + // Both deadlines should be the same since the local anchor uses the + // parameters from the pending remote commitment. + require.EqualValues( + t, expectedPendingDeadline, deadlines[0], + "local deadline not matched, want %v, got %v", + expectedPendingDeadline, deadlines[0], + ) + require.EqualValues( + t, expectedPendingDeadline, deadlines[1], + "pending remote deadline not matched, want %v, got %v", + expectedPendingDeadline, deadlines[1], + ) +} + // TestChannelArbitratorAnchors asserts that the commitment tx anchor is swept. func TestChannelArbitratorAnchors(t *testing.T) { log := &mockArbitratorLog{ @@ -2621,7 +2731,7 @@ func TestChannelArbitratorAnchors(t *testing.T) { mockPreimageDB := newMockWitnessBeacon() mockPreimageDB.lookupPreimage[rHash] = rHash - // Attack a mock PreimageDB and Registry to channel arbitrator. + // Attach a mock PreimageDB and Registry to channel arbitrator. chanArb := chanArbCtx.chanArb chanArb.cfg.PreimageDB = mockPreimageDB chanArb.cfg.Registry = &mockRegistry{}