wallet: group independent db writes on single batched db transaction

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.

DrahtBot added the label Wallet on Jun 7, 2022

DrahtBot commented at 1:02 am on June 8, 2022: contributor

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

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.

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.

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:

0                MarkConflicted(batch, /*hashBlock=*/prev->conflicting_block_hash, /*conflicting_height=*/prev->conflicting_block_height, /*hashTx=*/wtx.GetHash());

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:

0                            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.

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:

0                        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.

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:

0            return AddToWallet(batch, MakeTransactionRef(tx), /*state=*/tx_state, /*update_wtx=*/nullptr, rescanning_old_block);

w0xlt approved

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.

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).

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.

DrahtBot added the label Needs rebase on Jun 22, 2022

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).

furszy force-pushed on Jun 28, 2022

DrahtBot removed the label Needs rebase on Jun 28, 2022

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).

furszy force-pushed on Jun 30, 2022

maflcko referenced this in commit c892cb7d8d on Jun 30, 2022

sidhujag referenced this in commit f4b3a4a99b on Jun 30, 2022

furszy force-pushed on Jun 30, 2022

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

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

DrahtBot added the label Needs rebase on Jul 8, 2022

furszy force-pushed on Jul 11, 2022

DrahtBot removed the label Needs rebase on Jul 11, 2022

furszy force-pushed on Jul 11, 2022

DrahtBot added the label Needs rebase on Jul 11, 2022

furszy force-pushed on Jul 11, 2022

DrahtBot removed the label Needs rebase on Jul 11, 2022

DrahtBot added the label Needs rebase on Jul 12, 2022

furszy force-pushed on Jul 12, 2022

DrahtBot removed the label Needs rebase on Jul 12, 2022

DrahtBot added the label Needs rebase on Jul 19, 2022

furszy force-pushed on Jul 20, 2022

DrahtBot removed the label Needs rebase on Jul 20, 2022

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

DrahtBot added the label Needs rebase on Aug 2, 2022

furszy force-pushed on Aug 3, 2022

DrahtBot removed the label Needs rebase on Aug 3, 2022

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:

