Age | Commit message (Collapse) | Author |
|
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.
|
|
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.
|
|
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.
|
|
|
|
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|
|
|
|
|
|
- add AssertLockNotHeld(m_tx_download_mutex) in net_processing
- move doc about m_tx_download_mutex and mempool mutex to ActiveTipChange
|
|
|
|
|
|
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
|
|
|
|
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
|
|
|
|
No need to have two functions with different names that achieve the
exact same thing.
|
|
|
|
This is needed for the next commit.
|
|
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.
|
|
See: https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html.
|
|
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.
|
|
|
|
|
|
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.
|
|
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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.
|
|
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-
|
|
This avoids a hex-decoding and makes the next commit smaller.
|
|
|
|
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
|
|
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
|
|
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
|
|
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.
|
|
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
|
|
Currently, winpthreads is compiled with GCC 11, when we want to be using
GCC 12 for all compilation.
|
|
Add a patch to set the minimum CMake to 3.16.
|
|
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.
|
|
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.
|
|
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.
|