ebc4ab721b refactor: post Optional<> removal cleanups (fanquake)
57e980d13c scripted-diff: remove Optional & nullopt (fanquake)
Pull request description:
Same rationale & motivation as #21404, which turned out to be quite low in the number of potential conflicts. Lets see what the bot has to say here.
ACKs for top commit:
practicalswift:
cr ACK ebc4ab721b: patch looks correct
jnewbery:
utACK ebc4ab721b
laanwj:
Code review ACK ebc4ab721b
Tree-SHA512: 550fbeef09b9d35ddefaa805d1755c18c8fd499c4b0f77ebfece8c20296a7abd1cf6c699e2261f92fe3552deeb7555ec2a2287ffe3ab9e98bb9f8612a4d43be3
68afd3eeec tests: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) (practicalswift)
91af6b97c9 validation: Make DumpMempool(...) and LoadMempool(...) easier to test/fuzz/mock (practicalswift)
af322c7494 tests: Set errno in FuzzedFileProvider. Implement seek(..., ..., SEEK_END). (practicalswift)
Pull request description:
Add fuzzing harness for `LoadMempool(...)` and `DumpMempool(...)`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
jonatack:
Tested re-ACK 68afd3eeec
Tree-SHA512: 4b5fcaa87e6eb478611d3b68eb6859645a5e121e7e3b056ad2815699dace0a6123706ff542def371b47f4ab3ce2b8a29782026d84fb505827121e9b4cc7dac31
1a6323bdbe doc: update developer notes for removal of MakeUnique (fanquake)
3ba2840e7e scripted-diff: remove MakeUnique<T>() (fanquake)
Pull request description:
Since requiring C++17, this is just pointless abstraction. I think we should just "tear the band-aid off" and remove it. Similar to the changes happening in #21366.
Also, having a comment saying this is deprecated doesn't prevent it's usage in new code. i.e : https://github.com/bitcoin/bitcoin/pull/20946#discussion_r561949731.
The repository is fairly quiet at the moment, so any potential complaints about having to rebase should be minimal. Might as well get this over and done with.
ACKs for top commit:
jnewbery:
utACK 1a6323bdbe
practicalswift:
cr ACK 1a6323bdbe: patch looks correct
ajtowns:
ACK 1a6323bdbe -- code review only
glozow:
ACK 1a6323bdbe looks correct
Tree-SHA512: 4a14b9611b60b9b3026b54d6f5a2dce4c5d9b63a7b93d7de1307512df736503ed84bac66e7b93372c76e3117f49bf9f29cd473d3a47cb41fb2775bc10234736f
...also update comments to remove mention of ::ChainActive()
From: https://github.com/bitcoin/bitcoin/pull/20750#discussion_r579400663
> Also, what about passing a const reference instead of a pointer? I
> know this is only theoretical, but previously if the tip was nullptr,
> then Height() evaluated to -1, now it evaluates to UB
25c57d6409 [doc] Add a note about where lock annotations should go. (Amiti Uttarwar)
ad5f01b960 [validation] Move the lock annotation from function definition to declaration (Amiti Uttarwar)
Pull request description:
Based on reviewing #21188
the first commit switches the lock annotations on `CheckInputScripts` to be on the function declaration instead of on the function definition. this ensures that all call sites are checked, not just ones that come after the definition.
the second commit adds a note to the developer-notes section to clarify where the annotations should be applied.
ACKs for top commit:
MarcoFalke:
ACK 25c57d6409🥘
promag:
Code review ACK 25c57d6409.
Tree-SHA512: 61b6ef856bf6c6016d535fbdd19daf57b9e59fe54a1f30d47282a071b9b9d60b2466b044ee57929e0320cb1bdef52e7a1687cacaa27031bbc43d058ffffe22ba
Several other parameters are now redundant since they can be safely
obtained from the chainstate given that ::cs_main is locked. These are
now removed.
84716b134e Add "index/blockfilterindex -> validation -> index/blockfilterindex" to expected circular dependencies (Jonas Schnelli)
ab3a0a2fb9 Add functional test for blockfilterindex in prune-mode (Jonas Schnelli)
c286a22f7b Add debug startup parameter -fastprune for more effective pruning tests (Jonas Schnelli)
5e112269c3 Avoid pruning below the blockfilterindex sync height (Jonas Schnelli)
00d57ff768 Avoid accessing nullpointer in BaseIndex::GetSummary() (Jonas Schnelli)
6abe9f5b11 Allow blockfilter in conjunction with prune (Jonas Schnelli)
Pull request description:
Maintaining the blockfilterindexes in prune mode is possible and may lead to efficient p2p based rescans of wallets (restore backups, import/sweep keys) beyond the prune height (rescans not part of that PR).
This PR allows running the blockfilterindex(es) in conjunction with pruning.
* Bitcoind/Qt will shutdown during startup when missing block data has been detected ([re]enable `-blockfilterindex` when we already have pruned)
* manual block pruning is disabled during blockfilterindex sync
* auto-pruning is delayed during blockfilterindex sync
ToDos:
* [x] Functional tests
ACKs for top commit:
fjahr:
Code review ACK 84716b1
ryanofsky:
Code review ACK 84716b134e. Only changes since last review were suggested new FindFilesToPrune argument and test.
benthecarman:
tACK 84716b134e
Tree-SHA512: 91d832c6c562c463f7ec7655c08956385413a99a896640b9737bda0183607fac530435d03d87c3c0e70c61ccdfe73fe8f3639bc7d26d33ca7e60925ebb97d77a
Values for mainnet and testnet will be specified in a follow-up PR that can be
scrutinized accordingly. This structure is required for use in snapshot activation
logic.
25f899cc23 log: Move "Pre-allocating up to position 0x[...] in [...].dat" log message to debug category (practicalswift)
acd7980b37 log: Move "Leaving block file [...]: [...]" log message to debug category (practicalswift)
Pull request description:
Move `Pre-allocating up to position 0x[…] in […].dat` log message to debug category.
After the cleanup of `-debug=net` log messages PR (#20724) was merged recently the console log now has very high signal to noise ratio. That's great! :)
This PR increases the signal to noise ratio slightly more by moving the most common remaining implementation detail log message (`Pre-allocating up to position 0x[…] in […].dat`) to the debug category where it belongs :)
Expected standard output from `bitcoind` (when in steady state) before this patch:
```
$ src/bitcoind
…
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z Pre-allocating up to position 0x0000000 in blk00000.dat
0000-00-00T00:00:00Z Pre-allocating up to position 0x000000 in rev00000.dat
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
```
Expected standard output from `bitcoind` (when in steady state) after this patch:
```
$ src/bitcoind
…
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo)
```
I find the latter alternative much easier to visually scan for anomalies (and more aesthetically pleasing TBH!).
Non-GUI users deserve nice interfaces too :)
ACKs for top commit:
laanwj:
ACK 25f899cc23
Tree-SHA512: 5970798c41b041527ebdcbd843c5e136c257c28c3b21fc74102da8970406ca5c0c7e406305c5e6e67de5c1708dc1858af07a77a2e05f44159b7103423e8ab32f
