wallet: group independent db writes on single batched db transaction #25297

pull furszy wants to merge 23 commits into bitcoin:master from furszy:2022_wallet_unified_dbbatch changing 14 files +262 −188
  1. furszy commented at 6:55 PM on June 7, 2022: member

    The block connection process, same as many other wallet processes, contains plenty individual db writes.

    This PR batches all the db transaction that occurs along each wallet workflow. Dumping all the information to disk at once atomically at the end of the process.

    Then, for BDB, fixed places where we are flushing to db directly on individual writes. e.g we do it in the in the chain sync/scan process, when an output that belongs to the wallet is found if the address is not inside the address book.


    Plus, in several places across the wallet flows, we create new WalletBatch objects. Which, internally, mean: Increasing the db references number, try to open the db and, for SQLite, setup and bind the statements. This PR avoids all this overhead by sharing the same db handler instance through each entire workflow.

  2. DrahtBot added the label Wallet on Jun 7, 2022
  3. DrahtBot commented at 1:02 AM on June 8, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Stale ACK w0xlt

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #bitcoin-core/gui/650 (Add Import to Wallet GUI by KolbyML)
    • #27601 (wallet: don't duplicate change output if already exist by furszy)
    • #27469 (wallet: improve IBD sync time by skipping block scanning prior birth time by furszy)
    • #27307 (wallet: track mempool conflicts with wallet transactions by ishaanam)
    • #27286 (wallet: Keep track of the wallet's own transaction outputs in memory by achow101)
    • #27145 (wallet: when a block is disconnected, update transactions that are no longer conflicted by ishaanam)
    • #26840 (refactor: importpubkey, importprivkey, importaddress, importmulti, and importdescriptors rpc by KolbyML)
    • #26728 (wallet: Have the wallet store the key for automatically generated descriptors by achow101)
    • #26715 (Introduce MockableDatabase for wallet unit tests by achow101)
    • #26627 (wallet: Migrate non-HD keys with single combo containing a list of keys by achow101)
    • #26008 (wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets by achow101)
    • #25907 (wallet: rpc to add automatically generated descriptors by achow101)
    • #25881 (wallet: remove unused DummySignTx and CKeyPool from GetReservedDestination by furszy)
    • #24914 (wallet: Load database records in a particular order by achow101)
    • #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
    • #22341 (rpc: add getxpub by Sjors)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. in src/wallet/wallet.cpp:1169 in 843b987865 outdated 
    1076 |          auto it = mapWallet.find(txin.prevout.hash);