0./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).

  0-wallet_abandonconflict.py --descriptors                | ✓ Passed  | 54 s
  1+wallet_abandonconflict.py --descriptors                | ✓ Passed  | 42 s
  2-wallet_abandonconflict.py --legacy-wallet              | ✓ Passed  | 66 s
  3+wallet_abandonconflict.py --legacy-wallet              | ✓ Passed  | 53 s
  4-wallet_address_types.py --descriptors                  | ✓ Passed  | 142 s
  5+wallet_address_types.py --descriptors                  | ✓ Passed  | 84 s
  6-wallet_address_types.py --legacy-wallet                | ✓ Passed  | 316 s
  7+wallet_address_types.py --legacy-wallet                | ✓ Passed  | 239 s
  8-wallet_avoid_mixing_output_types.py --descriptors      | ✓ Passed  | 50 s
  9+wallet_avoid_mixing_output_types.py --descriptors      | ✓ Passed  | 42 s
 10-wallet_avoidreuse.py --descriptors                     | ✓ Passed  | 340 s
 11+wallet_avoidreuse.py --descriptors                     | ✓ Passed  | 241 s
 12-wallet_avoidreuse.py --legacy-wallet                   | ✓ Passed  | 694 s
 13+wallet_avoidreuse.py --legacy-wallet                   | ✓ Passed  | 443 s
 14-wallet_backup.py --descriptors                         | ✓ Passed  | 110 s
 15+wallet_backup.py --descriptors                         | ✓ Passed  | 83 s
 16-wallet_backup.py --legacy-wallet                       | ✓ Passed  | 341 s
 17+wallet_backup.py --legacy-wallet                       | ✓ Passed  | 239 s
 18-wallet_balance.py --descriptors                        | ✓ Passed  | 63 s
 19+wallet_balance.py --descriptors                        | ✓ Passed  | 30 s
 20-wallet_balance.py --legacy-wallet                      | ✓ Passed  | 85 s
 21+wallet_balance.py --legacy-wallet                      | ✓ Passed  | 66 s
 22-wallet_basic.py --legacy-wallet                        | ✓ Passed  | 455 s
 23+wallet_basic.py --legacy-wallet                        | ✓ Passed  | 349 s
 24-wallet_coinbase_category.py --descriptors              | ✓ Passed  | 19 s
 25+wallet_coinbase_category.py --descriptors              | ✓ Passed  | 34 s
 26-wallet_coinbase_category.py --legacy-wallet            | ✓ Passed  | 43 s
 27+wallet_coinbase_category.py --legacy-wallet            | ✓ Passed  | 25 s
 28-wallet_createwallet.py --descriptors                   | ✓ Passed  | 34 s
 29+wallet_createwallet.py --descriptors                   | ✓ Passed  | 23 s
 30-wallet_createwallet.py --legacy-wallet                 | ✓ Passed  | 62 s
 31+wallet_createwallet.py --legacy-wallet                 | ✓ Passed  | 36 s
 32-wallet_createwallet.py --usecli                        | ✓ Passed  | 66 s
 33+wallet_createwallet.py --usecli                        | ✓ Passed  | 46 s
 34-wallet_crosschain.py                                   | ✓ Passed  | 24 s
 35+wallet_crosschain.py                                   | ✓ Passed  | 16 s
 36-wallet_descriptor.py --descriptors                     | ✓ Passed  | 111 s
 37+wallet_descriptor.py --descriptors                     | ✓ Passed  | 78 s
 38-wallet_disable.py                                      | ✓ Passed  | 9 s
 39+wallet_disable.py                                      | ✓ Passed  | 6 s
 40-wallet_dump.py --legacy-wallet                         | ✓ Passed  | 96 s
 41+wallet_dump.py --legacy-wallet                         | ✓ Passed  | 66 s
 42-wallet_encryption.py --descriptors                     | ✓ Passed  | 16 s
 43+wallet_encryption.py --descriptors                     | ✓ Passed  | 11 s
 44-wallet_encryption.py --legacy-wallet                   | ✓ Passed  | 18 s
 45+wallet_encryption.py --legacy-wallet                   | ✓ Passed  | 12 s
 46-wallet_fallbackfee.py --descriptors                    | ✓ Passed  | 33 s
 47+wallet_fallbackfee.py --descriptors                    | ✓ Passed  | 37 s
 48-wallet_fallbackfee.py --legacy-wallet                  | ✓ Passed  | 37 s
 49+wallet_fallbackfee.py --legacy-wallet                  | ✓ Passed  | 39 s
 50-wallet_groups.py --descriptors                         | ✓ Passed  | 176 s
 51+wallet_groups.py --descriptors                         | ✓ Passed  | 161 s
 52-wallet_groups.py --legacy-wallet                       | ✓ Passed  | 200 s
 53+wallet_groups.py --legacy-wallet                       | ✓ Passed  | 190 s
 54-wallet_hd.py --legacy-wallet                           | ✓ Passed  | 209 s
 55+wallet_hd.py --legacy-wallet                           | ✓ Passed  | 112 s
 56-wallet_import_rescan.py --legacy-wallet                | ✓ Passed  | 971 s
 57+wallet_import_rescan.py --legacy-wallet                | ✓ Passed  | 619 s
 58-wallet_import_with_label.py --legacy-wallet            | ✓ Passed  | 27 s
 59+wallet_import_with_label.py --legacy-wallet            | ✓ Passed  | 25 s
 60-wallet_importprunedfunds.py --descriptors              | ✓ Passed  | 50 s
 61+wallet_importprunedfunds.py --descriptors              | ✓ Passed  | 23 s
 62-wallet_importprunedfunds.py --legacy-wallet            | ✓ Passed  | 50 s
 63+wallet_importprunedfunds.py --legacy-wallet            | ✓ Passed  | 26 s
 64-wallet_keypool.py --descriptors                        | ✓ Passed  | 24 s
 65+wallet_keypool.py --descriptors                        | ✓ Passed  | 14 s
 66-wallet_keypool.py --legacy-wallet                      | ✓ Passed  | 38 s
 67+wallet_keypool.py --legacy-wallet                      | ✓ Passed  | 20 s
 68-wallet_keypool_topup.py --descriptors                  | ✓ Passed  | 133 s
 69+wallet_keypool_topup.py --descriptors                  | ✓ Passed  | 69 s
 70-wallet_keypool_topup.py --legacy-wallet                | ✓ Passed  | 374 s
 71+wallet_keypool_topup.py --legacy-wallet                | ✓ Passed  | 152 s
 72-wallet_labels.py --descriptors                         | ✓ Passed  | 75 s
 73+wallet_labels.py --descriptors                         | ✓ Passed  | 27 s
 74-wallet_labels.py --legacy-wallet                       | ✓ Passed  | 141 s
 75+wallet_labels.py --legacy-wallet                       | ✓ Passed  | 60 s
 76-wallet_listdescriptors.py --descriptors                | ✓ Passed  | 13 s
 77+wallet_listdescriptors.py --descriptors                | ✓ Passed  | 9 s
 78-wallet_listreceivedby.py --descriptors                 | ✓ Passed  | 113 s
 79+wallet_listreceivedby.py --descriptors                 | ✓ Passed  | 66 s
 80-wallet_listsinceblock.py --descriptors                 | ✓ Passed  | 137 s
 81+wallet_listsinceblock.py --descriptors                 | ✓ Passed  | 81 s
 82-wallet_listsinceblock.py --legacy-wallet               | ✓ Passed  | 137 s
 83+wallet_listsinceblock.py --legacy-wallet               | ✓ Passed  | 93 s
 84-wallet_listtransactions.py --descriptors               | ✓ Passed  | 89 s
 85+wallet_listtransactions.py --descriptors               | ✓ Passed  | 68 s
 86-wallet_listtransactions.py --legacy-wallet             | ✓ Passed  | 118 s
 87+wallet_listtransactions.py --legacy-wallet             | ✓ Passed  | 89 s
 88-wallet_miniscript.py                                   | ✓ Passed  | 24 s
 89+wallet_miniscript.py                                   | ✓ Passed  | 18 s
 90-wallet_multiwallet.py --descriptors                    | ✓ Passed  | 208 s
 91+wallet_multiwallet.py --descriptors                    | ✓ Passed  | 144 s
 92-wallet_multiwallet.py --legacy-wallet                  | ✓ Passed  | 292 s
 93+wallet_multiwallet.py --legacy-wallet                  | ✓ Passed  | 214 s
 94-wallet_multiwallet.py --usecli                         | ✓ Passed  | 285 s
 95+wallet_multiwallet.py --usecli                         | ✓ Passed  | 217 s
 96-wallet_reorgsrestore.py                                | ✓ Passed  | 68 s
 97+wallet_reorgsrestore.py                                | ✓ Passed  | 60 s
 98-wallet_resendwallettransactions.py --descriptors       | ✓ Passed  | 36 s
 99+wallet_resendwallettransactions.py --descriptors       | ✓ Passed  | 13 s
