aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2023-02-10Zero out wallet master key upon lockJohn Moffett
When an encrypted wallet is locked (for instance via the RPC `walletlock`), the docs indicate that the key is removed from memory. However, the vector (with a secure allocator) is merely cleared. This allows the key to persist indefinitely in memory. Instead, manually fill the bytes with zeroes before clearing.
2023-02-05Remove `-sysperms` optionHennadii Stepanov
This change effectively reverts commits from https://github.com/bitcoin/bitcoin/pull/4286. Users, who rely on non-default access permissions, should use `chmod` command.
2023-02-03Merge bitcoin/bitcoin#25966: test: Remove redundant testAndrew Chow
fb1c6c14c11ccd4833c1a24f77c507f098d369ad test: Remove redundant test (yancy) Pull request description: I can't think of any reason to keep this test case around labeled [fix me](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L242). The test was originally added [here](https://github.com/bitcoin/bitcoin/commit/4566ab75f277612425337bf7786c1d3a410d894a) however there was never an assertion about the coins that should be selected, only that a solution is found (which is a redundant solution to the test [above](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L222)). The comment was later added here to [fix](https://github.com/bitcoin/bitcoin/commit/384273260a6ccbcf79dade0830011f528e5a1581) it, however it's unclear what exactly it's testing. A test was later added [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L366) where if the [long term fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L357) is less than the current [fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L356), then select fewer UTXOs, which may have been the original intent. ACKs for top commit: S3RK: ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad Zero-1729: Concept ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad achow101: ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad Tree-SHA512: bce2cdae669c144ffaa130237a1643e3b6728e13d603cebf5d9493c4c7c68b3635868e4d93d210783c2ded2a871f185ca09a2053168c05b26a1e056ff6edf68f
2023-02-01Merge bitcoin/bitcoin#26910: wallet: migrate wallet, exit early if no legacy ↵Andrew Chow
data exist 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa wallet: migrate wallet, exit early if no legacy data exist (furszy) Pull request description: The process first creates a backup file then return an error, without removing the recently created file, when notices that the db is already running sqlite. ACKs for top commit: john-moffett: ACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa achow101: ACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa ishaanam: crACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa Tree-SHA512: 9fb52e80de96e129487ab91bef13647bc4570a782003b1e37940e2a00ca26283fd24ad39bdb63a984ae0a56140b518fd0d74aa2fc59ab04405b2c179b7d3c54a
2023-02-01Fix clang-tidy readability-const-return-type violationsMarcoFalke
2023-01-31clang-tidy: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
2023-01-30Merge bitcoin/bitcoin#26499: wallet: Abandon descendants of orphaned coinbasesglozow
b0fa5989e1b77a343349bd36f8bc407f9366a589 test: Check that orphaned coinbase unconf spend is still abandoned (Andrew Chow) 9addbd78901124a48fd41a82a9557fcf3490191d wallet: Automatically abandon orphaned coinbases and their children (Andrew Chow) Pull request description: When a block is reorged out of the main chain, any descendants of the coinbase will no longer be valid. Currently they are only marked as inactive, which means that our balance calculations will still include them. In order to be excluded from the balance calculation, they need to either be abandoned or conflicted. This PR goes with the abandoned method. Note that even when they are included in balance calculations, coin selection will not select outputs belonging to these transactions because they are not in the mempool. Fixes #14148 ACKs for top commit: furszy: ACK b0fa5989 with a not-blocking nit. aureleoules: reACK b0fa5989e1b77a343349bd36f8bc407f9366a589 ishaanam: ACK b0fa5989e1b77a343349bd36f8bc407f9366a589 Tree-SHA512: 68f12e7aa8df392d8817dc6ac0becce8dbe83837bfa538f47027e6730e5b2e1b1a090cfcea2dc598398fdb66090e02d321d799f087020d7e1badcf96e598c3ac
2023-01-30Merge bitcoin/bitcoin#15294: refactor: Extract RipeMd160MarcoFalke
6879be691bf636a53208ef058f2ebe18bfa8017c refactor: Extract RIPEMD160 (Ben Woosley) Pull request description: To directly return a CRIPEMD160 hash from data. Simplifies the call sites. ACKs for top commit: achow101: ACK 6879be691bf636a53208ef058f2ebe18bfa8017c theStack: re-ACK 6879be691bf636a53208ef058f2ebe18bfa8017c MarcoFalke: review ACK 6879be691bf636a53208ef058f2ebe18bfa8017c 🏔 Tree-SHA512: 6ead85d8060c2ac6afd43ec716ff5a82d6754c4132fe7df3b898541fa19f1dfd8b301b2b66ae7cb7594b1b1a8c7f68bce3790a8c610d4a1164e995d89bc5ae34
2023-01-26refactor: Extract RIPEMD160Ben Woosley
To directly return a CRIPEMD160 hash from data. Incidentally, decoding this acronym: * RIPEMD -> RIPE Message Digest * RIPE -> RACE Integrity Primitives Evaluation * RACE -> Research and Development in Advanced Communications Technologies in Europe
2023-01-26Merge bitcoin/bitcoin#25296: Add DataStream without ser-type and ser-version ↵fanquake
and use it where possible fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 Remove unused CDataStream::SetType (MarcoFalke) fa29e73cdab82f98682821322cda89b1084ba887 Use DataStream where possible (MarcoFalke) fa9becfe1cea5040e7cea36324d1b0789cbbd25d streams: Add DataStream without ser-type and ser-version (MarcoFalke) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `DataStream`. `CDataStream` remains in places where it is not yet possible. ACKs for top commit: stickies-v: re-ACK [fa035fe](https://github.com/bitcoin/bitcoin/commit/fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4) aureleoules: diff re-ACK fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 https://github.com/bitcoin/bitcoin/compare/fa0e6640bac8c6426af7c5744125c85c0f74b9e5..fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 Tree-SHA512: cb5e53d0df7c94319ffadc6ea1d887fc38516decaf43f0673396d79cc62d450a1a61173654a91b8c2b52d2cecea53fe4a500b8f6466596f35731471163fb051c
2023-01-26Use DataStream where possibleMarcoFalke
2023-01-26Merge bitcoin/bitcoin#26960: refactor: Remove c_str from util/checkMarcoFalke
fab958290b84037fad1d78ffb2bce34d2b0e8c36 refactor: Remove c_str from util/check (MarcoFalke) Pull request description: Seems confusing and fragile to require calling code to call `c_str()` when passing a read-only view of a std::string. Fix that by using std::string_view, which can be constructed from string literals and std::string. Also, remove the now unused `c_str()` from `src/wallet/bdb.cpp`. ACKs for top commit: stickies-v: ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36 aureleoules: ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36 theStack: ACK fab958290b84037fad1d78ffb2bce34d2b0e8c36 Tree-SHA512: ae39812c6bb8e2ef095e1b843774af2718f48404cb848c3e43b16d3c22240557d69d54a13a038a4a9c48b3ba0e4523e1f87abdd60f91486092f50fd43c0e8483
2023-01-24Merge bitcoin/bitcoin#26955: wallet: permit mintxfee=0fanquake
f11eb1fe279c4a92e1bfc2c139e8838c73459d12 wallet: permit mintxfee=0 (willcl-ark) Pull request description: Fixes #26797 Permit nodes to use `-mintxfee=0`. Values below 0 are handled by the ParseMoney() check. ACKs for top commit: MarcoFalke: review ACK f11eb1fe279c4a92e1bfc2c139e8838c73459d12 john-moffett: ACK f11eb1fe279c4a92e1bfc2c139e8838c73459d12 Tree-SHA512: 3bf50362bced4fee8e3a846cfb46f1c65dd607c9c824aa3f8c52294371b0646d167a04772d5302bdbee35bbaf407ef0aa634228f70e522c3e423f4213b4ae071
2023-01-24refactor: Remove c_str from util/checkMarcoFalke
2023-01-23Merge bitcoin/bitcoin#26690: wallet: Refactor database cursor into its own ↵fanquake
object with proper return codes 4aebd832a405090c2608e4b60bb4f34501bcea61 db: Change DatabaseCursor::Next to return status enum (Andrew Chow) d79e8dcf2981ef1964a2fde8c472b5de1ca1c963 wallet: Have cursor users use DatabaseCursor directly (Andrew Chow) 7a198bba0a1d0a0f0fd4ca947955cb52b84bdd4b wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow) 69efbc011bb74fcd8dd9ed2a8a5d31bc9e323c10 Move SafeDbt out of BerkeleyBatch (Andrew Chow) Pull request description: Instead of having database cursors be tied to a particular `DatabaseBatch` object and requiring its setup and teardown be separate functions in that batch, we can have cursors be separate RAII classes. This makes it easier to create and destroy cursors as well as having cursors that have slightly different behaviors. Additionally, since reading data from a cursor is a tri-state, this PR changes the return value of the `Next` function (formerly `ReadAtCursor`) to return an Enum rather than the current system of 2 booleans. This greatly simplifies and unifies the code that deals with cursors as now there is no confusion as to what the function returns when there are no records left to be read. Extracted from #24914 ACKs for top commit: furszy: diff ACK 4aebd83 theStack: Code-review ACK 4aebd832a405090c2608e4b60bb4f34501bcea61 Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1
2023-01-23wallet: permit mintxfee=0willcl-ark
Fixes #26797 Permit nodes to use a mintxfee of `0` if they choose. Values below 0 are handled by the ParseMoney() check.
2023-01-22scripted-diff: use RPCArg::Optional::OMITTED over OMITTED_NAMED_ARGfanquake
-BEGIN VERIFY SCRIPT- sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/) -END VERIFY SCRIPT-
2023-01-18Merge bitcoin/bitcoin#25659: wallet: simplify ListCoins implementationAndrew Chow
a2ac6f9582c4c996fa36e4801fa0aac756235754 wallet: unify FindNonChangeParentOutput functions (furszy) b3f4e827378e010cd2a5d1b876d01db52c054d26 wallet: simplify ListCoins implementation (furszy) Pull request description: Focused on the following changes: 1) Removed the entire locked coins lookup that was inside `ListCoins` by including them directly on the `AvailableCoins` result (where we were skipping them before). 2) Unified both `FindNonChangeParentOutput` functions (only called from `ListCoins`) ACKs for top commit: achow101: ACK a2ac6f9582c4c996fa36e4801fa0aac756235754 aureleoules: ACK a2ac6f9582c4c996fa36e4801fa0aac756235754, LGTM theStack: Code-review ACK a2ac6f9582c4c996fa36e4801fa0aac756235754 Tree-SHA512: f72b21ee1600c5992559b5dcd8ff260527afac2ec696737f998343a0850b84d439e7f86ea52a14cc0cddabf132a30bf5b52fb34950578ac323083d4375b937f1
2023-01-18wallet: migrate wallet, exit early if no legacy data existfurszy
otherwise the process will create a backup file then return an error when notices that the db is already running sqlite.
2023-01-17doc: Fix incorrect sendmany RPC docMarcoFalke
This enables the type check and fixes the wrong docs. Otherwise the enabled check would lead to test errors, such as: > "wallet_labels.py", line 96, in run_test > node.sendmany( > > test_framework.authproxy.JSONRPCException: > JSON value of type null is not of expected type string (-3)
2023-01-17Merge bitcoin/bitcoin#26039: refactor: Run type check against RPCArgs (1/2)fanquake
fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d rpc: Run type check against RPCArgs (MarcoFalke) faf96721a66dcc215ea9d6affb30f9a00cc37000 test: Fix wrong types passed to RPCs (MarcoFalke) Pull request description: It seems brittle to require `RPCTypeCheck` being called inside the code logic. Without compile-time enforcement this will lead to places where it is forgotten and thus to inconsistencies and bugs. Fix this by removing the calls to `RPCTypeCheck` and doing the check internally. The changes should be reviewed as refactoring changes. However, if they change behavior, it will be a bugfix. For example the changes here happen to also detect/fix bugs like the one fixed in commit 3b5fb6e77a93f58b3d03b1eec3595f5c45e633a9. ACKs for top commit: ajtowns: ACK fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d Tree-SHA512: fb2c0981fe6e24da3ca7dbc06898730779ea4e02ea485458505a281cf421015e44dad0221a04023fc547ea2c660d94657909843fc85d92b847611ec097532439
2023-01-16Merge bitcoin/bitcoin#25375: rpc: add minconf/maxconf options to sendall and ↵Andrew Chow
fund transaction calls cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad rpc: add minconf and maxconf options to sendall (ishaanam) a07a413466a0edd47eab9189b46a70aafbbe22b7 Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund calls (Juan Pablo Civile) Pull request description: This PR adds a "minconf" option to `fundrawtransaction`, `walletcreatefundedpsbt`, and `sendall`. Alternative implementation of #14641 Fixes #14542 Edit: This PR now also adds this option to `send` ACKs for top commit: achow101: ACK cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad Xekyo: ACK cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad furszy: diff ACK cfe5aebc, only a non-blocking nit. Tree-SHA512: 836e610926eec3a62308fba88ddbd6a13d8f4dac37352d0309599f893cde9c1df5e9c298fda6e076493068e4d213e4afa7290a9e3bdb5a95a5d507da3f7b59e8
2023-01-11Merge bitcoin/bitcoin#26675: wallet: For feebump, ignore abandoned ↵Andrew Chow
descendant spends f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 For feebump, ignore abandoned descendant spends (John Moffett) Pull request description: Closes #26667 To be eligible for fee-bumping, a transaction must not have any of its outputs (eg - change) spent in other unconfirmed transactions in the wallet. This behavior is currently [enforced](https://github.com/bitcoin/bitcoin/blob/9e229a542ff2107be43eff2e4b992841367f0366/src/wallet/feebumper.cpp#L25-L28) and [tested](https://github.com/bitcoin/bitcoin/blob/9e229a542ff2107be43eff2e4b992841367f0366/test/functional/wallet_bumpfee.py#L270-L286). However, this check shouldn't apply to spends in abandoned descendant transactions, as explained by #26667. `CWallet::IsSpent` already carves out an exception for abandoned transactions, so we can just use that. I've also added a new test to cover this case. ACKs for top commit: Sjors: re-utACK f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 achow101: ACK f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 furszy: ACK f9ce0ead Tree-SHA512: 19d957d1cf6747668bb114e27a305027bfca5a9bed2b1d9cc9e1b0bd4666486c7c4b60b045a7fe677eb9734d746f5de76390781fb1e9e0bceb4a46d20acd1749
2023-01-11rpc: add minconf and maxconf options to sendallishaanam
2023-01-11Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund callsJuan Pablo Civile
Enables users to craft BIP-125 replacements with changes to the output list, ensuring that if additional funds are needed they will be added.
2023-01-11rpc: Run type check against RPCArgsMarcoFalke
2023-01-11Merge bitcoin/bitcoin#26758: refactor: Add `performance-no-automatic-move` ↵MarcoFalke
clang-tidy check 9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201). For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html. The following types are affected: - `std::pair<CAddress, NodeSeconds>` - `std::vector<CAddress>` - `UniValue`, also see bitcoin/bitcoin#25429 - `QColor` - `CBlock` - `MempoolAcceptResult` - `std::shared_ptr<CWallet>` - `std::optional<SelectionResult>` - `CTransactionRef`, which is `std::shared_ptr<const CTransaction>` ACKs for top commit: andrewtoth: ACK 9567bfeab95cc0932073641dd162903850987d43 aureleoules: ACK 9567bfeab95cc0932073641dd162903850987d43 Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
2023-01-10Merge bitcoin/bitcoin#26679: wallet: Skip rescanning if wallet is more ↵Andrew Chow
recent than tip 378400953424598fd78ccec5ba8cc38bc253c550 wallet: Skip rescanning if wallet is more recent than tip (Andrew Chow) Pull request description: If a wallet has key birthdates that are more recent than the currrent chain tip, or a bestblock height higher than the current tip, we should not attempt to rescan as there is nothing to scan for. Fixes #26655 ACKs for top commit: ishaanam: re-utACK 378400953424598fd78ccec5ba8cc38bc253c550 w0xlt: utACK https://github.com/bitcoin/bitcoin/pull/26679/commits/378400953424598fd78ccec5ba8cc38bc253c550 furszy: Code review ACK 37840095 Tree-SHA512: f0d90b62940d97d50f21e1e01fa6dcb54409fad819cea4283612825c4d93d733df323cd92787fed43956b0a8e386a5bf88218f1f5749c913398667a5c8f54470
2023-01-10wallet: Automatically abandon orphaned coinbases and their childrenAndrew Chow
2023-01-10Merge bitcoin/bitcoin#26186: rpc: Sanitize label name in various RPCs with testsAndrew Chow
65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d test: Invalid label name coverage (Aurèle Oulès) 552b51e682b5a52d9e2fbe64e44e623451692bd3 refactor: Add sanity checks in LabelFromValue (Aurèle Oulès) 67e7ba8e1aea58fc864f9bb1fc0e56b70777185e rpc: Sanitize label name in various RPCs (Aurèle Oulès) Pull request description: The following RPCs did not sanitize the optional label name: - importprivkey - importaddress - importpubkey - importmulti - importdescriptors - listsinceblock Thus is was possible to import an address with a label `*` which should not be possible. The wildcard label is used for backwards compatibility in the `listtransactions` rpc. I added test coverage for these RPCs. ACKs for top commit: ajtowns: ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d achow101: ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d furszy: diff ACK 65e78bd stickies-v: re-ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d theStack: re-ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d Tree-SHA512: ad99f2824d4cfae352166b76da4ca0069b7c2eccf81aaa0654be25bbb3c6e5d6b005d93960f3f4154155f80e12be2d0cebd5529922ae3d2a36ee4eed82440b31
2023-01-09Merge bitcoin/bitcoin#26618: rpc: Prevent unloading a wallet when rescanningAndrew Chow
109cbb819ddd20220a255791c40261f746260f42 doc: Add release notes for #26618 (Aurèle Oulès) b13902d2e45ad1e73f79e221ffc16ce26b7c3ba5 rpc: Prevent unloading a wallet when rescanning (Aurèle Oulès) Pull request description: Fixes #26463. This PR prevents a user from unloading a wallet if it is currently rescanning. To test: ```bash ./src/bitcoin-cli -testnet -named createwallet wallet_name=wo disable_private_keys=true ./src/bitcoin-cli -testnet -rpcwallet=wo importdescriptors '[{ "desc": "addr(mmcuW74MyJUZuLnWXGQLoNXPrS9RbFz6gD)#tpnrahgc", "timestamp": 0, "active": false, "internal": false, "next": 0 }]' ./src/bitcoin-cli -testnet unloadwallet wo error code: -4 error message: Wallet is currently rescanning. Abort existing rescan or wait. ACKs for top commit: achow101: ACK 109cbb819ddd20220a255791c40261f746260f42 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26618/commits/109cbb819ddd20220a255791c40261f746260f42 kouloumos: ACK 109cbb819ddd20220a255791c40261f746260f42 promag: ACK 109cbb819ddd20220a255791c40261f746260f42 Tree-SHA512: 15fdddf4cf9f3fa08f52069fe4a25a76e04a55bb2586b031bfb0393dce7f175dcdb52823e132a7dff6a894539beeb980a1aad2a792790be036be6977628149b2
2023-01-05Merge bitcoin/bitcoin#26761: wallet: fully migrate address book entries for ↵Andrew Chow
watchonly/solvable wallets 730e14a317ae45fe871c8d6f44a51936756bbbea test: wallet: check that labels are migrated to watchonly wallet (Sebastian Falbesoner) d5f4ae7fac0bceb0c9ad939b9a4fbdb85da0bf95 wallet: fully migrate address book entries for watchonly/solvable wallets (Sebastian Falbesoner) Pull request description: Currently `migratewallet` migrates the address book (i.e. labels and purposes) for watchonly and solvable wallets only in RAM, but doesn't persist them on disk. Fix this by adding another loop for both of the special wallet types after which writes the corresponding NAME and PURPOSE entries to the database in a single batch. Also adds a corresponding test that checks if labels were migrated correctly for a watchonly wallet. ACKs for top commit: achow101: ACK 730e14a317ae45fe871c8d6f44a51936756bbbea furszy: code ACK 730e14a3, left a non-blocking nit. aureleoules: ACK 730e14a317ae45fe871c8d6f44a51936756bbbea Tree-SHA512: 159487e11e858924ef762e0190ccaea185bdff239e3d2280c8d63c4ac2649ec71714dc4d53dec644f03488f91c3b4bbbbf3434dad23bc0fcecb6657f353ea766
2023-01-05Merge bitcoin/bitcoin#23829: refactor: use braced init for integer literals ↵MarcoFalke
instead of c style casts f2fc03ec856d7d19a20c482514350cced38f9504 refactor: use braced init for integer constants instead of c style casts (Pasta) Pull request description: See https://github.com/bitcoin/bitcoin/pull/23810 for more context. This is broken out from that PR, as it is less breaking, and should be trivial to review and merge. EDIT: Long term, the intention is to remove all C-style casts, as they can dangerously introduce reinterpret_casts. This is one step which removes a number of trivially removable C-style casts ACKs for top commit: aureleoules: ACK f2fc03ec856d7d19a20c482514350cced38f9504 Tree-SHA512: 2fd11b92c9147e3f970ec3e130e3b3dce70e707ff02950a8c697d4b111ddcbbfa16915393db20cfc8f384bc76f13241c9b994a187987fcecd16a61f8cc0af14c
2023-01-04Merge bitcoin/bitcoin#26747: wallet: fix confusing error / GUI crash on ↵Andrew Chow
cross-chain legacy wallet restore 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 test: add coverage for cross-chain wallet restore (Sebastian Falbesoner) 8c7222bda3f7136f312a6e57b76d6a2d0a114f68 wallet: fix GUI crash on cross-chain legacy wallet restore (Sebastian Falbesoner) Pull request description: Restoring a wallet backup from another chain should result in a dedicated error message (we have _"Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override."_ for that). Unfortunately this is currently not the case for legacy wallet restores, as in the course of cleaning up the newly created wallet directory a `filesystem_error` exception is thrown due to the directory not being empty; the wallet database did indeed load successfully (otherwise we wouldn't know that the chain doesn't match) and hence BDB-related files and directories are already created in the wallet directory. For bitcoind, this leads to a very confusing error message: ``` $ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat error code: -1 error message: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/test123"] ``` Even worse, the GUI crashes in such a scenario: ``` libc++abi: terminating with uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/foobar"] Abort trap (core dumped) ``` Fix this by simply deleting the whole folder via `fs::remove_all`. With this, the expected error message appears both for the `restorewallet` RPC call and in the GUI (as a message-box): ``` $ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat error code: -4 error message: Wallet loading failed. Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override. ``` ACKs for top commit: achow101: ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 aureleoules: ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 furszy: utACK 21ad4e26 Tree-SHA512: 313f6494c2fbe823bff9b975cb2d9410bb518977a1e59a5159ee9836bc012947fa50b56be0e41b1a2f50d9c0c7f4fddfdf4fbe479d8a59a6ee44bb389c804abc
2023-01-04Merge bitcoin/bitcoin#26752: wallet: Remove `mempool_sequence` from ↵glozow
interface methods 55696a0ac30bcfbd555f71cbc8eac23b725f7dcf wallet: remove `mempool_sequence` from `transactionRemovedFromMempool` (w0xlt) bf19069c53501231a2f3ba59afa067913ec4d3b2 wallet: remove `mempool_sequence` from `transactionAddedToMempool` (w0xlt) Pull request description: This PR removes `mempool_sequence` from `transactionRemovedFromMempool` and `transactionAddedToMempool`. `mempool_sequence` is not used in these methods, only in ZMQ notifications. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26752/commits/55696a0ac30bcfbd555f71cbc8eac23b725f7dcf Tree-SHA512: 621e89230bcb6edfed83e2758601a2b093822fc2dc4e9bfb00487e340f2bc4c5ac3bf6df3ca00b7fe55bb3df15858820f2bf698f403d2e48b915dd9eb47b63e0
2023-01-04Merge bitcoin/bitcoin#26020: test: Change coinselection parameter location ↵Andrew Chow
to make tests independent b942c94d153f83b77ef5d603211252d9abadde95 test: Change coinselection parameter location to make tests independent (yancy) Pull request description: the `subtract_fee_outputs` param is expected to be `true` for all subsequent tests. It should be defined outside of a single test so that if it's removed or changed, all subsequent tests won't fail. Currently if you remove this [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L304:L325) the following [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L327:L345) fails. This change makes the tests independent. ACKs for top commit: achow101: ACK b942c94d153f83b77ef5d603211252d9abadde95 aureleoules: ACK b942c94d153f83b77ef5d603211252d9abadde95. rajarshimaitra: tACK b942c94d153f83b77ef5d603211252d9abadde95 theStack: ACK b942c94d153f83b77ef5d603211252d9abadde95 Tree-SHA512: 461e19d15351318102ef9f96c68442365d8ca238c48ad7aefe23e8532b33b91dadf6c7840c7894574bccede6da162a55ad7a6f6a330d61a11ce804e68ddc5e9c
2023-01-04refactor: Add sanity checks in LabelFromValueAurèle Oulès
2023-01-04rpc: Sanitize label name in various RPCsAurèle Oulès
- importprivkey - importaddress - importpubkey - listtransactions - listsinceblock - importmulti - importdescriptors
2023-01-04Merge bitcoin/bitcoin#26795: rpc: Correct RPCHelpMan for ↵MarcoFalke
fundrawtransaction's input_weights field 927b8d4e0cddd89e1f71093c10bd697c25b7a7d8 rpc: Correct RPCHelpMan for fundrawtransaction's input_weights field (jdjkelly@gmail.com) Pull request description: `input_weights` is incorrectly documented as a fixed length JSON array, but it is actually a JSON array of JSON objects - this commit changes `input_weights` to use `RPCArg::Type::OBJ` The behavior of `input_weights` as an object exists as a functional test in [wallet_fundrawtransaction.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_fundrawtransaction.py). ACKs for top commit: achow101: ACK 927b8d4e0cddd89e1f71093c10bd697c25b7a7d8 Tree-SHA512: 384f5e16be36dba670d64d96f16f1fde2d0d51357e1094ae13eb71d004af0f4dc8bac965b4d2d724ccf64fb671faad37b73055152a9882af24f65dfceaf1e5fb
2023-01-03refactor: use braced init for integer constants instead of c style castsPasta
2023-01-03Merge bitcoin/bitcoin#26661: wallet: Coin Selection, return accurate error ↵Andrew Chow
messages 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy) f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy) 0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy) 7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy) e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy) Pull request description: Work decoupled from #25806, which cleanup and improves the Coin Selection flow further. Adding the capability to propagate specific error messages from the Coin Selection process to the user. Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally. Letting us instruct the user how to proceed under certain circumstances. The following error messages were added: 1) If the selection result exceeds the maximum transaction weight, we now will return: -> "The inputs size exceeds the maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs". 2) If the user pre-selected inputs and disallowed the automatic coin selection process (no other inputs are allowed), we now will return: -> "The preselected coins total amount does not cover the transaction target. Please allow other inputs to be automatically selected or include more coins manually". 3) The double-counted preset inputs during Coin Selection error will now throw an "internal bug detected" message instead of crashing the node. The essence of this work comes from several comments: 1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665 2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491 3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825 4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845) ACKs for top commit: ishaanam: crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 achow101: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 aureleoules: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 theStack: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 :city_sunrise: Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
2023-01-03wallet: unify FindNonChangeParentOutput functionsfurszy
The function is only used in ListCoins.
2023-01-03wallet: simplify ListCoins implementationfurszy
Can remove the locked coins lookup if we include them directly inside the AvailableCoins result
2023-01-03Merge bitcoin/bitcoin#26192: rpc: Improve error when wallet is already loadedAndrew Chow
04609284ad5e0b72651f2d4b43263461ada40816 rpc: Improve error when wallet is already loaded (Aurèle Oulès) Pull request description: Currently, trying to load a descriptor (sqlite) wallet that is already loaded throws the following error: > error code: -4 > error message: > Wallet file verification failed. SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of Bitcoin Core? I don't think it is very clear what it means for a user. While a legacy wallet would throw: > error code: -35 > error message: > Wallet file verification failed. Refusing to load database. Data file '/home/user/.bitcoin/signet/wallets/test_wallet/wallet.dat' is already loaded. This PR changes the error message for both types of wallet to: > error code: -35 > error message: > Wallet file verification failed. Wallet "test_wallet" is already loaded. ACKs for top commit: achow101: ACK 04609284ad5e0b72651f2d4b43263461ada40816 hernanmarino: ACK 0460928 theStack: Tested ACK 04609284ad5e0b72651f2d4b43263461ada40816 Tree-SHA512: a8f3d5133bfaef7417a6c05d160910ea08f32ac62bfdf7f5ec305ff5b62e9113b55f385abab4d5a4ad711aabcb1eb7ef746eb41f841b196e8fb5393ab3ccc01e
2023-01-03Merge bitcoin/bitcoin#25789: test: clean and extend availablecoins_tests ↵Andrew Chow
coverage 9622fe64b8785430c71d4abc8637075026dc690c test: move coins result test to wallet_tests.cpp (furszy) f69347d0588647ff9a4e986c7be987827a0417f4 test: extend and simplify availablecoins_tests (furszy) 212ccdf2c2b70d973b18ae78f0158ec5f0c3bbb4 wallet: AvailableCoins, add arg to include/skip locked coins (furszy) Pull request description: Negative PR with extended test coverage :). 1) Cleaned duplicated code and added coverage for the 'AvailableCoins' incremental result. 2) The class `AvailableCoinsTestingSetup` inside `availablecoins_tests.cpp` is a plain copy of `ListCoinsTestingSetup` that is inside `wallet_tests.cpp`. So, deleted the file and moved the `BasicOutputTypesTest` test case to `wallet_tests.cpp`. 3) Added arg to include/skip locked coins from the `AvailableCoins` result. This is needed for point (1) as otherwise the wallet will spend the coins that we recently created due its closeness to the recipient amount. Note: this last point comes from #25659 where I'm using the same functionality to clean/speedup another flow as well. ACKs for top commit: achow101: ACK 9622fe64b8785430c71d4abc8637075026dc690c theStack: ACK 9622fe64b8785430c71d4abc8637075026dc690c aureleoules: reACK 9622fe64b8785430c71d4abc8637075026dc690c, nice cleanup! Tree-SHA512: 1ed9133120bfe8815455d1ad317bb0ff96e11a0cc34ee8098716ab9b001749168fa649212b2fa14b330c1686cb1f29039ff1f88ae306db68881b0428c038f388
2023-01-03Merge bitcoin/bitcoin#25932: refactor: Simplify backtrack logicAndrew Chow
81d4a2b14ff65fe07085ef2a967a466015370ce3 refactor: Move feerate comparison invariant outside of the loop (yancy) 365aca40453995163bbd17231251512f9f9a103b refactor: Simplify feerate comparison statement (yancy) Pull request description: This is a small nit, however I think it's more understandable to write: `utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee` vs `(utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0` ACKs for top commit: Xekyo: ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3 achow101: ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3 aureleoules: ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3 Tree-SHA512: 3e89377989c36716b53114fe40178261671dde5688075fab1c21ec173ac310f8c84ed6af90354d7c329176cb7262dfcaa7191fd19847d3b7147a9a10c3e31176
2023-01-03Merge bitcoin/bitcoin#26702: refactor: walletdb: drop unused `FindWalletTx` ↵Andrew Chow
parameter and rename f496528556a67107d3d75d9c2ae345f7f4565d77 walletdb: refactor: drop unused `FindWalletTx` parameter and rename (Sebastian Falbesoner) Pull request description: Since commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b ("Remove -zapwallettxes"), the `FindWalletTx` helper is only needed to read tx hashes, so drop the other parameter and rename the method accordingly. ACKs for top commit: S3RK: code review ACK f496528556a67107d3d75d9c2ae345f7f4565d77 achow101: ACK f496528556a67107d3d75d9c2ae345f7f4565d77 vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/26702/commits/f496528556a67107d3d75d9c2ae345f7f4565d77 Tree-SHA512: ead85bc724462f9e920f9d7fe89679931361187579ffd6e63427c8bf5305cd5f71da24ed84f3b1bd22a12be46b5abec13f11822e71a3e1a63bf6cf49de950ab5
2023-01-02rpc: Correct RPCHelpMan for fundrawtransaction's input_weights fieldjdjkelly@gmail.com
input_weights is incorrectly documented as a fixed length JSON array, but it is actually a JSON array of JSON objects - this commit changes input_weights to use RPCArg::Type::OBJ
2022-12-30rpc: Remove duplicate field in RPCHelpMan for gettransactionsJoshua Kelly
The field 'comment' appears twice in TransactionDescriptionString, incorrectly - this commit removes the instance of the comment field without a description, preserving the one with a description
2022-12-28wallet: fully migrate address book entries for watchonly/solvable walletsSebastian Falbesoner
Currently `migratewallet` migrates the address book (i.e. labels and purposes) for watchonly and solvable wallets only in RAM, but doesn't persist them on disk. Fix this by adding another loop for both of the special wallet types after which writes the corresponding NAME and PURPOSE entries to the database in a single batch.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 18:29:40 +0000