1077 |          if (it != mapWallet.end()) {
1078 |              CWalletTx& prevtx = it->second;
1079 |              if (auto* prev = prevtx.state<TxStateConflicted>()) {
1080 | -                MarkConflicted(prev->conflicting_block_hash, prev->conflicting_block_height, wtx.GetHash());
1081 | +                MarkConflicted(batch, prev->conflicting_block_hash, prev->conflicting_block_height, wtx.GetHash());
    w0xlt commented at 4:17 PM on June 10, 2022:

    nit:

                    MarkConflicted(batch, /*hashBlock=*/prev->conflicting_block_hash, /*conflicting_height=*/prev->conflicting_block_height, /*hashTx=*/wtx.GetHash());
  5. in src/wallet/wallet.cpp:1226 in 843b987865 outdated 
    1129 | @@ -1130,7 +1130,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
1130 |                          // (e.g. it wasn't generated on this node or we're restoring from backup)
1131 |                          // add it to the address book for proper transaction accounting
1132 |                          if (!*dest.internal && !FindAddressBookEntry(dest.dest, /* allow_change= */ false)) {
1133 | -                            SetAddressBook(dest.dest, "", "receive");
1134 | +                            SetAddressBookWithDB(batch, dest.dest, "", "receive");
    w0xlt commented at 4:20 PM on June 10, 2022:

    nit:

                                SetAddressBookWithDB(batch, /*address=*/dest.dest, /*strName=*/"", /*strPurpose=*/"receive");
    furszy commented at 5:12 PM on June 10, 2022:

    👍🏼, will do if have to retouch the area.

  6. in src/wallet/wallet.cpp:1187 in 843b987865 outdated 
    1096 |                  std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(txin.prevout);
1097 |                  while (range.first != range.second) {
1098 |                      if (range.first->second != tx.GetHash()) {
1099 |                          WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), conf->confirmed_block_hash.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n);
1100 | -                        MarkConflicted(conf->confirmed_block_hash, conf->confirmed_block_height, range.first->second);
1101 | +                        MarkConflicted(batch, conf->confirmed_block_hash, conf->confirmed_block_height, range.first->second);
    w0xlt commented at 4:21 PM on June 10, 2022:

    nit:

                            MarkConflicted(batch, /*hashBlock=*/conf->confirmed_block_hash, /*conflicting_height=*/conf->confirmed_block_height, /*hashTx=*/range.first->second);
    furszy commented at 5:09 PM on June 10, 2022:

    Probably better to do a const auto& txid = range.first->second; above the if block (the same value is accessed multiple times).

    Will do it if have to retouch the area.

  7. in src/wallet/wallet.cpp:1128 in 843b987865 outdated 
    1138 | @@ -1139,7 +1139,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
1139 |              // Block disconnection override an abandoned tx as unconfirmed
1140 |              // which means user may have to call abandontransaction again
1141 |              TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state);
1142 | -            return AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
1143 | +            return AddToWallet(batch, MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, rescanning_old_block);
    w0xlt commented at 4:22 PM on June 10, 2022:

    nit:

                return AddToWallet(batch, MakeTransactionRef(tx), /*state=*/tx_state, /*update_wtx=*/nullptr, rescanning_old_block);
  8. w0xlt approved
  9. w0xlt commented at 4:24 PM on June 10, 2022: contributor

    Code Review ACK https://github.com/bitcoin/bitcoin/pull/25297/commits/843b987865d8020da584962c67f31238b2ccf864 These changes optimize and simplify the code.

    Some nits. Feel free to ignore them.

  10. furszy commented at 5:04 PM on June 10, 2022: member

    Thanks for the review w0xlt!

    About the nits: I don't think that the arguments names are needed in most of those cases, there isn't a real benefit adding them, the variables have a clear name that represent what they are (except one).

  11. achow101 commented at 8:44 PM on June 13, 2022: member

    Flush on close is probably not quite the performance improvement that you think it is. While it is an improvement for BDB, with SQLite, the argument is ignored. SQLite flushes after each write operation regardless. It would be better to use transactions with TxnBegin and TxnCommit as these do actually batch the operations into single atomic writes.

    Otherwise, I think this is a good idea to be reusing the batches.

  12. DrahtBot added the label Needs rebase on Jun 22, 2022
  13. furszy commented at 8:04 PM on June 23, 2022: member

    Thanks for the feedback 👌🏼

    It would be better to use transactions with TxnBegin and TxnCommit as these do actually batch the operations into single atomic writes.

    Almost there, been working on it :).

    Otherwise, I think this is a good idea to be reusing the batches.

    Have another branch with more of them (chained to the batched operations work). I'll probably end up decoupling some of them into another PR (otherwise this will end up being pretty big).

  14. furszy force-pushed on Jun 28, 2022
  15. furszy force-pushed on Jun 28, 2022
  16. DrahtBot removed the label Needs rebase on Jun 28, 2022
  17. furszy commented at 11:53 PM on June 28, 2022: member

    Update:

    Pushed the first step toward the implementation of db write batched operations for entire processes (single atomic writes).

    Basically, is the utilization of the same db handler across all the methods, whom internally write to db, that are called during a specific process workflow (only the parent method who starts the process will construct the WalletBatch).

    In order to provide the WalletBatch reference, without knowing if the underlying method/s will use it or not, implemented the capability to construct an uninitialized WalletBatch, who will be automatically initialized on the first write call. If no db write method is called, the object will remain uninitialized for the entire process workflow without consuming any extra resource.

    Note: I haven't updated the PR description yet. Will do it once the work is finished (still have more to push).

  18. furszy force-pushed on Jun 30, 2022
  19. maflcko referenced this in commit c892cb7d8d on Jun 30, 2022
  20. sidhujag referenced this in commit f4b3a4a99b on Jun 30, 2022
  21. furszy force-pushed on Jun 30, 2022
  22. furszy renamed this:
    wallet: speedup transactions sync, rescan and load not flushing to db constantly
    WIP, walXlet speedup transactions sync, rescan and load not flushing to db constantly
    on Jul 1, 2022
  23. furszy renamed this:
    WIP, walXlet speedup transactions sync, rescan and load not flushing to db constantly
    WIP, wallet: speedup transactions sync, rescan and load not flushing to db constantly
    on Jul 1, 2022
  24. DrahtBot added the label Needs rebase on Jul 8, 2022
  25. furszy force-pushed on Jul 11, 2022
  26. DrahtBot removed the label Needs rebase on Jul 11, 2022
  27. furszy force-pushed on Jul 11, 2022
  28. DrahtBot added the label Needs rebase on Jul 11, 2022
  29. furszy force-pushed on Jul 11, 2022
  30. DrahtBot removed the label Needs rebase on Jul 11, 2022
  31. DrahtBot added the label Needs rebase on Jul 12, 2022
  32. furszy force-pushed on Jul 12, 2022
  33. DrahtBot removed the label Needs rebase on Jul 12, 2022
  34. DrahtBot added the label Needs rebase on Jul 19, 2022
  35. furszy force-pushed on Jul 20, 2022
  36. DrahtBot removed the label Needs rebase on Jul 20, 2022
  37. furszy renamed this:
    WIP, wallet: speedup transactions sync, rescan and load not flushing to db constantly
    wallet: speedup transactions sync, rescan and load by grouping all independent db writes on a single batched db transaction
    on Jul 20, 2022
  38. DrahtBot added the label Needs rebase on Aug 2, 2022
  39. furszy force-pushed on Aug 3, 2022
  40. DrahtBot removed the label Needs rebase on Aug 3, 2022
  41. furszy commented at 1:01 AM on August 3, 2022: member

    Have gotten some pretty interesting numbers :)

    Simply compared this PR (tip 9806250) vs master (tip de3c46c9) running all the wallet functional tests on a M1 Pro:

    Command:

    ./test/functional/test_runner.py test/functional/wallet* --jobs 9

    Green --> this PR (9806250). Red --> master (de3c46c9).

    (note: all the failing test cases are due the OSX timeout issue, which this PR seems to be improving).

    -wallet_abandonconflict.py --descriptors                | ✓ Passed  | 54 s
