From ae08a7541004355c34140b63c8a8abf7f568dc9c Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 31 Jul 2025 05:48:01 +0800 Subject: [PATCH 1/5] contractcourt+sweep: make anchor inputs exclusive We now make sure to sweep each anchor input in its own sweeping tx, if economically feasible. --- contractcourt/anchor_resolver.go | 11 ++++++++++- sweep/sweeper.go | 5 +++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index c59e5d063..7e2676782 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -202,7 +202,10 @@ func (c *anchorResolver) Launch() error { // an output that we want to sweep only if it is economical to do so. // // An exclusive group is not necessary anymore, because we know that - // this is the only anchor that can be swept. + // this is the only anchor that can be swept. However, to avoid this + // anchor input being group with other inputs, we still keep the + // exclusive group here such that the anchor will be swept + // independently. // // We also clear the parent tx information for cpfp, because the // commitment tx is confirmed. @@ -222,6 +225,8 @@ func (c *anchorResolver) Launch() error { c.broadcastHeight, nil, ) + exclusiveGroup := c.ShortChanID.ToUint64() + resultChan, err := c.Sweeper.SweepInput( &anchorInput, sweep.Params{ @@ -233,6 +238,10 @@ func (c *anchorResolver) Launch() error { // There's no rush to sweep the anchor, so we use a nil // deadline here. DeadlineHeight: fn.None[int32](), + + // Use the chan id as the exclusive group. This prevents + // any of the anchors from being batched together. + ExclusiveGroup: &exclusiveGroup, }, ) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index fdaecbefb..1058c9503 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -45,8 +45,9 @@ var ( // Params contains the parameters that control the sweeping process. type Params struct { - // ExclusiveGroup is an identifier that, if set, prevents other inputs - // with the same identifier from being batched together. + // ExclusiveGroup is an identifier that, if set, ensures this input is + // swept in a transaction by itself, and not batched with any other + // inputs. ExclusiveGroup *uint64 // DeadlineHeight specifies an absolute block height that this input From 5ee54b34ce4c3efca1f387d891113be372ced228 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 31 Jul 2025 07:58:24 +0800 Subject: [PATCH 2/5] sweep: only remove the other input from the exclusive group We now make sure when removing inputs identified by the exclusive group ID, we only remove the other one, not the one that invoked the removal. --- sweep/sweeper.go | 59 ++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 1058c9503..2b9bad082 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -740,15 +740,23 @@ func (s *UtxoSweeper) collector() { } } -// removeExclusiveGroup removes all inputs in the given exclusive group. This -// function is called when one of the exclusive group inputs has been spent. The -// other inputs won't ever be spendable and can be removed. This also prevents -// them from being part of future sweep transactions that would fail. In -// addition sweep transactions of those inputs will be removed from the wallet. -func (s *UtxoSweeper) removeExclusiveGroup(group uint64) { +// removeExclusiveGroup removes all inputs in the given exclusive group except +// the input specified by the outpoint. This function is called when one of the +// exclusive group inputs has been spent or updated. The other inputs won't ever +// be spendable and can be removed. This also prevents them from being part of +// future sweep transactions that would fail. In addition sweep transactions of +// those inputs will be removed from the wallet. +func (s *UtxoSweeper) removeExclusiveGroup(group uint64, op wire.OutPoint) { for outpoint, input := range s.inputs { outpoint := outpoint + // Skip the input that caused the exclusive group to be removed. + if outpoint == op { + log.Debugf("Skipped removing exclusive input %v", input) + + continue + } + // Skip inputs that aren't exclusive. if input.params.ExclusiveGroup == nil { continue @@ -767,6 +775,8 @@ func (s *UtxoSweeper) removeExclusiveGroup(group uint64) { continue } + log.Debugf("Removing exclusive group for input %v", input) + // Signal result channels. s.signalResult(input, Result{ Err: ErrExclusiveGroupSpend, @@ -1367,22 +1377,19 @@ func (s *UtxoSweeper) decideRBFInfo( func (s *UtxoSweeper) handleExistingInput(input *sweepInputMessage, oldInput *SweeperInput) { - // Before updating the input details, check if an exclusive group was - // set. In case the same input is registered again without an exclusive - // group set, the previous input and its sweep parameters are outdated - // hence need to be replaced. This scenario currently only happens for - // anchor outputs. When a channel is force closed, in the worst case 3 - // different sweeps with the same exclusive group are registered with - // the sweeper to bump the closing transaction (cpfp) when its time - // critical. Receiving an input which was already registered with the - // sweeper but now without an exclusive group means non of the previous - // inputs were used as CPFP, so we need to make sure we update the - // sweep parameters but also remove all inputs with the same exclusive - // group because the are outdated too. + // Before updating the input details, check if a previous exclusive + // group was set. In case the same input is registered again, the + // previous input and its sweep parameters are outdated hence need to be + // replaced. This scenario currently only happens for anchor outputs. + // When a channel is force closed, in the worst case 3 different sweeps + // with the same exclusive group are registered with the sweeper to bump + // the closing transaction (cpfp) when its time critical. Receiving an + // input which was already registered with the sweeper means none of the + // previous inputs were used as CPFP, so we need to make sure we update + // the sweep parameters but also remove all inputs with the same + // exclusive group because they are outdated too. var prevExclGroup *uint64 - if oldInput.params.ExclusiveGroup != nil && - input.params.ExclusiveGroup == nil { - + if oldInput.params.ExclusiveGroup != nil { prevExclGroup = new(uint64) *prevExclGroup = *oldInput.params.ExclusiveGroup } @@ -1401,7 +1408,7 @@ func (s *UtxoSweeper) handleExistingInput(input *sweepInputMessage, oldInput.listeners = append(oldInput.listeners, input.resultChan) if prevExclGroup != nil { - s.removeExclusiveGroup(*prevExclGroup) + s.removeExclusiveGroup(*prevExclGroup, input.input.OutPoint()) } } @@ -1493,7 +1500,9 @@ func (s *UtxoSweeper) markInputsSwept(tx *wire.MsgTx, isOurTx bool) { // Remove all other inputs in this exclusive group. if input.params.ExclusiveGroup != nil { - s.removeExclusiveGroup(*input.params.ExclusiveGroup) + s.removeExclusiveGroup( + *input.params.ExclusiveGroup, outpoint, + ) } } } @@ -1908,7 +1917,9 @@ func (s *UtxoSweeper) markInputSwept(inp *SweeperInput, tx *wire.MsgTx) { // Remove all other inputs in this exclusive group. if inp.params.ExclusiveGroup != nil { - s.removeExclusiveGroup(*inp.params.ExclusiveGroup) + s.removeExclusiveGroup( + *inp.params.ExclusiveGroup, inp.OutPoint(), + ) } } From 3da4093012aed04d9e077d20cf4b2905b3871971 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 31 Jul 2025 07:59:29 +0800 Subject: [PATCH 3/5] itest: fix tests re the new anchor behavior --- itest/lnd_channel_force_close_test.go | 20 +++++-------- itest/lnd_sweep_test.go | 43 ++++++++++++++++++--------- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/itest/lnd_channel_force_close_test.go b/itest/lnd_channel_force_close_test.go index b05b838ff..3cb6d30e4 100644 --- a/itest/lnd_channel_force_close_test.go +++ b/itest/lnd_channel_force_close_test.go @@ -978,15 +978,6 @@ func runChannelForceClosureTestRestart(ht *lntest.HarnessTest, Outpoint: commitSweep.Outpoint, AmountSat: uint64(aliceBalance), } - op = fmt.Sprintf("%v:%v", anchorSweep.Outpoint.TxidStr, - anchorSweep.Outpoint.OutputIndex) - aliceReports[op] = &lnrpc.Resolution{ - ResolutionType: lnrpc.ResolutionType_ANCHOR, - Outcome: lnrpc.ResolutionOutcome_CLAIMED, - SweepTxid: sweepTxid.String(), - Outpoint: anchorSweep.Outpoint, - AmountSat: uint64(anchorSweep.AmountSat), - } // Check that we can find the commitment sweep in our set of known // sweeps, using the simple transaction id ListSweeps output. @@ -1101,8 +1092,9 @@ func runChannelForceClosureTestRestart(ht *lntest.HarnessTest, // Since Alice had numInvoices (6) htlcs extended to Carol before force // closing, we expect Alice to broadcast an htlc timeout txn for each - // one. - ht.AssertNumPendingSweeps(alice, numInvoices) + // one. In addition, the anchor input is still pending due to it's + // uneconomical to sweep. + ht.AssertNumPendingSweeps(alice, numInvoices+1) // Wait for them all to show up in the mempool htlcTxid := ht.AssertNumTxsInMempool(1)[0] @@ -1198,7 +1190,9 @@ func runChannelForceClosureTestRestart(ht *lntest.HarnessTest, numBlocks := int(htlcCsvMaturityHeight - uint32(curHeight) - 1) ht.MineEmptyBlocks(numBlocks) - ht.AssertNumPendingSweeps(alice, numInvoices) + // We should see numInvoices HTLC sweeps plus the uneconomical anchor + // sweep. + ht.AssertNumPendingSweeps(alice, numInvoices+1) // Fetch the htlc sweep transaction from the mempool. htlcSweepTx := ht.GetNumTxsFromMempool(1)[0] @@ -1220,7 +1214,7 @@ func runChannelForceClosureTestRestart(ht *lntest.HarnessTest, }, defaultTimeout) require.NoError(ht, err, "timeout while checking force closed channel") - ht.AssertNumPendingSweeps(alice, numInvoices) + ht.AssertNumPendingSweeps(alice, numInvoices+1) // Ensure the htlc sweep transaction only has one input for each htlc // Alice extended before force closing. diff --git a/itest/lnd_sweep_test.go b/itest/lnd_sweep_test.go index d0fe5f077..c5bcd3b15 100644 --- a/itest/lnd_sweep_test.go +++ b/itest/lnd_sweep_test.go @@ -1224,8 +1224,8 @@ func testSweepHTLCs(ht *lntest.HarnessTest) { // 4. Alice force closes the channel. // // Test: -// 1. Alice's anchor sweeping is not attempted, instead, it should be swept -// together with her to_local output using the no deadline path. +// 1. Alice's CPFP-anchor sweeping is not attempted, instead, it should be +// swept using the no deadline path and failed due it's not economical. // 2. Bob would also sweep his anchor and to_local outputs separately due to // they have different deadline heights, which means only the to_local // sweeping tx will succeed as the anchor sweeping is not economical. @@ -1241,10 +1241,15 @@ func testSweepCommitOutputAndAnchor(ht *lntest.HarnessTest) { // config. deadline := uint32(1000) - // deadlineA is the deadline used for Alice, since her commit output is - // offered to the sweeper at CSV-1. With a deadline of 1000, her actual - // width of her fee func is CSV+1000-1. Given we are using a CSV of 2 - // here, her fee func deadline then becomes 1001. + // deadlineA is the deadline used for Alice, given that, + // - the force close tx is broadcast at height 445, her inputs are + // registered at the same height, so her to_local and anchor outputs + // have a deadline height of 1445. + // - the force close tx is mined at 446, which means her anchor output + // now has a deadline delta of (1445-446) = 999 blocks. + // - for her to_local output, with a deadline of 1000, the width of the + // fee func is CSV+1000-1. Given we are using a CSV of 2 here, her fee + // func deadline then becomes 1001. deadlineA := deadline + 1 // deadlineB is the deadline used for Bob, the actual deadline used by @@ -1267,6 +1272,11 @@ func testSweepCommitOutputAndAnchor(ht *lntest.HarnessTest) { // conf target is the deadline. ht.SetFeeEstimateWithConf(startFeeRate, deadlineB) + // Set up the starting fee for Alice's anchor sweeping. With this low + // fee rate, her anchor sweeping should be attempted and failed due to + // dust output generated in the sweeping tx. + ht.SetFeeEstimateWithConf(startFeeRate, deadline-1) + // toLocalCSV is the CSV delay for Alice's to_local output. We use a // small value to save us from mining blocks. // @@ -1427,10 +1437,10 @@ func testSweepCommitOutputAndAnchor(ht *lntest.HarnessTest) { // With Alice's starting fee rate being validated, we now calculate her // ending fee rate and fee rate delta. // - // Alice sweeps two inputs - anchor and commit, so the starting budget - // should come from the sum of these two. However, due to the value - // being too large, the actual ending fee rate used should be the - // sweeper's max fee rate configured. + // Alice sweeps the to_local input, so the starting budget should come + // from the to_local balance. However, due to the value being too large, + // the actual ending fee rate used should be the sweeper's max fee rate + // configured. aliceTxWeight := uint64(ht.CalculateTxWeight(aliceSweepTx)) aliceEndingFeeRate := sweep.DefaultMaxFeeRate.FeePerKWeight() aliceFeeRateDelta := (aliceEndingFeeRate - aliceStartingFeeRate) / @@ -1507,10 +1517,10 @@ func testSweepCommitOutputAndAnchor(ht *lntest.HarnessTest) { } // We should see two txns in the mempool: - // - Alice's sweeping tx, which sweeps both her anchor and - // commit outputs, using the increased fee rate. - // - Bob's previous sweeping tx, which sweeps both his anchor - // and commit outputs, at the possible increased fee rate. + // - Alice's sweeping tx, which sweeps her commit output, using + // the increased fee rate. + // - Bob's previous sweeping tx, which sweeps his commit output, + // at the possible increased fee rate. txns := ht.GetNumTxsFromMempool(2) // Assume the first tx is Alice's sweeping tx, if the second tx @@ -1577,6 +1587,11 @@ func testSweepCommitOutputAndAnchor(ht *lntest.HarnessTest) { // Mine a block to confirm both sweeping txns, this is needed to clean // up the mempool. ht.MineBlocksAndAssertNumTxes(1, 2) + + // Finally, assert that both Alice and Bob still have the anchor + // outputs, which cannot be swept due to it being uneconomical. + ht.AssertNumPendingSweeps(alice, 1) + ht.AssertNumPendingSweeps(bob, 1) } // testBumpForceCloseFee tests that when a force close transaction, in From ab8e0354ab398769491a0863c23f32b8d3a2c71e Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 8 Jul 2025 23:38:35 +0300 Subject: [PATCH 4/5] sweep: fix typos --- sweep/fee_bumper.go | 12 ++++++------ sweep/fee_bumper_test.go | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index 1602165ea..18d848b21 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -111,8 +111,8 @@ const ( // error, which means they cannot be retried with increased budget. TxFatal - // sentinalEvent is used to check if an event is unknown. - sentinalEvent + // sentinelEvent is used to check if an event is unknown. + sentinelEvent ) // String returns a human-readable string for the event. @@ -137,13 +137,13 @@ func (e BumpEvent) String() string { // Unknown returns true if the event is unknown. func (e BumpEvent) Unknown() bool { - return e >= sentinalEvent + return e >= sentinelEvent } // BumpRequest is used by the caller to give the Bumper the necessary info to // create and manage potential fee bumps for a set of inputs. type BumpRequest struct { - // Budget givens the total amount that can be used as fees by these + // Budget gives the total amount that can be used as fees by these // inputs. Budget btcutil.Amount @@ -589,7 +589,7 @@ func (t *TxPublisher) createRBFCompliantTx( // used up the budget, we will return an error // indicating this tx cannot be made. The // sweeper should handle this error and try to - // cluster these inputs differetly. + // cluster these inputs differently. increased, err = f.Increment() if err != nil { return nil, err @@ -1332,7 +1332,7 @@ func (t *TxPublisher) createAndPublishTx( // the fee bumper retry it at next block. // // NOTE: we may get this error if we've bypassed the mempool check, - // which means we are suing neutrino backend. + // which means we are using neutrino backend. if errors.Is(result.Err, chain.ErrInsufficientFee) || errors.Is(result.Err, lnwallet.ErrMempoolFee) { diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index aa5561b5a..fb939e732 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -73,7 +73,7 @@ func TestBumpResultValidate(t *testing.T) { // Unknown event type will give an error. b = BumpResult{ Tx: &wire.MsgTx{}, - Event: sentinalEvent, + Event: sentinelEvent, } require.ErrorIs(t, b.Validate(), ErrInvalidBumpResult) @@ -457,7 +457,7 @@ func TestCreateAndCheckTx(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -550,7 +550,7 @@ func TestCreateRBFCompliantTx(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -1120,9 +1120,9 @@ func TestBroadcastImmediate(t *testing.T) { require.Empty(t, tp.subscriberChans.Len()) } -// TestCreateAnPublishFail checks all the error cases are handled properly in -// the method createAndPublish. -func TestCreateAnPublishFail(t *testing.T) { +// TestCreateAndPublishFail checks all the error cases are handled properly in +// the method createAndPublishTx. +func TestCreateAndPublishFail(t *testing.T) { t.Parallel() // Create a publisher using the mocks. @@ -1152,7 +1152,7 @@ func TestCreateAnPublishFail(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -1190,9 +1190,9 @@ func TestCreateAnPublishFail(t *testing.T) { require.True(t, resultOpt.IsNone()) } -// TestCreateAnPublishSuccess checks the expected result is returned from the -// method createAndPublish. -func TestCreateAnPublishSuccess(t *testing.T) { +// TestCreateAndPublishSuccess checks the expected result is returned from the +// method createAndPublishTx. +func TestCreateAndPublishSuccess(t *testing.T) { t.Parallel() // Create a publisher using the mocks. @@ -1218,7 +1218,7 @@ func TestCreateAnPublishSuccess(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -1445,7 +1445,7 @@ func TestHandleFeeBumpTx(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -1830,7 +1830,7 @@ func TestHandleInitialBroadcastSuccess(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) @@ -1916,7 +1916,7 @@ func TestHandleInitialBroadcastFail(t *testing.T) { // // NOTE: we are not testing the utility of creating valid txes here, so // this is fine to be mocked. This behaves essentially as skipping the - // Signer check and alaways assume the tx has a valid sig. + // Signer check and always assume the tx has a valid sig. script := &input.Script{} m.signer.On("ComputeInputScript", mock.Anything, mock.Anything).Return(script, nil) From 4d7622ab05b1f01d5062ae60ea8465b85eb28f41 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 31 Jul 2025 09:40:22 +0800 Subject: [PATCH 5/5] docs: add and update release notes for 0.19.3 --- docs/release-notes/release-notes-0.19.3.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/release-notes-0.19.3.md b/docs/release-notes/release-notes-0.19.3.md index 24d60fec6..e282b2bbc 100644 --- a/docs/release-notes/release-notes-0.19.3.md +++ b/docs/release-notes/release-notes-0.19.3.md @@ -29,6 +29,13 @@ ## Functional Enhancements +- Previously, when sweeping non-time sensitive anchor outputs, they might be + grouped with other non-time sensitive outputs such as `to_local` outputs, + which potentially allow the sweeping tx to be pinned. This is now + [fixed](https://github.com/lightningnetwork/lnd/pull/10117) by moving sweeping + anchors into its own tx, which means the anchor outputs won't be swept in a + high fee environment. + ## RPC Additions ## lncli Additions @@ -62,4 +69,6 @@ ## Tooling and Documentation # Contributors (Alphabetical Order) -* Olaoluwa Osuntokun \ No newline at end of file + +* Olaoluwa Osuntokun +* Yong Yu