aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2024-02-15refactor: De-globalize g_signalsTheCharlatan
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-02-12wallet: simplify EraseRecords by using 'ErasePrefix'furszy
2024-02-12wallet: bdb batch 'ErasePrefix', do not create txn internallyfurszy
Transactions are intended to be started on upper layers rather than internally by the bdb batch object. This enables us to consolidate different write operations within a procedure in the same db txn, improving consistency due to the atomic property of the transaction, as well as its performance due to the reduction of disk write operations. Important Note: This approach also ensures that the BerkeleyBatch::ErasePrefix function behaves exactly as the SQLiteBatch::ErasePrefix function, which does not create a db txn internally. Furthermore, since the `BerkeleyBatch::ErasePrefix' implementation erases records one by one (by traversing the db), this change ensures that the function is always called within an active txn context. Without this measure, there's a potential risk to consistency; certain records may be removed while others could persist due to an internal failure during the procedure.
2024-02-12wallet: db, introduce 'RunWithinTxn()' helper functionfurszy
'RunWithinTxn()' provides a way to execute db operations within a transactional context. It avoids writing repetitive boilerplate code for starting and committing the database transaction.
2024-02-12Merge bitcoin/bitcoin#28987: wallet: simplify and batch zap wallet txes processAva Chow
9a3c5c8697659e34d0476103af942a4615818f4e scripted-diff: rename ZapSelectTx to RemoveTxs (furszy) 83b762845f5804f23b63526d403b2f327fe99632 wallet: batch and simplify ZapSelectTx process (furszy) 595d50a1032ad7ffa9945464c86aa57f16665e93 wallet: migration, remove extra NotifyTransactionChanged call (furszy) a2b071f9920c2f4893afcc43a152f593c03966bf wallet: ZapSelectTx, remove db rewrite code (furszy) Pull request description: Work decoupled from #28574. Brother of #28894. Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process. 1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`. 2) Instead of performing single write operations per removed tx record, this PR batches them all within a single atomic db txn. Moreover, these changes will enable us to consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future. ACKs for top commit: achow101: ACK 9a3c5c8697659e34d0476103af942a4615818f4e josibake: ACK https://github.com/bitcoin/bitcoin/pull/28987/commits/9a3c5c8697659e34d0476103af942a4615818f4e Tree-SHA512: fb2ecc48224c400ab3b1fbb32e174b5b13bf03794717727f80f01f55fb183883b067a68c0a127b2de8885564da15425d021a96541953bf38a72becc2e9929ccf
2024-02-10wallet, rpc: implement ancestor aware funding for sendallishaanam
2024-02-10rpc, test: test sendall spends unconfirmed change and unconfirmed inputs ↵ishaanam
when specified
2024-02-09Merge bitcoin/bitcoin#27877: wallet: Add CoinGrinder coin selection algorithmAva Chow
13161ecf032b7a850686e5942c12222c8f3d0d52 opt: Skip over barren combinations of tiny UTXOs (Murch) b7672c7cdd87acb105639f475744094b53cc9891 opt: Skip checking max_weight separately (Murch) 1edd2baa37a69ff69c2eaeceaad1028f1968cbab opt: Cut if last addition was minimal weight (Murch) 5248e2a60d243b3d5c77ecd9e4c335daca399a48 opt: Skip heavier UTXOs with same effective value (Murch) 9124c73742287b06dfe6e8a94be56ace25f07b2c opt: Tiebreak UTXOs by weight for CoinGrinder (Murch) 451be19dc10381122f705bbb2127b083f1d598c6 opt: Skip evaluation of equivalent input sets (Murch) 407b1e3432b77511900b77be84d5d7450352f462 opt: Track remaining effective_value in lookahead (Murch) 5f84f3cc043c5fb15072f5072fee752eaa01a2ec opt: Skip branches with worse weight (Murch) d68bc74fb2e3ae4ae775ab544fe5b4ab46025abb fuzz: Test optimality of CoinGrinder (Murch) 67df6c629a2e9878b06c1903e90b67087eaa3688 fuzz: Add CoinGrinder fuzz target (Murch) 1502231229dbc32c94e80a2fc2959275c5d246ef coinselection: Track whether CG completed (Murch) 7488acc64685ae16e20b78e7ad018137f27fe404 test: Add coin_grinder_tests (Murch) 6cc9a46cd0f4ed80d4523bbef1508142e0c80d27 coinselection: Add CoinGrinder algorithm (Murch) 89d09566431f57034d9a7df32547ceb13d79c62c opt: Tie-break UTXO sort by waste for BnB (Murch) aaee65823c6e620bef5cc96d8026567e64d822fe doc: Document max_weight on BnB (Murch) Pull request description: ***Please refer to the [topic on Delving Bitcoin](https://delvingbitcoin.org/t/gutterguard-and-coingrinder-simulation-results/279) discussing Gutter Guard/Coingrinder simulation results.*** Adds a coin selection algorithm that minimizes the weight of the input set while creating change. Motivations --- - At high feerates, using unnecessary inputs can significantly increase the fees - Users are upset when fees are relatively large compared to the amount sent - Some users struggle to maintain a sufficient count of UTXOs in their wallet Approach --- So far, Bitcoin Core has used a balanced approach to coin selection, where it will generate multiple input set candidates using various coin selection algorithms and pick the least wasteful among their results, but not explicitly minimize the input set weight. Under some circumstances, we _do_ want to minimize the weight of the input set. Sometimes changeless solutions require many or heavy inputs, and there is not always a changeless solution for Branch and Bound to find in the first place. This can cause expensive transactions unnecessarily. Given a wallet with sufficient funds, `CoinGrinder` will pick the minimal-waste input set for a transaction with a change output. The current implementation only runs `CoinGrinder` at feerates over 3×long-term-feerate-estimate (by default 30 ṩ/vB), which may be a decent compromise between our goal to reduce costs for the users, but still permit transactions at lower feerates to naturally reduce the wallet’s UTXO pool to curb bloat. Trade-offs --- Simulations for my thesis on coin selection ([see Section 6.3.2.1 [PDF]](https://murch.one/erhardt2016coinselection.pdf)) suggest that minimizing the input set for all transactions tends to grind a wallet’s UTXO pool to dust (pun intended): an approach selecting inputs per coin-age-priority (in effect similar to “largest first selection”) on average produced a UTXO pool with 15× the UTXO count as Bitcoin Core’s Knapsack-based Coin Selection then (in 2016). Therefore, I do not recommend running `CoinGrinder` under all circumstances, but only at extreme feerates or when we have another good reason to minimize the input set for other reasons. In the long-term, we should introduce additional metrics to score different input set candidates, e.g. on basis of their privacy and wallet health impact, to pick from all our coin selection results, but until then, we may want to limit use of `CoinGrinder` in other ways. ACKs for top commit: achow101: ACK 13161ecf032b7a850686e5942c12222c8f3d0d52 sr-gi: ACK [13161ec](https://github.com/bitcoin/bitcoin/pull/27877/commits/13161ecf032b7a850686e5942c12222c8f3d0d52) sipa: ACK 13161ecf032b7a850686e5942c12222c8f3d0d52 Tree-SHA512: 895b08b2ebfd0b71127949b7dba27146a6d10700bf8590402b14f261e7b937f4e2e1b24ca46de440c35f19349043ed2eba4159dc2aa3edae57721384186dae40
2024-02-09scripted-diff: rename ZapSelectTx to RemoveTxsfurszy
-BEGIN VERIFY SCRIPT- sed -i 's/ZapSelectTx/RemoveTxs/g' $(git grep -l 'ZapSelectTx' ./src/wallet) -END VERIFY SCRIPT-
2024-02-09wallet: batch and simplify ZapSelectTx processfurszy
The goal of the function is to erase the wallet transactions that match the inputted hashes. There is no need to traverse the database, reading record by record, to then perform single entry removals for each of them. To ensure consistency and improve performance, this change-set removes all tx records within a single atomic db batch operation, as well as it cleans up code, improves error handling and simplifies the transactions removal process entirely. This optimizes the removal of watch-only transactions during the wallet migration process and the 'removeprunedfunds' RPC command.
2024-02-09rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCsSebastian Falbesoner
This deduplicates code and leads to more consistent and detailed error messages. Affected are legacy import RPCs (`importpubkey`, `importmulti`) and other ones where solving data can be provided (`fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendall`).
2024-02-09opt: Skip over barren combinations of tiny UTXOsMurch
Given a lot of small amount UTXOs it is possible that the lookahead indicates sufficient funds, but any combination of them would push us beyond the current best_weight. We can estimate a lower bound for the minimal necessary weight to reach target from the maximal amount and minimal weight in the tail of the UTXO pool: if adding a number of hypothetical UTXOs of this maximum amount and minimum weight would not be able to beat `best_weight`, we can SHIFT to the omission branch, and CUT if the last selected UTXO is not heavier than the minimum weight of the remainder.
2024-02-09opt: Skip checking max_weight separatelyMurch
Initialize `best_selection_weight` as `max_weight` allows us to skip the separate `max_weight` check on every loop.
2024-02-09opt: Cut if last addition was minimal weightMurch
In situations where we have UTXO groups of various weight, we can CUT rather than SHIFT when we exceeded the max_weight or the best selection’s weight while the last step was equal to the minimum weight in the lookahead.
2024-02-09opt: Skip heavier UTXOs with same effective valueMurch
When two successive UTXOs differ in weight but match in effective value, we can skip the second if the first is not selected, because all input sets we can generate by swapping out a lighter UTXOs with a heavier UTXO of matching effective value would be strictly worse.
2024-02-09opt: Tiebreak UTXOs by weight for CoinGrinderMurch
2024-02-09opt: Skip evaluation of equivalent input setsMurch
When two successive UTXOs match in effective value and weight, we can skip the second if the prior is not selected: adding it would create an equivalent input set to a previously evaluated. E.g. if we have three UTXOs with effective values {5, 3, 3} of the same weight each, we want to evaluate {5, _, _}, {5, 3, _}, {5, 3, 3}, {_, 3, _}, {_, 3, 3}, but skip {5, _, 3}, and {_, _, 3}, because the first 3 is not selected, and we therefore do not need to evaluate the second 3 at the same position in the input set. If we reach the end of the branch, we must SHIFT the previously selected UTXO group instead.
2024-02-09opt: Track remaining effective_value in lookaheadMurch
Introduces a dedicated data structure to track the total effective_value available in the remaining UTXOs at each index of the UTXO pool. In contrast to the approach in BnB, this allows us to immediately jump to a lower index instead of visiting every UTXO to add back their eff_value to the lookahead.
2024-02-09opt: Skip branches with worse weightMurch
Once we exceed the weight of the current best selection, we can always shift as adding more inputs can never yield a better solution.
2024-02-09fuzz: Test optimality of CoinGrinderMurch
Co-authored-by: Pieter Wuille <pieter@wuille.net>
2024-02-09fuzz: Add CoinGrinder fuzz targetMurch
2024-02-09coinselection: Track whether CG completedMurch
CoinGrinder may not be able to exhaustively search all potentially interesting combinations for large UTXO pools, so we keep track of whether the search was terminated by the iteration limit.
2024-02-09test: Add coin_grinder_testsMurch
2024-02-09coinselection: Add CoinGrinder algorithmMurch
CoinGrinder is a DFS-based coin selection algorithm that deterministically finds the input set with the lowest weight creating a change output.
2024-02-08Merge bitcoin/bitcoin#26836: wallet: batch and simplify addressbook ↵Ava Chow
migration process 86960cdb7f75eaa2ae150914c54240d1d5ef96d1 wallet: migration, batch addressbook records removal (furszy) 342c45f80e32b0320829ce380b5854844cd74bc8 wallet: addressbook migration, batch db writes (furszy) 595bbe6e81885d35179aba6137dc63d0e652cc1f refactor: wallet, simplify addressbook migration (furszy) d0943315b1d00905fe7f4513b2f3f47b88a99e8f refactor: SetAddressBookWithDB, minimize number of map lookups (furszy) bba4f8dcb55de3ca4963711dc17882b43cb0bc4a refactor: SetAddrBookWithDB, signal only if write succeeded (furszy) 97b075392305becfbad4d497614478cff2d9237f wallet: clean redundancies in DelAddressBook (furszy) Pull request description: Commits decoupled from #28574, focused on the address book cloning process Includes: 1) DB batch operations and flow simplification for the address book migration process. 2) Code improvements to `CWallet::DelAddressBook` and `Wallet::SetAddrBookWithDB` methods. These changes will let us consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future. ACKs for top commit: achow101: ACK 86960cdb7f75eaa2ae150914c54240d1d5ef96d1 josibake: reACK https://github.com/bitcoin/bitcoin/commit/86960cdb7f75eaa2ae150914c54240d1d5ef96d1 Tree-SHA512: 10c941df3cd84fd8662b9c9ca6a1ed2c7402d38c677d2fc66b8b6c9edc6d73e827a5821487bbcacb5569d502934fa548fd10699e2ec45185f869e43174d8b2a1
2024-02-07Merge bitcoin/bitcoin#29112: sqlite: Disallow writing from multiple ↵Ryan Ofsky
`SQLiteBatch`s cfcb9b1ecf9be5267487713fa1e112ca09a1ae55 test: wallet, coverage for concurrent db transactions (furszy) 548ecd11553bea28bc79e6f9840836283e9c4e99 tests: Test for concurrent writes with db tx (Ava Chow) 395bcd245476d7c83abf2d82e31e90b6d0c432f3 sqlite: Ensure that only one SQLiteBatch is writing to db at a time (Ava Chow) Pull request description: The way that we have configured SQLite to run means that only one database transaction can be open at a time. Typically, each individual read and write operation will be its own transaction that is opened and committed automatically by SQLite. However, sometimes we want these operations to be batched into a multi-statement transaction, so `SQLiteBatch::TxnBegin`, `SQLiteBatch::TxnCommit`, and `SQLiteBatch::TxnAbort` are used to manage the transaction of the database. However, once a db transaction is begun with one `SQLiteBatch`, any operations performed by another `SQLiteBatch` will also occur within the same transaction. Furthermore, those other `SQLiteBatch`s will not be expecting a transaction to be active, and will abort it once the `SQLiteBatch` is destructed. This is problematic as it will prevent some data from being written, and also cause the `SQLiteBatch` that opened the transaction in the first place to be in an unexpected state and throw an error. To avoid this situation, we need to prevent the multiple batches from writing at the same time. To do so, I've implemented added a `CSemaphore` within `SQLiteDatabase` which will be used by any `SQLiteBatch` trying to do a write operation. `wait()` is called by `TxnBegin`, and at the beginning of `WriteKey`, `EraseKey`, and `ErasePrefix`. `post()` is called in `TxnCommit`, `TxnAbort` and at the end of `WriteKey`, `EraseKey`, and `ErasePrefix`. To avoid deadlocking on ` TxnBegin()` followed by a `WriteKey()`, `SQLiteBatch will now also track whether a transaction is in progress so that it knows whether to use the semaphore. This issue is not a problem for BDB wallets since BDB uses WAL and provides transaction objects that must be used if an operation is to occur within a transaction. Specifically, we either pass a transaction pointer, or a nullptr, to all BDB operations, and this allows for concurrent transactions so it doesn't have this problem. Fixes #29110 ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/29112/commits/cfcb9b1ecf9be5267487713fa1e112ca09a1ae55 furszy: ACK cfcb9b1ecf9be5267487713fa1e112ca09a1ae55 ryanofsky: Code review ACK cfcb9b1ecf9be5267487713fa1e112ca09a1ae55. This looks great and I think it is ready for merge. Just holding off because josibake seemed ready to review https://github.com/bitcoin/bitcoin/pull/29112#issuecomment-1930372190 and might have more feedback. Tree-SHA512: 2dd5a8e76df52451a40e0b8a87c7139d68a0d8e1bf2ebc79168cc313e192dab87cfa4270ff17fea4f7b370060d3bc9b5d294d50f7e07994d9b5a69b40397c927
2024-02-07wallet: migration, batch addressbook records removalfurszy
Instead of doing one db transaction per removed record, we now batch all removals in a single db transaction. Speeding up the process and preventing the wallet from entering an inconsistent state when any of the intermediate writes fail.
2024-02-07wallet: addressbook migration, batch db writesfurszy
Optimizing the process performance and consistency.
2024-02-07refactor: wallet, simplify addressbook migrationfurszy
Same process written in a cleaner manner. Removing code duplication.
2024-02-07refactor: SetAddressBookWithDB, minimize number of map lookupsfurszy
2024-02-07refactor: SetAddrBookWithDB, signal only if write succeededfurszy
2024-02-07wallet: clean redundancies in DelAddressBookfurszy
1) Encode destination only once (instead of three). 2) Fail if the entry's linked data cannot be removed. 3) Don't remove entry from memory if db write fail. 4) Notify GUI only if removal succeeded
2024-02-06Merge bitcoin/bitcoin#29388: fuzz: remove unused `args` and `context` from ↵Ryan Ofsky
`FuzzedWallet` b14298c5bca395e5ed6a27fe1758c0d1f4b824ac fuzz: remove unused `args` and `context` from `FuzzedWallet` (brunoerg) Pull request description: `ArgsManager args` and `WalletContext context` were previously used to create the wallet into `FuzzedWallet`. After fa15861763df71e788849b587883b3c16bb12229, they are not used anymore. This PR removes them. ACKs for top commit: maflcko: lgtm ACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac epiccurious: utACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac ryanofsky: Code review ACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac Tree-SHA512: 164e6a66ba05e11176a0cf68db6257f0ac07459cf7aa01ec4302b303c156c205a68128373a0b8daba0a6dfbff990af7fa14465a6341a296312fb20ea778c7a8c
2024-02-06Merge bitcoin/bitcoin#28833: wallet: refactor: remove unused `SignatureData` ↵Ava Chow
instances in spkm's `FillPSBT` methods e2ad343f69af4f924b22dccf94a52b6431ef6e3c wallet: remove unused `SignatureData` instances in spkm's `FillPSBT` methods (Sebastian Falbesoner) Pull request description: These are filled with signature data from a PSBT input, but not used anywhere after, hence they can be removed. Note that the same code is in the `SignPSBTInput` function where the `sigdata` result is indeed used. ACKs for top commit: achow101: ACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c brunoerg: crACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c Tree-SHA512: f0cabcc000bcea6bc7d7ec9d3be2e2a8accbdbffbe35252250ea2305b65a5813fde2b8096fbdd2c7cccdf417ea285183dc325fc2d210d88bce62978ce642930e
2024-02-06test: wallet, coverage for concurrent db transactionsfurszy
Verifying that a database handler can't commit/abort changes occurring in a different database handler.
2024-02-06sqlite: Ensure that only one SQLiteBatch is writing to db at a timeAva Chow
A SQLiteBatch need to wait for any other batch to finish writing before it can begin writing, otherwise db txn state may be incorrectly modified. To enforce this, each SQLiteDatabase has a semaphore which acts as a lock and is acquired by a batch when it begins a write, erase, or a transaction, and is released by it when it is done. To avoid deadlocking on itself for writing during a transaction, SQLiteBatch also keeps track of whether it has begun a transaction.
2024-02-05fuzz: remove unused `args` and `context` from `FuzzedWallet`brunoerg
2024-02-03wallet: remove unused 'accept_no_keys' arg from decryption processfurszy
The wallet decryption process (CheckDecryptionKey() and Unlock()) contains an arg 'accept_no_keys,' introduced in #13926, that has never been used. Additionally, this also removes the unimplemented SplitWalletPath function.
2024-02-02Merge bitcoin/bitcoin#28868: wallet: Fix migration of wallets with txs that ↵Ryan Ofsky
have both spendable and watchonly outputs 4da76ca24725eb9ba8122317e04a6e1ee14ac846 test: Test migration of tx with both spendable and watchonly (Ava Chow) c62a8d03a862fb124b4f4b88efd61978e46605f8 wallet: Keep txs that belong to both watchonly and migrated wallets (Ava Chow) 71cb28ea8cb579ac04cefc47a57557c94080d1af test: Make sure that migration test does not rescan on reloading (Ava Chow) 78ba0e6748d2a519a96c41dea851e7c43b82f251 wallet: Reload the wallet if migration exited early (Ava Chow) 9332c7edda79a39bb729b71b6f8db6a9d37343bb wallet: Write bestblock to watchonly and solvable wallets (Ava Chow) Pull request description: A transaction does not necessarily have to belong to either the migrated wallet (with the private keys) and the watchonly wallet (with watchonly things), it could have multiple outputs with each isminetype. So we should be putting such transactions in one or the other wallet, but rather putting it in both. I've added a test for this behavior, however the test also revealed a few other issues. Notably, it revealed that `migratewallet` would have the watchonly wallet rescan from genesis when it is reloaded at the end of migration. This could be a cause for migration appearing to be very slow. This is resolved by first writing best block records to the watchonly and solvable wallets, as well as updating the test to make sure that rescans don't happen. The change to avoid rescans also found an issue where some of our early exits would result in unloading the wallet even though nothing happened. So there is also a commit to reload the wallet for such early exits. ACKs for top commit: ryanofsky: Code review ACK 4da76ca24725eb9ba8122317e04a6e1ee14ac846. This looks great. The code is actually cleaner than before, two bugs are fixed, and the test checking for rescanning is pretty clever and broadens test coverage. furszy: Code review ACK 4da76ca2 Tree-SHA512: 5fc210cff16ca6720d7b2d0616d7e3f295c974147854abc704cf99a3bfaad17572ada084859e7a1b1ca94da647ad130303219678f429b7995f85e040236db35c
2024-02-01wallet: Make sure that the descriptors flag is set for blank walletsAva Chow
2024-02-01wallet: Keep txs that belong to both watchonly and migrated walletsAva Chow
It is possible for a transaction that has an output that belongs to the mgirated wallet, and another output that belongs to the watchonly wallet. Such transaction should appear in both wallets during migration.
2024-02-01wallet: Reload the wallet if migration exited earlyAva Chow
Migration will unload loaded wallets prior to beginning. It will then perform some checks which may exit early. Such unloaded wallets should be reloaded prior to exiting.
2024-02-01wallet: Write bestblock to watchonly and solvable walletsAva Chow
When migrating, we should also be writing the bestblock record to the watchonly and solvable wallets to avoid rescanning on the reload as that can be slow.
2024-01-31Merge bitcoin/bitcoin#28976: wallet: Fix migration of blank walletsRyan Ofsky
c11c404281d2d0e22967e30e16c0733db84f4eee tests: Test migration of blank wallets (Andrew Chow) 563b2a60d6a371f26474410397da435547e58786 wallet: Better error message when missing LegacySPKM during migration (Andrew Chow) b1d2c771d43b802db12768e620588ed179e92b29 wallet: Check for descriptors flag before migration (Andrew Chow) 8c127ff1edb6b9a607bf1ad247893347252a38e3 wallet: Skip key and script migration for blank wallets (Andrew Chow) Pull request description: Blank wallets (wallets without any keys are scripts) are being detected as already being descriptor wallets even though they are not. This is because the check for whether a wallet is already a descriptor wallet uses the presence of a `LegacyScriptPubKeyMan` which is only setup when keys or scripts are found. This PR resolves this issue by checking for the descriptor wallet flag instead and subsequently skipping the keys and scripts part of migration for blank wallets. Fixes the issue mentioned in https://github.com/bitcoin/bitcoin/pull/28868#issuecomment-1809641110 ACKs for top commit: furszy: reACK c11c404281d2d0e22967e30e16c0733db84f4eee. CI failure is unrelated. ryanofsky: Code review ACK c11c404281d2d0e22967e30e16c0733db84f4eee Tree-SHA512: 2466fdf1542eb8489c841253191f85dc88365493f0bb3395b67dee3e43709a9993c68b9d7623657b54b779adbe68fc81962d60efef4802c5d461f154167af7f4
2024-01-31Merge bitcoin/bitcoin#29253: wallet: guard against dangling to-be-reverted ↵Ava Chow
db transactions b298242c8d495c36072415e1b95eaa7bf485a38a test: sqlite, add coverage for dangling to-be-reverted db txns (furszy) fc0e747192e98e779c5f31f2df808f62b3fdd071 sqlite: guard against dangling to-be-reverted db transactions (furszy) 472d2ca98170049e0edec830e2d11c5ef23740a4 sqlite: introduce HasActiveTxn method (furszy) dca874e838c2599bd24433675b291168f8e7b055 sqlite: add ability to interrupt statements (furszy) fdf9f66909a354a95f4b7c5f092f0e9fbe1baa7c test: wallet db, exercise deadlock after write failure (furszy) Pull request description: Discovered while was reviewing #29112, specifically https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1821862931. If the db handler that initiated the database transaction is destroyed, the ongoing transaction cannot be left dangling when the db txn fails to abort. It must be forcefully reverted; otherwise, any subsequent db handler executing a write operation will dump the dangling, to-be-reverted transaction data to disk. This not only breaks the isolation property but also results in the improper storage of incomplete information on disk, impacting the wallet consistency. This PR fixes the issue by resetting the db connection, automatically rolling back the transaction (per https://www.sqlite.org/c3ref/close.html) when the handler object is being destroyed and the txn abortion failed. Testing Notes Can verify the failure by reverting the fix e5217fea and running the test. It will fail without e5217fea and pass with it. ACKs for top commit: achow101: ACK b298242c8d495c36072415e1b95eaa7bf485a38a ryanofsky: Code review ACK b298242c8d495c36072415e1b95eaa7bf485a38a. Just fix for exec result codes and comment update since last review. Tree-SHA512: 44ba0323ab21440e79e9d7791bc1c56a8873c8bd3e8f6a85641b91576e1293011fa8032d8ae5b0580f3fb7a949356f7b9676693d7ceffa617aaad9f6569993eb
2024-01-30test: sqlite, add coverage for dangling to-be-reverted db txnsfurszy
2024-01-30sqlite: guard against dangling to-be-reverted db transactionsfurszy
If the handler that initiated the database transaction is destroyed, the ongoing transaction cannot be left dangling when the db txn fails to abort. It must be forcefully reversed; otherwise, any subsequent db handler executing a write operation will dump the dangling, to-be-reverted transaction data to disk. This not only breaks the database isolation property but also results in the improper storage of incomplete information on disk, impacting the wallet consistency.
2024-01-30sqlite: introduce HasActiveTxn methodfurszy
Util function to clean up code and let us verify, in the following-up commit, that dangling, to-be-reverted db transactions cannot occur anymore.
2024-01-30sqlite: add ability to interrupt statementsfurszy
By encapsulating sqlite3_exec into its own standalone method and introducing the 'SQliteExecHandler' class, we enable the ability to test db statements execution failures within the unit test framework. This is used in the following-up commit to exercise a deadlock and improve our wallet db error handling code. Moreover, the future encapsulation of other sqlite functions within this class will contribute to minimize the impact of any future API changes.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 06:14:43 +0000