100-wallet_resendwallettransactions.py --legacy-wallet     | ✓ Passed  | 33 s
101+wallet_resendwallettransactions.py --legacy-wallet     | ✓ Passed  | 24 s
102-wallet_send.py --descriptors                           | ✓ Passed  | 110 s
103+wallet_send.py --descriptors                           | ✓ Passed  | 73 s
104-wallet_send.py --legacy-wallet                         | ✓ Passed  | 228 s
105+wallet_send.py --legacy-wallet                         | ✓ Passed  | 138 s
106-wallet_sendall.py --legacy-wallet                      | ✓ Passed  | 148 s
107+wallet_sendall.py --legacy-wallet                      | ✓ Passed  | 68 s
108-wallet_signer.py --descriptors                         | ✓ Passed  | 36 s
109+wallet_signer.py --descriptors                         | ✓ Passed  | 26 s
110-wallet_signmessagewithaddress.py                       | ✓ Passed  | 13 s
111+wallet_signmessagewithaddress.py                       | ✓ Passed  | 10 s
112-wallet_startup.py                                      | ✓ Passed  | 61 s
113+wallet_startup.py                                      | ✓ Passed  | 47 s
114-wallet_taproot.py                                      | ✓ Passed  | 372 s
115+wallet_taproot.py                                      | ✓ Passed  | 239 s
116-wallet_timelock.py                                     | ✓ Passed  | 18 s
117+wallet_timelock.py                                     | ✓ Passed  | 17 s
118-wallet_transactiontime_rescan.py --legacy-wallet       | ✓ Passed  | 132 s
119+wallet_transactiontime_rescan.py --legacy-wallet       | ✓ Passed  | 91 s
120-wallet_txn_clone.py                                    | ✓ Passed  | 46 s
121+wallet_txn_clone.py                                    | ✓ Passed  | 44 s
122-wallet_importdescriptors.py --descriptors              | ✖ Failed  | 101 s
123+wallet_importdescriptors.py --descriptors              | ✓ Passed  | 78 s
124-wallet_importmulti.py --legacy-wallet                  | ✖ Failed  | 168 s
125+wallet_importmulti.py --legacy-wallet                  | ✓ Passed  | 146 s
126-wallet_listreceivedby.py --legacy-wallet               | ✖ Failed  | 162 s
127+wallet_listreceivedby.py --legacy-wallet               | ✓ Passed  | 57 s
128-wallet_multisig_descriptor_psbt.py                     | ✖ Failed  | 111 s
129+wallet_multisig_descriptor_psbt.py                     | ✓ Passed  | 36 s
130-wallet_signrawtransactionwithwallet.py --descriptors   | ✖ Failed  | 102 s
131+wallet_signrawtransactionwithwallet.py --descriptors   | ✓ Passed  | 46 s
132-wallet_signrawtransactionwithwallet.py --legacy-wallet | ✖ Failed  | 108 s
133+wallet_signrawtransactionwithwallet.py --legacy-wallet | ✓ Passed  | 55 s
134-wallet_txn_clone.py --segwit                           | ✓ Passed  | 42 s
135+wallet_txn_clone.py --segwit                           | ✓ Passed  | 38 s
136-wallet_txn_doublespend.py --descriptors                | ✓ Passed  | 44 s
137+wallet_txn_doublespend.py --descriptors                | ✓ Passed  | 25 s
138-wallet_txn_doublespend.py --legacy-wallet              | ✓ Passed  | 50 s
139+wallet_txn_doublespend.py --legacy-wallet              | ✓ Passed  | 30 s
140-wallet_txn_doublespend.py --mineblock                  | ✓ Passed  | 52 s
141+wallet_txn_doublespend.py --mineblock                  | ✓ Passed  | 43 s
142-wallet_watchonly.py --legacy-wallet                    | ✓ Passed  | 30 s
143+wallet_watchonly.py --legacy-wallet                    | ✓ Passed  | 21 s
144-wallet_watchonly.py --usecli --legacy-wallet           | ✓ Passed  | 29 s
145+wallet_watchonly.py --usecli --legacy-wallet           | ✓ Passed  | 23 s
146-wallet_upgradewallet.py --legacy-wallet                | ○ Skipped | 1 s
147+wallet_upgradewallet.py --legacy-wallet                | ○ Skipped | 0 s
148-wallet_basic.py --descriptors                          | ✖ Failed  | 181 s
149+wallet_basic.py --descriptors                          | ✓ Passed  | 291 s
150-wallet_bumpfee.py --descriptors                        | ✖ Failed  | 97 s
151+wallet_bumpfee.py --descriptors                        | ✖ Failed  | 202 s
152-wallet_bumpfee.py --legacy-wallet                      | ✖ Failed  | 258 s
153+wallet_bumpfee.py --legacy-wallet                      | ✓ Passed  | 279 s
154-wallet_create_tx.py --descriptors                      | ✖ Failed  | 99 s
155+wallet_create_tx.py --descriptors                      | ✖ Failed  | 92 s
156-wallet_create_tx.py --legacy-wallet                    | ✖ Failed  | 98 s
157+wallet_create_tx.py --legacy-wallet                    | ✖ Failed  | 96 s
158-wallet_hd.py --descriptors                             | ✖ Failed  | 100 s
159+wallet_hd.py --descriptors                             | ✓ Passed  | 80 s
160-wallet_sendall.py --descriptors                        | ✖ Failed  | 96 s
161+wallet_sendall.py --descriptors                        | ✓ Passed  | 49 s
162-wallet_transactiontime_rescan.py --descriptors         | ✖ Failed  | 116 s
163+wallet_transactiontime_rescan.py --descriptors         | ✖ Failed  | 119 s
164-wallet_orphanedreward.py                               | ✖ Failed  | 107 s
165+wallet_orphanedreward.py                               | ✖ Failed  | 100 s
166 
167-ALL                                                    | ✖ Failed  | 10720 s (accumulated)
168+ALL                                                    | ✖ Failed  | 7692 s (accumulated) 
169-Runtime: 1418 s
170+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.

