From 59526988cf6b3b16dbc8441f197b34f0a0501b64 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 21 Mar 2024 17:12:17 +0800 Subject: [PATCH] sweep: add a dedicated method to create sweeping txns This takes the old `createSweepTx` and refactors it to be sweep-specific. A sweeping txns differs from a normal tx as it doesn't need to take outputs as params. --- sweep/fee_bumper.go | 245 +++++++++++++++++++++++++++++++++++++- sweep/fee_bumper_test.go | 12 +- sweep/txgenerator.go | 6 +- sweep/weight_estimator.go | 13 ++ 4 files changed, 269 insertions(+), 7 deletions(-) diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index 58a7f8b45..9bd0cdf66 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -9,6 +9,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/rpcclient" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/chain" "github.com/davecgh/go-spew/spew" @@ -28,6 +29,14 @@ var ( // ErrNotEnoughBudget is returned when the fee bumper decides the // current budget cannot cover the fee. ErrNotEnoughBudget = errors.New("not enough budget") + + // ErrLocktimeImmature is returned when sweeping an input whose + // locktime is not reached. + ErrLocktimeImmature = errors.New("immature input") + + // ErrTxNoOutput is returned when an output cannot be created during tx + // preparation, usually due to the output being dust. + ErrTxNoOutput = errors.New("tx has no output") ) // Bumper defines an interface that can be used by other subsystems for fee @@ -458,11 +467,8 @@ func (t *TxPublisher) createAndCheckTx(req *BumpRequest, f FeeFunction) ( // Create the sweep tx with max fee rate of 0 as the fee function // guarantees the fee rate used here won't exceed the max fee rate. - // - // TODO(yy): refactor this function to not require a max fee rate. - tx, fee, err := createSweepTx( - req.Inputs, nil, req.DeliveryAddress, uint32(t.currentHeight), - f.FeeRate(), 0, t.cfg.Signer, + tx, fee, err := t.createSweepTx( + req.Inputs, req.DeliveryAddress, f.FeeRate(), ) if err != nil { return nil, 0, fmt.Errorf("create sweep tx: %w", err) @@ -951,3 +957,232 @@ func calcCurrentConfTarget(currentHeight, deadline int32) uint32 { return confTarget } + +// createSweepTx creates a sweeping tx based on the given inputs, change +// address and fee rate. +func (t *TxPublisher) createSweepTx(inputs []input.Input, changePkScript []byte, + feeRate chainfee.SatPerKWeight) (*wire.MsgTx, btcutil.Amount, error) { + + // Validate and calculate the fee and change amount. + txFee, changeAmtOpt, locktimeOpt, err := prepareSweepTx( + inputs, changePkScript, feeRate, t.currentHeight, + ) + if err != nil { + return nil, 0, err + } + + var ( + // Create the sweep transaction that we will be building. We + // use version 2 as it is required for CSV. + sweepTx = wire.NewMsgTx(2) + + // We'll add the inputs as we go so we know the final ordering + // of inputs to sign. + idxs []input.Input + ) + + // We start by adding all inputs that commit to an output. We do this + // since the input and output index must stay the same for the + // signatures to be valid. + for _, o := range inputs { + if o.RequiredTxOut() == nil { + continue + } + + idxs = append(idxs, o) + sweepTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: *o.OutPoint(), + Sequence: o.BlocksToMaturity(), + }) + sweepTx.AddTxOut(o.RequiredTxOut()) + } + + // Sum up the value contained in the remaining inputs, and add them to + // the sweep transaction. + for _, o := range inputs { + if o.RequiredTxOut() != nil { + continue + } + + idxs = append(idxs, o) + sweepTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: *o.OutPoint(), + Sequence: o.BlocksToMaturity(), + }) + } + + // If there's a change amount, add it to the transaction. + changeAmtOpt.WhenSome(func(changeAmt btcutil.Amount) { + sweepTx.AddTxOut(&wire.TxOut{ + PkScript: changePkScript, + Value: int64(changeAmt), + }) + }) + + // We'll default to using the current block height as locktime, if none + // of the inputs commits to a different locktime. + sweepTx.LockTime = uint32(locktimeOpt.UnwrapOr(t.currentHeight)) + + prevInputFetcher, err := input.MultiPrevOutFetcher(inputs) + if err != nil { + return nil, 0, fmt.Errorf("error creating prev input fetcher "+ + "for hash cache: %v", err) + } + hashCache := txscript.NewTxSigHashes(sweepTx, prevInputFetcher) + + // With all the inputs in place, use each output's unique input script + // function to generate the final witness required for spending. + addInputScript := func(idx int, tso input.Input) error { + inputScript, err := tso.CraftInputScript( + t.cfg.Signer, sweepTx, hashCache, prevInputFetcher, idx, + ) + if err != nil { + return err + } + + sweepTx.TxIn[idx].Witness = inputScript.Witness + + if len(inputScript.SigScript) == 0 { + return nil + } + + sweepTx.TxIn[idx].SignatureScript = inputScript.SigScript + + return nil + } + + for idx, inp := range idxs { + if err := addInputScript(idx, inp); err != nil { + return nil, 0, err + } + } + + log.Debugf("Created sweep tx %v for %v inputs", sweepTx.TxHash(), + len(inputs)) + + return sweepTx, txFee, nil +} + +// prepareSweepTx returns the tx fee, an optional change amount and an optional +// locktime after a series of validations: +// 1. check the locktime has been reached. +// 2. check the locktimes are the same. +// 3. check the inputs cover the outputs. +// +// NOTE: if the change amount is below dust, it will be added to the tx fee. +func prepareSweepTx(inputs []input.Input, changePkScript []byte, + feeRate chainfee.SatPerKWeight, currentHeight int32) ( + btcutil.Amount, fn.Option[btcutil.Amount], fn.Option[int32], error) { + + noChange := fn.None[btcutil.Amount]() + noLocktime := fn.None[int32]() + + // Creating a weight estimator with nil outputs and zero max fee rate. + // We don't allow adding customized outputs in the sweeping tx, and the + // fee rate is already being managed before we get here. + inputs, estimator, err := getWeightEstimate( + inputs, nil, feeRate, 0, changePkScript, + ) + if err != nil { + return 0, noChange, noLocktime, err + } + + txFee := estimator.fee() + + var ( + // Track whether any of the inputs require a certain locktime. + locktime = int32(-1) + + // We keep track of total input amount, and required output + // amount to use for calculating the change amount below. + totalInput btcutil.Amount + requiredOutput btcutil.Amount + ) + + // Go through each input and check if the required lock times have + // reached and are the same. + for _, o := range inputs { + // If the input has a required output, we'll add it to the + // required output amount. + if o.RequiredTxOut() != nil { + requiredOutput += btcutil.Amount( + o.RequiredTxOut().Value, + ) + } + + // Update the total input amount. + totalInput += btcutil.Amount(o.SignDesc().Output.Value) + + lt, ok := o.RequiredLockTime() + + // Skip if the input doesn't require a lock time. + if !ok { + continue + } + + // Check if the lock time has reached + if lt > uint32(currentHeight) { + return 0, noChange, noLocktime, ErrLocktimeImmature + } + + // If another input commits to a different locktime, they + // cannot be combined in the same transaction. + if locktime != -1 && locktime != int32(lt) { + return 0, noChange, noLocktime, ErrLocktimeConflict + } + + // Update the locktime for next iteration. + locktime = int32(lt) + } + + // Make sure total output amount is less than total input amount. + if requiredOutput+txFee > totalInput { + return 0, noChange, noLocktime, fmt.Errorf("insufficient "+ + "input to create sweep tx: input_sum=%v, "+ + "output_sum=%v", totalInput, requiredOutput+txFee) + } + + // The value remaining after the required output and fees is the + // change output. + changeAmt := totalInput - requiredOutput - txFee + changeAmtOpt := fn.Some(changeAmt) + + // We'll calculate the dust limit for the given changePkScript since it + // is variable. + changeFloor := lnwallet.DustLimitForSize(len(changePkScript)) + + // If the change amount is dust, we'll move it into the fees. + if changeAmt < changeFloor { + log.Infof("Change amt %v below dustlimit %v, not adding "+ + "change output", changeAmt, changeFloor) + + // If there's no required output, and the change output is a + // dust, it means we are creating a tx without any outputs. In + // this case we'll return an error. This could happen when + // creating a tx that has an anchor as the only input. + if requiredOutput == 0 { + return 0, noChange, noLocktime, ErrTxNoOutput + } + + // The dust amount is added to the fee. + txFee += changeAmt + + // Set the change amount to none. + changeAmtOpt = fn.None[btcutil.Amount]() + } + + // Optionally set the locktime. + locktimeOpt := fn.Some(locktime) + if locktime == -1 { + locktimeOpt = noLocktime + } + + log.Debugf("Creating sweep tx for %v inputs (%s) using %v, "+ + "tx_weight=%v, tx_fee=%v, locktime=%v, parents_count=%v, "+ + "parents_fee=%v, parents_weight=%v, current_height=%v", + len(inputs), inputTypeSummary(inputs), feeRate, + estimator.weight(), txFee, locktimeOpt, len(estimator.parents), + estimator.parentsFee, estimator.parentsWeight, currentHeight) + + return txFee, changeAmtOpt, locktimeOpt, nil +} diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index 5f031a9bf..fe45910a5 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -1169,7 +1169,11 @@ func TestHandleTxConfirmed(t *testing.T) { // // NOTE: must be called in a goroutine in case it blocks. tp.wg.Add(1) - go tp.handleTxConfirmed(record, requestID) + done := make(chan struct{}) + go func() { + tp.handleTxConfirmed(record, requestID) + close(done) + }() select { case <-time.After(time.Second): @@ -1185,6 +1189,12 @@ func TestHandleTxConfirmed(t *testing.T) { require.Equal(t, feerate, result.FeeRate) } + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("timeout waiting for handleTxConfirmed to return") + } + // We expect the record to be removed from the maps. _, found := tp.records.Load(requestID) require.False(t, found) diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index a06078b44..a6afd92bf 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -273,7 +273,11 @@ func getWeightEstimate(inputs []input.Input, outputs []*wire.TxOut, err := weightEstimate.add(inp) if err != nil { - log.Warn(err) + // TODO(yy): check if this is even possible? If so, we + // should return the error here instead of filtering! + log.Errorf("Failed to get weight estimate for "+ + "input=%v, witnessType=%v: %v ", inp.OutPoint(), + inp.WitnessType(), err) // Skip inputs for which no weight estimate can be // given. diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go index 2a923d5a0..f705c826e 100644 --- a/sweep/weight_estimator.go +++ b/sweep/weight_estimator.go @@ -106,6 +106,19 @@ func (w *weightEstimator) weight() int { return w.estimator.Weight() } +// fee returns the tx fee to use for the aggregated inputs and outputs, which +// is different from feeWithParent as it doesn't take into account unconfirmed +// parent transactions. +func (w *weightEstimator) fee() btcutil.Amount { + // Calculate the weight of the transaction. + weight := int64(w.estimator.Weight()) + + // Calculate the fee. + fee := w.feeRate.FeeForWeight(weight) + + return fee +} + // feeWithParent returns the tx fee to use for the aggregated inputs and // outputs, taking into account unconfirmed parent transactions (cpfp). func (w *weightEstimator) feeWithParent() btcutil.Amount {