This aims to complete our test framework BDB parser to reflect
our read-only BDB parser in the wallet codebase. This could be
useful both for making review of #26606 easier and to also possibly
improve our functional tests for the BDB parser by comparing with
an alternative implementation.
Github-Pull: #30125
Rebased-From: 01ddd9f646
Let's say an attacker wants to use/exhaust the network's bandwidth, and
has the choice between renting resources from a commercial provider and
getting the network to "spam" itself it by sending unconfirmed
transactions. We'd like the latter to be more expensive than the former.
The bandwidth for relaying a transaction across the network is roughly
its serialized size (plus relay overhead) x number of nodes. A 1000vB
transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB
If the going rate for commercial services is 10c/GB, that's like 1-4c per kvB
of transaction data, so a 1000vB transaction should pay at least $0.04.
At a price of 120k USD/BTC, 100sat is about $0.12. This price allows us
to tolerate a large decrease in the conversion rate or increase in the
number of nodes.
Github-Pull: #33106
Rebased-From: 6da5de58ca
Back when we implemented coin age priority as a miner policy, miners
mempools might admit transactions paying very low fees, but then want to
set a higher fee for block inclusion. However, since coin age priority
was removed in v0.15, the block assembly policy is solely based on fees,
so we do not need to apply minimum feerate rules in multiple places. In
fact, the block assembly policy ignoring transactions that are added to
the mempool is likely undesirable as we waste resources accepting and
storing this transaction.
Instead, rely on mempool policy to enforce a minimum entry feerate to
the mempool (minrelaytxfee). Set the minimum block feerate to the
minimum non-zero amount (1sat/kvB) so it collects everything it finds in
mempool into the block.
Github-Pull: #33106
Rebased-From: 5f2df0ef78
log.exception is more verbose and useful to debug timeouts.
Also, log stderr for CalledProcessError to make debugging easier.
Github-Pull: #33001
Rebased-From: faa3e68411
This adds a missing catch for BaseException (e.g. SystemExit), which
would otherwise be silently ignored.
Also, remove the redundant other catches, which are just calling
log.exception with a redundant log message.
Github-Pull: #33001
Rebased-From: fa30b34026
After port collisions are no longer tolerated but lead to
a startup failure in v28.0, local setups of multiple nodes,
each with a different -port value would not be possible anymore
due to collision of the onion default port - even if the nodes
were using tor or not interested in receiving onion inbound connections.
Fix this by deriving the onion listening port to be -port + 1.
(idea by vasild / laanwj)
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Github-Pull: bitcoin/bitcoin#31223
Rebased-From: 0e2b12b92a
Fixes a race between node 1 catching up with the chain and mining a
new block in the sanity_check_rbf_estimates subtest.
Github-Pull: #31016
Rebased-From: a1576edab3
Exercising and verifying the following points:
1. An IBD node can sync headers from an AssumeUTXO node at
any time.
2. IBD nodes do not request historical blocks from AssumeUTXO
nodes while they are syncing the background-chain.
3. The assumeUTXO node dynamically adjusts the network services
it offers according to its state.
4. IBD nodes can fully sync from AssumeUTXO nodes after they
finish the background-chain sync.
Github-Pull: bitcoin/bitcoin#30807
Rebased-From: 992f83bb6f
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local
address through the network before completing the background chain sync.
This, combined with the advertising of full-node service (NODE_NETWORK), can
result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing)
and requesting an historical block the node does not have. This behavior leads to
an abrupt disconnection due to perceived unresponsiveness (lack of response)
from the AssumeUTXO node.
This lack of response occurs because nodes ignore getdata requests when they do
not have the block data available (further discussion can be found in PR 30385).
Fix this by refraining from signaling full-node service support while the
background chain is being synced. During this period, the node will only
signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK')
support will be re-enabled once the background chain sync is completed.
Github-Pull: bitcoin/bitcoin#30807
Rebased-From: 6d5812e5c8
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
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
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
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.
77ff0ec1f1 contrib: support reading XORed blocks in linearize-data.py script (Sebastian Falbesoner)
Pull request description:
This PR is a small follow-up for #28052, adding support for the block linearization script to handle XORed blocksdir *.dat files. Note that if no xor.dat file exists, the XOR pattern is set to all-zeros, in order to still support blockdirs that have been created with versions earlier than 28.x.
Partly fixes issue #30599.
ACKs for top commit:
achow101:
ACK 77ff0ec1f1
tdb3:
ACK 77ff0ec1f1
hodlinator:
ACK 77ff0ec1f1
Tree-SHA512: 011eb02e2411de373cbbf4b26db4640fc693a20be8c2430529fba6e36a3a3abfdfdc3b005d330f9ec2846bfad9bfbf34231c574ba99289ef37dd51a68e6e7f3d
6b2dcba076 wallet: List sqlite wallets with empty string name (Ava Chow)
3ddbdd1815 wallet: Ignore .bak files when listing wallet files (Ava Chow)
Pull request description:
When the default wallet is migrated, we do not rename the wallet so we end up having a descriptor wallet with the empty string as its name and the wallet.dat file in the root of the walletdir. This is supposed to be an unsupported configuration and there is no other way to achieve this (other than file copying), but the wallet loading code does not disallow loading such wallets. However `listwalletdir` does not currently list the default wallet if it is sqlite. This is confusing to users, so change `listwalletdir` to include these wallets.
Additionally, the migration of the default wallet, and of any plain wallet files in the walletdir, produces a backup file in the walletdir itself. Since these backups are a BDB file, `listwalletdir` will detect them as being another wallet that we could open, but this is erroneous and could lead to confusion and potentially funds loss if both the backup and the migrated wallet are in use simultaneously. To reduce the likelihood of this issue, don't list these wallets in `listwalletdir`.
***
Possibly we could have more stringent checks on loading to resolve these issues, but I'm concerned that that will just confuse users and gratuitously break things that already worked.
Since the original intent was to disallow default wallets for sqlite/descriptors, a possible alternative would be to prevent people from loading such wallets and change migration to rename those wallets. However, given that this behavior with migrating default wallets has existed since default wallet migration was fixed, I think that making such a change would be confusing and break things for no good reason. Although perhaps we should still do the renaming.
For the backups, we could also change loading to refuse to load any wallet named with `.bak` (or `.legacy.bak`) as such wallets can still be loaded by giving the path to them directly, which some users may do to "restore" the backup. However restricting what can be loaded based on filename seems a little heavyhanded. It wouldn't be funds loss though since the correct way to restore the backup is with `restorewallet`.
ACKs for top commit:
fjahr:
Code review ACK 6b2dcba076
furszy:
Code ACK 6b2dcba076
glozow:
ACK 6b2dcba076
Tree-SHA512: 0b033f6ed55830f8a054afea3fb2cf1fa82a94040053ebfaf123bda36c99f45d3f01a2aec4ed02fed9c61bb3d320b047ed892d7f6644b5a356a7bc5974b10cff
