From 3e3d8487fb7e06ef0a36a680baa54faa309cda29 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 4 Sep 2020 14:03:14 +0200 Subject: [PATCH 1/3] sweep: wrap weight estimator Preparation for a cpfp-aware weight estimator. For cpfp, a regular weight estimator isn't sufficient, because it needs to take into account the weight of the parent transaction(s) as well. --- sweep/tx_input_set.go | 34 ++++++++++++++++----------------- sweep/txgenerator.go | 11 +++++------ sweep/weight_estimator.go | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 sweep/weight_estimator.go diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index d2ce6e5a4..47d18ce49 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -33,7 +33,7 @@ const ( type txInputSetState struct { // weightEstimate is the (worst case) tx weight with the current set of // inputs. - weightEstimate input.TxWeightEstimator + weightEstimate *weightEstimator // inputTotal is the total value of all inputs. inputTotal btcutil.Amount @@ -54,7 +54,7 @@ type txInputSetState struct { func (t *txInputSetState) clone() txInputSetState { s := txInputSetState{ - weightEstimate: t.weightEstimate, + weightEstimate: t.weightEstimate.clone(), inputTotal: t.inputTotal, outputValue: t.outputValue, walletInputTotal: t.walletInputTotal, @@ -95,15 +95,20 @@ func newTxInputSet(wallet Wallet, feePerKW, btcutil.Amount(relayFee.FeePerKVByte()), ) + state := txInputSetState{ + weightEstimate: newWeightEstimator(), + } + b := txInputSet{ - feePerKW: feePerKW, - dustLimit: dustLimit, - maxInputs: maxInputs, - wallet: wallet, + feePerKW: feePerKW, + dustLimit: dustLimit, + maxInputs: maxInputs, + wallet: wallet, + txInputSetState: state, } // Add the sweep tx output to the weight estimate. - b.weightEstimate.AddP2WKHOutput() + b.weightEstimate.addP2WKHOutput() return &b } @@ -126,29 +131,22 @@ func (t *txInputSet) addToState(inp input.Input, constraints addConstraints) *tx return nil } - // Can ignore error, because it has already been checked when - // calculating the yields. - size, isNestedP2SH, _ := inp.WitnessType().SizeUpperBound() - // Clone the current set state. s := t.clone() // Add the new input. s.inputs = append(s.inputs, inp) - // Add weight of the new input. - if isNestedP2SH { - s.weightEstimate.AddNestedP2WSHInput(size) - } else { - s.weightEstimate.AddWitnessInput(size) - } + // Can ignore error, because it has already been checked when + // calculating the yields. + _ = s.weightEstimate.add(inp) // Add the value of the new input. value := btcutil.Amount(inp.SignDesc().Output.Value) s.inputTotal += value // Recalculate the tx fee. - weight := s.weightEstimate.Weight() + weight := s.weightEstimate.weight() fee := t.feePerKW.FeeForWeight(int64(weight)) // Calculate the new output value. diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index 1e47dad21..3b98239db 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -120,7 +120,7 @@ func generateInputPartitionings(sweepableInputs []txInput, "has yield=%v, weight=%v", inputCount, len(txInputs.inputs)-inputCount, txInputs.outputValue-txInputs.walletInputTotal, - txInputs.weightEstimate.Weight()) + txInputs.weightEstimate.weight()) sets = append(sets, txInputs.inputs) sweepableInputs = sweepableInputs[inputCount:] @@ -222,11 +222,11 @@ func getWeightEstimate(inputs []input.Input) ([]input.Input, int64) { // // TODO(roasbeef): can be more intelligent about buffering outputs to // be more efficient on-chain. - var weightEstimate input.TxWeightEstimator + weightEstimate := newWeightEstimator() // Our sweep transaction will pay to a single segwit p2wkh address, // ensure it contributes to our weight estimate. - weightEstimate.AddP2WKHOutput() + weightEstimate.addP2WKHOutput() // For each output, use its witness type to determine the estimate // weight of its witness, and add it to the proper set of spendable @@ -235,8 +235,7 @@ func getWeightEstimate(inputs []input.Input) ([]input.Input, int64) { for i := range inputs { inp := inputs[i] - wt := inp.WitnessType() - err := wt.AddWeightEstimation(&weightEstimate) + err := weightEstimate.add(inp) if err != nil { log.Warn(err) @@ -248,7 +247,7 @@ func getWeightEstimate(inputs []input.Input) ([]input.Input, int64) { sweepInputs = append(sweepInputs, inp) } - return sweepInputs, int64(weightEstimate.Weight()) + return sweepInputs, int64(weightEstimate.weight()) } // inputSummary returns a string containing a human readable summary about the diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go new file mode 100644 index 000000000..def29e7a7 --- /dev/null +++ b/sweep/weight_estimator.go @@ -0,0 +1,40 @@ +package sweep + +import ( + "github.com/lightningnetwork/lnd/input" +) + +// weightEstimator wraps a standard weight estimator instance. +type weightEstimator struct { + estimator input.TxWeightEstimator +} + +// newWeightEstimator instantiates a new sweeper weight estimator. +func newWeightEstimator() *weightEstimator { + return &weightEstimator{} +} + +// clone returns a copy of this weight estimator. +func (w *weightEstimator) clone() *weightEstimator { + return &weightEstimator{ + estimator: w.estimator, + } +} + +// add adds the weight of the given input to the weight estimate. +func (w *weightEstimator) add(inp input.Input) error { + wt := inp.WitnessType() + + return wt.AddWeightEstimation(&w.estimator) +} + +// addP2WKHOutput updates the weight estimate to account for an additional +// native P2WKH output. +func (w *weightEstimator) addP2WKHOutput() { + w.estimator.AddP2WKHOutput() +} + +// weight gets the estimated weight of the transaction. +func (w *weightEstimator) weight() int { + return w.estimator.Weight() +} From 681496b474552e2dd1e269d92ac689b171d88062 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 4 Sep 2020 11:28:17 +0200 Subject: [PATCH 2/3] sweep: make sweeper aware of unconfirmed parent transactions. Extend the fee estimator to take into account parent transactions with their weights and fees. Do not try to cpfp parent transactions that have a higher fee rate than the sweep tx fee rate. --- breacharbiter.go | 5 ++ contractcourt/anchor_resolver.go | 1 + contractcourt/channel_arbitrator.go | 1 + input/input.go | 37 ++++++++++-- sweep/sweeper.go | 25 +++++++- sweep/sweeper_test.go | 87 +++++++++++++++++++++++++++- sweep/test_utils.go | 1 + sweep/tx_input_set.go | 9 +-- sweep/txgenerator.go | 22 ++++--- sweep/txgenerator_test.go | 3 +- sweep/walletsweep.go | 4 +- sweep/weight_estimator.go | 90 +++++++++++++++++++++++++++-- sweep/weight_estimator_test.go | 79 +++++++++++++++++++++++++ 13 files changed, 334 insertions(+), 30 deletions(-) create mode 100644 sweep/weight_estimator_test.go diff --git a/breacharbiter.go b/breacharbiter.go index 865e67ce0..b504d1b83 100644 --- a/breacharbiter.go +++ b/breacharbiter.go @@ -914,6 +914,11 @@ func (bo *breachedOutput) HeightHint() uint32 { return bo.confHeight } +// UnconfParent returns information about a possibly unconfirmed parent tx. +func (bo *breachedOutput) UnconfParent() *input.TxInfo { + return nil +} + // Add compile-time constraint ensuring breachedOutput implements the Input // interface. var _ input.Input = (*breachedOutput)(nil) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index bc9b251d8..ac67173fc 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -99,6 +99,7 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { input.CommitmentAnchor, &c.anchorSignDescriptor, c.broadcastHeight, + nil, ) resultChan, err := c.Sweeper.SweepInput( diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 33857c9ff..46bb61ba1 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -1073,6 +1073,7 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, input.CommitmentAnchor, &anchor.AnchorSignDescriptor, heightHint, + nil, ) // Sweep anchor output with the minimum fee rate. This usually diff --git a/input/input.go b/input/input.go index 38a1651e5..2e3a71c0b 100644 --- a/input/input.go +++ b/input/input.go @@ -3,6 +3,7 @@ package input import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" ) // Input represents an abstract UTXO which is to be spent using a sweeping @@ -41,6 +42,19 @@ type Input interface { // HeightHint returns the minimum height at which a confirmed spending // tx can occur. HeightHint() uint32 + + // UnconfParent returns information about a possibly unconfirmed parent + // tx. + UnconfParent() *TxInfo +} + +// TxInfo describes properties of a parent tx that are relevant for CPFP. +type TxInfo struct { + // Fee is the fee of the tx. + Fee btcutil.Amount + + // Weight is the weight of the tx. + Weight int64 } type inputKit struct { @@ -49,6 +63,10 @@ type inputKit struct { signDesc SignDescriptor heightHint uint32 blockToMaturity uint32 + + // unconfParent contains information about a potential unconfirmed + // parent transaction. + unconfParent *TxInfo } // OutPoint returns the breached output's identifier that is to be included as @@ -82,6 +100,11 @@ func (i *inputKit) BlocksToMaturity() uint32 { return i.blockToMaturity } +// Cpfp returns information about a possibly unconfirmed parent tx. +func (i *inputKit) UnconfParent() *TxInfo { + return i.unconfParent +} + // BaseInput contains all the information needed to sweep a basic output // (CSV/CLTV/no time lock) type BaseInput struct { @@ -91,14 +114,16 @@ type BaseInput struct { // MakeBaseInput assembles a new BaseInput that can be used to construct a // sweep transaction. func MakeBaseInput(outpoint *wire.OutPoint, witnessType WitnessType, - signDescriptor *SignDescriptor, heightHint uint32) BaseInput { + signDescriptor *SignDescriptor, heightHint uint32, + unconfParent *TxInfo) BaseInput { return BaseInput{ inputKit{ - outpoint: *outpoint, - witnessType: witnessType, - signDesc: *signDescriptor, - heightHint: heightHint, + outpoint: *outpoint, + witnessType: witnessType, + signDesc: *signDescriptor, + heightHint: heightHint, + unconfParent: unconfParent, }, } } @@ -109,7 +134,7 @@ func NewBaseInput(outpoint *wire.OutPoint, witnessType WitnessType, signDescriptor *SignDescriptor, heightHint uint32) *BaseInput { input := MakeBaseInput( - outpoint, witnessType, signDescriptor, heightHint, + outpoint, witnessType, signDescriptor, heightHint, nil, ) return &input diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 68698f5cb..501ed0784 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -505,8 +505,11 @@ func (s *UtxoSweeper) collector(blockEpochs <-chan *chainntnfs.BlockEpoch) { log.Debugf("Already pending input %v received", outpoint) - // Update sweep parameters. + // Update input details and sweep parameters. + // The re-offered input details may contain a + // change to the unconfirmed parent tx info. pendInput.params = input.params + pendInput.Input = input.input // Add additional result channel to signal // spend of this input. @@ -763,6 +766,26 @@ func (s *UtxoSweeper) clusterBySweepFeeRate() []inputCluster { log.Warnf("Skipping input %v: %v", op, err) continue } + + // Only try to sweep inputs with an unconfirmed parent if the + // current sweep fee rate exceeds the parent tx fee rate. This + // assumes that such inputs are offered to the sweeper solely + // for the purpose of anchoring down the parent tx using cpfp. + parentTx := input.UnconfParent() + if parentTx != nil { + parentFeeRate := + chainfee.SatPerKWeight(parentTx.Fee*1000) / + chainfee.SatPerKWeight(parentTx.Weight) + + if parentFeeRate >= feeRate { + log.Debugf("Skipping cpfp input %v: fee_rate=%v, "+ + "parent_fee_rate=%v", op, feeRate, + parentFeeRate) + + continue + } + } + feeGroup := s.bucketForFeeRate(feeRate) // Create a bucket list for this fee rate if there isn't one diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index d7b6ab131..2f71c35bd 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -17,6 +17,7 @@ import ( "github.com/lightningnetwork/lnd/lntest/mock" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" ) var ( @@ -77,6 +78,7 @@ func createTestInput(value int64, witnessType input.WitnessType) input.BaseInput }, }, 0, + nil, ) testInputCount++ @@ -176,6 +178,18 @@ func (ctx *sweeperTestContext) tick() { } } +// assertNoTick asserts that the sweeper does not wait for a tick. +func (ctx *sweeperTestContext) assertNoTick() { + ctx.t.Helper() + + select { + case <-ctx.timeoutChan: + ctx.t.Fatal("unexpected tick") + + case <-time.After(processingDelay): + } +} + func (ctx *sweeperTestContext) assertNoNewTimer() { select { case <-ctx.timeoutChan: @@ -337,9 +351,10 @@ func assertTxFeeRate(t *testing.T, tx *wire.MsgTx, outputAmt := tx.TxOut[0].Value fee := btcutil.Amount(inputAmt - outputAmt) - _, txWeight := getWeightEstimate(inputs) + _, estimator := getWeightEstimate(inputs, 0) + txWeight := estimator.weight() - expectedFee := expectedFeeRate.FeeForWeight(txWeight) + expectedFee := expectedFeeRate.FeeForWeight(int64(txWeight)) if fee != expectedFee { t.Fatalf("expected fee rate %v results in %v fee, got %v fee", expectedFeeRate, expectedFee, fee) @@ -1293,3 +1308,71 @@ func TestExclusiveGroup(t *testing.T) { t.Fatal("expected third input to be canceled") } } + +// TestCpfp tests that the sweeper spends cpfp inputs at a fee rate that exceeds +// the parent tx fee rate. +func TestCpfp(t *testing.T) { + ctx := createSweeperTestContext(t) + + ctx.estimator.updateFees(1000, chainfee.FeePerKwFloor) + + // Offer an input with an unconfirmed parent tx to the sweeper. The + // parent tx pays 3000 sat/kw. + hash := chainhash.Hash{1} + input := input.MakeBaseInput( + &wire.OutPoint{Hash: hash}, + input.CommitmentTimeLock, + &input.SignDescriptor{ + Output: &wire.TxOut{ + Value: 330, + }, + KeyDesc: keychain.KeyDescriptor{ + PubKey: testPubKey, + }, + }, + 0, + &input.TxInfo{ + Weight: 300, + Fee: 900, + }, + ) + + feePref := FeePreference{ConfTarget: 6} + result, err := ctx.sweeper.SweepInput( + &input, Params{Fee: feePref, Force: true}, + ) + require.NoError(t, err) + + // Because we sweep at 1000 sat/kw, the parent cannot be paid for. We + // expect the sweeper to remain idle. + ctx.assertNoTick() + + // Increase the fee estimate to above the parent tx fee rate. + ctx.estimator.updateFees(5000, chainfee.FeePerKwFloor) + + // Signal a new block. This is a trigger for the sweeper to refresh fee + // estimates. + ctx.notifier.NotifyEpoch(1000) + + // Now we do expect a sweep transaction to be published with our input + // and an attached wallet utxo. + ctx.tick() + tx := ctx.receiveTx() + require.Len(t, tx.TxIn, 2) + require.Len(t, tx.TxOut, 1) + + // As inputs we have 10000 sats from the wallet and 330 sats from the + // cpfp input. The sweep tx is weight expected to be 759 units. There is + // an additional 300 weight units from the parent to include in the + // package, making a total of 1059. At 5000 sat/kw, the required fee for + // the package is 5295 sats. The parent already paid 900 sats, so there + // is 4395 sat remaining to be paid. The expected output value is + // therefore: 10000 + 330 - 4395 = 5935. + require.Equal(t, int64(5935), tx.TxOut[0].Value) + + // Mine the tx and assert that the result is passed back. + ctx.backend.mine() + ctx.expectResult(result, nil) + + ctx.finish(1) +} diff --git a/sweep/test_utils.go b/sweep/test_utils.go index 2fd916c8e..5e0c67626 100644 --- a/sweep/test_utils.go +++ b/sweep/test_utils.go @@ -13,6 +13,7 @@ import ( var ( defaultTestTimeout = 5 * time.Second + processingDelay = 1 * time.Second mockChainHash, _ = chainhash.NewHashFromStr("00aabbccddeeff") mockChainHeight = int32(100) ) diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 47d18ce49..708369006 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -71,9 +71,6 @@ func (t *txInputSetState) clone() txInputSetState { type txInputSet struct { txInputSetState - // feePerKW is the fee rate used to calculate the tx fee. - feePerKW chainfee.SatPerKWeight - // dustLimit is the minimum output value of the tx. dustLimit btcutil.Amount @@ -96,11 +93,10 @@ func newTxInputSet(wallet Wallet, feePerKW, ) state := txInputSetState{ - weightEstimate: newWeightEstimator(), + weightEstimate: newWeightEstimator(feePerKW), } b := txInputSet{ - feePerKW: feePerKW, dustLimit: dustLimit, maxInputs: maxInputs, wallet: wallet, @@ -146,8 +142,7 @@ func (t *txInputSet) addToState(inp input.Input, constraints addConstraints) *tx s.inputTotal += value // Recalculate the tx fee. - weight := s.weightEstimate.weight() - fee := t.feePerKW.FeeForWeight(int64(weight)) + fee := s.weightEstimate.fee() // Calculate the new output value. s.outputValue = s.inputTotal - fee diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index 3b98239db..f023c3178 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -134,9 +134,9 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte, currentBlockHeight uint32, feePerKw chainfee.SatPerKWeight, signer input.Signer) (*wire.MsgTx, error) { - inputs, txWeight := getWeightEstimate(inputs) + inputs, estimator := getWeightEstimate(inputs, feePerKw) - txFee := feePerKw.FeeForWeight(txWeight) + txFee := estimator.fee() // Sum up the total value contained in the inputs. var totalSum btcutil.Amount @@ -208,21 +208,29 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte, } log.Infof("Creating sweep transaction %v for %v inputs (%s) "+ - "using %v sat/kw, tx_fee=%v", sweepTx.TxHash(), len(inputs), - inputTypeSummary(inputs), int64(feePerKw), txFee) + "using %v sat/kw, tx_weight=%v, tx_fee=%v, parents_count=%v, "+ + "parents_fee=%v, parents_weight=%v", + sweepTx.TxHash(), len(inputs), + inputTypeSummary(inputs), int64(feePerKw), + estimator.weight(), txFee, + len(estimator.parents), estimator.parentsFee, + estimator.parentsWeight, + ) return sweepTx, nil } // getWeightEstimate returns a weight estimate for the given inputs. // Additionally, it returns counts for the number of csv and cltv inputs. -func getWeightEstimate(inputs []input.Input) ([]input.Input, int64) { +func getWeightEstimate(inputs []input.Input, feeRate chainfee.SatPerKWeight) ( + []input.Input, *weightEstimator) { + // We initialize a weight estimator so we can accurately asses the // amount of fees we need to pay for this sweep transaction. // // TODO(roasbeef): can be more intelligent about buffering outputs to // be more efficient on-chain. - weightEstimate := newWeightEstimator() + weightEstimate := newWeightEstimator(feeRate) // Our sweep transaction will pay to a single segwit p2wkh address, // ensure it contributes to our weight estimate. @@ -247,7 +255,7 @@ func getWeightEstimate(inputs []input.Input) ([]input.Input, int64) { sweepInputs = append(sweepInputs, inp) } - return sweepInputs, int64(weightEstimate.weight()) + return sweepInputs, weightEstimate } // inputSummary returns a string containing a human readable summary about the diff --git a/sweep/txgenerator_test.go b/sweep/txgenerator_test.go index 575ae65aa..377ac24f9 100644 --- a/sweep/txgenerator_test.go +++ b/sweep/txgenerator_test.go @@ -39,7 +39,8 @@ func TestWeightEstimate(t *testing.T) { )) } - _, weight := getWeightEstimate(inputs) + _, estimator := getWeightEstimate(inputs, 0) + weight := int64(estimator.weight()) if weight != expectedWeight { t.Fatalf("unexpected weight. expected %d but got %d.", expectedWeight, weight) diff --git a/sweep/walletsweep.go b/sweep/walletsweep.go index fb25ccf3a..aa6d658d7 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -255,7 +255,9 @@ func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, blockHeight uint32, // Now that we've constructed the items required, we'll make an // input which can be passed to the sweeper for ultimate // sweeping. - input := input.MakeBaseInput(&output.OutPoint, witnessType, signDesc, 0) + input := input.MakeBaseInput( + &output.OutPoint, witnessType, signDesc, 0, nil, + ) inputsToSweep = append(inputsToSweep, &input) } diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go index def29e7a7..5a4068158 100644 --- a/sweep/weight_estimator.go +++ b/sweep/weight_estimator.go @@ -1,33 +1,91 @@ package sweep import ( + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" ) -// weightEstimator wraps a standard weight estimator instance. +// weightEstimator wraps a standard weight estimator instance and adds to that +// support for child-pays-for-parent. type weightEstimator struct { - estimator input.TxWeightEstimator + estimator input.TxWeightEstimator + feeRate chainfee.SatPerKWeight + parents map[chainhash.Hash]struct{} + parentsFee btcutil.Amount + parentsWeight int64 } // newWeightEstimator instantiates a new sweeper weight estimator. -func newWeightEstimator() *weightEstimator { - return &weightEstimator{} +func newWeightEstimator(feeRate chainfee.SatPerKWeight) *weightEstimator { + return &weightEstimator{ + feeRate: feeRate, + parents: make(map[chainhash.Hash]struct{}), + } } // clone returns a copy of this weight estimator. func (w *weightEstimator) clone() *weightEstimator { + parents := make(map[chainhash.Hash]struct{}, len(w.parents)) + for hash := range w.parents { + parents[hash] = struct{}{} + } + return &weightEstimator{ - estimator: w.estimator, + estimator: w.estimator, + feeRate: w.feeRate, + parents: parents, + parentsFee: w.parentsFee, + parentsWeight: w.parentsWeight, } } // add adds the weight of the given input to the weight estimate. func (w *weightEstimator) add(inp input.Input) error { + // If there is a parent tx, add the parent's fee and weight. + w.tryAddParent(inp) + wt := inp.WitnessType() return wt.AddWeightEstimation(&w.estimator) } +// tryAddParent examines the input and updates parent tx totals if required for +// cpfp. +func (w *weightEstimator) tryAddParent(inp input.Input) { + // Get unconfirmed parent info from the input. + unconfParent := inp.UnconfParent() + + // If there is no parent, there is nothing to add. + if unconfParent == nil { + return + } + + // If we've already accounted for the parent tx, don't do it + // again. This can happens when two outputs of the parent tx are + // included in the same sweep tx. + parentHash := inp.OutPoint().Hash + if _, ok := w.parents[parentHash]; ok { + return + } + + // Calculate parent fee rate. + parentFeeRate := chainfee.SatPerKWeight(unconfParent.Fee) * 1000 / + chainfee.SatPerKWeight(unconfParent.Weight) + + // Ignore parents that pay at least the fee rate of this transaction. + // Parent pays for child is not happening. + if parentFeeRate >= w.feeRate { + return + } + + // Include parent. + w.parents[parentHash] = struct{}{} + w.parentsFee += unconfParent.Fee + w.parentsWeight += unconfParent.Weight +} + // addP2WKHOutput updates the weight estimate to account for an additional // native P2WKH output. func (w *weightEstimator) addP2WKHOutput() { @@ -38,3 +96,25 @@ func (w *weightEstimator) addP2WKHOutput() { func (w *weightEstimator) weight() int { return w.estimator.Weight() } + +// fee returns the tx fee to use for the aggregated inputs and outputs, taking +// into account unconfirmed parent transactions (cpfp). +func (w *weightEstimator) fee() btcutil.Amount { + // Calculate fee and weight for just this tx. + childWeight := int64(w.estimator.Weight()) + + // Add combined weight of unconfirmed parent txes. + totalWeight := childWeight + w.parentsWeight + + // Subtract fee already paid by parents. + fee := w.feeRate.FeeForWeight(totalWeight) - w.parentsFee + + // Clamp the fee to what would be required if no parent txes were paid + // for. This is to make sure no rounding errors can get us into trouble. + childFee := w.feeRate.FeeForWeight(childWeight) + if childFee > fee { + fee = childFee + } + + return fee +} diff --git a/sweep/weight_estimator_test.go b/sweep/weight_estimator_test.go new file mode 100644 index 000000000..e4f1d489b --- /dev/null +++ b/sweep/weight_estimator_test.go @@ -0,0 +1,79 @@ +package sweep + +import ( + "testing" + + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" +) + +// TestWeightEstimator tests weight estimation for inputs with and without +// unconfirmed parents. +func TestWeightEstimator(t *testing.T) { + testFeeRate := chainfee.SatPerKWeight(20000) + + w := newWeightEstimator(testFeeRate) + + // Add an input without unconfirmed parent tx. + input1 := input.MakeBaseInput( + &wire.OutPoint{}, input.CommitmentAnchor, + &input.SignDescriptor{}, 0, nil, + ) + + require.NoError(t, w.add(&input1)) + + // The expectations is that this input is added. + const expectedWeight1 = 322 + require.Equal(t, expectedWeight1, w.weight()) + require.Equal(t, testFeeRate.FeeForWeight(expectedWeight1), w.fee()) + + // Define a parent transaction that pays a fee of 30000 sat/kw. + parentTxHighFee := &input.TxInfo{ + Weight: 100, + Fee: 3000, + } + + // Add an output of the parent tx above. + input2 := input.MakeBaseInput( + &wire.OutPoint{}, input.CommitmentAnchor, + &input.SignDescriptor{}, 0, + parentTxHighFee, + ) + + require.NoError(t, w.add(&input2)) + + // Pay for parent isn't possible because the parent pays a higher fee + // rate than the child. We expect no additional fee on the child. + const expectedWeight2 = expectedWeight1 + 280 + require.Equal(t, expectedWeight2, w.weight()) + require.Equal(t, testFeeRate.FeeForWeight(expectedWeight2), w.fee()) + + // Define a parent transaction that pays a fee of 10000 sat/kw. + parentTxLowFee := &input.TxInfo{ + Weight: 100, + Fee: 1000, + } + + // Add an output of the low-fee parent tx above. + input3 := input.MakeBaseInput( + &wire.OutPoint{}, input.CommitmentAnchor, + &input.SignDescriptor{}, 0, + parentTxLowFee, + ) + require.NoError(t, w.add(&input3)) + + // Expect the weight to increase because of the third input. + const expectedWeight3 = expectedWeight2 + 280 + require.Equal(t, expectedWeight3, w.weight()) + + // Expect the fee to cover the child and the parent transaction at 20 + // sat/kw after subtraction of the fee that was already paid by the + // parent. + expectedFee := testFeeRate.FeeForWeight( + expectedWeight3+parentTxLowFee.Weight, + ) - parentTxLowFee.Fee + + require.Equal(t, expectedFee, w.fee()) +} From 29602e88e8114f28338c1f3bef908dab62e4fec1 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 4 Sep 2020 11:19:27 +0200 Subject: [PATCH 3/3] cnct: cpfp-sweep anchors For unconfirmed commit tx anchors, supply the sweeper with cpfp info and a confirmation target fee estimate. The sweeper will try to pay for the parent commit tx as long as the current fee estimate exceeds the pre-signed commit tx fee rate. --- contractcourt/anchor_resolver.go | 3 + contractcourt/channel_arbitrator.go | 29 +++++--- lntest/itest/lnd_channel_backup_test.go | 4 ++ ...d_multi-hop_htlc_local_chain_claim_test.go | 4 ++ .../lnd_multi-hop_htlc_local_timeout_test.go | 4 ++ ...ulti-hop_htlc_receiver_chain_claim_test.go | 4 ++ ..._multi-hop_htlc_remote_chain_claim_test.go | 4 ++ ..._force_close_on_chain_htlc_timeout_test.go | 4 ++ ..._force_close_on_chain_htlc_timeout_test.go | 4 ++ lntest/itest/lnd_multi-hop_test.go | 4 ++ lntest/itest/lnd_test.go | 67 +++++++++++++++++-- lnwallet/channel.go | 21 ++++++ 12 files changed, 136 insertions(+), 16 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index ac67173fc..98a4fccae 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -89,6 +89,9 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { // An exclusive group is not necessary anymore, because we know that // this is the only anchor that can be swept. // + // We also clear the parent tx information for cpfp, because the + // commitment tx is confirmed. + // // After a restart or when the remote force closes, the sweeper is not // yet aware of the anchor. In that case, it will be added as new input // to the sweeper. diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 46bb61ba1..2d50ca1de 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -30,6 +30,12 @@ var ( "process of being force closed") ) +const ( + // anchorSweepConfTarget is the conf target used when sweeping + // commitment anchors. + anchorSweepConfTarget = 6 +) + // WitnessSubscription represents an intent to be notified once new witnesses // are discovered by various active contract resolvers. A contract resolver may // use this to be notified of when it can satisfy an incoming contract after we @@ -1060,9 +1066,6 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, // anchors from being batched together. exclusiveGroup := c.cfg.ShortChanID.ToUint64() - // Retrieve the current minimum fee rate from the sweeper. - minFeeRate := c.cfg.Sweeper.RelayFeePerKW() - for _, anchor := range anchors { log.Debugf("ChannelArbitrator(%v): pre-confirmation sweep of "+ "anchor of tx %v", c.cfg.ChanPoint, anchor.CommitAnchor) @@ -1073,19 +1076,25 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, input.CommitmentAnchor, &anchor.AnchorSignDescriptor, heightHint, - nil, + &input.TxInfo{ + Fee: anchor.CommitFee, + Weight: anchor.CommitWeight, + }, ) - // Sweep anchor output with the minimum fee rate. This usually - // (up to a min relay fee of 3 sat/b) means that the anchor - // sweep will be economical. Also signal that this is a force - // sweep. If the user decides to bump the fee on the anchor - // sweep, it will be swept even if it isn't economical. + // 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. + // + // Also signal that this is a force sweep, so that the anchor + // will be swept even if it isn't economical purely based on the + // anchor value. _, err := c.cfg.Sweeper.SweepInput( &anchorInput, sweep.Params{ Fee: sweep.FeePreference{ - FeeRate: minFeeRate, + ConfTarget: anchorSweepConfTarget, }, Force: true, ExclusiveGroup: &exclusiveGroup, diff --git a/lntest/itest/lnd_channel_backup_test.go b/lntest/itest/lnd_channel_backup_test.go index 9a7914637..a55228cc4 100644 --- a/lntest/itest/lnd_channel_backup_test.go +++ b/lntest/itest/lnd_channel_backup_test.go @@ -368,6 +368,10 @@ func testChannelBackupRestore(net *lntest.NetworkHarness, t *harnessTest) { for _, testCase := range testCases { success := t.t.Run(testCase.name, func(t *testing.T) { h := newHarnessTest(t, net) + + // Start each test with the default static fee estimate. + net.SetFeeEstimate(12500) + testChanRestoreScenario(h, net, &testCase, password) }) if !success { diff --git a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go index 0bab9b41b..256a8afe1 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go @@ -96,6 +96,10 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest, // hop logic. waitForInvoiceAccepted(t, carol, payHash) + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // At this point, Bob decides that he wants to exit the channel // immediately, so he force closes his commitment transaction. ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) diff --git a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go index 03320f08c..67b13b568 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go @@ -105,6 +105,10 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest, t.Fatalf("htlc mismatch: %v", predErr) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // We'll now mine enough blocks to trigger Bob's broadcast of his // commitment transaction due to the fact that the HTLC is about to // timeout. With the default outgoing broadcast delta of zero, this will diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go index b157f4f0a..a19e51d18 100644 --- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go @@ -117,6 +117,10 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest, t.Fatalf("settle invoice: %v", err) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Now we'll mine enough blocks to prompt carol to actually go to the // chain in order to sweep her HTLC since the value is high enough. // TODO(roasbeef): modify once go to chain policy changes diff --git a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go index 5ea1c4a1a..7e3fe616a 100644 --- a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go @@ -96,6 +96,10 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest // hop logic. waitForInvoiceAccepted(t, carol, payHash) + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Next, Alice decides that she wants to exit the channel, so she'll // immediately force close the channel by broadcast her commitment // transaction. diff --git a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go index 576295ec0..a2a972697 100644 --- a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go @@ -79,6 +79,10 @@ func testMultiHopLocalForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, t.Fatalf("htlc mismatch: %v", err) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Now that all parties have the HTLC locked in, we'll immediately // force close the Bob -> Carol channel. This should trigger contract // resolution mode for both of them. diff --git a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go index 9d30a2dc3..90fc24ebb 100644 --- a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go @@ -80,6 +80,10 @@ func testMultiHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, t.Fatalf("htlc mismatch: %v", predErr) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // At this point, we'll now instruct Carol to force close the // transaction. This will let us exercise that Bob is able to sweep the // expired HTLC on Carol's version of the commitment transaction. If diff --git a/lntest/itest/lnd_multi-hop_test.go b/lntest/itest/lnd_multi-hop_test.go index ec73e1877..100a58425 100644 --- a/lntest/itest/lnd_multi-hop_test.go +++ b/lntest/itest/lnd_multi-hop_test.go @@ -101,6 +101,10 @@ func testMultiHopHtlcClaims(net *lntest.NetworkHarness, t *harnessTest) { success := ht.t.Run(subTest.name, func(t *testing.T) { ht := newHarnessTest(t, net) + // Start each test with the default + // static fee estimate. + net.SetFeeEstimate(12500) + subTest.test(net, ht, alice, bob, commitType) }) if !success { diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index b5a532a85..010b7c434 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -3460,6 +3461,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, numInvoices = 6 ) + const commitFeeRate = 20000 + net.SetFeeEstimate(commitFeeRate) + // TODO(roasbeef): should check default value in config here // instead, or make delay a param defaultCLTV := uint32(lnd.DefaultBitcoinTimeLockDelta) @@ -3574,6 +3578,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // execute a force closure of the channel. This will also assert that // the commitment transaction was immediately broadcast in order to // fulfill the force closure request. + const actualFeeRate = 30000 + net.SetFeeEstimate(actualFeeRate) + ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) _, closingTxID, err := net.CloseChannel(ctxt, alice, chanPoint, true) if err != nil { @@ -3635,17 +3642,35 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // broadcast as a result of the force closure. If there are anchors, we // also expect the anchor sweep tx to be in the mempool. expectedTxes := 1 + expectedFeeRate := commitFeeRate if channelType == commitTypeAnchors { expectedTxes = 2 + expectedFeeRate = actualFeeRate } - sweepTxns, err := waitForNTxsInMempool( + sweepTxns, err := getNTxsFromMempool( net.Miner.Node, expectedTxes, minerMempoolTimeout, ) if err != nil { t.Fatalf("failed to find commitment in miner mempool: %v", err) } + // Verify fee rate of the commitment tx plus anchor if present. + var totalWeight, totalFee int64 + for _, tx := range sweepTxns { + utx := btcutil.NewTx(tx) + totalWeight += blockchain.GetTransactionWeight(utx) + + fee, err := getTxFee(net.Miner.Node, tx) + require.NoError(t.t, err) + totalFee += int64(fee) + } + feeRate := totalFee * 1000 / totalWeight + + // Allow some deviation because weight estimates during tx generation + // are estimates. + require.InEpsilon(t.t, expectedFeeRate, feeRate, 0.005) + // Find alice's commit sweep and anchor sweep (if present) in the // mempool. aliceCloseTx := waitingClose.Commitments.LocalTxid @@ -3737,7 +3762,7 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // Carol's sweep tx should be in the mempool already, as her output is // not timelocked. If there are anchors, we also expect Carol's anchor // sweep now. - sweepTxns, err = waitForNTxsInMempool( + sweepTxns, err = getNTxsFromMempool( net.Miner.Node, expectedTxes, minerMempoolTimeout, ) if err != nil { @@ -4416,12 +4441,13 @@ type sweptOutput struct { // we have to bring another input to add fees to the anchor. Note that the // anchor swept output may be nil if the channel did not have anchors. func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, - sweepTxns []*chainhash.Hash, closeTx string) (*sweptOutput, *sweptOutput) { + sweepTxns []*wire.MsgTx, closeTx string) (*sweptOutput, *sweptOutput) { var commitSweep, anchorSweep *sweptOutput for _, tx := range sweepTxns { - sweepTx, err := net.Miner.Node.GetRawTransaction(tx) + txHash := tx.TxHash() + sweepTx, err := net.Miner.Node.GetRawTransaction(&txHash) require.NoError(t.t, err) // We expect our commitment sweep to have a single input, and, @@ -4434,7 +4460,7 @@ func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, if len(inputs) == 1 { commitSweep = &sweptOutput{ OutPoint: inputs[0].PreviousOutPoint, - SweepTx: tx.String(), + SweepTx: txHash.String(), } } else { // Since we have more than one input, we run through @@ -4445,7 +4471,7 @@ func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, if outpointStr == closeTx { anchorSweep = &sweptOutput{ OutPoint: txin.PreviousOutPoint, - SweepTx: tx.String(), + SweepTx: txHash.String(), } } } @@ -7574,6 +7600,28 @@ func getNTxsFromMempool(miner *rpcclient.Client, n int, return txes, nil } +// getTxFee retrieves parent transactions and reconstructs the fee paid. +func getTxFee(miner *rpcclient.Client, tx *wire.MsgTx) (btcutil.Amount, error) { + var balance btcutil.Amount + for _, in := range tx.TxIn { + parentHash := in.PreviousOutPoint.Hash + rawTx, err := miner.GetRawTransaction(&parentHash) + if err != nil { + return 0, err + } + parent := rawTx.MsgTx() + balance += btcutil.Amount( + parent.TxOut[in.PreviousOutPoint.Index].Value, + ) + } + + for _, out := range tx.TxOut { + balance -= btcutil.Amount(out.Value) + } + + return balance, nil +} + // testFailingChannel tests that we will fail the channel by force closing ii // in the case where a counterparty tries to settle an HTLC with the wrong // preimage. @@ -9423,6 +9471,10 @@ func assertDLPExecuted(net *lntest.NetworkHarness, t *harnessTest, dave *lntest.HarnessNode, daveStartingBalance int64, anchors bool) { + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // We disabled auto-reconnect for some tests to avoid timing issues. // To make sure the nodes are initiating DLP now, we have to manually // re-connect them. @@ -14527,6 +14579,9 @@ func TestLightningNetworkDaemon(t *testing.T) { t.Fatalf("unable to add to log: %v", err) } + // Start every test with the default static fee estimate. + lndHarness.SetFeeEstimate(12500) + success := t.Run(testCase.name, func(t1 *testing.T) { ht := newHarnessTest(t1, lndHarness) ht.RunTestCase(testCase) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index ccdf55308..fdfda736d 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5961,6 +5961,12 @@ type AnchorResolution struct { // CommitAnchor is the anchor outpoint on the commit tx. CommitAnchor wire.OutPoint + + // CommitFee is the fee of the commit tx. + CommitFee btcutil.Amount + + // CommitWeight is the weight of the commit tx. + CommitWeight int64 } // LocalForceCloseSummary describes the final commitment state before the @@ -6399,9 +6405,24 @@ func NewAnchorResolution(chanState *channeldb.OpenChannel, HashType: txscript.SigHashAll, } + // Calculate commit tx weight. This commit tx doesn't yet include the + // witness spending the funding output, so we add the (worst case) + // weight for that too. + utx := btcutil.NewTx(commitTx) + weight := blockchain.GetTransactionWeight(utx) + + input.WitnessCommitmentTxWeight + + // Calculate commit tx fee. + fee := chanState.Capacity + for _, out := range commitTx.TxOut { + fee -= btcutil.Amount(out.Value) + } + return &AnchorResolution{ CommitAnchor: *outPoint, AnchorSignDescriptor: *signDesc, + CommitWeight: weight, + CommitFee: fee, }, nil }