d8f4e7caf0 doc: add release notes (ismaelsadeeq)
248c175e3d test: ensure `ValidateInputsStandardness` optionally returns debug string (ismaelsadeeq)
d2716e9e5b policy: update `AreInputsStandard` to return error string (ismaelsadeeq)
Pull request description:
This PR is another attempt at #13525.
Transactions that fail `PreChecks` Validation due to non-standard inputs now returns invalid validation state`TxValidationResult::TX_INPUTS_NOT_STANDARD` along with a debug error message.
Previously, the debug error message for non-standard inputs do not specify why the inputs were considered non-standard.
Instead, the same error string, `bad-txns-nonstandard-inputs`, used for all types of non-standard input scriptSigs.
This PR updates the `AreInputsStandard` to include the reason why inputs are non-standard in the debug message.
This improves the `Precheck` debug message to be more descriptive.
Furthermore, I have addressed all remaining comments from #13525 in this PR.
ACKs for top commit:
instagibbs:
ACK d8f4e7caf0
achow101:
ACK d8f4e7caf0
sedited:
Re-ACK d8f4e7caf0
Tree-SHA512: 19b1a73c68584522f863b9ee2c8d3a735348667f3628dc51e36be3ba59158509509fcc1ffc5683555112c09c8b14da3ad140bb879eac629b6f60b8313cfd8b91
faf71d6cb4 test: [refactor] Use verbosity=0 named arg (MarcoFalke)
99996f6c06 test: Fix intermittent issue in feature_assumeutxo.py (MarcoFalke)
Pull request description:
The test has many issues:
* It fails intermittently, due to the use of `-stopatheight` (https://github.com/bitcoin/bitcoin/issues/33635)
* Using `-stopatheight` is expensive, because it requires two restarts, making the test slow
Fix all issues by using `dumb_sync_blocks` and avoid the two restarts.
Fixes https://github.com/bitcoin/bitcoin/issues/33635
Diff to test:
```diff
diff --git a/src/util/tokenpipe.cpp b/src/util/tokenpipe.cpp
index c982fa6fc4..1dc12ea1d5 100644
--- a/src/util/tokenpipe.cpp
+++ b/src/util/tokenpipe.cpp
@@ -4,2 +4,3 @@
#include <util/tokenpipe.h>
+#include <util/time.h>
@@ -60,2 +61,3 @@ int TokenPipeEnd::TokenRead()
ssize_t result = read(m_fd, &token, 1);
+ UninterruptibleSleep(999ms);
if (result < 0) {
```
On master: Test fails
On this pull: Test passes
ACKs for top commit:
fjahr:
Code review ACK faf71d6cb4
sedited:
ACK faf71d6cb4
Tree-SHA512: 8f7705d2b3f17881134d6e696207fe6710d7c4766f1b74edf5a40b4a6eb5e0f4b12be1adfabe56934d27abc46e789437526bcdd26e4f8172f41a11bc6bed8605
fa050da980 test: Move event loop creation to network thread (MarcoFalke)
fa9168ffcd test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy() (MarcoFalke)
Pull request description:
It is deprecated according to https://docs.python.org/3.14/library/asyncio-policy.html#asyncio.WindowsSelectorEventLoopPolicy The replacement exists since python 3.7
Also, move the event loop creation to happen in the thread that runs the loop.
ACKs for top commit:
l0rinc:
ACK fa050da980
sedited:
ACK fa050da980
Tree-SHA512: dce25596a04e8f133630d84c03a770185a81b1bcd0aae975f0dbdd579d22b7b79a9b1172abf46c61d0845d3f5ab4a6414fa0f17c59f0ea0f6fa9bdcac085a2a7
fa90b21430 test: Remove unused feature_segwit.py functions (MarcoFalke)
fa6b05c96f test: Remove unused CUSTOM_._COUNT (MarcoFalke)
fa7bac94d8 test: Remove unused wait_for_addr, firstAddrnServices, on_addr (MarcoFalke)
fa388a3585 test: Remove unused self.p2p_conn_index = 1 (MarcoFalke)
fa803710e2 test: Remove unused AddressType (MarcoFalke)
fab5072ce1 ci: Remove vulture (MarcoFalke)
Pull request description:
Currently, `vulture` is run with `--min-confidence=100`, which reduces its checks to dead code after control statements, which is nice, but not really a common nor severe issue. See the discussion in https://github.com/bitcoin/bitcoin/issues/34810#issuecomment-4045927137 and commit 5c005363a8, which had to remove dead code manually.
Reducing the confidence has shown to be too brittle/tedious in the past, so remove the tool for now from CI.
Of course, removing this from CI does not prevent anyone from running it locally and removing dead code.
Fixes https://github.com/bitcoin/bitcoin/issues/34810
ACKs for top commit:
fanquake:
ACK fa90b21430
willcl-ark:
ACK fa90b21430
Tree-SHA512: 6a5998470dae3a17baec29b70b02333f4cd9b81bc4c6a05b56085ff1ba527ed7bdeccd17b09d9ad785ae03c97982ee1f3147e4df3bd537c66b02e9a44d0e5f15
111864ac30 qa: Avoid duplicating output in case the diff is the same (Hodlinator)
c2e28d455a ci: Enable `wallet_multiwallet.py` in "Windows, test cross-built" job (Hodlinator)
850a80c199 qa: Disable parts of the test when running under Windows or root (Hodlinator)
fb803e3c79 qa: Test scanning errors individually (Hodlinator)
ed43ce57cc qa: Check for platform-independent part of error message (Hodlinator)
64a098a9b6 refactor(qa): Break apart ginormous run_test() (Hodlinator)
bb1aff7ed7 move-only(qa): Move wallet creation check down to others (Hodlinator)
d1a4ddb58e refactor(qa): Lift out functions to outer scopes (Hodlinator)
c811e47367 scripted-diff: self.nodes[0] => node (Hodlinator)
73cf858911 refactor(qa): Remove unused option (Hodlinator)
Pull request description:
Makes the functional test compatible with *Linux->Windows cross-built executables*.
Main parts:
* Commit "qa: Check for platform-independent part of error message" switches to match on platform-independent part of error message.
* Commit "qa: Test scanning errors individually" disentangles code causing the same error message substring, based on #31410.
* Commit "qa: Disable parts of the test when running under Windows or root" enables the test to be run on Windows, based in part on https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-3554721014.
Also:
* Removes unused option in wallet_multiwallet.py.
* Breaks apart wallet_multiwallet.py's `run_test()` into smaller test functions.
* Improves `assert_equal()` output for dicts.
Fixes#31409.
ACKs for top commit:
achow101:
ACK 111864ac30
janb84:
re ACK 111864ac30
w0xlt:
reACK 111864ac30
Tree-SHA512: 4e3ff92588ac9f2611fc963ce62097b6c0dd4d4eb8da7952c72619c7b554ff3cae5163fe1886d4d9bbd7af1acca5b846411e7f5b46f9bddb08719b61108efbba
This should fix https://github.com/bitcoin/bitcoin/issues/34367
I am not familiar with Windows sockets thread-safety, but creating the
event loop on the main thread, and running it in the network thread
could lead to a fast abort in Python on Windows (without any stderr):
```
77/276 - wallet_txn_clone.py failed, Duration: 1 s
stdout:
2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117
stderr:
Combine the logs and print the last 99999999 lines ...
============
Combined log for D:\a\_temp/test_runner_₿_🏃_20251210_075632/wallet_txn_clone_196:
============
test 2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117
test 2025-12-10T08:04:27.500433Z TestFramework (DEBUG): Setting up network thread
```
Also, I couldn't find any docs that require the loop must be created on
the thread that runs them:
* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.new_event_loop
* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_forever
However, the patch seems trivial to review, harmless, and easy to
revert, so it may be a good try to fix the intermittent Windows Python
crash.
57bfa864fe test: use static methods and clarify comment in addr_relay (stratospher)
7ee8c0abc6 test: protect outbound connection from eviction in getaddr_test (stratospher)
ecb5ce6e76 test: fix addr relay test silent pass and wrong peerinfo index (stratospher)
Pull request description:
couple of improvements in the addr relay test:
- fixes the silent test pass discovered in https://github.com/bitcoin/bitcoin/pull/34717#issuecomment-3990585763 (will remove this if that PR gets merged - the test doesn't fail even though #34717 changes the behaviour)
- correct wrong peerinfo index
- prevent intermittent disconnection warnings like the one shown below by protecting outbound peer from `ConsiderEviction`
```
TestFramework (INFO): Check that we answer getaddr messages only once per connection
TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer
```
- remove a no longer applicable test comment since we don't need to send initial GETADDR for intial self announcement anymore
ACKs for top commit:
Bortlesboat:
ACK 57bfa864fe. Ran both `p2p_addr_relay.py` and `p2p_addr_selfannouncement.py` locally, both pass. Good catch on the stale `peerinfo` reference in `inbound_blackhole_tests` — that would silently check the wrong peer.
naiyoma:
ACK 57bfa864fe
mzumsande:
Code Review ACK 57bfa864fe
Tree-SHA512: 22e4f87f66569bfa629a68a8b440cd21b5285c6dad6eb7926514f2d74e16fe3711525b264f82765c83020be976a0438b8d2ab1e48e7c0b7d85437ee672d52324
b19caeea09 doc: add release note for #31560 (named pipe support for `dumptxoutset` RPC) (Sebastian Falbesoner)
61a5460d0d test: add test for utxo-to-sqlite conversion using named pipe (Sebastian Falbesoner)
2e8072edbe rpc: support writing UTXO set dump (`dumptxoutset`) to a named pipe (Sebastian Falbesoner)
Pull request description:
This PR slightly modifies the `dumptxoutset` RPC to allow writing the UTXO set dump into a [named pipe](https://askubuntu.com/a/449192), so that the output data can be consumed by another process, see #31373. Taking use of this with the utxo-to-sqlite.py tool (introduced in #27432), creating an UTXO set in SQLite3 format is possible on the fly. E.g. for signet:
```
$ mkfifo /tmp/utxo_fifo && ./build/bin/bitcoin-cli -signet dumptxoutset /tmp/utxo_fifo latest &
$ ./contrib/utxo-tools/utxo_to_sqlite.py /tmp/utxo_fifo ./utxo.sqlite
UTXO Snapshot for Signet at block hash 000000012711f0a4e741be4a22792982..., contains 61848352 coins
1048576 coins converted [1.70%], 2.800s passed since start
....
....
60817408 coins converted [98.33%], 159.598s passed since start
{
"coins_written": 61848352,
"base_hash": "000000012711f0a4e741be4a22792982370f51326db20fca955c7d45da97f768",
"base_height": 294305,
"path": "/tmp/utxo_fifo",
"txoutset_hash": "34ae7fe7af33f58d4b83e00ecfc3b9605d927f154e7a94401226922f8e3f534e",
"nchaintx": 28760852
}
TOTAL: 61848352 coins written to ./utxo.sqlite, snapshot height is 294305.
```
Note that the `dumptxoutset` RPC calculates an UTXO set hash as a first step before any data is emitted, so especially on mainnet it takes quite a while until the conversion starts and something is happening visibly.
ACKs for top commit:
ajtowns:
utACK b19caeea09
sedited:
Re-ACK b19caeea09
Tree-SHA512: 7101563d0dba15439cdef8c8fb535f8593d5a779ff04208e2d72382a3f99072db8eac3651d1b3fe72c5e1f03e164efb281c3030d45d0723b943ebbbcf2a841d6
22335474d7 net: format peer+addr logs with `LogPeer` (Lőrinc)
e55ea534f7 test: add pre-`LogPeer` net log assertion (Lőrinc)
736b17c0f0 log: fix minor formatting in debug logs (Lőrinc)
9cf82bed32 log: show placeholders for missing peer fields (Lőrinc)
Pull request description:
This is an alternative to #34293, but aims to address the remaining logging inconsistencies more broadly.
It extends the example fixed there to every instance, restores the original separator behavior, applies it consistently via a single helper, and adds tests for `logips` (covering both current and new behavior).
### Problem
After #28521 centralized peer address logging into `CNode::LogIP()`, the original comma separator before `peeraddr=` was lost, resulting in inconsistent formatting across net (and recent private broadcast) logs.
Some lines also had double spaces, empty fields, or mismatched format specifiers.
### Fix
Introduces `CNode::LogPeer(bool)` which always emits `peer=<id>` and, when `-logips=1`, appends `, peeraddr=<addr>`. This eliminates hand-rolled separators and makes peer identification predictable.
Minor issues (double spaces, empty placeholders, format specifiers) are fixed along the way in separate commits.
### Reproducer
Run with `-debug=net -logips=1` and observe peer log lines now show `peer=<id>, peeraddr=<addr>` (comma-separated). The new assertion in `feature_logging.py` automates this check.
ACKs for top commit:
naiyoma:
ACK 22335474d7
vasild:
ACK 22335474d7
sedited:
ACK 22335474d7
Tree-SHA512: 562262a58c3042f139099ff4c41e3fc6a97505fe9603c2bf700a97fd0aa052954b47c14da0e50c1fc311db1ae6c04e6a92156c9b85e25c777a637b7766c1dafe
73e3853110 test: add test for rebroadcast of transaction received via p2p (Martin Zumsande)
Pull request description:
The wallet doesn't only rebroadcast transactions it created, but also relevant transactions received via p2p.
Since this is not self-evident, add test coverage for it.
ACKs for top commit:
maflcko:
re-ACK 73e3853110🦌
vasild:
ACK 73e3853110
furszy:
ACK 73e3853110
danielabrozzoni:
tACK 73e3853110
Tree-SHA512: b5249bb5eb6a124a98030816319aa4364d7aee9c28ee28750ebba5fc8d6bfc9d7960a97ed638611f4560e051760ec4a7a75d302a4cb106dbfadfe11adc604f22
fa0587a306 test: Remove fixed TODO in address_to_scriptpubkey (MarcoFalke)
Pull request description:
After commit d178082996 added the bech32 format, the TODO about adding other formats can be removed.
ACKs for top commit:
sedited:
ACK fa0587a306
rkrux:
crACK fa0587a306
musaHaruna:
ACK [fa0587a](fa0587a306)
Tree-SHA512: 49a8a514bed11e945bd8f9b13b84ae14b4dbc8a7ebb7224b1746776d9dbf68abc3b53d67f1b7fff83bc4360b15324fb96611550f4aca808b16beb03bcbfd0a55
5b2c3960b9 test: clean up tx resurrection (re-org) test in feature_block.py (Sebastian Falbesoner)
Pull request description:
The following comment about ECDSA signatures created with the test framework not passing mempool policy has been obsolete for a long time (at least since 2019, see PR #15826):
8c07800b19/test/functional/feature_block.py (L1167-L1172)
so remove it. While at it, change the resurrected txs to be indeed standard valid, so the `acceptnonstdtxn=1` parameter can also be removed from the functional test.
Kudos to stratospher, who (IIRC) mentioned this outdated comment a while ago.
ACKs for top commit:
sedited:
ACK 5b2c3960b9
Bortlesboat:
Tested ACK 5b2c3960b9. Ran feature_block.py locally, passes. Nice cleanup — removing `-acceptnonstdtxn=1` by making the resurrection txs standard (P2PK) is the right approach. The old unsigned OP_TRUE workaround comment explains why this was needed, glad to see it gone.
stratospher:
ACK 5b2c3960. nice!
Tree-SHA512: 94517e432647a8e8db51fad90a26493c57ce9dfd924b17ce909ea40d50bc6279d974c6d2046b34f5f92b481ca5e0abdedaf412ce41b4cb904605c140a8dba583
20ae9b98ea Extend functional test for setBlockIndexCandidates UB (marcofleon)
854a6d5a9a validation: fix UB in LoadChainTip (marcofleon)
9249e6089e validation: remove LoadChainTip call from ActivateSnapshot (marcofleon)
Pull request description:
Addresses https://github.com/bitcoin/bitcoin/issues/34503. See this issue for more details as well.
Fixes a bug where, under certain conditions, `setBlockIndexCandidates` had blocks in it that were worse than the tip. The block index candidate set uses `nSequenceId` as a sort key, so modifying this field while blocks are in the set results in undefined behavior. This PR populates `setBlockIndexCandidates` after the `nSequenceId` modifications, avoiding the UB.
ACKs for top commit:
achow101:
ACK 20ae9b98ea
sedited:
Re-ACK 20ae9b98ea
sipa:
Code review ACK 20ae9b98ea
Tree-SHA512: 121c170bb70fb6365089d578db63c811e7926e129d7206e569947f7a1f6c5ddc8d5f4937b80f1ba1b7d7daa42789b143ca5b56f154b7ab968a1cd55f925f378d
cbdb891de2 test: Wait for txospender index to be synced in rpc_gettxspendingprevout (Ava Chow)
2db5c049bb test: Sync mempools after tx creation in rpc_gettxspendingprevout (Ava Chow)
Pull request description:
On slower runs, the txospender index may not be synced yet when the tests of its behavior begin, causing intermittent failures. Wait for them to be synced before starting the tests.
The tests also query mempool txs from other nodes, make sure mempools are synced before doing so.
The first commit with the following diff reproduces #34735:
```
diff --git a/src/index/txospenderindex.cpp b/src/index/txospenderindex.cpp
index d451bb1e0a4..e786f05a98c 100644
--- a/src/index/txospenderindex.cpp
+++ b/src/index/txospenderindex.cpp
@@ -129,6 +129,7 @@ static std::vector<std::pair<COutPoint, CDiskTxPos>> BuildSpenderPositions(const
bool TxoSpenderIndex::CustomAppend(const interfaces::BlockInfo& block)
{
+ UninterruptibleSleep(100ms);
WriteSpenderInfos(BuildSpenderPositions(block));
return true;
}
```
Fixes#34735
ACKs for top commit:
furszy:
ACK cbdb891de2
andrewtoth:
ACK cbdb891de2
sedited:
ACK cbdb891de2
rkrux:
crACK cbdb891de2
Tree-SHA512: ba1ab6216ac3b647a5f9e20f4899e51e643bf20829e698aa63c127c88ed4e33afa222bebeae1210fc869c46288e99c0b344330197913e244ffe1a6aca943568d
15c4889497 index: document TxoSpenderIndex::FindSpender (furszy)
f8b9595aaa test: Add missing txospenderindex coverage in feature_init (Fabian Jahr)
a1074d852a index, rpc, test: Misc formatting fixes (Fabian Jahr)
Pull request description:
This addresses my own comments in the last review of #24539: https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-3829110465
The first commit fixes three small formatting errors.
The second commit adds some missing coverage in `feature_init` and refactors the code a bit as well so these misses don't happen so easily in the future.
The third commit is by furzy:
> TxoSpenderIndex::FindSpender returns an Expected<optional<TxoSpender>> but
the two levels of the return type were undocumented, making it unclear what a returned
nullopt means. So added doc clarifying each return case.
ACKs for top commit:
furszy:
ACK 15c4889497
sedited:
ACK 15c4889497
rkrux:
crACK 15c4889497
Tree-SHA512: 2e0f060a54b558d2967ebae0835cf81bd86c2d8d983d670a48d1bee7d347f186623e75db7ae311ca1566807f715c1b3fa67cf734c9467d35e13b84d082f28253
we don't need to send GETADDR for initial self announcement
anymore + can construct addr_receivers using
AddrReceiver(send_getaddr=False).
however we would need to send an empty ADDR message to each
of the addr_receivers to initialise addr relay for inbound
connections. so current code is simpler and we can just
clarify the comment.
since we're bumping mocktime more than CHAIN_SYNC_TIMEOUT = 20 * 60,
it's possible for disconnections like this to happen in the test:
$ test/functional/p2p_addr_relay.py --randomseed=7758649581790797022
...
TestFramework (INFO): Check that we answer getaddr messages only once per connection
TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer
...
5c005363a8 test: improve `wallet_backup` test (rkrux)
04d9515748 test: improve `wallet_assumeutxo` func test (rkrux)
Pull request description:
Relates to #34354
While the actual fix of the issue is in another PR, this one improves the
affected tests by trying to reduce the chain notifications that
need to be processed while simulating erroneous wallet restoration
scenarios.
ACKs for top commit:
maflcko:
lgtm ACK 5c005363a8
furszy:
ACK 5c005363a8
w0xlt:
ACK 5c005363a8
brunoerg:
code review ACK 5c005363a8
Tree-SHA512: 176e3ea8275c7aa082af695f5b76d82c079ff9a7178855b4ce95504edb8ce89b59a772e2d38dd43e997a5bea3d64be700b74cfec7bbc6992538f837877ab7222
Fix the from-disk subtest to use a separate node so it builds on a
clean genesis block, rather than the leftover chain from the
in-memory subtest.
Change from a two-way to a three-way block race. The UB in the old
LoadChainTip (mutating nSequenceId, a sort key, while the block is
in setBlockIndexCandidates) corrupts the internal tree structure,
resulting in a failed erase that leaves stale blocks in the set
alongside the tip. With only two competing blocks, this is caught
by libstdc++ but not by libc++. A three-way split triggers the bug
on both implementations.
To trigger CheckBlockIndex (where the crashing assertion is), replace
the restart loop with sending a new block after a single restart.
faa68ed4bd test: Fix intermittent issue in wallet_assumeutxo.py (MarcoFalke)
Pull request description:
The test has many issues:
* It fails intermittently, due to the use of `-stopatheight` (https://github.com/bitcoin/bitcoin/issues/34710)
* Using `-stopatheight` is expensive, because it requires two restarts, making the test slow
* The overwritten `def setup_network` does not store the extra args via the `add_nodes` call, so if code is added in the future to restart a node, it may not pick up its global extra args.
Fix all issues by:
* Adding and using a fast `dumb_sync_blocks` util helper to achieve what `-stopatheight` doesn't achieve
* Calling `self.add_nodes(self.num_nodes, self.extra_args)` in the overridden `setup_network`
Can be tested via this diff:
```diff
diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp
index ab0e5ccb69..49384c10b8 100644
--- a/src/node/kernel_notifications.cpp
+++ b/src/node/kernel_notifications.cpp
@@ -61,2 +61,3 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
+ LogInfo("Send shutdown signal after reaching stop height");
if (!m_shutdown_request()) {
diff --git a/src/util/tokenpipe.cpp b/src/util/tokenpipe.cpp
index c982fa6fc4..a5565ebd36 100644
--- a/src/util/tokenpipe.cpp
+++ b/src/util/tokenpipe.cpp
@@ -4,2 +4,3 @@
#include <util/tokenpipe.h>
+#include <util/time.h>
@@ -60,2 +61,3 @@ int TokenPipeEnd::TokenRead()
ssize_t result = read(m_fd, &token, 1);
+ UninterruptibleSleep(500ms);
if (result < 0) {
```
On master: The test fails
On this pull: The test passes
Fixes https://github.com/bitcoin/bitcoin/issues/34710
ACKs for top commit:
kevkevinpal:
ACK [faa68ed](faa68ed4bd)
achow101:
ACK faa68ed4bd
w0xlt:
ACK faa68ed4bd
Tree-SHA512: 6fcd52b6f6a5fa5a5e41e7b3cf5c733af62af9c60271e7d22c266aca90f2af67f91ffe80a3ed8b8d1a91d001700870f493207998bac988c4e3671a3a0dba7ba7
f51665bee7 psbt: validate pubkeys in MuSig2 pubnonce/partial sig deserialization (tboy1337)
Pull request description:
The previous fix for invalid MuSig2 pubkeys (bitcoin/bitcoin#34010) only
addressed the PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS field. However, the
PSBT_IN_MUSIG2_PUB_NONCE and PSBT_IN_MUSIG2_PARTIAL_SIG fields also
deserialize pubkeys without validation, which could lead to crashes when
invalid pubkeys are processed.
This commit adds validation to the DeserializeMuSig2ParticipantDataIdentifier
function to ensure all pubkeys in MuSig2 pubnonce and partial signature
fields are fully valid elliptic curve points.
The fix:
- Validates both aggregate and participant pubkeys in MuSig2 pubnonce and
partial signature deserialization
- Throws std::ios_base::failure with descriptive error messages for invalid
pubkeys
- Prevents potential crashes from invalid elliptic curve points
- Maintains backward compatibility for valid PSBTs
This completes the fix for issues [#33999](https://github.com/bitcoin/bitcoin/issues/33999) and [#34201](https://github.com/bitcoin/bitcoin/issues/34201).
ACKs for top commit:
rkrux:
lgtm ACK f51665bee7
w0xlt:
ACK f51665bee7
darosior:
utACK f51665bee7
Tree-SHA512: 8454d77a05aa003a3121b0a5975e8a000125ee0d62343bfa625a75db113358ba7a210ae0376ca1666957b7de7005e06e5a54c95170430ee5e9e1416719b8af53
1c1de334e9 test: avoid interface_ipc.py race and null pointer dereference (Ryan Ofsky)
Pull request description:
Avoid race condition in `run_deprecated_mining_test` caused by creating and immediately destroying an unused worker thread. This is leading to test failures reported by maflcko in #34711
ACKs for top commit:
optout21:
utACK 1c1de334e9
l0rinc:
Tested ACK 1c1de334e9
w0xlt:
ACK 1c1de334e9
enirox001:
ACK 1c1de33
Tree-SHA512: d0af9676a46e991a3f4fda3795c02d1998d30de24991436b8ada425585c6699ff32a7057ca7a0ef2925f782fd3bf73e0381f5d4325e4f1c09f487fce1de49e45
This commit renames AreInputsStandard to ValidateInputsStandardness.
ValidateInputsStandardness now returns valid TxValidationState if all inputs
(scriptSigs) use only standard transaction forms else returns invalid
TxValidationState which states why an input is not standard.
Avoid race condition in run_deprecated_mining_test caused by creating and
immediately destroying an unused worker thread. This leads to test failures
reported by maflcko in #34711
da7f70a532 test: use port 0 for I2P addresses in p2p_private_broadcast.py (Vasil Dimov)
a8ebcfd34c test: let connections happen in any order in p2p_private_broadcast.py (Vasil Dimov)
67696b207f net: extend log message to include attempted connection type (Vasil Dimov)
Pull request description:
If the following two events happen:
* (likely) the automatic 10 initial connections are not made to all
networks
* (unlikely) the network-specific logic kicks in almost immediately.
It is using exponential distribution with a mean of 5 minutes
(`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).
So if both happen, then the 11th connection may not be the expected
private broadcast, but a network-specific connection.
Fix this by retrieving the connection type from
`destinations_factory()`. This is more flexible because it allows
connections to happen in any order and does not break if e.g. the 11th
connection is not the expected first private broadcast.
This also makes the test run faster:
before: 19-44 sec
now: 10-25 sec
because for example there is no need to wait for the initial 10
automatic outbound connections to be made in order to proceed.
Fixes: https://github.com/bitcoin/bitcoin/issues/34387
ACKs for top commit:
achow101:
ACK da7f70a532
andrewtoth:
ACK da7f70a532
mzumsande:
Code Review ACK da7f70a532
Tree-SHA512: 7c293e59c15c148a438e0119343b05eb278205640658c99336d4caf4848c5bae92b48e15f325fa616cbc9d5f394649abfa02406a76e802cffbd3d312a22a6885
Reduce the number of blocks that need to be generated before pruning
the blockchain.
Unload the wallet that was restored in a prior test because it is not
needed anymore after the test.
Both the above steps should reduce the number of chain notifications
that need to be processed by the wallet(s) when an erroneous scenario
of restoring wallet is checked.
fa48f8c865 test: Add missing timeout_factor to zmq socket (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/34189
Otherwise, the test may intermittently fail on slow CI systems that have `--timeout-factor=` properly set.
It can be tested by running `./bld-cmake/test/functional/interface_zmq.py --timeout-factor=10` with this diff:
```diff
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index c7be6abc3a..b14cf2aee6 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -166,2 +166,3 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
LOG_EVENT(fmt, local_name, __VA_ARGS__); \
+ UninterruptibleSleep(45ms); \
event(); \
diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py
index 6717007626..eee377daea 100755
--- a/test/functional/interface_zmq.py
+++ b/test/functional/interface_zmq.py
@@ -176,3 +176,3 @@ class ZMQTest (BitcoinTestFramework):
for sub in subscribers:
- sub.socket.set(zmq.RCVTIMEO, recv_timeout*1000)
+ sub.socket.set(zmq.RCVTIMEO, int(recv_timeout * 1000))
@@ -271,3 +271,3 @@ class ZMQTest (BitcoinTestFramework):
[(topic, address) for topic in ["hashblock", "hashtx"]],
- recv_timeout=2) # 2 second timeout to check end of notifications
+ recv_timeout=0.2) # 2 second timeout to check end of notifications
self.disconnect_nodes(0, 1)
```
Before this pull: Test fails with `zmq.error.Again: Resource temporarily unavailable`
After this pull: Test passes
ACKs for top commit:
l0rinc:
code review ACK fa48f8c865
achow101:
ACK fa48f8c865
Tree-SHA512: 5ff8bdd807ff4b644daa634bb7b469da3da3915f58afba63a90e662df99cbebc86636e34e2b1b313c8629773aef2a239fb3025226a84d2ec22f6ecd4cea666c4
bbc8f1e0a7 ipc mining: Prevent ``Assertion `m_node.chainman' failed`` errors on early startup (Ryan Ofsky)
a7cabf92e4 init refactor: Only initialize node.notifications one time (Ryan Ofsky)
c8e332cb33 init refactor: Remove node.init accesss in AppInitInterfaces (Ryan Ofsky)
Pull request description:
This fixes ``Assertion `m_node.chainman' failed`` errors first reported https://github.com/bitcoin/bitcoin/issues/33994#issuecomment-3602551596 when IPC mining methods are called before ChainstateManager is loaded.
The fix works by making the `Init.makeMining` method wait until chainstate data is loaded. It's probably the simplest possible fix but other alternatives like moving the wait to `Mining.createNewBlock` were discussed in the thread https://github.com/bitcoin/bitcoin/pull/34661#discussion_r2848176298 and could be implemented later without changes to clients.
ACKs for top commit:
Sjors:
utACK bbc8f1e0a7
ismaelsadeeq:
ACK bbc8f1e0a7
achow101:
ACK bbc8f1e0a7
Tree-SHA512: 3e2e4e28ccff364b2303efd06ce337a229c28609076638500acb29559f716a15ad99409c6970ce9ad91776d53e3f9d959f1bbbd144ea9a4a2fb578ddbf2da267
The wallet doesn't only rebroadcast transactions it created, but
also relevant transactions received via p2p. Since this is not
self-evident, add test coverage for it.
