From 7897b96e6ab1a4c5a4816a8eb2f0bc00a86217c4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 19 Sep 2019 14:59:07 +0200 Subject: [PATCH 1/4] lnwallet/interface_test: extract local utility functions In preparation for extending the testPublishTransaction test, shorten it by moving utility methods out of the local scope. --- lnwallet/interface_test.go | 478 ++++++++++++++++++++----------------- 1 file changed, 256 insertions(+), 222 deletions(-) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 8a2793904..8377d95b8 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -1418,177 +1418,197 @@ func testTransactionSubscriptions(miner *rpctest.Harness, } } -// testPublishTransaction checks that PublishTransaction returns the -// expected error types in case the transaction being published -// conflicts with the current mempool or chain. -func testPublishTransaction(r *rpctest.Harness, - alice, _ *lnwallet.LightningWallet, t *testing.T) { - - // mineAndAssert mines a block and ensures the passed TX - // is part of that block. - mineAndAssert := func(tx *wire.MsgTx) error { - blockHashes, err := r.Node.Generate(1) - if err != nil { - return fmt.Errorf("unable to generate block: %v", err) - } - - block, err := r.Node.GetBlock(blockHashes[0]) - if err != nil { - return fmt.Errorf("unable to find block: %v", err) - } - - if len(block.Transactions) != 2 { - return fmt.Errorf("expected 2 txs in block, got %d", - len(block.Transactions)) - } - - blockTx := block.Transactions[1] - if blockTx.TxHash() != tx.TxHash() { - return fmt.Errorf("incorrect transaction was mined") - } - - // Sleep for a second before returning, to make sure the - // block has propagated. - time.Sleep(1 * time.Second) - return nil - } - - // Generate a pubkey, and pay-to-addr script. - pubKey, err := alice.DeriveNextKey( - keychain.KeyFamilyMultiSig, +// scriptFromKey creates a P2WKH script from the given pubkey. +func scriptFromKey(pubkey *btcec.PublicKey) ([]byte, error) { + pubkeyHash := btcutil.Hash160(pubkey.SerializeCompressed()) + keyAddr, err := btcutil.NewAddressWitnessPubKeyHash( + pubkeyHash, &chaincfg.RegressionNetParams, ) if err != nil { - t.Fatalf("unable to obtain public key: %v", err) - } - pubkeyHash := btcutil.Hash160(pubKey.PubKey.SerializeCompressed()) - keyAddr, err := btcutil.NewAddressWitnessPubKeyHash(pubkeyHash, - &chaincfg.RegressionNetParams) - if err != nil { - t.Fatalf("unable to create addr: %v", err) + return nil, fmt.Errorf("unable to create addr: %v", err) } keyScript, err := txscript.PayToAddrScript(keyAddr) + if err != nil { + return nil, fmt.Errorf("unable to generate script: %v", err) + } + + return keyScript, nil +} + +// mineAndAssert mines a block and ensures the passed TX is part of that block. +func mineAndAssert(r *rpctest.Harness, tx *wire.MsgTx) error { + txid := tx.TxHash() + err := waitForMempoolTx(r, &txid) + if err != nil { + return fmt.Errorf("tx not relayed to miner: %v", err) + } + + blockHashes, err := r.Node.Generate(1) + if err != nil { + return fmt.Errorf("unable to generate block: %v", err) + } + + block, err := r.Node.GetBlock(blockHashes[0]) + if err != nil { + return fmt.Errorf("unable to find block: %v", err) + } + + if len(block.Transactions) != 2 { + return fmt.Errorf("expected 2 txs in block, got %d", + len(block.Transactions)) + } + + blockTx := block.Transactions[1] + if blockTx.TxHash() != tx.TxHash() { + return fmt.Errorf("incorrect transaction was mined") + } + + // Sleep for a second before returning, to make sure the block has + // propagated. + time.Sleep(1 * time.Second) + return nil +} + +// txFromOutput takes a tx paying to fromPubKey, and creates a new tx that +// spends the output from this tx, to an address derived from payToPubKey. +func txFromOutput(tx *wire.MsgTx, signer input.Signer, fromPubKey, + payToPubKey *btcec.PublicKey, txFee btcutil.Amount) ( + *wire.MsgTx, error) { + + // Generate the script we want to spend from. + keyScript, err := scriptFromKey(fromPubKey) + if err != nil { + return nil, fmt.Errorf("unable to generate script: %v", err) + } + + // We assume the output was paid to the keyScript made earlier. + var outputIndex uint32 + if len(tx.TxOut) == 1 || bytes.Equal(tx.TxOut[0].PkScript, keyScript) { + outputIndex = 0 + } else { + outputIndex = 1 + } + outputValue := tx.TxOut[outputIndex].Value + + // With the index located, we can create a transaction spending the + // referenced output. + tx1 := wire.NewMsgTx(2) + + tx1.AddTxIn(&wire.TxIn{ + PreviousOutPoint: wire.OutPoint{ + Hash: tx.TxHash(), + Index: outputIndex, + }, + // We don't support RBF, so set sequence to max. + Sequence: wire.MaxTxInSequenceNum, + }) + + // Create a script to pay to. + payToScript, err := scriptFromKey(payToPubKey) + if err != nil { + return nil, fmt.Errorf("unable to generate script: %v", err) + } + tx1.AddTxOut(&wire.TxOut{ + Value: outputValue - int64(txFee), + PkScript: payToScript, + }) + + // Now we can populate the sign descriptor which we'll use to generate + // the signature. + signDesc := &input.SignDescriptor{ + KeyDesc: keychain.KeyDescriptor{ + PubKey: fromPubKey, + }, + WitnessScript: keyScript, + Output: tx.TxOut[outputIndex], + HashType: txscript.SigHashAll, + SigHashes: txscript.NewTxSigHashes(tx1), + InputIndex: 0, // Has only one input. + } + + // With the descriptor created, we use it to generate a signature, then + // manually create a valid witness stack we'll use for signing. + spendSig, err := signer.SignOutputRaw(tx1, signDesc) + if err != nil { + return nil, fmt.Errorf("unable to generate signature: %v", err) + } + witness := make([][]byte, 2) + witness[0] = append(spendSig, byte(txscript.SigHashAll)) + witness[1] = fromPubKey.SerializeCompressed() + tx1.TxIn[0].Witness = witness + + // Finally, attempt to validate the completed transaction. This should + // succeed if the wallet was able to properly generate the proper + // private key. + vm, err := txscript.NewEngine( + keyScript, tx1, 0, txscript.StandardVerifyFlags, nil, + nil, outputValue, + ) + if err != nil { + return nil, fmt.Errorf("unable to create engine: %v", err) + } + if err := vm.Execute(); err != nil { + return nil, fmt.Errorf("spend is invalid: %v", err) + } + + return tx1, nil +} + +// newTx sends coins from Alice's wallet, mines this transaction, and creates a +// new, unconfirmed tx that spends this output to pubKey. +func newTx(t *testing.T, r *rpctest.Harness, pubKey *btcec.PublicKey, + alice *lnwallet.LightningWallet, rbf bool) *wire.MsgTx { + t.Helper() + + keyScript, err := scriptFromKey(pubKey) if err != nil { t.Fatalf("unable to generate script: %v", err) } - // txFromOutput takes a tx, and creates a new tx that spends - // the output from this tx, to an address derived from payToPubKey. - // NB: assumes that the output from tx is paid to pubKey. - txFromOutput := func(tx *wire.MsgTx, payToPubKey *btcec.PublicKey, - txFee btcutil.Amount) *wire.MsgTx { - // Create a script to pay to. - payToPubkeyHash := btcutil.Hash160(payToPubKey.SerializeCompressed()) - payToKeyAddr, err := btcutil.NewAddressWitnessPubKeyHash(payToPubkeyHash, - &chaincfg.RegressionNetParams) - if err != nil { - t.Fatalf("unable to create addr: %v", err) - } - payToScript, err := txscript.PayToAddrScript(payToKeyAddr) - if err != nil { - t.Fatalf("unable to generate script: %v", err) - } - - // We assume the output was paid to the keyScript made earlier. - var outputIndex uint32 - if len(tx.TxOut) == 1 || bytes.Equal(tx.TxOut[0].PkScript, keyScript) { - outputIndex = 0 - } else { - outputIndex = 1 - } - outputValue := tx.TxOut[outputIndex].Value - - // With the index located, we can create a transaction spending - // the referenced output. - tx1 := wire.NewMsgTx(2) - tx1.AddTxIn(&wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: tx.TxHash(), - Index: outputIndex, - }, - // We don't support RBF, so set sequence to max. - Sequence: wire.MaxTxInSequenceNum, - }) - tx1.AddTxOut(&wire.TxOut{ - Value: outputValue - int64(txFee), - PkScript: payToScript, - }) - - // Now we can populate the sign descriptor which we'll use to - // generate the signature. - signDesc := &input.SignDescriptor{ - KeyDesc: keychain.KeyDescriptor{ - PubKey: pubKey.PubKey, - }, - WitnessScript: keyScript, - Output: tx.TxOut[outputIndex], - HashType: txscript.SigHashAll, - SigHashes: txscript.NewTxSigHashes(tx1), - InputIndex: 0, // Has only one input. - } - - // With the descriptor created, we use it to generate a - // signature, then manually create a valid witness stack we'll - // use for signing. - spendSig, err := alice.Cfg.Signer.SignOutputRaw(tx1, signDesc) - if err != nil { - t.Fatalf("unable to generate signature: %v", err) - } - witness := make([][]byte, 2) - witness[0] = append(spendSig, byte(txscript.SigHashAll)) - witness[1] = pubKey.PubKey.SerializeCompressed() - tx1.TxIn[0].Witness = witness - - // Finally, attempt to validate the completed transaction. This - // should succeed if the wallet was able to properly generate - // the proper private key. - vm, err := txscript.NewEngine(keyScript, - tx1, 0, txscript.StandardVerifyFlags, nil, - nil, outputValue) - if err != nil { - t.Fatalf("unable to create engine: %v", err) - } - if err := vm.Execute(); err != nil { - t.Fatalf("spend is invalid: %v", err) - } - return tx1 + // Instruct the wallet to fund the output with a newly created + // transaction. + newOutput := &wire.TxOut{ + Value: btcutil.SatoshiPerBitcoin, + PkScript: keyScript, + } + tx, err := alice.SendOutputs([]*wire.TxOut{newOutput}, 2500) + if err != nil { + t.Fatalf("unable to create output: %v", err) } - // newTx sends coins from Alice's wallet, mines this transaction, - // and creates a new, unconfirmed tx that spends this output to - // pubKey. - newTx := func() *wire.MsgTx { - - // With the script fully assembled, instruct the wallet to fund - // the output with a newly created transaction. - newOutput := &wire.TxOut{ - Value: btcutil.SatoshiPerBitcoin, - PkScript: keyScript, - } - tx, err := alice.SendOutputs([]*wire.TxOut{newOutput}, 2500) - if err != nil { - t.Fatalf("unable to create output: %v", err) - } - txid := tx.TxHash() - - // Query for the transaction generated above so we can located - // the index of our output. - err = waitForMempoolTx(r, &txid) - if err != nil { - t.Fatalf("tx not relayed to miner: %v", err) - } - - if err := mineAndAssert(tx); err != nil { - t.Fatalf("unable to mine tx: %v", err) - } - txFee := btcutil.Amount(0.1 * btcutil.SatoshiPerBitcoin) - tx1 := txFromOutput(tx, pubKey.PubKey, txFee) - - return tx1 + // Query for the transaction generated above so we can located the + // index of our output. + if err := mineAndAssert(r, tx); err != nil { + t.Fatalf("unable to mine tx: %v", err) } - // We will first check that publishing a transaction already - // in the mempool does NOT return an error. Create the tx. - tx1 := newTx() + // Create a new unconfirmed tx that spends this output. + txFee := btcutil.Amount(0.1 * btcutil.SatoshiPerBitcoin) + tx1, err := txFromOutput( + tx, alice.Cfg.Signer, pubKey, pubKey, txFee, + ) + if err != nil { + t.Fatal(err) + } + + return tx1 +} + +// testPublishTransaction checks that PublishTransaction returns the expected +// error types in case the transaction being published conflicts with the +// current mempool or chain. +func testPublishTransaction(r *rpctest.Harness, + alice, _ *lnwallet.LightningWallet, t *testing.T) { + + // Generate a pubkey, and pay-to-addr script. + keyDesc, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig) + if err != nil { + t.Fatalf("unable to obtain public key: %v", err) + } + + // We will first check that publishing a transaction already in the + // mempool does NOT return an error. Create the tx. + tx1 := newTx(t, r, keyDesc.PubKey, alice, false) // Publish the transaction. if err := alice.PublishTransaction(tx1); err != nil { @@ -1601,9 +1621,8 @@ func testPublishTransaction(r *rpctest.Harness, t.Fatalf("tx not relayed to miner: %v", err) } - // Publish the exact same transaction again. This should - // not return an error, even though the transaction is - // already in the mempool. + // Publish the exact same transaction again. This should not return an + // error, even though the transaction is already in the mempool. if err := alice.PublishTransaction(tx1); err != nil { t.Fatalf("unable to publish: %v", err) } @@ -1613,62 +1632,56 @@ func testPublishTransaction(r *rpctest.Harness, t.Fatalf("unable to generate block: %v", err) } - // We'll now test that we don't get an error if we try - // to publish a transaction that is already mined. + // We'll now test that we don't get an error if we try to publish a + // transaction that is already mined. // - // Create a new transaction. We must do this to properly - // test the reject messages from our peers. They might - // only send us a reject message for a given tx once, - // so we create a new to make sure it is not just - // immediately rejected. - tx2 := newTx() + // Create a new transaction. We must do this to properly test the + // reject messages from our peers. They might only send us a reject + // message for a given tx once, so we create a new to make sure it is + // not just immediately rejected. + tx2 := newTx(t, r, keyDesc.PubKey, alice, false) // Publish this tx. if err := alice.PublishTransaction(tx2); err != nil { t.Fatalf("unable to publish: %v", err) } - txid2 := tx2.TxHash() - err = waitForMempoolTx(r, &txid2) - if err != nil { - t.Fatalf("tx not relayed to miner: %v", err) - } - // Mine the transaction. - if err := mineAndAssert(tx2); err != nil { + if err := mineAndAssert(r, tx2); err != nil { t.Fatalf("unable to mine tx: %v", err) } - // Publish the transaction again. It is already mined, - // and we don't expect this to return an error. + // Publish the transaction again. It is already mined, and we don't + // expect this to return an error. if err := alice.PublishTransaction(tx2); err != nil { t.Fatalf("unable to publish: %v", err) } // Now we'll try to double spend an output with a different - // transaction. Create a new tx and publish it. This is - // the output we'll try to double spend. - tx3 := newTx() + // transaction. Create a new tx and publish it. This is the output + // we'll try to double spend. + tx3 := newTx(t, r, keyDesc.PubKey, alice, false) if err := alice.PublishTransaction(tx3); err != nil { t.Fatalf("unable to publish: %v", err) } - txid3 := tx3.TxHash() - err = waitForMempoolTx(r, &txid3) - if err != nil { - t.Fatalf("tx not relayed to miner: %v", err) - } - // Mine the transaction. - if err := mineAndAssert(tx3); err != nil { + if err := mineAndAssert(r, tx3); err != nil { t.Fatalf("unable to mine tx: %v", err) } - // Now we create a transaction that spends the output - // from the tx just mined. This should be accepted - // into the mempool. + // Now we create a transaction that spends the output from the tx just + // mined. txFee := btcutil.Amount(0.05 * btcutil.SatoshiPerBitcoin) - tx4 := txFromOutput(tx3, pubKey.PubKey, txFee) + tx4, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, keyDesc.PubKey, + txFee, + ) + if err != nil { + t.Fatal(err) + } + + // This should be accepted into the mempool. if err := alice.PublishTransaction(tx4); err != nil { t.Fatalf("unable to publish: %v", err) } @@ -1679,50 +1692,63 @@ func testPublishTransaction(r *rpctest.Harness, t.Fatalf("tx not relayed to miner: %v", err) } - // Create a new key we'll pay to, to ensure we create - // a unique transaction. - pubKey2, err := alice.DeriveNextKey( + // Create a new key we'll pay to, to ensure we create a unique + // transaction. + keyDesc2, err := alice.DeriveNextKey( keychain.KeyFamilyMultiSig, ) if err != nil { t.Fatalf("unable to obtain public key: %v", err) } - // Create a new transaction that spends the output from - // tx3, and that pays to a different address. We expect - // this to be rejected because it is a double spend. - tx5 := txFromOutput(tx3, pubKey2.PubKey, txFee) - if err := alice.PublishTransaction(tx5); err != lnwallet.ErrDoubleSpend { + // Create a new transaction that spends the output from tx3, and that + // pays to a different address. We expect this to be rejected because + // it is a double spend. + tx5, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, keyDesc2.PubKey, + txFee, + ) + if err != nil { + t.Fatal(err) + } + + err = alice.PublishTransaction(tx5) + if err != lnwallet.ErrDoubleSpend { t.Fatalf("expected ErrDoubleSpend, got: %v", err) } - // Create another transaction that spends the same output, - // but has a higher fee. We expect also this tx to be - // rejected, since the sequence number of tx3 is set to Max, - // indicating it is not replacable. - pubKey3, err := alice.DeriveNextKey( - keychain.KeyFamilyMultiSig, - ) + // Create another transaction that spends the same output, but has a + // higher fee. We expect also this tx to be rejected, since the + // sequence number of tx3 is set to Max, indicating it is not + // replacable. + pubKey3, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig) if err != nil { t.Fatalf("unable to obtain public key: %v", err) } - tx6 := txFromOutput(tx3, pubKey3.PubKey, 3*txFee) + tx6, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, + pubKey3.PubKey, 2*txFee, + ) + + if err != nil { + t.Fatal(err) + } // Expect rejection. - if err := alice.PublishTransaction(tx6); err != lnwallet.ErrDoubleSpend { + err = alice.PublishTransaction(tx6) + if err != lnwallet.ErrDoubleSpend { t.Fatalf("expected ErrDoubleSpend, got: %v", err) } - // At last we try to spend an output already spent by a - // confirmed transaction. - // TODO(halseth): we currently skip this test for neutrino, - // as the backing btcd node will consider the tx being an - // orphan, and will accept it. Should look into if this is - // the behavior also for bitcoind, and update test - // accordingly. + // At last we try to spend an output already spent by a confirmed + // transaction. + // TODO(halseth): we currently skip this test for neutrino, as the + // backing btcd node will consider the tx being an orphan, and will + // accept it. Should look into if this is the behavior also for + // bitcoind, and update test accordingly. if alice.BackEnd() != "neutrino" { // Mine the tx spending tx3. - if err := mineAndAssert(tx4); err != nil { + if err := mineAndAssert(r, tx4); err != nil { t.Fatalf("unable to mine tx: %v", err) } @@ -1733,16 +1759,24 @@ func testPublishTransaction(r *rpctest.Harness, if err != nil { t.Fatalf("unable to obtain public key: %v", err) } - tx7 := txFromOutput(tx3, pubKey4.PubKey, txFee) + tx7, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, + pubKey4.PubKey, txFee, + ) + + if err != nil { + t.Fatal(err) + } // Expect rejection. - if err := alice.PublishTransaction(tx7); err != lnwallet.ErrDoubleSpend { + err = alice.PublishTransaction(tx7) + if err != lnwallet.ErrDoubleSpend { t.Fatalf("expected ErrDoubleSpend, got: %v", err) } } - // TODO(halseth): test replaceable transactions when btcd - // gets RBF support. + // TODO(halseth): test replaceable transactions when btcd gets RBF + // support. } func testSignOutputUsingTweaks(r *rpctest.Harness, From 96ebce684243b7acff207b6eaa6da597fe018636 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 19 Sep 2019 14:59:07 +0200 Subject: [PATCH 2/4] go mod: update btcwallet dependency We update to a new version of btcwallet where specific errors (ErrDoubleSpend and ErrReplacement) will be returned from PublishTransaction. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 41e3472a5..d6aea5618 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/btcsuite/btcd v0.0.0-20190824003749-130ea5bddde3 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-20190911065739-d5cdeb4b91b0 + github.com/btcsuite/btcwallet v0.0.0-20190925005052-95d7aa0b4953 github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 github.com/btcsuite/btcwallet/walletdb v1.0.0 diff --git a/go.sum b/go.sum index 27321d106..e9ed3de6b 100644 --- a/go.sum +++ b/go.sum @@ -25,8 +25,8 @@ github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d h1:yJzD/yFppdVCf6ApMkVy8cUxV0XrxdP9rVf6D87/Mng= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg= -github.com/btcsuite/btcwallet v0.0.0-20190911065739-d5cdeb4b91b0 h1:S9+cnZ7N4EvkkOBQ3lUy4p7+XjW4GS81R4QjwuT06Cw= -github.com/btcsuite/btcwallet v0.0.0-20190911065739-d5cdeb4b91b0/go.mod h1:ntLqUbZ12G8FmPX1nJj7W83WiAFOLRGiuarH4zDYdlI= +github.com/btcsuite/btcwallet v0.0.0-20190925005052-95d7aa0b4953 h1:NG3SmXd3KMOF4/BHVQaJuayrlXBosJgwUjeHcX4k198= +github.com/btcsuite/btcwallet v0.0.0-20190925005052-95d7aa0b4953/go.mod h1:ntLqUbZ12G8FmPX1nJj7W83WiAFOLRGiuarH4zDYdlI= github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 h1:KGHMW5sd7yDdDMkCZ/JpP0KltolFsQcB973brBnfj4c= github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0/go.mod h1:VufDts7bd/zs3GV13f/lXc/0lXrPnvxD/NvmpG/FEKU= github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 h1:2VsfS0sBedcM5KmDzRMT3+b6xobqWveZGvjb+jFez5w= From 61e1b48f5793c2103ad2312075bf709db659eb8a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 19 Sep 2019 14:59:07 +0200 Subject: [PATCH 3/4] lnwallet/btcwallet: check publication error types, handle replacement error Since btcwallet will return typed errors now, we can simplify the matching logic in order to return ErrDoubleSpend. In case a transaction cannot be published since it did not satisfy the requirements for a valid replacement, return ErrDoubleSpend to indicate it was not propagated. --- lnwallet/btcwallet/btcwallet.go | 51 +++++++++------------------------ 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index 9e2cab9e6..31c2e3206 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/lnwallet/btcwallet/btcwallet.go @@ -5,7 +5,6 @@ import ( "encoding/hex" "fmt" "math" - "strings" "sync" "time" @@ -429,47 +428,25 @@ func (b *BtcWallet) ListUnspentWitness(minConfs, maxConfs int32) ( // network (either in the mempool or chain) no error will be returned. func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx) error { if err := b.wallet.PublishTransaction(tx); err != nil { - switch b.chain.(type) { - case *chain.RPCClient: - if strings.Contains(err.Error(), "already spent") { - // Output was already spent. - return lnwallet.ErrDoubleSpend - } - if strings.Contains(err.Error(), "already been spent") { - // Output was already spent. - return lnwallet.ErrDoubleSpend - } - if strings.Contains(err.Error(), "orphan transaction") { - // Transaction is spending either output that - // is missing or already spent. - return lnwallet.ErrDoubleSpend - } - case *chain.BitcoindClient: - if strings.Contains(err.Error(), "txn-mempool-conflict") { - // Output was spent by other transaction - // already in the mempool. - return lnwallet.ErrDoubleSpend - } - if strings.Contains(err.Error(), "insufficient fee") { - // RBF enabled transaction did not have enough fee. - return lnwallet.ErrDoubleSpend - } - if strings.Contains(err.Error(), "Missing inputs") { - // Transaction is spending either output that - // is missing or already spent. - return lnwallet.ErrDoubleSpend - } + // If we failed to publish the transaction, check whether we + // got an error of known type. + switch err.(type) { - case *chain.NeutrinoClient: - if strings.Contains(err.Error(), "already spent") { - // Output was already spent. - return lnwallet.ErrDoubleSpend - } + // If the wallet reports a double spend, convert it to our + // internal ErrDoubleSpend and return. + case *base.ErrDoubleSpend: + return lnwallet.ErrDoubleSpend + + // If the wallet reports a replacement error, return + // ErrDoubleSpend, as we currently are never attempting to + // replace transactions. + case *base.ErrReplacement: + return lnwallet.ErrDoubleSpend default: + return err } - return err } return nil } From 2e9452916e9a308de168b831443c3b43e398a97c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 19 Sep 2019 14:59:07 +0200 Subject: [PATCH 4/4] lnwallet/interface_test: add RBF test cases to testPublishTransaction Checks that we get ErrDoubleSpend as expected when publishing a conflicting mempool transaction with the same fee as the existing one, and that we can publish a replacement with a higher fee successfully. --- lnwallet/interface_test.go | 206 ++++++++++++++++++++----------------- 1 file changed, 114 insertions(+), 92 deletions(-) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 8377d95b8..deda71e71 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -22,6 +22,7 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/integration/rpctest" + "github.com/btcsuite/btcd/mempool" "github.com/btcsuite/btcd/rpcclient" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" @@ -1472,8 +1473,8 @@ func mineAndAssert(r *rpctest.Harness, tx *wire.MsgTx) error { // txFromOutput takes a tx paying to fromPubKey, and creates a new tx that // spends the output from this tx, to an address derived from payToPubKey. func txFromOutput(tx *wire.MsgTx, signer input.Signer, fromPubKey, - payToPubKey *btcec.PublicKey, txFee btcutil.Amount) ( - *wire.MsgTx, error) { + payToPubKey *btcec.PublicKey, txFee btcutil.Amount, + rbf bool) (*wire.MsgTx, error) { // Generate the script we want to spend from. keyScript, err := scriptFromKey(fromPubKey) @@ -1494,13 +1495,20 @@ func txFromOutput(tx *wire.MsgTx, signer input.Signer, fromPubKey, // referenced output. tx1 := wire.NewMsgTx(2) + // If we want to create a tx that signals replacement, set its + // sequence number to the max one that signals replacement. + // Otherwise we just use the standard max sequence. + sequence := wire.MaxTxInSequenceNum + if rbf { + sequence = mempool.MaxRBFSequence + } + tx1.AddTxIn(&wire.TxIn{ PreviousOutPoint: wire.OutPoint{ Hash: tx.TxHash(), Index: outputIndex, }, - // We don't support RBF, so set sequence to max. - Sequence: wire.MaxTxInSequenceNum, + Sequence: sequence, }) // Create a script to pay to. @@ -1585,7 +1593,7 @@ func newTx(t *testing.T, r *rpctest.Harness, pubKey *btcec.PublicKey, // Create a new unconfirmed tx that spends this output. txFee := btcutil.Amount(0.1 * btcutil.SatoshiPerBitcoin) tx1, err := txFromOutput( - tx, alice.Cfg.Signer, pubKey, pubKey, txFee, + tx, alice.Cfg.Signer, pubKey, pubKey, txFee, rbf, ) if err != nil { t.Fatal(err) @@ -1657,87 +1665,109 @@ func testPublishTransaction(r *rpctest.Harness, t.Fatalf("unable to publish: %v", err) } - // Now we'll try to double spend an output with a different - // transaction. Create a new tx and publish it. This is the output - // we'll try to double spend. - tx3 := newTx(t, r, keyDesc.PubKey, alice, false) - if err := alice.PublishTransaction(tx3); err != nil { - t.Fatalf("unable to publish: %v", err) - } - - // Mine the transaction. - if err := mineAndAssert(r, tx3); err != nil { - t.Fatalf("unable to mine tx: %v", err) - } - - // Now we create a transaction that spends the output from the tx just - // mined. - txFee := btcutil.Amount(0.05 * btcutil.SatoshiPerBitcoin) - tx4, err := txFromOutput( - tx3, alice.Cfg.Signer, keyDesc.PubKey, keyDesc.PubKey, - txFee, - ) - if err != nil { - t.Fatal(err) - } - - // This should be accepted into the mempool. - if err := alice.PublishTransaction(tx4); err != nil { - t.Fatalf("unable to publish: %v", err) - } - - txid4 := tx4.TxHash() - err = waitForMempoolTx(r, &txid4) - if err != nil { - t.Fatalf("tx not relayed to miner: %v", err) - } - - // Create a new key we'll pay to, to ensure we create a unique - // transaction. - keyDesc2, err := alice.DeriveNextKey( - keychain.KeyFamilyMultiSig, - ) - if err != nil { - t.Fatalf("unable to obtain public key: %v", err) - } - - // Create a new transaction that spends the output from tx3, and that - // pays to a different address. We expect this to be rejected because - // it is a double spend. - tx5, err := txFromOutput( - tx3, alice.Cfg.Signer, keyDesc.PubKey, keyDesc2.PubKey, - txFee, - ) - if err != nil { - t.Fatal(err) - } - - err = alice.PublishTransaction(tx5) - if err != lnwallet.ErrDoubleSpend { - t.Fatalf("expected ErrDoubleSpend, got: %v", err) - } - - // Create another transaction that spends the same output, but has a - // higher fee. We expect also this tx to be rejected, since the - // sequence number of tx3 is set to Max, indicating it is not - // replacable. - pubKey3, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig) - if err != nil { - t.Fatalf("unable to obtain public key: %v", err) - } - tx6, err := txFromOutput( - tx3, alice.Cfg.Signer, keyDesc.PubKey, - pubKey3.PubKey, 2*txFee, + // We'll do the next mempool check on both RBF and non-RBF enabled + // transactions. + var ( + txFee = btcutil.Amount(0.05 * btcutil.SatoshiPerBitcoin) + tx3, tx3Spend *wire.MsgTx ) - if err != nil { - t.Fatal(err) - } + for _, rbf := range []bool{false, true} { + // Now we'll try to double spend an output with a different + // transaction. Create a new tx and publish it. This is the + // output we'll try to double spend. + tx3 = newTx(t, r, keyDesc.PubKey, alice, false) + if err := alice.PublishTransaction(tx3); err != nil { + t.Fatalf("unable to publish: %v", err) + } - // Expect rejection. - err = alice.PublishTransaction(tx6) - if err != lnwallet.ErrDoubleSpend { - t.Fatalf("expected ErrDoubleSpend, got: %v", err) + // Mine the transaction. + if err := mineAndAssert(r, tx3); err != nil { + t.Fatalf("unable to mine tx: %v", err) + } + + // Now we create a transaction that spends the output from the + // tx just mined. + tx4, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, + keyDesc.PubKey, txFee, rbf, + ) + if err != nil { + t.Fatal(err) + } + + // This should be accepted into the mempool. + if err := alice.PublishTransaction(tx4); err != nil { + t.Fatalf("unable to publish: %v", err) + } + + // Keep track of the last successfully published tx to spend + // tx3. + tx3Spend = tx4 + + txid4 := tx4.TxHash() + err = waitForMempoolTx(r, &txid4) + if err != nil { + t.Fatalf("tx not relayed to miner: %v", err) + } + + // Create a new key we'll pay to, to ensure we create a unique + // transaction. + keyDesc2, err := alice.DeriveNextKey( + keychain.KeyFamilyMultiSig, + ) + if err != nil { + t.Fatalf("unable to obtain public key: %v", err) + } + + // Create a new transaction that spends the output from tx3, + // and that pays to a different address. We expect this to be + // rejected because it is a double spend. + tx5, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, + keyDesc2.PubKey, txFee, rbf, + ) + if err != nil { + t.Fatal(err) + } + + err = alice.PublishTransaction(tx5) + if err != lnwallet.ErrDoubleSpend { + t.Fatalf("expected ErrDoubleSpend, got: %v", err) + } + + // Create another transaction that spends the same output, but + // has a higher fee. We expect also this tx to be rejected for + // non-RBF enabled transactions, while it should succeed + // otherwise. + pubKey3, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig) + if err != nil { + t.Fatalf("unable to obtain public key: %v", err) + } + tx6, err := txFromOutput( + tx3, alice.Cfg.Signer, keyDesc.PubKey, + pubKey3.PubKey, 2*txFee, rbf, + ) + if err != nil { + t.Fatal(err) + } + + // Expect rejection in non-RBF case. + expErr := lnwallet.ErrDoubleSpend + if rbf { + // Expect success in rbf case. + expErr = nil + tx3Spend = tx6 + } + err = alice.PublishTransaction(tx6) + if err != expErr { + t.Fatalf("expected ErrDoubleSpend, got: %v", err) + } + + // Mine the tx spending tx3. + if err := mineAndAssert(r, tx3Spend); err != nil { + t.Fatalf("unable to mine tx: %v", err) + } } // At last we try to spend an output already spent by a confirmed @@ -1747,11 +1777,6 @@ func testPublishTransaction(r *rpctest.Harness, // accept it. Should look into if this is the behavior also for // bitcoind, and update test accordingly. if alice.BackEnd() != "neutrino" { - // Mine the tx spending tx3. - if err := mineAndAssert(r, tx4); err != nil { - t.Fatalf("unable to mine tx: %v", err) - } - // Create another tx spending tx3. pubKey4, err := alice.DeriveNextKey( keychain.KeyFamilyMultiSig, @@ -1761,7 +1786,7 @@ func testPublishTransaction(r *rpctest.Harness, } tx7, err := txFromOutput( tx3, alice.Cfg.Signer, keyDesc.PubKey, - pubKey4.PubKey, txFee, + pubKey4.PubKey, txFee, false, ) if err != nil { @@ -1774,9 +1799,6 @@ func testPublishTransaction(r *rpctest.Harness, t.Fatalf("expected ErrDoubleSpend, got: %v", err) } } - - // TODO(halseth): test replaceable transactions when btcd gets RBF - // support. } func testSignOutputUsingTweaks(r *rpctest.Harness,