aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-07-30Merge bitcoin/bitcoin#28118: test: Add SyncWithValidationInterfaceQueue to ↵fanquake
mockscheduler RPC fabef121b0cdfac6ec1985f6c08c5685a886ba5a refactor: Use EnsureAnyNodeContext (MarcoFalke) fa1640617e061431059908fbf496dccca6b4e112 test: Add SyncWithValidationInterfaceQueue to mockscheduler RPC (MarcoFalke) Pull request description: There should be no risk or downside in adding a call to `SyncWithValidationInterfaceQueue` here. In fact, it will make tests less brittle. For example, * If one sets the timeouts in `test/functional/feature_fee_estimation.py` to `0`, on `master` the test will fail and here it will pass. * It may avoid a rare (theoretic) intermittent issue in https://github.com/bitcoin/bitcoin/pull/28108/files#r1268966663 ACKs for top commit: TheCharlatan: ACK fabef121b0cdfac6ec1985f6c08c5685a886ba5a furszy: Code review ACK fabef121. Convinced by checking all current tests usages. Tree-SHA512: c9e9a536a8721d1b3f267a66b40578b34948892301affdcad121ef8e02bf17037305d0dd53aa94b1b064753e66f9cfb31823b916b707a9d812627f502b818003
2023-07-28Merge bitcoin/bitcoin#28162: refactor: Revert additional univalue check in ↵fanquake
ParseSighashString 06199a995f20c55583f6948cfe99e608679fcdf1 refactor: Revert addition of univalue sighash string check (TheCharlatan) 0b47c1621524a96b79cbdc3c45ee5ad36e7ba541 doc: Correct release-notes for sighashtype exceptions (TheCharlatan) Pull request description: This is a follow up for #28113. The string type check is already done by the rpc parser / RPCHelpMan. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274568557). Also correct the release note for the correct sighashtype exception change. There is no change in the handling of non-string sighashtype arugments. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274567555). ACKs for top commit: MarcoFalke: lgtm ACK 06199a995f20c55583f6948cfe99e608679fcdf1 jonatack: Tested ACK 06199a995f20c55583f6948cfe99e608679fcdf1 stickies-v: ACK 06199a995f20c55583f6948cfe99e608679fcdf1 Tree-SHA512: 3faa6b3d2247624c0973df8d79c09fbf1f90ffb99f1be484e359b528f485c31affea45976759bd206e4c81cbb54ebba5ad0ef4127d1deacbfe2a58153fcc94ee
2023-07-28Merge bitcoin/bitcoin#28168: refactor: Remove unused raw-pointer read helper ↵fanquake
from univalue fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Remove unused raw-pointer read helper from univalue (MarcoFalke) Pull request description: The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`. Fix both issues by removing them. Also, simplify the tests code by removing a `std::string` constructor where possible. ACKs for top commit: stickies-v: utACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 TheCharlatan: tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Tree-SHA512: 60c154c1046f01551335af79bf820a6104844f63e89977271b4336b3cd06ac3bab1379e18b7bc61d12bef7446029e91c16541ddecf9e88bc8bc897fc1f6ee2c8
2023-07-27Merge bitcoin/bitcoin#27888: Fuzz: a more efficient descriptor parsing targetAndrew Chow
131314b62e899f95d1863083d303b489b3212b16 fuzz: increase coverage of the descriptor targets (Antoine Poinsot) 90a24741e79cbf20d4456050f0fe39c3f88f5246 fuzz: add a new, more efficient, descriptor parsing target (Antoine Poinsot) d60229ede54e05724d444eaba02a9ed72f5ada02 fuzz: make the parsed descriptor testing into a function (Antoine Poinsot) Pull request description: The current descriptor parsing fuzz target requires valid public or private keys to be provided. This is unnecessary as we are only interested in fuzzing the descriptor parsing logic here (other targets are focused on fuzzing keys serializations). And it's pretty inefficient, especially for formats that need a checksum (`xpub`, `xprv`, WIF). This introduces a new target that mocks the keys as an index in a list of precomputed keys. Keys are represented as 2 hex characters in the descriptor. The key type (private, public, extended, ..) is deterministically based on this one-byte value. Keys are deterministically generated at target initialization. This is much more efficient and also largely reduces the size of the seeds. TL;DR: for instance instead of requiring the fuzzer to generate a `pk(xpub6DdBu7pBoyf7RjnUVhg8y6LFCfca2QAGJ39FcsgXM52Pg7eejUHLBJn4gNMey5dacyt4AjvKzdTQiuLfRdK8rSzyqZPJmNAcYZ9kVVEz4kj)` to parse a valid descriptor, it just needs to generate a `pk(03)`. Note we only mock the keys themselves, not the entire descriptor key expression. As we want to fuzz the real code that parses the rest of the key expression (origin, derivation paths, ..). This is a target i used for reviewing #17190 and #27255, and figured it was worth PR'ing on its own since the added complexity for mocking the keys is minimal and it could help prevent introducing bugs to the descriptor parsing logic much more efficiently. ACKs for top commit: MarcoFalke: re-ACK 131314b62e899f95d1863083d303b489b3212b16 🐓 achow101: ACK 131314b62e899f95d1863083d303b489b3212b16 Tree-SHA512: 485a8d6a0f31a3a132df94dc57f97bdd81583d63507510debaac6a41dbbb42fa83c704ff3f2bd0b78c8673c583157c9a3efd79410e5e79511859e1470e629118
2023-07-27Merge bitcoin/bitcoin#28148: refactor: consistently use ApplyArgsManOptions ↵Andrew Chow
for PeerManager::Options 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da refactor: deduplicate ignores_incoming_txs (stickies-v) 5f41afcc46913dbd4b5f08e622c5f74cd1eb50a5 refactor: set ignore_incoming_txs in ApplyArgsManOptions (stickies-v) Pull request description: Consistently use `ApplyArgsManOptions` for `PeerManager::Options`, and initialize `PeerManager::Options` early to avoid reading `"-blocksonly"` twice. Suggested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268400386 and also requested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273346189. No behaviour change, but the [`TestingSetup`](https://github.com/bitcoin/bitcoin/blob/e35fb7bc48d360585b80d0c7f89ac5087c1d405e/src/test/util/setup_common.cpp#L255-L256) is now also able to access `"-blocksonly"`. ACKs for top commit: MarcoFalke: lgtm ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da achow101: ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da TheCharlatan: ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da dergoegge: utACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da Tree-SHA512: 6cb489d79ac2a87e8faedb76c96973ab3fc597426f274a90a3ffd0bc5fe3f2b25db9c7ec2e55a0c806c2bcbc0fdded6e228adb43d2cd81f14fd6552863847698
2023-07-27Remove unused raw-pointer read helper from univalueMarcoFalke
2023-07-27refactor: Revert addition of univalue sighash string checkTheCharlatan
This check is already done by the rpc parser. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. To make the fuzz test more accurate and not swallow all runtime errors, add a check that the passed in UniValue sighash argument is either a string or null. Co-authored-by: stickies-v <stickies-v@protonmail.com>
2023-07-26Merge bitcoin/bitcoin#28127: refactor: Remove C-style const-violating cast, ↵fanquake
Use reinterpret_cast fa9108f85afdc926fd6a8b96cc2acff7ca25d7a8 refactor: Use reinterpret_cast where appropriate (MarcoFalke) 3333f950d49f13662842650ae76599a0dff052eb refactor: Avoid casting away constness (MarcoFalke) fa6394dd10ae71755e46fd523dd43c2a1f2b832d refactor: Remove unused C-style casts (MarcoFalke) Pull request description: Using a C-style cast to convert pointer types to a byte-like pointer type has many issues: * It may accidentally and silently throw away `const`. * It forces reviewers to check that it doesn't accidentally throw away `const`. For example, on current master a `const char*` is cast to `unsigned char*` (without `const`), see https://github.com/bitcoin/bitcoin/blob/d23fda05842ba4539b225bbab01b94df0060f697/src/span.h#L273 . This can lead to UB, and the only reason why it didn't lead to UB is because the return type added back the `const`. (Obviously this would break if the return type was deduced via `auto`) Fix all issues by adding back the `const` and using `reinterpret_cast` where appropriate. ACKs for top commit: darosior: re-utACK fa9108f85afdc926fd6a8b96cc2acff7ca25d7a8 hebasto: re-ACK fa9108f85afdc926fd6a8b96cc2acff7ca25d7a8. john-moffett: ACK fa9108f85afdc926fd6a8b96cc2acff7ca25d7a8 Tree-SHA512: 87f6e4b574f9bd96d4e0f2a0631fd0a9dc6096e5d4f1b95042fe9f197afc2fe9a24e333aeb34fed11feefcdb184a238fe1ea5aff10d580bb18d76bfe48b76a10
2023-07-26Merge bitcoin/bitcoin#28150: test: Avoid intermittent issues due to async ↵fanquake
events in validationinterface_tests faca9a3d5a6887517d02b994a43d0e1101b718bc test: Avoid intermittent issues due to async events in validationinterface_tests (MarcoFalke) Pull request description: Currently the tests have many issues: * They setup the genesis block, even though it is not needed * They queue an async `UpdatedBlockTip` even, which causes intermittent issues: https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650064645 Fix all issues by trimming down the setup to just `ChainTestingSetup`. ACKs for top commit: Crypt-iQ: tACK faca9a3d5a6887517d02b994a43d0e1101b718bc Tree-SHA512: 4449040330f89bbaf5ce5b2052417c160b451c373987fdf1069596c07834ed81f0aea1506d53c7d2cd21062b27332d30679285dae194b272fd0cb9ce5ded32cf
2023-07-25Merge bitcoin/bitcoin#27930: util: Don't derive secure_allocator from ↵Andrew Chow
std::allocator 07c59eda00841aafaafd8fd648217b56b1e907c9 Don't derive secure_allocator from std::allocator (Casey Carter) Pull request description: Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`. (Discovered by microsoft/STL#3712, which will be reverted by microsoft/STL#3819 before it ships.) ACKs for top commit: jonatack: re-ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 no change since my previous ACK apart from squashing the commits achow101: ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 john-moffett: ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 Reviewed and tested. Performance appears unaffected in my environment. Tree-SHA512: 23606c40414d325f5605a9244d4dd50907fdf5f2fbf70f336accb3a2cb98baa8acd2972f46eab1b7fdec1d28a843a96b06083cd2d09791cda7c90ee218e5bbd5
2023-07-25Merge bitcoin/bitcoin#28113: kernel: Remove UniValue from kernel libraryAndrew Chow
6960c81cbfa6208d4098353e53b313e13a21cb49 kernel: Remove Univalue from kernel library (TheCharlatan) 10eb3a9faa977371facacee937b2e6dc26f008e0 kernel: Split ParseSighashString (TheCharlatan) Pull request description: Besides the build system changes, this is a mostly move-only change for moving the few UniValue-related functions out of kernel files. UniValue is not required by any of the kernel components and a JSON library should not need to be part of a consensus library. ACKs for top commit: achow101: ACK 6960c81cbfa6208d4098353e53b313e13a21cb49 theuni: Re-ACK 6960c81cbfa6208d4098353e53b313e13a21cb49 stickies-v: re-ACK https://github.com/bitcoin/bitcoin/commit/6960c81cbfa6208d4098353e53b313e13a21cb49 Tree-SHA512: d92e4cb4e12134c94b517751bd746d39f9b8da528ec3a1c94aaedcce93274a3bae9277832e8a7c0243c13df0397ca70ae7bbb24ede200018c569f8d81103c1da
2023-07-25kernel: Remove Univalue from kernel libraryTheCharlatan
It is not required by any of the kernel components. A JSON library should not need to be part of a consensus library.
2023-07-25kernel: Split ParseSighashStringTheCharlatan
This split is done in preparation for the next commit where the dependency on UniValue in the kernel library is removed.
2023-07-25test: Avoid intermittent issues due to async events in validationinterface_testsMarcoFalke
2023-07-25refactor: deduplicate ignores_incoming_txsstickies-v
Initialize PeerManager::Options early to avoid reading -blocksonly twice.
2023-07-25refactor: set ignore_incoming_txs in ApplyArgsManOptionsstickies-v
Refactor to consistently use ApplyArgsManOptions to set all PeerManager::Options, including ignore_incoming_txs.
2023-07-24Don't derive secure_allocator from std::allocatorCasey Carter
Affects both secure_allocator and zero_after_free_allocator. Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`. Drive-by: Aggressively remove facilities unnecessary since C++11 from both allocators to keep things simple.
2023-07-24Remove unused function `reliesOnAssumedValid`Suhas Daftuar
2023-07-24Cache block index entry corresponding to assumeutxo snapshot base blockhashSuhas Daftuar
This is to (a) avoid repeated lookups into the block index for an entry that should never change and (b) emphasize that the snapshot base should always exist when set and not change during the runtime of the program. Thanks to Russ Yanofsky for suggesting this approach.
2023-07-24Move CheckBlockIndex() from Chainstate to ChainstateManagerSuhas Daftuar
Also rewrite CheckBlockIndex() to perform tests on all chainstates. This increases sanity-check coverage, as any place in our code where we were invoke CheckBlockIndex() on a single chainstate will now invoke the sanity checks on all chainstates. This change also tightens up the checks on setBlockIndexCandidates and mapBlocksUnlinked, to more precisely match what we aim for even in the presence of assumed-valid blocks.
2023-07-24Documentation improvements for assumeutxoRyan Ofsky
2023-07-24Fix initialization of setBlockIndexCandidates when working with multiple ↵Suhas Daftuar
chainstates When using assumeutxo and multiple chainstates are active, the background chainstate should consider all HAVE_DATA blocks that are ancestors of the snapshotted block and that have more work than the tip as potential candidates.
2023-07-24Tighten requirements for adding elements to setBlockIndexCandidatesSuhas Daftuar
When using assumeutxo, we only need the background chainstate to consider blocks that are on the chain leading to the snapshotted block. Note that this introduces the new invariant that we can only have an assumeutxo snapshot where the snapshotted blockhash is in our block index. Unknown block hashes that are somehow passed in will cause assertion failures when processing new blocks. Includes test fixes and improvements by Andrew Chow and Fabian Jahr.
2023-07-24[net processing] Move -capturemessages to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -blockreconstructionextratxn to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -maxorphantx to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -txreconciliation to PeerManager::Optionsdergoegge
2023-07-24[net processing] Use ignore_incoming_txs from m_optsdergoegge
2023-07-24[net processing] Introduce PeerManager optionsdergoegge
2023-07-24refactor: Use reinterpret_cast where appropriateMarcoFalke
Also, wrap reinterpret_cast into a CharCast to ensure it is only called on byte pointers.
2023-07-24refactor: Avoid casting away constnessMarcoFalke
Seems confusing and brittle to remove const and then add it back in the return type.
2023-07-24refactor: Remove unused C-style castsMarcoFalke
2023-07-22fuzz: use `ConnmanTestMsg` in `connman`brunoerg
Using `ConnmanTestMsg` we can add nodes and be more effective fuzzing functions like `DisconnectNode`, `FindNode`, `GetNodeStats` and other ones.
2023-07-22refactor: Make more transaction size variables signedHennadii Stepanov
This change gets rid of `static_cast`s and compiler warnings.
2023-07-21fuzz: increase coverage of the descriptor targetsAntoine Poinsot
Once a descriptor is successfully parsed, execute more of its methods. There is probably still room for improvements by checking for some invariants, but this is a low hanging fruit that significantly increases the code coverage of these targets.
2023-07-21fuzz: add a new, more efficient, descriptor parsing targetAntoine Poinsot
This new target focuses on fuzzing the actual descriptor parsing logic by not requiring the fuzzer to produce valid keys (nor a valid checksum for that matter). This should make it much more efficient to find bugs we could introduce moving forward. Using a character as a marker (here '%') to be able to search and replace in the string without having to mock the actual descriptor parsing logic was an insight from Pieter Wuille.
2023-07-21Move block-storage-related logic to ChainstateManagerSuhas Daftuar
Separate the notion of which blocks are stored on disk, and what data is in our block index, from what tip a chainstate might be able to get to. We can use chainstate-agnostic data to determine when to store a block on disk (primarily, an anti-DoS set of criteria) and let the chainstates figure out for themselves when a block is of interest for being a candidate tip. Note: some of the invariants in CheckBlockIndex are modified, but more work is needed (ie to move CheckBlockIndex to ChainstateManager, as most of what CheckBlockIndex is doing is checking the consistency of the block index, which is outside of Chainstate).
2023-07-21refactor: Use EnsureAnyNodeContextMarcoFalke
node_context is never null, but if it was, it would lead to a nullptr dereference in node_context->scheduler. Just use EnsureAnyNodeContext everywhere for more robust, consistent, and correct code.
2023-07-21test: Add SyncWithValidationInterfaceQueue to mockscheduler RPCMarcoFalke
This makes existing tests less brittle, see https://github.com/bitcoin/bitcoin/pull/28108/files#r1268966663
2023-07-21fuzz: make the parsed descriptor testing into a functionAntoine Poinsot
We'll be reusing it in the new target.
2023-07-20Merge bitcoin/bitcoin#28067: descriptors: do not return top-level only funcs ↵Andrew Chow
as sub descriptors dd9633b516d6936ac4e23a40f9b0bea120117d35 test: wallet, add coverage for watch-only raw sh script migration (furszy) cc781a21800a6ce13875feefd0cb14ab0a84524c descriptor: InferScript, do not return top-level only func as sub descriptor (furszy) 286e0c7d5e9538198b28b792c5168b8fafa1534f wallet: loading, log descriptor parsing error details (furszy) Pull request description: Linked to #28057. Currently, the `InferScript` function returns an invalid descriptor when it tries to infer a p2sh-p2pkh script whose pubkey is not known by the wallet. This behavior occurs because the inference process bypasses the `pkh` subscript when the pubkey is not contained by the wallet (no pubkey provider), interpreting it as a `sh(addr(ADDR))` descriptor. Then, the failure arises because the `addr()` function is restricted to being used only at the top level. For reviewers, would recommend to start by examining the functional test to understand the context and the circumstances on which this can result in a fatal error (e.g. during the migration process). ACKs for top commit: achow101: ACK dd9633b516d6936ac4e23a40f9b0bea120117d35 darosior: utACK dd9633b516d6936ac4e23a40f9b0bea120117d35 Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
2023-07-20descriptor: InferScript, do not return top-level only func as sub descriptorfurszy
e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors. Making sh and wsh top level functions to return addr/raw descriptors when the subscript inference fails.
2023-07-20Merge bitcoin/bitcoin#26654: util: Show descriptive error messages when ↵fanquake
FileCommit fails 5408a55fc87350baeae3a32f1003f956d5533a79 Consolidate Win32-specific error formatting (John Moffett) c95a4432d7c9d0e5829cd802908700ba2e2c25dc Show descriptive error messages when FileCommit fails (John Moffett) Pull request description: Only raw [`errno`](https://en.cppreference.com/w/cpp/error/errno) int values are logged if `FileCommit` fails. These values are implementation-specific, so it makes it harder to debug based on user reports. For instance, https://github.com/bitcoin/bitcoin/issues/26455#issue-1436654238 and [another](https://bitcointalk.org/index.php?topic=5182526.0#:~:text=FileCommit%3A%20FlushFileBuffers%20failed%3A%205). Instead, use `SysErrorString` (or the refactored Windows equivalent `Win32ErrorString`) to display both the raw int value and the descriptive message. All other instances in the code I could find where `errno` or (Windows-only) `GetLastError()`/`WSAGetLastError()` are logged use the full descriptive string. For example: https://github.com/bitcoin/bitcoin/blob/1b680948d43b1d39645b9d839a6fa7c6c1786b51/src/util/sock.cpp#L390 https://github.com/bitcoin/bitcoin/blob/1b680948d43b1d39645b9d839a6fa7c6c1786b51/src/util/sock.cpp#L272 https://github.com/bitcoin/bitcoin/blob/7e1007a3c6c9a921c2b60919b84a60eaabfe1c5d/src/netbase.cpp#L515-L516 https://github.com/bitcoin/bitcoin/blob/8ccab65f289e3cce392cbe01d5fc0e7437f51f1e/src/init.cpp#L164 I refactored the Windows formatting code to put it in `syserror.cpp`, as it's applicable to all Win32 API system errors, not just networking errors. To be clear, the Windows API functions `WSAGetLastError()` and `GetLastError()` are currently [equivalent](https://stackoverflow.com/questions/15586224/is-wsagetlasterror-just-an-alias-for-getlasterror). ACKs for top commit: MarcoFalke: lgtm ACK 5408a55fc87350baeae3a32f1003f956d5533a79 💡 Tree-SHA512: 3921cbac98bd9edaf84d3dd7a43896c7921f144c8ca2cde9bc96d5fb05281f7c55e7cc99db8debf6203b5f916f053025e4fa741f51458fe2c53bb57b0a781027
2023-07-20Merge bitcoin/bitcoin#28066: fuzz: Generate process_message targets individuallyfanquake
fa6245da6061050eb77ad07cd4caf8c596d89dc6 fuzz: Generate process_message targets individually (MarcoFalke) fa1471e5754484f997ddf9db70888679dcd1d64a refactor: Remove duplicate allNetMessageTypesVec (MarcoFalke) Pull request description: Now that `LIMIT_TO_MESSAGE_TYPE` is a runtime setting after commit 927b001502a74a7224f04cfe6ffddc9a59409ba1, it shouldn't hurt to also generate each message type individually. Something similar was done for the `rpc` target in commit cf4da5ec29f9e8cd6cc6577e5ecbd87174edba62. ACKs for top commit: stickies-v: re-crACK fa6245da6061050eb77ad07cd4caf8c596d89dc6 brunoerg: reACK fa6245da6061050eb77ad07cd4caf8c596d89dc6 Tree-SHA512: 8f3ec71bab89781f10820a0e027fcde8949f3333eb19a30315aaad6f90f5167028113cea255b2d60b700da817c7eaac20b7b4c92f931052d7f5c2f148d33aa5a
2023-07-20Merge bitcoin/bitcoin#26467: bumpfee: Allow the user to choose which output ↵fanquake
is change e8c31f135c6e9a5f57325dbf4feceafd384f7762 tests: Test for bumping single output transaction (Andrew Chow) 4f4d4407e3d2cc5ac784524c0cb0602837dc7860 test: Test bumpfee reduce_output (Andrew Chow) 7d83502d3d52218e7b0b0634cff2a9aba9cc77ef bumpfee: Allow original change position to be specified (Andrew Chow) Pull request description: When bumping the transaction fee, we will try to find the change output of the transaction in order to have an output whose value we can modify so that we can meet the target feerate. However we do not always find the change output which can cause us to unnecessarily add an additional output to the transaction. We can avoid this issue by outsourcing the determination of change to the user if they so desire. This PR adds a `orig_change_pos` option to bumpfee which the user can use to specify the index of the change output. Fixes #11233 Fixes #20795 ACKs for top commit: ismaelsadeeq: re ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762 pinheadmz: re-ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762 furszy: Code review ACK e8c31f13 Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
2023-07-19Merge bitcoin/bitcoin#27928: test: Add more tests for the BIP21 implementationRyan Ofsky
f1d807e383942ae20e080174d33ac17afd649351 Add more tests for the BIP21 implementation (Kiminuo) Pull request description: This PR is an attempt to make it clear how the current BIP21 implementation behaves in Bitcoin Core. Especially, I'm interested whether one can specify multiple `amount` (`message`, etc.) parameters. My primary end goal is to answer [this question of mine](https://bitcoin.stackexchange.com/questions/118654/how-to-interpret-bip21-uri-with-amount-specified-twice/) but I figured that maybe it's worth a PR. If not, I'll close the PR. ACKs for top commit: MarcoFalke: lgtm ACK f1d807e383942ae20e080174d33ac17afd649351 kevkevinpal: ACK [f1d807e](https://github.com/bitcoin/bitcoin/pull/27928/commits/f1d807e383942ae20e080174d33ac17afd649351) Tree-SHA512: d287809d47c5cfc667f850927bfd969bd345a996d3d53a4c26ef0ffd29eb75ef53358692a15f9a0493ec9e1c101123b6584572e25f87bcb98ff67f6b6c166de4
2023-07-19Merge bitcoin/bitcoin#28085: refactor: use Span for SipHash::WriteAndrew Chow
7d92b1430a6fd42c4438810640576830d0ff8d13 refactor: use Span for SipHash::Write (Sebastian Falbesoner) Pull request description: This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers). ACKs for top commit: sipa: utACK 7d92b1430a6fd42c4438810640576830d0ff8d13 MarcoFalke: lgtm ACK 7d92b1430a6fd42c4438810640576830d0ff8d13 achow101: ACK 7d92b1430a6fd42c4438810640576830d0ff8d13 Tree-SHA512: f17a27013c942aead4b09f5a64e0c3ff8dbc7e83fe63eb9a2e3ace8be9921c9cbba3ec67e3e83fbe3332ca941c42370efd059e702c060f9b508307e9657c66f2
2023-07-19streams: Teach AutoFile how to XORMarcoFalke
2023-07-19Merge bitcoin/bitcoin#28056: rpc: doc: Added `longpollid` and `data` params ↵fanquake
to `template_request` f6a26196cfb7e2c90e25f82b0e2f569a05013cae Added `longpollid` and `data` params to `template_request` #27998 (Rhythm Garg) Pull request description: This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan. I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters. This PR refers to the issue #27998 ACKs for top commit: ItIsOHM: > tACK [f6a2619](https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae) russeree: tACK https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae stickies-v: tACK f6a26196cfb7e2c90e25f82b0e2f569a05013cae Tree-SHA512: 6c592db59cb11b2d031ce5265c547fa296266278f6c25f96afe18a420e0d547f4d483e0f66de75d52c0c319ac1585f3558b9f70c12ef208c96ec96a51f786c6a
2023-07-19Merge bitcoin/bitcoin#28093: subtree: update libsecp256k1 to latest masterfanquake
5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 build: adapt Windows builds for libsecp256k1 build changes (fanquake) ff061fde183f68d3b77db24b5742f54e58952de6 Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374 (fanquake) Pull request description: Includes https://github.com/bitcoin-core/secp256k1/pull/1378. Which fixes #28079. Adapts Windows build for https://github.com/bitcoin-core/secp256k1/pull/1367. ACKs for top commit: hebasto: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7, I've made the `src/secp256k1` subtree update locally and got zero diff with this PR branch. jonasnick: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 Tree-SHA512: 37915d420ebacefc6bc82c2511bff3d6884e01d5c92795f19cd61862f96b30aa1fe768aeabec128c9d25c1d8bc62b46b46969626067266074b39566ad9e2f5ba