From eaa043f585ba292e9659528cc0384590cb6215f8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 18 Jun 2019 17:04:59 -0700 Subject: [PATCH 1/3] lnwallet/btcwallet: use relay fee not tx fee rate for dust check In this commit we fix a hidden bug in the transaction creating logic that was only manifested recently due to higher fees on Bitcoin's mainnet. Before this commit, we would use the target fee rate to determine if an output was dust or not. However, this is incorrect, as instead the relay fee should be used as this matches the policy checks widely deployed in Bitcoin full node today. To fix this issue we now properly use the relay fee when computing dust. This fixes the issue for the `EstimateFee` call, but the `SendOutputs` call also has a similar issue. However, this must be fixed within `btcwallet` itself, so it has been left out of this commit Fixes #3217. --- lnwallet/btcwallet/btcwallet.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index 966cbb5b4..268d8dd42 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/lnwallet/btcwallet/btcwallet.go @@ -330,7 +330,14 @@ func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut, return nil, lnwallet.ErrNoOutputs } for _, output := range outputs { - err := txrules.CheckOutput(output, feeSatPerKB) + // When checking an output for things like dusty-ness, we'll + // use the default mempool relay fee rather than the target + // effective fee rate to ensure accuracy. Otherwise, we may + // mistakenly mark small-ish, but not quite dust output as + // dust. + err := txrules.CheckOutput( + output, txrules.DefaultRelayFeePerKb, + ) if err != nil { return nil, err } From 2012b5dc1a67e2ac5d0e3a652b86e3d3bddf4720 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 18 Jun 2019 17:52:50 -0700 Subject: [PATCH 2/3] lnwallet: fix logic in testCreateSimpleTx test case In this commit, we fix a logic flaw in the testCreateSimpleTx test case which emerged once we the bug fix for dust outputs landed. Before this commit, we would erroneously fail during valid test execution. --- lnwallet/interface_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 4d30d07f0..0f9662639 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -2246,7 +2246,7 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet, }, { - outVals: []int64{1e3}, + outVals: []int64{200}, feeRate: 2500, valid: false, // Dust output. }, @@ -2279,7 +2279,7 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet, }, } - for _, test := range testCases { + for i, test := range testCases { feeRate := test.feeRate // Grab some fresh addresses from the miner that we will send @@ -2308,10 +2308,15 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet, createTx, createErr := w.CreateSimpleTx( outputs, feeRate, true, ) - if test.valid == (createErr != nil) { + switch { + case test.valid && createErr != nil: fmt.Println(spew.Sdump(createTx.Tx)) t.Fatalf("got unexpected error when creating tx: %v", createErr) + + case !test.valid && createErr == nil: + t.Fatalf("test #%v should have failed on tx "+ + "creation", i) } // Also send to these outputs. This should result in a tx @@ -2319,9 +2324,13 @@ func testCreateSimpleTx(r *rpctest.Harness, w *lnwallet.LightningWallet, // only difference is that the dry run tx is not signed, and // that the change output position might be different. tx, sendErr := w.SendOutputs(outputs, feeRate) - if test.valid == (sendErr != nil) { + switch { + case test.valid && sendErr != nil: t.Fatalf("got unexpected error when sending tx: %v", sendErr) + + case !test.valid && sendErr == nil: + t.Fatalf("test #%v should fail for tx sending", i) } // We expected either both to not fail, or both to fail with From 42035421547b7dd965bf6b1684f4ea5d25b5d60f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 18 Jun 2019 18:00:53 -0700 Subject: [PATCH 3/3] btcwallet: update to btcwallet version w/ SendOutputs dust fix --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 57922a3b3..08ef9b234 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/btcsuite/btcd v0.0.0-20190614013741-962a206e94e9 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d - github.com/btcsuite/btcwallet v0.0.0-20190614043544-a335c566148c + github.com/btcsuite/btcwallet v0.0.0-20190619005538-de02d6fdfb23 github.com/btcsuite/fastsha256 v0.0.0-20160815193821-637e65642941 github.com/coreos/bbolt v1.3.2 github.com/davecgh/go-spew v1.1.1 diff --git a/go.sum b/go.sum index e4b7dce7b..c3eae414a 100644 --- a/go.sum +++ b/go.sum @@ -39,8 +39,8 @@ github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+q github.com/btcsuite/btcwallet v0.0.0-20180904010540-284e2e0e696e33d5be388f7f3d9a26db703e0c06/go.mod h1:/d7QHZsfUAruXuBhyPITqoYOmJ+nq35qPsJjz/aSpCg= github.com/btcsuite/btcwallet v0.0.0-20190313032608-acf3b04b0273/go.mod h1:mkOYY8/psBiL5E+Wb0V7M0o+N7NXi2SZJz6+RKkncIc= github.com/btcsuite/btcwallet v0.0.0-20190319010515-89ab2044f962/go.mod h1:qMi4jGpAO6YRsd81RYDG7o5pBIGqN9faCioJdagLu64= -github.com/btcsuite/btcwallet v0.0.0-20190614043544-a335c566148c h1:9fQATx2+LheGbExN2jYuPJXNkvve5/9n/1EaUhzsZf0= -github.com/btcsuite/btcwallet v0.0.0-20190614043544-a335c566148c/go.mod h1:GlcKHrCBxtujd/6coLUvczN68EkaBezgyN+JnEGVDUY= +github.com/btcsuite/btcwallet v0.0.0-20190619005538-de02d6fdfb23 h1:tja9kILSaG27Z/kiR91dxmMk3T5+qplNLOxWT8iShKg= +github.com/btcsuite/btcwallet v0.0.0-20190619005538-de02d6fdfb23/go.mod h1:GlcKHrCBxtujd/6coLUvczN68EkaBezgyN+JnEGVDUY= github.com/btcsuite/fastsha256 v0.0.0-20160815193821-637e65642941 h1:kij1x2aL7VE6gtx8KMIt8PGPgI5GV9LgtHFG5KaEMPY= github.com/btcsuite/fastsha256 v0.0.0-20160815193821-637e65642941/go.mod h1:QcFA8DZHtuIAdYKCq/BzELOaznRsCvwf4zTPmaYwaig= github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd h1:R/opQEbFEy9JGkIguV40SvRY1uliPX8ifOvi6ICsFCw=