+wallet_abandonconflict.py --descriptors                | ✓ Passed  | 42 s
-wallet_abandonconflict.py --legacy-wallet              | ✓ Passed  | 66 s
+wallet_abandonconflict.py --legacy-wallet              | ✓ Passed  | 53 s
-wallet_address_types.py --descriptors                  | ✓ Passed  | 142 s
+wallet_address_types.py --descriptors                  | ✓ Passed  | 84 s
-wallet_address_types.py --legacy-wallet                | ✓ Passed  | 316 s
+wallet_address_types.py --legacy-wallet                | ✓ Passed  | 239 s
-wallet_avoid_mixing_output_types.py --descriptors      | ✓ Passed  | 50 s
+wallet_avoid_mixing_output_types.py --descriptors      | ✓ Passed  | 42 s
-wallet_avoidreuse.py --descriptors                     | ✓ Passed  | 340 s
+wallet_avoidreuse.py --descriptors                     | ✓ Passed  | 241 s
-wallet_avoidreuse.py --legacy-wallet                   | ✓ Passed  | 694 s
+wallet_avoidreuse.py --legacy-wallet                   | ✓ Passed  | 443 s
-wallet_backup.py --descriptors                         | ✓ Passed  | 110 s
+wallet_backup.py --descriptors                         | ✓ Passed  | 83 s
-wallet_backup.py --legacy-wallet                       | ✓ Passed  | 341 s
+wallet_backup.py --legacy-wallet                       | ✓ Passed  | 239 s
-wallet_balance.py --descriptors                        | ✓ Passed  | 63 s
+wallet_balance.py --descriptors                        | ✓ Passed  | 30 s
-wallet_balance.py --legacy-wallet                      | ✓ Passed  | 85 s
+wallet_balance.py --legacy-wallet                      | ✓ Passed  | 66 s
-wallet_basic.py --legacy-wallet                        | ✓ Passed  | 455 s
+wallet_basic.py --legacy-wallet                        | ✓ Passed  | 349 s
-wallet_coinbase_category.py --descriptors              | ✓ Passed  | 19 s
+wallet_coinbase_category.py --descriptors              | ✓ Passed  | 34 s
-wallet_coinbase_category.py --legacy-wallet            | ✓ Passed  | 43 s
+wallet_coinbase_category.py --legacy-wallet            | ✓ Passed  | 25 s
-wallet_createwallet.py --descriptors                   | ✓ Passed  | 34 s
+wallet_createwallet.py --descriptors                   | ✓ Passed  | 23 s
-wallet_createwallet.py --legacy-wallet                 | ✓ Passed  | 62 s
+wallet_createwallet.py --legacy-wallet                 | ✓ Passed  | 36 s
-wallet_createwallet.py --usecli                        | ✓ Passed  | 66 s
+wallet_createwallet.py --usecli                        | ✓ Passed  | 46 s
-wallet_crosschain.py                                   | ✓ Passed  | 24 s
+wallet_crosschain.py                                   | ✓ Passed  | 16 s
-wallet_descriptor.py --descriptors                     | ✓ Passed  | 111 s
+wallet_descriptor.py --descriptors                     | ✓ Passed  | 78 s
-wallet_disable.py                                      | ✓ Passed  | 9 s
+wallet_disable.py                                      | ✓ Passed  | 6 s
-wallet_dump.py --legacy-wallet                         | ✓ Passed  | 96 s
+wallet_dump.py --legacy-wallet                         | ✓ Passed  | 66 s
-wallet_encryption.py --descriptors                     | ✓ Passed  | 16 s
+wallet_encryption.py --descriptors                     | ✓ Passed  | 11 s
-wallet_encryption.py --legacy-wallet                   | ✓ Passed  | 18 s
+wallet_encryption.py --legacy-wallet                   | ✓ Passed  | 12 s
-wallet_fallbackfee.py --descriptors                    | ✓ Passed  | 33 s
+wallet_fallbackfee.py --descriptors                    | ✓ Passed  | 37 s
-wallet_fallbackfee.py --legacy-wallet                  | ✓ Passed  | 37 s
+wallet_fallbackfee.py --legacy-wallet                  | ✓ Passed  | 39 s
-wallet_groups.py --descriptors                         | ✓ Passed  | 176 s
+wallet_groups.py --descriptors                         | ✓ Passed  | 161 s
-wallet_groups.py --legacy-wallet                       | ✓ Passed  | 200 s
+wallet_groups.py --legacy-wallet                       | ✓ Passed  | 190 s
-wallet_hd.py --legacy-wallet                           | ✓ Passed  | 209 s
+wallet_hd.py --legacy-wallet                           | ✓ Passed  | 112 s
-wallet_import_rescan.py --legacy-wallet                | ✓ Passed  | 971 s
+wallet_import_rescan.py --legacy-wallet                | ✓ Passed  | 619 s
-wallet_import_with_label.py --legacy-wallet            | ✓ Passed  | 27 s
+wallet_import_with_label.py --legacy-wallet            | ✓ Passed  | 25 s
-wallet_importprunedfunds.py --descriptors              | ✓ Passed  | 50 s
+wallet_importprunedfunds.py --descriptors              | ✓ Passed  | 23 s
-wallet_importprunedfunds.py --legacy-wallet            | ✓ Passed  | 50 s
+wallet_importprunedfunds.py --legacy-wallet            | ✓ Passed  | 26 s
-wallet_keypool.py --descriptors                        | ✓ Passed  | 24 s
+wallet_keypool.py --descriptors                        | ✓ Passed  | 14 s
-wallet_keypool.py --legacy-wallet                      | ✓ Passed  | 38 s
+wallet_keypool.py --legacy-wallet                      | ✓ Passed  | 20 s
-wallet_keypool_topup.py --descriptors                  | ✓ Passed  | 133 s
+wallet_keypool_topup.py --descriptors                  | ✓ Passed  | 69 s
-wallet_keypool_topup.py --legacy-wallet                | ✓ Passed  | 374 s
+wallet_keypool_topup.py --legacy-wallet                | ✓ Passed  | 152 s
-wallet_labels.py --descriptors                         | ✓ Passed  | 75 s
+wallet_labels.py --descriptors                         | ✓ Passed  | 27 s
-wallet_labels.py --legacy-wallet                       | ✓ Passed  | 141 s
+wallet_labels.py --legacy-wallet                       | ✓ Passed  | 60 s
-wallet_listdescriptors.py --descriptors                | ✓ Passed  | 13 s
+wallet_listdescriptors.py --descriptors                | ✓ Passed  | 9 s
-wallet_listreceivedby.py --descriptors                 | ✓ Passed  | 113 s
+wallet_listreceivedby.py --descriptors                 | ✓ Passed  | 66 s
-wallet_listsinceblock.py --descriptors                 | ✓ Passed  | 137 s
+wallet_listsinceblock.py --descriptors                 | ✓ Passed  | 81 s
-wallet_listsinceblock.py --legacy-wallet               | ✓ Passed  | 137 s
+wallet_listsinceblock.py --legacy-wallet               | ✓ Passed  | 93 s
-wallet_listtransactions.py --descriptors               | ✓ Passed  | 89 s
+wallet_listtransactions.py --descriptors               | ✓ Passed  | 68 s
-wallet_listtransactions.py --legacy-wallet             | ✓ Passed  | 118 s
+wallet_listtransactions.py --legacy-wallet             | ✓ Passed  | 89 s
-wallet_miniscript.py                                   | ✓ Passed  | 24 s
+wallet_miniscript.py                                   | ✓ Passed  | 18 s
-wallet_multiwallet.py --descriptors                    | ✓ Passed  | 208 s
+wallet_multiwallet.py --descriptors                    | ✓ Passed  | 144 s
-wallet_multiwallet.py --legacy-wallet                  | ✓ Passed  | 292 s
+wallet_multiwallet.py --legacy-wallet                  | ✓ Passed  | 214 s
-wallet_multiwallet.py --usecli                         | ✓ Passed  | 285 s
+wallet_multiwallet.py --usecli                         | ✓ Passed  | 217 s
-wallet_reorgsrestore.py                                | ✓ Passed  | 68 s
+wallet_reorgsrestore.py                                | ✓ Passed  | 60 s
-wallet_resendwallettransactions.py --descriptors       | ✓ Passed  | 36 s
+wallet_resendwallettransactions.py --descriptors       | ✓ Passed  | 13 s
-wallet_resendwallettransactions.py --legacy-wallet     | ✓ Passed  | 33 s
+wallet_resendwallettransactions.py --legacy-wallet     | ✓ Passed  | 24 s
-wallet_send.py --descriptors                           | ✓ Passed  | 110 s
+wallet_send.py --descriptors                           | ✓ Passed  | 73 s
-wallet_send.py --legacy-wallet                         | ✓ Passed  | 228 s
+wallet_send.py --legacy-wallet                         | ✓ Passed  | 138 s
-wallet_sendall.py --legacy-wallet                      | ✓ Passed  | 148 s
+wallet_sendall.py --legacy-wallet                      | ✓ Passed  | 68 s
-wallet_signer.py --descriptors                         | ✓ Passed  | 36 s
+wallet_signer.py --descriptors                         | ✓ Passed  | 26 s
-wallet_signmessagewithaddress.py                       | ✓ Passed  | 13 s
+wallet_signmessagewithaddress.py                       | ✓ Passed  | 10 s
-wallet_startup.py                                      | ✓ Passed  | 61 s
+wallet_startup.py                                      | ✓ Passed  | 47 s
-wallet_taproot.py                                      | ✓ Passed  | 372 s
+wallet_taproot.py                                      | ✓ Passed  | 239 s
-wallet_timelock.py                                     | ✓ Passed  | 18 s
+wallet_timelock.py                                     | ✓ Passed  | 17 s
-wallet_transactiontime_rescan.py --legacy-wallet       | ✓ Passed  | 132 s
+wallet_transactiontime_rescan.py --legacy-wallet       | ✓ Passed  | 91 s
-wallet_txn_clone.py                                    | ✓ Passed  | 46 s
+wallet_txn_clone.py                                    | ✓ Passed  | 44 s
-wallet_importdescriptors.py --descriptors              | ✖ Failed  | 101 s
+wallet_importdescriptors.py --descriptors              | ✓ Passed  | 78 s
-wallet_importmulti.py --legacy-wallet                  | ✖ Failed  | 168 s
+wallet_importmulti.py --legacy-wallet                  | ✓ Passed  | 146 s
-wallet_listreceivedby.py --legacy-wallet               | ✖ Failed  | 162 s
+wallet_listreceivedby.py --legacy-wallet               | ✓ Passed  | 57 s
-wallet_multisig_descriptor_psbt.py                     | ✖ Failed  | 111 s
+wallet_multisig_descriptor_psbt.py                     | ✓ Passed  | 36 s
-wallet_signrawtransactionwithwallet.py --descriptors   | ✖ Failed  | 102 s
+wallet_signrawtransactionwithwallet.py --descriptors   | ✓ Passed  | 46 s
-wallet_signrawtransactionwithwallet.py --legacy-wallet | ✖ Failed  | 108 s
+wallet_signrawtransactionwithwallet.py --legacy-wallet | ✓ Passed  | 55 s
-wallet_txn_clone.py --segwit                           | ✓ Passed  | 42 s
+wallet_txn_clone.py --segwit                           | ✓ Passed  | 38 s
-wallet_txn_doublespend.py --descriptors                | ✓ Passed  | 44 s
+wallet_txn_doublespend.py --descriptors                | ✓ Passed  | 25 s
-wallet_txn_doublespend.py --legacy-wallet              | ✓ Passed  | 50 s
+wallet_txn_doublespend.py --legacy-wallet              | ✓ Passed  | 30 s
-wallet_txn_doublespend.py --mineblock                  | ✓ Passed  | 52 s
+wallet_txn_doublespend.py --mineblock                  | ✓ Passed  | 43 s
-wallet_watchonly.py --legacy-wallet                    | ✓ Passed  | 30 s
+wallet_watchonly.py --legacy-wallet                    | ✓ Passed  | 21 s
-wallet_watchonly.py --usecli --legacy-wallet           | ✓ Passed  | 29 s
+wallet_watchonly.py --usecli --legacy-wallet           | ✓ Passed  | 23 s
-wallet_upgradewallet.py --legacy-wallet                | ○ Skipped | 1 s
+wallet_upgradewallet.py --legacy-wallet                | ○ Skipped | 0 s
-wallet_basic.py --descriptors                          | ✖ Failed  | 181 s
+wallet_basic.py --descriptors                          | ✓ Passed  | 291 s
-wallet_bumpfee.py --descriptors                        | ✖ Failed  | 97 s
+wallet_bumpfee.py --descriptors                        | ✖ Failed  | 202 s
-wallet_bumpfee.py --legacy-wallet                      | ✖ Failed  | 258 s
+wallet_bumpfee.py --legacy-wallet                      | ✓ Passed  | 279 s
-wallet_create_tx.py --descriptors                      | ✖ Failed  | 99 s
+wallet_create_tx.py --descriptors                      | ✖ Failed  | 92 s
-wallet_create_tx.py --legacy-wallet                    | ✖ Failed  | 98 s
+wallet_create_tx.py --legacy-wallet                    | ✖ Failed  | 96 s
-wallet_hd.py --descriptors                             | ✖ Failed  | 100 s
+wallet_hd.py --descriptors                             | ✓ Passed  | 80 s
-wallet_sendall.py --descriptors                        | ✖ Failed  | 96 s
+wallet_sendall.py --descriptors                        | ✓ Passed  | 49 s
-wallet_transactiontime_rescan.py --descriptors         | ✖ Failed  | 116 s
+wallet_transactiontime_rescan.py --descriptors         | ✖ Failed  | 119 s
-wallet_orphanedreward.py                               | ✖ Failed  | 107 s
+wallet_orphanedreward.py                               | ✖ Failed  | 100 s
 
