Extract the part of `p2p_private_broadcast.py` that fills a given node's
addrman and put it into `test_framework/test_framework.py`.
Github-Pull: #35410
Rebased-From: ab35a028ed
Extract the part of `p2p_private_broadcast.py` that starts
listening on a `P2PConnection` object (or its children classes)
and put it into `test_framework/p2p.py`.
Github-Pull: #35410
Rebased-From: 2333be9cbc
Extract the part of `p2p_private_broadcast.py` that configures and
starts the SOCKS5 server into a reusable function and put it into
`test_framework/socks5.py`.
Use bind port 0 to let the OS pick an available port instead of
hackishly assuming that `p2p_port(N)` is available where N is more
than the number of the nodes the test uses.
Github-Pull: #35410
Rebased-From: 2ffa81fac4
Private broadcast connections use either Tor or I2P, which require a
proxy intrinsically or IPv4 or IPv6 which must use a proxy in the
context of private broadcast to avoid leaking the originator's IP
address.
Add a safety check to guard against future mistakes.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Github-Pull: #35410
Rebased-From: d01b461f71
`OpenNetworkConnection()` supports overriding the proxy to use for
connecting. However when v2 connection is attempted and it fails a v1
connection is tried without that proxy.
Store the override proxy in `CNode` and pass it to
`CConnman::m_reconnections` to be used for v1 retries.
Github-Pull: #35410
Rebased-From: fd230f942d
Speeds of 1MB/s and 15 minute cached docker image pulls during builds
are not uncommon.
Warp runners provide a local GitHub Actions cache protocol proxy for
Docker layer cache traffic. Point BuildKit's gha cache backend at that
proxy on Warp runners so cached image layers do not have to be fetched
from GitHub's slower cache service.
Add a default for provider so other users (e.g. qa-assets) don't have to
update this unless they use custome runners.
Github-Pull: #35447
Rebased-From: 82901981bf
The GHA cache is very slow, taking on the order of minutes to save and
restore from.
Use WarpBuild's cache instead as this is in the same region and much
faster.
WarpBuild cache action does not auto-fallback to GHA if not being run on
Warp. To allow fork runs to fallback to GHA caching, whilst minimising
duplication in the action files, create new "interal" actions which
perform the switching logic, and use these in the (renamed) cache|save
actions.
Without this we would need the `if` logic in our prvious actions, 4
times in each of save and restore.
Plumb the provider through into the action, as a composite action can't
read `env` (`GITHUB_OUTPUT`) from previous steps.
Github-Pull: #35430
Rebased-From: 2ce4ae7d8f
The existing Clang-only no_sanitize("address") guard is extended to
also cover GCC. When GCC compiles this file with -fsanitize=address
in debug builds, the instrumented inline assembly causes a SEGV during
SHA256AutoDetect()'s self-test on CPUs that use the SSE4 code path
(i.e. those without SHA-NI support), regardless of optimization level.
The original Clang code placed the attribute between the function
declarator and the opening brace. GCC's Attribute Syntax
documentation notes that this position in a function definition
"may, in future, be permitted," so it is not currently supported.
The attribute is moved to the start of the function definition,
which is valid form for both GCC and Clang.
The preprocessor guards are restructured so each compiler branch is
explicit: __clang__ with __has_feature, and __GNUC__ with
__SANITIZE_ADDRESS__.
Github-Pull: #34953
Rebased-From: fedeff7f20
Seek compaction is causing a cascade effect in the chainstate DB, causing large parts of the database to be rewritten every ~hour.
Every periodic flush writes around 2 MiB. Since this is roughly the `write_buffer_size`, these writes regularly cause the memtable to rotate into a small L0 file. This file has a small seek budget, and with the random UTXO reads done during validation, it can get scheduled for seek compaction quickly.
That seek compaction pushes the small file down to L1. Since most UTXOs are already lower down in L4/L5, many reads that consult this file do not find the key there and continue downward. The bloom filter makes those misses cheap, but LevelDB still decrements the file's seek budget. The file then gets scheduled for another seek compaction, and the same pattern pushes it down through L2 and L3.
The expensive part happens around L3/L4. L4 has many ~32 MiB files holding the bulk of the UTXO set. When LevelDB compacts into L3, it may split the output into many smaller L3 files to limit how much L4 "grandparent" data any one output overlaps. Each of these small L3 files then gets its own small seek budget. Because chainstate keys are hash-random, each small L3 file can still have a broad key range, so many random reads consult it and quickly drain its budget. Once seek-compacted into L4, each tiny L3 file can overlap many L4 files, so compacting a few hundred KiB from L3 can require rewriting hundreds of MiB from L4. Repeating that across many small L3 files can rewrite most of the chainstate.
This is a poor fit for chainstate because UTXO keys are hash-random, the DB is large enough to have many levels, writes are relatively small and periodic, and reads are frequent. The result is that read misses trigger compactions much earlier than size pressure would, and those compactions have very high write amplification.
Disabling seek compaction may leave more files in upper levels for longer, so reads could theoretically consult more files. But Bitcoin Core enables bloom filters for all its LevelDB instances, so these misses are usually cheap in-memory filter checks rather than disk reads.
For the other DBs, the risk is much smaller. They also use bloom filters, and most are smaller and less read-heavy. With fewer levels and less random read pressure, disabling seek compaction should have little effect there.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Github-Pull: #35313
Rebased-From: 6bfdb6093bba4710d0f8313ed0113967a8b5176f
Cirrus is winding down and github now offers more than 10GB cache.
Switch to GH cache for all runner-types. Simplify docker build arg
construction, and reduce the number of needed action permissions.
Github-Pull: #35348
Rebased-From: c03107acf5
`CalculateMaximumSignedInputSize()` is passed the outpoint being sized, but that context was not used when estimating the signed input size.
Pass the outpoint through so externally selected inputs are not underestimated.
Co-authored-by: Antoine Poinsot <darosior@protonmail.com>
Github-Pull: #35228
Rebased-From: cd8d3bd937
Because the corresponding Taproot fields were added in PSBT in PR 22558, so
these restrictions are no longer necessary.
Github-Pull: #35279
Rebased-From: 81348576cc
The logging.warning call referenced `bci["bestblockhash"]`, a variable
from the calling scope `do_generate()` that is not available inside the
`Generate.gbt()` method. This would crash with a NameError when
getblocktemplate returned a template based on an unexpected previous
block.
Use the `bestblockhash` parameter that was already being passed in and
used correctly in the comparison on the line above.
The bug was introduced in 7b31332370 when the gbt logic was extracted
into its own method — the if-condition was updated but the logging
call was not.
Github-Pull: #35044
Rebased-From: 701bc2dc02
The FreeBSD, NetBSD, and OpenBSD build guides state that ZMQ support is compiled in when the package is installed. Since WITH_ZMQ defaults to OFF, update the wording to mention the required CMake option.
Github-Pull: #35283
Rebased-From: ca93ab808c
The BDB metadata field `last_page` stores the last valid page number, not the number of pages.
The read-only wallet migration parser currently checks reset LSNs with a half-open loop, so it skips the final page and may accept a database whose last page still depends on BDB log files.
Github-Pull: #35227
Rebased-From: e2b0984f99
The `coinscache_sim` fuzz target builds a 23-byte P2SH scriptPubKey manually.
Place `OP_EQUAL` at index 22, after `OP_HASH160`, the 20-byte push opcode, and the 20-byte script hash.
This matches `CScript::IsPayToScriptHash()`, which checks byte 22 for `OP_EQUAL`, see src/script/script.cpp#L229
Github-Pull: #35218
Rebased-From: ac58e6c53c
Add the flag --extended to a test (00_setup_env_i686_no_ipc.sh) with the --usecli flag to cover all tests with --usecli.
Github-Pull: #34991
Rebased-From: a49bc1e24e
send_batch_request() was building raw dicts without a "jsonrpc" version, which made Core to treat them as version 1.0 requests.
This worked in normal mode, but failed with --usecli because TestNodeCLI.batch() expects callables, not dicts.
This commit fixes it by using get_request() which is defined in both AuthServiceProxy and TestNodeCLIAttr.
The assert is changed because by using get_reques() AuthServiceProxy treats it as "jsonrpc" version 2.0 requests, which don't return "error" keys.
Github-Pull: #34991
Rebased-From: 5603ae0ffa
0cedd6abf2 validation: correct lifetime of precomputed tx data (Antoine Poinsot)
Pull request description:
This backports #35209 to the version 31 branch.
ACKs for top commit:
fanquake:
ACK 0cedd6abf2
Tree-SHA512: 12e3b73c5e2e5b49d9abd9d3dcf9aee98f98fcaa1d9f05ae8fe4e7d19bef5a22dad0eafd51acf9720b215c4d166dae2931a45528ac362e5cdb953c690febea96
This makes sure `txsdata` always outlives the Script check queue (since local
objects are destructed in reverse order of construction).
This is the root cause for a security vulnerability reported by Cory Fields in
2024 that could be exploited by crafting an invalid block to cause nodes to
read freed memory. The vulnerability was covertly fixed in commit
`492e1f09943fcb6145c21d470299305a19e17d8b`.
See security advisory for CVE-2024-52911 for more details.
Github-Pull: #35209
Rebased-From: 1ed799fb21
This effectively reverts a3cb309e7c from PR #30194.
That PR reduced the multi_index type signatures as recommended upstream, but
this is no longer supported as of boost 1.91 because it is no longer necessary.
1.91 drops support for the pre-c++11 work-arounds that bloated the type
signatures to begin with.
The upstream `BOOST_MULTI_INDEX_ENABLE_MPL_SUPPORT` define is meant to provide
compatibility with removed features, but it does not work for this case. Using
`indexed_by` directly when defining the `multi_index` (as opposed to inheriting
from it) works with all versions, and avoids the use of the back-compat define.
This is a slight regression when building against boost < 1.91 because the
bloated type signatures are reintroduced in that case, but it's not significant
enough to go to the trouble of introducing version detection and ifdefs.
Github-Pull: #35175
Rebased-From: 0bc9d354df
The option is unused since the last removals in:
* 4c40a923f0, and
* 81bf3ebff7
It was brittle and lead to intermittent test issues. Generally, it is
also confusing, because if a test wanted to connect nodes without
checking their connection, it can use `addnode`, like the rpc_setban.py
test.
So fix all issues by removing it.
Github-Pull: #34425
Rebased-From: fae807ed25
This waits for any disconnect (e.g. from a restart of one of the nodes)
to fully happen before the next connect.
Can be reviewed with the git option:
--color-moved=dimmed-zebra
Github-Pull: #34425
Rebased-From: fab2772647
Make the checks stricter and easier to follow:
* Fix a typo.
* After the first ban from node 1 wait until node 0 "sees" the ban.
* Move the restart_node out of the debug log context, to avoid bloat.
* Removed the timeout from the outer/lower exit stack to check "dropped
(banned)\n" on node 1, because the inner/top exit stack waits longer.
* The inner/top exit stack checks for the both disconnections peer=2 and
possibly peer=3 (for v2->v1 retry).
* And finally, add a redundant assert to confirm once more that node 0
is has "seen" the ban.
Github-Pull: #34425
Rebased-From: fa21edddb2
The `Socks5Server` utility handles multiple incoming connections,
which are handled in separate background threads.
The `stop()` method unblocks and waits for the main background thread
cleanly, but it doesn't attempt to wait for any handler threads.
This change stores handler threads and connections, and attempts
to shut them down before `stop()` returns.
Co-authored-by: vasild <vd@FreeBSD.org>
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
Github-Pull: #34863
Rebased-From: 6ac49373aa
Apply the timeout factor inside the add_block function.
Also, force named args for the two expected strings.
Also, add trailing comma for style.
Github-Pull: #35080
Rebased-From: fa02eb87df
It is best if the internal addrman database is not modified with
information coming from private broadcast connections because that
information can potentially later be sent via other connections.
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
Github-Pull: #35032
Rebased-From: 1ed1a12402
4a5fdb0d88 doc: update manpages for v31.0rc4 (Ava Chow)
7d56e1017d build: bump version to v31.0rc4 (Ava Chow)
2937d4ba41 guix: Restore download and build-system-cmake modules (Ava Chow)
Pull request description:
Restoring missing guix modules required for guix building to work, and bump to rc4.
ACKs for top commit:
davidgumberg:
ACK 4a5fdb0d88
hebasto:
ACK 4a5fdb0d88, the Guix manifest has been processed for `x86_64-w64-mingw32` without errors on my machine.
Tree-SHA512: e7bbf17fc761b926fedea076b85a103e146c22d6dbad645ab087472daa78d8728dcfacf49103bb979652144da74e94e7e5878e05c9ef04b101ce96b234689ddb
