1e0de7a6ba fees: document non-monotonic estimation edge case (willcl-ark)
Pull request description:
Closes: https://github.com/bitcoin/bitcoin/issues/11800
In scenarios where data is available for higher targets but not for lower ones, this method *may* return lower fee rates for higher confirmation targets. This could occur if `estimateCombinedFee` returns no valid data (`-1`) for some estimates for a low target, but **does** return valid data for a higher target.
Users of this function should be aware of this potential, if unlikely, inconsistency in behaviour in data-sparse scenarios.
ACKs for top commit:
adamandrews1:
Code review ACK 1e0de7a
ismaelsadeeq:
Code review ACK 1e0de7a6ba
glozow:
ACK 1e0de7a6ba
Tree-SHA512: 161e5dafdd131570853a89491753ae39a7b725d1a86cab5a7294c2a5939da1a9a5f2c4aca0900e9ad810e828b6e0e636f256384e3d1fda6dd552da189bbbe747
0750249289 mining: document gbt_rule_value helper (Sjors Provoost)
5e87c3ec09 scripted-diff: rename gbt_force and gbt_force_name (Sjors Provoost)
Pull request description:
The term "force" is ambiguous and not used in [BIP9](https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate-changes) where there ! rule prefix is introduced.
E.g. this code is hard to read:
```cpp
if (!gbt_force) {
s.insert(s.begin(), '!');
```
Additionally, #29039 renamed `gbt_vb_name` to `gbt_force_name` which, at least for me, further increased the confusion.
This is a pure (variable rename) refactor (plus documentation) and does not change behavior.
Reminder of how to verify a scripted diff:
```sh
test/lint/commit-script-check.sh origin/master..HEAD
```
ACKs for top commit:
achow101:
ACK 0750249289
janb84:
ACK [0750249](0750249289)
musaHaruna:
ACK [0750249](0750249289)
glozow:
ACK 0750249289, seems sensible
Tree-SHA512: 8c88a273a3b36040f6c641843bd20579d0065b051aad4b39fc14f0d2af2808690dff6772bd8b1a4d9699b72279a700d2661012651bc315433a123dcc8996adaa
2aa63d511a test: Use uninvolved pruned node in feature_pruning undo test (enoch)
772ba7f9ce test: Fix nTimes typo in feature_pruning test (enoch)
Pull request description:
This PR contains two commits:
1. Fixes a typo in feature_pruning.py where 'nTimes' was incorrectly
used instead of 'nTime'. This typo caused the test to always reset
mine_large_blocks.nTime to 0, rather than only on the first run.
2. Fixes the test failure exposed by the typo fix. The
test_pruneheight_undo_presence test was failing because it was using
node 2, which is involved in reorg testing and could be on a
different chain than other nodes. The solution switches to using
node 5, which is also a pruned node but isn't involved in reorg
testing.
Testing:
- Ran test/functional/feature_pruning.py multiple times to verify
consistent passing
- Verified that the test now passes with the correct nTime variable name
- Confirmed the test behavior matches the intended functionality of
verifying pruned block availability
- Ran the full test suite to ensure the changes did not introduce any
regressions or affect other tests
Thanks to fjahr for his assistance in diagnosing the issue and
suggesting the solution.
This fixes the test failure reported in #32249
ACKs for top commit:
fjahr:
tACK 2aa63d511a
maflcko:
lgtm ACK 2aa63d511a
naiyoma:
tACK 2aa63d511a
stratospher:
tested ACK 2aa63d5. verified that `nTime` is being incremented now.
Tree-SHA512: a543528fd4eeb30e978c0b43cfa109768252edaf1f94679dbbc7fe684122c00da34224e2cc1abd2a265af1b267eef1cd34246207946cf7d8e93d2c0f11aa56d8
8673e8f019 txgraph: Special-case singletons in chunk index (optimization) (Pieter Wuille)
abdd9d35a3 txgraph: Skipping end of cluster has no impact (optimization) (Pieter Wuille)
604acc2c28 txgraph: Reuse discarded chunkindex entries (optimization) (Pieter Wuille)
c734081454 txgraph: Introduce TxGraph::GetWorstMainChunk (feature) (Pieter Wuille)
394dbe2142 txgraph: Introduce BlockBuilder interface (feature) (Pieter Wuille)
883df3648e txgraph: Generalize GetClusterRefs to support subsections (preparation) (Pieter Wuille)
c28a602e00 txgraph: Introduce TxGraphImpl observer tracking (preparation) (Pieter Wuille)
9095d8ac1c txgraph: Maintain chunk index (preparation) (Pieter Wuille)
87e74e1242 txgraph: abstract out transaction ordering (refactor) (Pieter Wuille)
2614fea17f txgraph: Add GetMainStagingDiagrams function (feature) (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289.
This adds more functionality to the txgraph module, specifically:
* `TxGraph::GetMainStagingDiagrams()`, a function to obtain feerate diagrams for both the main graph and the staged changes to it, including only the clusters that differ between the two.
* `TxGraph::GetBlockBuilder()`, a function to obtain an object which can efficiently iterate the chunks of the (main) graph from high to low chunk feerate, allowing each to be skipped or included.
* `TxGraph::GetWorstMainChunk()`, a function to obtain the last chunk that would be returned by `GetBlockBuilder()`'s returned object, intended for eviction.
ACKs for top commit:
monlovesmango:
reACK 8673e8f019
instagibbs:
reACK 8673e8f019
glozow:
reACK 8673e8f019
Tree-SHA512: 5c98c54919c44eb2f9545dfc130e54dfc25b5b54d43cf5ca9bcf46e019b9fd405a572fcd70e71e2a7c5b4b096cfd540a4d09ef1f52ba188504418682f1dfc4af
During init, the test framework will start using rpc after the
mempool was loaded. It will not wait for postInitProcess or
outstanding transactionAddedToMempool notifications, leading to
a possible race, in which listunspent is being called while the
tx is still in Inactive status. Prevent this by processing
outstanding notifications.
There could be a race with outstanding TxAddedToMempool notifications
being applied to the soon-to-be created wallet.
Fixes an intermittent timeout reproducable by adding a sleep to
AddToWallet.
- Create a new function `AddMerkleRootAndCoinbase` that compute the
block's merkle root, insert the coinbase transaction and the merkle
root into the block.
This interface lets one iterate efficiently over the chunks of the main
graph in a TxGraph, in the same order as CompareMainOrder. Each chunk
can be marked as "included" or "skipped" (and in the latter case,
dependent chunks will be skipped).
This is preparation for a next commit which will introduce a class whose
objects hold references to internals in TxGraphImpl, which disallows
modifications to the graph while such objects exist.
415650cea9 guix: move *-check.py scripts under contrib/guix (fanquake)
Pull request description:
These scripts are not meant for general developer usage. They are for use on the release binaries, which have been compiled in an environment that makes various assumptions in regards to c library, compiler options, hardening options, dependency patching etc.
Anyone is free to run these scripts against self-compiled binaries, but this isn't something we want to modify/generalize the scripts to support.
ACKs for top commit:
laanwj:
Code review ACK 415650cea9
TheCharlatan:
ACK 415650cea9
Tree-SHA512: 469cc5da9df014b4a9dc39080f31c3283641691209497cdb7cc5dc4767681a7f4b67425768e81838153ce39a26547ed94cd3be4dc27352d33960765bfac9d0e5
Support new BITCOIN_CMD environment variable in functional test to be able to
test the new bitcoin wrapper executable and run other commands through it
instead of calling them directly.
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
Intended to make bitcoin command line features more discoverable and allow
installing new multiprocess binaries in libexec/ instead of bin/ so they don't
cause confusion.
Idea and implementation of this were discussed in
https://github.com/bitcoin/bitcoin/issues/30983
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
These functions are just meant to serve the needs of the bitcoin wrapper
executable, and are intentionally not very general purpose so they can be
simple.
d847e17c96 doc: Fix typo (Hennadii Stepanov)
Pull request description:
A translator on Transifex noticed:
> This is the only label which has two dots: ..
> Usually we see the elipsis (…)
This PR addresses this issue.
ACKs for top commit:
janb84:
good find, ACK d847e17c96
Tree-SHA512: cbdc5f283fe30655a923c34813c97406bba85657554984c26acbfed585ce3a652bcb21348d7e0f6fd9c82ca04111c412ff62c7d4b046869d13785b713e69ab6d
5b7ed460c7 cmake: Allow `WITH_DBUS` on all Unix-like systems (Hennadii Stepanov)
Pull request description:
This PR makes the `WITH_DBUS` option available on all Unix-like systems, not just Linux, thereby fixing a regression that was overlooked during the migration from Autotools.
Note: Enabling D-Bus support on macOS still makes no sense, since the `Notificator` class uses the User Notification Center regardless:746ab19d5a/src/qt/notificator.cpp (L43-L56)
Fixes https://github.com/bitcoin/bitcoin/issues/32464.
ACKs for top commit:
laanwj:
ACK 5b7ed460c7
theStack:
tACK 5b7ed460c7
Tree-SHA512: 01f5a365a8109c273489e2d46b08d8b64b82251c4814a486561221b55d8a47cb1effc9140dbd6b2ab5f12e2966d0ea0d07a4cbf67d310001ee681b0f5b63f300
3bbdbc0a5e qt, docs: Unify term "clipboard" (Hennadii Stepanov)
Pull request description:
A translator on Transifex noticed:
> The term "system clipboard" appears twice. The term "clipboard" appears 10 times. Perhaps we could standardize on just saying "clipboard"?
This PR addresses this issue.
ACKs for top commit:
davidgumberg:
ACK 3bbdbc0a5e
pablomartin4btc:
ACK 3bbdbc0a5e
Tree-SHA512: 61a100f60890d81122a4b8ce3e2cb7d355c7fb643de3196573f7f9107c6f52fa0b3e7a4f743ce2833e8c67b9cdad3568b761d730fef5c9781f5e1c45252888c4
002b792b9a gui: decouple WalletModel from RPCExecutor (furszy)
Pull request description:
A more comprehensive fix for the issue described in #837.
Since the `WalletModel` class is unavailable when compiling without wallet support
`(-DENABLE_WALLET=0)`, the RPC executor class should not be coupled to it.
This decoupling ensures GUI compatibility with builds that omit wallet support.
This also drops an extra `#ifdef ENABLE_WALLET` block which is always good.
ACKs for top commit:
w0xlt:
Code Review ACK 002b792b9a
pablomartin4btc:
tACK 002b792b9a
BrandonOdiwuor:
tACK 002b792b9a
hebasto:
ACK 002b792b9a, I have reviewed the code and it looks OK.
Tree-SHA512: a8e6b7e9d88dd8e0ff5e2d0de91be2f85fd0559265267d3bf6cae5a37606cf1ab6bc7415d5817a11006008de362f2ca3557ba772b4e1bd9fbef5f564be3b53bb
This change makes the `WITH_DBUS` option available on all Unix-like
systems, not just Linux, thereby fixing a regression that was
overlooked during the migration from Autotools.
Note: Enabling D-Bus support on macOS still makes no sense, since the
`Notificator` class uses the User Notification Center regardless.
ab878a7e74 build: simplify *ifaddr handling (fanquake)
Pull request description:
We really just want to skip this when building for Windows. So do that,
and remove the two header checks (we also already use both of these
headers, unguarded, in the !windows part of the codebase).
Squash the two *iffaddrs defines into one, as I haven't seen an
`iffaddrs.h` that implements one, but not the other.
ACKs for top commit:
hebasto:
ACK ab878a7e74. Only addressed my [comment](https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079994126) and rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825606189).
TheCharlatan:
ACK ab878a7e74
Tree-SHA512: 7667305df9fef4728526c7217f85b51e739ec63b38e808da51d6ae65cb6f2696afa5ba82e5a72ed4a7a9b79ffa2402640448af4392587253027122eab7618e30
