From d65789950c320b6abf8655cb45b3e0ba4a3e9145 Mon Sep 17 00:00:00 2001 From: Bjarne Magnussen Date: Fri, 23 Apr 2021 14:30:11 +0200 Subject: [PATCH] chanfunding: adds ability to fund up to some maximum amount Allows to define a maximum amount to provision a channel opening with using a new field `FundUpToMaxAmt` on the `Request` struct. Also adds a new coin select function `CoinSelectUpToAmount` to select coins up to a maximum amount respecting a minimum amount. --- lnwallet/chanfunding/assembler.go | 21 ++- lnwallet/chanfunding/canned_assembler.go | 12 +- lnwallet/chanfunding/coin_select.go | 77 +++++++++ lnwallet/chanfunding/coin_select_test.go | 206 ++++++++++++++++++++++- lnwallet/chanfunding/psbt_assembler.go | 10 +- lnwallet/chanfunding/wallet_assembler.go | 51 +++++- 6 files changed, 370 insertions(+), 7 deletions(-) diff --git a/lnwallet/chanfunding/assembler.go b/lnwallet/chanfunding/assembler.go index 4e6e62d25..34ea5c611 100644 --- a/lnwallet/chanfunding/assembler.go +++ b/lnwallet/chanfunding/assembler.go @@ -52,13 +52,32 @@ type OutpointLocker interface { // Assembler. type Request struct { // LocalAmt is the amount of coins we're placing into the funding - // output. + // output. LocalAmt must not be set if FundUpToMaxAmt is set. LocalAmt btcutil.Amount // RemoteAmt is the amount of coins the remote party is contributing to // the funding output. RemoteAmt btcutil.Amount + // FundUpToMaxAmt should be set to a non-zero amount if the channel + // funding should try to add as many funds to LocalAmt as possible + // until at most this amount is reached. + FundUpToMaxAmt btcutil.Amount + + // MinFundAmt should be set iff the FundUpToMaxAmt field is set. It + // either carries the configured minimum channel capacity or, if an + // initial remote balance is specified, enough to cover the initial + // remote balance. + MinFundAmt btcutil.Amount + + // RemoteChanReserve is the channel reserve we required for the remote + // peer. + RemoteChanReserve btcutil.Amount + + // PushAmt is the number of satoshis that should be pushed over the + // responder as part of the initial channel creation. + PushAmt btcutil.Amount + // MinConfs controls how many confirmations a coin need to be eligible // to be used as an input to the funding transaction. If this value is // set to zero, then zero conf outputs may be spent. diff --git a/lnwallet/chanfunding/canned_assembler.go b/lnwallet/chanfunding/canned_assembler.go index 603d90fe2..cf33a0cd4 100644 --- a/lnwallet/chanfunding/canned_assembler.go +++ b/lnwallet/chanfunding/canned_assembler.go @@ -195,13 +195,21 @@ func NewCannedAssembler(thawHeight uint32, chanPoint wire.OutPoint, // // NOTE: This method satisfies the chanfunding.Assembler interface. func (c *CannedAssembler) ProvisionChannel(req *Request) (Intent, error) { - // We'll exit out if this field is set as the funding transaction has - // already been assembled, so we don't influence coin selection.. + // We'll exit out if SubtractFees is set as the funding transaction has + // already been assembled, so we don't influence coin selection. if req.SubtractFees { return nil, fmt.Errorf("SubtractFees ignored, funding " + "transaction is frozen") } + // We'll exit out if FundUpToMaxAmt or MinFundAmt is set as the funding + // transaction has already been assembled, so we don't influence coin + // selection. + if req.FundUpToMaxAmt != 0 || req.MinFundAmt != 0 { + return nil, fmt.Errorf("FundUpToMaxAmt and MinFundAmt " + + "ignored, funding transaction is frozen") + } + intent := &ShimIntent{ localKey: c.localKey, remoteKey: c.remoteKey, diff --git a/lnwallet/chanfunding/coin_select.go b/lnwallet/chanfunding/coin_select.go index 049d94cdd..de2eb5be6 100644 --- a/lnwallet/chanfunding/coin_select.go +++ b/lnwallet/chanfunding/coin_select.go @@ -1,6 +1,7 @@ package chanfunding import ( + "errors" "fmt" "github.com/btcsuite/btcd/btcutil" @@ -53,6 +54,7 @@ type Coin struct { // selected coins are returned in order for the caller to properly handle // change+fees. func selectInputs(amt btcutil.Amount, coins []Coin) (btcutil.Amount, []Coin, error) { + satSelected := btcutil.Amount(0) for i, coin := range coins { satSelected += btcutil.Amount(coin.Value) @@ -251,3 +253,78 @@ func CoinSelectSubtractFees(feeRate chainfee.SatPerKWeight, amt, return selectedUtxos, outputAmt, changeAmt, nil } + +// CoinSelectUpToAmount attempts to select coins such that we'll select up to +// maxAmount exclusive of fees if sufficient funds are available. If +// insufficient funds are available this method selects all available coins. +func CoinSelectUpToAmount(feeRate chainfee.SatPerKWeight, minAmount, maxAmount, + dustLimit btcutil.Amount, coins []Coin) ([]Coin, btcutil.Amount, + btcutil.Amount, error) { + + var ( + // selectSubtractFee is tracking if our coin selection was + // unsuccessful and whether we have to start a new round of + // selecting coins considering fees. + selectSubtractFee = false + outputAmount = 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, + ) + + var errInsufficientFunds *ErrInsufficientFunds + if errors.As(err, &errInsufficientFunds) { + // If the initial coin selection fails due to insufficient funds + // we select our total available balance minus fees. + selectSubtractFee = true + } else if err != nil { + return nil, 0, 0, err + } + + // If we determined that our local balance is insufficient we check our + // total balance minus fees. + if selectSubtractFee { + // Get balance from coins. + var totalBalance btcutil.Amount + for _, coin := range coins { + totalBalance += btcutil.Amount(coin.Value) + } + + selected, outputAmount, changeAmt, err = CoinSelectSubtractFees( + feeRate, totalBalance, dustLimit, coins, + ) + if err != nil { + return nil, 0, 0, err + } + } + + // Sanity check the resulting output values to make sure we don't burn a + // great part to fees. + totalOut := outputAmount + changeAmt + sum := func(coins []Coin) btcutil.Amount { + var sum btcutil.Amount + for _, coin := range coins { + sum += btcutil.Amount(coin.Value) + } + + return sum + } + err = sanityCheckFee(totalOut, sum(selected)-totalOut) + if err != nil { + return nil, 0, 0, err + } + + // In case the selected amount is lower than minimum funding amount we + // must return an error. The minimum funding amount is determined + // upstream and denotes either the minimum viable channel size or an + // amount sufficient to cover for the initial remote balance. + if outputAmount < minAmount { + return nil, 0, 0, fmt.Errorf("available funds(%v) below the "+ + "minimum amount(%v)", outputAmount, minAmount) + } + + return selected, outputAmount, changeAmt, nil +} diff --git a/lnwallet/chanfunding/coin_select_test.go b/lnwallet/chanfunding/coin_select_test.go index d243cd358..8ff89887d 100644 --- a/lnwallet/chanfunding/coin_select_test.go +++ b/lnwallet/chanfunding/coin_select_test.go @@ -518,15 +518,219 @@ func TestCoinSelectSubtractFees(t *testing.T) { } } - // Assert we got the expected change amount. + // Assert we got the expected funding amount. if localFundingAmt != test.expectedFundingAmt { t.Fatalf("expected %v local funding amt, got %v", test.expectedFundingAmt, localFundingAmt) } + + // Assert we got the expected change amount. + require.EqualValues( + t, test.expectedChange, changeAmt, + ) + }) + } +} + +// TestCoinSelectUpToAmount tests that we pick coins adding up to the +// expected amount when creating a funding transaction, and that a change +// output is created only when necessary. +func TestCoinSelectUpToAmount(t *testing.T) { + t.Parallel() + + const ( + feeRate = chainfee.SatPerKWeight(100) + dustLimit = btcutil.Amount(1000) + dust = btcutil.Amount(100) + coin = btcutil.SatoshiPerBitcoin + minValue = 20_000 + ) + + type testCase struct { + name string + minValue btcutil.Amount + maxValue btcutil.Amount + coins []Coin + + expectedInput []btcutil.Amount + expectedFundingAmt btcutil.Amount + expectedChange btcutil.Amount + expectErr string + } + + testCases := []testCase{{ + // We have 1.0 BTC available, spend them all. + // This should lead to a funding TX with one output, the rest + // goes to fees. + name: "spend exactly all", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: 1 * coin, + }, + }}, + minValue: minValue, + maxValue: 1 * coin, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{1 * coin}, + expectedFundingAmt: 1*coin - fundingFee(feeRate, 1, false), + expectedChange: 0, + }, { + // We have 1.0 BTC available and want to spend up to 2 BTC. + // This should lead to a funding TX with one output, the rest + // goes to fees. + name: "spend more", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: 1 * coin, + }, + }}, + minValue: minValue, + maxValue: 2 * coin, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{1 * coin}, + expectedFundingAmt: 1*coin - fundingFee(feeRate, 1, false), + expectedChange: 0, + }, { + // We have 1.0 BTC available and want to spend up to 0.5 BTC. + // This should lead to a funding TX with one output and a + // change to subtract the fees from. + name: "spend far below", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: 1 * coin, + }, + }}, + minValue: minValue, + maxValue: 0.5 * coin, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{1 * coin}, + expectedFundingAmt: 0.5 * coin, + expectedChange: 0.5*coin - fundingFee(feeRate, 1, true), + }, { + // We have 1.0 BTC available and want to spend just 1 Satoshi + // below that amount. + // This should lead to a funding TX with one output where the + // fee is subtracted from the total 1 BTC input value. + name: "spend little below", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: 1 * coin, + }, + }}, + minValue: minValue, + maxValue: 1*coin - 1, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{ + 1 * coin, + }, + expectedFundingAmt: 1*coin - fundingFee(feeRate, 1, false), + expectedChange: 0, + }, { + // The total funds available is below the dust limit after + // paying fees. + name: "dust output", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: int64( + fundingFee(feeRate, 1, false) + dust, + ), + }, + }}, + minValue: minValue, + maxValue: fundingFee(feeRate, 1, false) + dust, + + expectErr: "output amount(0.000001 BTC) after subtracting " + + "fees(0.00000048 BTC) below dust limit(0.00001 BTC)", + }, { + // If more than 20% of available wallet funds goes to fees, it + // should fail. + name: "high fee", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: int64( + 20 * fundingFee(feeRate, 1, false), + ), + }, + }}, + minValue: minValue, + maxValue: 16 * fundingFee(feeRate, 1, false), + + expectErr: "fee 0.00000192 BTC on total output value " + + "0.00000768 BTC", + }, { + // This test makes sure that the implementation detail of using + // CoinSelect and CoinSelectSubtractFees is done correctly. + // CoinSelect always defaults to use a fee for single input - + // one change tx, whereas CoinSelectSubtractFees will use a fee + // of single input - no change output, which without a sanity + // check could result in a local amount higher than the maximum + // amount that was expected. + name: "sanity check for correct maximum amount", + coins: []Coin{{ + TxOut: wire.TxOut{ + PkScript: p2wkhScript, + Value: 1 * coin, + }, + }}, + minValue: minValue, + maxValue: 1*coin - fundingFee(feeRate, 1, false) - 1, + + expectedInput: []btcutil.Amount{1 * coin}, + expectedFundingAmt: 1*coin - fundingFee(feeRate, 1, false) - 1, + expectedChange: 0, + }} + + for _, test := range testCases { + test := test + + t.Run(test.name, func(t *testing.T) { + t.Parallel() + selected, localFundingAmt, changeAmt, + err := CoinSelectUpToAmount( + feeRate, test.minValue, test.maxValue, + dustLimit, test.coins, + ) + if len(test.expectErr) == 0 && err != nil { + t.Fatalf(err.Error()) + } if changeAmt != test.expectedChange { t.Fatalf("expected %v change amt, got %v", test.expectedChange, changeAmt) } + if len(test.expectErr) > 0 && err == nil { + t.Fatalf("expected error: %v", test.expectErr) + } + if len(test.expectErr) > 0 && err != nil { + require.EqualError(t, err, test.expectErr) + } + + // Check that the selected inputs match what we expect. + require.Equal(t, len(test.expectedInput), len(selected)) + + for i, coin := range selected { + require.EqualValues( + t, test.expectedInput[i], coin.Value, + ) + } + + // Assert we got the expected funding amount. + require.Equal( + t, test.expectedFundingAmt, localFundingAmt, + ) + // Assert we got the expected change amount. + require.Equal( + t, test.expectedChange, changeAmt, + ) }) } } diff --git a/lnwallet/chanfunding/psbt_assembler.go b/lnwallet/chanfunding/psbt_assembler.go index 8632b175d..f13319516 100644 --- a/lnwallet/chanfunding/psbt_assembler.go +++ b/lnwallet/chanfunding/psbt_assembler.go @@ -514,12 +514,20 @@ func NewPsbtAssembler(fundingAmt btcutil.Amount, basePsbt *psbt.Packet, // // NOTE: This method satisfies the chanfunding.Assembler interface. func (p *PsbtAssembler) ProvisionChannel(req *Request) (Intent, error) { - // We'll exit out if this field is set as the funding transaction will + // We'll exit out if SubtractFees is set as the funding transaction will // be assembled externally, so we don't influence coin selection. if req.SubtractFees { return nil, fmt.Errorf("SubtractFees not supported for PSBT") } + // We'll exit out if FundUpToMaxAmt or MinFundAmt is set as the funding + // transaction will be assembled externally, so we don't influence coin + // selection. + if req.FundUpToMaxAmt != 0 || req.MinFundAmt != 0 { + return nil, fmt.Errorf("FundUpToMaxAmt and MinFundAmt not " + + "supported for PSBT") + } + intent := &PsbtIntent{ ShimIntent: ShimIntent{ localFundingAmt: p.fundingAmt, diff --git a/lnwallet/chanfunding/wallet_assembler.go b/lnwallet/chanfunding/wallet_assembler.go index c81c1cb53..5321dbc9d 100644 --- a/lnwallet/chanfunding/wallet_assembler.go +++ b/lnwallet/chanfunding/wallet_assembler.go @@ -284,16 +284,63 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { // If there's no funding amount at all (receiving an inbound // single funder request), then we don't need to perform any // coin selection at all. - case r.LocalAmt == 0: + case r.LocalAmt == 0 && r.FundUpToMaxAmt == 0: break + // The local funding amount cannot be used in combination with + // the funding up to some maximum amount. If that is the case + // we return an error. + case r.LocalAmt != 0 && r.FundUpToMaxAmt != 0: + return fmt.Errorf("cannot use a local funding amount " + + "and fundmax parameters") + + // We cannot use the subtract fees flag while using the funding + // up to some maximum amount. If that is the case we return an + // error. + case r.SubtractFees && r.FundUpToMaxAmt != 0: + return fmt.Errorf("cannot subtract fees from local " + + "amount while using fundmax parameters") + + // In case this request uses funding up to some maximum amount, + // we will call the specialized coin selection function for + // that. + case r.FundUpToMaxAmt != 0 && r.MinFundAmt != 0: + selectedCoins, localContributionAmt, changeAmt, + err = CoinSelectUpToAmount( + r.FeeRate, r.MinFundAmt, r.FundUpToMaxAmt, + w.cfg.DustLimit, coins, + ) + if err != nil { + return err + } + + // Now where the actual channel capacity is determined + // we can check for local contribution constraints. + // + // Ensure that the remote channel reserve does not + // exceed 20% of the channel capacity. + if r.RemoteChanReserve >= localContributionAmt/5 { + return fmt.Errorf("remote channel reserve " + + "must be less than the %%20 of the " + + "channel capacity") + } + // Ensure that the initial remote balance does not + // exceed our local contribution as that would leave a + // negative balance on our side. + if r.PushAmt >= localContributionAmt { + return fmt.Errorf("amount pushed to remote " + + "peer for initial state must be " + + "below the local funding amount") + } + // In case this request want the fees subtracted from the local // amount, we'll call the specialized method for that. This // ensures that we won't deduct more that the specified balance // from our wallet. case r.SubtractFees: dustLimit := w.cfg.DustLimit - selectedCoins, localContributionAmt, changeAmt, err = CoinSelectSubtractFees( + selectedCoins, localContributionAmt, changeAmt, + err = CoinSelectSubtractFees( r.FeeRate, r.LocalAmt, dustLimit, coins, ) if err != nil {