DrahtBot added the label Needs rebase on Aug 5, 2022

furszy force-pushed on Aug 9, 2022

DrahtBot removed the label Needs rebase on Aug 9, 2022

DrahtBot added the label Needs rebase on Aug 10, 2022

furszy force-pushed on Aug 12, 2022

DrahtBot removed the label Needs rebase on Aug 12, 2022

furszy force-pushed on Aug 16, 2022

DrahtBot added the label Needs rebase on Aug 19, 2022

furszy force-pushed on Aug 20, 2022

DrahtBot removed the label Needs rebase on Aug 20, 2022

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

maflcko referenced this in commit 3c1e75ef60 on Aug 24, 2022

furszy force-pushed on Aug 24, 2022

sidhujag referenced this in commit 01e96c29e7 on Aug 24, 2022

DrahtBot added the label Needs rebase on Sep 1, 2022

furszy marked this as a draft on Oct 12, 2022

furszy force-pushed on Jan 10, 2023

DrahtBot removed the label Needs rebase on Jan 10, 2023

DrahtBot added the label Needs rebase on Jan 26, 2023

furszy force-pushed on Jan 26, 2023

DrahtBot removed the label Needs rebase on Jan 26, 2023

furszy force-pushed on Feb 16, 2023

DrahtBot added the label Needs rebase on Feb 17, 2023

