0972f5504021b482b27523fd3bcb8036cf6b439c from #33229 broke manpage
generation, because the assumption that the last word in the line
containing the version number, was the version number, no-longer holds
for some binaries. i.e bitcoind.
cb7d5bfe4a59d6034e3a77486cbd423e1a3bfa44 test, assumeutxo: loading a wallet (backup) on a pruned node (Alfonso Roman Zubeldia)
7a365244f839eeea23619e92a73ecc78a70dc4bd test, refactor snapshot import and background validation (Alfonso Roman Zubeldia)
Pull request description:
Adding tests in `./test/functional/wallet_assumeutxo.py` to cover the following scenario:
- test loading a wallet (backup) on a pruned node
ACKs for top commit:
fjahr:
re-ACK cb7d5bfe4a59d6034e3a77486cbd423e1a3bfa44
theStack:
re-ACK cb7d5bfe4a59d6034e3a77486cbd423e1a3bfa44
Tree-SHA512: 88cc419f340d31e80120e0c6cafe567efc678df27576db6e08aeab62d2b50ed1153d56f3f3343e9bae49262e38f9fb81db7769f02a4a01e4ef25c5d029c12323
fad61185861a6a9ed806c387aa63d2b31262b1db test: Fix "typo" in written invalid content (MarcoFalke)
fab085c15f7221986f73af7e05e799edf3eadaf0 contrib: Use text=True in subprocess over manual encoding handling (MarcoFalke)
fa71c15f8610816a6ee0426cd396315da3d27c30 scripted-diff: Bump copyright headers after encoding changes (MarcoFalke)
fae612424b3e70acd6011a4459518174463b3424 contrib: Remove confusing and redundant encoding from IO (MarcoFalke)
fa7d72bd1be9a45e8c09525aee68caad1e57963e lint: Drop check to enforce encoding to be specified in Python scripts (MarcoFalke)
faf39d8539c9d563f68071054bbd533157f586ef test: Clarify that Python UTF-8 mode is the default today for most systems (MarcoFalke)
fa83e3a81ddb2170a0d7b0d86b94641a80d026ee lint: Do not allow locale dependent shell scripts (MarcoFalke)
Pull request description:
Historically, there was an attempt via `test/lint/lint-python-utf8-encoding.py` to enforce explicit UTF8 in every Python IO statement (`open`, `subprocess`, ...). However, the lint check has many problems:
* The check is incomplete and many IO statements lack the explicit UTF8 specification.
* It was added at a time when some systems were not UTF8 by default.
* The check is brittle, as it depends on a fragile regex.
In theory, now that the minimum Python version is 3.10 (since commit 2123c94448ed142e78942421c597a1f264859c48), the check could be replaced by `PYTHONWARNDEFAULTENCODING=1` from https://docs.python.org/3/whatsnew/3.10.html#optional-encodingwarning-and-encoding-locale-option. However, this comes with many other problems:
* All our Python scripts already assume and require UTF8 to be set externally. On almost all modern systems, this is already the default. Some Windows versions do not have UTF8 by default and require `PYTHONUTF8=1` to be set for the tests to run already today (with or without the changes in this pull). Also, the CI and many other Bash scripts force UTF8 via `LC_ALL`. Finally, Python 3.15 will likely enable UTF8 on *all* systems by default, per https://peps.python.org/pep-0686/#abstract.
* So adding UTF8 to every single IO call is redundant, verbose, and confusing, given that it is the expected default.
So fix all issues, by:
* Removing the `test/lint/lint-python-utf8-encoding.py` check.
* Removing the encoding on the individual IO calls.
* Clarifying the existing docs around the existing UTF8 requirement and assumption.
Obviously, every IO call is still free to specify UTF8 or any other encoding explicitly, if there is a documented need for it in the future.
ACKs for top commit:
theStack:
re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db
laanwj:
Re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db
Tree-SHA512: 78025ea3508597d2299490347614f0ee3e4c66e3ba559ff50e498045a9c8bbd92f3a5ced18719d8fcebbd1e47bdbb56a0c85a5b73b425adb0ea4f02fe69c3149
804329400a73df00dfd7a5209c659d4a22b9ce47 fuzz: gate mempool entry based on weight (Greg Sanders)
Pull request description:
The mempool implementation now uses TxGraph with entries using FeePerWeight, not vsize. This means our package_rbf harness will erroneously add more transaction weight than we can support inside of FeeFrac. Gate more aggressively using WITNESS_SCALE_FACTOR.
Fixes https://github.com/bitcoin/bitcoin/issues/33981
ACKs for top commit:
sdaftuar:
ACK 804329400a73df00dfd7a5209c659d4a22b9ce47
ismaelsadeeq:
utACK 804329400a73df00dfd7a5209c659d4a22b9ce47
dergoegge:
utACK 804329400a73df00dfd7a5209c659d4a22b9ce47
Tree-SHA512: e78d0f73f9b9cbb8c0db1e8e91dbffeb4110cf8113e90f34af5c132acf0819c54254891a4dd5da63016e4edf9d8e886f469f959bd3504b7deb66989d96fe4cf1
fa45a1503eee603059166071857215ea9bd7242a log: Use LogWarning for non-critical logs (MarcoFalke)
fa0018d01102ad1d358eee20d8bae1e438ceebf8 log: Use LogError for fatal errors (MarcoFalke)
22229de7288fed6369bc70b2af674906e6777ce4 doc: Fix typo in init log (MarcoFalke)
Pull request description:
Logging supports severity levels above info via the legacy `LogPrintf`. So use the more appropriate `LogError` or `LogWarning`, where it applies.
This has a few small benefits:
* It often allows to remove the manual and literal "error: ", "Warning:", ... prefixes. Instead the uniform log level formatting is used.
* It is easier to grep or glance for more severe logs, which indicate some kind of alert.
* `LogPrintf` didn't indicate any severity level, but it is an alias for `LogInfo`. So having the log level explicitly spelled out makes it easier to read the code.
* Also, remove the redundant trailing `\n` newline, while touching.
* Also, remove the `__func__` formatting in the log string, which is redundant with `-logsourcelocations`. Instead, use a unique log string for each location.
ACKs for top commit:
l0rinc:
Code review ACK fa45a1503eee603059166071857215ea9bd7242a
stickies-v:
ACK fa45a1503eee603059166071857215ea9bd7242a
rkrux:
crACK fa45a1503eee603059166071857215ea9bd7242a
Tree-SHA512: 516d439c36716f969c6e82d00bcda03c92c8765a9e41593b90052c86f8fa3a3dacbb2c3dc98bfc862cefa54cae34842b488671a20dd86cf1d15fb94aa5563406
b8d279a81c16fe9f5b6d422e518c77344e217d4f doc: add comment to explain correctness of GatherClusters() (Suhas Daftuar)
aba7500a30eecf742c56e292e9a385ca57066a6c Fix parameter name in getmempoolcluster rpc (Suhas Daftuar)
6c1325a0913e22258ab6b62f381e56c7bebbd462 Rename weight -> clusterweight in RPC output, and add doc explaining mempool terminology (Suhas Daftuar)
bc2eb931da30bd98670528c0b96f6ca05f14f8b9 Require mempool lock to be held when invoking TRUC checks (Suhas Daftuar)
957ae232414b38adcf9358e198fded42f7c1feea Improve comments for getTransactionAncestry to reference cluster counts instead of descendants (Suhas Daftuar)
d97d6199ce506cda858afa867f2582c8138953a5 Fix comment to reference cluster limits, not chain limits (Suhas Daftuar)
a1b341ef9875a8a160464f320886f8dac7491237 Sanity check feerate diagram in CTxMemPool::check() (Suhas Daftuar)
23d6f457c4c06e405464594c7a2be1a11e9bcc1b rpc: improve getmempoolcluster output (Suhas Daftuar)
d2dcd37aac1e723a4103f2d6fefaa492141f5d42 Avoid using mapTx.modify() to update modified fees (Suhas Daftuar)
d84ffc24d2dc35642864924aaf7466fa17ac5875 doc: add release notes snippet for cluster mempool (Suhas Daftuar)
b0417ba94437d8bb23a7b66a3641ee8f3682a2dc doc: Add design notes for cluster mempool and explain new mempool limits (Suhas Daftuar)
2d88966e43c6c6323d8af5272ab7841f5c896f12 miner: replace "package" with "chunk" (Suhas Daftuar)
6f3e8eb3001a87d0a6d9ec8662ddb40ce7a673f4 Add a GetFeePerVSize() accessor to CFeeRate, and use it in the BlockAssembler (Suhas Daftuar)
b5f245f6f2193a3c19bea3eed7ceda1e80b83160 Remove unused DEFAULT_ANCESTOR_SIZE_LIMIT_KVB and DEFAULT_DESCENDANT_SIZE_LIMIT_KVB (Suhas Daftuar)
1dac54d506b5765f3d86a6efc30538931305b000 Use cluster size limit instead of ancestor size limit in txpackage unit test (Suhas Daftuar)
04f65488ca3e8e8eb7d290982e55e70be96491bb Use cluster size limit instead of ancestor/descendant size limits when sanity checking TRUC policy limits (Suhas Daftuar)
634291a7dc4485942cc9cbde510b92f9580d5c5e Use cluster limits instead of ancestor/descendant limits when sanity checking package policy limits (Suhas Daftuar)
fc18ef1f3f333dd28d8cc7e3571d76a985d90240 Remove ancestor and descendant vsize limits from MemPoolLimits (Suhas Daftuar)
ed8e819121d7065c6e34a6ae422842369c4a1659 Warn user if using -limitancestorsize/-limitdescendantsize that the options have no effect (Suhas Daftuar)
80d8df2d47c25851b51fe3319605fe41c34ca9f8 Invoke removeUnchecked() directly in removeForBlock() (Suhas Daftuar)
9292570f4cb85fc6690dfeeb55ea867d575ebba3 Rewrite GetChildren without sets (Suhas Daftuar)
3e39ea8c307010bc0132615ecef55b39851f7437 Rewrite removeForReorg to avoid using sets (Suhas Daftuar)
a3c31dfd71def7ce4414c627261fa4516f943547 scripted-diff: rename AddToMempool -> TryAddToMempool (Suhas Daftuar)
a5a7905d83dfa8a5173f886f7007132e18b53e3a Simplify removeRecursive (Suhas Daftuar)
01d8520038eafa0e00eeddcea29cba2b1b87917e Remove unused argument to RemoveStaged (Suhas Daftuar)
bc64013e6fad2d054bc5a31630c09f33a62b8f4f Remove unused variable (cacheMap) in mempool (Suhas Daftuar)
Pull request description:
As suggested in the main cluster mempool PR (https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3177119367), I've pulled out some of the non-essential optimizations and cleanups into this separate PR.
Will continue to add more commits here to address non-blocking suggestions/improvements as they come up.
ACKs for top commit:
instagibbs:
ACK b8d279a81c
sipa:
ACK b8d279a81c16fe9f5b6d422e518c77344e217d4f
Tree-SHA512: 1a05e99eaf8db2e274a1801307fed5d82f8f917e75ccb9ab0e1b0eb2f9672b13c79d691d78ea7cd96900d0e7d5031a3dd582ebcccc9b1d66eb7455b1d3642235
The mempool implementation now uses TxGraph with entries
using FeePerWeight, not vsize. This means our package_rbf
harness will erroneously add more transaction weight than we
can support inside of FeeFrac. Gate more aggressively using
WITNESS_SCALE_FACTOR.
fe1815d48f0cee57d2f1af50b377c7f9e462369e cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB` (Hennadii Stepanov)
Pull request description:
The CMake script in the `test/kernel` subdirectory is already gated by `BUILD_KERNEL_LIB`:f6acbef108/src/CMakeLists.txt (L405-L409)
As a result, the following configuration summary is misleading:
```
$ cmake -B build -DBUILD_KERNEL_LIB=OFF -DBUILD_KERNEL_TEST=ON
<snip>
bitcoin-chainstate (experimental) ... OFF
libbitcoinkernel (experimental) ..... OFF
kernel-test (experimental) .......... ON
<snip>
```
This PR fixes the behaviour by making the `BUILD_KERNEL_TEST` option explicitly depend on `BUILD_KERNEL_LIB`.
ACKs for top commit:
maflcko:
lgtm ACK fe1815d48f0cee57d2f1af50b377c7f9e462369e
sedited:
ACK fe1815d48f0cee57d2f1af50b377c7f9e462369e
Tree-SHA512: 24524d43b195b0e3907f3257ef907c5ead8e9921b888bc82765f4dbbe44728b92956233c8fe624e8509bf8146a41cf8c1ac26f6043b8a21f681ad2ae19bebc5d
49c672853503e561cd1e7fed19a32f21ad345370 cmake: Set `WITH_ZMQ` to `ON` in Windows presets (Hennadii Stepanov)
Pull request description:
The "zeromq" feature is already enabled by default in `vcpkg.json`, and there appears to be no reason to omit this configuration option when building on Windows.
ACKs for top commit:
maflcko:
lgtm ACK 49c672853503e561cd1e7fed19a32f21ad345370
Tree-SHA512: acaef1eba56e75f9979db1809c6ebac59b2ed49002ae557fcb172f6119f6b8927580583616628f24d71ccbb32544f4d96317ff1d3125a9b5446ae89d1d318de0
Also update the help text for -limitancestorcount/-limitdescendantcount to
explain they no longer affect the mempool, and are only used by the wallet for
coin selection.
Also improve test coverage for removeForReorg by creating a scenario where
there are in-mempool descendants that are only invalidated due to an in-mempool
parent no longer spending a mature coin.
The "zeromq" feature is already enabled by default in `vcpkg.json`, and
there appears to be no reason to omit this configuration option when
building on Windows.
2e27bd9c3af91eb9fcc626fe65d065df0a80974d ci: Add Windows + UCRT jobs for cross-compiling and native testing (Hennadii Stepanov)
bd130db994e2a3a137bf232e5cc0ed164aa58b17 ci: Rename items specific to Windows + MSVCRT (Hennadii Stepanov)
Pull request description:
This PR is part of the ongoing effort to migrate to the modern UCRT runtime for cross-compiled Windows binaries, including release builds.
For more details about this migration, see:
- https://github.com/bitcoin/bitcoin/issues/30210
- https://github.com/bitcoin/bitcoin/pull/33593
MSVCRT-related CI jobs should be removed from the CI framework once the migration to UCRT is complete.
ACKs for top commit:
maflcko:
review ACK 2e27bd9c3af91eb9fcc626fe65d065df0a80974d 🖊
fanquake:
ACK 2e27bd9c3af91eb9fcc626fe65d065df0a80974d
Tree-SHA512: 222ca5e54646bcce9db6e20191d5891e988274e18b2f30085de6435a3b288a9d0fc414e8f76342e275ae58ee6603f751933d1faa8bdff446edf2695091f8ca4c
3e01b5d0e7be3dabe7f52d70e577f03f31505ad9 contrib: rename gen-sdk to gen-sdk.py (fanquake)
c1213a35abed01a97a9c52954919158f91f974d2 macdeploy: disable compression in macOS gen-sdk script (fanquake)
a33d03454508187abed764e55351ffcececc4c6e contrib: more selectively pick files for macOS SDK (fanquake)
Pull request description:
This includes three changes. The first is to more selectively pick files for inclusion into our macOS SDK tarball (skip manpages, binaries etc), which is nice because it redues the size of the tarball (from ~80mb to 20mb), and makes the size increase that happens with the next commit, less-bad.
The second change removes compression of the tarball. Starting with Python 3.11, Pythons gzip might delegate to zlib. Depending on the OS, i.e Ubuntu vs Fedora, the underlying zlib implementation might differ, resulting in different output.
For now, or until a better solution exists, remove compression. This results in the SDK increasing in size to ~157mb. Which is not unreasonable, to regain determinism (and would be significantly worse without the previous commit).
See: https://docs.python.org/3/library/gzip.html#gzip.compress
The third renames `gen-sdk` to `gen-sdk.py`, so that it will be linted, along with the rest of our Python files.
Fixes#31873. We could probably also put this into 30.x.
ACKs for top commit:
stickies-v:
ACK 3e01b5d0e7 modulo the new .tar SDK being uploaded
davidgumberg:
Tested ACK 3e01b5d0e7be3d
Tree-SHA512: 272164a98e0e6f10822870162c1b3a405693c2f64d3ed085a2d2243a48641d940704b5ef6022256915ac9cf383e87a4f8d4dc2ec4eaa9d25e2abd30f5498778b
e07e57368e9fab8ecfc140d44aef7db9b23c7ce0 ci: clear out space on centos job (will)
Pull request description:
Fixes#33293
Clear out space on jobs running on GHA by deleteing unnecessary files.
Raised in #33293 which pointed to a solution like b7f04d7822 which is adapted slightly here.
Only runs when cache provider (runner) is `gha`.
A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/19703413734/job/56444984809
ACKs for top commit:
maflcko:
lgtm ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
m3dwards:
ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
janb84:
ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
Tree-SHA512: 723589df4c434dd3eaed43acefe25f1788837743882e910e79eceee25e2bd98990cd01b8b80a46ba82418867b32c5ee1b96341223696244504e118eae6ad4a16
c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3 Change Parse descriptor argument to string_view (Sjors Provoost)
Pull request description:
While investigating a silent merge conflict in #33135 I noticed that #32983 changed the descriptor `Parse` function signature from `const std::string& descriptor` to `std::span<const char> descriptor`.
Calling that new version of `Parse` with a string literal will trigger a confusing "Invalid characters in payload" due to the trailing "\0".
It can be worked around by having (the test) wrap string literals in `std::string()`, but that's easy to forget.
Using `string_view` is easier and more compact than (as a previous version of this PR did) checking for trailing `\0`.
Also add a test.
ACKs for top commit:
maflcko:
review ACK c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3 🍨
enirox001:
tACK c0bfe72
stickies-v:
ACK c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3
rkrux:
crACK c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3
Tree-SHA512: 6b20307f834dae66826c8763f6c2ba0071f4e369375184cb5ff8543b85220fcaf33a47ddb065e418d1af3ed9a3fac401a7854f8924f52aab2b000b1f65328f2c
52230a7f697fd99abdc4550d6a60737be024e246 test: check for output to stdout in `TestShell` test (Sebastian Falbesoner)
Pull request description:
This is a small follow-up PR to the recently added `TestShell` test (#33546), verifying the stdout message "TestShell is already running!" when trying to instantiate a second instance.
ACKs for top commit:
maflcko:
lgtm ACK 52230a7f697fd99abdc4550d6a60737be024e246
rkrux:
crACK 52230a7f697fd99abdc4550d6a60737be024e246
Tree-SHA512: 096d70e1bd0f09c1b389e58fa4b880442406c56f0c8ef8b8fbd0627081bc390b1ce5d6032bcca19b03206b7a444d9c523f9b62078b5ca5b7f1ae3c57bb4129c9
2909655fba91a7cc59c484fc74afafdf7ccc0cfa fix: remove redundant mempool lock in ChainImpl::isInMempool() (Fibonacci747)
Pull request description:
This PR removes an unnecessary `LOCK(mempool->cs)` in `ChainImpl::isInMempool()`. The method calls `CTxMemPool::exists()`, which already locks `mempool->cs` internally. Because the mempool mutex is a RecursiveMutex, double-locking was safe but redundant. Dropping the outer lock matches patterns used elsewhere in ChainImpl (e.g. `hasDescendantsInMempool()` and `GetTransactionAncestry()` callers) where mempool read APIs are invoked without an additional lock and rely on the callee’s internal locking. `isRBFOptIn()` remains unchanged since `IsRBFOptIn(tx, pool)` explicitly requires the caller to hold `pool.cs` as indicated by its thread-safety annotation.
ACKs for top commit:
maflcko:
lgtm ACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
instagibbs:
utACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
stickies-v:
ACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
Tree-SHA512: 4dfd88e01d8c7a4b6ceb3c736243fb22bfee5ccfc422d134acb633b908ca14c807637a2aa20de89e86e583b23ec70a1d121d77e35af60e114d93971b2a4bfd3b
Clear out space on the centos job be deleteing unnecessary files.
Raised by #33293 which pointed to a solution like b7f04d7822
Only runs when cache provider (runner) is `gha`, and on the CentOS job.
70d9e8f0a15d07a27ae37befb5c1bce71c98d8de fix: reorg behaviour in mempool tests to match real one (yuvicc)
540ed333f6c81e8d191dfa8fd7cf162e980edfa1 Move the create_empty_fork method to the test framework's blocktools.py module to enable reuse across multiple tests. (yuvicc)
Pull request description:
Updated functional tests to replace direct use of `invalidateblock` with proper fork-based reorg behaviour. The direct invalidation approach bypasses important validation checks and has depth limitations(10 block) that don't match real-world reorg scenarios. For more details see #32531.
Fixes#32531
ACKs for top commit:
instagibbs:
reACK 70d9e8f0a15d07a27ae37befb5c1bce71c98d8de
theStack:
re-ACK 70d9e8f0a15d07a27ae37befb5c1bce71c98d8de
Tree-SHA512: 8aae298bfa295b4e0e4627b522e9eac549399008fd8e336a66f8c9950c886917da0b3f0bdc62d0c8ea2b8082f36639300cac4070986a7766398e15bc1f666da5
