Gets rid of the Dummy class and adds coverage of get_socks_cb.
Also explicitly handles an exception case within Torcontrol rather than
relying on a guard in the fuzz test.
This simplifies the implementation and eliminates an unnecessary shared_ptr.
Suggested by Marco Falke.
<Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
These were necessary to work around unnecessary constraints that have been
fixed in the (upcoming) boost::signals2 version 1.91.
Our implementation's constraints match those of that version.
These tests are compatible with boost::signals2 as well as the replacement
implementation that will be introduced in the next commit.
This is intended to demonstrate some equivalency between the implementations.
Prior to this commit, decodepsbt_inputs would call TxDoc during initialization
which lives in another TLU. TxDoc relies on CURRENCY_UNIT to be initialized when
it may not have been (note this is different from the TLU containing
decodepsbt_inputs which also has a CURRENCY_UNIT). Fix this by lazy initializing
decodepsbt_inputs.
Prevent the issue from occurring in the future by also doing the same for
decodepsbt_outputs and getblock_vin.
This was introduced for distros shipping older make, such as
Ubuntu 20.04 (4.2.1). It's likely that all distros being used for
Darwin and Qindows cross compilation, are shipping a newer make at
this point.
0587c56091 kernel: Expose context-free block validation (w0xlt)
71f827c3c2 kernel: Expose consensus parameters (`btck_ConsensusParams`) (w0xlt)
Pull request description:
This PR exposes Bitcoin Core’s context‑free block checks to library users via a new C API entry point, `btck_check_block_context_free`.
Callers can validate a block’s structure (size/weight, coinbase rules, per‑tx context‑free checks) and optionally re‑run Proof‑of‑Work and Merkle‑root verification without touching chainstate, the block index, or the UTXO set.
Rationale
Clients embedding the kernel need a pure block sanity check without requiring node state or disk writes (candidate block validation, for example). This API offers that surface in a single call, with optional PoW/Merkle toggles to avoid redundant work when the header has already been validated or when Merkle verification is deferred.
ACKs for top commit:
yuvicc:
re-ACK 0587c56091
achow101:
ACK 0587c56091
sedited:
ACK 0587c56091
Tree-SHA512: 6bd53e4964909335d1f2fee30ff96c95a8dd2c84bcdfe11c50ba369301822e5dea9bbe2376bb6d6b4652875152071eb0446657042b00429f29581da4fcea71a9
Instead this new approach uses a temporary coins db to roll back the
UTXO set.
This new approach also prevents the node from pruning necessary blocks
during dumptxoutset execution by using prune locks.
The mocking functionality of MockableDatabase, MockableBatch, and
MockableCursor was not really being used. These are changed to be
subclasses of their respective SQLite* classes and will use in-memory
SQLite databases so that the tests are more representative of actual
database behavior.
MockableCursor is removed as there are no overrides needed in
SQLiteCursor for the tests.
Instead of directly copying the stored records map when duplicating a
MockableDatabase, use a Cursor to read the records, and a Batch to write
them into the new database. This prepares for using SQLite as the
database backend for MockableDatabase.
43c528aba9 wallet, test: update `gethdkeys` functional test (rkrux)
6e3a0afc2f wallet: fix `gethdkeys` RPC for descriptors with partial xprvs (rkrux)
Pull request description:
Fixes#34378
A non-watch-only wallet allows to import descriptors with partial private keys, eg: a multisig descriptor with one private key and one public key. In case an xpub is imported in any such descriptors whose private key the wallet doesn't have, then the `gethdkeys` RPC throws an unhandled error like below when the private keys are requested.
This fix ensures that such calls are properly handled by conditionally finding the corresponding xprv and the related functional test is accordingly updated.
```
➜ bitcoincli -named gethdkeys private=true
error code: -1
error message:
map::at: key not found
```
ACKs for top commit:
achow101:
ACK 43c528aba9
w0xlt:
ACK 43c528aba9
Eunovo:
Tested ACK 43c528aba9
Tree-SHA512: 106e02ee368a3fa94d116f54f2fa71f9886e4753e331b91ce13a346559d5497ef6cd7ddaa8736fcfe1a7ac427a44d647fb75e523eb72f917fa866adbe0dbad30
fa955af618 lint: Clarify rmtree/remove_all error message with preferred alternatives (MarcoFalke)
Pull request description:
An error message without a suggestion to fix will often lead developers to come up with a creative solution to side-step the grep in the linter (such as using an `auto` alias or other style-change).
Try to avoid this by mentioning a recommended solution.
ACKs for top commit:
davidgumberg:
ACK fa955af618
achow101:
ACK fa955af618
hodlinator:
ACK fa955af618
Tree-SHA512: d5741b2fdb1cfcffc26144c3bb78478e46d7ecb5c3340cbb09584fe1274a3cc8c6872b997e33b8bd2097630d916133a3f5bf41a1d09527e597be9829d6d3e51b
PartiallyDownloadedBlock::InitData() intentionally treats duplicate short IDs in a compact block as READ_STATUS_FAILED.
For an honest peer, block-level short-ID collisions are rare enough that failing the compact block early is preferable to spending CPU scanning the mempool or attempting selective recovery.
Remove the old TODO that suggested requesting both collided transactions, since it since it points at unwanted behavior.
75608547b4 kernel: Remove NONNULL annotation from destroy method (Alexander Wiederin)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/33796#discussion_r2959817508
### Summary
This PR removes the `BITCOINKERNEL_ARG_NONNULL` annotation from the `btck_block_validation_state_destroy` method in the Kernel API.
### Motivation
No other *_destroy function in the Kernel API carries the NONNULL annotation. Following the convention set by free(), destroy functions should accept null pointers.
### Usage:
Before:
```c
btck_BlockValidationState* state = NULL;
btck_block_validation_state_destroy(state); // violates nonnull contract
```
After:
```c
btck_BlockValidationState* state = NULL;
btck_block_validation_state_destroy(state); // well-defined
```
ACKs for top commit:
yuvicc:
lgtm! ACK 75608547b4
kevkevinpal:
ACK [7560854](75608547b4)
achow101:
ACK 75608547b4
janb84:
ACK 75608547b4
theStack:
ACK 75608547b4
w0xlt:
ACK 75608547b4
stickies-v:
ACK 75608547b4
Tree-SHA512: 9577098940f49b8f7fd1fb20afe750e9eaaf9d90d25ec3c4f423de223a1fbd0bca6d520061f576eb3923f18e1f2744b3f60d0d0715dfa3832fea83cb538170c2
9316d96240 test: sock: Enable socket pair tests on Windows (David Gumberg)
Pull request description:
Some `class Sock` tests were previously disabled because Windows lacks [`socketpair(2)`](https://man7.org/linux/man-pages/man2/socketpair.2.html) which is used as a helper function in the socket tests, but is not strictly necessary or related to the `Socket` class under testing. This PR adds a `CreateSocketPair()` helper which creates a sender socket and receiver socket with a TCP connection, enabling these test cases for Windows. This also enables future tests that require more granular control over sockets than what `socketpair()` allows for, like using `setsockopt()` before connecting a socket.
This change is generally an improvement, but is also broken out of a [branch](github.com/davidgumberg/bitcoin/tree/2025-09-02-0xB10C-prefill-rebase) that does compact block prefilling up to the available bytes in the connection's current TCP window (see [delving post](https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052/34)). Creating connected socket pairs is useful for added tests in that branch that validate querying the current TCP window state, and without this change those tests don't run on Windows.
ACKs for top commit:
achow101:
ACK 9316d96240
sedited:
ACK 9316d96240
w0xlt:
reACK 9316d96240
Tree-SHA512: 23ad7070555bb934b0eb18d114e918bd2d50657d20d2221d8c98cd0e558e27e32e5ef89e8074c108a90d7309e539318d23cea43d2ad8eb10e635b114f5f1a87d
This function is already strongly fuzzed by other harness.
E.g: descriptor_parse calls it several times during parsing
and serialization. Also, calling GetDescriptorChecksum with
a string of length 32 is not effective to exercise it.
No other *_destroy function in the Kernel API carries this annotation.
Following the convention set by free(), destroy functions should accept
null pointers.
325f743eed guix: Clean up module list in manifest (Hennadii Stepanov)
Pull request description:
These modules are no longer in use:
1. `cmake-build-system` since bitcoin/bitcoin#34550.
2. `gnu-build-system` since bitcoin/bitcoin#34627.
3. `download` since bitcoin/bitcoin#34550.
4. `cc-for-target` from `utils` since bitcoin/bitcoin#34627.
ACKs for top commit:
janb84:
ACK 325f743eed
Tree-SHA512: 5f32a74cc5c13cd4cd181f6d23fe5198e71a0d8f18378f4a2e38aa7a06eedd6f2c5cfbeea9b8eb50baac4d6c5c5bf6aa4d898143c8f2f9899871c6c2991d1b59
2e041b4905 help: enrich help text for `-loadblock` (Hao Xu)
Pull request description:
`-loadblock` doesn't support XOR-ed files, mention it in its help text to avoid troubles for users.
ACKs for top commit:
maflcko:
lgtm ACK 2e041b4905
sedited:
ACK 2e041b4905
Tree-SHA512: 1883824c15687799d60ddabc53d5dc2c274e56c8292da5e7e412adaef65abf70c7f778ef7675c65f738c1e4855e5b0f929e87631b0a52695873e58f8245b3a8d
98fcd7af23 wallet: rpc: Improve error message for low feerates. (David Gumberg)
Pull request description:
Giving the user a hint about what action to take when they encounter an error related to a feerate that is below the minimum. I encountered this myself not knowing about `-mintxfee` and the manpage was slightly less than clear,
```
-mintxfee=<amt>
Fee rates (in BTC/kvB) smaller than this are considered zero fee for transaction creation
(default: 0.00001)
```
ACKs for top commit:
achow101:
ACK 98fcd7af23
ismaelsadeeq:
ACK 98fcd7af23
w0xlt:
ACK 98fcd7af23
Tree-SHA512: 8471bfd5682ca59137541ab4da670e85947cf7812baa9e07d816e352fccfb5e0e71a06c2baf1a5422d78a7da8c08b9fcbb73d85cc9daba88b7c22e802abe5b2e
5a81d73a81 scripted-diff: rpc: Don't pointlessly capture in RPCMethod lambdas (Anthony Towns)
4e789299af scripted-diff: rpc: Rename RPCHelpMan to RPCMethod (Anthony Towns)
Pull request description:
When defining `RPCHelpMan` objects, we usually return a lambda, and mostly we define those via `[&](...) { ... }` which explicitly captures any parameters or local variables by reference. If we were to actually use any of those captures (we don't), we would invoke undefined behaviour. So instead, convert all the `[&]` to `[]` to avoid capturing.
While we're at it, rename `RPCHelpMan` to `RPCMethod`, reflecting its greater responsibility since #19386.
ACKs for top commit:
maflcko:
review ACK 5a81d73a81🏣
stickies-v:
ACK 5a81d73a81
rkrux:
code review ACK 5a81d73a81
Tree-SHA512: 72e9232857ba5bf4c346fb963a2028047f7c7e9b420ef58b3108669204bfbb6952342cfcfd2e8a06f813a21545abf7fc8b0ad422f00d7ec9c1134626cd1650e6
12c3c3f81d test: mining: add coverage for GBT's "coinbasevalue" result field (Sebastian Falbesoner)
Pull request description:
This PR adds missing coverage for the "coinbasevalue" result field of the `getblocktemplate` RPC call. Specifically, the introduced test checks that the value is set to claim the full block reward (subsidy plus fees). Can be verified with the following patch, which succeeds on master and fails on the PR branch:
```diff
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index a935810d91..ba9ac9dadb 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -998,7 +998,7 @@ static RPCHelpMan getblocktemplate()
result.pushKV("previousblockhash", block.hashPrevBlock.GetHex());
result.pushKV("transactions", std::move(transactions));
result.pushKV("coinbaseaux", std::move(aux));
- result.pushKV("coinbasevalue", block.vtx[0]->vout[0].nValue);
+ result.pushKV("coinbasevalue", block.vtx[0]->vout[0].nValue - 1);
result.pushKV("longpollid", tip.GetHex() + ToString(nTransactionsUpdatedLast));
result.pushKV("target", hashTarget.GetHex());
result.pushKV("mintime", GetMinimumTime(pindexPrev, consensusParams.DifficultyAdjustmentInterval()));
```
I'm not sure how relevant this field is nowadays in real-world mining scenarios (we use it for the signet miner at least), but it seems useful to have a test for it anyways. Stumbled upon this while looking at https://github.com/bitcoin-inquisition/bitcoin/pull/111.
ACKs for top commit:
maflcko:
lgtm ACK 12c3c3f81d
Sjors:
ACK 12c3c3f81d
Tree-SHA512: ae10f63c99b21cf7771feefe3d0e47b2e0d511aceb344cf11efa9182d78f2960f1e079797b8a36db110a3c54acb27a3706413a710a74bf56aed29ea162a6aab2
5fa6898818 policy: remove incorrect MANDATORY_SCRIPT_VERIFY_FLAGS comment (ismaelsadeeq)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/33050
This comment claiming that failing mandatory script checks may trigger a DoS ban/punishment is false.
`CheckInputScript` also dispute this comment.
b0f68f0a3a/src/validation.cpp (L2113-L2118)
ACKs for top commit:
darosior:
utACK 5fa6898818
instagibbs:
ACK 5fa6898818
Tree-SHA512: 84ad68b158e334b0e945f33007de58807b0b3335b1873412b0f359f0fa52d0819e93f42b28ebb6fbe5b355f2710f61e0dd731dc4bce20ccc1b7b2c546a9df550
fabab69e9e refactor: Return std::optional from ParseDouble (MarcoFalke)
fa0a09441d refactor: Return std::optional from GetWalletNameFromJSONRPCRequest (MarcoFalke)
fafb0c4cbe refactor: Return std::optional from GetLogCategory (MarcoFalke)
Pull request description:
Using a bool to indicate whether a mutable in-out param was written to is fine in legacy code, but otherwise confusing and brittle:
* Sometimes the in-out-param is written to, even when the function returns `false`, like in `ParseDouble`.
* Call sites must manually check the return value
Fix those issues by returning `std::optional<_>` from `ParseDouble` (and a few other functions).
This refactor is a style cleanup and does not change any behavior.
ACKs for top commit:
stickies-v:
re-ACK fabab69e9e
hodlinator:
crACK fabab69e9e
rkrux:
code review ACK fabab69e9e
Tree-SHA512: e27a27174e9d2200da8b0ca9b6cc056e94d2b25e6332975f1ad660ee85b02680a65ac93b2ed29f10da0ae5f6dc8395bddc9e973a3f925c68a0c8116c9282ea09
The two fields represent a time point, not a duration. Also, it is
unclear why they use second precision.
Fix both issues by using NodeClock::time_point.
This refactor should not change any behavior.
This resolves the two temporary calls to time_since_epoch() added in the
previous commit. However, it adds one new call to time_since_epoch(),
which is resolved in the next commit.
