From 4c82fb6cbb651cef2122670ca4d64fd1f24b4adb Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Tue, 6 Feb 2024 12:25:49 +0100 Subject: [PATCH] chanfunding: make coin selection generic Before this commit the coin selection logic in the chanfunding package would always assume that there is a P2WSH funding output and potentially a P2TR change output. But because we want to re-use the coin selection for things other than just channel funding, we make the logic more generic by allowing us to specify both the existing weight of the transaction (the already known, static parts of the TX) as well as the type of the potential change output we would use. --- lnwallet/chanfunding/coin_select.go | 61 +++++++++++++++++------- lnwallet/chanfunding/coin_select_test.go | 23 ++++++++- lnwallet/chanfunding/wallet_assembler.go | 14 ++++++ 3 files changed, 79 insertions(+), 19 deletions(-) diff --git a/lnwallet/chanfunding/coin_select.go b/lnwallet/chanfunding/coin_select.go index f4f2e9645..88c2f7419 100644 --- a/lnwallet/chanfunding/coin_select.go +++ b/lnwallet/chanfunding/coin_select.go @@ -38,6 +38,20 @@ func (e *errUnsupportedInput) Error() string { return fmt.Sprintf("unsupported address type: %x", e.PkScript) } +// ChangeAddressType is an enum-like type that describes the type of change +// address that should be generated for a transaction. +type ChangeAddressType uint8 + +const ( + // P2WKHChangeAddress indicates that the change output should be a + // P2WKH output. + P2WKHChangeAddress ChangeAddressType = 0 + + // P2TRChangeAddress indicates that the change output should be a + // P2TR output. + P2TRChangeAddress ChangeAddressType = 1 +) + // selectInputs selects a slice of inputs necessary to meet the specified // selection amount. If input selection is unable to succeed due to insufficient // funds, a non-nil error is returned. Additionally, the total amount of the @@ -69,11 +83,11 @@ func selectInputs(amt btcutil.Amount, coins []wallet.Coin, // calculateFees returns for the specified utxos and fee rate two fee // estimates, one calculated using a change output and one without. The weight // added to the estimator from a change output is for a P2WKH output. -func calculateFees(utxos []wallet.Coin, - feeRate chainfee.SatPerKWeight) (btcutil.Amount, btcutil.Amount, - error) { +func calculateFees(utxos []wallet.Coin, feeRate chainfee.SatPerKWeight, + existingWeight input.TxWeightEstimator, + changeType ChangeAddressType) (btcutil.Amount, btcutil.Amount, error) { - var weightEstimate input.TxWeightEstimator + weightEstimate := existingWeight for _, utxo := range utxos { switch { case txscript.IsPayToWitnessPubKeyHash(utxo.PkScript): @@ -92,17 +106,23 @@ func calculateFees(utxos []wallet.Coin, } } - // Channel funding multisig output is P2WSH. - weightEstimate.AddP2WSHOutput() - // Estimate the fee required for a transaction without a change // output. totalWeight := int64(weightEstimate.Weight()) requiredFeeNoChange := feeRate.FeeForWeight(totalWeight) // Estimate the fee required for a transaction with a change output. - // Assume that change output is a P2TR output. - weightEstimate.AddP2TROutput() + switch changeType { + case P2WKHChangeAddress: + weightEstimate.AddP2WKHOutput() + + case P2TRChangeAddress: + weightEstimate.AddP2TROutput() + + default: + return 0, 0, fmt.Errorf("unknown change address type: %v", + changeType) + } // Now that we have added the change output, redo the fee // estimate. @@ -130,9 +150,9 @@ func sanityCheckFee(totalOut, fee btcutil.Amount) error { // specified fee rate should be expressed in sat/kw for coin selection to // function properly. func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount, - coins []wallet.Coin, - strategy wallet.CoinSelectionStrategy) ([]wallet.Coin, btcutil.Amount, - error) { + coins []wallet.Coin, strategy wallet.CoinSelectionStrategy, + existingWeight input.TxWeightEstimator, + changeType ChangeAddressType) ([]wallet.Coin, btcutil.Amount, error) { amtNeeded := amt for { @@ -148,7 +168,7 @@ func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount, // Obtain fee estimates both with and without using a change // output. requiredFeeNoChange, requiredFeeWithChange, err := calculateFees( - selectedUtxos, feeRate, + selectedUtxos, feeRate, existingWeight, changeType, ) if err != nil { return nil, 0, err @@ -204,7 +224,9 @@ func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount, // coins, the final output and change values are returned. func CoinSelectSubtractFees(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount, coins []wallet.Coin, - strategy wallet.CoinSelectionStrategy) ([]wallet.Coin, btcutil.Amount, + strategy wallet.CoinSelectionStrategy, + existingWeight input.TxWeightEstimator, + changeType ChangeAddressType) ([]wallet.Coin, btcutil.Amount, btcutil.Amount, error) { // First perform an initial round of coin selection to estimate @@ -219,7 +241,7 @@ func CoinSelectSubtractFees(feeRate chainfee.SatPerKWeight, amt, // Obtain fee estimates both with and without using a change // output. requiredFeeNoChange, requiredFeeWithChange, err := calculateFees( - selectedUtxos, feeRate, + selectedUtxos, feeRate, existingWeight, changeType, ) if err != nil { return nil, 0, 0, err @@ -268,7 +290,9 @@ func CoinSelectSubtractFees(feeRate chainfee.SatPerKWeight, amt, // available coins. func CoinSelectUpToAmount(feeRate chainfee.SatPerKWeight, minAmount, maxAmount, reserved, dustLimit btcutil.Amount, coins []wallet.Coin, - strategy wallet.CoinSelectionStrategy) ([]wallet.Coin, btcutil.Amount, + strategy wallet.CoinSelectionStrategy, + existingWeight input.TxWeightEstimator, + changeType ChangeAddressType) ([]wallet.Coin, btcutil.Amount, btcutil.Amount, error) { var ( @@ -289,7 +313,8 @@ func CoinSelectUpToAmount(feeRate chainfee.SatPerKWeight, minAmount, maxAmount, // First we try to select coins to create an output of the specified // maxAmount with or without a change output that covers the miner fee. selected, changeAmt, err := CoinSelect( - feeRate, maxAmount, dustLimit, coins, strategy, + feeRate, maxAmount, dustLimit, coins, strategy, existingWeight, + changeType, ) var errInsufficientFunds *ErrInsufficientFunds @@ -329,7 +354,7 @@ func CoinSelectUpToAmount(feeRate chainfee.SatPerKWeight, minAmount, maxAmount, if selectSubtractFee { selected, outputAmount, changeAmt, err = CoinSelectSubtractFees( feeRate, totalBalance-reserved, dustLimit, coins, - strategy, + strategy, existingWeight, changeType, ) if err != nil { return nil, 0, 0, err diff --git a/lnwallet/chanfunding/coin_select_test.go b/lnwallet/chanfunding/coin_select_test.go index b37b027f7..468acd78e 100644 --- a/lnwallet/chanfunding/coin_select_test.go +++ b/lnwallet/chanfunding/coin_select_test.go @@ -25,6 +25,8 @@ var ( p2khScript, _ = hex.DecodeString( "76a91411034bdcb6ccb7744fdfdeea958a6fb0b415a03288ac", ) + + defaultChanFundingChangeType = P2TRChangeAddress ) // fundingFee is a helper method that returns the fee estimate used for a tx @@ -117,11 +119,15 @@ func TestCalculateFees(t *testing.T) { }, } + fundingOutputEstimate := input.TxWeightEstimator{} + fundingOutputEstimate.AddP2WSHOutput() + for _, test := range testCases { test := test t.Run(test.name, func(t *testing.T) { feeNoChange, feeWithChange, err := calculateFees( - test.utxos, feeRate, + test.utxos, feeRate, fundingOutputEstimate, + defaultChanFundingChangeType, ) require.Equal(t, test.expectedErr, err) @@ -259,6 +265,9 @@ func TestCoinSelect(t *testing.T) { }, } + fundingOutputEstimate := input.TxWeightEstimator{} + fundingOutputEstimate.AddP2WSHOutput() + for _, test := range testCases { test := test t.Run(test.name, func(t *testing.T) { @@ -267,6 +276,8 @@ func TestCoinSelect(t *testing.T) { selected, changeAmt, err := CoinSelect( feeRate, test.outputValue, dustLimit, test.coins, wallet.CoinSelectionLargest, + fundingOutputEstimate, + defaultChanFundingChangeType, ) if !test.expectErr && err != nil { t.Fatalf(err.Error()) @@ -472,6 +483,9 @@ func TestCoinSelectSubtractFees(t *testing.T) { }, } + fundingOutputEstimate := input.TxWeightEstimator{} + fundingOutputEstimate.AddP2WSHOutput() + for _, test := range testCases { test := test @@ -484,6 +498,8 @@ func TestCoinSelectSubtractFees(t *testing.T) { selected, localFundingAmt, changeAmt, err := CoinSelectSubtractFees( feeRate, test.spendValue, dustLimit, test.coins, wallet.CoinSelectionLargest, + fundingOutputEstimate, + defaultChanFundingChangeType, ) if err != nil { switch { @@ -714,6 +730,9 @@ func TestCoinSelectUpToAmount(t *testing.T) { expectedChange: 10000, }} + fundingOutputEstimate := input.TxWeightEstimator{} + fundingOutputEstimate.AddP2WSHOutput() + for _, test := range testCases { test := test @@ -724,6 +743,8 @@ func TestCoinSelectUpToAmount(t *testing.T) { feeRate, test.minValue, test.maxValue, test.reserved, dustLimit, test.coins, wallet.CoinSelectionLargest, + fundingOutputEstimate, + defaultChanFundingChangeType, ) if len(test.expectErr) == 0 && err != nil { t.Fatalf(err.Error()) diff --git a/lnwallet/chanfunding/wallet_assembler.go b/lnwallet/chanfunding/wallet_assembler.go index 3c187d808..651bc5558 100644 --- a/lnwallet/chanfunding/wallet_assembler.go +++ b/lnwallet/chanfunding/wallet_assembler.go @@ -308,6 +308,17 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { } } + // The coin selection algorithm requires to know what + // inputs/outputs are already present in the funding + // transaction and what a change output would look like. Since + // a channel funding is always either a P2WSH or P2TR output, + // we can use just P2WSH here (both of these output types have + // the same length). And we currently don't support specifying a + // change output type, so we always use P2TR. + var fundingOutputWeight input.TxWeightEstimator + fundingOutputWeight.AddP2WSHOutput() + changeType := P2TRChangeAddress + var ( coins []wallet.Coin selectedCoins []wallet.Coin @@ -393,6 +404,7 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { r.FeeRate, r.MinFundAmt, r.FundUpToMaxAmt, reserve, w.cfg.DustLimit, coins, w.cfg.CoinSelectionStrategy, + fundingOutputWeight, changeType, ) if err != nil { return err @@ -427,6 +439,7 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { err = CoinSelectSubtractFees( r.FeeRate, r.LocalAmt, dustLimit, coins, w.cfg.CoinSelectionStrategy, + fundingOutputWeight, changeType, ) if err != nil { return err @@ -440,6 +453,7 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { selectedCoins, changeAmt, err = CoinSelect( r.FeeRate, r.LocalAmt, dustLimit, coins, w.cfg.CoinSelectionStrategy, + fundingOutputWeight, changeType, ) if err != nil { return err