Works around MinGW/libc++ bug where `BOOST_CHECK_EXCEPTION` catching `const char*` exceptions by reference is broken, see https://github.com/mstorsjo/llvm-mingw/issues/522
While throwing string literals is generally acceptable (clang-tidy explicitly allows it as idiomatic), using proper exception objects provides better type safety and avoids this platform-specific issue.
Also deleted `HasReason::operator()(const char*)` to prevent future reliance on const char* exception handling.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
156927903d ci: Check windows manifests for all executables (Max Edwards)
e1a1b14c93 ci: use a more generic way of finding mt.exe (Max Edwards)
7ae0497eef ci: remove 3rd party js from windows dll gha job (Max Edwards)
Pull request description:
The windows job uses the external dependency `ilammy/msvc-dev-cmd` which runs javascript. We use this to put various tools on the path such as `MSBuild.exe` and `mt.exe`. We can remove this dependency and use `vswhere.exe` directly to find these tools and create a "[Developer command prompt](https://github.com/microsoft/vswhere/wiki/Start-Developer-Command-Prompt#using-powershell)" as someone would on their dev machine.
While in this area of the code, this PR also runs some additional manifest checks on the windows binaries.
Fixes: #32508
ACKs for top commit:
davidgumberg:
crACK 156927903d
hebasto:
ACK 156927903d.
Tree-SHA512: df640dff27579a1c95daddc5a5ba8fd655bbd0a6f2aff74d0f63439c7185c0b18a90abfee3f1f032fe833cd19b822ef71812f44b24c4c044222e46d01c271864
652424ad16 test: additional test coverage for script_verify_flags (Anthony Towns)
417437eb01 script/verify_flags: extend script_verify_flags to 64 bits (Anthony Towns)
3cbbcb66ef script/interpreter: make script_verify_flag_name an ordinary enum (Anthony Towns)
bddcadee82 script/verify_flags: make script_verify_flags type safe (Anthony Towns)
a5ead122fe script/interpreter: introduce script_verify_flags typename (Anthony Towns)
4577fb2b1e rpc: have getdeploymentinfo report script verify flags (Anthony Towns)
a3986935f0 validation: export GetBlockScriptFlags() (Anthony Towns)
5db8cd2d37 Move mapFlagNames and FormatScriptFlags logic to script/interpreter.h (Anthony Towns)
Pull request description:
We currently use 21 of 32 possible bits for `SCRIPT_VERIFY_*` flags, with open PRs that may use 8 more (#29247, #31989, #32247, #32453). The mutinynet fork that has included many experimental soft fork features is [already reusing bits here](d4a86277ed/src/script/interpreter.h (L175-L195)). Therefore, bump this to 64 bits.
In order to make it easier to update this logic in future, this PR also introduces a dedicated type for the script flags, and disables implicit conversion between that type and the underlying integer type. To make verifying that this change doesn't cause flags to disappear, this PR also resurrects the changes from #28806 so that the script flags that are consensus enforced on each block can be queried via getdeploymentinfo.
ACKs for top commit:
instagibbs:
reACK 652424ad16
achow101:
ACK 652424ad16
darosior:
ACK 652424ad16
theStack:
Code-review ACK 652424ad16🎏
Tree-SHA512: 7b30152196cdfdef8b9700b571b7d7d4e94d28fbc5c26ea7532788037efc02e4b1d8de392b0b20507badfdc26f5c125f8356a479604a9149b8aae23a7cf5549f
3635d62f5a chain: make use of pskip in LastCommonAncestor (optimization) (Pieter Wuille)
2e09d66fbb tests: add unit tests for CBlockIndex::GetAncestor and LastCommonAncestor (Pieter Wuille)
Pull request description:
In theory, the `LastCommonAncestor` function in chain.cpp can take $\mathcal{O}(n)$ time, walking over the entire chain, if the forking point is very early, which could take ~milliseconds. I expect this to be very rare in normal occurrences, but it seems nontrivial to reason about worst cases as it's accessible from several places in net_processing.
This PR modifies the algorithm to make use of the `CBlockIndex::pskip` skip pointers to find the forking point in sublinear time (a simulation shows that for heights up to $34 \cdot 4^k - 2$ and $k \geq 8$, no more than $k^2 + 10k + 13$ steps are ever needed), in a way that should be nearly free - at worst the same number of memory accesses should be made, with a tiny increase in computation.
As it appears we didn't really have tests for this function, unit tests are added for that function as well as `CBlockIndex::GetAncestor()`.
This is inspired by https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2394877881
ACKs for top commit:
optout21:
ACK 3635d62f5a
achow101:
ACK 3635d62f5a
vasild:
ACK 3635d62f5a
mzumsande:
Code Review ACK 3635d62f5a
furszy:
ACK 3635d62f5a
stratospher:
ACK 3635d62f5a.
Tree-SHA512: f9b7dea1e34c1cc1ec1da3fb9e90c4acbf4aaf0f04768844f538201efa6b11eeeefc97b720509e78c21878977192e2c4031fd8974151667e2e756247002b8164
1aaaaa078b fuzz: Drop unused workaround after Apple-Clang bump (MarcoFalke)
fadad7a494 Drop support for EOL macOS 13 (MarcoFalke)
Pull request description:
Now that macOS 13 is EOL (https://en.wikipedia.org/wiki/MacOS_Ventura), it seems odd to still support it.
(macOS Ventura 13.7.8 received its final security update on 20 Aug 2025: https://support.apple.com/en-us/100100)
This patch will only be released in version 31.x, another 6 months out from now.
So:
* Update the depends build and release note template to drop EOL macOS 13.
* As a result, update the earliest Xcode to version 16 in CI.
* Also, bump the macOS CI runner to version 15, to avoid issues when version 14 will be at its EOL in about 1 year.
This also allows to drop a small workaround in the fuzz tests and unlocks libcpp hardening (https://github.com/bitcoin/bitcoin/pull/33462)
ACKs for top commit:
stickies-v:
re-ACK 1aaaaa078b
l0rinc:
code review ACK 1aaaaa078b
hodlinator:
re-ACK 1aaaaa078b
hebasto:
ACK 1aaaaa078b.
Tree-SHA512: 6d247a8432ef8ea8c6ff2a221472b278f8344346b172980299507f9898bb9e8e16480c128b1f4ca692bcbcc393da2b2fd6895ac5f118bc09e0f30f910529d20c
c76de2eea1 net: support overriding the proxy selection in ConnectNode() (Vasil Dimov)
Pull request description:
Normally `ConnectNode()` would choose whether to use a proxy and which one. Make it possible to override this from the callers and same for `OpenNetworkConnection()` - pass down the proxy to `ConnectNode()`.
Document both functions.
This is useful if we want to open connections to IPv4 or IPv6 peers through the Tor SOCKS5 proxy.
Also have `OpenNetworkConnection()` return whether the connection succeeded or not. This can be used when the caller needs to keep track of how many (successful) connections were opened.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.
ACKs for top commit:
stratospher:
ACK c76de2e.
optout21:
ACK c76de2eea1
mzumsande:
Code Review ACK c76de2eea1
andrewtoth:
ACK c76de2eea1
Tree-SHA512: 1d266e4280cdb1d0599971fa8b5da58b1b7451635be46abb15c0b823a1e18cf6e7bcba4a365ad198e6fd1afee4097d81a54253fa680c8b386ca6b9d68d795ff0
0f7d4ee4e8 p2p: Use different inbound inv timer per network (Martin Zumsande)
94db966a3b net: use generic network key for addrcache (Martin Zumsande)
Pull request description:
Currently, `NextInvToInbounds` schedules each round of `inv` at the same time for all inbound peers. It's being done this way because with a separate timer per peer (like it's done for outbounds), an attacker could do multiple connections to learn about the time a transaction arrived. (#13298).
However, having a single timer for inbounds of all networks is also an obvious fingerprinting vector: Connecting to a suspected pair of privacy-network and clearnet addresses and observing the `inv` pattern makes it trivial to confirm or refute that they are the same node.
This PR changes it such that a separate timer is used for each network.
It uses the existing method from `getaddr` caching and generalizes it to be saved in a new field `m_network_key` in `CNode` which will be used for both `getaddr` caching and `inv` scheduling, and can also be used for any future anti-fingerprinting measures.
ACKs for top commit:
sipa:
utACK 0f7d4ee4e8
stratospher:
reACK 0f7d4ee.
naiyoma:
Tested ACK 0f7d4ee4e8
danielabrozzoni:
reACK 0f7d4ee4e8
Tree-SHA512: e197c3005b2522051db432948874320b74c23e01e66988ee1ee11917dac0923f58c1252fa47da24e68b08d7a355d8e5e0a3ccdfa6e4324cb901f21dfa880cd9c
dda5228e02 test: set par=2 in default config for functional test framework (Andrew Toth)
Pull request description:
Depending on the host machine, a default `par` value can spawn up to 15 script verification threads for each node. Running the functional test suite with default `par` can exhaust file descriptors or hit other resource limits when many threads are spawned. These threads are mostly idle and the same code paths are executed with a value of `par=2`. Limit this to 2 for functional tests that do not override the default option.
ACKs for top commit:
maflcko:
lgtm ACK dda5228e02
pablomartin4btc:
ACK dda5228e02
l0rinc:
Code review ACK dda5228e02
theStack:
ACK dda5228e02
Tree-SHA512: 4459972330ff50ac7391141db6382579de09d84e68959eaeb5f20972bb9daf9aac1bd68355028ded9ee65e838c12dbd53e6f3bb6cdc375d269f666c19a19eaec
8e47ed6906 test: addrman: check isTerrible when time is more than 10min in the future (brunoerg)
Pull request description:
This PR adds test coverage to kill the following mutant (https://corecheck.dev/mutation/src/addrman.cpp#L76):
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 9c3a24db90..0ffd349315 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -73,7 +73,7 @@ bool AddrInfo::IsTerrible(NodeSeconds now) const
}
if (nTime > now + 10min) { // came in a flying DeLorean
- return true;
+ return false;
}
```
When the `nTime` is set 10 minutes in the future the addr should be marked as terrible.
ACKs for top commit:
Crypt-iQ:
crACK 8e47ed6906
danielabrozzoni:
tACK 8e47ed6906
marcofleon:
Nice, code review ACK 8e47ed6906
Tree-SHA512: b53b3aa234a73ec7808cb1555916ac64dd707f230ec290a1712493ece8e274a060e16d862b31df0f744804ebd3c0c2825c49becb7d3040cc358e48c4002524cb
14ae71f323 test: make notfound_on_unannounced more reliable (David Gumberg)
99bc552980 test: fix (w)txid confusion in p2p_leak_tx.py (Martin Zumsande)
576dd97cb9 test: increase timeout in p2p_leak_tx.py (Martin Zumsande)
Pull request description:
This fixes two issues with `p2p_leak_tx.py`:
1.) #33090: As far as I can see, this is just the randomness of `NextInvToInbounds`/ `rand_exp_duration`, which has a probability of `e^-(60s/5s) = 6.14×10^−6` to result in a period > 60s (our waiting time), so that the test would fail every 160k runs... Doubling the timeout should be sufficient to lower the probability drastically.
2.) The subtest `test_notfound_on_unannounced_tx` has some (w)txid confusion: we send a `MSG_TX`-type getdata with a `wtxid` in it, which necessarily always results in a NOTFOUND. Fixed this, and change the subtest to be more deterministic based on `mocktime`.
ACKs for top commit:
stratospher:
ACK 14ae71f. nice restructuring using mocktime!
davidgumberg:
reACK 14ae71f323
vasild:
ACK 14ae71f323
Tree-SHA512: be5a4ca7bf56f82b6fa04d90ef9312dc2e6f8ff7ddf70b39d979dc42fbdd823157109b8b5dc46eb7f81ac1e816f40e6966b3c8a7d384aadee01e2189c20d3e3a
7b5261f7ef contrib: fix using macdploy script without translations. (amisha)
Pull request description:
**Description**
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
The config comments also mentioned that adding translation file is optional.
```
./macdeployqtplus --help
usage: macdeployqtplus [-h] [-verbose [VERBOSE]] [-no-plugins] [-no-strip] [-translations-dir path] [-zip zip] app-bundle
Improved version of macdeployqt. Outputs a ready-to-deploy app in a folder "dist" and optionally wraps it in a .zip file. Note, that the "dist" folder will be deleted before deploying on each run. Optionally, Qt translation files
(.qm) can be added to the bundle.
```
**Steps to reproduce**
So I was following the general steps to set up app on macos however I didn't download any qt translations presuming it was optional from the comment linkedin in PR, so to reproduce if you have translation directories in place ull need to delete them and then try to build the file, otherwise don't download it at all and try to build it. It should fail on that flag as translations dir was never downloaded.
**Approach taken**
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
ACKs for top commit:
ismaelsadeeq:
ACK 7b5261f7ef
Tree-SHA512: 8d51b17569e42c9feb95e1be17b1551c708a05eb44b82c74db0b25e07006b4ee223d64484f8bdb2ee1420f6e571686561ae1c09bd3362f77dcbb507bc5085f86
eca50854e1 depends: static libxcb_cursor (fanquake)
Pull request description:
Remove the runtime requirement of `libxcb-cursor`. This library is no-longer present on modern Ubuntu.
Fixes#33432.
Also related to #32097.
ACKs for top commit:
davidgumberg:
Addendum ACK eca50854e1
willcl-ark:
Code review ACK eca50854e1
Tree-SHA512: d545a03baf5030de64874b79add87b6ef5f95eb5ca31aa66007ee03554103d2eda5e56dfd4395d0a12e24b2e489457e4f19ed9e6d390351c72a0da630f03cc42
By using the pskip pointer, which regularly allows jumping back much faster
than pprev, the forking point between two CBlockIndex entries can be found
much faster.
A simulation shows that no more than 136 steps are needed to jump anywhere
within the first 2^20 block heights, and on average 65 jumps for uniform
forking points around that height.
The other executables have manifests and these should be checked in
addition to bitcoind. Skipping fuzz.exe, bench_bitcoin.exe and
test_bitcoin-qt.exe as they do not have manifests.
This sets up a vs developer command prompt and should hopefully should
be more resilient to upstream changes
Co-authored-by: David Gumberg <davidzgumberg@gmail.com>
06df14ba75 test: add more TRUC reorg coverge (Greg Sanders)
26e71c237d Mempool: Do not enforce TRUC checks on reorg (Greg Sanders)
bbe8e9063c fuzz: don't bypass_limits for most mempool harnesses (Greg Sanders)
Pull request description:
This was the intended behavior but our tests didn't cover the scenario where in-block transactions themselves violate TRUC topological constraints.
The behavior in master will potentially lead to many erroneous evictions during a reorg, where evicted TRUC packages may be very high feerate and make sense to mine all together in the next block and are well within the normal anti-DoS chain limits.
This issue exists since the merge of https://github.com/bitcoin/bitcoin/pull/28948/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R956
ACKs for top commit:
sdaftuar:
ACK 06df14ba75
glozow:
ACK 06df14ba75
ismaelsadeeq:
Code review ACK 06df14ba75
Tree-SHA512: bdb6e4dd622ed8b0b11866263fff559fcca6e0ca1c56a884cca9ac4572f0026528a63a9f4c8a0660df2f5efe0766310a30e5df1d6c560f31e4324ea5d4b3c1a8
Normally `ConnectNode()` would choose whether to use a proxy and which
one. Make it possible to override this from the callers and same for
`OpenNetworkConnection()` - pass down the proxy to `ConnectNode()`.
Document both functions.
This is useful if we want to open connections to IPv4 or IPv6 peers
through the Tor SOCKS5 proxy.
Also have `OpenNetworkConnection()` return whether the connection
succeeded or not. This can be used when the caller needs to keep track
of how many (successful) connections were opened.
87e7f37918 doc: clarify peer address in getpeerinfo and addnode RPC help (Vasil Dimov)
2a4450ccbb net: change FindNode() to not return a node and rename it (Vasil Dimov)
4268abae1a net: avoid recursive m_nodes_mutex lock in DisconnectNode() (Vasil Dimov)
3a4d1a25cf net: merge AlreadyConnectedToAddress() and FindNode(CNetAddr) (Vasil Dimov)
Pull request description:
`CConnman::FindNode()` would lock `m_nodes_mutex`, find the node in `m_nodes`, release the mutex and return the node. The current code is safe but it is a dangerous interface where a caller may end up using the node returned from `FindNode()` without owning `m_nodes_mutex` and without having that node's reference count incremented.
Change `FindNode()` to return a boolean since all but one of its callers used its return value to check whether a node exists and did not do anything else with the return value.
Remove a recursive lock on `m_nodes_mutex`.
Rename `FindNode()` to better describe what it does.
ACKs for top commit:
achow101:
ACK 87e7f37918
furszy:
Code review ACK 87e7f37918
hodlinator:
re-ACK 87e7f37918
Tree-SHA512: 44fb64cd1226eca124ed1f447b4a1ebc42cc5c9e8561fc91949bbeaeaa7fa16fcfd664e85ce142e5abe62cb64197c178ca4ca93b3b3217b913e3c498d0b7d1c9
The returned value in `getpeerinfo/addr` could be a hostname as well as
an IP address and the `:port` part could be missing. It is displayed
from `CNode::m_addr_name` which could have been set from RPC `addnode`
where the argument is allowed to be a hostname and an optional port.
All callers of `CConnman::FindNode()` use its return value `CNode*` only
as a boolean null/notnull. So change that method to return `bool`.
This removes the dangerous pattern of handling a `CNode` object (the
return value of `FindNode()`) without holding `CConnman::m_nodes_mutex`
and without having that object's reference count incremented for the
duration of the usage.
Also rename the method to better describe what it does.
Have `CConnman::DisconnectNode()` iterate `m_nodes` itself instead of
using `FindNode()`. This avoids recursive mutex lock and drops the only
caller of `FindNode()` which used the return value for something else
than a boolean found/notfound.
50194029e7 ci: Remove bash -c from cmake invocation using eval (Brandon Odiwuor)
Pull request description:
Follow up to https://github.com/bitcoin/bitcoin/pull/32970https://github.com/bitcoin/bitcoin/pull/32970#r2213730157
> Does `cmake -S ...` still need to be wrapped in `bash -c "..."`?
https://github.com/bitcoin/bitcoin/pull/32970#r2213741192
> It is not trivial to replace. Maybe the `eval` hack from below can be used:
>
> ```shell
> # parses TEST_RUNNER_EXTRA as an array which allows for multiple arguments such as TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6"'
>
> eval "TEST_RUNNER_EXTRA=($TEST_RUNNER_EXTRA)"
> ```
>however, I haven't tried this yet.
https://github.com/bitcoin/bitcoin/pull/32970#r2213801696
> Yeah, the eval hack should work:
>
> ```
> $ export T="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"; eval "T=($T)"; for i in "${T[@]}"; do echo "_${i}_" ; done
> _-DREDUCE_EXPORTS=ON_
> _-DCMAKE_CXX_FLAGS=-Wno-psabi -Wno-error=maybe-uninitialized_
> ```
>
> (can be done in a follow-up)
This replaces the `bash -c` wrapper with an eval-based array parsing to preserve spaces in flag values (e.g., in CMAKE_CXX_FLAGS), allowing ShellCheck to lint the cmake command
ACKs for top commit:
maflcko:
lgtm ACK 50194029e7
hebasto:
ACK 50194029e7.
Tree-SHA512: 6fd22569e2c719a8d13805f18e1e7e3b8eb57d0a6307f2e7175988b25750eafb7c8260796c8e7350db67d622dbe97e6af7bab8ee52187bb8e8eeae3740a47c01
0802398e74 fuzz: make it possible to mock (fuzz) CThreadInterrupt (Vasil Dimov)
6d9e5d130d fuzz: add CConnman::SocketHandler() to the tests (Vasil Dimov)
3265df63a4 fuzz: add CConnman::InitBinds() to the tests (Vasil Dimov)
91cbf4dbd8 fuzz: add CConnman::CreateNodeFromAcceptedSocket() to the tests (Vasil Dimov)
50da7432ec fuzz: add CConnman::OpenNetworkConnection() to the tests (Vasil Dimov)
e6a917c8f8 fuzz: add Fuzzed NetEventsInterface and use it in connman tests (Vasil Dimov)
e883b37768 fuzz: set the output argument of FuzzedSock::Accept() (Vasil Dimov)
Pull request description:
Extend `CConnman` fuzz tests to also exercise the methods `OpenNetworkConnection()`, `CreateNodeFromAcceptedSocket()`, `InitBinds()` and `SocketHandler()`.
Previously fuzzing those methods would have resulted in real socket functions being called in the operating system which is undesirable during fuzzing. Now that https://github.com/bitcoin/bitcoin/pull/21878 is complete all those are mocked to a fuzzed socket and a fuzzed DNS resolver (see how `CreateSock` and `g_dns_lookup` are replaced in the first commit).
ACKs for top commit:
achow101:
ACK 0802398e74
jonatack:
Review re-ACK 0802398e74
dergoegge:
Code review ACK 0802398e74
Tree-SHA512: a717d4e79f42bacf2b029c821fdc265e10e4e5c41af77cd4cb452cc5720ec83c62789d5b3dfafd39a22cc8c0500b18169aa7864d497dded729a32ab863dd6c4d
451ba9ada4 datacarrier: Undeprecate configuration option (Anthony Towns)
Pull request description:
Removes the deprecation for the `datacarrier` and `datacarriersize` options by reverting commit 0b4048c733 from https://github.com/bitcoin/bitcoin/pull/32406
**Many current Bitcoin Core users want to continue using this option**
This statement is based on public postings from many Bitcoin Core users and not a formal survey. AJ Towns’ observation from [#32406](0b4048c733 (r2084024874)) that “_for now there seem to be a bunch of users who like the option_” has only become more apparent in the months since.
**The deprecation intent is unclear to users**
This echo’s Ava Chow’s comment from #32714 that “_IMO we should not have removal warnings if there is no current plan to actually remove them._” In months since that comment, partially due to increased feedback from Bitcoin Core users wanting to keep this option, there is even less likelihood of a near term plan to remove these options. That leaves Bitcoin Core users in an unclear situation: the option could be removed in the next version or perhaps never. Removing the deprecation gives clarity for their planning purposes. Deprecating the option in the future, preferably with a removal schedule to better inform users, would still be possible.
**Minimal downsides to removing deprecation**
As a best practice, Bitcoin Core has avoided an option when the developers cannot articulate when they should be used. There is non-zero maintenance cost to keeping this code around (although leaving the options deprecated for a long time has the same effect). “Don’t offer users footguns” is also a good principle, but with this option, there seems to be only small impacts that can quickly be remedied by changing the option value by Bitcoin Core users. There already exist in Bitcoin Core more potentially-user-harmful options/values than what datacarrier might cause.
ACKs for top commit:
ajtowns:
ACK 451ba9ada4
darosior:
That said, certain users care strongly about using those options. In these conditions, i do not see the project removing the option anytime soon. Therefore i think it's technically incorrect (and confusing) to mark it as deprecated. utACK 451ba9ada4 on removing the deprecation.
instagibbs:
crACK 451ba9ada4
Raimo33:
ACK 451ba9ada4
Ademan:
utACK 451ba9a
ryanofsky:
Code review ACK 451ba9ada4
marcofleon:
ACK 451ba9ada4
achow101:
ACK 451ba9ada4
moonsettler:
ACK 451ba9ada4
ismaelsadeeq:
utACK 451ba9ada4🛰️
jonatack:
ACK 451ba9ada4
Zero-1729:
crACK 451ba9ada4
vasild:
ACK 451ba9ada4
Tree-SHA512: b83fc509f5dd820976596e1ae9fb69a22ada567e0e0ac88da5fc5e940a46d8894b40cc70c3eff2cbdabd4da5ec913f0d18c1632fc906f210b308855868410699
fabc2615af test: Use extra_port() helper in feature_bind_extra.py (MarcoFalke)
Pull request description:
This is a refactor for self-validating and self-documenting code.
Currently, the test assumes that extra ports are available and just increments them without checking. However, this may not be the case when the test is modified to use more ports. In this case, the tests may fail intermittently and the failure is hard to debug.
Fix this confusion, by calling `p2p_port` each time. This ensures the required `assert n <= MAX_NODES` is checked each time.
Closes https://github.com/bitcoin/bitcoin/issues/33250
ACKs for top commit:
achow101:
ACK fabc2615af
janb84:
crACK fabc2615af
w0xlt:
ACK fabc2615af
Tree-SHA512: 1eff00be7f43104ae8a66e79fbf64075ec22bb20f392ac1e4c8a7dd694d4f1760aa44ea54ab7b1f2b947ab018851ab3c10d3c717714c0bee4d8d24617594c2bb
By using mocktime, we will always hit both the notfound
branch and the tx sent branch.
The previous version didn't achieve that due to timing
issues.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
With a low but not negligible probability in the order
of 10^-6 the exponential timer NextInvToInBounds can lead
to an interval >60s, making the test fail.
Also uses mocktime to speed up the test and fixes a
non-matching on_inv override.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
fc861332b3 wallet, log: reduce unconditional logging during load (furszy)
Pull request description:
Currently the unconditional log during init with a default wallet happens three times:
```
2025-09-03T19:57:16Z init message: Verifying wallet(s)…
2025-09-03T19:57:16Z Using SQLite Version 3.45.1
2025-09-03T19:57:16Z Using wallet XXX/.bitcoin/regtest
2025-09-03T19:57:16Z Using SQLite Version 3.45.1
2025-09-03T19:57:16Z Using wallet XXX/.bitcoin/regtest
(...)
2025-09-03T19:57:16Z Using SQLite Version 3.45.1
2025-09-03T19:57:16Z Using wallet XXX/.bitcoin/regtest
2025-09-03T19:57:16Z init message: Loading wallet…
```
For non-default wallets it's logged two times.
That seems a bit too much, so just log the SQLite version just one, and remove the log for the full path of the wallet, since it's already clear from other logs which wallet is being loaded.
ACKs for top commit:
achow101:
ACK fc861332b3
furszy:
utACK fc861332b3
stickies-v:
ACK fc861332b3
Tree-SHA512: ca45c8ede985e6feab0cb93d718a6d633691276ca6e5f13f6471759f11dee98b312e1c802a7fb42c7fa859b6edc44a8c54b9e2ca389655cf028aebf2dabe51f6
Currently nodes schedule their invs to all inbound peers at the same time.
It is trivial to make use this timing pattern for fingerprinting
identities on different networks. Using a separate timers for each network will
make the fingerprinting harder.
The https://fukuchi.org/ homepage no longer links to the source tarball,
and previously available files appear to have been removed. The homepage
now instructs users to download source tarballs from the GitHub releases
page instead.
The diff between the source trees is immaterial.
d4f47f9771 ci: use latest versions of lint deps (fanquake)
Pull request description:
Some of the versions used here are > 2 years old. i.e `mypy`. Use the latest avilable versions, except for LIEF, which is generally changed with Guix.
Side note. I can't remember the last time one of these tools (mypy, ruff, vulture) actually caught an issue in the lint job.
ACKs for top commit:
maflcko:
lgtm ACK d4f47f9771
janb84:
lgtm ACK d4f47f9771
hebasto:
ACK d4f47f9771, I have reviewed the code and it looks OK.
Tree-SHA512: 8b312535c9fea8e76d58f517ada6d6ea7a119c5e03c8cb84a41b5b6ca80dfaaff65a81478bdc1a5acf734cfb0bc66a8b3ba5400db8973c43ca913b07568abfe4
Not enforcing TRUC topology on reorg was the intended
behavior, but the appropriate bypass argument was not
checked.
This mistake means we could potentially invalidate a long
chain of perfectly incentive-compatible transactions that
were made historically, including subsequent non-TRUC
transactions, all of which may have been very high feerate.
Lastly, it wastes CPU cycles doing topology checks since
this behavior cannot actually enforce the topology in
general for the reorg setting.
Using bypass_limits=true is essentially fuzzing part of a
reorg only, and results in TRUC invariants unable to be
checked. Remove most instances of bypassing limits, leaving
one harness able to do so.
The removed statements were logged up to two or three times for each loaded
wallet. The SQLite version only needs to be logged once.
The full wallet path is dropped, since the existing unconditional
logging while loading wallets is sufficient (also reduces anonymization
efforts in case of sharing logs).
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
`CConnman::AlreadyConnectedToAddress()` is the only caller of
`CConnman::FindNode(CNetAddr)`, so merge the two in one function.
The unit test that checked whether `AlreadyConnectedToAddress()` ignores
the port is now unnecessary because now the function takes a `CNetAddr`
argument. It has no access to the port.
ff05bebcc4 doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness) (Sebastian Falbesoner)
Pull request description:
The lower-case spelling matches the `decodepsbt` result field:
200150beba/src/rpc/rawtransaction.cpp (L871)200150beba/src/rpc/rawtransaction.cpp (L1253)
ACKs for top commit:
l0rinc:
ACK ff05bebcc4
rkrux:
Ah crACK ff05bebcc4
Tree-SHA512: c0a0e29e95fed3fcee4df4f3fc87b32774d76bebadcda5aa010bc45142727536d6a71e4c0e70564db8bdb734e8647c80953793ac9ecd6c434345e972f8d9b7b0
Depending on the host machine, a default `par` value can spawn up to 15 script verification threads for each node.
Running the functional test suite with default `par` can exhaust file descriptors or hit other resource limits when many threads are spawned.
These threads are mostly idle and the same code paths are executed with a value of `par=2`.
Limit this to 2 for functional tests that do not override the default option.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Lőrinc
Ava Chow
Hennadii Stepanov
merge-script
merge-script
brunoerg
Pieter Wuille
Max Edwards
Vasil Dimov
MarcoFalke
Brandon Odiwuor
David Gumberg
Martin Zumsande
Greg Sanders
furszy
Andrew Toth
Sebastian Falbesoner