Currently, there are issues with headers in `batchpriority.cpp`:
1. `SCHED_BATCH` is not defined on all supported *BSD platforms.
2. `pthread.h` is necessary on other platforms.
This addresses both issues and fixes other includes.
4fab35cf88 miniscript: correct and_v() properties (Antoine Poinsot)
Pull request description:
`and_v()` must never be 'd'. This is not a bug fix since this was unreachable in valid Miniscripts: the first sub of an `and_v()` must be of type V, which conflicts with (i.e. never has) property 'd'.
ACKs for top commit:
sipa:
ACK 4fab35cf88. Fuzzed for 2 months worth of CPU time.
achow101:
ACK 4fab35cf88
Tree-SHA512: 8932ad2c9188747299cb9147ff097dca8d078ce7bdd0caefa71ee2724ff81d9bef836664211c2081519a45afd50c539974d67c2a3a1a42a65a3b10b1daef8cbe
d3e681bc06 fuzz: Use `__AFL_SHM_ID` for naming test directories (marcofleon)
Pull request description:
During long multicore fuzzing campaigns with AFL++, stale datadirs can eventually accumulate from time outs, resulting in disk running out of space (see https://github.com/bitcoin/bitcoin/issues/28811). The easiest way to reproduce this is by running our `utxo_total_supply` target using multiple cores with AFL++ and observing the crashes that occur because of all the directories in `/tmp/test_common\ bitcoin/utxo_total_supply/`.
Fix this by using the AFL++ shared memory ID to name the test dirs and cleaning it up before each setup. This ID is unique per AFL++ instance, so multiple cores can run in parallel without conflicts.
Fixes https://github.com/bitcoin/bitcoin/issues/28811
ACKs for top commit:
maflcko:
lgtm ACK d3e681bc06
dergoegge:
utACK d3e681bc06
Tree-SHA512: 420373e5f8a63c84797303ba2ef6657dfe9dacf9c2f3d818524421c24681a0e984c212ecb706217d93f67c2ec16b146a2d37fddcbd6918b2e5e9f634f5e13c10
fafdae46ff test: Check that redundant verack message is ignored (MarcoFalke)
Pull request description:
The code exists and is uncovered (ref https://maflcko.github.io/b-c-cov/total.coverage/src/net_processing.cpp.gcov.html#L3795), so add a trivial test to cover it.
ACKs for top commit:
brunoerg:
ACK fafdae46ff
sedited:
ACK fafdae46ff
Tree-SHA512: 157f434c2faa16243890b2344c4ee36bc359e56c80ba8a04f0bba71e9760cf9106c38ed755ff57eff8d1957f35516d20b3d010e0ecb8633b845f5314cc0d050a
fad2876ec3 ci: Always print low ccache hit rate notice (MarcoFalke)
Pull request description:
Looks like the hit rate is low, even on test changes such as https://github.com/bitcoin/bitcoin/actions/runs/21476546461/job/61867393974#step:10:3349
to make it easier to debug, unconditionally print the low hit rate notice
ACKs for top commit:
l0rinc:
ACK fad2876ec3
sedited:
ACK fad2876ec3
Tree-SHA512: 0cd85e3572e8465ec424766b1fdb6181d7e607cae991889b46cc66e5f08354772b6040a9f14c0864d36e1f38894628819a3a7458d3ec9ea32e063257177740a0
7d9e1a8102 test: Verify peer usage after assumeutxo validation completes (stringintech)
0067abe153 p2p: Allow block downloads from peers without snapshot block after assumeutxo validation (stringintech)
Pull request description:
Currently, after assumeutxo background validation finishes, the node continues to skip peers that don't have the snapshot block in their best chain until restart. This unnecessarily excludes peers from block downloads even though the background sync has completed and undo data is available.
The restriction persists because `m_chainman.CurrentChainstate().SnapshotBase()` continues to return the snapshot base block until restart, even after validation completes. Added `m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED` check to only apply the peer restriction while background validation is ongoing.
Also added test coverage in `feature_assumeutxo.py` that verifies peers without the snapshot block can be used for block downloads after background validation completes. The test fails without this fix.
ACKs for top commit:
fjahr:
Re-ACK 7d9e1a8102
achow101:
ACK 7d9e1a8102
sedited:
Re-ACK 7d9e1a8102
Tree-SHA512: 5515971da7bf7efc55eecdf03686f44c20c9e52dd168e7cfa119032d6a8ebccee69df7143075e4e9d0a01426cd9ae7202dce5c00919a82478ebf49a15dc0fe19
3e0fd0e4dd refactor: rename will_reuse_cache to reallocate_cache (Andrew Toth)
44b4ee194d validation: reuse same CCoinsViewCache for every ConnectBlock call (Andrew Toth)
8fb6043231 coins: introduce CCoinsViewCache::ResetGuard (Andrew Toth)
041758f5ed coins: use hashBlock setter internally for CCoinsViewCache methods (Andrew Toth)
8dd9200fc9 coins: add Reset on CCoinsViewCache (Andrew Toth)
Pull request description:
This is the first commit of #31132, which can be merged as an independent change. It has a small benefit on its own, but will help in moving the parent PR forward.
Add a `Reset()` method to `CCoinsViewCache` that clears `cacheCoins`, `cachedCoinsUsage`, and `hashBlock` without flushing to the `base` view. This allows efficiently reusing a cache instance across multiple blocks.
Add `CCoinsViewCache::CreateResetGuard` method to return a `CCoinsViewCache::ResetGuard`. The `ResetGuard` automatically calls `Reset()` on destruction. This RAII pattern ensures the cache is always properly reset between blocks.
Add `m_connect_block_view` as a persistent `CCoinsViewCache` for `ConnectBlock`, avoiding repeated memory allocations.
ACKs for top commit:
l0rinc:
ACK 3e0fd0e4dd
achow101:
ACK 3e0fd0e4dd
sedited:
ACK 3e0fd0e4dd
Tree-SHA512: a95feaa062a9eb7cf7514425a7e7adffd347cd1f7b32b4c1fefcde30002141757c184174702b3104a029dcd33194f8bd734159deebb2e668716089305b42cb00
c6ca2b85a3 validation: do not wipe utxo cache for stats/scans/snapshots (Pieter Wuille)
7099e93d0a refactor: rename `FlushStateMode::ALWAYS` to `FORCE_FLUSH` (Lőrinc)
Pull request description:
Revival of https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432564955 with the remaining comments applied on top
> Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
>
> Split the `FlushStateMode::ALWAYS` mode into a FORCE_SYNC (non-wiping) and a FORCE_FLUSH (wiping), and then use the former in `scantxoutset`, `gettxoutsetinfo`, snapshot creation.
(slightly updated after #30214)
ACKs for top commit:
optout21:
reACK c6ca2b85a3
cedwies:
reACK c6ca2b8 (trivial)
achow101:
ACK c6ca2b85a3
sedited:
ACK c6ca2b85a3
Tree-SHA512: f3525a85dc512db4a0a9c749ad47c0d3fa44085a121aa54cd77646260a719c71f754ec6570ae77779c0ed68a24799116f79c686e7a17ce57a26f6a598f7bf926
Add m_connect_block_view to ChainState's CoinsViews.
Call CreateResetGuard inside ConnectTip to ensure the view
is Reset after each block, avoiding repeated memory allocations.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
CCoinsViewCache::CreateResetGuard returns a guard that calls
Reset on the cache when the guard goes out of scope.
This RAII pattern ensures the cache is always properly reset
when it leaves current scope.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Add a Reset() method to CCoinsViewCache that clears cacheCoins,
cachedCoinsUsage, and hashBlock without flushing to the base view.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Use the AFL++ shared memory ID environment variable to create
a deterministic datadir path. This prevents accumulation of stale
directories after a fuzz iteration crashes or times out. During
long fuzz campaigns, this accumulation has occasionally resulted
in running out of disk space.
2845f10a2b test: extend FreeBSD ephemeral port range fix to P2P listeners (node)
34bed0ed8c test: use IP_PORTRANGE_HIGH on FreeBSD for dynamic port allocation (woltx)
Pull request description:
Reopening #34336. I’ve now tested it on FreeBSD and confirmed it works.
On FreeBSD, the default ephemeral port range (10000-65535) overlaps with the test framework's static port range (11000-26000), possibly causing intermittent "address already in use" failures when tests use dynamic port allocation (`port=0`).
This PR adds a helper that sets `IP_PORTRANGE_HIGH` via `setsockopt()` before binding, requesting ports from 49152-65535 instead, which avoids the overlap, as suggested in https://github.com/bitcoin/bitcoin/issues/34331#issuecomment-3767161843 by @maflcko .
From FreeBSD's [sys/netinet/in.h](https://cgit.freebsd.org/src/tree/sys/netinet/in.h):
```c
#define IP_PORTRANGE 19
#define IP_PORTRANGE_HIGH 1
#define IPPORT_EPHEMERALFIRST 10000 /* default range start */
#define IPPORT_HIFIRSTAUTO 49152 /* high range start */
```
See also: FreeBSD https://man.freebsd.org/cgi/man.cgi?query=ip&sektion=4 man page.
Fixes#34331
ACKs for top commit:
vasild:
ACK 2845f10a2b
hebasto:
ACK 2845f10a2b, I have reviewed the code and it looks OK.
Tree-SHA512: ce501ce3e8a4023e07bad572df2b85d6829becf133813e4529aebba83e4eba59fa8b48e9d2197ebbb226adaf3054fad720775a787244d6b38c0078ee086102f4
fad042235b refactor: Remove remaining std::bind, check via clang-tidy (MarcoFalke)
Pull request description:
`std::bind` has many issues:
* It is verbose in a meaningless way
* Overriden args are silently accepted and dropped at runtime without a compile error. Same for accidental duplicates.
One could use `std::bind_front` similar to commit fa267551c4. Though, I think the remaining cases are better off with lambdas.
So do that here, and enable the `modernize-avoid-bind` clang-tidy rule to avoid `std::bind` bugs in the future.
ACKs for top commit:
fjahr:
Code review ACK fad042235b
purpleKarrot:
Code review ACK fad042235b
Tree-SHA512: 38b17e26eda3ae47d84a8c34298309dc1eeb4ed434fda58b5803ef031c4c2edfb17222f5208f37af727bf340e32b37c7f81784f461d2b65fbc6227f3cd53eea4
e770392084 test: addrman: test self-announcement time penalty handling (Bruno Garcia)
Pull request description:
This PR adds a test case for addrman that verifies that addresses announcing themselves (addr == source) are exempt from time penalties, while addresses announced by others receive the expected penalty.
It fixes the following mutant (https://corecheck.dev/mutation/src/addrman.cpp#L561):
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 206b54118e..c6a045fd8d 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -558,7 +558,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, std::c
AddrInfo* pinfo = Find(addr, &nId);
// Do not set a penalty for a source's self-announcement
- if (addr == source) {
+ if (addr != source) {
time_penalty = 0s;
}
```
ACKs for top commit:
maflcko:
review ACK e770392084🐤
achow101:
ACK e770392084
fjahr:
Code review ACK e770392084
naiyoma:
tACK e770392084
Tree-SHA512: ec029d1e1e979f91840af944984cad530a1ce9a0eceb123230817f0ef3b9ad47253eebc4c953d350de2d904b59496fcd4757123c8bd63cf0e09c3581da48fff8
2ee7f9b259 coins: assume `GetCoin` only returns unspent coins (Andrew Toth)
eec551aaf1 fuzz: keep `coinscache_sim` backend free of spent coins (Andrew Toth)
3e4155fcef test: do not return spent coins from `CCoinsViewTest::GetCoin` (Andrew Toth)
ee1e40f580 txdb: assert `CCoinsViewDB::GetCoin` only returns unspent coins (Lőrinc)
Pull request description:
This PR is split out from #33018 to keep that PR focused on removing the `FRESH-but-not-DIRTY` cache state.
### Problem
`::GetCoin()` is an interface for querying the UTXO set, so production implementations should only ever return unspent coins. Tests should mimic this to provide useful feedback.
### Fix:
* Add a fail-fast assertion that `CCoinsViewDB::GetCoin()` never returns a spent coin.
* Align unit tests and fuzz simulations with the production `GetCoin()` contract by never returning spent coins.
* Replace the unreachable “spent coin returned by parent” handling in `CCoinsViewCache::FetchCoin()` with `Assert(!coin.IsSpent())`, drop outdated `spent+FRESH` docs, and tighten `SanityCheck()` invariants.
Behavior is unchanged, it just aligns our tests to exercise valid states.
ACKs for top commit:
andrewtoth:
re-ACK 2ee7f9b259
optout21:
crACK 2ee7f9b259
achow101:
ACK 2ee7f9b259
w0xlt:
reACK 2ee7f9b259
Tree-SHA512: be21cc09690410fc04ca25e1ba47aae6186bc037e413b3bb1e6e9a04e6364cbfac5a2fcdc49b638fec848cd29243fab0cc0581b9923f34fafe8366828f690ed4
and_v() must never be 'd'. This is not a bug fix since this was
unreachable in valid Miniscripts: the first sub of an and_v() must be of
type V, which conflicts with (i.e. never has) property 'd'.
1f60ca360e wallet: fix removeprunedfunds bug with conflicting transactions (Martin Zumsande)
Pull request description:
`removeprunedfunds` removes all entries from `mapTxSpends` for the inputs of the pruned tx. However, this is incorrect, because there could be multiple entries from conflicting transactions (that shouldn't be removed as well). This could lead to the wallet creating invalid transactions, trying to double spend utxos.
The bug persists when the conflicting tx was mined, because the wallet trusts its internal accounting instead of calling `AddToSpends` again.
The added test should fail on master.
ACKs for top commit:
achow101:
ACK 1f60ca360e
fjahr:
tACK 1f60ca360e
furszy:
utACK 1f60ca360e
vasild:
ACK 1f60ca360e
Tree-SHA512: 3cc9ed547530fd53e25721177b76ab2e1eae16ce2c0e63fc01b20fdbf8bd02655dae51167ad56f9dec748d34c61ce65d38f993370820601f8257c73b876a3347
905dfdee86 test: use ModuleNotFoundError in interface_ipc.py (fanquake)
Pull request description:
Change this so we catch the case where the capnp shared libs have been updated, and can no-longer be loaded by the Python module, resulting in a skipped test, even though pycapnp is installed. i.e:
```bash
stderr:
Traceback (most recent call last):
File "/root/ci_scratch/build/test/functional/interface_ipc.py", line 20, in <module>
import capnp # type: ignore[import] # noqa: F401
^^^^^^^^^^^^
File "/usr/local/lib64/python3.14/site-packages/capnp/__init__.py", line 36, in <module>
from .version import version as __version__
File "/usr/local/lib64/python3.14/site-packages/capnp/version.py", line 1, in <module>
from .lib.capnp import _CAPNP_VERSION_MAJOR as LIBCAPNP_VERSION_MAJOR # noqa: F401
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: libcapnpc.so.1.0.1: cannot open shared object file: No such file or directory
```
Failing in this way should make it clear that `pycapnp` needs to be reinstalled/rebuilt.
If `pycapnp` is not installed, the test still skips as expected:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py skipped (capnp module not available.)
TEST | STATUS | DURATION
interface_ipc.py | ○ Skipped | 0 s
```
Fixes: #34016.
ACKs for top commit:
maflcko:
lgtm ACK 905dfdee86
hebasto:
ACK 905dfdee86, I have reviewed the code and it looks OK. However, I'm [not able](https://github.com/bitcoin/bitcoin/issues/34016#issuecomment-3799532047) to reproduce https://github.com/bitcoin/bitcoin/issues/34016.
sedited:
ACK 905dfdee86
Tree-SHA512: 3cedbe8fc51cc18f1c993f7747d20905f3bf94c736db99a9c4090f5823bf8c09dfbc19ef03c573d504dcdfba6ea0f7d088a7f4563b220742c9a441167c04cfd6
fa9c92d7b6 log: Print warning about privacy-sensitive log info unconditionally (MarcoFalke)
Pull request description:
There is a warning about logs containing privacy-sensitive information. However, it is only printed when at least one debug log category is enabled.
This is confusing, because:
* Setting (let's say) `-debug=reindex` enables this warning, but it is hard to see what sensitive logs could be contained in reindex debug logs.
* Dropping `-debug=reindex` again disabled this warning, but the wallet continues to log txids (and other sensitive stuff) at info level.
So instead of implying the wrong thing, it would be better to remove this log line (because it should be common sense), or log it unconditionally.
ACKs for top commit:
l0rinc:
ACK fa9c92d7b6
sedited:
ACK fa9c92d7b6
Tree-SHA512: 42f71b030e7722203f225f04e979143e829dae3556f64e322a791361a3b9c16150d53bb7bb9a99839c975d9052115770b9473138acc58baeee457253526fd892
ab649ce459 guix: documented shasum gathering command (janb84)
Pull request description:
When a PR requires proof of Guix builds (sha256sums), the PR author or reviewer uses a not well documented command to collect the sha256sums of build outputs or manually gathers them from files.
This pull request introduces a new section in the documentation, providing some documentation on the command's functionality and usage.
ACKs for top commit:
willcl-ark:
ACK ab649ce459
sedited:
ACK ab649ce459
Tree-SHA512: 0188663ad117b636c7d32a1b655db97610f558cfcffe4abd6f0fb097b3990db0dc6d23ab972926fefd2531b21f429742dcbea6b0fa579d22d5da7a7d6a4c753e
fa578d9434 lint: [move-only] Move python related lints to lint_py.rs (MarcoFalke)
fa392c31e7 lint: [move-only] Move repo related lints to lint_repo_hygiene.rs (MarcoFalke)
fab0cfa987 lint: [move-only] Move cpp related lints to lint_cpp.rs (MarcoFalke)
fa3e48e3fd lint: [move-only] Move docs related lints to lint_docs.rs (MarcoFalke)
fad09e77db lint: [move-only] Move text related lints to text_format.rs (MarcoFalke)
faf40c2f84 lint: [move-only] Move util functions to util.rs (MarcoFalke)
Pull request description:
The single, large `main.rs` file is fine, but at some point it becomes harder to read.
So reduce the size by pulling functions out into modules.
This can be reviewed with the git option: `--color-moved=dimmed-zebra`
ACKs for top commit:
l0rinc:
Lightly tested code review ACK fa578d9434
sedited:
ACK fa578d9434
Tree-SHA512: f1e29fd3cf695fb6634d0b9f9e55508992b4b9885afee9dbe4d5d9e99cad3061e7141f39acbfe69d698422888169128cd7658a6dc991fd904b8520328b51586d
c8abac9941 ci: mount .git dir rw (ci)
Pull request description:
On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when "GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py.
This requires write access to the .git directory.
Make the mounted .git directory writable.
This is currently not run on PR branches or locally which caused a miss during review.
Ideally we can have the same checks running in PRs as on merges to master to avoid future discrepancies like this.
ACKs for top commit:
maflcko:
lgtm ACK c8abac9941
l0rinc:
untested code review ACK c8abac9941
Tree-SHA512: 7ae4f63227ecffe1dc9003454a7473d6d592550af2e1c899457f34a947e5604b04c13319fb8979f36789ae7787bed62066be60697d163ad5ebedde3fbe8ce45f
On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when
"GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py.
This requires write access to the .git directory.
Make the mounted .git directory writable.
This is currently not run on PR branches or locally which caused a miss
during review.
3400db8040 doc: add missing param description to SRD (yancy)
Pull request description:
The params documentation is missing `change_fee` and the description is lacking recent changes.
ACKs for top commit:
murchandamus:
ACK 3400db8040
brunoerg:
code review ACK 3400db8040
Tree-SHA512: 8f6fac0d92873c5c9f77b19fbc0c6ecfb425b2a6b3d5f5ad69c82ed706b21cf4627e68c71acbc43661000e6063e8f8dbcd3b8ff60e3c727bdcba497d13ee1383
ddae1b4efa ci: remove gnu-getopt usage (fanquake)
Pull request description:
This is used for argument parsing in the `retry` script, however we don't use the script with any arguments. So remove the unused code, and the dependency on `gnu-getopt`.
This came up in the context of adding new CI jobs, where gnu-getopt might not be available, or working properly. It seemed easier to just remove the unused code, than look for more workarounds.
ACKs for top commit:
maflcko:
review ACK ddae1b4efa🔀
sedited:
ACK ddae1b4efa
Tree-SHA512: a73cf61fe0965127f87f1725b3a25a305ebfd354c318f5f44ecfa20da02ba72fef42dca656dae07f6e1ece956b9d7c58e99edb124d968a4bffb2ce6ac8fc018b
fad7bd9ba3 noui: Remove always empty caption while formatting (MarcoFalke)
fa8ebeb332 refactor: [gui] Document that the title is always empty for node message (MarcoFalke)
fafe71b743 refactor: Remove empty caption from ThreadSafeMessageBox (MarcoFalke)
fa8d0088e7 refactor: Remove empty caption from ThreadSafeQuestion (MarcoFalke)
fa0195499c refactor: [gui] Use lambdas over std::bind (MarcoFalke)
eeee1e341f refactor: Remove trailing semicolon after ADD_SIGNALS_DECL_WRAPPER (MarcoFalke)
Pull request description:
Currently, the user interface (noui, gui) has a caption for each message. However, the caption has many issues:
* It is always hard-coded to the empty string.
* This is confusing and tedious when reading or maintaining the code.
* It is redundant, because `noui` will ignore the caption and set the logging prefix (error, warning, info) based on the `style`.
* The gui does prefer to set the title based on the caption, but since it the caption is always empty, the fallback will always be used.
Fix all issues by removing it.
ACKs for top commit:
hebasto:
ACK fad7bd9ba3, I have reviewed the code and it looks OK. Tested on Ubuntu 25.10.
sedited:
ACK fad7bd9ba3
Tree-SHA512: 58ef538b9b3e1cfdcf2955f6de9b8cee335edbf6339723cb693cb4d584817904c962dac5199ee44d7e2860a5332dec1a6abf47e621eb5cf919aa1cdae271b55f
477c5504e0 coins: replace `std::distance` with unambiguous pointer subtraction (Lőrinc)
Pull request description:
### Problem
Calling `std::distance(nullptr, nullptr)` has ambiguous status in the C++ standard [iterator.requirements.general](https://eel.is/c++draft/iterator.requirements.general#7):
> Iterators can also have singular values that are not associated with any sequence. Results of most expressions are undefined for singular values.
It seems to work correctly in every implementation we use, but [LWG 1213](https://cplusplus.github.io/LWG/issue1213) ("Meaning of valid and singular iterator underspecified") has been Open since 2009, acknowledging that the standard's wording on this topic is unclear.
<details>
<summary>Details</summary>
The [iterator.requirements.general](https://eel.is/c++draft/iterator.requirements.general#7) states:
> Iterators can also have singular values that are not associated with any sequence. Results of most expressions are undefined for singular values.
And [LWG 208](https://cplusplus.github.io/LWG/issue208)'s rationale explicitly confirms:
> Null pointers are singular.
Therefore they cannot form a valid range required by [std::distance](https://eel.is/c++draft/iterator.operations#4):
> Preconditions: last is reachable from first, or InputIterator meets the Cpp17RandomAccessIterator requirements and first is reachable from last.
</details>
### Fix
A previous version of this PR checked both values for `nullptr`, the current one uses unambiguously well-defined pointer subtraction instead, which is per [expr.add](https://eel.is/c++draft/expr.add#5):
> If P and Q both evaluate to null pointer values, the value is 0.
This applies on the first call before any memory is allocated, when both pointers are `nullptr`.
Using `operator-` directly is simpler and avoids the ambiguity entirely.
ACKs for top commit:
maflcko:
review ACK 477c5504e0🍶
optout21:
ACK 477c5504e0
sedited:
ACK 477c5504e0
Tree-SHA512: 5edfb19ab4820e2003928f60f20d4a5893bcd3c316afdfe91c9c06e9b465352769b2cddb0d0e2419ea083a906d35f4aada74149e81f4ea0315f8173ac538789f
5aeaa71c77 lint: pass args from lint.py to cargo run in container (will)
c17a2adb8d lint: upgrade lint scripts for worktrees (will)
Pull request description:
Fixes#29972
Use a single script to run the linter locally or in CI.
Works from inside a worktree.
ACKs for top commit:
maflcko:
review ACK 5aeaa71c77🔒
davidgumberg:
code review and lightly tested reACK 5aeaa71
l0rinc:
Tested (+ lightly reviewed) ACK 5aeaa71c77
Tree-SHA512: 7c11f649b4752739d31c4f9e6306a98bd2e615b27a0819bbb5e7d9284b9e28bd9f424e145f16361f672f1a63441a1ae2f901c4f99759e997b72a4bf2d56d8d39
fa2e1b85dd build: Remove outdated comment about -ffile-prefix-map (MarcoFalke)
fa06cd4ba7 doc: Remove outdated -fdebug-prefix-map section in dev notes (MarcoFalke)
Pull request description:
This removes some docs. See the commit messages for an explanation.
ACKs for top commit:
l0rinc:
ACK fa2e1b85dd
sedited:
ACK fa2e1b85dd
Tree-SHA512: 6be33bdf9365be5fb75d39a48fd1295b193649775a00e8344123dc0f588da22f7efe80b1490dde2c74aea3d7fec6a3fa75785791296f3fb248ddf45e40b95eb7
e71c4df168 refactor: replace manual promise with SyncWithValidationInterfaceQueue (ANtutov)
Pull request description:
`BroadcastTransaction()` now waits for validation callbacks using the built-in `validation_signals>SyncWithValidationInterfaceQueue()` instead of creating a local `std::promise` and scheduling a lambda. This removes an unnecessary allocation and uses the canonical API.
ACKs for top commit:
maflcko:
review ACK e71c4df168🌃
rkrux:
lgtm ACK e71c4df168
sedited:
ACK e71c4df168
Tree-SHA512: 602994ba3c2ac91996068aee6eac7e788c3832d7ab949519a9420d2b59e2a67d2d4e67c3c9191ba60e9caa75f1524a95b0851fcd40b6732f6a9956a011b4a120
This is used for argument parsing in the retry script, however we don't
use the script with any arguments. So remove the unused code, and the
dependency on gnu-getopt.
This came up in the context of adding new CI jobs, where gnu-getopt
might not be available, or working properly. It seemed easier to just
remove the unused code, than look for more workarounds.
Verify that addresses announcing themselves (addr == source) are exempt
from time penalties, while addresses announced by others receive the
expected penalty.
14f99cfe53 rpc: make `uptime` monotonic across NTP jumps (Lőrinc)
a9440b1595 util: add `TicksSeconds` (Lőrinc)
Pull request description:
### Problem
`bitcoin-cli uptime` was derived from wall-clock time, so it could jump by large amounts when the system clock is corrected after `bitcoind` starts (e.g. on RTC-less systems syncing NTP).
This breaks the expectation that uptime reflects process runtime.
### Fix
Compute uptime from a [monotonic clock](https://en.cppreference.com/w/cpp/chrono/steady_clock.html) so it is immune to wall-clock jumps, and use that monotonic uptime for the RPC.
GUI startup time is derived from wall clock time minus monotonic uptime so it remains sensible after clock corrections.
### Reproducer
Revert the fix commit and run the `rpc_uptime` functional test (it should fail with `AssertionError: uptime should not jump with wall clock`):
Or alternatively:
```bash
cmake -B build && cmake --build build --target bitcoind bitcoin-cli -j$(nproc)
DATA_DIR=$(mktemp -d)
./build/bin/bitcoind -regtest -datadir="$DATA_DIR" -connect=0 -daemon
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" -rpcwait uptime
sleep 1
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" setmocktime $(( $(date +%s) + 20000000 ))
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" uptime
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" stop
```
<details>
<summary>Before (uptime jumps with wall clock)</summary>
```bash
Bitcoin Core starting
0
20000001
Bitcoin Core stopping
```
</details>
<details>
<summary>After (uptime stays monotonic)</summary>
```bash
Bitcoin Core starting
0
1
Bitcoin Core stopping
```
</details>
----------
Issue: https://github.com/bitcoin/bitcoin/issues/34326
ACKs for top commit:
maflcko:
review ACK 14f99cfe53🎦
willcl-ark:
tACK 14f99cfe53
w0xlt:
ACK 14f99cfe53
sedited:
ACK 14f99cfe53
Tree-SHA512: 3909973f58666ffa0b784a6df087031b9e34d2022d354900a4dbb6cbe1d36285cd92770ee71350ebf64d6e8ab212d8ff0cd851f7dca1ec46ee2f19b417f53984
Add a ci/lint.py script to run the linter both locally or inside the CI
(replacing .github/ci-lint-exec.py) which supports running from a
worktree.
Determines whether we are in a worktree, and mounts the real `.git`
directory as a read-only volume if we are.
b39291f4cd doc: fix `-logips` description to clarify that non-debug logs can also contain IP addresses (Lőrinc)
c7028d3368 init: log that additional logs may contain privacy-sensitive information (Lőrinc)
31b771a942 net: move `privatebroadcast` logs to debug category (Lőrinc)
Pull request description:
### Motivation
The recently merged [private broadcast](https://github.com/bitcoin/bitcoin/pull/29415) is a privacy feature, and users may share `debug.log` with support.
Unconditional `LogInfo()` messages that mention private broadcast and/or include (w)txids can leak sensitive context (e.g. which transactions a user originated).
Since it's meant to be a private broadcast, we should minimize leaks.
It's a best effort, it's not invalidated by other logs possibly leaking identifiable information, those can be addressed separately.
We're not promising that the logs won't ever contain data that could be used against the user, but we should still try to minimize that data, especially for a feature that's advertised as privacy-focused.
Follow up to [#29415 (comment)](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2637012294)
### Changes
* Move private-broadcast event logs from `LogInfo()` to `LogDebug(BCLog::PRIVBROADCAST, ...)`, so they are only emitted when `-debug=privatebroadcast` was explicitly provided.
* Remove hardcoded `"[privatebroadcast]"` log-string prefixes (category logging already adds the prefix).
* Keep warning at the default log level for startup failures.
* Add an init log (not a warning since that would require excessive test framework updates) when any `-debug` categories are enabled that additional logs may contain privacy-sensitive information and should not be shared publicly.
* Update a related startup arg (`-logips`) to clarify that clarify that non-debug logs can also contain IP addresses.
### Reproducer
The new warning can be checked with:
```bash
./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 | grep 'Debug logging is enabled' | wc -l
0
./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 -debug | grep 'Debug logging is enabled' | wc -l
1
```
ACKs for top commit:
janb84:
re ACK b39291f4cd
vasild:
ACK b39291f4cd
andrewtoth:
ACK b39291f4cd
frankomosh:
crACK b39291f4cd .The approach and implementation look good. Moving private broadcast logs to debug only would effectively reduce privacy leaks for users sharing logs.
sedited:
ACK b39291f4cd
Tree-SHA512: feca25ebe72a03948ba436e25f9a682947966c4c09627e8f20201ef3872ddbce1c636cd82f06be1afdc09cb80da305058667c0c2eaeadeb351311155325ea06f
3f5211cba8 test: remove child_one/child_two (w)txid variables (naiyoma)
7cfe790820 test: replace ValidWitnessMalleatedTx class with function (naiyoma)
81675a781f test: use pre-generated chain (naiyoma)
Pull request description:
This PR refactors ` ValidWitnessMalleatedTx` class into a `build_malleated_tx_package` function. As a result, two tests are updated: `mempool_accept_wtxid` and `p2p_p2p_private_broadcast`. Also included are a few small refactors in mempool_accept_wtxid , (switching to MiniWallet, using a pre-mined chain, using txid directly.)
Together, these changes reduce complexity and improve test runtime.
ACKs for top commit:
stratospher:
reACK 3f5211c.
cedwies:
reACK 3f5211c
maflcko:
review ACK 3f5211cba8👥
rkrux:
ACK 3f5211cba8
Tree-SHA512: 1fd02be3432fef6b68e54fbe8b15ed56d2699580bb13d0777b21f9cbe4c6d33bbb710541e3ca2fc93eab771d17bf1c427e4b08fa216d561bdb320cc6b36ac8fc
14e56970cb Merge bitcoin-core/secp256k1#1794: ecmult: Use size_t for array indices
c7a52400d6 Merge bitcoin-core/secp256k1#1809: release cleanup: bump version after 0.7.1
ae7eb729c0 release cleanup: bump version after 0.7.1
1a53f4961f Merge bitcoin-core/secp256k1#1808: Prepare for 0.7.1
20a209f11c release: prepare for 0.7.1
c4b6a81a60 changelog: update in preparation for the v0.7.1 release
ebb35882da Merge bitcoin-core/secp256k1#1796: bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS
c09215f7af bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS
471e3a130d Merge bitcoin-core/secp256k1#1800: sage: verify Eisenstein integer connection for GLV constants
29ac4d8491 sage: verify Eisenstein integer connection for GLV constants
4721e077b4 Merge bitcoin-core/secp256k1#1793: doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult
bd5ced1fe1 doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult
47eb70959a ecmult: Use size_t for array indices in _odd_multiplies_table
bb1d199de5 ecmult: Use size_t for array indices into tables
2d9137ce9d Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules
f9a944ff2d Merge bitcoin-core/secp256k1#1790: doc: include arg -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON for cmake
0406cfc4d1 doc: include arg -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1 for cmake
8d445730ec Merge bitcoin-core/secp256k1#1783: Add VERIFY_CHECKs and documentation that flags must be 0 or 1
aa2a39c1a7 Merge bitcoin-core/secp256k1#1778: doc/bench: Added cmake build options to bench error messages
540fec8ae9 Merge bitcoin-core/secp256k1#1788: test: split monolithic ellswift test into independent cases
d822b29021 test: split monolithic ellswift test into independent cases
ae00c552df Add VERIFY_CHECKs that flags are 0 or 1
5c75183344 Merge bitcoin-core/secp256k1#1784: refactor: remove ret from secp256k1_ec_pubkey_serialize
be5e4f02fd Merge bitcoin-core/secp256k1#1779: Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
3daab83a60 refactor: remove ret from secp256k1_ec_pubkey_serialize
8bcda186d2 test: Add non-NULL checks for "pointer of array" API functions
5a08c1bcdc Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
3b5b03f301 doc/bench: Added cmake build options to bench error messages
e7f7083b53 Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants
b6c2a3cd77 Merge bitcoin-core/secp256k1#1761: ecmult_multi: reduce strauss memory usage by 30%
f5e815f430 remove secp256k1_eckey_pubkey_serialize function
0d3659c547 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig)
adb76f82ea use new `_eckey_pubkey_serialize{33,65}` functions in public API
fc7458ca3e introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions
c8206b1ce6 Merge bitcoin-core/secp256k1#1771: ci: Use Python virtual environment in "x86_64-macos-native" job
f252da7e6e ci: Use Python virtual environment in "x86_64-macos-native" job
115b135fe8 Merge bitcoin-core/secp256k1#1763: bench: Use `ALIGNMENT` macro instead of hardcoded value
2f73e5281d group: Avoid using infinity field directly in other modules
153eea20c2 bench: Use `ALIGNMENT` macro instead of hardcoded value
26166c4f5f ecmult_multi: reduce strauss memory usage by 30%
7a2fff85e8 Merge bitcoin-core/secp256k1#1758: ci: Drop workaround for Valgrind older than 3.20.0
43e7b115f7 Merge bitcoin-core/secp256k1#1759: ci: Switch to macOS 15 Sequoia Intel-based image
8bc50b72ff ci: Switch to macOS 15 Sequoia Intel-based image
c09519f0e3 ci: Drop workaround for Valgrind older than 3.20.0
git-subtree-dir: src/secp256k1
git-subtree-split: 14e56970cba37ffe4ee992c1e08707a16e22e345
