m_cached_from_me is used to track whether a transaction is "from me", i.e. has
any inputs which belong to the wallet. This is held in memory only in
the same way that a transaction's balances are.
Github-Pull: #33268
Rebased-From: 113a422822
Instead of checking whether the total amount of inputs known by the
wallet is greater than 0, we should be checking for whether the input is
known by the wallet. This enables us to determine whether a transaction
spends an of output with an amount of 0, which is necessary for marking
0-value dust outputs as spent.
Github-Pull: #33268
Rebased-From: 39a7dbdd27
If something is imported into the wallet, it can change the 'from me'
status of a transaction. This status is only visible through
gettransaction's "fee" field which is only shown for transactions that
are 'from me'.
Github-Pull: #33268
Rebased-From: e76c2f7a41
Rather than trying to match the apt installed clang version, which is
prone to intermittent issues. i.e #33345.
Github-Pull: #33364
Rebased-From: b736052e39
d00b82fc96 doc: update manual pages for v30.0rc1 (fanquake)
25f699daa5 contrib: add bitcoin binary to gen-manpages (fanquake)
8578991348 doc: generate example bitcoin.conf (fanquake)
9b75222b5e doc: point to v30.0 release notes draft (fanquake)
e69aba63cd build: bump version to v30.0rc1 (fanquake)
Pull request description:
* Bumps version to `v30.0rc1`.
* Generates example bitcoin.conf.
* Generates the manpages (includes backport of f5887a8de4 from #33348).
* Points release-notes.md to the devwiki (https://github.com/bitcoin-core/bitcoin-devwiki/wiki/v30.0-Release-Notes-Draft).
ACKs for top commit:
hebasto:
ACK d00b82fc96. On Ubuntu 25.04, I've got the same generated files.
janb84:
ACK d00b82fc96
stickies-v:
ACK d00b82fc96 - getting identical manpages and bitcoin.conf output on macos 15.6. Other changes LGTM too.
Tree-SHA512: 7c1cf6442f2380c90d6395d07f75297718bc323a740209efaf2020d7c94598a28c73ab5a638e1fd4ddf2b38cc6aaebe046ea968688f695abf8735b0d9315cd68
9f744fffc3 build: bump CLIENT_VERSION_MAJOR to 30 (fanquake)
Pull request description:
Last step before branch off.
ACKs for top commit:
hebasto:
ACK 9f744fffc3.
Tree-SHA512: f8ddbaa56213707c4d1719a6ade89103bcc1142d71f47cc527a20669995c1598ddbd61a88487841aa794340219e956deed403d8a7c229fc8b526e67e07dd7d69
fa8f081af3 ci: Checkout latest merged pulls (MarcoFalke)
Pull request description:
Currently, the `actions/checkout@v5` checks out pull requests merged against master, which is what we want.
However, it checks out ancient/stale merge commits on a re-run. This is documented (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs):
> Re-run workflows [...] will also use the same GITHUB_SHA (commit SHA) and GITHUB_REF (git ref) of the original event that triggered the workflow run.
For example:
* https://github.com/bitcoin/bitcoin/actions/runs/17458152407/job/49579638898?pr=29641#step:9:914 compiles with IPC=ON, even though latest master is at ed2ff3c63d
* https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3133536724 (example explained in comment)
This is problematic, because:
* Unrelated CI failures and intermittent issues, which are fixed or worked around in latest master can not be cleaned by re-running the task. The author has to actively go out and (force-)push the branch, invalidating review.
* It is odd to have a recent CI run, but it uses code and config from the past.
* Detecting silent merge conflicts by re-running the CI task is impossible.
Fix all issues by checking out the latest merged state of the pull request. The behavior is unchanged for non-pull-request actions. This patch changes the "re-run" default behaviour. Forcing it to use the new state instead of running the old state again.
ACKs for top commit:
janb84:
re ACK fa8f081af3
hebasto:
ACK fa8f081af3.
Tree-SHA512: c22c6f837402f61ec46be46817473e1946424b5312e36ed0e246cadb1ca89c04163bb471f71c309765a3d327f198a83cd83679d231f03828a99a97562a622fdd
5eeb2facbb ci: reduce runner sizes on various jobs (will)
Pull request description:
These jobs can likely use reduced runner sizes to avoid wasting our CPU quota, as much of the long-running part of the job is single-threaded.
This will also give us more (job) parallelisem from the same number of CPU that we are using.
Suggested in: https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2321775620
ACKs for top commit:
kevkevinpal:
ACK [5eeb2fa](5eeb2facbb)
m3dwards:
ACK 5eeb2facbb
janb84:
ACK 5eeb2facbb
Tree-SHA512: 6fb0352bc40623dd63b9bd6169d753d1ec9667c272445fda7a2db8bbedfa35350a51d08c1adf3fa5e070e84855c3f491668726d3c7ded07a39f2f9c63edacefc
790b440197 Fix benchmark CSV output (Hennadii Stepanov)
Pull request description:
The `SHA256AutoDetect` return output is used, among other use cases, to name benchmarks. Using a comma breaks the `bench_bitcoin` CSV output.
This PR replaces the comma with a semicolon, which fixes https://github.com/bitcoin/bitcoin/issues/33331.
ACKs for top commit:
Raimo33:
Code Review ACK 790b440197
l0rinc:
Code review ACK 790b440197
janb84:
code review ACK 790b440197
Tree-SHA512: 096bfa29a0639a4d97d510a3e2a15f071f384148c3035e4d0fc525794682e499c45a0d0c95728d5c78010098393b2c486a7fa9c21c1e2fbb600dea7c5638a55f
3cceda9f48 guix: strip binaries in libexec (fanquake)
Pull request description:
#31679 moved some internal binaries to `libexec/`, but the Guix build wasn't updated to stip these binaries of their debug symbols.
ACKs for top commit:
achow101:
ACK 3cceda9f48
ryanofsky:
Code review ACK 3cceda9f48. Good catch and thanks for the fix
hebasto:
ACK 3cceda9f48, I've checked Guix build outputs.
Tree-SHA512: 96ad57c2d3670a9ae8c58cdb428918d1dc0fa90014bb7c6fb7a7a68b3ece3fbea9c4fac90a626a005a0edb3cca8b6a33adc9a037fe6b915319387588ffe09e7b
8b62647680 test: send duplicate blocktxn message in p2p_compactblocks.py (Eugene Siegel)
5e585a0fc4 net: check for empty header before calling FillBlock (Eugene Siegel)
Pull request description:
This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the call right before the second `FillBlock` would crash [here](689a321976/src/net_processing.cpp (L3333)) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.
ACKs for top commit:
instagibbs:
reACK 8b62647680
fjahr:
tACK 8b62647680
achow101:
ACK 8b62647680
mzumsande:
Code Review ACK 8b62647680
Tree-SHA512: d43a6f652161d4f7e6137f207a3e95259fc51509279d20347b1698c91179c39c8fcb75d2668b13a6b220f478a03578573208a415804be1d8843acb057fa1a73a
c767974811 clang-tidy: Fix critical warnings (Fabian Jahr)
54dc34ec22 index: Remove unused coinstatsindex recovery code (Fabian Jahr)
37c4fba1f4 index: Check BIP30 blocks when rewinding Coinstatsindex (Fabian Jahr)
51df9de8e5 doc: Add release note for 30469 (Fabian Jahr)
bb8d673183 test: Add coinstatsindex compatibility test (Fabian Jahr)
b2e8b64ddc index, refactor: Append blocks to coinstatsindex without db read (Fabian Jahr)
431a076ae6 index: Fix coinstatsindex overflow issue (Fabian Jahr)
84e813a02b index, refactor: DRY coinbase check (Fabian Jahr)
fab842b324 index, refactor: Rename ReverseBlock to RevertBlock (Fabian Jahr)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/26362
This continues the work that was started with #26426. It fixes the overflow issue by switching the tracked values that are in danger of overflowing from `CAmount` to `arith_uint256`.
The current approach opts for a simple solution to ensure compatibility with datadirs including the previous version of the index: The new version of the index goes into a separate location in the datadir (`index/coinstatsindex/` rather than `index/coinstats/` before, the new naming is more consistent with the naming of the other indexes). There is no explicit concept of versioning of the index which earlier versions of this PR had. Having the two different versions of the index in separate places allows for downgrading of the node without having to rebuild the index. However, there will be a warning printed in the logs if the new code (v30) detects the old index still being present. A future version could delete a left-over legacy index automatically.
The PR also includes several minor improvements but most notably it lets new entries be calculated and stored without needing to read any DB records.
ACKs for top commit:
achow101:
ACK c767974811
TheCharlatan:
ACK c767974811
mzumsande:
Tested / Code Review ACK c767974811
Tree-SHA512: 3fa4a19dd1a01c1b01390247bc9daa6871eece7c1899eac976e0cc21ede09c79c65f758d14daafc46a43c4ddd7055c85fb28ff03029132d48936b248639c6ab9
Previously in debug builds, this would cause an Assume crash if
FillBlock had been called previously. This could happen when multiple
blocktxn messages were received.
Co-Authored-By: Greg Sanders <gsanders87@gmail.com>
188de70c86 net: Add interrupt to pcp retry loop (TheCharlatan)
Pull request description:
Without this interrupt bitcoind takes a long time to exit if requested to do so after a failed pcp lookup on startup.
ACKs for top commit:
achow101:
ACK 188de70c86
fjahr:
utACK 188de70c86
hodlinator:
utACK 188de70c86
Tree-SHA512: 426dabd10ac0ef5de246c83d281ba70957e4032d251054aa6028b4d7ce4e35cd35ac70e67dc07bd418673bcdd2f4457b76f174ac5e7d0dd3caa05de5da952dac
589b65f06c clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc` (Hennadii Stepanov)
Pull request description:
The warnings are false positive and have been fixed upstream. See: https://github.com/capnproto/capnproto/pull/2334.
This PR:
1. Disables the `UndefinedBinaryOperatorResult` clang-tidy check for source files generated by the `mpgen` tool.
2. Is an alternative to the draft https://github.com/bitcoin/bitcoin/pull/33281.
3. Fixes https://github.com/bitcoin/bitcoin/issues/33256.
ACKs for top commit:
Sjors:
ACK 589b65f06c
fjahr:
ACK 589b65f06c
achow101:
ACK 589b65f06c
ryanofsky:
Code review ACK 589b65f06c. Thanks for the fix!
Tree-SHA512: 6d376a82641a5b85d4dd1fa164fdcbd8e15f1262e7d4f582f4d9959031d35852e28ff1b8268336e39ba6779fdd10ecdb986af42407d0545f4217f41d64556272
The `SHA256AutoDetect` return output is used, among other use cases, to
name benchmarks. Using a comma breaks the CSV output.
This change replaces the comma with a semicolon, which fixes the issue.
a2a35b58cb doc: fix LIBRARY_PATH comment (fanquake)
Pull request description:
Now that we build capnp, qt isn't the only native package.
ACKs for top commit:
l0rinc:
code review ACK a2a35b58cb
willcl-ark:
ACK a2a35b58cb
Tree-SHA512: 7b4a54b53b4c673e31dea011e569c6ef9e7238a56a7f09c7f5a7bc521fb0ffdda3794f2b65764908005333a30f7c8c7b4d6b45d985bc92a70dc2daf8c3d1ff3d
The std::move in coinstatsindex was not necessary since it was passed as a const reference argument.
The other change in the utxo supply fuzz test changes a line that seems to have triggered a false alarm.
The coinstatsindex currently looks for block data at a hash key if the prev block in CustomAppend is different than expected. This is not needed since base index should always prevent us ending up in this scenario since it should rewind the index before calling CustomAppend in this case. But even if we run into this and our belt-and-suspenders code is getting hit, the index could not recover properly from the hash key index data so it can be removed without any real impact.
This is practically irrelevant due to the unlikeliness of a re-org
reaching so deep that it would drop the BIP30 blocks from the chain
(91842 and 91880). However this serves as documentation and ensures that
the functions RevertBlock and CustomAppend are consistent.
The index originally stored cumulative values in a CAmount type but this allowed for
potential overflow issues which were observed on Signet. Fix this by
storing the values that are in danger of overflowing in a arith_uint256.
Also turns an unnecessary copy into a reference in RevertBlock and
CustomAppend and gets
rid of the explicit total unspendable tracking which can be calculated
by adding the four categories of unspendables together.