-ALL                                                    | ✖ Failed  | 10720 s (accumulated)
+ALL                                                    | ✖ Failed  | 7692 s (accumulated) 
-Runtime: 1418 s
+Runtime: 984 s

    If someone else wants to try it, more result are very welcome :D.

    Would be great to have someone, who has a big wallet, trying to sync-up the blockchain from scratch (with and without this PR) and share the results. Will keep working on making a specific test for this meanwhile.

  42. DrahtBot added the label Needs rebase on Aug 5, 2022
  43. furszy force-pushed on Aug 9, 2022
  44. DrahtBot removed the label Needs rebase on Aug 9, 2022
  45. DrahtBot added the label Needs rebase on Aug 10, 2022
  46. furszy force-pushed on Aug 12, 2022
  47. DrahtBot removed the label Needs rebase on Aug 12, 2022
  48. furszy force-pushed on Aug 16, 2022
  49. DrahtBot added the label Needs rebase on Aug 19, 2022
  50. furszy force-pushed on Aug 20, 2022
  51. DrahtBot removed the label Needs rebase on Aug 20, 2022
  52. furszy renamed this:
    wallet: speedup transactions sync, rescan and load by grouping all independent db writes on a single batched db transaction
    wallet: group independent db writes on single batched db transaction
    on Aug 22, 2022
  53. maflcko referenced this in commit 3c1e75ef60 on Aug 24, 2022
  54. furszy force-pushed on Aug 24, 2022
  55. sidhujag referenced this in commit 01e96c29e7 on Aug 24, 2022
  56. DrahtBot added the label Needs rebase on Sep 1, 2022
  57. furszy marked this as a draft on Oct 12, 2022
  58. furszy force-pushed on Jan 10, 2023
  59. DrahtBot removed the label Needs rebase on Jan 10, 2023
  60. DrahtBot added the label Needs rebase on Jan 26, 2023
  61. furszy force-pushed on Jan 26, 2023
  62. DrahtBot removed the label Needs rebase on Jan 26, 2023
  63. furszy force-pushed on Feb 16, 2023
  64. DrahtBot added the label Needs rebase on Feb 17, 2023
  65. furszy force-pushed on Feb 20, 2023
  66. DrahtBot removed the label Needs rebase on Feb 20, 2023
  67. DrahtBot added the label Needs rebase on Apr 2, 2023
  68. furszy force-pushed on Apr 2, 2023
  69. DrahtBot removed the label Needs rebase on Apr 2, 2023
  70. DrahtBot added the label Needs rebase on Apr 12, 2023
  71. wallet: refactor `AddToWallet`, add `WalletBatch` ref arg
    Distinguishing clearly where 'AddToWallet' requires to flush db on close or not.
    7a91da6875
  72. wallet: speedup `AddToWalletIfInvolvingMe`, not flush to db on close for addressbook accounting
    Instead of calling `SetAddressBook`, which creates a `WalletBatch` that flushes on close, use `SetAddressBookWithDB` and provide a `WalletBatch` ref that does not flush on close.
    ee69a07c86
  73. wallet: speedup `LoadToWallet`, not flush to db on close inside `AddToSpends`
    Pass an existent reference of `WalletBatch`, who does not flush to db on close.
    868dd390cc
  74. wallet: refactor 'AddToSpends' to receive `WalletBatch` ref 3524a5e0e3
  75. wallet: refactor 'IncOrderPosNext' to receive `WalletBatch` ref 88a94b6d7f
  76. wallet: pass 'WalletBatch' to 'CWallet::LoadToWallet'
    So we stop creating and re-creating internal wallet batch objects
