This introduces an early exit in PeerManagerImpl::NewPoWValidBlock() if
segwit has not been activated for the block. This means that we won't cache the
block/compact block for fast relay and won't relay the cmpctblock
immediately to peers that have requested hb compact blocks. This is fine
because any block where segwit is not yet activated is buried deep in
the chain, and so compact block relay will not be effective.
It's ok not to cache the block/compact block for fast relay for the same
reason - the block must be very deeply buried in the block chain.
ProcessBlockAvailability() also won't get called for all nodes. This is
also fine, since that function only updates hashLastUnknownBlock
and pindexBestKnownBlock, and is called early in every SendMessages()
call.
Subsequent commits will remove support for other versions of compact blocks.
Add a test that a received `sendcmpct` message with version = 1 is
ignored.
4faa550072 test: Fix race condition in index pruning test (Fabian Jahr)
Pull request description:
Fixes#25031
The `feature_index_prune.py` test seems to be racy because connections are reestablished after restarts and the blocks are synced via the `sync_blocks` function. The `sync_blocks` function has a sanity check at the beginning to check that all nodes in the set have at least one established connection and that is not always the case.
As a solution nodes are not connected via the `-connect` parameter on start but instead via the `connect_nodes` helper.
Top commit has no ACKs.
Tree-SHA512: f88377715f455f1620725fe8ebd6b486fa0209660b193bf68d1ce1452e2086ac5d169d8ca4c2b61443566232e96fb9c6386ee482bc546cce38078d72e7c3c29f
Nodes are restarted and reconnected as part of the test. Afterwards
`sync_blocks` is called immediately on the nodes. `sync_blocks`
first checks that all the included nodes have at least one
connection. Since adding a connection is usually happening in a
thread, sometimes nodes could run into this check before the
connection was fully established so that it would fail the entire
test.
This fix uses the `connect_nodes` helper to make the connection the
nodes. `connect_nodes` has a wait for the connection built into it.
In order to deserialize received or read messages via lookup in
MESSAGEMAP (e.g.: `t = MESSAGEMAP[msgtype]()`), the messages must have a
default constructor, i.e. there needs to be the possibility to
initialize them with zero arguments.
f403531f97 Squashed 'src/univalue/' changes from a44caf65fe..6c19d050a9 (MacroFake)
Pull request description:
Only change is some header-shuffling and adding `getInt`.
ACKs for top commit:
fanquake:
ACK fac2c796cb
Tree-SHA512: 8bdf7d88ce06f0851f99f30c30fc926a13b79ae72bcebd5e170ed0e1d882b184d9279f96488e234fbe560036e31cb180aa1e5666aebd9833b9a119c6b214fa30
bb5c24b120 validation: move g_versionbitscache into ChainstateManager (Anthony Towns)
eca22c726a test/versionbits: make versionbitscache a parameter (Anthony Towns)
d603f1d8a7 deploymentstatus: make versionbitscache a parameter (Anthony Towns)
78adef1753 refactor: use chainman instead of chainParams for DeploymentActive* (Anthony Towns)
deffe0df6c deploymentstatus: allow chainman in place of consensusParams (Anthony Towns)
eaa2e3f25c validation: move UpdateUncommittedBlockStructures and GenerateCoinbaseCommitment into ChainstateManager (Anthony Towns)
5c67e84d37 validation: replace ::Params() calls with chainstate/chainman member (Anthony Towns)
38860f93b6 validation: remove redundant CChainParams params from ChainstateManager methods (Anthony Towns)
69675ea4e7 validation: add CChainParams to ChainstateManager (Anthony Towns)
Pull request description:
Gives `ChainstateManager` a reference to the `CChainParams` its working on, and simplifies some of the functions that would otherwise take that as a parameter. Removes the `g_versionbitscache` global by moving it into `ChainstateManager`.
ACKs for top commit:
dongcarl:
reACK bb5c24b120
MarcoFalke:
review ACK bb5c24b120📙
Tree-SHA512: 3fa74905e5df561e3e74bb0b8fce6085c5311e6633e7d74c0fb0c82a907f5bbb1fd4ebc5d11d4f0b1c019bb51eabb9f6e4bcc4652a696d36a5878c807b85f121
faa5a7a573 test: Check msg type in msg capture is followed by zeros (MacroFake)
Pull request description:
Checking that they are not printable is an odd (and wrong) way to check that all chars are zero.
ACKs for top commit:
theStack:
Code-review ACK faa5a7a573
Tree-SHA512: 63e001bd25298dcf47606f8ab11ddfb704ca963304149b0f6e188eb7dcf45c41f92d39f26bda32bceb03384720c9bdddb2673dba513cd9242dc9663d498b3f29
51ec96b904 refactor: move StartExtraBlockRelayPeers from header to implementation (Jon Atack)
Pull request description:
where all the other logging actions in src/net.{h,cpp} are located.
StartExtraBlockRelayPeers() does not appear to be a hotspot that needs to be inlined for performance, as it is called from CheckForStaleTipAndEvictPeers(), called in turn from StartScheduledTasks() with a scheduleEvery delta of 45 seconds, called at the end of AppInitMain() on bitcoind startup.
This allows dropping `#include <logging.h>` from net.h, which can improve compile time/speed. Currently, none of the other includes in net.h use logging.h, except src/sync.h if DEBUG_LOCKCONTENTION is defined.
ACKs for top commit:
LarryRuane:
ACK 51ec96b904
theStack:
ACK 51ec96b904
Tree-SHA512: 69b2c09163c48bfcb43355af0aa52ee7dd81efc755a7aa6a10f5e400b5e14109484437960a62a1cfac2524c2cfae981fee082846b19526b540ef5b86be97f0fe
1df42bc262 test: compare `/mempool/info` response with `getmempoolinfo` RPC (brunoerg)
Pull request description:
This PRs compares `/mempool/info` REST response with `getmempoolinfo` RPC in `interface_rest.py`.
Similar to #24936 and #24797.
ACKs for top commit:
theStack:
ACK 1df42bc262
Tree-SHA512: 2de36d70fa61612e7976f875e55f98e78b1cdb909b48cff18e6a70c55eda34b799e210bcd55361ea947388b7778d867290a73be4f799bb36afd65423ad49c487
672d49c863 scripted-diff: replace non-standard fixed width integer types (`u_int`...` -> `uint`...) (Sebastian Falbesoner)
Pull request description:
Fixed width integer types prefixed with `u_int` are not part of C++ (see https://en.cppreference.com/w/cpp/types/integer), so it's better to avoid and replace them with their standard-conforming counterparts. (For those interested in history, according to one theory those u_int... types have been introduced by BSD: https://stackoverflow.com/a/5163960, http://lists.freedesktop.org/archives/release-wranglers/2004-August/000923.html).
ACKs for top commit:
laanwj:
Code review ACK 672d49c863
fanquake:
ACK 672d49c863
Tree-SHA512: 68134a0adca0d5c87a7432367cb493491a67288d69a174be2181f8e26efa968d966b9eb1cde94813942405063ee3be2a3437cf2aa5f71375f59205cbdbf501bb
where all the other logging actions in src/net.{h,cpp} are located.
StartExtraBlockRelayPeers() does not appear to be a hotspot that needs to be
inlined for performance, as it is called from CheckForStaleTipAndEvictPeers(),
called in turn from StartScheduledTasks() with a scheduleEvery delta of 45
seconds, called at the end of AppInitMain() on bitcoind startup.
This allows dropping `#include <logging.h>` from net.h, which can improve
compile time/speed. Currently, none of the other includes in net.h use
logging.h, except src/sync.h if DEBUG_LOCKCONTENTION is defined.
6c19d050a9 Merge bitcoin-core/univalue-subtree#33: Add getInt<Integral>() helper
09e4a930fc Add getInt helper
10619e0d9a Merge bitcoin-core/univalue#32: refactor: include-what-you-use
431cdf5d27 refactor: use constexpr where appropriate
64fc881fa4 refactor: cleanup headers for iwyu
9c35bf38eb Merge bitcoin-core/univalue-subtree#30: doc: note that our API has diverged from upstream
09b65facb9 doc: note that our API has diverged from upstream
git-subtree-dir: src/univalue
git-subtree-split: 6c19d050a9bcb2be216121db0df57c930a9ee12e
fab9e8a29c Remove unused GetTimeSeconds (MacroFake)
Pull request description:
Seems confusing to have this helper when it is possible to get the system time in a type-safe way by simply calling `std::chrono::system_clock::now` (C++11).
This patch replaces `GetTimeSeconds` and removes it:
* in `bitcoin-cli.cpp` by `system_clock`
* in `test/fuzz/fuzz.cpp` by `steady_clock`
ACKs for top commit:
laanwj:
Code review ACK fab9e8a29c
naumenkogs:
ACK fab9e8a29c
Tree-SHA512: 517e300b0baf271cfbeebd4a0838871acbea9360f9dd23572a751335c20c1ba261b1b5ee0aec8a36abd20c94fab83ce94f46042745279aca1f0ca2f885a03b6e
Compiling glibc 2.24 and 2.27 with the new GCC 10 results in a number of new warnings,
i.e:
```bash
libc-tls.c: In function ‘__libc_setup_tls’:
libc-tls.c:208:30: error: array subscript 1 is outside the bounds of an interior zero-length array ‘struct dtv_slotinfo[0]’ [-Werror=zero-length-bounds]
208 | static_slotinfo.si.slotinfo[1].map = main_map;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from ../sysdeps/x86_64/ldsodefs.h:54,
from ../sysdeps/gnu/ldsodefs.h:46,
from ../sysdeps/unix/sysv/linux/ldsodefs.h:25,
from libc-tls.c:20:
../sysdeps/generic/ldsodefs.h:398:7: note: while referencing ‘slotinfo’
398 | } slotinfo[0];
| ^~~~~~~~
```
While we could try and backport all the patches required to fix these up, it would
currently seem easier to disable -Werror, which Guix uses by default when building
glibc.
ab1ea29ba1 refactor: make GetRand a template, remove GetRandInt (pasta)
Pull request description:
makes GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
This simplifies a lot of code from GetRand(std::numeric_limits<uint64_t>::max() -> GetRand<uint64_t>()
ACKs for top commit:
laanwj:
Code review ACK ab1ea29ba1
Tree-SHA512: db5082a0e21783389f1be898ae73e097b31ab48cab1a2c0e29348a4adeb545d4098193aa72a547c6baa6e8205699aafec38d6a27b3d65522fb3246f91b4daae9
7fd0860d12 Bugfix: configure: Define defaults for enable_arm_{crc,shani} (Luke Dashjr)
Pull request description:
Fix for #17398 and #24115
Trivial, mostly for consistency (you'd have to *try* to break this)
ACKs for top commit:
pk-b2:
ACK 7fd0860d12
seejee:
ACK 7fd0860d12
vincenzopalazzo:
ACK 7fd0860d12
Tree-SHA512: 51c389787c369f431ca57071f03392438bff9fd41f128c63ce74ca30d2257213f8be225efcb5c1329ad80b714f44427d721215d4f848cc8e63060fa5bc8f1f2e
fa90516422 Switch scheduler to steady_clock (MacroFake)
Pull request description:
There is already `mockscheduler`, so it seems brittle, confusing and redundant to be able to mock the scheduler by adjusting the system clock.
ACKs for top commit:
laanwj:
Code review ACK fa90516422
w0xlt:
crACK fa90516422
Tree-SHA512: 60e99065ffb881a9fb25a346d311d99424fbc72a3b636c94b5f5c17ed6373c40f358a9b27825c518d12968c033e6cfd3c62d2b62cacdddc44a0b5b74f6c1a7ae
bdc6881e2f wallet: Change log interval to use `steady_clock` (w0xlt)
Pull request description:
This refactors the log interval variables to use `steady_clock` as it is best suitable for measuring intervals.
ACKs for top commit:
laanwj:
This makes sense. Code review ACK bdc6881e2f
dunxen:
Code review ACK bdc6881
Tree-SHA512: 738b4aa45cef01df77102320f83096a0a7d0c63d7fcf098a8c0ab16b29453a87dc789c110105590e1e215d03499db1d889a94f336dcb385b6883c8364c9d39b7
9feb887082 rpc: check `fopen` return code in dumptxoutset (Sebastian Falbesoner)
Pull request description:
This change improves the usability of the `dumptxoutset` RPC in two ways, in the case that an invalid path is passed:
1. return from the RPC immediately, rather then when the file is first tried to be written (which is _after_ calculating the UTXO set hash)
2. return a proper return code and error message instead of the cryptic message that appears on master currently (see below)
master branch:
(error message appears after several minutes on my machine)
```
$ ./src/bitcoin-cli dumptxoutset /invalid/path
error code: -1
error message:
CAutoFile::operator<<: file handle is nullptr: unspecified iostream_category error
```
PR branch:
(error message appears immediately)
```
$ ./src/bitcoin-cli dumptxoutset /invalid/path
error code: -8
error message:
Couldn't open file /invalid/path.incomplete for writing.
```
ACKs for top commit:
w0xlt:
Code Review ACK 9feb887082
Tree-SHA512: e8695a7e86f26cc3b086d6bc6888388061f1dee439f76409b3ee11d35032bfd9cfa5349b728cd7f45bcffd999ecf9a6a991be172ce587b9b14503d9916b6e984
This change improves the usability of the `dumptxoutset` RPC in two ways,
in the case that an invalid path is passed:
1. return from the RPC immediately, rather then when the file is first
tried to be written (which is _after_ calculating the UTXO set hash)
2. return a proper return code and error message instead of the cryptic
"CAutoFile::operator<<: file handle is nullptr: unspecified
iostream_category error" (-1)