furszy force-pushed on Feb 20, 2023

DrahtBot removed the label Needs rebase on Feb 20, 2023

DrahtBot added the label Needs rebase on Apr 2, 2023

furszy force-pushed on Apr 2, 2023

DrahtBot removed the label Needs rebase on Apr 2, 2023

DrahtBot added the label Needs rebase on Apr 12, 2023

wallet: refactor `AddToWallet`, add `WalletBatch` ref arg

Distinguishing clearly where 'AddToWallet' requires to flush db on close or not.

7a91da6875

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

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

wallet: refactor 'AddToSpends' to receive `WalletBatch` ref 3524a5e0e3

wallet: refactor 'IncOrderPosNext' to receive `WalletBatch` ref 88a94b6d7f

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

wallet: re-use existent WalletBatch for 'AddWalletFlags' and 'SetMinVersion' ae8df7ec37

wallet: dedup activeTxn->abort call from bdb Close() 93fcd248ff

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

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

wallet: provide WalletBatch to GetReservedDestination, ReserveKeyFromKeyPool, KeepDestination bdf2f47aeb

wallet: pass WalletBatch ref across MarkUnusedAddresses and TopUp methods e9019c1ade

wallet: where is possible, use 'SetAddressBookWithDB' with existent batch caeb7d0626

wallet: pass existent WalletBatch to every SetupDescriptorGeneration call

Instead of creating one internally.

32082624f8

wallet: pass existent WalletBatch ref to DescriptorScriptPubKeyMan::WriteDescriptor

Instead of creating one internally.

7df5a2deda

wallet: pass existent WalletBatch ref to AddActiveScriptPubKeyMan and DeactivateScriptPubKeyMan

And remove unimplemented DescriptorScriptPubKeyMan::SetupDescriptor method.

b603459057

wallet: pass WalletBatch ref to GetNewDestination and GetKeyFromPool 9255e3b715

wallet: pass WalletBatch ref to MarkReserveKeysAsUsed 6ced3faeb8

wallet: pass WalletBatch ref to TopUpChain and TopUpInactiveHDChain 48758a45cc

walletdb: implement generic Read for on-demand WalletBatch 14a7d41850

walletdb: add db txn capability to on-demand WalletBatch 7704e05bda

walletdb: set synchronized write mode if on-demand TxnBegin fails df9690e465

wallet: pass WalletBatch ref to LearnAllRelatedScripts and LearnRelatedScripts c4b366f224

furszy force-pushed on Apr 20, 2023

DrahtBot removed the label Needs rebase on Apr 21, 2023

DrahtBot added the label Needs rebase on May 15, 2023

DrahtBot commented at 12:01 pm on May 15, 2023: contributor

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

DrahtBot commented at 1:32 am on August 13, 2023: contributor

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.

DrahtBot commented at 1:49 am on November 11, 2023: contributor

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.

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.

furszy closed this on Nov 18, 2023

bitcoin locked this on Nov 17, 2024

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

Reviews

Conflicts