Merge #395

395: Bail only if transaction does not make it on-chain r=luckysori a=luckysori

With bd15462af7 we no longer wrongly report an error when failing to broadcast a transaction because it's already on-chain. It's also a slight improvement imo.

Additionally, I tried to simplify this function with 3e8e6bc06e, which seems a lot better to me, but I could have missed something.

Co-authored-by: Lucas Soriano del Pino <l.soriano.del.pino@gmail.com>

daemon/src/wallet.rs

@@ -96,18 +96,10 @@ impl Wallet {
 
     pub async fn try_broadcast_transaction(&self, tx: Transaction) -> Result<Txid> {
         let wallet = self.wallet.lock().await;
-        // TODO: Optimize this match to be a map_err / more readable in general
         let txid = tx.txid();
 
         let result = wallet.broadcast(tx.clone());
 
-        if result.is_err() {
-            tracing::error!(
-                "Broadcasting transaction failed. Raw transaction: {}",
-                serialize_hex(&tx)
-            );
-        }
-
         if let Err(&bdk::Error::Electrum(electrum_client::Error::Protocol(ref value))) =
             result.as_ref()
         {
@@ -116,11 +108,21 @@ impl Wallet {
             })?;
 
             if error_code == i64::from(RpcErrorCode::RpcVerifyAlreadyInChain) {
+                tracing::trace!(
+                    %txid, "Attempted to broadcast transaction that was already on-chain",
+                );
+
                 return Ok(txid);
             }
         }
 
-        let txid = result?;
+        let txid = result.with_context(|| {
+            format!(
+                "Broadcasting transaction failed. Txid: {}. Raw transaction: {}",
+                txid,
+                serialize_hex(&tx)
+            )
+        })?;
 
         Ok(txid)
     }