f59bee3fb242c9e02781a35272cf9644f37e7fc1 fuzz: execute each file in dir without fuzz engine (Anthony Towns)
Pull request description:
Phony fuzzing (phuzzing)! Run the fuzz testing code against known inputs to detect errors. Advantage is you can easily test using the existing qa-assets datasets without having to compile with fuzzing enabled; disadvantage is that it doesn't do any actual fuzzing.
Example usage:
```
$ for a in ${QA_ASSETS}/fuzz_seed_corpus/*; do echo ${a##*/}; done | xargs -P8 -I {} /bin/sh -c "FUZZ={} test/fuzz/fuzz ${QA_ASSETS}/fuzz_seed_corpus/{}"
No fuzzer for address_deserialize.
No fuzzer for addrdb.
No fuzzer for banentry_deserialize.
addition_overflow: succeeded against 848 files in 0s.
asmap: succeeded against 981 files in 0s.
checkqueue: succeeded against 211 files in 0s.
...
```
(`-P8` says run 8 of the tasks in parallel)
If there are failures, the first one will be reported and the program will abort with output like:
```
fuzz: test/fuzz/versionbits.cpp:336: void (anonymous namespace)::versionbits_fuzz_target(FuzzBufferType): Assertion `exp_state != ThresholdState::FAILED' failed.
Error processing seed "corpus/versionbits/35345ae8e722234095810b1117a29b63af7621af"
```
Rebase of #22763, which was a rebase of #21496, but also reports the name of the fuzzer and the time taken.
Fixes#21461
Top commit has no ACKs.
Tree-SHA512: d8d046d4a309652eb13de42116276bf992480bc887ad3535a8ff18b354cb24826bc562b06af63802ec945c637f046563b6a5601d6321b46a5543127daafea09b
f865cf8ded2b2fbc82a6fbc41226d991909a6880 Add and use BlockManager::GetAllBlockIndices (Carl Dong)
28ba0313eac37e4a900b7e97af7169ce999c4024 Add and use CBlockIndexHeightOnlyComparator (Carl Dong)
12eb05df63f930969115af6dc66e2e5d02f2a517 move-only: Move CBlockIndexWorkComparator to blockstorage (Carl Dong)
c600ee38168a460d3026eae0e289c976194aad14 Only load BlockMan in BlockMan member functions (Carl Dong)
42e56d9b188f97c077ed2269e24acc0be35ece17 style-only: No need for std::pair for vSortedByHeight (Carl Dong)
3bbb6fea051f4e19bd2448e401a6c4e9b4cc7a41 style-only: Various blockstorage.cpp cleanups (Carl Dong)
5be9ee3c54dcb396ff52fc8c8b7e4e6e39ec4a3b refactor: more const annotations for uses of CBlockIndex* (Anthony Towns)
Pull request description:
The only important commit is "Only load BlockMan in BlockMan member functions", everything else is all just small style changes.
Here's the commit message, reproduced:
```
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
```
ACKs for top commit:
ajtowns:
ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 ; code review only
MarcoFalke:
review ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 🗂
Tree-SHA512: 7b204d782834e06fd7329d022e2ae860181b4e8105c33bfb928539a4ec24161dc7438a9c4d4ee279dcad77de310c160b997bb8aa18923243d0fd55ccf4ad7c3a
2efdfb88aab6496dcf2b98e0de30635bc6bade85 gui: restore Send for external signer (Sjors Provoost)
4b5a6cd14967b8ec3cb525e4cb18628de6c15091 refactor: helper function signWithExternalSigner() (Sjors Provoost)
026b5b4523317fdefc69cf5cec55f76f18ad0c0a move-only: helper function to present PSBT (Sjors Provoost)
Pull request description:
Fixes#551
For the simplest use case of a wallet with one external signer and "PSBT Controls" disabled in settings (the default), the send dialog will behave the same as when using a wallet with private keys. I.e. there's no "Create Unsigned" button.
When PSBT controls are turned on, you can now actually make a PSBT with signing it; before this PR that button would trigger a sign event and would broadcast the transaction.
In case a multisig, the Send button will sign on the device, and then fall back to presenting a PSBT (same behavior as before #441).
This PR starts with two refactoring commits to move some stuff into a helper function for improved readability in general, and to make the main commit easier to review.
ACKs for top commit:
jonatack:
utACK 2efdfb88aab6496dcf2b98e0de30635bc6bade85 diff review since my last review, code re-review, rebased to current master, verified clean debug build of each commit
luke-jr:
utACK 2efdfb88aab6496dcf2b98e0de30635bc6bade85
Tree-SHA512: e8731a0ef9e87564b2676c7b022b742d9621bba964c19dba9fd9f6961eb608737a9e1a22c0a3c8b2f2f6d583bba067606ee8392422e82082deefb20ea7b88c7c
12cc0201c26f4214d9e1dee1e6d0ddb97b7ab20f contrib: fix signet miner (sighash mismatch) (Sebastian Falbesoner)
Pull request description:
gruve-p reported that the signet miner doesn't work anymore (see https://github.com/bitcoin/bitcoin/issues/24501#issuecomment-1062088351), failing with the following error of the `walletprocesspsbt` RPC:
```
error code: -22
error message:
Specified sighash value does not match value stored in PSBT
.....
subprocess.CalledProcessError: Command '['bitcoin-cli', '-signet', '-stdin', 'walletprocesspsbt']' returned non-zero exit status 22
```
PSBT signing was changed to use SIGHASH_DEFAULT by default in #22514. The signet miner script sets the sighash type of the created PSBT to SIGHASH_ALL (3 is the per-input type PSBT_IN_SIGHASH_TYPE, following a little-endian 32 unsigned integer of the sighash type):
e04720ec33/contrib/signet/miner (L169-L170)
hence this leads to a sighash mismatch when the `walletprocesspsbt` RPC is called. Fix this by explicitly passing the correct sighash type. The same change was needed in one of our functional tests, see commit d3992669df826899a3de78a77a366dab46028026.
Note that instead of feeding the PSBT via `-stdin` it is directly passed as parameter, as I couldn't figure out a way to pass multiple parameters otherwise (separating by newline also didn't work).
ACKs for top commit:
kallewoof:
ACK 12cc0201c26f4214d9e1dee1e6d0ddb97b7ab20f
ajtowns:
ACK 12cc0201c26f4214d9e1dee1e6d0ddb97b7ab20f ; code review only
Tree-SHA512: 8509e768e96f85e28c0ca0dc2d35874aa29623febddc46bf90472ec38f38cb3a1b5407c563fd9101d07088775d0fdb18e9137cc38955e847885b83c16591c736
e359ba6b35edebf02b968fe60cae48473ed88826 doc: Drop a note about Intel-based Macs (Hennadii Stepanov)
Pull request description:
The work on building stuff during the recent months made the removed note obsolete.
ACKs for top commit:
fanquake:
ACK e359ba6b35edebf02b968fe60cae48473ed88826
Tree-SHA512: 8cf851c8602ef004c9ca009a97345b828bacbb6ecf1eee803d3ce64870a9766c196849b8843237e7bc1be5697de928b759a6dfa0407022c144d23d0293322200
5347c9732ff24bfcdcd62d0291972c86c80558dc doc: update multisig-tutorial.md to default wallet type (Jon Atack)
Pull request description:
Follow-up to #24281 and https://github.com/bitcoin/bitcoin/pull/24281#issuecomment-1033996386. The default wallet type was changed to descriptor wallets in #23002.
ACKs for top commit:
laanwj:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
michaelfolkson:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
achow101:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
theStack:
Concept and code-review ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
Tree-SHA512: 8074a33ad253ecb7d3f78645a00c808c7c224996cc1748067928aa59ef31a58f24fcfc75169494b26a19c7fbbf23bbd78516ab4102bc52fa92f08f1f49b18b63
9a5b4d78921ffd16c78d3661447faa065426da67 doc: Delete old line of code that was commented out (Michael Folkson)
Pull request description:
In #23288 MarcoFalke [highlighted](https://github.com/bitcoin/bitcoin/pull/23288/files#r739817055) an old BOOST_CHECK that was commented out and replaced by a different BOOST_CHECK. I think this can be deleted and wasn't deliberately left in by achow101.
ACKs for top commit:
achow101:
ACK 9a5b4d78921ffd16c78d3661447faa065426da67
jonatack:
ACK 9a5b4d78921ffd16c78d3661447faa065426da67
Tree-SHA512: 69aa71d362bb190c75d617766f6af945a43211ddb315ccfef5ebab2beefe020032d99f0d0d4b312aa349e605ba712a8bc91d7f0a22427681e08c95f8a6ea7e64
bce9aaf31e2b0428e686e151324f8561ad71f11f Unit tests for IsWitnessProgram and IsP2WSH. (Daniel Kraft)
Pull request description:
This adds basic unit tests for `CScript::IsPayToWitnessScriptHash` and `CScript::IsWitnessProgram`, similar to the existing tests for `CScript::IsPayToScriptHash`. These tests are probably not super important given the other existing tests for segwit related code, but may be useful in catching some errors early.
This implements #14737.
ACKs for top commit:
aureleoules:
tACK bce9aaf31e2b0428e686e151324f8561ad71f11f (`make check)`.
Tree-SHA512: 3cff5efc4ac53079289c72bfba8b1937bc103baadd32bb1fba41e78017f65f9cca17678c3202ad0711eae42b351d4132d9ed9b4e2dc07d138298691a09c4e822
3c74f775ac956de4da4fc076b2360b687531cd63 Update signapple for platform identifier fix (Andrew Chow)
Pull request description:
Apparently #23134 is caused by the platform identifier field being set to the incorrect value in our code signatures. The problem has been resolved in signapple, and so guix should point to the latest commit containing the fix.
I suppose guix does not strictly need to have this; only the macOS code signer will need to have the fix.
Fixes#23134
ACKs for top commit:
gruve-p:
ACK 3c74f775ac
hebasto:
re-ACK 3c74f775ac956de4da4fc076b2360b687531cd63
fanquake:
ACK 3c74f775ac956de4da4fc076b2360b687531cd63
Tree-SHA512: 7df844793fa77be4ddc4ef02f26980d6368b50421b7bd9a15f7d6a0c3b5c5f4f0cc0889e065689956583a2173875d33406dbe3a52a72c75a7f23a33c733c2378
fafe06c379316f165e88b8de7300a716cef25d0a bench: Sort bench_bench_bitcoin_SOURCES (MarcoFalke)
fa31dc9b714401b67480232ef552d1479f5e6902 bench: Add logging benchmark (MarcoFalke)
Pull request description:
Might make finding performance bottlenecks or regressions (https://github.com/bitcoin/bitcoin/pull/17218) easier.
For example, fuzzing relies on disabled logging to be as fast as possible.
ACKs for top commit:
dergoegge:
ACK fafe06c379316f165e88b8de7300a716cef25d0a
Tree-SHA512: dd858f3234a4dfb00bd7dec4398eb076370a4b9746aa24eecee7da86f6882398a2d086e5ab0b7c9f7321abcb135e7ffc54cc78e60d18b90379c6dba6d613b3f7
893e18059f1f92a254aa2026312a769c0e711db6 doc: rework dependencies.md (fanquake)
Pull request description:
This PR rewrites dependencies.md. The current list is hard to parse, includes information that is either incorrect and/or misleading, and duplicates info in other documentation. The list of dependencies is much smaller, because it's now just the actual dependencies of Bitcoin Core, not random Qt things, or the dependencies of other tooling. We don't need _another_ section on configure flag usage, or, to have duplicated lists of dependencies in other build docs, as that somewhat defeats the point of having dependencies.md, and just means more effort keeping things in sync.
ACKs for top commit:
MarcoFalke:
cr ACK 893e18059f1f92a254aa2026312a769c0e711db6
hebasto:
ACK 893e18059f1f92a254aa2026312a769c0e711db6, I have reviewed the code and it looks OK, I agree it can be merged.
jarolrod:
crACK 893e18059f
Tree-SHA512: 6750eaf70d5ebc9c364ade1d4b5b689e3094020eeb444a3de93b33d9a57a1577949a461f8209442d3954ccb22ab038c7e8cf6dfff5623e4f2713606b6798c37e
Before this change the send confirmation dialog would keep the Send option disabled. The Create Unsigned choice would actually send. This is potentially confusing.
With this change the Create Unsigned button will not attempt to sign and always produce a PSBT. The Send button will attempt to sign, and only return a PSBT if more signatures are needed.
When using an external signer, the Create Unsigned option only appears when PSBT controls are enabled in the wallet settings.
This commit maintains the pre-existing behavior of filling the PSBT (without signing) even when not using an external signer.
Closes#551
Co-authored-by: Jon Atack <jon@atack.com>
fad4c8934c7b4342076bd01daa4abf4d83617b17 Add RegisterMempoolRPCCommands helper (MarcoFalke)
fafd40b5413aea5f8ec47c24fdb2ccc702e7cdd7 refactor: Avoid int64_t -> size_t -> int64_t conversion (MarcoFalke)
fa2a5f301aa678aa7b6be4bb7987f2bcbdaf2be2 rpc: Move mempool RPCs to new file (MarcoFalke)
Pull request description:
The `blockchain.cpp` file is quite large. This makes it harder to navigate and increases the memory required to compile.
Improve on both issues by splitting up the mempool RPCs to a separate file.
ACKs for top commit:
promag:
Code review ACK fad4c8934c7b4342076bd01daa4abf4d83617b17.
theStack:
Code-review ACK fad4c8934c7b4342076bd01daa4abf4d83617b17 🏞️
Tree-SHA512: 7f13168ea2cbea51eaef05ca1604fddc919480a2128ec7fa6b1f9365ec5e4822c3df93eb408a19f038c627f2309fa282b9f7f7ec45e5e661fc728f6b33157f89
fa43933e3b3f8bd2992340bdd744fdab680565f8 ci: Temporarily use clang-13 to work around clang-14 TSan bug (MarcoFalke)
Pull request description:
There is an increase in intermittent issues in the TSan task. The increase correlates with Ubuntu Jammy's bump of `clang` from `clang-13` to `clang-14`.
Temporarily work around that.
ACKs for top commit:
hebasto:
ACK fa43933e3b3f8bd2992340bdd744fdab680565f8, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: adf7d385e1cddaeb2d06c3a3838f87070fb4ffdcc9a37da204b332ab28f40042bd34bd8b6d1d4710511865b4acafa7cf7303c4abf59862c5d29b168e2774da31
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 scripted-diff: Clarify CheckFinalTxAtTip name (MarcoFalke)
fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35 policy: Remove unused locktime flags (MarcoFalke)
Pull request description:
The locktime flags have many issues:
* They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5.
* They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (https://github.com/bitcoin/bitcoin/pull/6566#issuecomment-150310861)
* No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested.
* The dead code calls `GetAdjustedTime` (network adjusted time), which has its own issues. See https://github.com/bitcoin/bitcoin/issues/4521
Fix all issues by removing them
ACKs for top commit:
ajtowns:
ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1
theStack:
Code-review ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1
glozow:
ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1, agree the default arg `flags` is a massive footgun and just setting max flags is weird. Adding `AtTip` to the names makes sense to me, since they're both testing for *next* block and only ever used for {,re}addition to mempool.
Tree-SHA512: 79f4a52f34909eb598d88bbae7afe8abe5f85f45c128483d16aa83dacd0e5579e561b725d01b1e9a931d1821012a51ad2bc6fb2867f8d09ee541f9d234d696f8
