From 06e70c0f080cc100dcf98eb0e7871e7ff35742c4 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 20 Sep 2018 18:53:03 -0700 Subject: [PATCH 1/3] wallet/wallet: use addRelevantTx when publishing transactions In this commit, we simplify the logic when broadcasting transactions to the greater network. Rather than special casing when running with a Neutrino backend, we'll always add the transaction to the store as relevant when attempting to broadcast it. This will properly insert it into the store and update unconfirmed balances. In the event that the transaction failed to broadcast, it can be removed from the store with no side-effects, essentially acting as if the transaction was never added to the store in the first place. --- wallet/wallet.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index 0f5d1f7..e2ee0c8 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3312,9 +3312,8 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { if err != nil { return err } - err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { - txmgrNs := tx.ReadWriteBucket(wtxmgrNamespaceKey) - return w.TxStore.InsertTx(txmgrNs, txRec, nil) + err = walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { + return w.addRelevantTx(dbTx, txRec, nil) }) if err != nil { return err @@ -3323,15 +3322,6 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { _, err = server.SendRawTransaction(tx, false) switch { case err == nil: - switch w.chainClient.(type) { - // For neutrino we need to trigger adding relevant tx manually - // because for spv client - tx data isn't received from sync peer. - case *chain.NeutrinoClient: - return walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { - return w.addRelevantTx(tx, txRec, nil) - }) - } - return nil // The following are errors returned from btcd's mempool. @@ -3348,14 +3338,12 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { case strings.Contains(err.Error(), "Missing inputs"): fallthrough case strings.Contains(err.Error(), "already in block chain"): - // If the transaction was rejected, then we'll remove it from // the txstore, as otherwise, we'll attempt to continually // re-broadcast it, and the utxo state of the wallet won't be // accurate. dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey) - return w.TxStore.RemoveUnminedTx(txmgrNs, txRec) }) if dbErr != nil { From c125b59df49642db2c53a8004632e3e2bb32fa37 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 20 Sep 2018 18:59:02 -0700 Subject: [PATCH 2/3] wallet/wallet: refactor PublishTransaction to use unexported method In this commit, we refactor the logic outside of PublishTransaction into another unexported method. This will pave the road for unifying the logic between SendOutputs and PublishTransaction. --- wallet/wallet.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index e2ee0c8..d606351 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3299,9 +3299,14 @@ func (w *Wallet) SignTransaction(tx *wire.MsgTx, hashType txscript.SigHashType, // This function is unstable and will be removed once syncing code is moved out // of the wallet. func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { + _, err := w.publishTransaction(tx) + return err +} + +func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) { server, err := w.requireChainClient() if err != nil { - return err + return nil, err } // As we aim for this to be general reliable transaction broadcast API, @@ -3310,19 +3315,19 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { // set of records. txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now()) if err != nil { - return err + return nil, err } err = walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { return w.addRelevantTx(dbTx, txRec, nil) }) if err != nil { - return err + return nil, err } - _, err = server.SendRawTransaction(tx, false) + txid, err := server.SendRawTransaction(tx, false) switch { case err == nil: - return nil + return txid, nil // The following are errors returned from btcd's mempool. case strings.Contains(err.Error(), "spent"): @@ -3347,14 +3352,14 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { return w.TxStore.RemoveUnminedTx(txmgrNs, txRec) }) if dbErr != nil { - return fmt.Errorf("unable to broadcast tx: %v, "+ + return nil, fmt.Errorf("unable to broadcast tx: %v, "+ "unable to remove invalid tx: %v", err, dbErr) } - return err + return nil, err default: - return err + return nil, err } } From db51e8b8deb7e5f79d361c66a85177790a93c11a Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 20 Sep 2018 19:04:30 -0700 Subject: [PATCH 3/3] wallet/wallet: use publishTransaction within SendOutputs --- wallet/wallet.go | 66 ++++++------------------------------------------ 1 file changed, 8 insertions(+), 58 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index d606351..ff3f14d 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3086,74 +3086,24 @@ func (w *Wallet) TotalReceivedForAddr(addr btcutil.Address, minConf int32) (btcu func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, minconf int32, satPerKb btcutil.Amount) (*chainhash.Hash, error) { - chainClient, err := w.requireChainClient() - if err != nil { - return nil, err - } - + // Ensure the outputs to be created adhere to the network's consensus + // rules. for _, output := range outputs { - err = txrules.CheckOutput(output, satPerKb) - if err != nil { + if err := txrules.CheckOutput(output, satPerKb); err != nil { return nil, err } } - // Create transaction, replying with an error if the creation - // was not successful. + // Create the transaction and broadcast it to the network. The + // transaction will be added to the database in order to ensure that we + // continue to re-broadcast the transaction upon restarts until it has + // been confirmed. createdTx, err := w.CreateSimpleTx(account, outputs, minconf, satPerKb) if err != nil { return nil, err } - // Create transaction record and insert into the db. - rec, err := wtxmgr.NewTxRecordFromMsgTx(createdTx.Tx, time.Now()) - if err != nil { - log.Errorf("Cannot create record for created transaction: %v", err) - return nil, err - } - err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { - txmgrNs := tx.ReadWriteBucket(wtxmgrNamespaceKey) - err := w.TxStore.InsertTx(txmgrNs, rec, nil) - if err != nil { - return err - } - - if createdTx.ChangeIndex >= 0 { - err = w.TxStore.AddCredit(txmgrNs, rec, nil, uint32(createdTx.ChangeIndex), true) - if err != nil { - log.Errorf("Error adding change address for sent "+ - "tx: %v", err) - return err - } - } - return nil - }) - if err != nil { - return nil, err - } - - // TODO: The record already has the serialized tx, so no need to - // serialize it again. - txid, err := chainClient.SendRawTransaction(&rec.MsgTx, false) - switch { - case err == nil: - switch w.chainClient.(type) { - // For neutrino we need to trigger adding relevant tx manually - // because for spv client - tx data isn't received from sync peer. - case *chain.NeutrinoClient: - err := walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { - return w.addRelevantTx(tx, rec, nil) - }) - if err != nil { - return nil, err - } - } - - // TODO(roasbeef): properly act on rest of mapped errors - return txid, nil - default: - return nil, err - } + return w.publishTransaction(createdTx.Tx) } // SignatureError records the underlying error when validating a transaction