Change BlockBuilderImpl's m_excluded_clusters to unordered
set since ordering is not used.
Change the set to a set of sequence numbers for a modest
stability increase under fuzz testing.
2738b63e02 test: validate behaviour of getpeerinfo last_inv_sequence and inv_to_send (Anthony Towns)
77b2ebb811 rpc/net: report per-peer last_inv_sequence (Anthony Towns)
adefb51c54 rpc/net: add per-peer inv_to_send sizes (Anthony Towns)
Pull request description:
Adds per-peer entries to `getpeerinfo` for the size of the inv_to_send queue and the mempool sequence number as at the last INV. Can be helpful for debugging tx relay performance and privacy/fingerprinting issues.
ACKs for top commit:
sipa:
utACK 2738b63e02
instagibbs:
ACK 2738b63e02
Tree-SHA512: e3c9c52e8e38b099d405a177ffba6783c5821cc5ce1432b98218843e00906986ce2141dcd5b04a67006c328211a672e519fa3390e012688499bfc9ac99767599
b77137a564 ci: link against -lstdc++ in native fuzz with msan job (fanquake)
Pull request description:
Remove the Clang build from msan fuzz by using the apt install LLVM / Clang, and just linking against `-lstdc++`.
ACKs for top commit:
maflcko:
lgtm ACK b77137a564
Tree-SHA512: dc32b22a93196120a343d91265db3f42f6dc00afc887929986987ea62f2513580c855e98d088f037adb4c2e62358f98e47b914a412ef9c1069037917a36c0b03
cad9a7fd73 rpc: Always return per-wtxid entries in submitpackage tx-results (John Moffett)
Pull request description:
Follow-up to #28848
When `submitpackage` produced no per-transaction result for a member, the RPC set `"error": "unevaluated"` but then continued without inserting the entry into `tx-results`, making it impossible for callers to know which `wtxids` were unevaluated.
This inserts the error result before continuing, updates help text, and adjusts functional tests to expect entries for all submitted `wtxids`.
ACKs for top commit:
instagibbs:
ACK cad9a7fd73
glozow:
ACK cad9a7fd73
Tree-SHA512: 8df5c9b3d1f17aaf0311c38f028ae4b55d4c52a660f85171f105c4f65d130b14ab00698ac5d7c27403a0c37fff391c154c3ad44cc99ba4d549d9c30751b8360f
fbde8d9a81 doc: remove unrelated `bitcoin-wallet` binary from `libbitcoin_ipc` description (Sebastian Falbesoner)
Pull request description:
`bitcoin-wallet` as-is is merely an offline wallet inspection tool (introduced more than 9 years ago in PR #13926) that doesn't have any relation with IPC/multiprocess, so remove it from the list of binaries that use `libbitcoin_ipc`.
ACKs for top commit:
pablomartin4btc:
ACK fbde8d9a81
Tree-SHA512: e11720d35596575cd9785b9b00e6b11e46ba4c8aad6fe98e952d4aa4310f9e5c719dd2f177da8b5c3abefc831cbace0e1a0620f428d847f9bdcf7252a8889641
00c253d494 ci: disable cirrus cache in 32bit arm job (will)
ff18b6bbaf ci: refactor docker action to return provider str (will)
Pull request description:
Add an optional matrix field allowing opt-out of configuring cirrus GHA cache when not using cirrus runners.
This is not needed for the cirruslabs/[save|restore]-cache actions, as they automatically fallback based on runner type.
Addresses https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252638785
ACKs for top commit:
m3dwards:
ACK 00c253d494
Tree-SHA512: 4c79deec2b0018f62a982b2d1051c78e94e242a1b8faf5db037353b05b707827dafded56c9b5ffbc861fcadac5a90571077e6ab69410975f7a2f40c755630a8e
`bitcoin-wallet` as-is is merely an offline wallet inspection tool
(introduced more than 9 years ago in PR #13926) that doesn't have any
relation with IPC/multiprocess, so remove it from the list of binaries
that use `libbitcoin_ipc`.
56791b5829 test: split out `system_ram_tests` to signal when total ram cannot be determined (Lőrinc)
337a6e7386 system: improve handling around GetTotalRAM() (Vasil Dimov)
Pull request description:
1. Fix unused variable warning (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
2. Enable `GetTotalRAM()` on other platforms where it was tested to work.
3. Skip the `GetTotalRAM()` unit test on unsupported platforms.
Prior discussion: https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046
ACKs for top commit:
l0rinc:
ACK 56791b5829
hebasto:
ACK 56791b5829.
Tree-SHA512: bc419aa55edad77473dbcf810f02d02fa0c45a6355a93d17f7881051117b753c584296ab3840893270ecdc9bb2bee0fe4e070607c6560b794e97a25da733c47d
This patch achieves two things:
1. Fix unused variable warning (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
2. Enable GetTotalRAM() on other platforms where it was tested to work.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
6a33970fef fuzz: Reduce iterations in slow targets (marcofleon)
Pull request description:
The `mini_miner`, `txdownloadman`, `txdownloadman_impl`, and `tx_pool_standard` fuzz targets are all slow-running targets. Fix this by reducing the iteration count in the `LIMITED_WHILE` loops.
This should help decrease the run time of the fuzz CI jobs. See https://github.com/bitcoin/bitcoin/pull/33425.
Addresses https://github.com/bitcoin/bitcoin/issues/32870 as well.
ACKs for top commit:
Crypt-iQ:
crACK 6a33970fef
dergoegge:
utACK 6a33970fef
enirox001:
Concept ACK 6a33970
brunoerg:
ACK 6a33970fef
Tree-SHA512: d03d687507f497e587f7199866266298ca67d9843985dc96d1c957a6fbffb3c6cd5144a4876c471b84c84318295b0438908c745f3a4ac0254dca3e72655ecc14
79752b9c0b build(windows): Remove lingering registry entries and shortcuts upon install (Hodlinator)
Pull request description:
### Problem
Prior to fb2b05b125 / #32132 we installed using paths with an extra " (64-bit)"-suffix. Installing a version including that commit on top of a version that does not results in 2 entries in the "Installed apps" list. Both of them end up running the same `C:\Program Files\Bitcoin\uninstall.exe`. However, only one of the entries is removed by the uninstaller. The left over registry entry will now point to an executable that no longer exists and fail to work.
Removing the left over "Installed apps" entry on master currently requires the user to manually remove the Windows Registry entries (or run the correct old/new installer to ensure the uninstaller exists again).
### Solution
This PR automates removal of old entries (& shortcuts) when installing the new version.
### Disclaimer
Not an NSIS expert - confirmed that added deletion commands work without causing any visible errors both when prior items exist and when they don't.
ACKs for top commit:
achow101:
ACK 79752b9c0b
hebasto:
ACK 79752b9c0b.
Tree-SHA512: d23bd2e8f035ca93c3bd6187b3e5545c89c541b51d7b2b91b79bae1ebe328cd08c38b57e75a39bb376771fc85a537fe1d628903b9eadd32d04c3eb976c2e6d87
b81f37031c p2p: Increase tx relay rate (Anthony Towns)
Pull request description:
In the presence of smaller transactions on the network, blocks can sustain a higher relay rate than 7tx/second. In this event, the per-peer inventory queues can grow too large.
This commit bumps the rate up to 14 tx/s (for inbound peers), increasing the safety margin by a factor of 2.
Outbound peers continue to receive relayed transactions at 2.5x the rate of inbound peers, for a rate of 35tx/second.
ACKs for top commit:
sipa:
ACK b81f37031c
achow101:
ACK b81f37031c
darosior:
utACK b81f37031c.
glozow:
utACK b81f37031c
Tree-SHA512: 854ea0824d5f4c629f1dceb9ee61cc9226c8f0d4d26664737e68db917f65341d4800362ab55ed32673db920b2b59aa116b4cb9ee063367b2e43c94a904b41c08
Prior releases installed using these paths. Especially annoying was that the lingering registry entry for the uninstaller would show up as "Bitcoin Core (64-bit)" besides the current "Bitcoin Core" entry in the list of installed programs, and whichever was uninstalled last would fail to work as they would default to the same install directory.
When submitpackage produced no per-transaction result for a member,
the RPC previously set "error": "unevaluated" but then continued
without inserting the entry into tx-results, making it impossible for
callers to know which wtxids were unevaluated.
Insert the placeholder result before continuing, update help text, and
adjust functional tests to expect entries for all submitted wtxids.
67f632b6de net: remove unnecessary casts in socket operations (Matthew Zipkin)
Pull request description:
During review of https://github.com/bitcoin/bitcoin/pull/32747 several casting operations were questioned in existing code that had been copied or moved. That lead me to find a few other similar casts in the codebase.
It turns out that since the `Sock` class wraps syscalls with its own internal casting (see https://github.com/bitcoin/bitcoin/pull/24357 and https://github.com/bitcoin/bitcoin/pull/20788 written in 2020-2022) we no longer need to cast the arguments when calling these functions. The original argument-casts are old and were cleaned up a bit in https://github.com/bitcoin/bitcoin/pull/12855 written in 2018.
The casting is only needed for windows compatibility, where those syscalls require a data argument to be of type `char*` specifically:
https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-getsockopt
```
int getsockopt(
[in] SOCKET s,
[in] int level,
[in] int optname,
[out] char *optval,
[in, out] int *optlen
);
```
but on POSIX the argument is `void*`:
https://www.man7.org/linux/man-pages/man2/getsockopt.2.html
```
int getsockopt(socklen *restrict optlen;
int sockfd, int level, int optname,
void optval[_Nullable restrict *optlen],
socklen_t *restrict optlen);
```
ACKs for top commit:
Raimo33:
ACK 67f632b6de
achow101:
ACK 67f632b6de
hodlinator:
ACK 67f632b6de
vasild:
ACK 67f632b6de
davidgumberg:
ACK 67f632b6de
Tree-SHA512: c326d7242698b8d4d019f630fb6281398da2773c4e5aad1e3bba093a012c2119ad8815f42bd009e61a9a90db9b8e6ed5c75174aac059c9df83dd3aa5618a9ba6
168360f4ae coins: warn on oversized -dbcache (Lőrinc)
6c720459be system: add helper for fetching total system memory (Lőrinc)
Pull request description:
### Summary
Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).
### Fix
Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on unsupported platforms we just disable this warning completely).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GB (as a simple property test checking if the unit size is correct, e.g. doesn't return megabytes or bits).
### Details
`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via `CalculateDbCacheBytes()`. This isn't meant as a recommended setting, rather a likely upper limit.
Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside `size_t` on 32-bit systems - and this was simpler than casting back and forth.
We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.
### Cap
If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:
| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
| 1 GiB | 512 | 50.0% | 450* |
| 2 GiB | 1536 | 75.0% | 1536 |
| 4 GiB | 2560 | 62.5% | 3072 |
| 8 GiB | 4096 | 50.0% | 6144 |
| 16 GiB | 4096 | 25.0% | 12288 |
| 32 GiB | 4096 | 12.5% | 24576 |
[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.
### Reproducer
Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000
warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
Warning: A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
### Manual testing
Besides the [godbolt](https://godbolt.org/z/ec81Tjvrj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351
ACKs for top commit:
achow101:
ACK 168360f4ae
w0xlt:
reACK 168360f4ae
hodlinator:
re-ACK 168360f4ae
danielabrozzoni:
reACK 168360f4ae
Tree-SHA512: aa0c9b1034d55a6a4212685a19715d8cd89668ab7c33c688711a15559e6ad81aa65f3cd8b488c91385306e1e16cd9eeefa8f659ba90ef19ce9c7a2e64f8b561a
e9c52272eb test: Avoid interface_ipc.py Duplicate ID errors (Ryan Ofsky)
Pull request description:
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417 reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess system-wide, or to one of the locations listed in the python test's `imports` list before the local libmultiprocess subtree, and then running the test.
ACKs for top commit:
zaidmstrr:
Tested ACK [e9c5227](e9c52272eb)
Tree-SHA512: 5df7fe767989b91245ce96f7c43b6767b7af49ec6c7007175e462341ffd69e161f21632697804060ce286b3e102a8d141a57a53f7e0e32299ef9a3a69ca8794a
Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).
`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via CalculateDbCacheBytes(). This isn't meant as a recommended setting, rather a likely upper limit.
Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside size_t on 32-bit systems - and this was simpler than casting back and forth.
We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.
If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:
| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
| 1 GiB | 512 | 50.0% | 450* |
| 2 GiB | 1536 | 75.0% | 1536 |
| 4 GiB | 2560 | 62.5% | 3072 |
| 8 GiB | 4096 | 50.0% | 6144 |
| 16 GiB | 4096 | 25.0% | 12288 |
| 32 GiB | 4096 | 12.5% | 24576 |
[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.
Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000
warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
Besides the [godbolt](https://godbolt.org/z/EPsaE3xTj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: w0xlt <woltx@protonmail.com>
Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on other platforms we just return an empty optional).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 10 TiB.
The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex
> ullTotalPhys The amount of actual physical memory, in bytes.
https://man7.org/linux/man-pages/man3/sysconf.3.html:
> _SC_PHYS_PAGES The number of pages of physical memory. Note that it is possible for the product of this value and the value of _SC_PAGESIZE to overflow.
> _SC_PAGESIZE Size of a page in bytes. Must not be less than 1.
See https://godbolt.org/z/ec81Tjvrj for further details
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417
reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0:
failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess
system-wide, or to one of the locations listed in the python test's `imports`
list before the local libmultiprocess subtree, and then running the test.
47d79db8a552 Merge bitcoin-core/libmultiprocess#201: bug: fix mptest hang, ProxyClient<Thread> deadlock in disconnect handler
f15ae9c9b9fb Merge bitcoin-core/libmultiprocess#211: Add .gitignore
4a269b21b8c8 bug: fix ProxyClient<Thread> deadlock if disconnected as IPC call is returning
85df96482c49 Use try_emplace in SetThread instead of threads.find
ca9b380ea91a Use std::optional in ConnThreads to allow shortening locks
9b0799113557 doc: describe ThreadContext struct and synchronization requirements
d60db601ed9b proxy-io.h: add Waiter::m_mutex thread safety annotations
4e365b019a9f ci: Use -Wthread-safety not -Wthread-safety-analysis
15d7bafbb001 Add .gitignore
fe1cd8c76131 Merge bitcoin-core/libmultiprocess#208: ci: Test minimum cmake version in olddeps job
b713a0b7bfbc Merge bitcoin-core/libmultiprocess#207: ci: output CMake version in CI script
0f580397c913 ci: Test minimum cmake version in olddeps job
d603dcc0eef0 ci: output CMake version in CI script
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 47d79db8a5528097b408e18f7b0bae11a6702d26
7584a4fda9 cmake: Install `bitcoin` manpage (Hennadii Stepanov)
Pull request description:
This PR is an amendment to https://github.com/bitcoin/bitcoin/pull/31375.
ACKs for top commit:
ryanofsky:
Code review ACK 7584a4fda9.
Tree-SHA512: 66810c1d65fa8ae469b8161a5f807aa7b43a7b18e88d40b05617c7110b2e03e07bcb8f310c1736fb2c3738e274fc524032ff5d34d5c644824a4edd64372f1e9f
f563ce9081 net: Do not apply whitelist permission to onion inbounds (Martin Zumsande)
Pull request description:
Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective.
ACKs for top commit:
darosior:
ACK f563ce9081
furszy:
ACK f563ce9081
vasild:
ACK f563ce9081
Tree-SHA512: 49ae70e382fc2f78b7073553fe649a6843a41214b2986ea7f77e285d02b7bd00fe0320a1b71d1aaca08713808fb14af058f0b1f19f19adb3a77b97cb9d3449ce
Tor inbound connections do not reveal the peer's actual network address.
Therefore do not apply whitelist permissions to them.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
0a26731c4c test: Add submitblock test in interface_ipc (TheCharlatan)
Pull request description:
Expands the ipc mining test a bit with submitting a solved block and checking its validity.
ACKs for top commit:
Sjors:
ACK 0a26731c4c
marcofleon:
code review ACK 0a26731c4c
zaidmstrr:
Tested ACK [0a26731](0a26731c4c)
Tree-SHA512: 35c87d88496eec469bddedf2ae82c494626abb47ae15d5a45d6ab0400199c86501199c3e569e83836549830042be76b197b470e1100a317bdfef2578a9d5a92f
These methods in the Sock class wrap corresponding syscalls,
accepting void* arguments and casting to char* internally, which is
needed for Windows support and ignored on other platforms because
the syscall itself accepts void*:
Send()
Recv()
GetSockOpt()
SetSockOpt()
bdf01c6f61 test: Prevent disk space warning during node_init_tests (Ryan Ofsky)
Pull request description:
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was print a warning:
```
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
```
Fix by setting regtest instead of mainnet network before running the test.
ACKs for top commit:
achow101:
ACK bdf01c6f61
Eunovo:
Tested ACK bdf01c6f61:
janb84:
ACK bdf01c6f61
l0rinc:
tested ACK bdf01c6f61
mzumsande:
utACK bdf01c6f61
enirox001:
utACK bdf01c6
Tree-SHA512: ac4e1e48246c84a4c4b80ccb25e962b0090359ab0e541ee4f1a9e18ac9da8ec35a78c9a55501d231423053e945ff785862f0db141d4b620d622327670c764f8c
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was causing a warning:
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
Fix by setting regtest instead of mainnet network before running the test.
113a422822 wallet: Add m_cached_from_me to cache "from me" status (Ava Chow)
609d265ebc test: Add a test for anchor outputs in the wallet (Ava Chow)
c40dc822d7 wallet: Throw an error in sendall if the tx size cannot be calculated (Ava Chow)
39a7dbdd27 wallet: Determine IsFromMe by checking for TXOs of inputs (Ava Chow)
e76c2f7a41 test: Test wallet 'from me' status change (Ava Chow)
Pull request description:
One of the ways that the wallet would determine if a transaction was sent from the wallet was by checking if the total amount being spent by a transaction from outputs known to the wallet was greater than 0. This has worked fine until recently since there was no reason for 0-value outputs to be created. However, with ephemeral dust and P2A, it is possible to create standard 0-value outputs, and the wallet was not correctly identifying the spends of such outputs. This PR updates `IsFromMe` to only check whether the wallet knows any of the inputs, rather than checking the debit amount of a transaction.
Additionally, a new functional test is added to test for this case, as well as a few other anchor output related scenarios. This also revealed a bug in `sendall` which would cause an assertion error when trying to spend all of the outputs in a wallet that has anchor outputs.
Fixes#33265
ACKs for top commit:
rkrux:
lgtm ACK 113a422822
enirox001:
Tested ACK 113a422. Ran the full functional test suite including `wallet_anchor.py`; all tests passed. Fix for 0 value anchor detection and sendall size errors looks good. LGTM.
furszy:
ACK 113a422822
Tree-SHA512: df2ce4b258d1875ad0b4f27a5b9b4437137a5889a7d5ed7fbca65f904615e9572d232a8b8d070760f75ac168c1a49b7981f6b5052308575866dc610d191ca964
93a29ff283 trace: Workaround GCC bug compiling with old systemtap (Luke Dashjr)
Pull request description:
ACKs for top commit:
0xB10C:
lgtm ACK 93a29ff283 - I did not test this.
Tree-SHA512: 9ce9ed8b7733af721134462073a3417e52d67e9e9853eebbddfa795842b381de98e28756ebfa6652536cbfdd08181142eccd198f4dc00a57d8748801b362b4b7
b736052e39 ci: always use tag for LLVM checkout (fanquake)
Pull request description:
Rather than trying to match the apt installed clang version, which is prone to intermittent issues. i.e #33345.
ACKs for top commit:
davidgumberg:
ACK b736052e39
willcl-ark:
ACK b736052e39
Tree-SHA512: 8e3fcc8219f573cec65941576c7995f21cae3330bcdbf615f799e8c5facd1146d3239a7284e9af7b013c37170ddf7435d7df6d2966f63fe7b4a8e4937311ff36
fa96a4afea ci: Enable CI_LIMIT_STACK_SIZE=1 in i686_no_ipc task (MarcoFalke)
facfde2cdc test: Fix CLI_MAX_ARG_SIZE issues (MarcoFalke)
Pull request description:
`CLI_MAX_ARG_SIZE` has many edge case issues:
* It seems to be lower on some systems, but it is unknown how to reproduce locally: https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3139957274
* `MAX_ARG_STRLEN` is a limit per arg, but we probably want "The maximum length of [all of] the arguments": See https://www.man7.org/linux/man-pages/man3/sysconf.3.html, section `ARG_MAX - _SC_ARG_MAX`.
* It doesn't account for the additional args added by the `bitcoin` command later on: 73220fc0f9/src/bitcoin.cpp (L85-L92)
* It doesn't account for unicode encoding a string to bytes before taking its length.
The issues are mostly harmless edge cases, but it would be good to fix them. So do that here, by:
* Replacing `max()` by `sum()`, to correctly take into account all args, not just the largest one.
* Reduce `CLI_MAX_ARG_SIZE`, to account for the `bitcoin` command additional args.
Also, there is a test. The test can be called with `ulimit` to hopefully limit the max args size to the hard-coded value in the test framework. For reference:
```
$ ( ulimit -s 512 && python3 -c 'import os; print(os.sysconf("SC_ARG_MAX") )' )
131072
```
On top of this pull it should pass, ...
```
bash -c 'ulimit -s 512 && BITCOIN_CMD="bitcoin -M" ./bld-cmake/test/functional/rpc_misc.py --usecli -l DEBUG'
```
... and with the test_framework changes reverted, it should fail:
```
OSError: [Errno 7] Argument list too long: 'bitcoin'
```
Also, there is a commit to enable `CI_LIMIT_STACK_SIZE=1` in the i686 task, because it should now be possible and no longer hit the hard-to-reproduce issue mentioned above.
ACKs for top commit:
cedwies:
ACK fa96a4a
achow101:
ACK fa96a4afea
enirox001:
ACK fa96a4a — thanks for addressing the nits and clarifying the test; LGTM.
mzumsande:
Code Review ACK fa96a4afea
Tree-SHA512: d12211bd097d692d560c3615970ec0e911707d8c6cbbb145591abc548beed55f487a80b08f0a8c89d4eef4d76a9fbd6a33edc0b42b5860a93dd7b954355bc887
653a9849d5 common: Make arith_uint256 trivially copyable (Fabian Jahr)
Pull request description:
Makes `arith_uint256`/`base_uint` trivially copyable by removing the custom copy constructor and copy assignment operators. Removing of the custom code should not result in a change of behavior since `base_uint` contains a simple array of `uint32_t` and compiler generated versions of the code could be better optimized.
This was suggested by maflcko here: https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3186533494
ACKs for top commit:
Raimo33:
ACK 653a9849d5
l0rinc:
ACK 653a9849d5
achow101:
ACK 653a9849d5
hodlinator:
re-ACK 653a9849d5
Tree-SHA512: 38db5220a2cf773c0c5fb5591671e329b6b87458d972db4f5f3f98c025ec329a8c39b32b5bc24ef8b50b1002b43bb248d8b35aa1c9a56c68c6bbd1d470485bd7
75d9b72475 kernel: make blockTip index const (stickies-v)
Pull request description:
Notification interface subscribers need to view, but not mutate, the index.
This change allows improving the #30595 kernel interface, see e.g. `BlockTreeEntry` where [currently](https://github.com/bitcoin/bitcoin/pull/30595/files#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2R617) a `View` is constructed from a non-const pointer, whereas really this should be a `const btck_BlockTreeEntry* entry`.
ACKs for top commit:
achow101:
ACK 75d9b72475
TheCharlatan:
ACK 75d9b72475
l0rinc:
Code review ACK 75d9b72475
yuvicc:
Code review ACK 75d9b72475
Tree-SHA512: 6151374a040cead36490c5fa5ce9dc4d93499a02110f444c50bd90f9095912747bc5b2fd7294815e6794c96a6843f43eb0507706d41d7296af96071b5f704ff4
fa4885ef2f test: Remove polling loop from test_runner (MarcoFalke)
Pull request description:
(This picks up my prior attempt from https://github.com/bitcoin/bitcoin/pull/13384)
Currently, the test_runner is using a `time.sleep` before polling to check if any tests have completed. This is largely fine when running a few tests, or when the tests take a long time.
However, when running many fast tests, this can accumulate and leave the CPU idle for no reason.
A trivial improvement would be to only sleep when really needed:
```diff
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 7c8c15f391..1d9f28cee4 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -747,7 +747,6 @@ class TestHandler:
dot_count = 0
while True:
# Return all procs that have finished, if any. Otherwise sleep until there is one.
- time.sleep(.5)
ret = []
for job in self.jobs:
(name, start_time, proc, testdir, log_out, log_err) = job
@@ -771,6 +770,7 @@ class TestHandler:
ret.append((TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr, skip_reason))
if ret:
return ret
+ time.sleep(.5)
if self.use_term_control:
print('.', end='', flush=True)
dot_count += 1
```
However, ideally there is no sleep at all. So do that by using a `ThreadPoolExecutor`.
This can be tested via something like:
```
time ./bld-cmake/test/functional/test_runner.py $(for i in {1..200}; do echo -n "tool_rpcauth "; done) -j 200
```
The result should show:
* Current `master` is the slowest
* The "sleep patch" from above is a bit faster (1.5x improvement)
* This pull request is the fastest (2x improvement)
ACKs for top commit:
achow101:
ACK fa4885ef2f
l0rinc:
tested ACK fa4885ef2f
Eunovo:
ReACK fa4885ef2f
Tree-SHA512: f097636c5d9e005781012d8e20c2886cd9968544d4d555b1d2e28982d420ff63fec15cfabb6bd30e4d3c389b8b8350a1ddad721cceaf4b7760cad38b95160175
d45f3717d2 txgraph: use enum Level instead of bool main_only (Pieter Wuille)
Pull request description:
Part of #30289. Inspired by https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331387778.
Since there has been more than one case in the development of #28676 of calling a `TxGraph` function without correctly setting the `bool main_only` argument that many of its interface functions have, make these mandatory and explicit, using an `enum class Level`:
```c++
enum class Level {
TOP, //!< Refers to staging if it exists, main otherwise.
MAIN //!< Always refers to the main graph, whether staging is present or not.
};
```
ACKs for top commit:
instagibbs:
ACK d45f3717d2
vasild:
ACK d45f3717d2
glozow:
code review ACK d45f3717d2
Tree-SHA512: d1c4b37e8ab3ec91b414df8970cb47aa080803f68da5881c8e1cbdc6939dea7851e0f715192cf3edd44b7f328cd6b678474d41f9cd9da8cb68f6c5fd78cb71b1
Replacing the custom code with default behavior should not result in a change of behavior since base_uint contains a simple array of uint32_t and compiler generated versions of the code could be better optimized.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Greg Sanders
merge-script
Anthony Towns
Sebastian Falbesoner
Hennadii Stepanov
Lőrinc
Vasil Dimov
Hodlinator
John Moffett
marcofleon
Ava Chow
Ryan Ofsky
Martin Zumsande
merge-script
Matthew Zipkin
TheCharlatan
Fabian Jahr