aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2024-06-27Merge bitcoin/bitcoin#30050: refactor, wallet: get serialized size of ↵Ava Chow
`CRecipient`s directly a9c7300135f0188daa5bce5491e2daf2dd8da8ae move-only: refactor CreateTransactionInternal (josibake) adc6ab25bba42ce9e7ed6bd7599aabb6dead6987 wallet: use CRecipient instead of CTxOut (josibake) Pull request description: Broken out from #28201 --- In order to estimate fees properly, we need to know what the final serialized transaction size will be. This PR refactors `CreateTransactionInternal` to: * Get the serialized size directly from the `CRecipient`: this sets us up in a future PR to calculate the serialized size of silent payment `CTxDestinations` (see https://github.com/bitcoin/bitcoin/pull/28201/commits/797e21c8c1bf393e668eeef8bb7ee9cae1e5e41e) * Use the new `GetSerializeSizeForRecipient` to move the serialize size calculation to *before* coin selection and the output creation to *after* coin selection: this also sets us up for silent payments sending in a future PR in that silent payments outputs cannot be created until after the inputs to the transaction have been selected Aside from the silent payments use case, I think this structure logically makes more sense. As a reminder, move-only commits are best reviewed with something like `git diff -w --color-moved=dimmed-zebra` ACKs for top commit: S3RK: reACK a9c7300135f0188daa5bce5491e2daf2dd8da8ae achow101: ACK a9c7300135f0188daa5bce5491e2daf2dd8da8ae rkrux: tACK [a9c7300](https://github.com/bitcoin/bitcoin/pull/30050/commits/a9c7300135f0188daa5bce5491e2daf2dd8da8ae) Tree-SHA512: 412e1764b98f7428c8530c3a68f55e32063d6b66ab2ff613e1c7e12d49b049807cb60055cfe7f7e8ffe7ac7f0f9931427cbfd3efe7d4f97a5a0f6d1bf1aaac58
2024-06-26Merge bitcoin/bitcoin#30309: wallet: notify when preset + automatic inputs ↵Ava Chow
exceed max weight 72b226882fe2348a9a66aee1d8d21b4e2d275e68 wallet: notify when preset + automatic inputs exceed max weight (furszy) Pull request description: Small change. Found it while finishing my review on #29523. This does not interfere with it. Basically, we are erroring out early when the automatic coin selection process exceeds the maximum weight, but we are not doing so when the user-preselected inputs combined with the wallet-selected inputs exceed the maximum weight. This change avoids signing all inputs before erroring out and introduces test coverage for `fundrawtransaction`. ACKs for top commit: achow101: ACK 72b226882fe2348a9a66aee1d8d21b4e2d275e68 tdb3: re ACK for 72b226882fe2348a9a66aee1d8d21b4e2d275e68 rkrux: tACK [72b2268](https://github.com/bitcoin/bitcoin/pull/30309/commits/72b226882fe2348a9a66aee1d8d21b4e2d275e68) ismaelsadeeq: utACK 72b226882fe2348a9a66aee1d8d21b4e2d275e68 Tree-SHA512: d77be19231023383a9c79a5d66b642dcbc6ebfc31a363e0b9f063c44898720a7859ec211cdbc0914ac7a3bfdf15e52fb8fc20d97f171431f70492c0f159dbc36
2024-06-21wallet: notify when preset + automatic inputs exceed max weightfurszy
This also avoids signing all inputs prior to erroring out.
2024-06-20refactor: remove extraneous lock annotations from function definitionsCory Fields
These annotations belong in the declarations rather than the definitions. While harmless now, future versions of clang may warn about these.
2024-06-20Merge bitcoin/bitcoin#30307: fuzz: Fix wallet_bdb_parser 32-bit unhandled ↵merge-script
fseek error fa7bc9bbca9348cf31b97bee0789ea7caeec635c fuzz: Fix wallet_bdb_parser 32-bit unhandled fseek error (MarcoFalke) Pull request description: `std::fseek` on 64-bit past the end of the file may work fine (the following read would fail). However, on 32-bit it may fail early. Fix it, by ignoring the error, treating it similar to a read error. This was found by OSS-Fuzz. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69414 ACKs for top commit: TheCharlatan: ACK fa7bc9bbca9348cf31b97bee0789ea7caeec635c brunoerg: utACK fa7bc9bbca9348cf31b97bee0789ea7caeec635c Tree-SHA512: 7a752a005837bae6846ce315a7b3b1a5fe1f440c7797c750f2c0bbb20b1ef1537cd390c425747c0c85d012655e2f908bd300ea084f82e5ada19badbf826e1ec9
2024-06-20Merge bitcoin/bitcoin#30248: refactor: Add explicit cast to ↵merge-script
expected_last_page to silence fuzz ISan fa9cb101cf33b57b2c043b29f1f3d55b990ba4c6 refactor: Add explicit cast to expected_last_page to silence fuzz ISan (MarcoFalke) Pull request description: Fixes #30247 I don't think this implicit cast can lead to any bugs, so make it explicit to silence the fuzz integer sanitizer. Can be tested with: ``` FUZZ=wallet_bdb_parser UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/1376869be72eebcc87fe737020add634b1a29533 ``` After downloading the raw fuzz input from https://github.com/bitcoin-core/qa-assets/blob/24c507b3ea6263e6b121fb8dced01123065c44c2/fuzz_seed_corpus/wallet_bdb_parser/1376869be72eebcc87fe737020add634b1a29533 ACKs for top commit: dergoegge: utACK fa9cb101cf33b57b2c043b29f1f3d55b990ba4c6 Tree-SHA512: 226dcc58be8d70b4eec1657f232c9c6648b5dac5eb2706e7390e65ce0a031fbaf8afce97d71a535c8294467dca4757c96f294d8cc03d5e6a1c0a036b0e070325
2024-06-19fuzz: Fix wallet_bdb_parser 32-bit unhandled fseek errorMarcoFalke
2024-06-17move-only: refactor CreateTransactionInternaljosibake
Move the output serialization size and dust calculation into the loop where the outputs are iterated over to calculate the total sum. Move the code for adding the the txoutputs to the transaction to after coin selection. While this code structure generally follows a more logical flow, the primary motivation for moving the code for adding outputs to the transaction sets us up nicely for silent payments (in a future PR): we need to know the input set before generating the final output scriptPubKeys.
2024-06-17wallet: use CRecipient instead of CTxOutjosibake
Now that a CRecipient holds a CTxDestination, we can get the serialized size and determine if the output is dust using the CRecipient directly. This does not change any current behavior, but provides a nice generalization that can be used to apply special logic to a CTxDestination serialization and dust calculations in the future. Specifically, in a later PR when support for `V0SilentPayment` destinations is added, we need to use `WitnessV1Taproot` as the scriptPubKey for serialized size calcuations whenever the `CRecipient` destination is a `V0SilentPayment` destination.
2024-06-14Merge bitcoin/bitcoin#27969: bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE ↵Ava Chow
when user specifies fee_rate f58beabe754363cb7d5b24032fd392654b9514ac test: bumpfee with user specified fee_rate ignores walletIncrementalRelayFee (ismaelsadeeq) 436e88f4336199998184cbfa5d1c889ffaefbfb5 bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee rate (ismaelsadeeq) Pull request description: Fixes #26973 When using the `bumpfee` RPC and manually specifying `fee_rate`, there should be no requirement that the new fee must be at least the sum of the original fee and `incrementalFee` (maximum of `relayIncrementalFee` and `WALLET_INCREMENTAL_RELAY_FEE`). This restriction should only apply when user did not specify `fee_rate`. > because the GUI doesn't let the user specify the new fee rate yet (https://github.com/bitcoin-core/gui/issues/647), it would be very annoying to have to bump 20 times to increment by 20 sat/vbyte. The restriction should instead be the new fee must be at least the sum of the original fee and `incrementalFee` (`relayIncrementalFee`) ACKs for top commit: achow101: ACK f58beabe754363cb7d5b24032fd392654b9514ac murchandamus: ACK f58beabe754363cb7d5b24032fd392654b9514ac Tree-SHA512: 193259f87173b7d5a8e68e0e29f2ca7e75c550e3cf0dee3d6d822b5b1e07c2e6dec0bfc8fb435855736ebced97a10dbdbfef72e8c5abde06fdefcba122f2e7f1
2024-06-12Merge bitcoin/bitcoin#29015: kernel: Streamline util libraryAva Chow
c7376babd19d0c858fef93ebd58338abd530c1f4 doc: Clarify distinction between util and common libraries in libraries.md (Ryan Ofsky) 4f74c59334d496f28e1a5c0d84c412f9020b366f util: Move util/string.h functions to util namespace (Ryan Ofsky) 4d05d3f3b42a41525aa6ec44b90f543dfab53ecf util: add TransactionError includes and namespace declarations (Ryan Ofsky) 680eafdc74021c1e0893c3a62404e607fd4724f5 util: move fees.h and error.h to common/messages.h (Ryan Ofsky) 02e62c6c9af4beabaeea58fb1ea3ad0dc5094678 common: Add PSBTError enum (Ryan Ofsky) 0d44c44ae33434f366229c612d6edeedf7658963 util: move error.h TransactionError enum to node/types.h (Ryan Ofsky) 9bcce2608dd2515dc35a0f0866abc9d43903c795 util: move spanparsing.h to script/parsing.h (Ryan Ofsky) 6dd2ad47922694d2ab84bad4dac9dd442c5df617 util: move spanparsing.h Split functions to string.h (Ryan Ofsky) 23cc8ddff472d259605d7790ba98a1900e77efab util: move HexStr and HexDigit from util to crypto (TheCharlatan) 6861f954f8ff42c87ad638037adae86a5bd89600 util: move util/message to common/signmessage (Ryan Ofsky) cc5f29fbea15d33e4d1aa95591253c6b86953fe7 build: move memory_cleanse from util to crypto (Ryan Ofsky) 5b9309420cc9721a0d5745b6ad3166a4bdbd1508 build: move chainparamsbase from util to common (Ryan Ofsky) ffa27af24da81a97d6c4912ae0e10bc5b6f17f69 test: Add check-deps.sh script to check for unexpected library dependencies (Ryan Ofsky) Pull request description: Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically: - Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity. - Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing. - Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of the util library. Motivation for this change is that the util library is a dependency of the kernel, and we should remove functionality from util that shouldn't be called by kernel code or kernel applications. These changes should also improve code organization and make functions easier to discover. Some of these same moves are (or were) part of #28690, but did not help with code organization, or made it worse, so it is better to move them and clean them up in the same PR so code only has to change one time. ACKs for top commit: achow101: ACK c7376babd19d0c858fef93ebd58338abd530c1f4 TheCharlatan: Re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4 hebasto: re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4. Tree-SHA512: 5bcef16c1255463b1b69270548711e7ff78ca0dd34e300b95e3ca1ce52ceb34f83d9ddb2839e83800ba36b200de30396e504bbb04fa02c6d0c24a16d06ae523d
2024-06-07refactor: Rename CTransaction::nVersion to versionAva Chow
In order to ensure that the change of nVersion to a uint32_t in the previous commit has no effect, rename nVersion to version in this commit so that reviewers can easily spot if a spot was missed or if there is a check somewhere whose semantics have changed.
2024-06-07refactor: Add explicit cast to expected_last_page to silence fuzz ISanMarcoFalke
2024-06-04Merge bitcoin/bitcoin#28307: rpc, wallet: fix incorrect segwit redeem script ↵Ava Chow
size limit 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 test: addmultisigaddress, coverage for script size limits (furszy) 53302a09817e5b799d345dfea432546a55a9d727 bugfix: addmultisigaddress, add unsupported operation for redeem scripts over 520 bytes (furszy) 9be6065cc03f2408f290a332b203eef9c9cebf24 test: coverage for 16-20 segwit multisig scripts (furszy) 9d9a91c4ea6b3bb32ef4131bca86f1d6683fc901 rpc: bugfix, incorrect segwit redeem script size used in signrawtransactionwithkey (furszy) 0c9fedfc45fa7cbd6801ca5fd756863ec9a6911c fix incorrect multisig redeem script size limit for segwit (furszy) f7a173b5785cda460470df9a74a0e0f94d7f9a18 test: rpc_createmultisig, decouple 'test_sortedmulti_descriptors_bip67' (furszy) 4f33dbd8f8c0e29f37b04e6af6d2c7905ecceaf6 test: rpc_createmultisig, decouple 'test_mixing_uncompressed_and_compressed_keys' (furszy) 25a81705d376e8c96dad45436ae3fca975b3daf5 test: rpc_createmultisig, remove unnecessary checkbalances() (furszy) b5a328943362cfac6e90fd4e1b167c357d53b7d4 test: refactor, multiple cleanups in rpc_createmultisig.py (furszy) 3635d432681847313c098f9827483372a840e70f test: rpc_createmultisig, remove manual wallet initialization (furszy) Pull request description: Fixing https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1674830104 and more. Currently, redeem scripts longer than 520 bytes, which are technically valid under segwit rules, have flaws in the following processes: 1) The multisig creation process fails to deduce the output descriptor, resulting in the generation of an incorrect descriptor. Additionally, the accompanying user warning is also inaccurate. 2) The `signrawtransactionwithkey` RPC command fail to sign them. 3) The legacy wallet `addmultisigaddress` wrongly discards them. The issue arises because most of these flows are utilizing the legacy spkm keystore, which imposes the [p2sh max redeem script size rule](https://github.com/bitcoin/bitcoin/blob/ded687334031f4790ef6a36b999fb30a79dcf7b3/src/script/signingprovider.cpp#L160) on all scripts. Which blocks segwit redeem scripts longer than the max element size in all the previously mentioned processes (`createmultisig`, `addmultisigaddress`, and `signrawtransactionwithkey`). This PR fixes the problem, enabling the creation of multisig output descriptors involving more than 15 keys and allowing the signing of these scripts, along with other post-segwit redeem scripts that surpass the 520-byte p2sh limit. Important note: Instead of adding support for these longer redeem scripts in the legacy wallet, an "unsupported operation" error has been added. The reasons behind this decision are: 1) The introduction of this feature brings about a compatibility-breaking change that requires downgrade protection; older wallets would be unable to interact with these "new" legacy wallets. 2) Considering the ongoing deprecation of the legacy spkm, this issue provides another compelling reason to transition towards descriptors. Testing notes: To easily verify each of the fixes, I decoupled the tests into standalone commits. So they can be cherry-picked on top of master. Where `rpc_createmultisig.py` (with and without the `--legacy-wallet` arg) will fail without the bugs fixes commits. Extra note: The initial commits improves the `rpc_createmultisig.py` test in many ways. I found this test very antiquated, screaming for an update and cleanup. ACKs for top commit: pinheadmz: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 theStack: Code-review ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 achow101: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 Tree-SHA512: 71794533cbd46b3a1079fb4e9d190d3ea3b615de0cbfa443466e14f05e4616ca90e12ce2bf07113515ea8113e64a560ad572bb9ea9d4835b6fb67b6ae596167f
2024-06-04Merge bitcoin/bitcoin#28074: fuzz: wallet, add target for `Crypter`Ava Chow
d7290d662f494503f28e087dd728b492c0bb2c5f fuzz: wallet, add target for Crypter (Ayush Singh) Pull request description: This PR adds fuzz coverage for `wallet/crypter`. Motivation: Issue [27272](https://github.com/bitcoin/bitcoin/issues/27272#issue-1628327906) I ran this for a long time with Sanitizers on and had no crashes; the average `exec/sec` also looks good to me. However, I would really appreciate it if some of the reviewers could try it on their machines too, and give their feedback. ACKs for top commit: maflcko: utACK d7290d662f494503f28e087dd728b492c0bb2c5f achow101: ACK d7290d662f494503f28e087dd728b492c0bb2c5f brunoerg: utACK d7290d662f494503f28e087dd728b492c0bb2c5f Tree-SHA512: f5c496cabdd3263a7e1ad49eeff702725336f76bf19a82e5dbbead082e990889dd43c851d0d2d6ab740f44b8ec2aa06defd9ff6b02be68b5f8b4eaf963f88599
2024-06-04Merge bitcoin/bitcoin#29997: rpc: Remove index-based Arg accessorAva Chow
fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 rpc: Remove index-based Arg accessor (MarcoFalke) Pull request description: The index-based Arg accessor is redundant with the name-based one. It does not provide any benefit to the code reader, or otherwise, so remove it. ACKs for top commit: stickies-v: re-ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54, addressed doc nits achow101: ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 ryanofsky: Code review ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54. One changes since last review are some documentation improvements Tree-SHA512: f9da1c049dbf38c3b47a8caf8d24d195c2d4b88c7ec45a9ccfb78f1e39f29cb86869f84b308f6e49856b074c06604ab634c90eb89c9c93d2a8169e070aa1bd40
2024-06-04Merge bitcoin/bitcoin#28979: wallet, rpc: document and update `sendall` ↵Ava Chow
behavior around unconfirmed inputs 71aae72e1fc998b2629d68a7301d85dc1b65641e test: test sendall does ancestor aware funding (ishaanam) 36757941a05b65c2b61a83820afdf5effd8fc9a2 wallet, rpc: implement ancestor aware funding for sendall (ishaanam) 544131f3fba9ea07fee29f9d3ee0116cd5d8a5b2 rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified (ishaanam) Pull request description: This PR: - Adds a functional test that `sendall` spends unconfirmed change - Adds a functional test that `sendall` spends regular unconfirmed inputs when specified by user - Adds ancestor aware funding to `sendall` by using `calculateCombinedBumpFee` and adjusting the effective value accordingly - Adds a functional test for ancestor aware funding in `sendall` ACKs for top commit: S3RK: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e achow101: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e furszy: ACK 71aae72e1f Tree-SHA512: acaeb7c65166ce53123a1d6cb5012197202246acc02ef9f37a28154cc93afdbd77c25e840ab79bdc7e0b88904014a43ab1ddea79d5337dc310ea210634ab61f0
2024-06-04Merge bitcoin/bitcoin#28366: Fix waste calculation in SelectionResultAva Chow
bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 Use `exact_target` shorthand in coinselector_tests (Murch) 7aa7e30441fe77bf8e8092916e36b004bbbfe2a7 Fold GetSelectionWaste() into ComputeAndSetWaste() (Murch) Pull request description: PR #26152 moved waste calculation into SelectionResult to be able to correct the waste score on basis of the bump_fee_group_discount for overlapping ancestries. This left two functions with largely overlapping purpose, where one was simply a wrapper of the other. This PR cleans up the overlap, and fixes the double-meaning of `change_cost` where the `GetChange()` function assumed that no change was created when `change_cost` was set to 0. This behavior was exploited in a bunch of tests, but is problematic, because a `change_cost` of 0 is permitted with custom settings for feerate and discard_feerate (i.e. when they’re both 0). ACKs for top commit: achow101: ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 furszy: Code ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 ismaelsadeeq: Code Review ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 Tree-SHA512: 83a2688d45d719dc61a64b5180fe136107faccf401a59df65245c05d701748a03e85ed56fde8c9b7ef39a3ab54374dd3718c559bda5b3f55dafedfd7fed25161
2024-06-03Merge bitcoin/bitcoin#30134: fuzz: add more coverage for `ScriptPubKeyMan`merge-script
e3249f21111f1dd4beb66f10af933c34a36c30ac fuzz: add more coverage for `ScriptPubKeyMan` (brunoerg) Pull request description: This PR adds more coverage for `ScriptPubKeyMan`: - Check `GetKey` and `HasPrivKey` after adding descriptor key. - Cover `GetEndRange` and `GetKeyPoolSize`. - Cover `MarkUnusedAddresses` with the scripts from ScriptPubKeys and `GetMetadata` with the destinations from them. ACKs for top commit: marcofleon: Tested ACK e3249f21111f1dd4beb66f10af933c34a36c30ac. I ran the updated harness for ~9 hours on an empty corpus, generated a coverage report, and checked that the new functions mentioned were hit. Coverage of `scriptpubkeyman.cpp` increased. murchandamus: Tested ACK e3249f21111f1dd4beb66f10af933c34a36c30ac Tree-SHA512: cfab91f6c8401174842e79209c0e9225c08f011fe9b41d0a58bcec716ae4545eaf803867f899ed7b5fbcefea45711f91894e36df082ba19732dd310cd9e61a79
2024-05-29fuzz: Handle missing BDBRO errorsAva Chow
Adds error messages that were not being handled. Also removes error messages that no longer exist.
2024-05-28Use `exact_target` shorthand in coinselector_testsMurch
2024-05-24Fold GetSelectionWaste() into ComputeAndSetWaste()Murch
Both `GetSelectionWaste()` and `ComputeAndSetWaste()` now are part of `SelectionResult`. Instead of `ComputeAndSetWaste()` being a wrapper for `GetSelectionWaste()`, we combine them to a new function `RecalculateWaste()`. As I was combining the logic of the two functions, I noticed that `GetSelectionWaste()` was making the odd assumption that the `change_cost` being set to zero means that no change is created. However, if we build transactions at a feerate of zero with the `discard_feerate` also set to zero, we'd organically have a `change_cost` of zero, even when we create change on a transaction. This commit cleans up this duplicate meaning of `change_cost` and relies on `GetChange()` to figure out whether there is change on basis of the `min_viable_change` and whatever is left after deducting fees. Since this broke a bunch of tests that relied on the double-meaning of `change_cost` a bunch of tests had to be fixed.
2024-05-24fuzz: Fix wallet_bdb_parser stdlib error matchingMarcoFalke
2024-05-23Merge bitcoin/bitcoin#30115: rpc: avoid copying into UniValueRyan Ofsky
d7707d9843b03f20d2a8c5a45d7b3db58e169e6f rpc: avoid copying into UniValue (Cory Fields) Pull request description: These are the simple (and hopefully obviously correct) copies that can be moves instead. This is a follow-up from https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842 As it turns out, there are hundreds of places where we copy UniValues needlessly. It should be the case that moves are always preferred over copies, so there should be no downside to these changes. willcl-ark, however, noticed that memory usage may increase in some cases. Logically this makes no sense to me. The only plausible explanation imo is that because the moves are faster, more ops/second occur in some cases. This list of moves was obtained by changing the function signatures of the UniValue functions to accept only rvalues, then compiling and fixing them up one by one. There still exist many places where copies are being made. These can/should be fixed up, but weren't done here for the sake of doing the easy ones first. I ran these changes through clang-tidy with `performance-move-const-arg` and `bugprone-use-after-move` and no bugs were detected (though that's obviously not to say it can be trusted 100%). As stated above, there are still lots of other less trivial fixups to do after these including: - Using non-const UniValues where possible so that moves can happen - Refactoring code in order to be able to move a UniValue without introducing a use-after-move - Refactoring functions to accept UniValues by value rather than by const reference ACKs for top commit: achow101: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f ryanofsky: Code review ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f. No changes since last review other than rebase. I agree benchmarks showing increased peak memory usage and RSS are surprising, but number of allocations is down as expected, and runtime is also decreased. willcl-ark: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f Tree-SHA512: 7f511be73984553c278186286a7d161a34b2574c7f5f1a0edc87c2913b4c025a0af5241ef9af2df17547f2e4ef79710aa5bbb762fc9472435781c0488dba3435
2024-05-22Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning ↵merge-script
in PollutePubKey 2289d4524053ab71c0d9133987cb36412797c1a2 wallet, tests: Avoid stringop-overflow warning in PollutePubKey (Ava Chow) Pull request description: Fixes #30114 ACKs for top commit: maflcko: ACK 2289d4524053ab71c0d9133987cb36412797c1a2 with g++ 14.1.1 🦄 theStack: utACK 2289d4524053ab71c0d9133987cb36412797c1a2 laanwj: ACK 2289d4524053ab71c0d9133987cb36412797c1a2 Tree-SHA512: 173c3c299bdd890f73e8a67a37880dbf816265e8b3c8298557ef2fc4670f5447005c0d2d81726f9bc43f6a69d677365d90a604354b3cbab0e3c52c4526d0407e
2024-05-21wallet, tests: Avoid stringop-overflow warning in PollutePubKeyAva Chow
2024-05-21Merge bitcoin/bitcoin#26606: wallet: Implement independent BDB parsermerge-script
d51fbab4b32d56765e8faab6ad01245fb259b0ca wallet, test: Be able to always swap BDB endianness (Ava Chow) 0b753156ce60c29efb2386954ba7555ad8f642f5 test: Test bdb_ro dump of wallet without reset LSNs (Ava Chow) c1984f128284589423b7e0cc06c9a3b23a242d95 test: Test dumping dbs with overflow pages (Ava Chow) fd7b16e391ed320e35255157a28be14c947ef30a test: Test dumps of other endian BDB files (Ava Chow) 6ace3e953f0864bd7818f040c59a1bc70aa47512 bdb: Be able to make byteswapped databases (Ava Chow) d9878903fb34939dee8e1462f079acc68110253d Error if LSNs are not reset (Ava Chow) 4d7a3ae78e55f25868979f1bd920857a4aecb825 Berkeley RO Database fuzz test (TheCharlatan) 3568dce9e93295674cdf5458c5bdf93ff01fd0a2 tests: Add BerkeleyRO to db prefix tests (Ava Chow) 70cfbfdadf16d3b115309c6938f07ef5b96c7cc1 wallettool: Optionally use BERKELEY_RO as format when dumping BDB wallets (Ava Chow) dd57713f6ede3d46e97ee7df87c10001b0bf4c3d Add MakeBerkeleyRODatabase (Ava Chow) 6e50bee67d1d58aecd8a0ce8b7c3f5a7979365f5 Implement handling of other endianness in BerkeleyRODatabase (Ava Chow) cdd61c9cc108df8e13f4e3891ff2c96355b3ee38 wallet: implement independent BDB deserializer in BerkeleyRODatabase (Ava Chow) ecba23097955dad7208baa687fc405c846aee794 wallet: implement BerkeleyRODatabase::Backup (Ava Chow) 0c8e72847603540bb29b8b8aeb80fa3f2e3a2c9a wallet: implement BerkeleyROBatch (Ava Chow) 756ff9b478484b17c4a6e65c171c2e4fecb21ad4 wallet: add dummy BerkeleyRODatabase and BerkeleyROBatch classes (Ava Chow) ca18aea5c4975ace4e307be96c74641d203fa389 Add AutoFile::seek and tell (Ava Chow) Pull request description: Split from #26596 This PR adds `BerkeleyRODatabase` which is an independent implementation of a BDB file parser. It provides read only access to a BDB file, and can therefore be used as a read only database backend for wallets. This will be used for dumping legacy wallet records and migrating legacy wallets without the need for BDB itself. Wallettool's `dump` command is changed to use `BerkeleyRODatabase` instead of `BerkeleyDatabase` (and `CWallet` itself) to demonstrate that this parser works and to test it against the existing wallettool functional tests. ACKs for top commit: josibake: reACK https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca TheCharlatan: Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca furszy: reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca laanwj: re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca theStack: ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
2024-05-20rpc: avoid copying into UniValueCory Fields
These are simple (and hopefully obviously correct) copies that can be moves instead.
2024-05-16wallet, test: Be able to always swap BDB endiannessAva Chow
2024-05-16bdb: Be able to make byteswapped databasesAva Chow
Byteswapped databases make it easier to test opening and deserializing other endian databases.
2024-05-16Error if LSNs are not resetAva Chow
2024-05-16Berkeley RO Database fuzz testTheCharlatan
2024-05-16tests: Add BerkeleyRO to db prefix testsAva Chow
2024-05-16wallettool: Optionally use BERKELEY_RO as format when dumping BDB walletsAva Chow
In order to ease the transition to not having BDB, make the dump tool use DatabaseFormmat::BERKELEY_RO when -withinternalbdb is set.
2024-05-16Add MakeBerkeleyRODatabaseAva Chow
Implements MakeBerkeleyRODatabase and adds DatabaseFormat::BERKELEY_RO so that MakeDatabase can use BerkeleyRO as the backend database.
2024-05-16Implement handling of other endianness in BerkeleyRODatabaseAva Chow
2024-05-16wallet: implement independent BDB deserializer in BerkeleyRODatabaseAva Chow
BerkeleyRODatabase is intended for use after BDB is removed, so it needs to be able to read all of the records from a BDB file. Thus an independent deserializer for BDB data files is implemented in it. This deserializer is targeted towards the data files that Bitcoin Core creates so it does not fully support all of BDB's features (e.g. other database types, encryption, etc.).
2024-05-16fuzz: add more coverage for `ScriptPubKeyMan`brunoerg
2024-05-16util: Move util/string.h functions to util namespaceRyan Ofsky
There are no changes to behavior. Changes in this commit are all additions, and are easiest to review using "git diff -U0 --word-diff-regex=." options. Motivation for this change is to keep util functions with really generic names like "Split" and "Join" out of the global namespace so it is easier to see where these functions are defined, and so they don't interfere with function overloading, especially since the util library is a dependency of the kernel library and intended to be used with external code.
2024-05-16util: add TransactionError includes and namespace declarationsRyan Ofsky
Add TransactionError to node namespace and include it directly instead of relying on indirect include through common/messages.h This is a followup to a previous commit which moved the TransactionError enum. These changes were done in a separate followup just to keep the previous commit more minimal and easy to review.
2024-05-16util: move fees.h and error.h to common/messages.hRyan Ofsky
Move enum and message formatting functions to a common/messages header where they should be more discoverable, and also out of the util library, so they will not be a dependency of the kernel The are no changes in behavior and no changes to the moved code.
2024-05-16common: Add PSBTError enumRyan Ofsky
Add separate PSBTError enum instead of reusing TransactionError enum for PSBT operations, and drop unused error codes. The error codes returned by PSBT operations and transaction broadcast functions mostly do not overlap, so using an unified enum makes it harder to call any of these functions and know which errors actually need to be handled. Define PSBTError in the common library because PSBT functionality is implemented in the common library and used by both the node (for rawtransaction RPCs) and the wallet.
2024-05-16util: move error.h TransactionError enum to node/types.hRyan Ofsky
New node/types.h file is analagous to existing wallet/types.h and is a better place to define simple node types that are shared externally with wallet and gui code than the util library. Motivation for this change is to completely remove util/error.h file currently holding TransactionError in a followup commit.
2024-05-16util: move util/message to common/signmessageRyan Ofsky
Move util/message to common/signmessage so it is named more clearly, and because the util library is not supposed to depend on other libraries besides the crypto library. The signmessage functions use CKey, CPubKey, PKHash, and DecodeDestination functions in the consensus and common libraries.
2024-05-15rpc: Remove index-based Arg accessorMarcoFalke
2024-05-13wallet: implement BerkeleyRODatabase::BackupAva Chow
2024-05-13wallet: implement BerkeleyROBatchAva Chow
Implement ReadKey and HasKey of BerkeleyROBatch, and Next of BerkeleyROCursor. Also adds the containers for records to BerkeleyRODatabase so that BerkeleyROBatch will be able to access the records.
2024-05-13wallet: add dummy BerkeleyRODatabase and BerkeleyROBatch classesAva Chow
BerkeleyRODatabase and BerkeleyROBatch will be used to access a BDB file without the use of BDB. For now, these are dummy classes.
2024-05-08Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with ↵Ava Chow
specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
2024-05-03bugfix: addmultisigaddress, add unsupported operation for redeem scripts ↵furszy
over 520 bytes Due to a bug in the legacy wallet, the p2sh maximum script size limit is also imposed on 'p2sh-segwit' and 'bech32' redeem scripts. Although redeem scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are technically valid for segwit output types, we don't want to enable this feature in legacy wallets for the following reasons: 1) It introduces a compatibility-breaking change requiring downgrade protection; older wallets would be unable to interact with these "new" legacy wallets. 2) Considering the ongoing deprecation of the legacy spkm, this issue adds another good reason to transition towards descriptors.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 19:14:06 +0000