every time that a new transaction is loaded into the wallet.
    74bbfd7da6
  77. wallet: re-use existent WalletBatch for 'AddWalletFlags' and 'SetMinVersion' ae8df7ec37
  78. wallet: dedup activeTxn->abort call from bdb Close() 93fcd248ff
  79. wallet: introduce init on-demand capability for 'WalletBatch'
    Add `initialize` flag to 'WalletBatch' constructor.

If "initialize=false" the handler will not open nor initialize the db on the constructor.

Users can create the handler object beforehand and pass it across different functions as ref.
Allowing an entire process to use the same handler, perform batched operations and
avoid db access acquisition if no write occurs.

The db access will, only, be initialized before the first write/erase operation.
    269759b618
  80. wallet: make 'SyncTransaction' receive a 'WalletBatch' arg
    So it can be re-used in time-long scenarios like whole chain scan/rescan
and walk-through a block vtx for connection/disconnection where we are
creating a new 'WalletBatch' on every single operation.

Example: new tx write, addressbook record storage, addressbook "mark used"
update, mark tx conflicted, update spent output etc.
    d8345dd04a
  81. wallet: provide WalletBatch to GetReservedDestination, ReserveKeyFromKeyPool, KeepDestination bdf2f47aeb
  82. wallet: pass WalletBatch ref across MarkUnusedAddresses and TopUp methods e9019c1ade
  83. wallet: where is possible, use 'SetAddressBookWithDB' with existent batch caeb7d0626
  84. wallet: pass existent WalletBatch to every SetupDescriptorGeneration call
    Instead of creating one internally.
    32082624f8
  85. wallet: pass existent WalletBatch ref to DescriptorScriptPubKeyMan::WriteDescriptor
    Instead of creating one internally.
    7df5a2deda
  86. wallet: pass existent WalletBatch ref to AddActiveScriptPubKeyMan and DeactivateScriptPubKeyMan
    And remove unimplemented DescriptorScriptPubKeyMan::SetupDescriptor method.
    b603459057
  87. wallet: pass WalletBatch ref to GetNewDestination and GetKeyFromPool 9255e3b715
  88. wallet: pass WalletBatch ref to MarkReserveKeysAsUsed 6ced3faeb8
  89. wallet: pass WalletBatch ref to TopUpChain and TopUpInactiveHDChain 48758a45cc
  90. walletdb: implement generic Read for on-demand WalletBatch 14a7d41850
  91. walletdb: add db txn capability to on-demand WalletBatch 7704e05bda
  92. walletdb: set synchronized write mode if on-demand TxnBegin fails df9690e465
  93. wallet: pass WalletBatch ref to LearnAllRelatedScripts and LearnRelatedScripts c4b366f224
  94. furszy force-pushed on Apr 20, 2023
  95. DrahtBot removed the label Needs rebase on Apr 21, 2023
  96. DrahtBot added the label Needs rebase on May 15, 2023
  97. DrahtBot commented at 12:01 PM on May 15, 2023: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

  98. DrahtBot commented at 1:32 AM on August 13, 2023: contributor

    <!--13523179cfe9479db18ec6c5d236f789-->

    There hasn't been much activity lately and the patch still needs rebase. What is the status here?

    • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
    • Is it no longer relevant? ➡️ Please close.
    • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
  99. DrahtBot commented at 1:49 AM on November 11, 2023: contributor

    <!--13523179cfe9479db18ec6c5d236f789-->

    There hasn't been much activity lately and the patch still needs rebase. What is the status here?

    • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
    • Is it no longer relevant? ➡️ Please close.
    • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
  100. furszy commented at 6:12 PM on November 18, 2023: member

    Still relevant and quite useful but closing it until #28574 and its derivatives are merged.

  101. furszy closed this on Nov 18, 2023
  102. bitcoin locked this on Nov 17, 2024
  103. sedited commented at 11:01 AM on April 22, 2026: contributor

    Might want to re-open this again now?

Contributors
furszyDrahtBotw0xltachow101sedited
Labels
WalletNeeds rebase
Linked (view graph)
#25383 wallet: don't read db every time that a new 'WalletBatch' is created#25865 test: speedup wallet tests by whitelisting peers (immediate tx relay)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-01 03:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me