aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-07-25tests: framework for testing DepGraph classPieter Wuille
This introduces a bespoke fuzzing-focused serialization format for DepGraphs, and then tests that this format can represent any graph, roundtrips, and then uses that to test the correctness of DepGraph itself. This forms the basis for future fuzz tests that need to work with interesting graphs.
2024-07-25clusterlin: introduce cluster_linearize.h with Cluster and DepGraph typesPieter Wuille
This primarily adds the DepGraph class, which encapsulates precomputed ancestor/descendant information for a given transaction cluster, with a number of utility features (inspectors for set feerates, computing reduced parents/children, adding transactions, adding dependencies), which will become needed in future commits.
2024-07-25depends: remove ENV unsetting for darwinfanquake
Now that we use the native compiler, and have fixed Qt, and these vars are unset it Guix, we can remove the unsetting from our compiler command here. Fixes #21552.
2024-07-25guix: improve ENV unsetting for macOSfanquake
2024-07-25depends: patch explicit -lm usage out of Qt toolsfanquake
2024-07-25Merge bitcoin/bitcoin#30507: m_tx_download_mutex followupsmerge-script
7c29e556c573a63351096c34bc072ae0c60ffa29 m_tx_download_mutex followups (glozow) e543c657dad830294609424b459755884bd44f3c release m_tx_download_mutex before MakeAndPushMessage GETDATA (glozow) bce5f37c7bef2755e8d5f0886f37dd58357dadad [refactor] change ActiveTipChange to use CBlockIndex ref instead of ptr (glozow) 7cc5ac5a674f31af2ac7e5b5842e1c1aeb9e6744 [doc] TxOrphanage is no longer thread-safe (glozow) 6f49548670d5ab12a963c0ada3b315c544b95e2e [refactor] combine block vtx loops in BlockConnected (glozow) Pull request description: Followup to #30111. Includes suggestions: - https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686303768 - https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686314984 - https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1683186792 - https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2242819514 - https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686372826 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30507/commits/7c29e556c573a63351096c34bc072ae0c60ffa29 theStack: re-ACK 7c29e556c573a63351096c34bc072ae0c60ffa29 dergoegge: reACK 7c29e556c573a63351096c34bc072ae0c60ffa29 Tree-SHA512: 79a9002d74739367789bbc64bb1d431f4d43a25a7934231e55814c2cb6981c15ef2d8465544ae2a4fbd734d9bed6cc41b37a923938a88cb8fea139523c1e98da
2024-07-25Merge bitcoin/bitcoin#30506: depends: Cleanup postprocess commands after ↵merge-script
switching to CMake a0314c151679a348d842b68c5ecb7a556700811c depends: cleanup after qrencode build (fanquake) 745bf0fa7e9afc3989e9c60d7ef09e96ae172277 depends: cleanup after miniupnpc build (fanquake) 06d4aab77af4e75f0e8fd96a93e108f92210d878 depends: Cleanup postprocess commands after switching to CMake (Hennadii Stepanov) Pull request description: I overlooked this while reviewing https://github.com/bitcoin/bitcoin/pull/29723, https://github.com/bitcoin/bitcoin/pull/29835, and https://github.com/bitcoin/bitcoin/pull/29880. ACKs for top commit: fanquake: ACK a0314c151679a348d842b68c5ecb7a556700811c Tree-SHA512: debeffa7027e6213cc25c0652660ff0f36f51e63f688041d1d6cd6323e2c6cb02936fa0ecea86455b8c9874d6ea665684085189cfa523ca084792c57b0fb7c4e
2024-07-25Merge bitcoin/bitcoin#30511: guix: GCC 12 consolidationmerge-script
d1592d2eee1913e734a4f92907e796eb3350c64a guix: use gcc-12 to compile winpthreads (fanquake) b23690e8216f2b47e8e2c21a9ff057b25c4083ae guix: use GCC 12.4.0 over 12.3.0 (fanquake) 8b41ede55ebbc6978deb3f4fad5e18b76b372506 guix: consolidate back to GCC 12 toolchain for all HOSTS (fanquake) Pull request description: This PR contains 3 changes: * Bump GCC in Guix from [12.3.0 to 12.4.0](https://gcc.gnu.org/gcc-12/). A patch was sent upstream, https://lists.gnu.org/archive/html/guix-patches/2024-06/msg01025.html, but has not landed. * Consolidate all build environments back to using a GCC 12 toolchain. After #21778, the macOS environment is no-longer pinned to 11 (12 would otherwise cause issues building cctools). So, instead of requiring all builders to compile an additional GCC toolchain, use 12. * Use GCC 12 to compile winpthreads. Currently, GCC 11 is used; which became apparent in https://github.com/bitcoin/bitcoin/pull/30452#issuecomment-2244715566. ACKs for top commit: TheCharlatan: ACK d1592d2eee1913e734a4f92907e796eb3350c64a hebasto: ACK d1592d2eee1913e734a4f92907e796eb3350c64a. Tree-SHA512: e3aa1fa3e69500c93180e07cb4684661247ec6bc45245f746538d81406ff1d8777131590307496dda3287a112b6633e4991168586ca4c2036fa3a57b1efa9c87
2024-07-25Merge bitcoin/bitcoin#30399: test: Add arguments for creating a slimmer ↵merge-script
TestingSetup f46b2202560a76b473e229b77303b8f877c16cac fuzz: Use BasicTestingSetup for coins_view target (TheCharlatan) 9e2a723d5da4fc277a42fed37424f578e348ebf8 test: Add arguments for creating a slimmer setup (TheCharlatan) Pull request description: This adds arguments to some of the testing setup constructors for creating an environment without networking and a validation interface. This is useful for improving the performance of the utxo snapshot fuzz test, which constructs a new TestingSetup on each iteration. Using this slimmed down `TestingSetup` in future might also make the tests a bit faster when run in aggregate. ACKs for top commit: maflcko: review ACK f46b2202560a76b473e229b77303b8f877c16cac dergoegge: utACK f46b2202560a76b473e229b77303b8f877c16cac Tree-SHA512: 9dc62512b127b781fc9e2d8ef2b5a9b06ebb927a8294b6d872001c553984a7eb1f348e0257b32435b34b5505b5d0323f73bdd572a673da272d3e1e8538ab49d6
2024-07-25Merge bitcoin/bitcoin#30482: rest: Reject truncated hex txid early in ↵merge-script
getutxos parsing fac0c3d4bfc97b94f0594f7606650921feef2c8a doc: Add release notes for two pull requests (MarcoFalke) fa7b57e5f5a6dafbbadc361ffd27b58afff1ed59 refactor: Replace ParseHashStr with FromHex (MarcoFalke) fa9077724507faad207f29509a8202fc6ac9d502 rest: Reject truncated hex txid early in getutxos parsing (MarcoFalke) fab6ddbee64e50d5e2f499aebca35b5911896ec4 refactor: Expose FromHex in transaction_identifier (MarcoFalke) fad2991ba073de0bd1f12e42bf0fbaca4a265508 refactor: Implement strict uint256::FromHex() (MarcoFalke) fa103db2bb736bce4440f0bde564e6671e36311d scripted-diff: Rename SetHex to SetHexDeprecated (MarcoFalke) fafe4b80512a5a82712a3ee81b68cfeb21271dee test: refactor: Replace SetHex with uint256 constructor directly (MarcoFalke) Pull request description: In `rest_getutxos` truncated txids such as `aa` or `ff` are accepted. This is brittle at best. Fix it by rejecting any truncated (or overlarge) input. ---- Review note: This also starts a major refactor to rework hex parsing in Bitcoin Core, meaning that a few refactor commits are included as well. They are explained individually in the commit message and the work will be continued in the future. ACKs for top commit: stickies-v: re-ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a - only doc and test updates to address review comments, thanks! hodlinator: ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a Tree-SHA512: 473feb3fcf6118443435d1dd321006135b0b54689bfbbcb1697bb5811a449bef51f475c715de6911ff3c4ea3bdb75f601861ff93347bc4414d6b9e5298105dd7
2024-07-25refactor: Use untranslated error message in ActivateSnapshotMarcoFalke
The message is not exposed in the GUI or another translated context, so translating it is useless for now. Also, fix a nit from https://github.com/bitcoin/bitcoin/pull/30395#discussion_r1670972864
2024-07-25Merge bitcoin/bitcoin#30522: ci: Add missing qttools5-dev install to Asan taskmerge-script
faa359877270121b3cd442e1e5e865586ce7e530 ci: Add missing qttools5-dev install to Asan task (MarcoFalke) Pull request description: This is required, according to the docs: ``` $ git grep --line-number 'qtbase5-dev qttools5-dev qttools5-dev-tools' doc doc/build-unix.md:84: sudo apt-get install qtbase5-dev qttools5-dev qttools5-dev-tools ``` Also, needed for cmake. ACKs for top commit: hebasto: ACK faa359877270121b3cd442e1e5e865586ce7e530. Tree-SHA512: c986908f757d70d958267c1e902b5d7d94589360db61ddf7b9b398cd635b2172e83510c0c77fd6032810166342a286c0f95225b6c6639acd869e1e51c3348ea7
2024-07-25depends: cleanup after qrencode buildfanquake
2024-07-25depends: cleanup after miniupnpc buildfanquake
2024-07-25depends: Cleanup postprocess commands after switching to CMakeHennadii Stepanov
2024-07-25m_tx_download_mutex followupsglozow
- add AssertLockNotHeld(m_tx_download_mutex) in net_processing - move doc about m_tx_download_mutex and mempool mutex to ActiveTipChange
2024-07-25release m_tx_download_mutex before MakeAndPushMessage GETDATAglozow
2024-07-25[refactor] change ActiveTipChange to use CBlockIndex ref instead of ptrglozow
2024-07-25Merge bitcoin/bitcoin#30275: Fee Estimation: change `estimatesmartfee` ↵merge-script
default mode to `economical` 25bf86a225b0df3f48ade1016b47f5ee1636b988 [test]: ensure `estimatesmartfee` default mode is `economical` (ismaelsadeeq) 41a2545046bce315af697a3c6baf6e3fb2e824c2 [fees]: change `estimatesmartfee` default mode to `economical` (ismaelsadeeq) Pull request description: Fixes #30009 This PR changes the `estimatesmartfee` default mode to `economical`. This was also suggested on IRC https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-04-26#1021609 - `conservative` mode: This is the `estimatesmartfee` RPC mode which considers a longer history of blocks. It potentially returns a higher fee rate and is more likely to be sufficient for the desired target, but it is not as responsive to short-term drops in the prevailing fee market. - `economical` mode: This is the `estimatesmartfee` RPC mode where estimates are potentially lower and more responsive to short-term drops in the prevailing fee market. Since users are likely to use the default mode, this change will reduce overestimation for many users. The conservative mode remains available for those who wish to opt-in. For an in-depth analysis of how significantly the `conservative` mode overestimates, see https://delvingbitcoin.org/t/bitcoind-policy-estimator-modes-analysis/964. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30275/commits/25bf86a225b0df3f48ade1016b47f5ee1636b988 glozow: ACK 25bf86a225b0df3f48ade1016b47f5ee1636b988 willcl-ark: ACK 25bf86a225b0df3f48ade1016b47f5ee1636b988 Tree-SHA512: 78ebda667eb9c8f87dcc2f0e6c14968bd1de30358dc77a13611b186fb8427ad97d9f537bad6e32e0a1aa477ccd8c64fee4d41e19308ef3cb184ff1664e6ba8a6
2024-07-25ci: Add missing qttools5-dev install to Asan taskMarcoFalke
2024-07-25Merge bitcoin/bitcoin#30519: ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to ↵merge-script
TSAN (libc++) job e3edaccd9deb2da50be70d2d8768eca8821785c7 ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN job (fanquake) 6e786165ca08013fe3cfb2641241133563a3f051 refactor: fix missing includes (fanquake) Pull request description: Add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to one of the libc++ CI jobs, to catch missing includes, that are otherwise hidden by transitive includes inside libc++. A more appropriate place for this might be the tidy job, but that does not use libc++. See https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html for more information. ACKs for top commit: maflcko: re-ACK e3edaccd9deb2da50be70d2d8768eca8821785c7 Tree-SHA512: 3fb2e9bbbf4bb1570633d52939875ee674d934b645a4037a309643f84ab69edf0fb5b6cfcbd02fa7d92052a64fa63f31979a58fede23593c4df7c33a8cb2953a
2024-07-24doc: Add release notes for two pull requestsMarcoFalke
2024-07-24refactor: Replace ParseHashStr with FromHexMarcoFalke
No need to have two functions with different names that achieve the exact same thing.
2024-07-24rest: Reject truncated hex txid early in getutxos parsingMarcoFalke
2024-07-24refactor: Expose FromHex in transaction_identifierMarcoFalke
This is needed for the next commit.
2024-07-24refactor: Implement strict uint256::FromHex()MarcoFalke
This is a safe replacement of the previous SetHex, which now returns an optional to indicate success or failure. The code is similar to the ParseHashStr helper, which will be removed in a later commit.
2024-07-24ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN jobfanquake
See: https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html.
2024-07-24refactor: fix missing includesfanquake
These cause compile failures with _LIBCPP_REMOVE_TRANSITIVE_INCLUDES. i.e: ```bash In file included from init.cpp:8: ./init.h:46:54: error: no template named 'atomic' in namespace 'std' 46 | bool AppInitBasicSetup(const ArgsManager& args, std::atomic<int>& exit_status); | ~~~~~^ 1 error generated. ``` See: https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html.
2024-07-24[test]: ensure `estimatesmartfee` default mode is `economical`ismaelsadeeq
2024-07-24[doc] TxOrphanage is no longer thread-safeglozow
2024-07-24[refactor] combine block vtx loops in BlockConnectedglozow
Now that m_txrequest and m_recent_confirmed_transactions are guarded by the same mutex, there is no benefit to processing them separately. Instead, just loop through pblock->vtx once.
2024-07-24cleanse: Use SecureZeroMemory for mingw-w64 (release) buildsfanquake
2024-07-24Merge bitcoin/bitcoin#30423: contrib: simplify `test-security-check`merge-script
1bc9f64bee919bc46eb061ef8c66f936eb6a8918 contrib: assume binary existence in sec/sym checks (fanquake) 51d8f435c9ce8af0460380e52026b6d65b1de398 contrib: simplify ELF test-security-check (fanquake) 1810e20677fff974827ec433a4614d6fdad462b0 contrib: simplify PE test-security-check (fanquake) 6c9746ff9248e4f3c931a9bfd4dcc5f8bec7d412 contrib: simplify MACHO test-security-check (fanquake) Pull request description: The current `test-security-check` script is hard to understand, and change (i.e https://github.com/bitcoin/bitcoin/pull/29987/files#diff-52aa0cda44721f089e53b128cb1232a876006ef257b211655456b17dfb2ec712); tests are also not done in isolation (when-possible). Fix that, and add missing checks. Simplifies future toolchain/security/hardening changes. ACKs for top commit: hebasto: ACK 1bc9f64bee919bc46eb061ef8c66f936eb6a8918 (assuming my Guix hashes match; I'll provide them shortly). TheCharlatan: ACK 1bc9f64bee919bc46eb061ef8c66f936eb6a8918 Tree-SHA512: 1885d0ce63a94ffa61345327f919da20b63de6dd4148d6db3ee8bad4485253a36e8ab0dbee48cecc02ea35d139edfed75453af45fc364bcbef6fe16b6823bc7a
2024-07-24Merge bitcoin/bitcoin#30111: locks: introduce mutex for tx download, flush ↵merge-script
rejection filters once per tip change c85accecafc20f6a6ae94bdf6cdd3ba9747218fd [refactor] delete EraseTxNoLock, just use EraseTx (glozow) 6ff84069a5dd92303ed2ec28f0ec7c96bbda3938 remove obsoleted TxOrphanage::m_mutex (glozow) 61745c7451ec64b26c74f672c688e82efb3b96aa lock m_recent_confirmed_transactions using m_tx_download_mutex (glozow) 723ea0f9a5b5e3f3f58ea049a98299ff0ebde468 remove obsoleted hashRecentRejectsChainTip (glozow) 18a43552509603ddf83b752fd7b4b973ba1dcf82 update recent_rejects filters on ActiveTipChange (glozow) 36f170d87924e50d0ff9be2a1b0f2a8f13950a9b add ValidationInterface::ActiveTipChange (glozow) 3eb1307df0a38ac4ea52995fbb03ead37387b41e guard TxRequest and rejection caches with new mutex (glozow) Pull request description: See #27463 for full project tracking. This contains the first few commits of #30110, which require some thinking about thread safety in review. - Introduce a new `m_tx_download_mutex` which guards the transaction download data structures including `m_txrequest`, the rolling bloom filters, and `m_orphanage`. Later this should become the mutex guarding `TxDownloadManager`. - `m_txrequest` doesn't need to be guarded using `cs_main` anymore - `m_recent_confirmed_transactions` doesn't need its own lock anymore - `m_orphanage` doesn't need its own lock anymore - Adds a new `ValidationInterface` event, `ActiveTipChanged`, which is a synchronous callback whenever the tip of the active chainstate changes. - Flush `m_recent_rejects` and `m_recent_rejects_reconsiderable` on `ActiveTipChanged` just once instead of checking the tip every time `AlreadyHaveTx` is called. This should speed up calls to that function (no longer comparing a block hash each time) and removes the need to lock `cs_main` every time it is called. Motivation: - These data structures need synchronization. While we are holding `m_tx_download_mutex`, these should hold: - a tx hash in `m_txrequest` is not also in `m_orphanage` - a tx hash in `m_txrequest` is not also in `m_recent_rejects` or `m_recent_confirmed_transactions` - In the future, orphan resolution tracking should also be synchronized. If a tx has an entry in the orphan resolution tracker, it is also in `m_orphanage`, and not in `m_txrequest`, etc. - Currently, `cs_main` is used to e.g. sync accesses to `m_txrequest`. We should not broaden the scope of things it locks. - Currently, we need to know the current chainstate every time we call `AlreadyHaveTx` so we can decide whether we should update it. Every call compares the current tip hash with `hashRecentRejectsChainTip`. It is more efficient to have a validation interface callback that updates the rejection filters whenever the chain tip changes. ACKs for top commit: instagibbs: reACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd dergoegge: Code review ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd theStack: Light code-review ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd hebasto: ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd, I have reviewed the code and it looks OK. Tree-SHA512: c3bd524b5de1cafc9a10770dadb484cc479d6d4c687d80dd0f176d339fd95f73b85cb44cb3b6b464d38a52e20feda00aa2a1da5a73339e31831687e4bd0aa0c5
2024-07-24Merge bitcoin/bitcoin#30513: depends: Bump `libmultiprocess` for CMake fixesmerge-script
ec0e805d11d6a73c542032fc49a58a1d05b62d24 depends: Bump `libmultiprocess` for CMake fixes (Hennadii Stepanov) Pull request description: This PR amends https://github.com/bitcoin/bitcoin/pull/30490 and bumps the upstream branch, which now includes a required CMake [fix](https://github.com/chaincodelabs/libmultiprocess/pull/103). Addresses https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2241153244. The CI logs are available in https://github.com/bitcoin/bitcoin/pull/29790 where the recent [push](https://github.com/hebasto/bitcoin/tree/pr29790-0723.2.mp) uses this PR implementation. ACKs for top commit: ryanofsky: Code review ACK ec0e805d11d6a73c542032fc49a58a1d05b62d24 theuni: utACK ec0e805d11d6a73c542032fc49a58a1d05b62d24. Tree-SHA512: e300a27bcab80a63a518719e9af8e10a938294fc07289cadbf4a7744627c10b0e9541a36971d08b65152f3f7d0eb434e427274d9c9d9f0bdd216afd914027a0f
2024-07-24Merge bitcoin/bitcoin#29878: depends: build expat with CMakemerge-script
a517029646ac86f9d72fcea204ff45db41702e37 depends: switch to building expat with CMake (fanquake) Pull request description: Switch to building Expat with CMake, instead of Autotools. ACKs for top commit: hebasto: re-ACK a517029646ac86f9d72fcea204ff45db41702e37. Tree-SHA512: ca040545dd83fb81a8b209aa24cae6e22eaeff04f44bdabc4454adf6ea63d34f4ae27bd5980c65db2d2542e23eb2712102719023c262ab63a933c90b5999c11e
2024-07-24refactor: Add FlatFileSeq member variables in BlockManagerTheCharlatan
Instead of constructing a new class every time a file operation is done, construct them once for each of the undo and block file when a new BlockManager is created. In future, this might make it easier to introduce an abstract block store.
2024-07-24scripted-diff: Rename SetHex to SetHexDeprecatedMarcoFalke
SetHex is fragile, because it accepts any non-hex input or any length of input, without error feedback. This can lead to issues when the input is truncated or otherwise corrupted. Document the problem by renaming the method. In the future, the fragile method should be removed from the public interface. -BEGIN VERIFY SCRIPT- sed -i 's/SetHex/SetHexDeprecated/g' $( git grep -l SetHex ./src ) -END VERIFY SCRIPT-
2024-07-24test: refactor: Replace SetHex with uint256 constructor directlyMarcoFalke
This avoids a hex-decoding and makes the next commit smaller.
2024-07-23depends: Bump `libmultiprocess` for CMake fixesHennadii Stepanov
2024-07-23Merge bitcoin/bitcoin#30436: fix: Make TxidFromString() respect string_view ↵Ryan Ofsky
length 09ce3501fa2ea2885a857e380eddb74605f7038c fix: Make TxidFromString() respect string_view length (Hodlinator) 01e314ce0ae30228742b6f19d2f12a050ab97e4d refactor: Change base_blob::SetHex() to take std::string_view (Hodlinator) 2f5577dc2e7ba668798a89a2f6ef72795db6c285 test: uint256 - Garbage suffixes and zero padding (Hodlinator) f11f816800ac520064a1e96871d0b4cc9601ced7 refactor: Make uint256_tests no longer use deprecated BOOST_CHECK() (Hodlinator) f0eeee2dc1329b0647df09bea9ccc0395bb82698 test: Add test for TxidFromString() behavior (Ryan Ofsky) Pull request description: ### Problem Prior to this, `TxidFromString()` was passing `string_view::data()` into `uint256S()` which meant it would only receive the a naked `char*` pointer and potentially scan past the `string_view::length()` until it found a null terminator (or some other non-hex character). Appears to have been a fully dormant bug as callers were either passing a string literal or `std::string` directly to `TxidFromFromString()`, meaning a null terminator always existed at `pointer[length()]`. Bug existed since original merge of `TxidFromString()`. ### Solution Make `uint256S()` (and `base_blob::SetHex()`) take and operate on `std::string_view` instead of `const char*` and have `TxidFromString()` pass that in. (PR was prompted by comment in https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2208857200 (referring to https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378)). ACKs for top commit: maflcko: re-ACK 09ce3501fa2ea2885a857e380eddb74605f7038c 🕓 paplorinc: ACK 09ce3501fa2ea2885a857e380eddb74605f7038c ryanofsky: Code review ACK 09ce3501fa2ea2885a857e380eddb74605f7038c. I think the current code changes are about as small as you could make to fix the bug without introducing a string copy, and the surrounding test improvements are all very nice and welcome. Tree-SHA512: c2c10551785fb6688d1e2492ba42a8eee4c19abbe8461bb0774d56a70c23cd6b0718d2641632890bee880c06202dee148126447dd2264eaed4f5fee7e1bcb581
2024-07-23Merge bitcoin/bitcoin#30408: rpc: doc: use "output script" terminology ↵Ava Chow
consistently in "asm"/"hex" results 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b rpc: doc: use "output script" terminology consistently in "asm"/"hex" results (Sebastian Falbesoner) Pull request description: The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script". See also the draft BIP "Terminology for Transaction Components" (https://github.com/murchandamus/bips/blob/2022-04-tx-terminology/bip-tx-terminology.mediawiki) from murchandamus which suggests to use "output script" as well. Affects the help text of the following RPCs: - decodepsbt - decoderawtransaction - decodescript - getblock (if verbosity=3) - getrawtransaction (if verbosity=2,3) - gettxout ACKs for top commit: maflcko: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b achow101: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b BrandonOdiwuor: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b tdb3: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b Tree-SHA512: 62eb92d42bc44e36dc3090df7b248a123868a74af253d2046de02086e688bf6ff98307b927ba2fee3d599f85e073aeb8eca90ed15105ca63b648b6796cfa340b
2024-07-23test: Fix intermittent issue in p2p_v2_misbehaving.pyMarcoFalke
Without the fix, the test could fail intermittently. For example: node0 2024-07-22T16:31:54.104994Z [httpworker.0] [rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=setmocktime user=__cookie__ test 2024-07-22T16:31:54.291000Z TestFramework (INFO): Sending first 4 bytes of ellswift which match network magic test 2024-07-22T16:31:54.292000Z TestFramework (INFO): If a response is received, assertion failure would happen in our custom data_received() function test 2024-07-22T16:31:54.292000Z TestFramework.p2p (DEBUG): Connecting to Bitcoin Node: 127.0.0.1:12644 test 2024-07-22T16:31:54.293000Z TestFramework.p2p (DEBUG): Connected & Listening: 127.0.0.1:12644 test 2024-07-22T16:31:54.588000Z TestFramework.p2p (DEBUG): sending 4050 bytes of garbage data test 2024-07-22T16:31:54.588000Z TestFramework (INFO): Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is test 2024-07-22T16:31:54.588000Z TestFramework (INFO): expected now, our custom data_received() function wouldn't result in assertion failure node0 2024-07-22T16:31:55.523868Z (mocktime: 2024-07-22T16:31:54Z) [net] [net.cpp:3764] [CNode] [net] Added connection peer=0 node0 2024-07-22T16:31:55.625145Z (mocktime: 2024-07-22T16:31:54Z) [net] [net.cpp:1814] [CreateNodeFromAcceptedSocket] [net] connection from 127.0.0.1:45154 accepted node0 2024-07-22T16:31:55.625769Z (mocktime: 2024-07-22T16:31:54Z) [http] [httpserver.cpp:305] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:33320 node0 2024-07-22T16:31:55.626543Z (mocktime: 2024-07-22T16:31:54Z) [httpworker.1] [rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=getpeerinfo user=__cookie__ test 2024-07-22T16:31:55.818000Z TestFramework (ERROR): Unexpected exception caught during testing Traceback (most recent call last): File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main self.run_test() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 133, in run_test self.test_earlykeyresponse() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 151, in test_earlykeyresponse self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4) File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 791, in wait_until return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.options.timeout_factor) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 289, in wait_until_helper_internal if predicate(): ^^^^^^^^^^^ File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 151, in <lambda> self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4) ~~~~~~~~~~~~~~~~~~~^^^^ IndexError: list index out of range
2024-07-23net: Log accepted connection after m_nodes.push_backMarcoFalke
Otherwise, the debug log could read confusingly, when the getpeerinfo() RPC (calling GetNodeStats) happens after the "accepted connection" log line, but returns an empty list. For example, the following timeline in the debug log could correspond to a getpeerinfo reply that is empty: [net] [net.cpp:3764] [CNode] Added connection peer=0 [net] [net.cpp:1814] [CreateNodeFromAcceptedSocket] connection from 127.0.0.1:45154 accepted [http] [httpserver.cpp:305] [http_request_cb] Received a POST request for / from 127.0.0.1:33320 [httpworker.1] [rpc/request.cpp:232] [parse] ThreadRPCServer method=getpeerinfo user=__cookie__ Fix it by moving the log line.
2024-07-23Merge bitcoin/bitcoin#30403: test, assumeutxo: Remove resolved todo comments ↵Ava Chow
and add new test d63ef738001fb69ce04134cc8645dcd1e1cbccd1 test: Add loadtxoutset test with tip on snapshot block (Fabian Jahr) c2f86d4bcba290c33ed99383cc76380bb15ba384 test: Remove already resolved assumeutxo todo comments (Fabian Jahr) Pull request description: The first commit removes three Todos that have been addressed previously (see commit message for details). The second message resolves another todo by adding the missing test case. This is a special case of "the tip has more work than the snapshot" where the tip is the same block as the snapshot base block. Related to #28648. ACKs for top commit: jrakibi: ACK [d63ef73](https://github.com/bitcoin/bitcoin/commit/d63ef738001fb69ce04134cc8645dcd1e1cbccd1) achow101: ACK d63ef738001fb69ce04134cc8645dcd1e1cbccd1 maflcko: ACK d63ef738001fb69ce04134cc8645dcd1e1cbccd1 alfonsoromanz: Re ACK d63ef738001fb69ce04134cc8645dcd1e1cbccd1 Tree-SHA512: 8d5a25fc0b26531db3a9740132694138f2103b7b42eeb1d4a64095bfc901c1372e23601c0855c7def84c8a4e185d10611e4e830c4e479f1b663ae6ed53abb130
2024-07-23guix: use gcc-12 to compile winpthreadsfanquake
Currently, winpthreads is compiled with GCC 11, when we want to be using GCC 12 for all compilation.
2024-07-23depends: switch to building expat with CMakefanquake
Add a patch to set the minimum CMake to 3.16.
2024-07-23guix: use GCC 12.4.0 over 12.3.0fanquake
Our patch might be merged upstream soon: https://lists.gnu.org/archive/html/guix-patches/2024-06/msg01025.html. In the mean time, it's easy us for us to use the newer version of GCC.
2024-07-23guix: consolidate back to GCC 12 toolchain for all HOSTSfanquake
Using GCC 11 for the macOS build hasn't been required since #21778, and at this point, given a toolchain is still needed (#30206), it makes more sense to (re-)use 12, rather than make all builders compile another GCC toolchain.
2024-07-23fix: Make TxidFromString() respect string_view lengthHodlinator
Prior to this, passing string_view::data() into uint256S() meant the latter would only receive the a naked char* pointer and potentially scan past the string_view::length() until it found a null terminator (or some other non-hex character). Appears to have been a fully dormant bug as callers were either passing a string literal or std::string directly to TxidFromFromString(), meaning null terminator always existed at pointer[length()]. Bug existed since original merge of TxidFromString(), discussed in https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378.