bde8d63b17637c507a543cebe90f2998b5847373 depends: build libmultiprocess with position independant code (fanquake)
506634d79d6427925cd458f67799fe59e0ab14dd depends: always install libmultiprocess to /lib (fanquake)
beb309626381bf189cd2ae8bde83078b9de47c6a depends: always install capnp to /lib (fanquake)
Pull request description:
Change to always install libmultiprocess into `lib/`. On some systems (my Fedora aarch64 box), libmultiprocess was being installed into `lib64/`, and then configure would fail to pick it up, because we only add `lib/` to pkgconfig/ldflags out of depends. Rather than adding lib64 to those, I opted for installing libmultiprocess into lib, with every other dependency we build.
This was broken in our build after https://github.com/chaincodelabs/libmultiprocess/pull/79 upstream.
ACKs for top commit:
ryanofsky:
Code review ACK bde8d63b17637c507a543cebe90f2998b5847373. Only changes since last review were reverting the native_capnp change as suggested, and changing the order of the first two commits.
Tree-SHA512: ddd547e4ac224f2f199c569efd91104db7f2c243b124f9535aa0d9377315775ac566d699101580ce45ddd6676ad3e0c8cbe256334eeed9548205c2fa04d02102
fa3da629a1aebcb4500803d7417feed8e34285b0 Remove DirIsWritable, GetUniquePath (MarcoFalke)
fad3a9793b71df5bb0b17cc3758cf3466d08c015 Return LockResult::ErrorWrite in LockDirectory (MarcoFalke)
fa0afe740843c308f6287b923f1f4d758cf2a3f6 refactor: Return enum in LockDirectory (MarcoFalke)
Pull request description:
`GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`.
Fix the redundancy by removing everything, except `LockDirectory`.
ACKs for top commit:
TheCharlatan:
Re-ACK fa3da629a1aebcb4500803d7417feed8e34285b0
hebasto:
ACK fa3da629a1aebcb4500803d7417feed8e34285b0, I have reviewed the code and it looks OK.
Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9
576bee88fd36e207b7288077626947a1fce0fc33 fuzz: disable BnB when SFFO is enabled (furszy)
05e5ff194c7722b4ebc2b9309fc0bf47b3cf1df7 test: add coverage for BnB-SFFO restriction (furszy)
0c5755761c3e544547899ad096121585dffa73df wallet: create tx, log resulting coin selection info (furszy)
5cea25ba795d6eb9ccc721d01560783ae576af34 wallet: skip BnB when SFFO is active (Murch)
Pull request description:
Solves #28918. Coming from https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838626406 discussion.
The intention is to decouple only the bugfix relevant commits from #28985, allowing them to be included in the 26.x release. This way, we can avoid disabling the coin selection fuzzing test for an entire release.
Note:
Have introduced few changes to the bug fix commit so that the unit tests pass without the additional burden introduced in #28985.
ACKs for top commit:
josibake:
ACK 576bee88fd
murchandamus:
ACK 576bee88fd
achow101:
ACK 576bee88fd36e207b7288077626947a1fce0fc33
Tree-SHA512: f5d90eb3f3f524265afe4719495c9bf30f98b9af26cf039f7df5a7db977abae72caa7a3478cdd0ab10cd143bc1662e8fc5286b5bc10fc10f0dd582a45b45c31a
On some systems, libmultiprocess would be installed into `lib64`, I
assume due to the use of GNUInstallDirs, however all other libs we build
in depends, go into lib/. Rather than adding lib64/ to the pkg-config
and link flags, I opted for always installing into lib/.
This was changed in
https://github.com/chaincodelabs/libmultiprocess/pull/79 upstream.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
On some systems, capnp would be installed into `lib64`, I
assume due to the use of GNUInstallDirs, however all other libs we build
in depends, go into lib/. Rather than adding lib64/ to the pkg-config
and link flags, I opted for always installing into lib/.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
7b22cd80e050b903b5765133b8116f4b17ce0296 Revert "ci: Only run functional tests on windows in master" (Hennadii Stepanov)
Pull request description:
This PR reverts commit aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b from https://github.com/bitcoin/bitcoin/pull/28567.
The Windows-specific code received [quality](https://github.com/bitcoin/bitcoin/pull/28486) and [performance](https://github.com/bitcoin/bitcoin/pull/29045) improvements recently. So there are no reasons to skip functional tests in PRs anymore.
In my own repo, I've run the GHA Windows job more than 100 times with no failure.
ACKs for top commit:
maflcko:
lgtm ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
TheCharlatan:
ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
Tree-SHA512: 1e8687e8efe12db506e7cd2d5df9e48b5acb98a339f84684dd0fd67280e22227e2a5a206f1108b09e49038fab7a3ca2ffbd985677f00048c0962b39b2b9a2ba5
bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 wallet: Assert that the wallet is not initialized in LoadWallet (Andrew Chow)
fb0b6ca4e5d981cf58bf23ae2993117f171608e8 tests, bench: Remove incorrect LoadWallet() calls (Andrew Chow)
Pull request description:
`CWallet::LoadWallet()` expects to be called after a `CWallet` is constructed, but before any of its member functions called. Doing so invalidates pointers which causes issues with some PRs and branches that I am working on. This was being used incorrectly in a few tests and benchmarks, resulting in segfaults.
As a precaution for this kind of issue in the future, I've also added a few asserts to `LoadWallet()` so that developers will notice when it is used incorrectly.
As similar issue was fixed in #27666
ACKs for top commit:
S3RK:
ACK bd7f5d33e3b9e01cd600afe1682b7afa935f68a3
furszy:
ACK bd7f5d33
Tree-SHA512: 7664f12b8452994e7fc4d7d4f77697fb5f75edb0dba95ba99a4a23ec03d5b8e0ecbdcb7635547a0e8b4f89f708f98dcb5d039df0559e24b1ae411ed630e16e14
Verify the transaction creation process does not produce
a BnB solution when SFFO is enabled.
This is currently problematic because it could require a
change output. And BnB is specialized on changeless solutions.
Co-authored-by: Andrew Chow <achow101@gmail.com>
Co-authored-by: Murch <murch@murch.one>
LoadWallet() must only be called immediately after a CWallet is
constructed, or not at all. Doing so after any other CWallet member
functions have been called may cause pointers and other objects
setup by other those functions to become invalidated.
Since these tests and benchmarks are using completely new wallets with
mock databases, it's not necessary to call LoadWallet() anyways, so
these can be dropped.
fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 build: Enable -Wunreachable-code (MarcoFalke)
Pull request description:
It seems a bit confusing to write code after a `return`. This can even lead to bugs, or incorrect code, such as https://github.com/bitcoin/bitcoin/pull/28830/files#r1415372320 . (Edit: The linked instance is not found by clang's `-Wunreachable-code`).
Fix all issues by enabling `-Wunreachable-code`.
This flag also enables `-Wunreachable-code-loop-increment`, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code, so remove that.
ACKs for top commit:
ajtowns:
> ACK [fa8adbe](fa8adbe7c1)
stickies-v:
ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876
jonatack:
ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 tested with arm64 clang 17.0.6
Tree-SHA512: 12a2f74b69ae002e62ae08038f7458837090a12051a4c154d05ae4bb26fb19fc1fa76c63aedf2b3fbb36f048c593ca3b8c0efe03fe93cf07a0fd114fc84ce1e7
0295b44c257fe23f1ad344aff11372d67864f0db wallet: return CreatedTransactionResult from FundTransaction (Andrew Chow)
758501b71391136c33b525b1a0109b990d4f463e wallet: use optional for change position as an optional in CreateTransaction (Andrew Chow)
2d39db7aa128a948b6ad11242591ef26a342f5b1 wallet: Explicitly preserve scriptSig and scriptWitness in CreateTransaction (Andrew Chow)
14e50746f683361f4d511d384d6f1dc44ed2bf10 wallet: Explicitly preserve transaction version in CreateTransaction (Andrew Chow)
0fefcbb776063b7fcc03c28e544d830a2f540250 wallet: Explicitly preserve transaction locktime in CreateTransaction (Andrew Chow)
4d335bb1e00a414a4740007d5a192a73179b2262 wallet: Set preset input sequence through coin control (Andrew Chow)
596642c5a9f52dda2599b0bde424366bb22b3c6e wallet: Replace SelectExternal with SetTxOut (Andrew Chow)
5321786b9d90eaf35059bb07e6beaaa2cbb257ac coincontrol: Replace HasInputWeight with returning optional from Get (Andrew Chow)
e1abfb5b2037ca4fe5a05aa578030c8016491c8b wallet: Introduce and use PreselectedInput class in CCoinControl (Andrew Chow)
Pull request description:
Currently `FundTransaction` handles transaction locktime and preset input data by extracting the selected inputs and change output from `CreateTransaction`'s results. This means that `CreateTransaction` is actually unaware of any user desired locktime or sequence numbers. This can have an effect on whether and how anti-fee-sniping works.
This PR makes `CreateTransaction` aware of the locktime and preset input data by providing them to `CCoinControl`. `CreateTransasction` will then set the sequences, scriptSigs, scriptWItnesses, and locktime as appropriate if they are specified. This allows `FundTransaction` to actually use `CreateTransaction`'s result directly instead of having to extract the parts of it that it wants.
Additionally `FundTransaction` will return a `CreateTransactionResult` as `CreateTransaction` does instead of having several output parameters. Lastly, instead of using `-1` as a magic number for the change output position, the change position is changed to be an optional with no value set indicating no desired change output position (when provided as an input parameter) or no change output present (in the result).
ACKs for top commit:
josibake:
ACK 0295b44c25
S3RK:
Code review ACK 0295b44c257fe23f1ad344aff11372d67864f0db
Tree-SHA512: 016be4d41cbf97e1938506e70959bb5335b87006162a1c1c62fa0adb637cbe7aefb76d342b8efad5f37dc693f270c8d0a0839e239fd1ac32c6941a8172f1a710
9f265d88253ed464413dea5614fa13dea0d8cfd5 fuzz: Detect deadlocks in process_message (dergoegge)
fae1e7e012571201fd51c547ba4fb6044be9aeb5 fuzz: p2p: Detect peer deadlocks (MarcoFalke)
Pull request description:
It may be possible that a peer connection will deadlock, due to software bugs such as https://github.com/bitcoin/bitcoin/pull/18808.
Fix this by detecting them in the fuzz target.
Can be tested by introducing a bug such as:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 1067341495..97495a13df 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2436,3 +2436,3 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
if (it != peer.m_getdata_requests.end() && !pfrom.fPauseSend) {
- const CInv &inv = *it++;
+ const CInv& inv = *it;
if (inv.IsGenBlkMsg()) {
```
Using a fuzz input such as:
```
$ base64 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5
kNptdNbW1tbWYghvXIpwb25vPQAA////////cwAjLv8AXAB2ZXJhY2sAQW5v/62tra3Pz///////
//////////////////////9c8GZpbHRlcmxvYWQAAAEAAwAAAABVYwC2XABmaWx0ZXJhZGQAAAAX
Fxdn/////2V0F861tcqvEmAAACEAAABjYXB0dXJldmUAAH4AgAA1PNfX11x0Z2V0ZGF0YQBDACOw
AQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4zKh/HKLK3PPGIkQ9eE/////////8AAAAAAAAAAFtb
WyjDTzpeMSofx7K3PNfX11x0Z2V0ZGF0YQBDACMwAQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4z
Kh/Hsrc88YiRD2/Nzc3Nzc3Nzc3NTc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3N
zWWj1NTUudTU1NTU1P///0j+P/9cdHR4AAAAAAAAy/4AAHR4AAAAAAAAP8v+AAD/+P//////////
AX55bJl8HWnz/////wAgXGF0YVPxY2RkAAAA
```
And running the fuzz target:
```
$ FUZZ=process_messages ./src/test/fuzz/fuzz -runs=1 -timeout=18 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3436516708
INFO: Loaded 1 modules (390807 inline 8-bit counters): 390807 [0x55d0d6221e80, 0x55d0d6281517),
INFO: Loaded 1 PC tables (390807 PCs): 390807 [0x55d0d6281518,0x55d0d6877e88),
./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
Running: ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5
ALARM: working on the last Unit for 19 seconds
and the timeout value is 18 (use -timeout=N to change)
==375014== ERROR: libFuzzer: timeout after 19 seconds
```
ACKs for top commit:
naumenkogs:
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5
dergoegge:
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5
brunoerg:
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5
Tree-SHA512: da83ff90962bb679aae00e8e9dba639c180b7aaba544e0c4d0978d36e28a9ff1cd7a2e13009d8ab407ef57767656aca1ebc767a7d2f1bc880284f8f57c197a50
15f5a0d0c8ce6b306cdeba6a4777334b848a76aa fuzz: Improve fuzzing stability for txorphan harness (dergoegge)
Pull request description:
The `txorphan` harness has low stability as eviction of orphan txs is entirely random at the moment.
Fix this by passing the rng to `LimitOrphans`, which can be deterministic in tests.
Also see #29018.
ACKs for top commit:
maflcko:
lgtm ACK 15f5a0d0c8ce6b306cdeba6a4777334b848a76aa
brunoerg:
utACK 15f5a0d0c8ce6b306cdeba6a4777334b848a76aa
Tree-SHA512: 854ec34b3a0f16f26db6dc419096c6e7a380e8400119534aa278d6b1d54c253b572aa2fad13c383c796c431d8ff4263956e6f60326e99f8bf6abd16d9a280e97
1a5dae630df1eef9eac51557b2f1c5dba0924953 msvc: Define the same `QT_...` macros as in Autotools builds (Hennadii Stepanov)
Pull request description:
There are no reasons to have such a diversion.
Also it fixes https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1847971114.
ACKs for top commit:
sipsorcery:
tACK 1a5dae630df1eef9eac51557b2f1c5dba0924953.
TheCharlatan:
ACK 1a5dae630df1eef9eac51557b2f1c5dba0924953
Tree-SHA512: 75be5eabb8fec974b8d77a023c72323015a3d95fbc13b7fd85e5f25c250ae67850ddf0bcaef143828d75fe35a49e7c9b1966976b74f3ce7d14465174e6585ceb
ea00f982d21aab51001d422225f00626a74db298 test: fix intermittent error in rpc_net.py (#29030) (Sebastian Falbesoner)
Pull request description:
Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point.
Solve this by using the recently introduced `wait_for_new_peer` helper (see #29006, commit 00e0658e77f66103ebdeb29def99dc9f937c049d), which is more robust.
Fixes#29030.
ACKs for top commit:
maflcko:
lgtm ACK ea00f982d21aab51001d422225f00626a74db298
Tree-SHA512: dda307949a466fb3b24408a8c213d307e0af2155f2e8b4e52c836a22397f9d218bf9d8c54ca55bae62a96d7566f27167db9311dd8801785c327234783af5ed00
fd0bde2793239bd6d60a2435fa28df915cedd7e6 test: fix `addnode` functional test failure on OpenBSD (Sebastian Falbesoner)
Pull request description:
This is the functional test counterpart of PR #28891 / commit 007d6f0e85bc329040bb405ef6016339518caa66 (unfortunately, I missed it back then and only ran the unit tests -- sorry for the noise).
master branch on OpenBSD 7.4:
```
$ ./test/functional/rpc_net.py
2023-12-08T17:29:05.057000Z TestFramework (INFO): PRNG seed is: 6024296850131317403
2023-12-08T17:29:05.058000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_au3zchif
2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getconnectioncount
2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getpeerinfo
2023-12-08T17:29:06.643000Z TestFramework (INFO): Check getpeerinfo output before a version message was sent
2023-12-08T17:29:06.709000Z TestFramework (INFO): Test getnettotals
2023-12-08T17:29:06.773000Z TestFramework (INFO): Test getnetworkinfo
2023-12-08T17:29:06.978000Z TestFramework (INFO): Test addnode and getaddednodeinfo
2023-12-08T17:29:06.980000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 65, in run_test
self.test_addnode_getaddednodeinfo()
File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 224, in test_addnode_getaddednodeinfo
assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port2, command='add')
File "/home/thestack/bitcoin/test/functional/test_framework/util.py", line 131, in assert_raises_rpc_error
assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
AssertionError: No exception raised
```
On the PR branch, the same call succeeds.
ACKs for top commit:
kevkevinpal:
ACK [fd0bde2](fd0bde2793)
Tree-SHA512: ae20816fa4025c212e115ebd267b5e5784bfcdf0051219eb686faaade47ec4f91a3947af6d24258b159290000d2dcc3f6e65e788b83b8a9297282945dbdafbfb
6e0f1d2abbb700d4fd4b956a7d1f9505b653653c msvc: Optimize "Release" builds (Hennadii Stepanov)
Pull request description:
It is awkward not using optimization.
In addition to the obvious benefits for Windows users, this PR reduces the duration of functional tests by an hour.
Picked from https://github.com/bitcoin/bitcoin/pull/24773.
ACKs for top commit:
sipsorcery:
tACK 6e0f1d2abbb700d4fd4b956a7d1f9505b653653c.
Tree-SHA512: 5aa7fd38cb1a81d58ea3206756a8099891866c82a747d3b8079cab0b2afa1f40ba53adff2f32eb233efcd1227babee80ab175e35a83678fafa8a4f63c356e5ca
d08e820abf5da2be09b8a84b5bd3450d1a55a039 Add a note to msvc readme re building Qt for Bitcoin Core. (Aaron Clauson)
Pull request description:
Updated the msvc readme with a note about avoiding path too long errors when building Qt with Bitcoin Core.
Would have saved me half an hour if I'd remembered this from the last time I did the build.
ACKs for top commit:
hebasto:
ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039.
TheCharlatan:
ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039
Tree-SHA512: f51017b15383dbcd39ad1e5e978bb255b9205dc23d72b5e3530c6aefcbbc2dc4ec3a85e5fc8c0019c8511173c298f80b837cb35f268deac424d19365b25fb335
Asserting for the debug log message "Added connection peer=" is
insufficient for ensuring that this new connection will show up in a
following getpeerinfo() call, as the debug message is written in the
CNode ctor, which means it hasn't necessarily been added to
CConnman.m_nodes at this point.
Solve this by using the recently introduced `wait_for_new_peer`
helper, which is more robust.
Fixes#29030.
Instead of using the output parameters, return CreatedTransactionResult
from FundTransaction in the same way that CreateTransaction does.
Additionally, instead of modifying the original CMutableTransaction, the
result from CreateTransactionInternal is used.
When creating a transaction with preset inputs, also preserve the
scriptSig and scriptWitness for those preset inputs if they are provided
(e.g. in fundrawtransaction).
Instead of having a separate CCoinControl::SelectExternal function, we
can use the normal CCoinControl::Select function and explicitly use
PreselectedInput::SetTxOut in the caller. The semantics of what an
external input is remains.
Instead of having different maps for selected inputs, external inputs,
and input weight in CCoinControl, have a class PreselectedInput which
tracks stores that information for each input.
fa6e50d6c79633e22ad4cfc75f56aaa40112ecbb fuzz: Use C++20 starts_with in rpc.cpp (MarcoFalke)
faa48388bca06df1ca7ab92461b76a6720481e45 Revert "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings" (MarcoFalke)
fae3b77a87f4d799aca5907335a9dcbab3a51db6 refactor: Drop unused _Pragma to ignore -Wgnu-zero-variadic-macro-arguments (MarcoFalke)
fa02fc0a86c410f907de4fee91dd045547ea4b6e refactor: modernize-use-default-member-init for bit-fields (C++20) (MarcoFalke)
fa67f096bdea9db59dd20c470c9e32f3dac5be94 build: Require C++20 compiler (MarcoFalke)
Pull request description:
C++20 allows to write safer code, because it allows to enforce more stuff at compile time (`constinit`, `conteval`, `constexpr`, `std::span`, ...).
Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts).
See https://github.com/bitcoin/bitcoin/issues/23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20
With g++-10 (https://github.com/bitcoin/bitcoin/pull/28348) and clang-13 (https://github.com/bitcoin/bitcoin/pull/28210), there is broad support for almost all features of C++20.
It should be fine to require a C++20 compiler for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.
This pull request includes three small cleanups to make use of C++20 features. If any issues are detected before or after merge, this should be easy to revert. If no issues arise, it should be fine to make use of more involved C++20 features later on.
ACKs for top commit:
fanquake:
ACK fa6e50d6c79633e22ad4cfc75f56aaa40112ecbb
Tree-SHA512: 244d79bfb0b750a4bdd713f40573b9ca33816fb84b6c84a58f027b9d7d4bb0cc4f18642959e4cf3d094808a69e5b8a327ca8521d7c0c08af27dacb5da3e78e71
00e0658e77f66103ebdeb29def99dc9f937c049d test: fix v2 transport intermittent test failure (#29002) (Sebastian Falbesoner)
Pull request description:
This PR improves the following fragile construct for detection of a new connection to the node under test in `p2p_v2_transport.py`:
6d5790956f/test/functional/p2p_v2_transport.py (L154-L156)
Only relying on the number of peers for that suffers from race conditions, as unrelated previous peers could disconnect at anytime in-between. In the test run in #29002, the following happens:
- `getpeerinfo()` is called the first time -> assigned to `num_peers`
- **previous peer disconnects**, the node's peer count is now `num_peers - 1` (in most test runs, this happens before the first getpeerinfo call)
- new peer connects, the node's peer count is now `num_peers`
- the condition that the node's peer count is `num_peers + 1` is never true, test fails
Use the more robust approach of watching for an increased highest peer id instead (again using the `getpeerinfo` RPC call), with a newly introduced context manager method `TestNode.wait_for_new_peer()`. Note that for the opposite case of a disconnect, no new method is introduced; this is currently used only once in the test and is also simpler.
Still happy to take suggestions for alternative solutions.
Fixes#29002.
ACKs for top commit:
kevkevinpal:
Concept ACK [00e0658](00e0658e77)
maflcko:
Ok, lgtm ACK 00e0658e77f66103ebdeb29def99dc9f937c049d
stratospher:
ACK 00e0658.
Tree-SHA512: 0118b87f54ea5e6e080ff44f29d6af6674c757a588534b3add040da435f4359e71bf85bc0a5eb7170f99cc9956e1a03c35cce653d642d31eed41bbed1f94f44f
97c0dfa8942c7fd62c920deee03b4d0c59aba641 test: Extends MEMPOOL msg functional test (Sergi Delgado Segura)
Pull request description:
Currently, p2p_filter.py::test_msg_mempool is not testing much. This extends the tests so the interaction between sending `MEMPOOL` messages with a filter that does not include all transactions in the mempool reacts, plus how it interacts with `INV` messages, especially after the changes introduced by #27675
ACKs for top commit:
instagibbs:
ACK 97c0dfa894
theStack:
re-ACK 97c0dfa8942c7fd62c920deee03b4d0c59aba641
Tree-SHA512: 746fdc867630f40509e6341f484a238dd855ae6d1be5eca121974491e4ca272dee88af4b90dda55ea9a5a19cbff198fa91ffa0c5bf1ddf0232b2c1b215b05b9a
f05302427386fe63f4929a7198652cb1e4ab3bcc wallet: batch external signer descriptor import (Sjors Provoost)
1f65241b733cd1e962c88909ae66816bc6451fd1 wallet: descriptors setup, batch db operations (furszy)
3eb769f15013873755e482707cad341bc1ce8a8c wallet: batch legacy spkm TopUp (furszy)
075aa44ceba41fa82bb3ce2295e2962e5fd0508e wallet: batch descriptor spkm TopUp (furszy)
bb4554c81e0d819d74996f89cbb9c00476aedf8c bench: add benchmark for wallet creation procedure (furszy)
Pull request description:
Work decoupled from #28574.
Instead of performing multiple single write operations per spkm
setup call, this PR batches them all within a single atomic db txn.
Speeding up the process and preventing the wallet from entering
an inconsistent state if any of the intermediate transactions fail
(which shouldn't happen but.. if it does, it is better to not store
any spkm rather than storing them partially).
To compare the changes, added benchmark in the first commit.
ACKs for top commit:
Sjors:
re-utACK f05302427386fe63f4929a7198652cb1e4ab3bcc
achow101:
ACK f05302427386fe63f4929a7198652cb1e4ab3bcc
BrandonOdiwuor:
ACK f05302427386fe63f4929a7198652cb1e4ab3bcc
theStack:
Code-review ACK f05302427386fe63f4929a7198652cb1e4ab3bcc
Tree-SHA512: aead8548473e17d4d53e8e7039bbaf5e8bf2fe83f33b33f81cdedefe8a31b7003ceb6d5379b1bad1ca2692e909492009a21284ec8338eede078df3d19046ab5a
fa88953d6fb54fdb47485981279632c693534108 doc: Add link to needs-release-notes label (MarcoFalke)
Pull request description:
This makes it easier to spot and not forget. C.f. https://github.com/bitcoin/bitcoin/pull/28597#issuecomment-1845299642
ACKs for top commit:
kristapsk:
ACK fa88953d6fb54fdb47485981279632c693534108
TheCharlatan:
ACK fa88953d6fb54fdb47485981279632c693534108
Tree-SHA512: 28336cde36d62622d1b6627497291cbd4644bf1e4e6f17dc9cde39f254e7094dd02484da754e45558e59facb20941dd0c049ce7b33dcc62bfec6c26c16516cdf
ca5937553b4b4dde53995d0b66e30150401023eb doc: Missing additions to 26.0 release notes (fanquake)
7d4e47d1849ba00c5b45995a96f2c747f1a97c71 doc: add historical release notes for 26.0 (fanquake)
8df4aaabbe3da5036436b96b5839acb1d7cd45f1 doc: add minimum required Linux Kernel to release-notes (fanquake)
Pull request description:
Bins are now up, used for GH release etc.
ACKs for top commit:
willcl-ark:
ACK ca5937553b4b4dde53995d0b66e30150401023eb
achow101:
ACK ca5937553b4b4dde53995d0b66e30150401023eb
Tree-SHA512: 1fefd857092412231215dc72b5d79b2a7828a8c74aa6cb19a7dbc3c3b77feace3ce7fa89d517b4ce25ea41ed84e7ca4ba840d0923b97bf8f6b40b27ad96affa9
fa63f16018d9468e1751d2107b5102184ac2d5ae test: Add uint256 string parse tests (MarcoFalke)
facf629ce8ff1d1f6d254dde4e89b5018f8df60e refactor: Remove unused and fragile string interface from arith_uint256 (MarcoFalke)
Pull request description:
The string interface (`base_uint(const std::string&)`, as well as `base_uint::SetHex`) is problematic for many reasons:
* It is unused (except in test-only code).
* It is redundant with the `uint256` string interface: `std::string -> uint256 -> UintToArith256`.
* It is brittle, because it inherits the brittle `uint256` string interface, which is brittle due to the use of `c_str()` (embedded null will be treated as end-of string), etc ...
Instead of fixing the interface, remove it since it is unused and redundant with `UintToArith256`.
ACKs for top commit:
ajtowns:
ACK fa63f16018d9468e1751d2107b5102184ac2d5ae
TheCharlatan:
ACK fa63f16018d9468e1751d2107b5102184ac2d5ae
Tree-SHA512: a95d5b938ffd0473361336bbf6be093d01265a626c50be1345ce2c5e582c0f3f73eb11af5fd1884019f59d7ba27e670ecffdb41d2c624ffb9aa63bd52b780e62
