From 5c5c542f94cfbd881232e25e87a6bfc0d0d947ee Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 20 Feb 2019 15:06:21 -0800 Subject: [PATCH 1/2] lnwallet/btcwallet: remove unnecessary tx in mempool/chain checks The checks to determine whether the transaction broadcast failed due to it already existing in the mempool/chain are no longer needed since the underlying btcwallet PublishTransaction call will not return an error when running into these cases. --- lnwallet/btcwallet/btcwallet.go | 44 +++------------------------------ 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index 963ca002..b73ad524 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/lnwallet/btcwallet/btcwallet.go @@ -384,28 +384,13 @@ func (b *BtcWallet) ListUnspentWitness(minConfs, maxConfs int32) ( // PublishTransaction performs cursory validation (dust checks, etc), then // finally broadcasts the passed transaction to the Bitcoin network. If -// publishing the transaction fails, an error describing the reason is -// returned (currently ErrDoubleSpend). If the transaction is already -// published to the network (either in the mempool or chain) no error -// will be returned. +// publishing the transaction fails, an error describing the reason is returned +// (currently ErrDoubleSpend). If the transaction is already published to the +// 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 have") { - // Transaction was already in the mempool, do - // not treat as an error. We do this to mimic - // the behaviour of bitcoind, which will not - // return an error if a transaction in the - // mempool is sent again using the - // sendrawtransaction RPC call. - return nil - } - if strings.Contains(err.Error(), "already exists") { - // Transaction was already mined, we don't - // consider this an error. - return nil - } if strings.Contains(err.Error(), "already spent") { // Output was already spent. return lnwallet.ErrDoubleSpend @@ -421,19 +406,6 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx) error { } case *chain.BitcoindClient: - if strings.Contains(err.Error(), "txn-already-in-mempool") { - // Transaction in mempool, treat as non-error. - return nil - } - if strings.Contains(err.Error(), "txn-already-known") { - // Transaction in mempool, treat as non-error. - return nil - } - if strings.Contains(err.Error(), "already in block") { - // Transaction was already mined, we don't - // consider this an error. - return nil - } if strings.Contains(err.Error(), "txn-mempool-conflict") { // Output was spent by other transaction // already in the mempool. @@ -450,16 +422,6 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx) error { } case *chain.NeutrinoClient: - if strings.Contains(err.Error(), "already have") { - // Transaction was already in the mempool, do - // not treat as an error. - return nil - } - if strings.Contains(err.Error(), "already exists") { - // Transaction was already mined, we don't - // consider this an error. - return nil - } if strings.Contains(err.Error(), "already spent") { // Output was already spent. return lnwallet.ErrDoubleSpend From 7946d0a25645f80883b264a5a2a0ab595c3b3b7f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 20 Feb 2019 15:06:57 -0800 Subject: [PATCH 2/2] multi: remove ErrDoubleSpend check for PublishTransaction In this commit, we address a lingering issue within some subsystems that are responsible for broadcasting transactions. Previously, ErrDoubleSpend indicated that a transaction was already included in the mempool/chain. This error was then modified to actually be returned for conflicting transactions, but its callers were not modified accordingly. This would lead to conflicting transactions to be interpreted as valid, when they shouldn't be. --- contractcourt/htlc_success_resolver.go | 5 ++--- fundingmanager.go | 2 +- utxonursery.go | 2 +- watchtower/lookout/punisher.go | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 82fbdc02..abad34b4 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -7,7 +7,6 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" - "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lntypes" @@ -148,7 +147,7 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { // constructed, we'll broadcast the sweep transaction to the // network. err := h.PublishTx(h.sweepTx) - if err != nil && err != lnwallet.ErrDoubleSpend { + if err != nil { log.Infof("%T(%x): unable to publish tx: %v", h, h.payHash[:], err) return nil, err @@ -200,7 +199,7 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { // // TODO(roasbeef): after changing sighashes send to tx bundler err := h.PublishTx(h.htlcResolution.SignedSuccessTx) - if err != nil && err != lnwallet.ErrDoubleSpend { + if err != nil { return nil, err } diff --git a/fundingmanager.go b/fundingmanager.go index a86b1747..49b560a5 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -508,7 +508,7 @@ func (f *fundingManager) Start() error { channel.IsInitiator { err := f.cfg.PublishTransaction(channel.FundingTxn) - if err != nil && err != lnwallet.ErrDoubleSpend { + if err != nil { fndgLog.Errorf("Unable to rebroadcast funding "+ "tx for ChannelPoint(%v): %v", channel.FundingOutpoint, err) diff --git a/utxonursery.go b/utxonursery.go index 36b97815..829d4ba0 100644 --- a/utxonursery.go +++ b/utxonursery.go @@ -894,7 +894,7 @@ func (u *utxoNursery) sweepCribOutput(classHeight uint32, baby *babyOutput) erro // We'll now broadcast the HTLC transaction, then wait for it to be // confirmed before transitioning it to kindergarten. err := u.cfg.PublishTransaction(baby.timeoutTx) - if err != nil && err != lnwallet.ErrDoubleSpend { + if err != nil { utxnLog.Errorf("Unable to broadcast baby tx: "+ "%v, %v", err, spew.Sdump(baby.timeoutTx)) return err diff --git a/watchtower/lookout/punisher.go b/watchtower/lookout/punisher.go index fb441430..eb9c058c 100644 --- a/watchtower/lookout/punisher.go +++ b/watchtower/lookout/punisher.go @@ -2,7 +2,6 @@ package lookout import ( "github.com/btcsuite/btcd/wire" - "github.com/lightningnetwork/lnd/lnwallet" ) // PunisherConfig houses the resources required by the Punisher. @@ -44,7 +43,7 @@ func (p *BreachPunisher) Punish(desc *JusticeDescriptor, quit <-chan struct{}) e desc.SessionInfo.ID, justiceTxn.TxHash()) err = p.cfg.PublishTx(justiceTxn) - if err != nil && err != lnwallet.ErrDoubleSpend { + if err != nil { log.Errorf("Unable to publish justice txn for client=%s"+ "with breach-txid=%s: %v", desc.SessionInfo.ID, desc.BreachedCommitTx.TxHash(), err)