fa4068b4e2 Move minRelayTxFee to policy/settings (MacroFake)
Pull request description:
Seems a bit confusing to put policy stuff into validation, so fix that.
Also fix includes via `iwyu`.
ACKs for top commit:
ariard:
ACK fa4068b, the includes move compiles well locally.
ryanofsky:
Code review ACK fa4068b4e2. Make sense to move the global variable to policy/settings and the default constant to policy/policy. Ariard points out other constants that could be moved, which seems fine, but it seems like moving the global variable to be with other related global variables is more significant.
Tree-SHA512: adf9619002610d1877f3aef0a9e6115fc4c2ad64135a3e5100824c650b560c47f47ac28894c6214a50a7888355252a9f6f7cec98c23a771a1964160ef1ca77de
f3a50c9dfe miniscript: rename IsSane and IsSaneSubexpression to prevent misuse (Antoine Poinsot)
c5fe5163dc miniscript: nit: don't return after assert(false) (Antoine Poinsot)
7bbaca9d8d miniscript: explicit the threshold size computation in multi() (Antoine Poinsot)
8323e4249d miniscript: add an OpCode typedef for readability (Antoine Poinsot)
7a549c6c59 miniscript: mark nodes with duplicate keys as insane (Antoine Poinsot)
8c0f8bf7bc fuzz: add a Miniscript target for string representation roundtripping (Antoine Poinsot)
be34d5077b fuzz: rename and improve the Miniscript Script roundtrip target (Antoine Poinsot)
7eb70f0ac0 miniscript: tiny doc fixups (Antoine Poinsot)
5cea85f12c miniscript: split ValidSatisfactions from IsSane (Antoine Poinsot)
a0f064dc14 miniscript: introduce a CheckTimeLocksMix helper (Antoine Poinsot)
ed45ee3882 miniscript: use optional instead of bool/outarg (Antoine Poinsot)
1ab8d89fd1 miniscript: make equality operator non-recursive (Antoine Poinsot)
5922c662c0 scripted-diff: miniscript: rename 'nodetype' variables to 'fragment' (Antoine Poinsot)
c5f65db0f0 miniscript: remove a workaround for a GCC 4.8 bug (Antoine Poinsot)
Pull request description:
The Miniscript repository and the Miniscript integration PR here have been a moving target for the past months, and some final cleanups were done there that were not included here. I initially intended to add some small followup commits to #24148 but i think there are enough of them to be worth a followup PR on its own.
Some parts of the code did not change since it was initially written in 2019, and the code could use some modernization. (Use std::optional instead of out args, remove old compiler workarounds).
We refactored the helpers to be more meaningful, and also did some renaming. A new fuzz target was also added and both were merged in a single file. 2 more will be added in #24149 that will be contained in this file too.
The only behaviour change in this PR is to rule out Miniscript with duplicate keys from sane Miniscripts. In a P2WSH context, signatures can be rebounded (Miniscript does not use CODESEPARATOR) and it's reasonable to assume that reusing keys across the Script drops the malleability guarantees.
It was previously assumed such Miniscript would never exist in the first place since a compiler should never create them. We finally agreed that if one were to exist (say, written by hand or from a buggy compiler) it would be very confusing if an imported Miniscript descriptor (after #24148) with duplicate keys was deemed sane (ie, "safe to use") by Bitcoin Core. We now check for duplicate keys in the constructor.
This is (still) joint work with Pieter Wuille. (Actually he entirely authored the cleanups and code modernization.)
ACKs for top commit:
sipa:
utACK f3a50c9dfe (with the caveat that a lot of it is my own code)
sanket1729:
code review ACK f3a50c9dfe. Did not review the fuzz tests.
Tree-SHA512: c043325e4936fe25e8ece4266b46119e000c6745f88cea530fed1edf01c80f03ee6f9edc83b6e9d42ca01688d184bad16bfd967c5bb8037744e726993adf3deb
d87784ac87 kernel: SanityChecks: Return an error struct (Carl Dong)
265d6393bf Move init::SanityCheck to kernel::SanityCheck (Carl Dong)
fed085a1a4 init: Initialize globals with kernel::Context's life (Carl Dong)
7d03feef81 kernel: Introduce empty and unused kernel::Context (Carl Dong)
eeb4fc20c5 test: Use Set/UnsetGlobals in BasicTestingSetup (Carl Dong)
Pull request description:
The full `init/common.cpp` is dependent on things like ArgsManager (which we wish to remove from libbitcoinkernel in the future) and sanity checks. These aren't necessary for libbitcoinkernel so we only extract the portion that is necessary (namely `init::{Set,Unset}Globals()`.
ACKs for top commit:
theuni:
ACK d87784ac87
vasild:
ACK d87784ac87
Tree-SHA512: cd6b4923ea1865001b5f0caed9a4ff99c198d22bf74154d935dc09a47fda22ebe585ec912398cea69f722454ed1dbb4898faab5a2d02fb4c5e719c5c8d71a3f9
d2f8f1b307 use testing setup mempool in ComplexMemPool bench (glozow)
aecc332a71 create and use mempool transactions using real coins in MempoolCheck (glozow)
2118750631 [test util] to populate mempool with random transactions/packages (glozow)
5374dfc4e3 [test util] use -checkmempool for TestingSetup mempool check ratio (glozow)
d7d9c7b266 [test util] add chain name to TestChain100Setup ctor (glozow)
Pull request description:
Fixes #24634 by using the `testing_setup`'s actual mempool rather than a locally-declared mempool for running `check()`.
Also creates a test utility for populating the mempool with a bunch of random transactions. I imagine this could be useful in other places as well; it was necessary here because we needed the mempool to contain transactions *spending coins available in the current chainstate*. The existing `CreateOrderedCoins()` is insufficient because it creates coins out of thin air.
Also implements the separate suggestion to use the `TestingSetup` mempool in `ComplexMemPool` bench.
ACKs for top commit:
laanwj:
Code review ACK d2f8f1b307
Tree-SHA512: 44ab5a9e55b126b5a5bc33f05fbad1380b9c43c84736c7cf487be025e0e3f5d75216ccf5a3088b0935da817e3dacfba99d2885f75bcb6e7eaa24cd20a82c24c8
...instead of explicitly calling init::{Set,Unset}Globals.
Cool thing about this is that in both the testing and bitcoin-chainstate
codepaths, we no longer need to explicitly unset globals. The
kernel::Context goes out of scope and the globals are unset
"automatically".
Also construct kernel::Context outside of AppInitSanityChecks()
...instead of calling initialization functions directly and having to
keep around a ECCVerifyHandle member variable.
This makes the initialization codepath of our tests more closely
resemble those of AppInitMain and potentially eases the review of
subsequent commit removing init::{Set,Unset}Globals.
[META] In a future commit, we will introduce a kernel::Context which
calls init::{Set,Unset}Globals in its ctor and dtor. It will be
owned by node::NodeContext, so in the end, this patchset won't
have made the previously local ECCVerifyHandle global.
f565b2836d Fixup option name in bench message (Ben Woosley)
bf209ac7a7 doc: Fix spelling errors identified by codespell in coments (Ben Woosley)
Pull request description:
From the output [here](https://cirrus-ci.com/task/5275612980969472?logs=lint#L849):
```
src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard
src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard
src/test/miniscript_tests.cpp:227: nd ==> and, 2nd
src/test/versionbits_tests.cpp:260: everytime ==> every time
src/util/time.h:89: precicion ==> precision
src/util/time.h:90: precicion ==> precision
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
~~I left the 'nd' in miniscript_tests as-is, as it's valid miniscript,
and I'm wary of whitelisting it.~~
ACKs for top commit:
dunxen:
ACK f565b28
Tree-SHA512: 501a426c5f6f9761e2c8f980d5d955611428a827321888f53e0ae9526b0fecd43f9d1fa845fc70ae2489d77be6dc0b5b371dff55c5146f4b39ed874f4a1ea917
As stated on the website, duplicate keys make it hard to reason about
malleability as a single signature may unlock multiple paths.
We use a custom KeyCompare function instead of operator< to be explicit
about the requirement.
These checks were added in #4339, (see also #4081), to test
our back-compat stubs, however, those stubs no-longer exist (#22930),
meaning that these checks are now just testing some specific standard
library behaviour, without a particular rationale, or reason, compared
to any other standard library functions we use.
There has also been some discussion about the sanity checks in the
context of the libbitcoinkernel refactoring, see
https://github.com/bitcoin/bitcoin/pull/25065#discussion_r880668218.
Removing the checks removes the need to worry about atleast the glibcxx
checks.
Also remove the list of check from the doc in init.h, because it is
incomplete, and anyone who wants to know what checks are included can
look at the function.
4185570340 Add RPC to get mempool txs spending outputs (t-bast)
Pull request description:
We add an RPC to fetch mempool transactions spending any of the given outpoints.
Without this RPC, application developers need to first call `getrawmempool` which returns a long list of `txid`, then fetch each of these transactions individually (`getrawtransaction`) to check whether they spend the given outpoints, which wastes a lot of bandwidth (in the worst case we need to transfer the whole mempool).
For example in lightning, when we discover that one of our channel funding transactions has been spent, we need to find the spending transaction to claim our outputs from it. We are currently forced to fetch the whole mempool to do the analysis ourselves, which is quite costly.
I believe that this RPC is also generally useful when doing some introspection on your mempool after one of your transactions failed to broadcast, for example when you implement RBF at the application level. Fetching and analyzing the conflicting transaction gives you more information to successfully replace it.
ACKs for top commit:
darosior:
re-utACK 4185570340
vincenzopalazzo:
re-ACK 4185570340
danielabrozzoni:
re-tACK 4185570340
w0xlt:
reACK 4185570340
Tree-SHA512: 206687efb720308b7e0b6cf16dd0a994006c0b5a290c8eb386917a80130973a6356d0d5cae1c63a01bb29e066dd721594969db106cba7249214fcac90d2c3dbc
c4c5b9ca6e consensus/params: set default values for BIP9Deployment (Anthony Towns)
Pull request description:
Adds default values for `vDeployments` in `consensus/params.h` so that undefined behaviour is avoided if a deployment is not initialized. Also adds a check in the unit tests to alert if this is happening, since even if it doesn't invoke undefined behaviour it's probably a mistake.
ACKs for top commit:
laanwj:
Code review ACK c4c5b9ca6e
Tree-SHA512: 22d7ff86a817d9e9e67c47301fc3b7e9d5821c13565d7706711f113dea220eea29b413a7c8d029691009159cebc85a108d77cb52418734091c196bafb2b12423
f9fdcec7e9 settings: Add resetSettings() method (Ryan Ofsky)
77fabffef4 init: Remove Shutdown() node.args reset (Ryan Ofsky)
0e55bc6e7f settings: Add update/getPersistent/isIgnored methods (Ryan Ofsky)
Pull request description:
Add `interfaces::Node` `updateSetting`, `forceSetting`, `resetSettings`, `isSettingIgnored`, and `getPersistentSetting` methods so GUI is able to manipulate `settings.json` file and use and modify node settings.
(Originally this PR also contained GUI changes to unify bitcoin-qt and bitcoind persistent settings and call these methods, but the GUI commits have been dropped from this PR and moved to bitcoin-core/gui/pull/602)
ACKs for top commit:
vasild:
ACK f9fdcec7e9
hebasto:
re-ACK f9fdcec7e9, only a function renamed since my recent [review](https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-979324357).
Tree-SHA512: 4cac853ee29be96d2ff38404165b9dfb7c622b2a9c99a15979596f3484ffde0da3d9c9c372677dff5119ca7cffa6383d81037fd9889a29cc9285882a8dc0c268
This is more consistent with the other functions, as well as with the
logging output itself. If we want to make this change, we should do it
before it's all over the place.
From the output here:
src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard
src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard
src/test/miniscript_tests.cpp:227: nd ==> and, 2nd
src/test/versionbits_tests.cpp:260: everytime ==> every time
src/util/time.h:89: precicion ==> precision
src/util/time.h:90: precicion ==> precision
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
https://cirrus-ci.com/task/5275612980969472?logs=lint#L849
I added 'nd' to the spelling.ignored-words.txt, as it's valid miniscript.
e11cdc9303 logging: Add log severity level to net.cpp (klementtan)
a8290649a6 logging: Add severity level to logs. (klementtan)
Pull request description:
**Overview**: This PR introduces a new macro, `LogPrintLevel`, that allows developers to add logs with the severity level. Additionally, it will also print the log category if it is specified.
Sample log:
```
2022-03-04T16:41:15Z [opencon] [net:debug] trying connection XX.XX.XXX.XXX:YYYYY lastseen=2.7hrs
```
**Motivation**: This feature was suggested in #20576 and I believe that it will bring the following benefits:
* Allow for easier filtering of logs in `debug.log`
* Can be extended to allow users to select the minimum level of logs they would like to view (not in the scope of this PR)
**Details**:
* New log format. `... [category:level]...`. ie:
* Do not print category if `category == NONE`
* Do not print level if `level == NONE`
* If `category == NONE` and `level == NONE`, do not print any fields (current behaviour)
* Previous logging functions:
* `LogPrintf`: no changes in log as it calls `LogPrintf_` with `category = NONE` and `level = NONE`
* `LogPrint`: prints additional `[category]` field as it calls `LogPrintf_` with `category = category` and `level = NONE`
* `net.cpp`: As a proof of concept, updated logs with obvious severity (ie prefixed with `Warning/Error:..`) to use the new logging with severity.
**Testing**:
* Compiling and running `bitcoind` with this PR should instantly display logs with the category name (ie `net/tor/...`)
* Grepping for `net:debug` in `debug.log` should display the updated logs with severity level:
<details>
<summary>Code</summary>
```
$ grep "net:debug" debug.log
2022-03-04T16:41:15Z [opencon] [net:debug] trying connection XXX:YYY lastseen=2.7hrs
2022-03-04T16:41:16Z [opencon] [net:debug] trying connection XXX:YYY lastseen=16.9hrs
2022-03-04T16:41:17Z [opencon] [net:debug] trying connection XXX:YYY lastseen=93.2hrs
2022-03-04T16:41:18Z [opencon] [net:debug] trying connection XXX:YYY lastseen=2.7hrs
```
</details>
ACKs for top commit:
laanwj:
Code review and lightly tested ACK e11cdc9303
Tree-SHA512: 89a8c86667ccc0688e5acfdbd399aac1f5bec9f978a160e40b0210b0d9b8fdc338479583fc5bd2e2bc785821363f174f578d52136d228e8f638a20abbf0a568f
664a14ba7c coinstats: Move GetUTXOStats to rpc/blockchain (Carl Dong)
f100687566 kernel: Use ComputeUTXOStats in validation (Carl Dong)
faa52387e8 style-only: Rearrange using decls after scripted-diff (Carl Dong)
f329a9298c scripted-diff: Move src/kernel/coinstats to kernel:: (Carl Dong)
0e54456f04 Use only kernel/coinstats.h in index/coinstatsindex.h (Carl Dong)
80970985c9 coinstats: Split node/coinstats.h to kernel/coinstats.h (Carl Dong)
35f73ce4b2 coinstats: Move hasher codepath to kernel/coinstats (Carl Dong)
b7634fe02b Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats (Carl Dong)
1352e410a5 coinstats: Separate hasher/index lookup codepaths (Carl Dong)
524463daf6 coinstats: Return purely out-param CCoinsStats (Carl Dong)
46eb9fc56a coinstats: Extract index_requested in-member to in-param (Carl Dong)
a789f3f2b8 coinstats: Extract hash_type in-member to in-param (Carl Dong)
102294898d includes: Remove rpc/util.h -> node/coinstats.h (Carl Dong)
0848db9c35 fuzz: Remove useless GetUTXOStats fuzz case (Carl Dong)
52b1939993 kernel: Remove unnecessary blockfilter{index,}.cpp (Carl Dong)
Pull request description:
Part of: #24303
Depends on: #24322
The `GetUTXOStats` function has 2 codepaths:
- One which queries the `CoinStatsIndex` for the UTXO hash
- One which actually performs the hashing
For `libbitcoinkernel`, the only place where we call `GetUTXOStats` is in `PopulateAndValidateSnapshots`, which uses the `SHA256D` hash, and is therefore unable to use the `CoinStatsIndex` since that only provides `MuHash` hashes. Not that I think indices necessarily belong in `libbitcoinkernel` anyway.
This PR separates these 2 aforementioned codepaths of `GetUTXOStats`, uses the hashing codepath in `PopulateAndValidateSnapshots`, and removes the need to link in `index/coinstatsindex.cpp` and `node/coinstats.cpp`.
-----
Logistically, this PR:
- Extracts out the `index_requested` and `hash_type` members of `CoinStats`, which served as "in-params" to `GetUTXOStats` embedded within the `CoinStats` struct. This allows `CoinStats` to only consist of "out-param" members, and be returned by `GetUTXOStats` without needing to be an "in-out" param
- Introduce the purely virtual `UTXOHashers` class, with 3 implementations: `SHA256DHasher`, `MuHashHasher`, and `NullHasher`. These replace the existing template-based polymorphism.
- Split `GetUTXOStats` into:
- `CalculateUTXOStatsWithHasher(UTXOHasher&, ...)`, and
- `LookupUTXOStatsWithIndex(CoinStatsIndex&, ...)`
- Use `CalculateUTXOStatsWithHasher` directly where appropriate (`src/validation.cpp` and `src/fuzz`)
- Move `GetUTXOStats` to `rpc/blockchain`, which is the only place that depends on `GetUTXOStats`'s weird fallback behaviour
- Move `LookupUTXOStatsWithIndex` to `index/coinstatsindex`
Code organization:
- `src/`
- `kernel/` → only contains the hashing codepath
- `coinstats.cpp` → hashing codepath implementations
- `coinstats.h` → header for `kernel/coinstats.cpp`
- `index/` → only contains the index codepath
- `coinstatsindex.cpp` → index codepath implementations
- `coinstatsindex.h`
- `validation.cpp` → only uses the hashing codepath
- `rpc/blockchain.cpp` → uses both the hashing and index codepath, old `GetUTXOStats` fallback logic moved here as static
- `test/fuzz/coins_view.cpp` → only uses the hashing codepath
TODOs:
- [x] Commit messages could be fleshed out more
Would love any feedback!
ACKs for top commit:
laanwj:
Code review ACK 664a14ba7c
Tree-SHA512: 18722c7bd279174d2d1881fec33ea04a9b261aae1c12e998cf434ef297d8ded47de69c526c8033a2ba7abc93ba3d2ff5faf4ce05e8888c725c31cf885ce3ef73
The indexing codepath logic in node/coinstats.cpp is simple enough to be
moved into CoinStatsIndex::LookUpStats, avoiding an additional layer of
function calls. Callers are modified accordingly.
Also, add 2 missed BOOST_CHECKs to the coinstatsindex_initial_sync unit
test.
Since the removal of NODISCARD in 81d5af42f4,
the only attributes def is LIFETIMEBOUND, and it's included in many more
places that it is used.
This removes all includes which do not have an associated use of LIFETIMEBOUND,
and adds it to the following files, due to their use of the same:
* src/validationinterface.h
* src/script/standard.h
Currently, CCoinsStats is a struct with both in-params and out-params
where the hash_type and index_requested members are the only in-params.
This change removes CCoinsStats' hash_type in-param member and adds it
to the relevant functions instead.
[META] In subsequent commits, all of CCoinsStats' members which serve as
in-params will be moved out so as to make CCoinsStats a pure
out-param struct.
In the GetUTXOStats fuzz case, GetUTXOStats is always called with a
CCoinsViewCache. Which is guaranteed to throw a std::logic_error when
its ::Cursor() method is called on the first line of GetUTXOStats.
In the fuzz case, we basically catch this logic error and declare
victory if we caught it.
There is no point to fuzzing this deterministic logic.
Confirmed with IWYU that the node/coinstats.h #include is no longer
necessary.
This decouples validation.cpp from netaddress.cpp (transitively,
timedata.cpp, and asmap.cpp).
This is important for libbitcoinkernel as:
- There is no reason for the consensus engine to be coupled with
netaddress, timedata, and asmap
- Users of libbitcoinkernel can now easily supply their own
std::function that provides the adjusted time.
See the src/Makefile.am changes for some satisfying removals.
[META] Although it seems like we don't need it for just one option,
we're going to introduce another member to this struct *in the
next commit*. In future patchsets for libbitcoinkernel decoupling
it from ArgsManager, even more members will be added here.
fa1b76aeb0 Do not call global Params() when chainman is in scope (MacroFake)
fa30234be8 Do not pass CChainParams& to PeerManager::make (MacroFake)
fafe5c0ca2 Do not pass CChainParams& to BlockAssembler constructor (MacroFake)
faf012b438 Do not pass Consensus::Params& to Chainstate helpers (MacroFake)
fa4ee53dca Do not pass time getter to Chainstate helpers (MacroFake)
Pull request description:
It seems confusing to pass chain params, consensus params, or a time function around when it is not needed.
Fix this by:
* Inlining the passed time getter function. I don't see a use case why this should be mockable.
* Using `chainman.GetConsensus()` or `chainman.GetParams()`, where possible.
ACKs for top commit:
promag:
Code review ACK fa1b76aeb0.
vincenzopalazzo:
ACK fa1b76aeb0
Tree-SHA512: 1abff5cba4b4871d97f17dbcdf67bc9255ff21fa4150a79a74e39b28f0610eab3e7dee24d56872dd6e111f003b55e288958cdd467e6218368d896f191e4ec9cd
facd1fb911 refactor: Use Span of std::byte in CExtKey::SetSeed (MarcoFalke)
fae1006019 util: Add ParseHex<std::byte>() helper (MarcoFalke)
fabdf81983 test: Add test for embedded null in hex string (MarcoFalke)
Pull request description:
This adds the hex->`std::byte` helper after the `std::byte`->hex helper was added in commit 9394964f6b
ACKs for top commit:
pk-b2:
ACK facd1fb911
laanwj:
Code review ACK facd1fb911
Tree-SHA512: e2329fbdea2e580bd1618caab31f5d0e59c245a028e1236662858e621929818870b76ab6834f7ac6a46d7874dfec63f498380ad99da6efe4218f720a60e859be
fa305fd92c Add mockable clock type and TicksSinceEpoch helper (MarcoFalke)
Pull request description:
This will be used primarily by the addr time refactor (https://github.com/bitcoin/bitcoin/pull/24697) to make addr relay time type safe. However, it can also be used in other places, and can be reviewed independently, so I split it up.
ACKs for top commit:
jonatack:
ACK fa305fd92c per `git range-diff 7b3343f fa20781 fa305fd`
ajtowns:
ACK fa305fd92c
Tree-SHA512: da00200126833c1f55b1b1e68f596eab5c9254e82b188ad17779c08ffd685e198a7c5270791b4b69a858dc6ba4e051fe0c8b445d203d356d0c884f6365ee1cfe
fa9af21878 scripted-diff: Use getInt<T> over get_int/get_int64 (MacroFake)
Pull request description:
Seems better to see the return type directly and be able to modify it easier, as the return type is used for exceptions (in-range checking and parsing feedback).
ACKs for top commit:
fanquake:
ACK fa9af21878
Tree-SHA512: 284aa2527d0f663ca01550115025c9c64c787531d595f866c718f6ad09b9b0cac1e683a7d77f8009b75de990fd37166b44063ffa83fba8a04e9a31600b4c2725
Add interfaces::Node methods to give GUI finer grained control over
settings.json file. Update method is used to write settings to the file,
getPersistent and isIgnored methods are used to find out about settings
file and command line option interactions.
bf6526f4a0 [test] Remove segwit argument from build_block_on_tip() (John Newbery)
c65bf50b44 Remove fUseWTXID parameter from CBlockHeaderAndShortTxIDs constructor (John Newbery)
Pull request description:
This implements two of the suggestions from code reviews of PR 20799:
- Remove fUseWTXID parameter from CBlockHeaderAndShortTxIDs constructor
- Remove segwit argument from build_block_on_tip()
ACKs for top commit:
dergoegge:
Code review ACK bf6526f4a0
naumenkogs:
ACK bf6526f4a0
Tree-SHA512: d553791d1364b9e655183755e829b195c9b47f59c62371dbae49d9c0f8d84fec58cf18f4dde89591672ef5658e18c9cf0206c2efd70606980f87e506bc3bd4e5
All uses of CBlockHeaderAndShortTxIDs in the product code are
constructed with fUseWTXID=true, so remove the parameter.
There is one use of the CBlockHeaderAndShortTxIDs constructor with
fUseWTXID=false in the unit tests. This is used to construct a
CBlockHeaderAndShortTxIDs for a block with only the coinbase
transaction, so setting fUseWTXID to true or false makes no difference.
Suggested in https://github.com/bitcoin/bitcoin/pull/20799#pullrequestreview-963480278
bb5c24b120 validation: move g_versionbitscache into ChainstateManager (Anthony Towns)
eca22c726a test/versionbits: make versionbitscache a parameter (Anthony Towns)
d603f1d8a7 deploymentstatus: make versionbitscache a parameter (Anthony Towns)
78adef1753 refactor: use chainman instead of chainParams for DeploymentActive* (Anthony Towns)
deffe0df6c deploymentstatus: allow chainman in place of consensusParams (Anthony Towns)
eaa2e3f25c validation: move UpdateUncommittedBlockStructures and GenerateCoinbaseCommitment into ChainstateManager (Anthony Towns)
5c67e84d37 validation: replace ::Params() calls with chainstate/chainman member (Anthony Towns)
38860f93b6 validation: remove redundant CChainParams params from ChainstateManager methods (Anthony Towns)
69675ea4e7 validation: add CChainParams to ChainstateManager (Anthony Towns)
Pull request description:
Gives `ChainstateManager` a reference to the `CChainParams` its working on, and simplifies some of the functions that would otherwise take that as a parameter. Removes the `g_versionbitscache` global by moving it into `ChainstateManager`.
ACKs for top commit:
dongcarl:
reACK bb5c24b120
MarcoFalke:
review ACK bb5c24b120📙
Tree-SHA512: 3fa74905e5df561e3e74bb0b8fce6085c5311e6633e7d74c0fb0c82a907f5bbb1fd4ebc5d11d4f0b1c019bb51eabb9f6e4bcc4652a696d36a5878c807b85f121
fab9e8a29c Remove unused GetTimeSeconds (MacroFake)
Pull request description:
Seems confusing to have this helper when it is possible to get the system time in a type-safe way by simply calling `std::chrono::system_clock::now` (C++11).
This patch replaces `GetTimeSeconds` and removes it:
* in `bitcoin-cli.cpp` by `system_clock`
* in `test/fuzz/fuzz.cpp` by `steady_clock`
ACKs for top commit:
laanwj:
Code review ACK fab9e8a29c
naumenkogs:
ACK fab9e8a29c
Tree-SHA512: 517e300b0baf271cfbeebd4a0838871acbea9360f9dd23572a751335c20c1ba261b1b5ee0aec8a36abd20c94fab83ce94f46042745279aca1f0ca2f885a03b6e
ab1ea29ba1 refactor: make GetRand a template, remove GetRandInt (pasta)
Pull request description:
makes GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
This simplifies a lot of code from GetRand(std::numeric_limits<uint64_t>::max() -> GetRand<uint64_t>()
ACKs for top commit:
laanwj:
Code review ACK ab1ea29ba1
Tree-SHA512: db5082a0e21783389f1be898ae73e097b31ab48cab1a2c0e29348a4adeb545d4098193aa72a547c6baa6e8205699aafec38d6a27b3d65522fb3246f91b4daae9
