From 59c40ec8b482122f732af6f0f62871beb518ab05 Mon Sep 17 00:00:00 2001 From: Bjarne Magnussen Date: Tue, 13 Apr 2021 11:37:06 +0200 Subject: [PATCH] chanfunding: fee estimation based on change output in `CoinSelect` Add a dust-limit to `CoinSelect` and let the fee estimation consider if a change output is needed or not, assuring that if the change is below the dust-limit it will go towards the fee instead. --- lnwallet/chanfunding/coin_select.go | 48 ++++++++++++++++----- lnwallet/chanfunding/coin_select_test.go | 54 +++++++++++++++--------- lnwallet/chanfunding/wallet_assembler.go | 3 +- 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/lnwallet/chanfunding/coin_select.go b/lnwallet/chanfunding/coin_select.go index dfb55ec6..74a901ba 100644 --- a/lnwallet/chanfunding/coin_select.go +++ b/lnwallet/chanfunding/coin_select.go @@ -122,7 +122,7 @@ func sanityCheckFee(totalOut, fee btcutil.Amount) error { // change output to fund amt satoshis, adhering to the specified fee rate. The // specified fee rate should be expressed in sat/kw for coin selection to // function properly. -func CoinSelect(feeRate chainfee.SatPerKWeight, amt btcutil.Amount, +func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount, coins []Coin) ([]Coin, btcutil.Amount, error) { amtNeeded := amt @@ -136,29 +136,55 @@ func CoinSelect(feeRate chainfee.SatPerKWeight, amt btcutil.Amount, // Obtain fee estimates both with and without using a change // output. - _, requiredFeeWithChange, err := calculateFees( + requiredFeeNoChange, requiredFeeWithChange, err := calculateFees( selectedUtxos, feeRate, ) if err != nil { return nil, 0, err } + // The difference between the selected amount and the amount // requested will be used to pay fees, and generate a change // output with the remaining. overShootAmt := totalSat - amt - // Based on the estimated size and fee rate, if the excess - // amount isn't enough to pay fees, then increase the requested - // coin amount by the estimate required fee, performing another - // round of coin selection. - if overShootAmt < requiredFeeWithChange { - amtNeeded = amt + requiredFeeWithChange + var changeAmt btcutil.Amount + + switch { + + // If the excess amount isn't enough to pay for fees based on + // fee rate and estimated size without using a change output, + // then increase the requested coin amount by the estimate + // required fee without using change, performing another round + // of coin selection. + case overShootAmt < requiredFeeNoChange: + amtNeeded = amt + requiredFeeNoChange continue + + // If sufficient funds were selected to cover the fee required + // to include a change output, the remainder will be our change + // amount. + case overShootAmt > requiredFeeWithChange: + changeAmt = overShootAmt - requiredFeeWithChange + + // Otherwise we have selected enough to pay for a tx without a + // change output. + default: + changeAmt = 0 + } - // If the fee is sufficient, then calculate the size of the - // change output. - changeAmt := overShootAmt - requiredFeeWithChange + if changeAmt < dustLimit { + changeAmt = 0 + } + + // Sanity check the resulting output values to make sure we + // don't burn a great part to fees. + totalOut := amt + changeAmt + err = sanityCheckFee(totalOut, totalSat-totalOut) + if err != nil { + return nil, 0, err + } return selectedUtxos, changeAmt, nil } diff --git a/lnwallet/chanfunding/coin_select_test.go b/lnwallet/chanfunding/coin_select_test.go index eb000612..83c70dec 100644 --- a/lnwallet/chanfunding/coin_select_test.go +++ b/lnwallet/chanfunding/coin_select_test.go @@ -143,7 +143,7 @@ func TestCoinSelect(t *testing.T) { t.Parallel() const feeRate = chainfee.SatPerKWeight(100) - const dust = btcutil.Amount(100) + const dustLimit = btcutil.Amount(1000) type testCase struct { name string @@ -197,7 +197,7 @@ func TestCoinSelect(t *testing.T) { { // We have a 1 BTC input, and want to create an output // as big as possible, such that the remaining change - // will be dust. + // would be dust but instead goes to fees. name: "dust change", coins: []Coin{ { @@ -208,41 +208,53 @@ func TestCoinSelect(t *testing.T) { }, }, // We tune the output value by subtracting the expected - // fee and a small dust amount. - outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true) - dust, + // fee and the dustlimit. + outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, false) - dustLimit, expectedInput: []btcutil.Amount{ 1 * btcutil.SatoshiPerBitcoin, }, - // Change will the dust. - expectedChange: dust, + // Change must be zero. + expectedChange: 0, }, { - // We have a 1 BTC input, and want to create an output - // as big as possible, such that there is nothing left - // for change. - name: "no change", + // We got just enough funds to create a change output above the + // dust limit. + name: "change right above dustlimit", coins: []Coin{ { TxOut: wire.TxOut{ PkScript: p2wkhScript, - Value: 1 * btcutil.SatoshiPerBitcoin, + Value: int64(fundingFee(feeRate, 1, true) + 2*(dustLimit+1)), }, }, }, - // We tune the output value to be the maximum amount - // possible, leaving just enough for fees. - outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true), + // We tune the output value to be just above the dust limit. + outputValue: dustLimit + 1, expectedInput: []btcutil.Amount{ - 1 * btcutil.SatoshiPerBitcoin, + fundingFee(feeRate, 1, true) + 2*(dustLimit+1), }, - // We have just enough left to pay the fee, so there is - // nothing left for change. - // TODO(halseth): currently coinselect estimates fees - // assuming a change output. - expectedChange: 0, + + // After paying for the fee the change output should be just above + // the dust limit. + expectedChange: dustLimit + 1, + }, + { + // If more than 20% of funds goes to fees, it should fail. + name: "high fee", + coins: []Coin{ + { + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: int64(5 * fundingFee(feeRate, 1, false)), + }, + }, + }, + outputValue: 4 * fundingFee(feeRate, 1, false), + + expectErr: true, }, } @@ -252,7 +264,7 @@ func TestCoinSelect(t *testing.T) { t.Parallel() selected, changeAmt, err := CoinSelect( - feeRate, test.outputValue, test.coins, + feeRate, test.outputValue, dustLimit, test.coins, ) if !test.expectErr && err != nil { t.Fatalf(err.Error()) diff --git a/lnwallet/chanfunding/wallet_assembler.go b/lnwallet/chanfunding/wallet_assembler.go index 9cdf9b10..446e35ac 100644 --- a/lnwallet/chanfunding/wallet_assembler.go +++ b/lnwallet/chanfunding/wallet_assembler.go @@ -289,9 +289,10 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { // Otherwise do a normal coin selection where we target a given // funding amount. default: + dustLimit := w.cfg.DustLimit localContributionAmt = r.LocalAmt selectedCoins, changeAmt, err = CoinSelect( - r.FeeRate, r.LocalAmt, coins, + r.FeeRate, r.LocalAmt, dustLimit, coins, ) if err != nil { return err