aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-05-04net: reduce LOCK(cs_main) scope in ProcessGetBlockDataAndrew Toth
This also changes behavior if ReadBlockFromDisk or ReadRawBlockFromDisk fails. Previously, the node would crash due to an assert. This has been replaced with logging the error, disconnecting the peer, and returning early.
2024-05-04net: reduce LOCK(cs_main) scope in GETBLOCKTXNAndrew Toth
Also adds a static assertion that MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP
2024-05-04Merge bitcoin/bitcoin#28657: miniscript: make operator""_mst constevalmerge-script
63317103c9f2b0635558da814567bb79c17ae851 miniscript: make operator_mst consteval (Pieter Wuille) Pull request description: It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the `constexpr` keyword. Since C++20, we can force them to evaluate at compile time using `consteval`, turning all the miniscript type constants into actual compile-time constants. This should give a nice but not very important speedup for miniscript logic, but it's also a way to start testing C++20 features. ACKs for top commit: hebasto: re-ACK 63317103c9f2b0635558da814567bb79c17ae851. theuni: utACK 63317103c9f2b0635558da814567bb79c17ae851 Tree-SHA512: bdc9f1a6499b8bb3ca04f1a158c31e6876ba97206f95ee5718f50efd58b5b4e6b8867c07f791848430bfaa130b9676d8a68320b763cda9a340c75527acbfcc9e
2024-05-04Merge bitcoin/bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation ↵merge-script
on SunOS / illumos 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (Hennadii Stepanov) Pull request description: On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined. This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead. No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems. Fixes https://github.com/bitcoin/bitcoin/issues/29884. An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as well. ACKs for top commit: maflcko: lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b theuni: ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b. Nice to have the serialization concept actually tested :) Tree-SHA512: 1033863e584fa8e99a281b236fa01fc919f610a024bcec792116762e28c1c16ee481bd01325c3a0ca9dd9d753176aa63bd9ac7e08a9bbce772db2949d06f6e61
2024-05-04Merge bitcoin/bitcoin#25972: build: no-longer disable WARN_CXXFLAGS when ↵merge-script
CXXFLAGS is set f0e22be69a15248c42964d57f44ce8c37a36081d build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set (fanquake) b088062e687d95deff28b0715fd4859449b56584 ci: remove -Wdocumentation from -Werror in multiprocess CI (fanquake) bec6a88fbcff0bd974e820cefd2be2d00b6f6c56 ci: remove -Warray-bounds from -Werror for win64 (fanquake) 7469ac7032e26fe805c5d15ecded2d44253bc9c1 ci: disable -Werror=maybe-uninitialized for Windows builds (fanquake) Pull request description: Now that `CXXFLAGS` are [back in user control](https://github.com/bitcoin/bitcoin/pull/24391), I don't think there's a reason to no-longer use our warning flags when `CXXFLAGS` has been overriden (this includes, by default, when building from depends). Anyone can suppress warnings from third-party code by passing the relevant `-Wno-` options in `CXXFLAGS`. Closes: #18092. ACKs for top commit: maflcko: utACK f0e22be69a15248c42964d57f44ce8c37a36081d 🍡 hebasto: ACK f0e22be69a15248c42964d57f44ce8c37a36081d. theuni: ACK f0e22be69a15248c42964d57f44ce8c37a36081d. It'll be nice to have this fixed. TheCharlatan: ACK f0e22be69a15248c42964d57f44ce8c37a36081d Tree-SHA512: dcef4bd4a57bab6f586ca015fad725e7a38bf24b7a08808a74d8c8ca47cf68c5fca7b04ed38649a047c6929fb708e2c97f2000fc46d5a8d25da49951c5bb0f66
2024-05-03Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with ↵Ava Chow
MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
2024-05-03Merge bitcoin/bitcoin#30029: test: remove duplicate `WITNESS_SCALE_FACTOR` ↵Ava Chow
constant definition af3c18169a075222fe0795ab24b8b20ad5e30ae4 [test]: remove duplicate WITNESS_SCALE_FACTOR (ismaelsadeeq) Pull request description: Notice this while working on #29523 - `blocktools.py` and `messages.py` both define `WITNESS_SCALE_FACTOR` constant https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/blocktools.py#L48 https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/messages.py#L68 - This PR deletes the one in `blocktools.py` and update the tests to only use `WITNESS_SCALE_FACTOR` from `messages.py` ACKs for top commit: maflcko: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 sipa: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 achow101: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 glozow: lgtm ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 brunoerg: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 willcl-ark: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 Tree-SHA512: 6bd8060c9eea10e03940acee2aa4cd08e4e0afb6d26be3e6300ad405fd0af5b373a00e994eb39515a2dcafa1625562bcd57945049a84b9c9dcc7ea60c24f0911
2024-05-03miniscript: make operator_mst constevalPieter Wuille
It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the constexpr keyword. Since C++20, we can force them to evaluate at compile time, turning all the miniscript type constants into actual compile-time constants. It appears that MSVC does not support consteval operator"" when used inside certain expressions. For the few places where this happens, define a constant outside the operator call. Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2024-05-03Merge bitcoin/bitcoin#30026: refactor, test: Always initialize pointerRyan Ofsky
bd2de7ac591d7704b79304089ad1fb57e085da8b refactor, test: Always initialize pointer (Hennadii Stepanov) Pull request description: This change fixes MSVC warning [C4703](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703). All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`. Required to simplify warning suppression porting to the CMake-based build system. ACKs for top commit: maflcko: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b sipsorcery: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b. ryanofsky: Code review ACK bd2de7ac591d7704b79304089ad1fb57e085da8b Tree-SHA512: 006db041d3c3697a77d9df14de86cf7c8a10804b45789df01268b2236cf6452e77dc57e89f5d5a6bc26d4b5cd483f0722d6035649c8a523b57954bb1fc810d0c
2024-05-03[test]: remove duplicate WITNESS_SCALE_FACTORismaelsadeeq
2024-05-03build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is setfanquake
Now that CXXFLAGS are back in user control, I don't think there's a reason to no-longer use our warning flags when CXXFLAGS has been overriden (this includes when building from depends). Anyone can suppress warnings from third-party code by passing the relevant `-Wno-` options in CXXFLAGS. Fixes: #18092.
2024-05-03ci: remove -Wdocumentation from -Werror in multiprocess CIfanquake
The issues here are coming from Boost Test code.
2024-05-03ci: remove -Warray-bounds from -Werror for win64fanquake
These warnings are coming from leveldb, and appear to be fixed starting with GCC 13.
2024-05-03ci: disable -Werror=maybe-uninitialized for Windows buildsfanquake
This produces false positives, such as: ```bash torcontrol.cpp: In static member function ‘static void TorControlConnection::readcb(bufferevent*, void*)’: torcontrol.cpp:94:28: error: ‘result’ may be used uninitialized [-Werror=maybe-uninitialized] 94 | self->message.code = ToIntegral<int>(s.substr(0, 3)).value_or(0); | ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from ./netaddress.h:18, from ./torcontrol.h:11, from torcontrol.cpp:6: ./util/strencodings.h:184:7: note: ‘result’ was declared here 184 | T result; | ^~~~~~ cc1plus: all warnings being treated as errors make[2]: *** [Makefile:11088: libbitcoin_node_a-torcontrol.o] Error 1 ```
2024-05-03Merge bitcoin/bitcoin#30012: opportunistic 1p1c followupsmerge-script
7f6fb73c82fdff2afe5edefcc335ba6707d5627d [refactor] use reference in for loop through iters (glozow) 6119f76ef72c1adc55c1a384c20f8ba9e1a01206 Process every MempoolAcceptResult regardless of PackageValidationResult (glozow) 2b482dc1f3fb248ccbe7eeb8c9c07d4bf1295a70 [refactor] have ProcessPackageResult take a PackageToValidate (glozow) c2ada0530719e044bb498e0d78907a208214a71e [doc] remove redundant PackageToValidate comment (glozow) 9a762efc7a118c44556fa9ad404f6b54103bad41 [txpackages] use std::lexicographical_compare instead of sorting hex strings (glozow) 8496f69e1c2d1961db2604829cb6a289eb8dd3d6 [refactor] make MempoolAcceptResult::m_replaced_transactions non-optional (glozow) Pull request description: Followups from #28970: - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568781077 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585554972 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581019326 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581036209 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1586258730 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30012/commits/7f6fb73c82fdff2afe5edefcc335ba6707d5627d Tree-SHA512: 9d8393d5f2fedbc6ebce582ff2a8ed074a02dd8e7dbf562c14d48b439fdc1ee6c3203b3609366d3c883d44655cc1a5c83a75ca56e490d25c1a34d95a0622d458
2024-05-03Merge bitcoin/bitcoin#30017: refactor, fuzz: Make 64-bit shift explicitmerge-script
b50d127a7710d790c2ba4a08f01b832c2a0b1203 refactor: Make 64-bit shift explicit (Hennadii Stepanov) Pull request description: This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to https://github.com/bitcoin/bitcoin/pull/26252. All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`. Required to simplify warning suppression porting to the CMake-based build system. ACKs for top commit: maflcko: utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203 sipsorcery: utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203 Tree-SHA512: 18f6082b4234506ad2f9df54e577031b97cdf9f7ef64cad4162f275660716ab73587a97d3af0f778dfd48d2751d8676b5d3381d0aa837fcc60a09704473a9209
2024-05-02refactor, test: Always initialize pointerHennadii Stepanov
This change fixes MSVC warning C4703. See: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703 All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`.
2024-05-02Merge bitcoin/bitcoin#29617: test: Validate UTXO snapshot with coin height > ↵Ava Chow
base height & amount > MAX_MONEY supply ec1f1abfefa281e62bb876aa1c4738d576ef9a47 test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply (jrakibi) Pull request description: ### Ensure snapshot loading fails for coins exceeding base height **Objective**: This test verifies that snapshot loading is correctly rejected for coins with a height greater than the base height. **Update**: - Added `test_invalid_snapshot_wrong_coin_code` to `feature_assumeutxo.py`. - The test artificially sets a coin's height above 299 in a snapshot and checks for load failure. - Edit: Added a test case for outputs whose amounts surpass the MAX_MONEY supply limit. This implementation addresses the request for enhancing `assumeutxo` testing as outlined in issue #28648 --- **Edit: This is an explanation on how I arrive at content values: b"\x84\x58" and b"\xCA\xD2\x8F\x5A"** You can use this tool to decode the utxo snapshot https://github.com/jrakibi/utxo-live Here’s an overview of how it’s done: The serialization format for a UTXO in the snapshot is as follows: 1. Transaction ID (txid) - 32 bytes 2. Output Index (outnum)- 4 bytes 3. VARINT (code) - A varible-length integer encoding the height and whether the transaction is a coinbase. The format of this VARINT is (height << 1) | coinbase_flag. 4. VARINT (amount_v) - A variable-length integer that represents a compressed format of the output amount (in satoshis). For the test cases mentioned: * **`b"\x84\x58"`** - This value corresponds to a VARINT representing the height and coinbase flag. Once we decode this code, we can extract the height and coinbase using `height = code_decoded >> 1` and `coinbase = code_decoded & 0x01`. In our case, with code_decoded = 728, it results in `height = 364` and `coinbase = 0`. * **`b"\xCA\xD2\x8F\x5A"`** - This byte sequence represents a compressed amount value. The decompression function takes this value and translates it into a full amount in satoshis. In our case, the decompression of this amount translates to a number larger than the maximum allowed value of coins (21 million BTC) ACKs for top commit: fjahr: re-ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 maflcko: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑 achow101: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 Tree-SHA512: 42b36fd1d76e9bc45861028acbb776bd2710c5c8bff2f75c751ed505995fbc1d4bc698df3be24a99f20bcf6a534615d2d9678fb3394162b88133eaec88ca2120
2024-05-02Merge bitcoin/bitcoin#29961: refactor: remove remaining unused code from ↵Ava Chow
cpp-subprocess 8b52e7f628304e83b0e36fd97e617de0f71c5a62 update comments in cpp-subprocess (check_output references) (Sebastian Falbesoner) 97f159776ec06f767df1d4990aa7d0859140f52f remove unused method `Popen::kill` from cpp-subprocess (Sebastian Falbesoner) 908c51fe4afeba0af500c6275027b1afa1b3bd19 remove commented out code in cpp-subprocess (Sebastian Falbesoner) ff79adbe056220202f7a56d67f788c38fc49ef9f remove unused templates from cpp-subprocess (Sebastian Falbesoner) Pull request description: This PR removes remaining code that is unused within the cpp-subprocess module (templates and commented out code). Happy to add more removals if anyone finds more unused parts. Note that there are some API functions of the `Popen` class that we don't use, e.g. `wait()`, `pid()`, `poll()`, `kill()`, but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there. ACKs for top commit: fjahr: Code review ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62 achow101: ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62 hebasto: ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62. Tree-SHA512: 14c1cd2216185d941923f06fdc7acbeed66cd87e2691d9a352f7309b3e07fe4877b580f598a2e4106f9c48395ed6de00a0bfb5d3c3af9c4624d1956a0f543e99
2024-05-02Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZEJon Atack
2024-05-02Merge bitcoin/bitcoin#29968: refactor: Avoid unused-variable warning in init.cppmerge-script
fa9abf968840745e428a86a9545aaa6c923415e2 refactor: Avoid unused-variable warning in init.cpp (MarcoFalke) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/27679#discussion_r1580606777 ACKs for top commit: TheCharlatan: ACK fa9abf968840745e428a86a9545aaa6c923415e2 Tree-SHA512: dcf56d7aa68578ba611a2dc591de036ab1d08f7f4dfb35d325ecf7241d8e17abc0af7005b96c44da9777adc36961b4da7fdde282a1ab0e0a6f229c8108923101
2024-05-02refactor: Avoid unused-variable warning in init.cppMarcoFalke
2024-05-02[refactor] use reference in for loop through itersglozow
2024-05-02Merge bitcoin/bitcoin#29934: doc: add LLVM instruction for macOS < 13merge-script
22574046c90c0662f3aa9b1baea074aff54f92a9 doc: add LLVM instruction for macOS < 13 (Sjors Provoost) Pull request description: #29208 bumped clang to 14, which users of old macOS versions need to install manually. This PR adds instructions. Xcode 14.3.1 ships clang 14.0.3 (14.0.0 is broken, see #29918): https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework) The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-release-notes# Homebrew itself officially supports macOS 12 or later, but _may_ still work on macOS 11: https://docs.brew.sh/Installation Fwiw macOS 11 Big Sur last got an update in September 2023, so Apple has not _entirely_ written it off: https://en.wikipedia.org/wiki/MacOS_Big_Sur ACKs for top commit: maflcko: utACK 22574046c90c0662f3aa9b1baea074aff54f92a9 TheCharlatan: ACK 22574046c90c0662f3aa9b1baea074aff54f92a9 Tree-SHA512: 5b4bcc71966d1da84bc4da32da89e0dea9f519f37d9e14e169140c92af044b33f404f01ae7d10f53ab5345dd51ac404c161389efef93da5cacbfd52a43881695
2024-05-02Merge bitcoin/bitcoin#30010: lint: [doc] Clarify Windows line endings (CR ↵merge-script
LF) not to be used fa9be2f79520fff9cfe2ed35ace05cb322680af3 lint: [doc] Clarify Windows line endings (CR LF) not to be used (MarcoFalke) Pull request description: It has been this case since the linter was introduced years ago. Given a misunderstanding (https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2088028856), clarify the docs. ACKs for top commit: brunoerg: ACK fa9be2f79520fff9cfe2ed35ace05cb322680af3 Tree-SHA512: be714db9df533e0962ed84102ffdb72717902949b930d58cf5a79cba36297f6b2b1f75e65a2c1f46bcb8e2f4ad5d025f3d15210f468a5ec9631a580b74f923ea
2024-05-02Merge bitcoin/bitcoin#29707: depends: build miniupnpc with CMakemerge-script
5195baa60087ee366290887ad982fc491e14c111 depends: fix miniupnpc snprintf usage on Windows (fanquake) 3c2d440f1497f60bb444326f51383df244dcdfe9 depends: switch miniupnpc to CMake (Cory Fields) f5618c79d9e7af05c2987044dc2da03697f8bb89 depends: add upstream CMake patch to miniupnpc (fanquake) 6866b571ab96f03ca0775424e45458c5731f230f depends: miniupnpc 2.2.7 (fanquake) Pull request description: This picks up one of the changes from #29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I've upstreamed https://github.com/miniupnp/miniupnp/pull/721, as well as some suggestions from the previous PR. ACKs for top commit: theuni: ACK 5195baa60087ee366290887ad982fc491e14c111. TheCharlatan: utACK 5195baa60087ee366290887ad982fc491e14c111 Tree-SHA512: 5b27e132cd5eed285e9be34c8b96893417d92a1ae55c99345c9a89e1c1c5e40e4bc840bc061b879758b2b11fcb520cd98c3da985c1e153f2e5380cf63efe2d69
2024-05-02refactor: Make 64-bit shift explicitHennadii Stepanov
This change fixes MSVC level-3 warning C4334. See: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334 All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
2024-05-01Merge bitcoin/bitcoin#29120: test: Add test case for spending bare multisigmerge-script
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/29113 ACKs for top commit: ajtowns: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f achow101: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f willcl-ark: reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
2024-05-01Process every MempoolAcceptResult regardless of PackageValidationResultglozow
2024-05-01[refactor] have ProcessPackageResult take a PackageToValidateglozow
2024-05-01[doc] remove redundant PackageToValidate commentglozow
2024-05-01[txpackages] use std::lexicographical_compare instead of sorting hex stringsglozow
No behavior change, but getting the hex string is more expensive than necessary.
2024-05-01[refactor] make MempoolAcceptResult::m_replaced_transactions non-optionalglozow
2024-05-01lint: [doc] Clarify Windows line endings (CR LF) not to be usedMarcoFalke
2024-04-30Merge bitcoin/bitcoin#29645: doc: update release-process.mdAva Chow
1ea8674316f2dce0005f6094b6ee111b045dd770 [doc] update release-process.md and backports section of CONTRIBUTING (glozow) Pull request description: While doing various release process things for the first time, I noticed some of our docs are outdated and/or confusing. ACKs for top commit: achow101: ACK 1ea8674316f2dce0005f6094b6ee111b045dd770 Tree-SHA512: 4ad10d4ce2c33fe15cb02599353107bb72ecb867aefc6c120cfd5cdea42aa8fa3783f9e0218c2f3815f030e0694cc8fb24011ce88358a0206cb07416a256a962
2024-04-30Merge bitcoin/bitcoin#29983: msvc: Compile `test\fuzz\bitdeque.cpp`Ava Chow
774359b4a96d2724dc70f900cb71e084a77164da build, msvc: Compile `test\fuzz\bitdeque.cpp` (Hennadii Stepanov) 85f50a46c50e7e56b5ee2d7258021939cd80c550 refactor: Fix "error C2248: cannot access private member" on MSVC (Hennadii Stepanov) Pull request description: This PR resolves one point from the https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2028808614: > What is the issue with the bitdeque... ? ACKs for top commit: maflcko: lgtm ACK 774359b4a96d2724dc70f900cb71e084a77164da sipa: utACK 774359b4a96d2724dc70f900cb71e084a77164da achow101: ACK 774359b4a96d2724dc70f900cb71e084a77164da dergoegge: utACK 774359b4a96d2724dc70f900cb71e084a77164da Tree-SHA512: dba5c0217b915468af08475795437a10d8e8dedfadeb319f36d9b1bf54a91a8b2c61470a6047565855276c2bc8589c7776dc19237610b65b57cc841a303de8b3
2024-04-30Merge bitcoin/bitcoin#29813: doc: i2p: improve `-i2pacceptincoming` mentionAva Chow
2179e2c3209a41c1419f1f5ed6270a0dad68b50d doc: i2p: improve `-i2pacceptincoming` mention (brunoerg) Pull request description: In i2p documentation, it says that "the first time Bitcoin Core connects to the I2P router, it automatically generates a persistent I2P address and its corresponding private key by default _**or if `-i2pacceptincoming=1` is set**_". This is weird, because `-i2pacceptincoming=1` by itself does not have any effect. Moreover, `-i2pacceptincoming` is 1 by default anyway. ACKs for top commit: laanwj: This documentation change is correct and makes the documentation slightly shorter, thus easier to read. ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/29813/commits/2179e2c3209a41c1419f1f5ed6270a0dad68b50d achow101: ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d byaye: ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d Tree-SHA512: 18a6a627343fb0aa824029d99df8a232153ba288ce94ec8c5da25693885237381fba505ea1e71c756b2a611243a302d319ca7ae03b526020cd6588710fc2ac17
2024-04-30Merge bitcoin/bitcoin#28016: p2p: gives seednode priority over dnsseed if ↵Ava Chow
both are provided 82f41d76f1c6ad38290917dad5499ffbe6b3974d Added seednode prioritization message to help output (tdb3) 3120a4678ab2a71a381e847688f44068749cfa97 Gives seednode priority over dnsseed if both are provided (Sergi Delgado Segura) Pull request description: This is a follow-up of #27577 If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`. This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds` ACKs for top commit: davidgumberg: untested reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d itornaza: tested re-ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d achow101: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d cbergqvist: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d Tree-SHA512: 4e39e10a7449af6cd9b8f9f6878f846b94bca11baf89ff2d4fbcd4f28293978a6ed71a3a86cea36d49eca891314c834e32af93f37a09c2cc698a878f84d31c62
2024-04-30Merge bitcoin/bitcoin#29623: Simplify network-adjusted time warning logicAva Chow
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v) 92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge) 7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge) ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v) 55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v) 038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge) Pull request description: [An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes. Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are: - Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty. - Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number). - Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes - no more globals - remove the `-maxtimeadjustment` startup arg Closes #4521 ACKs for top commit: sr-gi: Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b) achow101: reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b dergoegge: utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
2024-04-30Merge bitcoin/bitcoin#28970: p2p: opportunistically accept 1-parent-1-child ↵Ava Chow
packages e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow) 87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow) 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow) 410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow) d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow) 2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow) 092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow) c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow) 6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow) Pull request description: This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking. Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful. - Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1] - Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate. - The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742. The first 2 commits are followups from #29619: - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034 - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257 Q: What makes this short of a more full package relay feature? (1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463. (2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer. (3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031. [1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions ACKs for top commit: sr-gi: tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f instagibbs: reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f theStack: Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package: dergoegge: light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f achow101: ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
2024-04-30Merge bitcoin/bitcoin#29906: Disable util::Result copying and assignmentAva Chow
6a8b2befeab25e4e92d8e947a23e78014695e06c refactor: Avoid copying util::Result values (Ryan Ofsky) 834f65e82405bbed336f98996bc8cef366bbed0f refactor: Drop util::Result operator= (Ryan Ofsky) Pull request description: This PR just contains the first two commits of #25665. It disables copying of `util::Result` objects because unnecessary copies are inefficient and not possible after #25665, which makes `util::Result` object move-only. It disables the assignment operator and replaces it with an `Update()` method, because #25665 adds more information to `util::Result` objects (warning and error messages and failure values) and having an assignment operator that overwrites data instead of merging it would make it easy to accidentally erase existing information while trying to assign new information. ACKs for top commit: stickies-v: re-ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c achow101: ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c furszy: re-ACK https://github.com/bitcoin/bitcoin/commit/6a8b2befeab25e4e92d8e947a23e78014695e06c Tree-SHA512: 3f21af9031d50d6c68cca69133de03080f69b1ddcf8b140bdeb762069f14645209b2586037236d15b6ebd8973af0fbefd7e83144aeb7b84078a4cb4df812f984
2024-04-30Merge bitcoin/bitcoin#29986: test: Don't rely on incentive incompatible ↵glozow
replacement in mempool_accept_v3.py f8a141c2dae2471a7ce7248e28a0bbeb8a291acd test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py (Suhas Daftuar) Pull request description: In the sibling eviction test, we're currently testing that a transaction with ancestor feerate (and mining score) of 179 s/b is able to replace a transaction with ancestor feerate (and mining score) of 300 s/b, due to a shortcoming in our current RBF rules. In preparation for fixing our RBF rules to not allow such replacements, fix the test by bumping the fee of the replacement to be a bit higher. ACKs for top commit: glozow: ACK f8a141c2dae2471a7ce7248e28a0bbeb8a291acd instagibbs: ACK f8a141c2dae2471a7ce7248e28a0bbeb8a291acd Tree-SHA512: 0babe60be2f41634301e434fedb7abc765daaa37c2c280acb569eaf02a793369d81401ab02b8ae1689bda4872f475bd4c2f48cae4a54a61ece20db0a014e23ac
2024-04-30Merge bitcoin/bitcoin#29990: fuzz: don't allow adding duplicate transactions ↵glozow
to the mempool cc15c5bfd1eb3903de246c124702a7c66c687008 fuzz: don't allow adding duplicate transactions to the mempool (Suhas Daftuar) Pull request description: Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target. I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have). ACKs for top commit: glozow: utACK cc15c5bfd1eb3903de246c124702a7c66c687008 makes sense to me maflcko: lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008 brunoerg: ACK cc15c5bfd1eb3903de246c124702a7c66c687008 dergoegge: utACK cc15c5bfd1eb3903de246c124702a7c66c687008 Tree-SHA512: 85f84ce405aba584e6d00391515f0a86c5648ce8b2da69036e50a6c1f6833d050d09b1972cc5ffbe7c4edb3e5f7f965ef34bd839deeddac27a889cc8d2e53b8f
2024-04-30Merge bitcoin/bitcoin#29985: depends: Fix build of Qt for 32-bit platforms ↵merge-script
with recent glibc 2e266f33b5d2be5c233c2c692481f75785714fa1 depends: Fix build of Qt for 32-bit platforms (laanwj) Pull request description: The 32 to 64-bit `time_t` transition causes a build failure in the built-in zlib about conflicting `_TIME_BITS` and `_FILE_OFFSET_BITS`. Note that zlib doesn't use `time_t` at all, so it is a false alarm. Take the following patch from upstream zlib: https://github.com/madler/zlib/commit/a566e156b3fa07b566ddbf6801b517a9dba04fa3.patch Closes #29980. ACKs for top commit: hebasto: re-ACK 2e266f33b5d2be5c233c2c692481f75785714fa1. fanquake: ACK 2e266f33b5d2be5c233c2c692481f75785714fa1 - at some point qt's open source 5.15.x branch will catch up to where they bumped the internal zlib to >= 1.3 (which contains this change), and we'll be able to drop this patch. Checked that it fixes the build issue in the interim. Tree-SHA512: b297aed8b299c671ff439b5b7b410832ff5004fd9b13c3b4a5fb5bde9dcf24a5eda08cd0a39565ae0641d9533711142bdc2889a32d343b9c4b41bfac24f0ca28
2024-04-30depends: fix miniupnpc snprintf usage on Windowsfanquake
2024-04-30depends: switch miniupnpc to CMakeCory Fields
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Co-authored-by: fanquake <fanquake@gmail.com>
2024-04-30depends: add upstream CMake patch to miniupnpcfanquake
2024-04-30depends: miniupnpc 2.2.7fanquake
Includes a temporary patch to fix the Windows Autotools build. See https://miniupnp.tuxfamily.org/files/changelog.php?file=miniupnpc-2.2.7.tar.gz.
2024-04-29test: Don't rely on incentive incompatible replacement in mempool_accept_v3.pySuhas Daftuar
2024-04-29Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by nameRyan Ofsky
30a6c999351041d6a1e8712a9659be1296a1b46a rpc: access some args by name (stickies-v) bbb31269bfa449e82d3b6a20c2c3481fb3dcc316 rpc: add named arg helper (stickies-v) 13525e0c248eab9b199583cde76430c6da2426e2 rpc: add arg helper unit test (stickies-v) Pull request description: Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful. Example usage: ```cpp const auto action{self.Arg<std::string>("action")}; ``` Most of the LoC is adding test coverage and documentation updates. No behaviour change. An alternative approach to #27788 with significantly less overhaul. ACKs for top commit: fjahr: Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a maflcko: ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑 ryanofsky: Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too. Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 23:35:15 +0000