GetPrivKey() needs the same handling of all keyids for xonly keys that
ToPrivateString() does. Refactor that into GetPrivKey() and reuse it in
ToPrivateString() to resolve this.
fa397177ac util: Add missing types in make_secure_unique (MarcoFalke)
Pull request description:
The return type of `std::forward` depends on the template type, and can not be recovered from the args. Attempting to do so will result in a compile failure. For example, `make_secure_unique<std::string>(std::string{});` does not compile on current master, but does with this pull.
Another example would be `make_secure_unique<std::pair<std::string, std::unique_ptr<int>>>(std::string{}, std::make_unique<int>(21));`
ACKs for top commit:
hodlinator:
ACK fa397177ac
hebasto:
ACK fa397177ac.
TheCharlatan:
ACK fa397177ac
Tree-SHA512: cc902c1111c929a79a6f806b5097136a465e8c727474176bad30a5777ebbb30bedb0bd35273b43bf839d2c00492500ddec724bd17349250451f6b329cb71e6f2
Now that we track all announcers of an orphan, it's not helpful to
consider an orphan provided by a peer that didn't send us this parent.
It can only hurt our chances of finding the right orphan when there are
multiple candidates.
Adapt the 2 tests in p2p_opportunistic_1p1c.py that looked at 1p1c
packages from different peers. Instead of checking that the right peer
is punished, we now check that the package is not submitted. We can't
use the functional test to see that the package was not considered
because the behavior is indistinguishable (except for the logs).
This means we no longer return parents we already have in the
m_unique_parents result from MempoolRejectedTx.
We need to separate the loop that checks AlreadyHave parents from the
loop that adds parents as announcements, because we may do the latter
loop multiple times for different peers.
Add ability to add and track multiple announcers per orphan transaction,
erasing announcers but not the entire orphan.
The tx creation code in orphanage_tests needs to be updated so that each
tx is unique, because the CountOrphans() check assumes that calling
EraseForPeer necessarily means its orphans are deleted.
Unused for now.
Needed for a later commit adding logic to ask the TxRequestTracker for a
list of announcers. These announcers should know the parents of the
transaction they announced.
e8f0e6efaf lint: output-only - Avoid repeated arrows, trim (Hodlinator)
fa9aacf614 lint: Move assertion linter into lint runner (MarcoFalke)
Pull request description:
On failure, this makes the output more consistent with the other linters. Each failure will be marked with an '⚠️ ' emoji and explanation, making it easier to spot.
Also, add --line-number to the filesystem linter.
Also, add newlines after each failing check, to visually separate different failures from each other.
Can be reviewed with:
`--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
ACKs for top commit:
davidgumberg:
crACK e8f0e6efaf
hodlinator:
re-ACK e8f0e6efaf
TheCharlatan:
ACK e8f0e6efaf
Tree-SHA512: 9896ff882af9d673ec3e6d2718f877b2fdc8514faba50942fcebacb9de95b1f5b4a5db595e1338fa7f505d06df2df304897350cc55c558c7a85232800e5fd804
0a76c292ac doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py` (Hennadii Stepanov)
Pull request description:
On FreeBSD, Python's `zmq` module is provided as a separate port.
This PR updates the FreeBSD Build Guide to include this port, enabling the `interface_zmq.py` functional test.
ACKs for top commit:
maflcko:
lgtm ACK 0a76c292ac
vasild:
ACK 0a76c292ac
Tree-SHA512: c13eada3e870149f47348145d6a29f41125ac75efd88eabe6dd2d0429e0377ed280e76a764cfaf627498c1d07b9135a995cc644146fa666bc3bfa0eb2c86e88b
fa0411ee30 ci: Run functional tests in msan task (MarcoFalke)
Pull request description:
Now that the CI machines have a bit more CPU, it seems good to run the functional tests as well under msan. (Also, bump the llvm minor version)
ACKs for top commit:
TheCharlatan:
ACK fa0411ee30
Tree-SHA512: 0dbb2b934485ed54b8caafb5bcd96ddef87088b148dab72a584f721c398bb7fda4095fb720b9ad602dc71f8f40a1e0f29e1b08b2879b78b90b29d46604df36c3
29bca9713d test: fix typo in mempool_ephemeral_dust (epysqyli)
Pull request description:
The `test_node_restart` test in `test/functional/mempool_ephemeral_dust.py` has a repetition in the comment.
ACKs for top commit:
maflcko:
lgtm ACK 29bca9713d
Tree-SHA512: 9828d23ca27e24d64031cd103ce9f9bd9e997ef9b63e6122ad6573073fb3c956964a72cd23dfa5773e52e195eee668762ab470bb540e686a4abd3d7561b40c59
faf7eac364 test: clang-format -i src/univalue/test/unitester.cpp (MarcoFalke)
fafa9cc7a5 test: Embed univalue json tests in binary (MarcoFalke)
fa044857ca test: Re-enable univalue test fail18.json (MarcoFalke)
63b6b638aa build: Use character literals for generated headers to avoid narrowing (Lőrinc)
Pull request description:
All other benchmarks and tests have their data embedded, except for the univalue json tests.
This is not only confusing, but also problematic, when the test binary is moved to a different system for testing, because one has to put the test files in the source dir that was used at compile-time.
Fix all issues by embedding them. Also, re-enable a disabled test. Also, fix an issue in the GenerateHeaderFromJson.cmake.
Requested in https://github.com/bitcoin/bitcoin/pull/31434/files#r1876000910
ACKs for top commit:
l0rinc:
ACK faf7eac364
fjahr:
tACK faf7eac364
achow101:
ACK faf7eac364
TheCharlatan:
Re-ACK faf7eac364
hebasto:
Re-ACK faf7eac364. The commit, which modifies CMake scripts, has been replaced with the one from https://github.com/bitcoin/bitcoin/pull/31547, and a formatting commit has been added since my recent [review](https://github.com/bitcoin/bitcoin/pull/31542#pullrequestreview-2517189261).
Tree-SHA512: 72ad202125746f32ccf07411ad3efd2771f27a40525c204cba3c9c83b3ca46d05dd18f6fa5985720c6684bdcbb4c4853fc609ced095ddd1a124832318dd8a55d
Verify that the DescriptorSPKM method `GetSigningProvider` should
only return a signing provider for the passed public key if its
corresponding private key of the passed public key is available.
It is always applied in the same way, no matter how the txindex is
setup. This was no longer accurate after 8181db8, where their
initialization was made independent.
fa83bec78e refactor: Allow std::byte in Read(LE/BE) (MarcoFalke)
Pull request description:
Starting with C++17, `std::byte` is often (not always) a better choice over `uint8_t` for new code.
However, the existing codebase discourages the use of `std::byte`, when helpers such as `ReadLE32` are used. This is because calling code will be cluttered with byte-casts.
Fix it by allowing `std::byte` pointers in `ReadLE32` (and friends).
ACKs for top commit:
sipa:
utACK fa83bec78e
fjahr:
Code review ACK fa83bec78e
theuni:
utACK fa83bec78e
l0rinc:
ACK fa83bec78e
Tree-SHA512: 83604dc9df9ad447ad1b6f81f1e1844554c2c5331fcb78bdba1300e050d9dcbe9cf7a1b2dd250772bb23a8bf02a4ec26441012fe2f4bcc670ef31c15151adb15
This is a documentation-only change following up on suggestions made in the
#30526 review.
Motivation for this change is that I was recently reviewing #31583, which
reminded me how confusing the arithmetic blob code was and made me want to
write better comments.
b6f0593f43 doc: add release note about testmempoolaccept debug-message (Matthew Zipkin)
f9cac63523 test: cover testmempoolaccept debug-message in RBF test (Matthew Zipkin)
f9650e18ea rbf: remove unecessary newline at end of error string (Matthew Zipkin)
221c789e91 rpc: include verbose reject-details field in testmempoolaccept response (Matthew Zipkin)
Pull request description:
Adds a new field `reject-details` in `testmempoolaccept` responses to include `m_debug_message` from `ValidationState`. This string is the complete error message thrown by the mempool in response to `sendrawtransaction`.
The extra verbosity is helpful to consumers of `testmempoolaccept`, which is sort of a debug tool anyway.
example:
>
> {
> "txid": "07d7a59a7bdad4c3a5070659ea04147c9b755ad9e173c52b6a38e017abf0f5b8",
> "wtxid": "5dc243b1b92ee2f5a43134eb3e23449be03d1abb3d7f3c03c836ed0f13c50185",
> "allowed": false,
> "reject-reason": "insufficient fee",
> "reject-details": "insufficient fee, rejecting replacement 07d7a59a7bdad4c3a5070659ea04147c9b755ad9e173c52b6a38e017abf0f5b8; new feerate 0.00300000 BTC/kvB <= old feerate 0.00300000 BTC/kvB"
> }
ACKs for top commit:
rkrux:
re-ACK b6f0593f43
glozow:
ACK b6f0593f43
Tree-SHA512: 340b8023d59cefa84598879c4efdb7c399a3f62da126e87c595523f302e53d33098fc69da9c5f8c92b7580dc75466c66cea372051f935b197265648fe15c43a3
1. Update the documented NetBSD version.
2. Add the optional ZeroMQ package to align the guide with other *BSD
systems.
3. Update the Python version to meet the minimum requirement specified
in https://github.com/bitcoin/bitcoin/pull/30527.
4. Install `net/py-zmq` package to enable the `interface_zmq.py`
functional test.
5. Fix a formatting issue.
366ae00b77 descriptor: Assume `ParseScript` is not being called with a P2WPKH context (brunoerg)
e366408590 descriptor: remove unreachable verification for `pkh` (brunoerg)
Pull request description:
This PR removes an unreachable verification in the `ParseScript` function. It returns an error if `pkh` is not being used at top level, sh, wsh or tr. However, any usage of `pkh` without these contexts will not reach this verification but other ones like "invalid keys" (e.g. `wpkh(pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd))`).
ACKs for top commit:
davidgumberg:
crACK 366ae00b77
achow101:
ACK 366ae00b77
tdb3:
cr ACK 366ae00b77
sipa:
crACK 366ae00b77
Tree-SHA512: b954221a77eed623aeed5eb54f14e82c49540a151d3388831924caa7a784e48a2a975e418af1e13d491e4f8cded3b1797aa39e0e4e39e302a991105df09cdec0
b29d68f942 test: descriptor: fix test for `MaxSatisfactionWeight` (brunoerg)
Pull request description:
To get the maximum size of a satisfaction for a descriptor with no max sig, the parameter `use_max_sig` should be false.
ACKs for top commit:
fjahr:
utACK b29d68f942
achow101:
ACK b29d68f942
tdb3:
re ACK b29d68f942
furszy:
utACK b29d68f942
Tree-SHA512: 8559718d126e60ce21a34183f74d227546108b43e3897e49622d6677ed9e7707caa962fd811d8787bd4dafc48a0e779ef11050d5990293faa2f91ded4aaa4f4b
fa63b8232f test: generateblocks called by multiple threads (MarcoFalke)
fa62c8b1f0 rpc: Extend scope of validation mutex in generateblock (MarcoFalke)
Pull request description:
The mutex (required by TestBlockValidity) must be held after creating the block, until TestBlockValidity is called. Otherwise, it is possible that the chain advances in the meantime and leads to a crash in TestBlockValidity: `Assertion failed: pindexPrev && pindexPrev == chainstate.m_chain.Tip() (validation.cpp: TestBlockValidity: 4338)`
Fixes #31562
ACKs for top commit:
davidgumberg:
reACK fa63b8232f
achow101:
ACK fa63b8232f
ismaelsadeeq:
re-ACK fa63b8232f
mzumsande:
utACK fa63b8232f
Tree-SHA512: 3dfda1192af52546ab11fbffe44af8713073763863f4a63fbcdbdf95b1c6cbeb003dc4b8b29e7ec67362238ad15e07d8f6855832a0c68dc5370254f8cbf9445c
bc43ecaf6d test: add functional test for balance after snapshot completion (Martin Zumsande)
226d03dd61 validation: Send correct notification during snapshot completion (Martin Zumsande)
Pull request description:
After AssumeUtxo background sync is completed in a `ActivateBestChain()` call, the `GetRole()` function called with `BlockConnected()` returns `ChainstateRole::NORMAL` instead of `ChainstateRole::BACKGROUND` for this chainstate.
This would make the wallet (which ignores `BlockConnected` notifications for the background chainstate) process it, change `m_last_block_processed_height` to the (ancient) snapshot height, and display an incorrect balance.
Fix this by caching the chainstate role before calling `ActivateBestChainStep()`.
Also contains a test for this situation that fails on master.
Fixes #31546
ACKs for top commit:
fjahr:
re-ACK bc43ecaf6d
achow101:
ACK bc43ecaf6d
furszy:
Code review ACK bc43ecaf6d
TheCharlatan:
lgtm ACK bc43ecaf6d
Tree-SHA512: c5db677cf3fbab3a33ec127ec6c27c8812299e8368fd3c986bc34d0e515c4eb256f6104479f27829eefc098197de3af75d64ddca636b6b612900a0e21243e4f2
