Bitcoin allocates 32M for signature caching by default, split
between a signature cache and a script validity cache (see
Core #10192). Since 0.14 we have added an additional 32M for
rangeproof caching *and* an additional 32M for surjectionproof
caching.
These cache entries cost a bit over 32 bytes, so these are room
for a million entries....or 4Gb of rangeproofs and 300M of
surjection proofs.
Presumably we did not intend to triple memory usage relative to
Core to deal with some champagne problem in which our mempool is
overfilled ten times with pure rangeproofs. So put the total
default cache size back to 32M. This should have no performance
hit under realistic circumstances and should reduce CI OOM failures.
On my system we now use 50M rather than 110M during the fedpeg
test; we still use 18M that Core does not by having three additional
global secp contexts (one in blind.cpp, one in pegins.cpp, one in
confidential_validation.cpp) but we can settle that in a future
commit.
Several conflicts in the C++ code related to the new `flags` parameter
to `CheckSignature` and the corresponding function being renamed upstream
to `CheckSignatureECDSA`.
Several conflicts in the test harness as Steven sorta pulled the new
upstream ECKey module into the Python code, and the actual upstream
code was slightly different. Also needed to update the feature_taproot
code to always use the non-RANGEPROOF sighash since dynafed is not
enabled in the Taproot test.
Also had to pull the `set_wif` method out of `ECKey` and inline it because
otherwise it triggers a "circular inclusion" error between script.py (which
would pull in `base58_to_bytes` from address.py) and address.py (which now
pulls in some taproot EC related stuff from script.py).
Noticed that #960 does not test the "sighash rangeproof flag set but no
witnesses" case.
281fd1a4a0 Replace KeyIDHasher with SaltedSipHasher (Andrew Chow)
210b693db6 Add generic SaltedSipHasher (Andrew Chow)
95e61c1cf2 Move Hashers to util/hasher.{cpp/h} (Andrew Chow)
Pull request description:
There are existing `SaltedOutPointHasher` and `SaltedTxidHasher` classes used for `std::unordered_map` and `std::unordered_set` that could be useful in other places in the codebase. So we these to their own `saltedhash.{cpp/h}` file. An existing `KeyIDHasher` is moved there too. Additionally, `ScriptIDHasher`, `SaltedPubkeyHasher`, and `SaltedScriptHasher` are added so that they can be used in future work.
`KeyIDHasher` and `ScriptIDHasher` are not salted so that equality comparisons of maps and sets keyed by `CKeyID` and `CScriptID` will actually work.
Split from #19602 (and a few other PRs/branches I have).
ACKs for top commit:
laanwj:
Code review ACK 281fd1a4a0
jonatack:
ACK 281fd1a4a0, code review, debug build and ran bitcoind after rebasing to master @ dff0f6f753
fjahr:
utACK 281fd1a4a0
Tree-SHA512: bb03b231ccf3c9ecefc997b8da9c3770af4819f9be5b0a72997a103864e84046a2ac39b8eadf0dc9247bdccd53f86f433642e3a098882e6748341a9e7736271b
faa8f68943 Replace boost::variant with std::variant (MarcoFalke)
Pull request description:
Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency
ACKs for top commit:
fjahr:
Code review ACK faa8f68943
fanquake:
ACK faa8f68943
Tree-SHA512: 6e3aecd33b00c2e31a763f999247944d5b2ce5e3018f1965c516c1000cd08ff6703a8d50fb0be64883153da2925ae72986b8a6b96586db74057bd05d6f4986e6
31b136e580 Don't declare de facto const reference variables as non-const (practicalswift)
1c65c075ee Don't declare de facto const member functions as non-const (practicalswift)
Pull request description:
_Meta: This is the second and final part of the `const` refactoring series (part one: #20581). **I promise: no more refactoring PRs from me in a while! :)** I'll now go back to focusing on fuzzing/hardening!_
Changes in this PR:
* Don't declare de facto const member functions as non-const
* Don't declare de facto const reference variables as non-const
Awards for finding candidates for the above changes go to:
* `clang-tidy`'s [`readability-make-member-function-const`](https://clang.llvm.org/extra/clang-tidy/checks/readability-make-member-function-const.html) check ([list of `clang-tidy` checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html))
* `cppcheck`'s `constVariable` check ([list of `cppcheck` checks](https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/))
See #18920 for instructions on how to analyse Bitcoin Core using Clang Static Analysis, `clang-tidy` and `cppcheck`.
ACKs for top commit:
ajtowns:
ACK 31b136e580
jonatack:
ACK 31b136e580
theStack:
ACK 31b136e580❄️
Tree-SHA512: f58f8f00744219426874379e9f3e9331132b9b48e954d24f3a85cbb858fdcc98009ed42ef7e7b4619ae8af9fc240a6d8bfc1c438db2e97b0ecd722a80dcfeffe
819d03b932 refactor: took out unused member functions (Zero)
ed69213c2b build: enable unused member function diagnostic (Zero)
Pull request description:
This PR enables the `-Wunused-member-function` compiler diagnostic, as discussed in #19702.
> **Notice**: The `unused-member-function` diagnostic is only available on clang. Therefore, clang should be used to test this PR.
- [x] Include the `-Wunused-member-function`diagnostic in `./configure.ac`. (ed69213c2b)
- [x] Resolve the reported warnings. (819d03b932)
Currently, enabling this flag no longer reports the following warnings:
> **Note**: output from `make 2>&1 | grep "warning: unused member function" | sort | uniq -c`
```
1 index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
2 script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
1 test/util_tests.cpp:1975:14: warning: unused member function 'operator=' [-Wunused-member-function]
```
All tests have passed locally (from `make check` & `src/test/test_bitcoin`).
This PR closes#19702.
ACKs for top commit:
practicalswift:
ACK 819d03b932 - patch still looks correct :)
MarcoFalke:
ACK 819d03b932
pox:
Tested ACK 819d03b932 with clang after `make clean`. No unused member function warnings.
theStack:
tested ACK 819d03b932
Tree-SHA512: 5fdfbbb02b3dc618a90a874a5caa5e01e596fc1d14a209e75a6981f01b253f9bca0cfac8fdd758dd7151986609fb76571c3745124a29cfd4f8cbb8d82a07272e
Also re-`static`'d a method in src/script/standard.cpp which we had made globally
visible for some historical reason, but which I noticed diffing that file against
upstream.