aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2023-05-30Merge bitcoin/bitcoin#27666: wallet, bench: Move commonly used functions to ↵fanquake
their own file and fix a bug 7379a54ec416c8c0a029cc41835a23d42cb6d800 bench: Remove incorrect LoadWallet call in WalletBalance (Andrew Chow) 846b2fe67ed76a678770d343153acedadfdacd0b tests: Move ADDRESS_BCRT1_UNSPENDABLE to wallet/test/util.h (Andrew Chow) c61d3f02f5122b38ea8bf0029aa9dfbbf38e10d0 tests, bench: Consolidate {Test,Bench}Un/LoadWallet helper (Andrew Chow) Pull request description: I have a few PRs and branches that use these two commits, probably makes sense to split them into a separate PR to be merged sooner. The first commit contains some things that end up being commonly used in new wallet benchmarks. These are moved into `wallet_common.{h/cpp}`. The second commit contains a bugfix for the wallet_balance benchmark where it calls `LoadWallet` in the wrong place. It's unnecessary to call that function in this benchmark. Although this does not cause any issues currently, it ends up causing issues in some PRs and branches that I'm working on. ACKs for top commit: Sjors: utACK 7379a54ec416c8c0a029cc41835a23d42cb6d800 furszy: ACK 7379a54 Tree-SHA512: 47773887a16c69ac7121c699d3446a8c399bd792a6a31714998b7b7a19fea179c6d3b29cb898b04397b2962c1b4120d57009352b8460b8283e188d4cb480c9ba
2023-05-30Merge bitcoin/bitcoin#27774: refactor: Add [[nodiscard]] where ignoring a ↵fanquake
Result return type is an error fa5680b7520c340952239c4e25ebe505d16c7c39 fix includes for touched header files (iwyu) (MarcoFalke) dddde27f6fbcff7cdb31f7138efc5d8363537b03 Add [[nodiscard]] where ignoring a Result return type is an error (MarcoFalke) Pull request description: Only add it for those where it is an error to ignore. Also, fix the gcc compile warning https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1564350880. Also, fix iwyu for touched header files. ACKs for top commit: TheCharlatan: ACK fa5680b7520c340952239c4e25ebe505d16c7c39 stickies-v: ACK fa5680b7520c340952239c4e25ebe505d16c7c39 Tree-SHA512: c3509103bfeae246e2cf565bc561fcd68d8118515bac5431ba5304c3a63c8254b9c4f40e268b6f6d6b79405675c5a960db9b4eb3bdd14aedca333dc1c9e76415
2023-05-30Merge bitcoin/bitcoin#27636: kernel: Remove util/system from kernel library, ↵fanquake
interface_ui from validation. 7d3b35004b039f2bd606bb46a540de7babdbc41e refactor: Move system from util to common library (TheCharlatan) 7eee356c0a7fefd70c8de21689efa335f52a69ba refactor: Split util::AnyPtr into its own file (TheCharlatan) 44de325d95447498036479c3112ba741caf45bf6 refactor: Split util::insert into its own file (TheCharlatan) 9ec5da36b62276ae22e348f26f88aaf646357d6d refactor: Move ScheduleBatchPriority to its own file (TheCharlatan) f871c69191dfe1331861ebcdbadb6bd47e45c8b1 kernel: Add warning method to notifications (TheCharlatan) 4452707edec91c7d7991f486dd41ef3edb4f7fbf kernel: Add progress method to notifications (TheCharlatan) 84d71457e7250ab25c0a11d1ad1c7657197ffd90 kernel: Add headerTip method to notifications (TheCharlatan) 447761c8228d58f948aae7e73ed079c028cacb97 kernel: Add notification interface (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- It removes the kernel library's dependency on `util/system` and `interface_ui`. `util/system` contains networking and shell-related code that should not be part of the kernel library. The following pull requests prepared `util/system` for this final step: https://github.com/bitcoin/bitcoin/pull/27419 https://github.com/bitcoin/bitcoin/pull/27254 https://github.com/bitcoin/bitcoin/pull/27238. `interface_ui` defines functions for a more general node interface and has a dependency on `boost/signals2`. After applying the patches from this pull request, the kernel's reliance on boost is down to `boost::multiindex`. The approach implemented here introduces some indirection, which makes the code a bit harder to read. Any suggestions for improving or reworking this pull request to make it more concise, or even reworking it into a more proper interface, are appreciated. ACKs for top commit: MarcoFalke: re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e (no change) 🎋 stickies-v: Code Review ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e hebasto: re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review. Tree-SHA512: c8cfc698dc9d78e20191c444708f2d957501229abe95e5806106d1126fb9c5fbcee686fb55645658c0107ce71f10646f37a2fdf7fde16bbf22cbf1ac885dd08d
2023-05-29Merge bitcoin/bitcoin#27759: Fix `#include`s in `src/wallet`fanquake
1f97572b9c0d339a8497340e7066050aba9d7694 Fix `#include`s in `src/wallet` (Hennadii Stepanov) Pull request description: This PR is a minimum required changes to fix https://github.com/bitcoin/bitcoin/pull/27571#discussion_r1195497290. ACKs for top commit: MarcoFalke: lgtm ACK 1f97572b9c0d339a8497340e7066050aba9d7694 Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
2023-05-29Add [[nodiscard]] where ignoring a Result return type is an errorMarcoFalke
2023-05-27Merge bitcoin/bitcoin#27145: wallet: when a block is disconnected, update ↵Andrew Chow
transactions that are no longer conflicted 89df7987c2f1eea42454c2b0efc31a924fbfd3a8 Add wallets_conflicts (Antoine Riard) dced203162d45e542f187f8d0d07dab85c52cf28 wallet, tests: mark unconflicted txs as inactive (ishaanam) 096487c4dcfadebe5ca959927f5426cae1c304d5 wallet: introduce generic recursive tx state updating function (ishaanam) Pull request description: This implements a fix for #7315. Previously when a block was disconnected any transactions that were conflicting with transactions mined in that block were not updated to be marked as inactive. The fix implemented here is described on the [Bitcoin DevWiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking#idea-refresh-conflicted). A test which tested the previous behavior has also been updated. Second attempt at #17543 ACKs for top commit: achow101: ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8 rajarshimaitra: tACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8. glozow: ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8 furszy: Tested ACK 89df7987 Tree-SHA512: 3133b151477e8818302fac29e96de30cd64c09a8fe3a7612074a34ba1a332e69148162e6cb3f1074d9d9c9bab5ef9996967d325d8c4c99ba42b5fe3b66a60546
2023-05-25tests: Move ADDRESS_BCRT1_UNSPENDABLE to wallet/test/util.hAndrew Chow
This static address is usable for other wallet tests and benchmarks, so make it available to them.
2023-05-25tests, bench: Consolidate {Test,Bench}Un/LoadWallet helperAndrew Chow
The wallet tests and benchmarks both had helper functions for loading and unloading the wallet for the test that were almost identical. These functions are consolidated and reused.
2023-05-25Fix `#include`s in `src/wallet`Hennadii Stepanov
2023-05-25wallet: skip block scan if block was created before wallet birthdayfurszy
To avoid wasting processing power, we can skip blocks that occurred before the wallet's creation time, since these blocks are guaranteed not to contain any relevant wallet data. This has direct implications (an speed improvement) on the underlying blockchain synchronization process as well. The reason is that the validation interface queue is limited to 10 tasks per time. This means that no more than 10 blocks can be waiting for the wallet(s) to be processed while we are synchronizing the chain (activating the best chain to be more precise). Which can be a bottleneck if blocks arrive and are processed faster from the network than what they are processed by the wallet(s).
2023-05-25refactor: single method to append new spkm to the walletfurszy
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
2023-05-20refactor: Split util::AnyPtr into its own fileTheCharlatan
2023-05-20refactor: Split util::insert into its own fileTheCharlatan
2023-05-18Merge bitcoin/bitcoin#27556: wallet: fix deadlock in bdb read write operationAndrew Chow
69d43905b7f1d4849dfaea1b5744ee473ccc8744 test: add coverage for wallet read write db deadlock (furszy) 12daf6fcdcbf5c7f03038338d843df3877714b24 walletdb: scope bdb::EraseRecords under a single db txn (furszy) 043fcb0b053eee6021cc75e3d3f41097f52698c0 wallet: bugfix, GetNewCursor() misses to provide batch ptr to BerkeleyCursor (furszy) Pull request description: Decoupled from #26644 so it can closed in favor of #26715. Basically, with bdb, we can't make a write operation while we are traversing the db with the same db handler. These two operations are performed in different txn contexts and cause a deadlock. Added coverage by using `EraseRecords()` which is the simplest function that executes this process. To replicate it, need bdb support and drop the first commit. The test will run forever. ACKs for top commit: achow101: ACK 69d43905b7f1d4849dfaea1b5744ee473ccc8744 hebasto: re-ACK 69d43905b7f1d4849dfaea1b5744ee473ccc8744 Tree-SHA512: b3773be78925f674e962f4a5c54b398a9d0cfe697148c01c3ec0d68281cc5c1444b38165960d219ef3cf1a57c8ce6427f44a876275958d49bbc0808486e19d7d
2023-05-15walletdb: Remove unused CreateMockWalletDatabaseAndrew Chow
This has been superseded by the MockableDatabase. Remove to avoid confusion as to which type of mock database to use for testing.
2023-05-15test: add coverage for wallet read write db deadlockfurszy
2023-05-15walletdb: scope bdb::EraseRecords under a single db txnfurszy
so we erase all the records atomically or abort the entire procedure. and, at the same time, we can share the same db txn context for the db cursor and the erase functionality. extra note from the Db.cursor doc: "If transaction protection is enabled, cursors must be opened and closed within the context of a transaction" thus why added a `CloseCursor` call before calling to `TxnAbort/TxnCommit`.
2023-05-15wallet: bugfix, GetNewCursor() misses to provide batch ptr to BerkeleyCursorfurszy
If the batch ptr is not passed, the cursor will not use the db active txn context which could lead to a deadlock if the code tries to modify the db while it is traversing it. E.g. the 'EraseRecords()' function.
2023-05-15Merge bitcoin/bitcoin#26715: Introduce `MockableDatabase` for wallet unit testsfanquake
33e2b82a4fc990253ff77655f437c7aed336bc55 wallet, bench: Remove unused database options from WalletBenchLoading (Andrew Chow) 80ace042d8fece9be50bfef1be64c6e5720e87e6 tests: Modify records directly in wallet ckey loading test (Andrew Chow) b3bb17d5d07f51ac2e501e4a7a3bbcd17144070f tests: Update DuplicateMockDatabase for MockableDatabase (Andrew Chow) f0eecf5e408238c64b77b0a4974ba2b9edb17487 scripted-diff: Replace CreateMockWalletDB with CreateMockableWalletDB (Andrew Chow) 075962bc25a90661612fe4613cd50ea1cae21f52 wallet, tests: Include wallet/test/util.h (Andrew Chow) 14aa4cb1e44f089a6022a2b14a98bca4a7dd9a01 wallet: Move DummyDatabase to salvage (Andrew Chow) f67a385556c60b2e4788a378196a395fca0539f5 wallet, tests: Replace usage of dummy db with mockable db (Andrew Chow) 33c6245ac1ecdfe25b1ee4fd9e93c43393634ae3 Introduce MockableDatabase for wallet unit tests (Andrew Chow) Pull request description: For the wallet's unit tests, we currently use either `DummyDatabase` or memory-only versions of either BDB or SQLite. The tests that use `DummyDatabase` just need a `WalletDatabase` so that the `CWallet` can be constructed, while the tests using the memory-only databases just need a backing data store. There is also a `FailDatabase` that is similar to `DummyDatabase` except it fails be default or can have a configured return value. Having all of these different database types can make it difficult to write tests, particularly tests that work when either BDB or SQLite is disabled. This PR unifies all of these different unit test database classes into a single `MockableDatabase`. Like `DummyDatabase`, most functions do nothing and just return true. Like `FailDatabase`, the return value of some functions can be configured on the fly to test various failure cases. Like the memory-only databases, records can actually be written to the `MockableDatabase` and be retrieved later, but all of this is still held in memory. Using `MockableDatabase` completely removes the need for having BDB or SQLite backed wallets in the unit tests for the tests that are not actually testing specific database behaviors. Because `MockableDatabase`s can be created by each unit test, we can also control what records are stored in the database. Records can be added and removed externally from the typical database modification functions. This will give us greater ability to test failure conditions, particularly those involving corrupted records. Possible alternative to #26644 ACKs for top commit: furszy: ACK 33e2b82 TheCharlatan: ACK 33e2b82a4fc990253ff77655f437c7aed336bc55 Tree-SHA512: c2b09eff9728d063d2d4aea28a0f0e64e40b76483e75dc53f08667df23bd25834d52656cd4eafb02e552db0b9e619cfdb1b1c65b26b5436ee2c971d804768bcc
2023-05-14wallet, tests: mark unconflicted txs as inactiveishaanam
In `blockDisconnected`, for each transaction in the block, look for any wallet transactions spending the same inputs. If any of these transactions were marked conflicted, they are now marked as inactive. Co-authored-by: ariard <antoine.riard@gmail.com>
2023-05-11Merge bitcoin/bitcoin#27125: refactor, kernel: Decouple ArgsManager from ↵fanquake
blockstorage 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3 refactor, blockstorage: Replace stopafterblockimport arg (TheCharlatan) 18e5ba7c8002bcd473ee29ce4b5bfc56df6142a4 refactor, blockstorage: Replace blocksdir arg (TheCharlatan) 02a0899527ba3d31329e56c791c9dbf36075bb84 refactor, BlockManager: Replace fastprune from arg with options (TheCharlatan) a498d699e3fdac5bfdb33020a1fd6c4a79989752 refactor/iwyu: Complete includes for blockmanager_args (TheCharlatan) f0bb1021f0d60f5f19176e67a66fcf7c325f88d1 refactor: Move functions to BlockManager methods (TheCharlatan) cfbb2124939822e95265a39242ffca3d86bac6e8 zmq: Pass lambda to zmq's ZMQPublishRawBlockNotifier (TheCharlatan) 8ed4ff8e05d61a8e954d72cebdc2e1d1ab24fb84 refactor: Declare g_zmq_notification_interface as unique_ptr (TheCharlatan) Pull request description: The libbitcoin_kernel library should not rely on the `ArgsManager`, but rather use option structs that can be passed to the various classes it uses. This PR removes reliance on the `ArgsManager` from the `blockstorage.*` files. Like similar prior work, it uses the options struct in the `BlockManager` that can be populated with `ArgsManager` values. Some related prior work: https://github.com/bitcoin/bitcoin/pull/26889 https://github.com/bitcoin/bitcoin/pull/25862 https://github.com/bitcoin/bitcoin/pull/25527 https://github.com/bitcoin/bitcoin/pull/25487 Related PR removing blockstorage globals: https://github.com/bitcoin/bitcoin/pull/25781 ACKs for top commit: ryanofsky: Code review ACK 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3. Since last ACK just added std::move and fixed commit title. Sorry for the noise! mzumsande: Code Review ACK 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3 Tree-SHA512: 4bde8fd140a40b97eca923e9016d85dcea6fad6fd199731f158376294af59c3e8b163a0725aa47b4be3519b61828044e0a042deea005e0c28de21d8b6c3e1ea7
2023-05-10refactor: Move functions to BlockManager methodsTheCharlatan
This is a commit in preparation for the next few commits. The functions are moved to methods to avoid their re-declaration for the purpose of passing in BlockManager options. The functions that were now moved into the BlockManager should no longer use the params as an argument, but instead use the member variable. In the moved ReadBlockFromDisk and UndoReadFromDisk, change the function signature to accept a reference to a CBlockIndex instead of a raw pointer. The pointer is expected to be non-null, so reflect that in the type. To allow for the move of functions to BlockManager methods all call sites require an instantiated BlockManager, or a callback to one.
2023-05-09scripted-diff: Use UniValue::find_value methodMarcoFalke
-BEGIN VERIFY SCRIPT- sed --regexp-extended -i 's/find_value\(([^ ,]+), /\1.find_value(/g' $(git grep -l find_value) -END VERIFY SCRIPT-
2023-05-09Merge bitcoin/bitcoin#27491: refactor: Move chain constants to the util libraryfanquake
d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7 scripted-diff: Remove unused chainparamsbase includes (TheCharlatan) e9ee8aaf3acdf6dce2b339916d4c602484570050 Add missing definitions in prep for scripted diff (TheCharlatan) ba8fc7d788932b25864fb260ca14983aa2398c23 refactor: Replace string chain name constants with ChainTypes (TheCharlatan) 401453df419af35957ec711423ac3d93ad512fe8 refactor: Introduce ChainType getters for ArgsManager (TheCharlatan) bfc21c31b2186f7d30fc9a9ca7d6887ab61c6fb9 refactor: Create chaintype files (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is also a follow up to #26177. It replaces pull request https://github.com/bitcoin/bitcoin/pull/27294, which just moved the constants to a new file, but did not re-declare them as enums. The code move of the chain name constants out of the `chainparamsbase` to their own separate header allows the kernel `chainparams` to no longer include `chainparamsbase`. The `chainparamsbase` contain references to the `ArgsManager` and networking related options that should not belong to the kernel library. Besides this move, the constants are re-declared as enums with helper functions facilitating string conversions. ACKs for top commit: ryanofsky: Code review ACK d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7. Just suggested changes since last review. Tree-SHA512: ac2fbe5cbbab4f52eae1e30af1f16700b6589eb4764c328a151a712adfc37f326cc94a65c385534c57d4bc92cc1a13bf1777d92bc924a20dbb30440e7380b316
2023-05-09refactor: Replace string chain name constants with ChainTypesTheCharlatan
This commit effectively moves the definition of these constants out of the chainparamsbase to their own file. Using the ChainType enums provides better type safety compared to passing around strings. The commit is part of an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager and other functionality that should not be part of the kernel library.
2023-05-08Merge bitcoin/bitcoin#26076: Switch hardened derivation marker to hAndrew Chow
fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost) bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost) Pull request description: This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet. For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually. Both markers can still be parsed. The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet. See discussion in #15740 ACKs for top commit: achow101: ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c darosior: re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
2023-05-06Merge bitcoin/bitcoin#27405: util: Use steady clock instead of system clock ↵fanquake
to measure durations fa83fb31619c19a1a30b4181486601a944941b16 wallet: Use steady clock to calculate number of derive iterations (MarcoFalke) fa2c099cec1780a498e198750be0cf5bf0ca315a wallet: Use steady clock to measure scanning duration (MarcoFalke) fa976218044f3ff244abbd797b183a1408375c74 qt: Use steady clock to throttle GUI notifications (MarcoFalke) fa1d8044abc2cd0f149a2d526b3b03441443cdb0 test: Use steady clock in index tests (MarcoFalke) fa454dcb20b9e7943cc25e6eeea72912b5f1c7b5 net: Use steady clock in InterruptibleRecv (MarcoFalke) Pull request description: `GetTimeMillis` has multiple issues: * It doesn't denote the underlying clock type * It isn't type-safe * It is used incorrectly in places that should use a steady clock Fix all issues here. ACKs for top commit: willcl-ark: ACK fa83fb3161 martinus: Code review ACK https://github.com/bitcoin/bitcoin/commit/fa83fb31619c19a1a30b4181486601a944941b16, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok. Tree-SHA512: 5ec4fede8c7f97e2e08863c011856e8304f16ba30a68fdeb42f96a50a04961092cbe46ccf9ea6ac99ff5203c09f9e0924eb483eb38d7df0759addc85116c8a9f
2023-05-03Merge bitcoin/bitcoin#26066: wallet: Refactor and document CoinControlAndrew Chow
daba95700b0b77a2e898299f218c47a69ed2c7d0 refactor: Make ListSelected return vector (Sebastian Falbesoner) 94776621ba6a79f3197ec71250bc48e974ad5e4a wallet: Move CoinCointrol definitions to .cpp (Aurèle Oulès) 1db23da6e163e793458ec702a9601d2837aea9cb wallet: Use std::optional for GetExternalOutput and fixups (Aurèle Oulès) becc45b589d07c4523276e4ba2dad8852d0d2632 scripted-diff: Rename setSelected->m_selected_inputs (Aurèle Oulès) Pull request description: - Moves CoinControl function definitions from `coincontrol.h` to `coincontrol.cpp` - Adds more documentation - Renames class member for an improved comprehension - Use `std::optional` for `GetExternalOutput` ACKs for top commit: achow101: ACK daba95700b0b77a2e898299f218c47a69ed2c7d0 Xekyo: ACK daba95700b0b77a2e898299f218c47a69ed2c7d0 Tree-SHA512: 3bf2dc834a3246c2f53f8c55154258e605fcb169431d3f7b156931f33c7e3b1ae28e03e16b37f9140a827890eb7798be485b2c36bfc23ff29bb01763f289a07c
2023-05-03tests: Modify records directly in wallet ckey loading testAndrew Chow
In the wallet ckey loading test, we modify various ckey records to test corruption handling. As the database is now a mockable database, we can modify the records that the database will be initialized with. This avoids having to use the verbose database reading and writing functions.
2023-05-03tests: Update DuplicateMockDatabase for MockableDatabaseAndrew Chow
2023-05-03scripted-diff: Replace CreateMockWalletDB with CreateMockableWalletDBAndrew Chow
Since we have a mockable wallet database, we don't really need to be using BDB or SQLite's in-memory database capabilities. It doesn't really help us to be using those as we aren't doing anything that requires one type of db over the other, and will just prefer SQLite if it's available. MockableDatabase is suitable for these uses, so use CreateMockableWalletDatabase to use that. -BEGIN VERIFY SCRIPT- sed -i "s/CreateMockWalletDatabase(options)/CreateMockableWalletDatabase()/" $(git grep -l "CreateMockWalletDatabase(options)" -- ":(exclude)src/wallet/walletdb.*") sed -i "s/CreateMockWalletDatabase/CreateMockableWalletDatabase/" $(git grep -l "CreateMockWalletDatabase" -- ":(exclude)src/wallet/walletdb.*") -END VERIFY SCRIPT-
2023-05-03wallet, tests: Include wallet/test/util.hAndrew Chow
This will be needed for the following scripted-diff to work.
2023-05-03wallet: Move DummyDatabase to salvageAndrew Chow
It's only used by salvage, so make it local to that only.
2023-05-03wallet, tests: Replace usage of dummy db with mockable dbAndrew Chow
2023-05-03Introduce MockableDatabase for wallet unit testsAndrew Chow
MockableDatabase is a WalletDatabase that allows us to interact with the records to change them independently from the wallet, as well as changing the return values from within the tests. This will give us greater flexibility in testing the wallet.
2023-05-02Merge bitcoin/bitcoin#26094: rpc: Return block hash & height in getbalances, ↵Andrew Chow
gettransaction and getwalletinfo 710b83938ab5bbc4bd324d8b2e69461a2a1d2eec rpc: return block hash & height in getbalances, gettransaction & getwalletinfo JSONs (Harris) Pull request description: Reopens #18570 and closes #18567. I have rebased the original PR. Not sure why the original got closed as it was about to get merged. ACKs for top commit: achow101: ACK 710b83938ab5bbc4bd324d8b2e69461a2a1d2eec Tree-SHA512: d4478d990be98b1642e9ffb2930987f4a224e8bd64e2e35a5dda927a54c509ec9d712cd0eac35dc2bb89f00a1678e530ce14d7445f1dd93aa3a4cce2bc9b130d
2023-05-01Merge bitcoin/bitcoin#27195: bumpfee: allow send coins back to yourselfAndrew Chow
be72663a1521bc6cdf16d43a4feae7c5b57735c0 test: bumpfee, add coverage for "send coins back to yourself" (furszy) 7bffec6715a75bb2c8631177d39f984aabc656ba bumpfee: enable send coins back to yourself (furszy) Pull request description: Simple example: 1) User_1 sends 0.1 btc to user_2 on a low fee transaction. 2) After few hours, the tx is still in the mempool, user_2 is not interested anymore, so user_1 decides to cancel it by sending coins back to himself. 3) User_1 has the bright idea of opening the explorer and copy the change output address of the transaction. Then call bumpfee providing such output (in the "outputs" arg). Currently, this is not possible. The wallet fails with "Unable to create transaction. Transaction must have at least one recipient" error. The error reason is because we discard the provided output from the recipients list and set it inside the coin control so the process adds it later (when the change is calculated). But.. there is no later if the tx has no outputs. ACKs for top commit: ishaanam: reACK be72663a1521bc6cdf16d43a4feae7c5b57735c0 achow101: ACK be72663a1521bc6cdf16d43a4feae7c5b57735c0 Tree-SHA512: c2c38290a998f9b426a830d9624c7feb730158980ac186f8fb0138d5e200935d6538307bc60a2c3d0b7b6ee2b4ffb77a1e98baf8feb1d20a7d825f6055ac377f
2023-05-01Merge bitcoin/bitcoin#25680: rpc, docs: Add note for commands that supports ↵Andrew Chow
only legacy wallets 9141e4395a5f923059ad61ac6ba42a1a89e92cb0 rpc, docs: Add note for commands that supports only legacy wallets (Yusuf Sahin HAMZA) Pull request description: Refs #25363, apparently issue is not updated since over a month, so i decided to put the same `importaddress` note in #25368 to other rpc commands that needs this note. Note is added for following commands: - `importprivkey` - `importpubkey` - `importwallet` - `dumpprivkey` - `dumpwallet` - `importmulti` - `addmultisigaddress` - `sethdseed` ACKs for top commit: achow101: ACK 9141e4395a5f923059ad61ac6ba42a1a89e92cb0 Tree-SHA512: f3dc05d26577fd8dbe2bd69cb5c14ffccebacd6010402af44427b3d01be8484895dfcf33d55dfa766eadb7f9f3bae5cc4c2add3ac816a2ac60e8beb5a97527f3
2023-05-01Merge bitcoin/bitcoin#27224: refactor: Remove CAddressBookData::destdataAndrew Chow
a5986e82dd2b8f923d60f9e31760ef62b9010105 refactor: Remove CAddressBookData::destdata (Ryan Ofsky) 5938ad0bdb013953861c7cd15a95f00998a06f44 wallet: Add DatabaseBatch::ErasePrefix method (Ryan Ofsky) Pull request description: This is cleanup that doesn't change external behavior. Benefits of the cleanup are: - Removes awkward `StringMap` intermediate representation for wallet address metadata. - Simplifies `CWallet`, deals with used address and received request serialization in `walletdb.cpp` instead of higher level wallet code - Adds test coverage and documentation This PR doesn't change externally observable behavior. Internally, the only change in behavior is that `EraseDestData` deletes rows directly from the database because they are no longer stored in memory. This is more direct and efficient because it uses a single lookup and scan instead of multiple lookups. Motivation for this cleanup is making changes like #18550, #18192, #13756 easier to reason about and less likely to result in unintended behavior and bugs --- This PR is a rebased copy of #18608. For some reason that PR is locked and couldn't be reopened or commented on. This PR is an alternative to #27215 with differences described in https://github.com/bitcoin/bitcoin/pull/27215#pullrequestreview-1329028143 ACKs for top commit: achow101: ACK a5986e82dd2b8f923d60f9e31760ef62b9010105 furszy: Code ACK a5986e82 Tree-SHA512: 6bd3e402f1f60263fbd433760bdc29d04175ddaf8307207c4a67d59f6cffa732e176ba57886e02926f7a1615dce0ed9cda36c8cbc6430aa8e5b56934c23f3fe7
2023-04-26rpc: return block hash & height in getbalances, gettransaction & ↵Harris
getwalletinfo JSONs Co-authored-by: Aurèle Oulès <aurele@oules.com>
2023-04-26Merge bitcoin/bitcoin#25158: rpc, wallet: add abandoned field for all ↵Andrew Chow
categories of transaction in ListTransaction 0c520679ab5f0ba99584cb356ec28ef089f14735 doc: add release notes for `abandoned` field in `gettransaction` and `listtransactions` (brunoerg) a1aaa7f51f4205ae4d27fbceb2c3a97bc114e828 rpc, wallet: add `abandoned` field for all categories of transactions in ListTransactions (brunoerg) Pull request description: Fixes #25130 ACKs for top commit: achow101: re-ACK 0c520679ab5f0ba99584cb356ec28ef089f14735 Tree-SHA512: 1864460d76decab7898737c96517d722055eb8f81ca52248fe1035723258c6cd4a93251e06a86ecbbb0b0a80af1466b2c86fb142ace4ccb74cc40d5dc3967d7f
2023-04-26refactor: Make ListSelected return vectorSebastian Falbesoner
2023-04-26wallet: Move CoinCointrol definitions to .cppAurèle Oulès
Move definitions to coincontrol.cpp and add documentation.
2023-04-26wallet: Use std::optional for GetExternalOutput and fixupsAurèle Oulès
2023-04-26scripted-diff: Rename setSelected->m_selected_inputsAurèle Oulès
-BEGIN VERIFY SCRIPT- sed -i 's/setSelected/m_selected_inputs/g' src/wallet/coincontrol.h src/wallet/coincontrol.cpp -END VERIFY SCRIPT-
2023-04-22wallet: introduce generic recursive tx state updating functionishaanam
This commit also changed `MarkConflicted` and `AbandonTransaction` to use this new function Co-authored-by: ariard <antoine.riard@gmail.com>
2023-04-21Merge bitcoin/bitcoin#25939: rpc: In `utxoupdatepsbt` also look for the tx ↵Andrew Chow
in the txindex 6e9f8bb050785dbc754b6bb493aad9139908ef98 rpc, tests: in `utxoupdatepsbt` also look for the transaction in the txindex (ishaanam) a5b4883fb43d01bfef1244df62c64bf8691ca63a rpc: extract psbt updating logic into ProcessPSBT (ishaanam) Pull request description: Previously the `utxoupdatepsbt` RPC, added in #13932, only updated the inputs spending segwit outputs with the `witness_utxo`, because the full transaction is needed for legacy inputs. Before this RPC looked for just the utxos in the utxo set and the mempool. This PR makes it so that if the user has txindex enabled, then the full transaction is looked for there for all inputs. If it is not found in the txindex or txindex isn't enabled, then the mempool is checked for the full transaction. If the transaction an input is spending from is still not found at that point, then the utxo set is searched and the inputs spending segwit outputs are updated with just the utxo. ACKs for top commit: achow101: ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98 Xekyo: ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98 Tree-SHA512: 078db3c37a1ecd5816d80a42e8bd1341e416d661f508fa5fce0f4e1249fefd7b92a0d45f44957781cb69d0953145ef096ecdd4545ada39062be27742402dac6f
2023-04-21Merge bitcoin/bitcoin#27419: move-only: Extract common/args from util/systemfanquake
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254. The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale. ACKs for top commit: MarcoFalke: re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲 ryanofsky: Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review. hebasto: ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
2023-04-20Merge bitcoin/bitcoin#26720: wallet: coin selection, don't return results ↵Andrew Chow
that exceed the max allowed weight 25ab14712b9d80276016f9fc9bff7fb9c1d09635 refactor: coinselector_tests, unify wallet creation code (furszy) ba9431c505e1590db6103b9632134985cd4704dc test: coverage for bnb max weight (furszy) 5a2bc45ee0b123e461c5191322ed0b43524c3d82 wallet: clean post coin selection max weight filter (furszy) 2d112584e384de10021c64e4700455d71326824e coin selection: BnB, don't return selection if exceeds max allowed tx weight (furszy) d3a1c098e4b5df2ebbae20c6e390c3d783950e93 test: coin selection, add coverage for SRD (furszy) 9d9689e5a657956db8a30829c994600ec7d3098b coin selection: heap-ify SRD, don't return selection if exceeds max tx weight (furszy) 6107ec2229c5f5b4e944a6b10d38010c850094ac coin selection: knapsack, select closest UTXO above target if result exceeds max tx size (furszy) 1284223691127e76135a46d251c52416104f0ff1 wallet: refactor coin selection algos to return util::Result (furszy) Pull request description: Coming from the following comment https://github.com/bitcoin/bitcoin/pull/25729#discussion_r1029324367. The reason why we are adding hundreds of UTXO from different sources when the target amount is covered only by one of them is because only SRD returns a usable result. Context: In the test, we create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection to fund 49.5 BTC. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not happening for the following reason: Knapsack returns a result that exceeds the max allowed transaction size, when it should return the closest utxo above the target, so we fallback to SRD who selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed. ACKs for top commit: S3RK: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 achow101: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Xekyo: reACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 theStack: Code-review ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Tree-SHA512: 2425de4cc479b4db999b3b2e02eb522a2130a06379cca0418672a51c4076971a1d427191173820db76a0f85a8edfff100114e1c38fb3b5dc51598d07cabe1a60