This was in fact untested until now. This can be checked with the following diff.
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0c4a89c44cb..f8b9adf910a 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1822,7 +1822,7 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
}
case BlockValidationResult::BLOCK_INVALID_HEADER:
case BlockValidationResult::BLOCK_INVALID_PREV:
- if (peer) Misbehaving(*peer, message);
+ if (!via_compact_block && peer) Misbehaving(*peer, message);
return;
// Conflicting (but not necessarily invalid) data or different policy:
case BlockValidationResult::BLOCK_MISSING_PREV:
```
b6d4688f77 [doc] reword comments in test_mid_package_replacement (glozow)
f3a613aa5b [cleanup] delete brittle test_mid_package_eviction (glozow)
c3cd7fcb2c [doc] remove references to now-nonexistent Finalize() function (glozow)
d8140f5f05 don't make a copy of m_non_base_coins (glozow)
98ba2b1db2 [doc] MemPoolAccept coins views (glozow)
ba02c30b8a [doc] always CleanupTemporaryCoins after a mempool trim (glozow)
Pull request description:
Deletes `test_mid_package_eviction` that is brittle and already covered in other places. It was introduced in #28251 addressing 2 issues: (1) calling `LimitMempoolSize()` in the middle of package validation and (2) not updating coins view cache when the mempool contents change, leading to "disappearing coins."
(1) If you let `AcceptSingleTransaction` call `LimitMempoolSize` in the middle of package validation, you should get a failure in `test_mid_package_eviction_success` (the package is rejected):
```
diff --git a/src/validation.cpp b/src/validation.cpp
index f2f6098e214..4bd6f059849 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1485,7 +1485,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
FinalizeSubpackage(args);
// Limit the mempool, if appropriate.
- if (!args.m_package_submission && !args.m_bypass_limits) {
+ if (!args.m_bypass_limits) {
LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
// If mempool contents change, then the m_view cache is dirty. Given this isn't a package
// submission, we won't be using the cache anymore, but clear it anyway for clarity.
```
Mempool modifications have a pretty narrow interface since #31122 and `TrimToSize()` cannot be called while there is an outstanding mempool changeset. So I think there is a low likelihood of accidentally reintroducing this problem and not immediately hitting e.g. a fuzzer crash on this line b53fab1467/src/txmempool.cpp (L1143)
(2) If you remove the `CleanupTemporaryCoins()` call from `ClearSubPackageState()` you should get a failure from `test_mid_package_replacement`:
```
diff --git a/src/validation.cpp b/src/validation.cpp
index f2f6098e214..01b904b69ef 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -779,7 +779,7 @@ private:
m_subpackage = SubPackageState{};
// And clean coins while at it
- CleanupTemporaryCoins();
+ // CleanupTemporaryCoins();
}
};
```
I also added/cleaned up the documentation about coins views to hopefully make it extremely clear when people should `CleanupTemporaryCoins`.
ACKs for top commit:
instagibbs:
reACK b6d4688f77
sdaftuar:
utACK b6d4688f77
marcofleon:
ACK b6d4688f77
Tree-SHA512: 79c68e263013b1153520f5453e6b579b8fe7e1d6a9952b1ac2c3c3c017034e6d21d7000a140bba4cc9d2ce50ea3a84cc6f91fd5febc52d7b3fa4f797955d987d
Submit the block with an invalid transaction Script again, leading to
CACHED_INVALID being returned by AcceptBlockHeader(). Ensure that also this
code path does not lead to a disconnection.
This was previously untested, as can be checked with the following diff:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0c4a89c44cb..e8e0c805367 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1814,10 +1814,10 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
{
// Discourage outbound (but not inbound) peers if on an invalid chain.
// Exempt HB compact block peers. Manual connections are always protected from discouragement.
- if (peer && !via_compact_block && !peer->m_is_inbound) {
+ //if (peer && !via_compact_block && !peer->m_is_inbound) {
if (peer) Misbehaving(*peer, message);
return;
- }
+ //}
break;
}
case BlockValidationResult::BLOCK_INVALID_HEADER:
```
The current test to exercise a block with an invalid transaction actually
creates a block with an invalid coinbase witness, which is checked early and
for which MaybePunishNodeForBlock() is not called.
Add a test case with an invalid regular transaction, which will lead
CheckInputScripts to return a CONSENSUS error and MaybePunishNodeForBlock() to
be called, appropriately not disconnecting upon an invalid compact block. This
was until now untested as can be checked with the following diff:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0c4a89c44cb..d243fb88d4b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1805,10 +1805,10 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
// The node is providing invalid data:
case BlockValidationResult::BLOCK_CONSENSUS:
case BlockValidationResult::BLOCK_MUTATED:
- if (!via_compact_block) {
+ //if (!via_compact_block) {
if (peer) Misbehaving(*peer, message);
return;
- }
+ //}
break;
case BlockValidationResult::BLOCK_CACHED_INVALID:
{
```
Finally, note this failure is cached (unlike the malleated witness failure),
which will be used in the following commits.
The error being checked here is BLOCK_MUTATED, as returned by IsBlockMutated()
in FillBlock(). Dropping the fourth transaction from the block is unnecessary
and would make testing of other block validation failures in following commits
more verbose.
1c10b7351e RPC: Return permitbaremultisig and maxdatacarriersize in getmempoolinfo (Kristaps Kaupe)
Pull request description:
Other node relay settings like `fullrbf` and `minrelaytxfee` are already returned, makes sense to add these two too.
ACKs for top commit:
ajtowns:
ACK 1c10b7351e
maflcko:
lgtm ACK 1c10b7351e
theStack:
ACK 1c10b7351e
Tree-SHA512: 1750d7d12de511f0ac34922ea9c58c4b9b55c3aaf22109abfd7dbe01ad1eb7b48fb4a6b074a0baf0e55ee2270fcc969b6830e499ff33adbcd0b9c761fb25e563
c5c1960f93 doc: Add release notes for changes in RPCs (pablomartin4btc)
90fd5acbe5 rpc, test: Fix error message in getdescriptoractivity (pablomartin4btc)
39fef1d203 test: Add missing logging info for each test (pablomartin4btc)
53ac704efd rpc, test: Fix error message in unloadwallet (pablomartin4btc)
1fc3a8e8e7 rpc, test: Add EnsureUniqueWalletName tests (pablomartin4btc)
b635bc0896 rpc, util: Add EnsureUniqueWalletName (pablomartin4btc)
Pull request description:
Currently, `unloadwallet` RPC call fails with a JSON parsing error when no `wallet_name` argument is provided. This behavior is misleading because the error originates from a low-level JSON type mismatch, rather than clearly indicating that the wallet name or RPC endpoint (`-rpcwallet=...`) is missing. Also, found out that the [issue](https://github.com/bitcoin/bitcoin/pull/13111#issuecomment-398831543) was noticed during its implementation but never addressed.
In addition, I've verified all RPC commands calls finding that `getdescriptoractivity` had the same problem, but related to the array input types (blockhashes & descriptors), so I've corrected that RPC as well. For consistency I've added the missing logging info for each test case in `test/functional/rpc_getdescriptoractivity.py` in preparation for the new test.
**_-Before_**
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc unloadwallet
error code: -3
error message:
JSON value of type number is not of expected type string
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity
error code: -3
error message:
JSON value of type null is not of expected type array
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity '[]'
error code: -3
error message:
JSON value of type null is not of expected type array
```
**_-After_**
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc unloadwallet
error code: -8
error message:
Either the RPC endpoint wallet or the wallet name parameter must be provided
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity
error code: -1
error message:
getdescriptoractivity ["blockhash",...] [scanobjects,...] ( include_mempool )
Get spend and receive activity associated with a set of descriptors for a set of blocks. This command pairs well with the `relevant_blocks` output of `scanblocks()`.
This call may take several minutes. If you encounter timeouts, try specifying no RPC timeout (bitcoin-cli -rpcclienttimeout=0)
Arguments:
1. blockhashes (json array, required) The list of blockhashes to examine for activity. Order doesn't matter. Must be along main chain or an error is thrown.
[
"blockhash", (string) A valid blockhash
...
]
2. scanobjects (json array, required) Array of scan objects. Every scan object is either a string descriptor or an object:
[
"descriptor", (string) An output descriptor
{ (json object) An object with output descriptor and metadata
"desc": "str", (string, required) An output descriptor
"range": n or [n,n], (numeric or array, optional, default=1000) The range of HD chain indexes to explore (either end or [begin,end])
},
...
]
3. include_mempool (boolean, optional, default=true) Whether to include unconfirmed activity
...
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity '[]'
error code: -1
error message:
getdescriptoractivity ["blockhash",...] [scanobjects,...] ( include_mempool )
...
```
ACKs for top commit:
achow101:
ACK c5c1960f93
stickies-v:
re-ACK c5c1960f93
furszy:
ACK c5c1960f93
Tree-SHA512: e831ff1acbfd15d2ce3a69bb408cce94664c0b63b2aa2f4627a05c6c052241ae3b5cc238219ef1b30afb489a4a3f4c3030e2168b0c8f08b4d20805d050d810f5
It is redundant with -logsourcelocations and the log messages are
clearer without it.
Also, remove a double-space.
Also, add braces around `if` touched in the next commit.
This tiny behavior change requires a test fixup.
This rule was originally introduced along with a very early proposal for
package relay as a way to verify that the "correct"
child-with-unconfirmed-parents package was provided for a transaction,
where correctness was defined as all of the transactions unconfirmed
parents. However, we are not planning to introduce a protocol where
peers would be asked to send these packages.
This rule has downsides: if a transaction has multiple parents but only
1 that requires package CPFP to be accepted, the current rule prevents
us from accepting that package. Even if the other parents are already in
mempool, the p2p logic will only submit the 1p1c package, which fails
this check. See the test in p2p_1p1c_network.py
060695c22a test: Failed load after migrate should restore backup (MarcoFalke)
8a4cfddf23 wallet: Set migrated wallet name only on success (Ava Chow)
Pull request description:
After a wallet is migrated and we are trying to load it, if it could not be loaded, don't try to set the wallet name. Otherwise we have a segfault.
This can be tested by migrated a legacy wallet from another network (e.g. trying to migrate a testnet wallet on mainnet). The fixed behavior is return an error and restore the backup.
ACKs for top commit:
davidgumberg:
ACK 060695c22a
furszy:
ACK 060695c22a
rkrux:
ACK 060695c22a
w0xlt:
reACK 060695c22a
pablomartin4btc:
ACK 060695c22a
Tree-SHA512: f4289e0b3dedef0a3d734c18604f2fd0df0dc65e9641bc342cfa45088d2540a3f6631bbea8bdd394b2631fa83b38e8ac37c83cfc4b53b19dcbd0b820a9beb6e4
We just need enough transactions to push us above the orphanage limits
and trigger trimming. Reducing the number of transactions cuts the
runtime of this test significantly.
faa3e68411 test: Log KeyboardInterrupt as exception (MarcoFalke)
fa30b34026 test: Do not pass tests on unhandled exceptions (MarcoFalke)
Pull request description:
Currently the functional tests are problematic, because they pass, even if they encounter an unhanded exception.
Fix this by handling all exceptions: Catch `BaseException` as fallback and mark it as failure.
Can be tested via:
```diff
diff --git a/test/functional/wallet_disable.py b/test/functional/wallet_disable.py
index da6e5d408f..ecc41fb041 100755
--- a/test/functional/wallet_disable.py
+++ b/test/functional/wallet_disable.py
@@ -19,6 +19,7 @@ class DisableWalletTest (BitcoinTestFramework):
self.wallet_names = []
def run_test (self):
+ import sys;sys.exit("fatal error")
# Make sure wallet is really disabled
assert_raises_rpc_error(-32601, 'Method not found', self.nodes[0].getwalletinfo)
x = self.nodes[0].validateaddress('3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy')
```
Previously, the test would pass. With this patch, it would fail.
ACKs for top commit:
enirox001:
Looks good to me—ACK faa3e68
stickies-v:
re-ACK faa3e68411
pablomartin4btc:
tACK faa3e68411
Tree-SHA512: 11ecd5201982e2c776e48d98834b17c15a415306a95524bc702daeba20a316aac797748e9592be8db575597804f149ee7ef104416037cc9e5891758625810e2d
Mark blockhashes and scanobjects arguments as required, so the user receives
a clear help message when either is missing.
Added a new functional test for this use case.
Co-authored-by: stickies-v <stickies-v@users.noreply.github.com>
Add self.log.info(...) calls at the beginning of each test
in GetBlocksActivityTest.
This improves readability and provides debugging information
by logging the purpose of each test upon its correct
execution.
This is in preparation for the next commit, which adds a new test
with log info, and it would look inconsistent without this commit.
The unloadwallet RPC previously failed with a low-level JSON parsing error
when called without any arguments (wallet_name).
Although this issue was first identified during review of the original unloadwallet
implementation in #13111, it was never addressed.
Raise RPC_INVALID_PARAMETER instead describing that either the RPC endpoint or wallet
name must be provided.
Adding a new functional test for this use case.
Refactor migratewallet to use the same logic as the wallet_name argument handling
is identical.
Co-authored-by: maflcko <maflcko@users.noreply.github.com>
96da68a38f qa: functional test a transaction running into the legacy sigop limit (Antoine Poinsot)
367147954d qa: unit test standardness of inputs packed with legacy sigops (Antoine Poinsot)
5863315e33 policy: make pathological transactions packed with legacy sigops non-standard. (Antoine Poinsot)
Pull request description:
The Consensus Cleanup soft fork proposal includes a limit on the number of legacy signature
operations potentially executed when validating a transaction. If this change is to be implemented
here and activated by Bitcoin users in the future, we should make transactions that are not valid
according to the new rules non-standard first because it would otherwise be a trivial DoS to
potentially unupgraded miners after the soft fork activates.
ML post: https://gnusha.org/pi/bitcoindev/49dyqqkf5NqGlGdinp6SELIoxzE_ONh3UIj6-EB8S804Id5yROq-b1uGK8DUru66eIlWuhb5R3nhRRutwuYjemiuOOBS2FQ4KWDnEh0wLuA=@protonmail.com/T/#u
ACKs for top commit:
instagibbs:
reACK 96da68a38f
maflcko:
review ACK 96da68a38f🚋
achow101:
ACK 96da68a38f
glozow:
light code review ACK 96da68a38f, looks correct to me
Tree-SHA512: 106ffe62e48952affa31c5894a404a17a3b4ea8971815828166fba89069f757366129f7807205e8c6558beb75c6f67d8f9a41000be2f8cf95be3b1a02d87bfe9
50024620b9 [bench] worst case LimitOrphans and EraseForBlock (glozow)
45c7a4b56d [functional test] orphan resolution works in the presence of DoSy peers (glozow)
835f5c77cd [prep/test] restart instead of bumpmocktime between p2p_orphan_handling subtests (glozow)
b113877545 [fuzz] Add simulation fuzz test for TxOrphanage (Pieter Wuille)
03aaaedc6d [prep] Return the made-reconsiderable announcements in AddChildrenToWorkSet (Pieter Wuille)
ea29c4371e [p2p] bump DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE to 3,000 (glozow)
24afee8d8f [fuzz] TxOrphanage protects peers that don't go over limit (glozow)
a2878cfb4a [unit test] strengthen GetChildrenFromSamePeer tests: results are in recency order (glozow)
7ce3b7ee57 [unit test] basic TxOrphanage eviction and protection (glozow)
4d23d1d7e7 [cleanup] remove unused rng param from LimitOrphans (glozow)
067365d2a8 [p2p] overhaul TxOrphanage with smarter limits (glozow)
1a41e7962d [refactor] create aliases for TxOrphanage Count and Usage (glozow)
b50bd72c42 [prep] change return type of EraseTx to bool (glozow)
3da6d7f8f6 [prep/refactor] make TxOrphanage a virtual class implemented by TxOrphanageImpl (glozow)
77ebe8f280 [prep/test] have TxOrphanage remember its own limits in LimitOrphans (glozow)
d0af4239b7 [prep/refactor] move DEFAULT_MAX_ORPHAN_TRANSACTIONS to txorphanage.h (glozow)
51365225b8 [prep/config] remove -maxorphantx (glozow)
8dd24c29ae [prep/test] modify test to not access TxOrphanage internals (glozow)
44f5327824 [fuzz] add SeedRandomStateForTest(SeedRand::ZEROS) to txorphan (glozow)
15a4ec9069 [prep/rpc] remove entry and expiry time from getorphantxs (glozow)
08e58fa911 [prep/refactor] move txorphanage to node namespace and directory (glozow)
bb91d23fa9 [txorphanage] change type of usage to int64_t (glozow)
Pull request description:
This PR is part of the orphan resolution project, see #27463.
This design came from collaboration with sipa - thanks.
We want to limit the CPU work and memory used by `TxOrphanage` to avoid denial of service attacks. On master, this is achieved by limiting the number of transactions in this data structure to 100, and the weight of each transaction to 400KWu (the largest standard tx) [0]. We always allow new orphans, but if the addition causes us to exceed 100, we evict one randomly. This is dead simple, but has problems:
- It makes the orphanage trivially churnable: any one peer can render it useless by spamming us with lots of orphans. It's possible this is happening: "Looking at data from node alice on 2024-09-14 shows that we’re sometimes removing more than 100k orphans per minute. This feels like someone flooding us with orphans." [1]
- Effectively, opportunistic 1p1c is useless in the presence of adversaries: it is *opportunistic* and pairs a low feerate tx with a child that happens to be in the orphanage. So if nothing is able to stay in orphanages, we can't expect 1p1cs to propagate.
- This number is also often insufficient for the volume of orphans we handle: historical data show that overflows are pretty common, and there are times where "it seems like [the node] forgot about the orphans and re-requested them multiple times." [1]
Just jacking up the `-maxorphantxs` number is not a good enough solution, because it doesn't solve the churnability problem, and the effective resource bounds scale poorly.
This PR introduces numbers for {global, per-peer} {memory usage, announcements + number of inputs}, representing resource limits:
- The (constant) **global latency score limit** is the number of unique (wtxid, peer) pairs in the orphanage + the number of inputs spent by those (deduplicated) transactions floor-divided by 10 [2]. This represents a cap on CPU or latency for any given operation, and does not change with the number of peers we have. Evictions must happen whenever this limit is reached. The primary goal of this limit is to ensure we do not spend more than a few ms on any call to `LimitOrphans` or `EraseForBlock`.
- The (variable) **per-peer latency score limit** is the global latency score limit divided by the number of peers. Peers are allowed to exceed this limit provided the global announcement limit has not been reached. The per-peer announcement limit decreases with more peers.
- The (constant) **per-peer memory usage reservation** is the amount of orphan weight [3] reserved per peer [4]. Reservation means that peers are effectively guaranteed this amount of space. Peers are allowed to exceed this limit provided the global usage limit is not reached. The primary goal of this limit is to ensure we don't oom.
- The (variable) **global memory usage limit** is the number of peers multiplied by the per-peer reservation [5]. As such, the global memory usage limit scales up with the number of peers we have. Evictions must happen whenever this limit is reached.
- We introduce a "Peer DoS Score" which is the maximum between its "CPU Score" and "Memory Score." The CPU score is the ratio between the number of orphans announced by this peer / peer announcement limit. The memory score is the total usage of all orphans announced by this peer / peer usage reservation.
Eviction changes in a few ways:
- It is triggered if either limit is exceeded.
- On each iteration of the loop, instead of selecting a random orphan, we select a peer and delete 1 of its announcements. Specifically, we select the peer with the highest DoS score, which is the maximum between its CPU DoS score (based on announcements) and Memory DoS score (based on tx weight). After the peer has been selected, we evict the oldest orphan (non-reconsiderable sorted before reconsiderable).
- Instead of evicting orphans, we evict announcements. An orphan is still in the orphanage as long as there is 1 peer announcer. Of course, over the course of several iteration loops, we may erase all announcers, thus erasing the orphan itself. The purpose of this change is to prevent a peer from being able to trigger eviction of another peer's orphans.
This PR also:
- Reimplements `TxOrphanage` as single multi-index container.
- Effectively bounds the number of transactions that can be in a peer's work set by ensuring it is a subset of the peer's announcements.
- Removes the `-maxorphantxs` config option, as the orphanage no longer limits by unique orphans.
This means we can receive 1p1c packages in the presence of spammy peers. It also makes the orphanage more useful and increases our download capacity without drastically increasing orphanage resource usage.
[0]: This means the effective memory limit in orphan weight is 100 * 400KWu = 40MWu
[1]: https://delvingbitcoin.org/t/stats-on-orphanage-overflows/1421
[2]: Limit is 3000, which is equivalent to one max size ancestor package (24 transactions can be missing inputs) for each peer (default max connections is 125).
[3]: Orphan weight is used in place of actual memory usage because something like "one maximally sized standard tx" is easier to reason about than "considering the bytes allocated for vin and vout vectors, it needs to be within N bytes..." etc. We can also consider a different formula to encapsulate more the memory overhead but still have an interface that is easy to reason about.
[4]: The limit is 404KWu, which is the maximum size of an ancestor package.
[5]: With 125 peers, this is 50.5MWu, which is a small increase from the existing limit of 40MWu. While the actual memory usage limit is higher (this number does not include the other memory used by `TxOrphanage` to store the outpoints map, etc.), this is within the same ballpark as the old limit.
ACKs for top commit:
marcofleon:
ReACK 50024620b9
achow101:
light ACK 50024620b9
instagibbs:
ACK 50024620b9
theStack:
Code-review ACK 50024620b9
Tree-SHA512: 270c11a2d116a1bf222358a1b4e25ffd1f01e24da958284fa8c4678bee5547f9e0554e87da7b7d5d5d172ca11da147f54a69b3436cc8f382debb6a45a90647fd
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.
It's useful to have an end-to-end test in addition to the unit test to sanity check the RPC error as
well as making sure the transaction is otherwise fully standard.
Note that we unfortunately can't use a scripted diff here, as the
`sha256` symbol is also used for other instances (e.g. as function
in hashlib, or in the `UTXO` class in p2p_segwit.py).
Since the previous commit, CBlockHeader/CBlock object calls to the
methods `.rehash()` and `.calc_sha256()` are effectively no-ops
if the returned value is not used, so we can just remove them.
Rather than block hashes (represented by the fields `.sha256` and
`.hash`) being stateful, simply compute them on-the-fly. This ensures
that the correct values are always returned and takes the burden of
rehashing from test writers, making the code shorter overall. In a
first step, the fields are kept at the same name with @property
functions as drop-in replacements, for a minimal diff. In later commits,
the names are changed to be more descriptive and indicating the return
type of the block hash.
This is a preparatory commit for removing the header hash
caching in the CBlockHeader class. In order to not lose the
old block hash, necessary for updating the internal state of
the test (represented by `self.block_heights` and `self.blocks`),
we should only modify it within the `update_block` method.
Note that we can't call `.serialize()` directly in
the `.calc_sha256()` method, as this could wrongly lead
to the serialization of the derived class (CBlock) if
called from an instance there.
This check ensures that when migrating a legacy wallet with a direct
filename, the backup file is named as expected.
Co-authored-by: Ava Chow <github@achow101.com>
This test was introduced in #28251 to ensure that the mempool is not
trimmed in the middle of a package evaluation and the m_view cache
is updated when evictions and replacements happen so coins are no longer
visible in subsequent package transactions. These two things have
coverage in other tests as well, and are pretty unlikely to happen.
This test is also brittle: it requires evaluation of the parents in a
particular order, and creates a transaction that itself is not
enough to trigger eviction but will be pushed out immediately by the
package spending from it. While the current magic number 2000 works, we
do not have a way to query remaining space in the mempool if mempool
data structures change, and it can differ across platforms.
4bb4c86599 test: document HOST for get_previous_releases.py (Sjors Provoost)
609203d507 test: stop signing previous releases >= v28.2 (Sjors Provoost)
c6dc2c29f8 test: replace v28.0 with notarized v28.2 (Sjors Provoost)
5bd73d96a3 test: fix macOS detection (Sjors Provoost)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/31407 macOS guix builds are signed and notarized. This was included in v29 and backported to 28.x.
This PR bumps the v28.0 previous release binary to v28.2 and adjusts the test that uses it. Additionally it no longer manually code signs binaries >= v28.2.
While testing on an M4 mac and redownloading all the binaries, I noticed that `platform == "arm64-apple-darwin"` doesn't actually work. This initially used `args.platform` in #26694, but that was changed to just `platform` in #32219.
So the first commit switches this to use `args.host`. I manually tested on Intel macOS 13.7.6 that code-signing still isn't needed there (when downloading using a script).
Also documented that you can set `HOST`.
ACKs for top commit:
m3dwards:
ACK 4bb4c86599
maflcko:
review ACK 4bb4c86599🚏
Tree-SHA512: b4803d39a21cb622fd2388a0528b76d2b502956e2505385d3da201143b0afcf6f9d71c8c28937f27b70d2588fb6da677da058bdcd67b90fb53617acc3a727818
61e800e75c test: headers sync timeout (stringintech)
Pull request description:
When reviewing PR #32051 and considering which functional tests might need to be adapted/extended accordingly, I noticed there appears to be limited functional test coverage for header sync timeouts and thought it might be helpful to add one.
This test attempts to cover two scenarios:
1. **Normal peer timeout behavior:** When a peer fails to respond to initial getheaders requests within the timeout period, it should be disconnected and the node should attempt to sync headers from the next available peer.
2. **Noban peer behavior:** When a peer with noban privileges times out, it should remain connected while the node still attempts to sync headers from other peers.
ACKs for top commit:
maflcko:
re-ACK 61e800e75c 🗝
stratospher:
reACK 61e800e7.
Tree-SHA512: b8a867e7986b6f0aa00d81a84b205f2bf8fb2e6047a2e37272e0244229d1f43020e9031467827dabbfe7849a91429f2685e00a25356e2ed477fa1a035fa0b1fd
