Merge bitcoindevkit/bdk#1090: fix(electrum): Don't ignore multiple coinbase txs

530ba36b07f2cddad90b844ef1a56cb27ee1cf74 ci: fix msrv dependency versions for reqest and h2 (Daniela Brozzoni)
7a359d5eef66e60e0ab604fb0d043c2db260252d fix(electrum): Don't ignore multiple coinbase txs (Daniela Brozzoni)

Pull request description:

  We would previously insert just one coinbase transaction in the database if we caught multiple in the same sync.
  When we sync with electrum, before committing to the database, we remove from the update conflicting transactions, using the `make_txs_consistent` function. This function considers two txs to be conflicting if they spend from the same outpoint - but every coinbase transaction spends from the same outpoint!
  Here we make sure to avoid filtering out coinbase transactions, by adding a check on the txid just before we do the filtering.

  Fixes #1051

  ### Changelog notice

  - Fix a bug when syncing coinbase utxos on electrum

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  notmandatory:
    tACK 530ba36b07f2cddad90b844ef1a56cb27ee1cf74

Tree-SHA512: ebbc6af86d4433ac4083212033a23f183d109641db345cc06ab4d506995ab71657761351c03772462ab4ff0d081226ecc97f1042490194aaf8661914cbeb72cb

.github/workflows/cont_integration.yml

@@ -72,6 +72,9 @@ jobs:
           cargo update -p cc --precise "1.0.81"
           cargo update -p rustls:0.21.6 --precise "0.21.1"
           cargo update -p flate2:1.0.27 --precise "1.0.26"
+          cargo update -p reqwest --precise "0.11.18"
+          cargo update -p h2 --precise "0.3.20"
+          cargo update -p rustls-webpki --precise "0.100.1"
       - name: Build
         run: cargo build --features ${{ matrix.features }} --no-default-features
       - name: Clippy
@@ -233,5 +236,8 @@ jobs:
         cargo update -p cc --precise "1.0.81"
         cargo update -p rustls:0.21.6 --precise "0.21.1"
         cargo update -p flate2:1.0.27 --precise "1.0.26"
+        cargo update -p reqwest --precise "0.11.18"
+        cargo update -p h2 --precise "0.3.20"
+        cargo update -p rustls-webpki --precise "0.100.1"
     - name: Test
       run: cargo test --features test-hardware-signer

README.md

@@ -229,4 +229,10 @@ cargo update -p cc --precise "1.0.81"
 cargo update -p rustls:0.21.6 --precise "0.21.1"
 # flate2 1.0.27 has MSRV 1.63.0+
 cargo update -p flate2:1.0.27 --precise "1.0.26"
+# reqwest 0.11.19 has MSRV 1.63.0+
+cargo update -p reqwest --precise "0.11.18"
+# h2 0.3.21 has MSRV 1.63.0+
+cargo update -p h2 --precise "0.3.20"
+# rustls-webpki 0.100.2 has MSRV 1.60+
+cargo update -p rustls-webpki --precise "0.100.2"
 ```

src/blockchain/script_sync.rs

@@ -9,7 +9,7 @@ use crate::{
     wallet::time::Instant,
     BlockTime, Error, KeychainKind, LocalUtxo, TransactionDetails,
 };
-use bitcoin::{OutPoint, Script, ScriptBuf, Transaction, TxOut, Txid};
+use bitcoin::{hashes::Hash, OutPoint, Script, ScriptBuf, Transaction, TxOut, Txid};
 use log::*;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque};
 
@@ -444,8 +444,14 @@ impl<'a, D: BatchDatabase> State<'a, D> {
 /// Remove conflicting transactions -- tie breaking them by fee.
 fn make_txs_consistent(txs: &[TransactionDetails]) -> Vec<&TransactionDetails> {
     let mut utxo_index: HashMap<OutPoint, &TransactionDetails> = HashMap::default();
+    let mut coinbase_txs = vec![];
     for tx in txs {
         for input in &tx.transaction.as_ref().unwrap().input {
+            if input.previous_output.txid == Txid::all_zeros() {
+                coinbase_txs.push(tx);
+                break;
+            }
+
             utxo_index
                 .entry(input.previous_output)
                 .and_modify(|existing| match (tx.fee, existing.fee) {
@@ -463,5 +469,6 @@ fn make_txs_consistent(txs: &[TransactionDetails]) -> Vec<&TransactionDetails> {
         .collect::<HashMap<_, _>>()
         .into_iter()
         .map(|(_, tx)| tx)
+        .chain(coinbase_txs)
         .collect()
 }

src/testutils/blockchain_tests.rs

@@ -1098,18 +1098,18 @@ macro_rules! bdk_blockchain_tests {
                 wallet.sync(&blockchain, SyncOptions::default()).unwrap();
                 assert_eq!(wallet.get_balance().unwrap().immature, 0, "incorrect balance");
 
-                test_client.generate(1, Some(wallet_addr));
+                test_client.generate(2, Some(wallet_addr));
 
                 wallet.sync(&blockchain, SyncOptions::default()).unwrap();
 
-                assert!(wallet.get_balance().unwrap().immature > 0, "incorrect balance after receiving coinbase");
+                assert_eq!(wallet.get_balance().unwrap().immature, 5000000000*2, "incorrect balance after receiving coinbase");
 
                 // make coinbase mature (100 blocks)
                 let node_addr = test_client.get_node_address(None);
                 test_client.generate(100, Some(node_addr));
                 wallet.sync(&blockchain, SyncOptions::default()).unwrap();
 
-                assert!(wallet.get_balance().unwrap().confirmed > 0, "incorrect balance after maturing coinbase");
+                assert_eq!(wallet.get_balance().unwrap().confirmed, 5000000000 * 2, "incorrect balance after maturing coinbase");
 
             }