wallet: request notification of tx confirmation that pays to relevant addr

In this commit, we address a slight regression within the wallet
that was introduced in a previous commit. When attempting to send coins
on-chain, we would never ask the chain backend to notify us of the
transaction upon confirmation. This, along with the rebroadcast of
unconfirmed transactions logic, would result in the wallet becoming out
of sync with the chain.

Below is an example of how this could have happened:

  1. Send funds on-chain.
  2. Wallet doesn't ask to be notified of the confirmation.
  3. Since the wallet is not notified of the confirmation, the
  transaction remains in the unconfirmed bucket, even though it might
  have already confirmed on-chain.
  4. Restart and trigger the rebroadcast of unconfirmed transactions.
  5. The unconfirmed transaction is removed from the unconfirmed bucket
  due to it already existing on-chain, without it being moved to the
  confirmed bucket. Moving to the confirmed bucket would require the
  block at which it confirmed, which we don't have at this point.

wallet/createtx.go

@@ -170,6 +170,21 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32,
 			" %v from imported account into default account.", changeAmount)
 	}
 
+	// Finally, we'll request the backend to notify us of the transaction
+	// that pays to the change address, if there is one, when it confirms.
+	if tx.ChangeIndex >= 0 {
+		changePkScript := tx.Tx.TxOut[tx.ChangeIndex].PkScript
+		_, addrs, _, err := txscript.ExtractPkScriptAddrs(
+			changePkScript, w.chainParams,
+		)
+		if err != nil {
+			return nil, err
+		}
+		if err := chainClient.NotifyReceived(addrs); err != nil {
+			return nil, err
+		}
+	}
+
 	return tx, nil
 }
 

wallet/wallet.go

@@ -2964,6 +2964,11 @@ func (w *Wallet) NewChangeAddress(account uint32,
 	return addr, nil
 }
 
+// newChangeAddress returns a new change address for the wallet.
+//
+// NOTE: This method requires the caller to use the backend's NotifyReceived
+// method in order to detect when an on-chain transaction pays to the address
+// being created.
 func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket,
 	account uint32) (btcutil.Address, error) {
 
@@ -3317,7 +3322,7 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error {
 // from the database (along with cleaning up all inputs used, and outputs
 // created) if the transaction is rejected by the back end.
 func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
-	server, err := w.requireChainClient()
+	chainClient, err := w.requireChainClient()
 	if err != nil {
 		return nil, err
 	}
@@ -3337,7 +3342,33 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 		return nil, err
 	}
 
-	txid, err := server.SendRawTransaction(tx, false)
+	// We'll also ask to be notified of the transaction once it confirms
+	// on-chain. This is done outside of the database transaction to prevent
+	// backend interaction within it.
+	//
+	// NOTE: In some cases, it's possible that the transaction to be
+	// broadcast is not directly relevant to the user's wallet, e.g.,
+	// multisig. In either case, we'll still ask to be notified of when it
+	// confirms to maintain consistency.
+	//
+	// TODO(wilmer): import script as external if the address does not
+	// belong to the wallet to handle confs during restarts?
+	for _, txOut := range tx.TxOut {
+		_, addrs, _, err := txscript.ExtractPkScriptAddrs(
+			txOut.PkScript, w.chainParams,
+		)
+		if err != nil {
+			// Non-standard outputs can safely be skipped because
+			// they're not supported by the wallet.
+			continue
+		}
+
+		if err := chainClient.NotifyReceived(addrs); err != nil {
+			return nil, err
+		}
+	}
+
+	txid, err := chainClient.SendRawTransaction(tx, false)
 	switch {
 	case err == nil:
 		return txid, nil