fa5b58ea01 test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)
Pull request description:
Fixes #30704
The goal of the test is to check the stalling timeout, not the block download timeout.
On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.
Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.
ACKs for top commit:
tdb3:
CR ACK fa5b58ea01
brunoerg:
utACK fa5b58ea01
Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
41ad84a00c seeds: Use fjahr's more up to date asmap (Ava Chow)
d8fd1e0faf seeds: Fixed seeds update (Ava Chow)
f1f24d7214 seeds: Add testnet4 fixed seeds file (Ava Chow)
8ace71c737 seeds: Remove manual onion and i2p seeds (Ava Chow)
ed5b86cbe4 seeds: Add testnet instructions (Ava Chow)
0676515397 seeds: Also pull from achow101 seeder (Ava Chow)
5bab3175a6 makeseeds: Configurable minimum blocks for testnet4's smaller chain (Ava Chow)
d2465dfac6 makeseeds: Shuffle ips after parsing (Ava Chow)
af550b3a0f makeseeds: Support CJDNS (Ava Chow)
d5a8c4c4bd makeseeds: Update user agent regex (Ava Chow)
Pull request description:
The [DNS seeder](https://github.com/achow101/dnsseedrs) that I wrote collects statistics on node reliability in the same way that sipa's seeder does, and also outputs this information in the same file format. Thus it can also be used in our fixed seeds update scripts. My seeder additionally crawls onion v3, i2p, and cjdns, so will now be able to set those fixed seeds automatically rather than curating manual lists.
In doing this update, I've found that `makeseeds.py` is missing newer versions from the regex as well as cjdns support; both of these have been updated.
I also noticed that the testnet fixed seeds are all manually curated and sipa's seeder does not appear to publish any testnet data. Since I am also running the seeder for testnet, I've added the commands to generate testnet fixed seeds from my seeder's data too.
Lastly, I've updated all of the fixed seeds. However, since my seeder has not found any cjdns nodes that met the reliability criteria (possibly due to connectivity issues present in those networks), I've left the previous manual seeds for that network.
ACKs for top commit:
fjahr:
re-ACK 41ad84a00c
virtu:
ACK [41ad84a](41ad84a00c)
Tree-SHA512: 6ba0141f053d9d6ae7d8c9574f061be38f3e65b28de1d6660c1885ab942623b5a0ec70754b4fcfc5d98fe970f5f179a940d5880b5061ed698f7932500e01d3ee
5b4f34006d devtools, utxo-snapshot: Fix block height out of range (pablomartin4btc)
Pull request description:
<details>
<summary>Fixing a <a href="https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2251032570">bug</a> in <code>utxo_snapshot.sh</code>.</summary>
```
/contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR}
Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n):
Disabling network activity
false
error code: -8
error message:
Block height out of range
```
And the user will see the following in the node and it would stay there if not reset:
```
2024-08-21T14:44:13Z UpdateTip: new best=00000000000000afa0cd000a16e244f56032735d41acd32ac00337aceb2a5240 height=235382 version=0x00000002 log2_work=69.987697 tx=17492185 date='2013-05-09T23:54:32Z' progress=0.016219 cache=71.0MiB(571085txo)
2024-08-21T14:44:13Z UpdateTip: new best=0000000000000087c5e0b820afff496b95ba44ad64640c73b234d3261d3f99d2 height=235383 version=0x00000002 log2_work=69.987750 tx=17492341 date='2013-05-09T23:54:47Z' progress=0.016219 cache=71.0MiB(571291txo)
2024-08-21T14:44:13Z UpdateTip: new best=000000000000014a4b5fddf3c8abb6209247255ca9e8df786b271dd1b2ac82a6 height=235384 version=0x00000002 log2_work=69.987804 tx=17492344 date='2013-05-10T00:20:18Z' progress=0.016219 cache=71.0MiB(571297txo)
2024-08-21T14:44:13Z SetNetworkActive: false
```
</details>
This is a "temporary" fix until #29553 gets merged, which will remove the script entirely.
Handle the "Block height out of range" error gracefully by checking if the node has synchronized to or beyond the required block height, otherwise without this validation the node would keep the network disabled if the user selected that option.
<details>
<summary>Provide a user-friendly message if the block height is out of range and exit the script cleanly.</summary>
```
/contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR}
Error: The node has not yet synchronized to block height 840001.
Please wait until the node has synchronized past this block height and try again.
```
</details>
ACKs for top commit:
achow101:
ACK 5b4f34006d
fjahr:
tACK 5b4f34006d
Tree-SHA512: 2b71286b627872d7cfdb367e29361afa3806a7ef9d65075b93892b735ff2ab729069e2f7259d30262909e73cef17fb7dca231615cc1863968cd042f4a2a4f901
e1d5dd732d test: check xor.dat recreated when missing (tdb3)
d1610962bf test: add null block xor key (tdb3)
1ad999b9da refactor: lift NUM_XOR_BYTES (tdb3)
d8399584dd refactor: move read_xor_key() to TestNode (tdb3)
d43948c3ef refactor: use unlink rather than os.remove (tdb3)
c8176f758b test: add blocks_key_path (tdb3)
Pull request description:
Builds on PR #30657.
Refactors `read_xor_key()` from `util.py` to `test_node.py` (comment https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1723358327)
Adds a check that `xor.dat` is created when missing (comment https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1717724161)
Help states:
```
-blocksxor
Whether an XOR-key applies to blocksdir *.dat files. The created XOR-key
will be zeros for an existing blocksdir or when `-blocksxor=0` is
set, and random for a freshly initialized blocksdir. (default: 1)
```
ACKs for top commit:
maflcko:
ACK e1d5dd732d
achow101:
ACK e1d5dd732d
theStack:
re-ACK e1d5dd732d
brunoerg:
reACK e1d5dd732d
Tree-SHA512: 325912ef646ec88e0a58e1ece263a2b04cbc06497e8fe5fcd603e509e80c6bcf84b09dd52dfac60e23013f07fc2b2f6db851ed0598649c3593f452c4a1424bd9
60055f1abc test: replace deprecated secp256k1 context flags usage (Sebastian Falbesoner)
Pull request description:
The flags `SECP256K1_CONTEXT_{SIGN,VERIFY}` have been marked as deprecated since libsecp256k1 version 0.2 (released in December 2022), with the recommendation to use SECP256K1_CONTEXT_NONE instead, see https://github.com/bitcoin-core/secp256k1/pull/1126 and 1988855079/CHANGELOG.md (L132). Note that in contrast to other deprecated functions/variables, these defines don't have a deprecated attribute and hence don't lead to a compiler warning (see https://github.com/bitcoin-core/secp256k1/pull/1126#discussion_r922105271), so they are not easily detected.
ACKs for top commit:
TheCharlatan:
ACK 60055f1abc
ismaelsadeeq:
utACK 60055f1abc
tdb3:
light CR and test ACK 60055f1abc
Tree-SHA512: d93cf49e018a58469620c0d2f50242141f22dabc70afb2a7cd64e416f4f55588714510ae5a877376dd1e6b6f7494261969489af4b18a1c9dff0d0dfdf93f1fa8
fa5aeab3cb test: Avoid duplicate curl call in get_previous_releases.py (MarcoFalke)
Pull request description:
Seems odd having to translate `404` to "Binary tag was not found". Also, it seems odd to write a for-loop over a list with one item.
Fix both issues by just using a single call to `curl --fail ...`.
Can be tested with: `test/get_previous_releases.py -b v99.99.99`
Before:
```
Releases directory: releases
Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0
Binary tag was not found
```
After:
```
Releases directory: releases
Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0
curl: (22) The requested URL returned error: 404
ACKs for top commit:
fanquake:
ACK fa5aeab3cb
brunoerg:
utACK fa5aeab3cb
tdb3:
tested ACK fa5aeab3cb
Tree-SHA512: d5d31e0bccdd9de9b4a8ecf2e69348f4e8cee773050c8259b61db1ce5de73f6fbfffbe8c4d2571f7bef2de29cb42fd244573deebfbec614e487e76ef41681b9c
cccc5bfd35 test: Enable detect_leaks=1 in ASAN_OPTIONS explicitly (MarcoFalke)
Pull request description:
It should be enabled by default, but being explicit can't hurt.
ACKs for top commit:
fanquake:
ACK cccc5bfd35
Tree-SHA512: ed284abd05c7a99c30b509844aa75785a5ccb506d8296a71347b4c328750a6a4ed1f87e7a3ec36ab17f27b467c033cc8ca5eb5e2b951f2ae7473327c5eb1ddae
59ff17e5af miner: adjust clock to timewarp rule (Sjors Provoost)
e929054e12 Add timewarp attack mitigation test (Sjors Provoost)
e85f386c4b consensus: enable BIP94 on regtest (Sjors Provoost)
dd154b0568 consensus: lower regtest nPowTargetTimespan to 144 (Sjors Provoost)
Pull request description:
Because #30647 reduced the timewarp attack threshold from 7200s to 600s, our miner code will fail to propose a block template (on testnet4) if the last block of the previous period has a timestamp two hours in the future. This PR fixes that and also adds a test.
The non-test changes in the last commit should be in v28, otherwise miners have to patch it themselves. If necessary I can split that out into a separate PR, but I prefer to get the tests in as well.
In order to add the test, we activate BIP94 on regtest.
In order for the test to run faster, we reduce its difficulty retarget period to 144, the same number that's already used for softfork activation logic. Regtest does not actually adjust its difficulty, so this change has no effect (except for `getnetworkhashps`, see commit).
An alternative approach would be to run this test on testnet4, by hardcoding its first 2015 in the test suite. But since the timewarp mitigation is a serious candidate for a future mainnet softfork, it seems better to just deploy it on regtest.
The next commits add a test and fix the miner code.
The `MAX_TIMEWARP` constant is moved to `consensus.h` so both validation and miner code have access to it.
ACKs for top commit:
achow101:
ACK 59ff17e5af
fjahr:
ACK 59ff17e5af
glozow:
ACK 59ff17e5af
Tree-SHA512: 50af9fdcba9b0d5c57e1efd5feffd870bd11b5318f1f8b0aabf684657f2d33ab108d5f00b1475fe0d38e8e0badc97249ef8dda20c7f47fcc1698bc1008798830
917e70a620 test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate (Sebastian Falbesoner)
Pull request description:
Inspired by some manual testing I did for #28553, this PR checks that RPCs which explicitly query the UTXO set database (i.e. `gettxoutsetinfo`, `scantxoutset` and `gettxout`) operate on the snapshot chainstate as expected.
ACKs for top commit:
fjahr:
utACK 917e70a620
achow101:
ACK 917e70a620
tdb3:
ACK 917e70a620
Tree-SHA512: 40ecd1c5dd879234df1667fa5444a1fbbee9b7c456f597dc982d1a2bce46fe9107711b005ab829e570ef919a4914792f72f342d71d92bad2ae9434b5e68d5bd3
Handle the Block height out of range error gracefully by checking if
the node has synchronized to or beyond the required block height,
otherwise without this validation the node would keep the network
disabled if the user selected that option.
Provide a user-friendly message if the block height is out of range
and exit the script cleanly.
fa899fb7aa fuzz: Speed up utxo_snapshot fuzz target (MarcoFalke)
fa386642b4 fuzz: Speed up utxo_snapshot by lazy re-init (MarcoFalke)
fa645c7a86 fuzz: Remove unused DataStream object (MarcoFalke)
fae8c73d9e test: Disallow fee_estimator construction in ChainTestingSetup (MarcoFalke)
Pull request description:
Two commits to speed up unit and fuzz tests.
Can be tested by running the fuzz target and looking at the time it took, or by looking at the flamegraph. For example:
```
FUZZ=utxo_snapshot perf record -g --call-graph dwarf ./src/test/fuzz/fuzz -runs=100
hotspot ./perf.data
ACKs for top commit:
TheCharlatan:
Re-ACK fa899fb7aa
marcofleon:
Re ACK fa899fb7aa
brunoerg:
ACK fa899fb7aa
Tree-SHA512: d3a771bb12d7ef491eee61ca47325dd1cea5c20b6ad42554babf13ec98d03bef8e7786159d077e59cc7ab8112495037b0f6e55edae65b871c7cf1708687cf717
The flags SECP256K1_CONTEXT_{SIGN,VERIFY} have been deprecated since
libsecp256k1 version 0.2 (released in December 2022), with the
recommendation to use SECP256K1_CONTEXT_NONE instead.
This currently has no effect due to fPowNoRetargeting,
except for the getnetworkhashps when called with -1.
It will when the next commit enforces the timewarp attack mitigation on regtest.
16e95bda86 Move maximum timewarp attack threshold back to 600s from 7200s (Matt Corallo)
Pull request description:
In 6bfa26048d the testnet4 timewarp attack fix block time variation was increased from the Great Consensus Cleanup value of 600s to 7200s on the thesis that this allows miners to always create blocks with the current time. Sadly, doing so does allow for some nonzero inflation, even if not a huge amount.
While it could be that some hardware ignores the timestamp provided to it over Stratum and forces the block header timestamp to the current time, I'm not aware of any such hardware, and it would also likely suffer from random invalid blocks due to relying on NTP anyway, making its existence highly unlikely.
This leaves the only concern being pools, but most of those rely on work generated by Bitcoin Core (in one way or another, though when spy mining possibly not), and it seems likely that they will also not suffer any lost work. While its possible that a pool does generate invalid work due to spy mining or otherwise custom logic, it seems unlikely that a substantial portion of hashrate would do so, making the difference somewhat academic (any pool that screws this up will only do so once and the network would come out just fine).
Further, while we may end up deciding these assumptions were invalid and we should instead use 7200s, it seems prudent to try with the value we "want" on testnet4, giving us the ability to learn if the compatibility concerns are an issue before we go to mainnet.
ACKs for top commit:
fjahr:
tACK 16e95bda86
achow101:
ACK 16e95bda86
murchandamus:
crACK 16e95bda86
Tree-SHA512: ae46d03b728b6e23cb6ace64c9813bc01c01e38dd7f159cf0fab53b331ef84b3b811edab225453ccdfedb53b242f55b0efd69829782657490fe393d24dacbeb2
6ed424f2db wallet: fix, detect blank legacy wallets in IsLegacy (furszy)
Pull request description:
Blank legacy wallets do not have active SPKM. They can only be
detected by checking the descriptors' flag or the db format.
This enables the migration of blank legacy wallets in the GUI.
To test this:
1) Create a blank legacy wallet.
2) Try to migrate it using the GUI's toolbar "Migrate Wallet" button.
-> In master: The button will be disabled because `CWallet::IsLegacy()` returns false for blank legacy wallet.
-> In this PR: the button will be enabled, allowing the migration of legacy wallets.
ACKs for top commit:
achow101:
ACK 6ed424f2db
tdb3:
ACK 6ed424f2db
glozow:
ACK 6ed424f2db
Tree-SHA512: c06c4c4c2e546ccb033287b9aa3aee4ca36b47aeb2fac6fbed5de774b65caef9c818fc8dfdaac6ce78839b2d5d642a5632a5b44c5e889ea169ced80ed50501a7
The seeders now produce onion and i2p seeds, so there is no need to keep these
in the manual list.
Although should also be produced, there are not enough
good ones detected by the seeder, so we keep the manual seeds for them.
faa1b9b0e6 test: add functional test for XORed block/undo files (`-blocksxor`) (Sebastian Falbesoner)
6b3676be3e test: refactor: move `read_xor_key`/`util_xor` helpers to util module (Sebastian Falbesoner)
Pull request description:
This PR adds a dedicated functional test for XORed block data/undo file support (bitcoind option `-blocksxor`, see PR #28052). In order to verify that the XOR pattern has been applied, the {blk,rev}*.dat files are rewritten un-XORed manually by the test while the node is shut down; the node is then started again with `-blocksxor=0`, and both the data and undo files are verified via the `verifychain` RPC (with checklevel=2). Note that starting bitcoind with `-blocksxor=0` fails if a xor key is present already, which is also tested explicitly.
Fixes #30599.
ACKs for top commit:
glozow:
ACK faa1b9b0e6
maflcko:
ACK faa1b9b0e6
ismaelsadeeq:
Tested ACK faa1b9b0e6
Tree-SHA512: e1df106f6b4e3ba67eca108e36d762f1b991673b881934b84cd36946496a09ce9c329c1363c36aa29409137ae4881e2d177e651359686511632ddf2870f7ca8e
