faca73000f ci: Install fixed version of clang-format for linters (MarcoFalke)
fa4695da4c build: Sort Makefile.am after renaming file (MarcoFalke)
cccc2784a3 scripted-diff: Move ui_interface to the node lib (MarcoFalke)
fa72ca6a9d qt: Remove unused includes (MarcoFalke)
fac96e6450 wallet: Do not include server symbols (MarcoFalke)
fa0f6c58c1 Revert "Fix link error with --enable-debug" (MarcoFalke)
Pull request description:
This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error.
The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols.
Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server.
ACKs for top commit:
Sjors:
ACK faca730
laanwj:
ACK faca73000f
hebasto:
re-ACK faca73000f, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review:
Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
5527be0627 refactor: Add AbortError alias (Hennadii Stepanov)
d924f2a596 Drop MSG_NOPREFIX flag (Hennadii Stepanov)
083daf7fba Pass bilingual_str argument to AbortNode() (Hennadii Stepanov)
d1cca129b4 refactor: Use bilingual_str::empty() (Hennadii Stepanov)
Pull request description:
This PR is a [followup](https://github.com/bitcoin/bitcoin/issues/16218#issuecomment-625919724) of #16224, and it adds `bilingual_str` type argument support to the `AbortNode()` functions.
ACKs for top commit:
MarcoFalke:
ACK 5527be0627👟
Tree-SHA512: bf8b15b14912b1f672e6e588fffa1e6eb6f00b4b23d15d0ced7f18fbdf76919244427feb7217007fe29617049308e13def893a03a87358db819cca9692f59905
b00266fe0c refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner)
Pull request description:
This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module.
For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash:
dcacea096e/src/consensus/tx_verify.cpp (L32)
ACKs for top commit:
Empact:
Code Review ACK b00266fe0c
Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
501e6ab4e7 doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call (Calvin Kim)
Pull request description:
Rationale: When ```bitcoin-cli help verifychain``` is called, the user doesn't get any documentation about the ```checklevel``` argument, leading to issues like #18995.
This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels.
ACKs for top commit:
jonatack:
ACK 501e6ab4e7 `git diff 292ed3c 501e6ab` shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway
MarcoFalke:
ACK 501e6ab4e7🚝
Tree-SHA512: 09239f79c25b5c3022b8eb1f76198ba681305d7e8775038e46becffe5f6a14c572e0c5d06b0723fe9d4a015ec42c9f7ca7b80a2a93df0b1b66f5a84a80eeeeb1
ac94141af0 validation: delay flushing undo files in syncing node case (Karl-Johan Alm)
Pull request description:
Fixes#17890. Replaces #17892.
Data files (`{blk|rev}<number>.dat`) pre-allocate space as they are written, and then trims down to the final size once they move on to the next sequence ("finalized flush"). The code currently assumes (incorrectly) that blk and rev files finish at the same time, but because blk files are written as blocks come in, and rev files are written in block height order, rev files end up being written to for awhile after moving on to the next block file, resulting in pre-allocation and waste of up to 1 MB of space per rev file.
The exact point at which rev file writing finishes is the highest height block found inside the corresponding block file, which is already available in the CBlockFileInfo vector. This PR moves finalized flushing of undo files to to directly after the undo data for the previous block file has been written.
There is a branch with annotation that demonstrates how this is handling flushing here: https://github.com/kallewoof/bitcoin/tree/200124-rev-files-annotated
ACKs for top commit:
vasild:
ACK ac94141af (no changes in the code since ed34e00da).
fjahr:
Code review re-ACK ac94141af0
jonatack:
Code review ACK ac94141af0
Tree-SHA512: 1d4e3b3d1d99bd7ebe7a2f632b1231146dd4f9f993c54db3a4090d9c086d95d2e4c327fd936066392b3afc6277b8f3a908d5c5993d4c8e49f72b92a417716dd2
789e9dd3aa validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake)
47be28c8bc validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake)
Pull request description:
Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable.
ACKs for top commit:
MarcoFalke:
ACK 789e9dd3aa
jonatack:
ACK 789e9dd3aa
Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
ActivateBestChain (ABC) is only called in the "msghand" or one of the
RPC threads, neither of which is a boost::thread. However, ABC is also
called in ThreadImport (which currently happens to be a boost::thread).
In all cases, the interruption_point is redundant with the breakpoint in
ABC that triggers when ShutdownRequested()
VerifyDB is only called in the main thread ("init") or one of the RPC
threads, neither of which is a boost::thread.
152e8baf08 Use salted hasher instead of nonce in sigcache (Jeremy Rubin)
5495fa5850 Add Hash Padding Microbenchmarks (Jeremy Rubin)
Pull request description:
This PR replaces nonces in two places with pre-salted hashers.
The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step.
I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures.
ACKs for top commit:
ryanofsky:
Code review ACK 152e8baf08. No code changes, just rebase since last review and expanded commit message
Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
0ae8f18dfe build: add -Wgnu to compile flags (fanquake)
3a0fd7726b Remove use of non-standard zero variadic macros (Ben Woosley)
49f6178c3e Drop unused LOG_TIME_MICROS helper (Ben Woosley)
5d4999951e prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes)
Pull request description:
Since we [started using](https://github.com/bitcoin/bitcoin/pull/7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile".
However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.
#### anonymous structs
```bash
./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
struct {
```
This is fixed in b849212c1e.
#### variadic macros
```bash
./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]
::Unserialize(s, VARINT(nVersionDummy));
```
This is taken care of in #18087.
The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html).
```bash
In file included from validation.cpp:22:
./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
^
./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
^
6 warnings generated.
```
This is fixed in 081a0ab64eb442bc85c4d4a4d3bc2c8e97ac2a6d and 612e8e138b97fc5ad2f38847300132a8fc423c3f.
#### prevention
To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.
This would close#14130.
Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute.
ACKs for top commit:
practicalswift:
ACK 0ae8f18dfe -- diff looks correct
MarcoFalke:
ACK 0ae8f18dfe
vasild:
utACK 0ae8f18df
dongcarl:
ACK 0ae8f18dfe
Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
faec3dc2ad init: Remove boost from ThreadImport (MarcoFalke)
Pull request description:
Can be tested by calling `-reindex` or `-loadblock` and then pressing `CTRL`+`C`.
Should print something like:
```
...
2020-04-27T19:34:31Z [loadblk] Reindexing block file blk00005.dat...
^C2020-04-27T19:34:32Z [loadblk] Shutdown requested. Exit ThreadImport
2020-04-27T19:34:32Z [qt-init] Interrupting HTTP server
...
```
ACKs for top commit:
laanwj:
Code review ACK faec3dc2ad
hebasto:
ACK faec3dc2ad, tested on Linux Mint 19.3 (x86_64) both `bitcoind` and `bitcoin-qt` binaries.
Tree-SHA512: e105af18d98296d82ec99f48e478cf44577e3c32f7e4b47617a7bc7cbf71d6becb92722f229a1be38d58ad29712704509ad9740d8ab8cd3104cf90057664b437
692f8307fc test: add test for witness commitment index (fanquake)
06442549f8 validation: Add minimum witness commitment size constant (fanquake)
Pull request description:
16101de5f3: Per [BIP 141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#Commitment_structure), the witness commitment structure is at least 38 bytes,
OP_RETURN (0x6a) + 36 (0x24) + 4 byte header (0xaa21a9ed) + 32 byte
SHA256 hash. It can be longer, however any additional data has no
consensus meaning.
54f8c48d6a: As per BIP 141, if there is more than 1 pubkey that matches the witness
commitment structure, the one with the highest output index should be
chosen. This adds a sanity check that we are doing that, which will fail
if anyone tries to "optimize" GetWitnessCommitmentIndex() by returning
early.
ACKs for top commit:
MarcoFalke:
ACK 692f8307fc🌵
jonatack:
Code review ACK 692f830
ajtowns:
ACK 692f8307fc
jnewbery:
utACK 692f8307fc
laanwj:
ACK 692f8307fc
Tree-SHA512: 7af3fe4b8a52fea2cdd0aec95f7bb935351a77b73d934bc88d6625a3503311b2a062cba5190b2228f97caa76840db3889032d910fc8e318ca8e7810a8afbafa0
Use salted hasher instead of nonce in Script Execution Cache
Don't read more than 32 bytes from GetRand
Apply g_* naming convention to scriptExecutionCache in validation.cpp
Fully apply g_* naming convention to scriptCacheHasher
Write same uint256 nonce twice for cache hash rather than calling getrand twice
Use salted hasher instead of nonce in sigcache
Use salted hasher instead of nonce in Script Execution Cache
Don't read more than 32 bytes from GetRand
Apply g_* naming convention to scriptExecutionCache in validation.cpp
Fully apply g_* naming convention to scriptCacheHasher
Write same uint256 nonce twice for cache hash rather than calling getrand twice
50fc4df6c4 [mempool] Persist unbroadcast set to mempool.dat (Amiti Uttarwar)
297a178536 [test] Integration tests for unbroadcast functionality (Amiti Uttarwar)
6851502472 [refactor/test] Extract P2PTxInvStore into test framework (Amiti Uttarwar)
dc1da48dc5 [wallet] Update the rebroadcast frequency to be ~1/day. (Amiti Uttarwar)
e25e42f20a [p2p] Reattempt initial send of unbroadcast transactions (Amiti Uttarwar)
7e93eecce3 [util] Add method that returns random time in milliseconds (Amiti Uttarwar)
89eeb4a333 [mempool] Track "unbroadcast" transactions (Amiti Uttarwar)
Pull request description:
This PR introduces mempool tracking of unbroadcast transactions and periodic reattempts at initial broadcast. This is a part of the rebroadcast project, and a standalone privacy win.
The current rebroadcast logic is terrible for privacy because 1. only the source wallet rebroadcasts transactions and 2. it does so quite frequently. In the current system, if a user submits a transaction that does not immediately get broadcast to the network (eg. they are offline), this "rebroadcast" behavior is the safety net that can actually serve as the initial broadcast. So, keeping the attempts frequent is important for initial delivery within a reasonable timespan.
This PR aims to improve # 2 by reducing the wallet rebroadcast frequency to ~1/day from ~1/15 min. It achieves this by separating the notion of initial broadcast from rebroadcasts. With these changes, the mempool tracks locally submitted transactions & periodically reattempts initial broadcast. Transactions submitted via the wallet or RPC are added to an "unbroadcast" set & are removed when a peer sends a `getdata` request, or the transaction is removed from the mempool. Every 10-15 minutes, the node reattempts an initial broadcast. This enables reducing the wallet rebroadcast frequency while ensuring the transactions will be propagated to the network.
For privacy improvements around # 1, please see #16698.
Thank you to gmaxwell for the idea of how to break out this subset of functionality (https://github.com/bitcoin/bitcoin/pull/16698#issuecomment-571399346)
ACKs for top commit:
fjahr:
Code review ACK 50fc4df6c4
MarcoFalke:
ACK 50fc4df6c4, I think this is ready for merge now 👻
amitiuttarwar:
The current tip `50fc4df` currently has 6 ACKs on it, so I've opened #18807 to address the last bits.
jnewbery:
utACK 50fc4df6c4.
ariard:
Code Review ACK 50fc4df (minor points no need to invalid other ACKs)
robot-visions:
ACK 50fc4df6c4
sipa:
utACK 50fc4df6c4
naumenkogs:
utACK 50fc4df
Tree-SHA512: 2dd935d645d5e209f8abf87bfaa3ef0e4492705ce7e89ea64279cb27ffd37f4727fa94ad62d41be331177332f8edbebf3c7f4972f8cda10dd951b80a28ab3c0f
Per BIP 141, the witness commitment structure is atleast 38 bytes,
OP_RETURN (0x6a) + 36 (0x24) + 4 byte header (0xaa21a9ed) + 32 byte
SHA256 hash. It can be longer, however any additional data has no
consensus meaning.
21fa0a44ab [docs] use consistent naming for possible_overwrite (John Newbery)
2685c214cc [tests] small whitespace fixup (John Newbery)
e9936966c0 scripted-diff: Rename PRUNED to SPENT in coins tests (John Newbery)
c205979031 [docs] Improve commenting in coins.cpp|h (John Newbery)
Pull request description:
- Add full commenting for spentness / DIRTYness / FRESHness and which combinations are valid
- Remove the 'pruned' terminology, which doesn't make sense since per-txout chainstate db was merged (#10195).
- Rename `potential_overwrite` to `possible_overwrite` to standardize terminology (there were previously examples of both, which made searching the codebase difficult).
- Make other minor improvements to the comments
ACKs for top commit:
jonatack:
Re-ACK 21fa0a4 per `git diff 98bee55 21fa0a4` the only change since my previous review is the following code commenting diff in `src/coins.cpp::L177-179`; rebuilt/ran unit tests anyway as a sanity check on the unit test changes.
Tree-SHA512: 391e01588ef5edb417250080cec17361f982c4454bc5f8c6d78bbd528c68a2bb94373297760691295c24660ce1022ad3ef7599762f736c8eed772ce096d38c3d
f63dec189c [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScripts (Pieter Wuille)
Pull request description:
This is a single commit taken from the Schnorr/Taproot PR #17977.
Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure. The spent UTXOs are required for the schnorr signature hash, since it commits to the scriptPubKeys. See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#common-signature-message for details.
By itself, this isn't really an improvement to the code, but I think it makes sense to separate out the refactor/moveonly commits from PR #17977 so that PR is only the logical changes needed for Schnorr/Taproot.
ACKs for top commit:
jonatack:
Re-ACK f63dec1 `git diff 851908d f63dec1` shows no change since last ACK.
sipa:
utACK f63dec189c
theStack:
re-ACK f63dec189c
fjahr:
Re-ACK f63dec189c
ariard:
Code Review ACK f63dec1
Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
Add a default constructor to `PrecomputedTransactionData`, which doesn't
initialize the struct's members. Instead they're initialized inside the
`CheckInputScripts()` function. This allows a later commit to add the
spent UTXOs to that structure.
c9017ce3bc protect g_chainman with cs_main (James O'Beirne)
2b081c4568 test: add basic tests for ChainstateManager (James O'Beirne)
4ae29f5f0c use ChainstateManager to initialize chainstate (James O'Beirne)
5b690f0aae refactor: move RewindBlockIndex to CChainState (James O'Beirne)
89cdf4d569 validation: introduce unused ChainstateManager (James O'Beirne)
8e2ecfe249 validation: add CChainState.m_from_snapshot_blockhash (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
This changeset introduces `ChainstateManager`, which is responsible for creating and managing access to multiple chainstates. Until we allow chainstate creation from UTXO snapshots (next assumeutxo PR?) it's basically unnecessary, but it is a prerequisite for background IBD support.
Changes are also made to the initialization process to make use of `g_chainman` and thus clear the way for multiple chainstates being loaded on startup.
One immediate benefit of this change is that we no longer have the `g_blockman` global, but instead have the ChainstateManager inject a reference of its shared BlockManager into any chainstate it creates.
Another immediate benefit is that uses of `ChainActive()` and `ChainstateActive()` are now covered by lock annotations. Because use of `g_chainman` is annotated to require cs_main, these two functions subsequently follow.
Because of whitespace changes, this diff looks bigger than it is. E.g., 4813167d98 is most easily reviewed with
```sh
git show --color-moved=dimmed_zebra -w 4813167d98
```
ACKs for top commit:
MarcoFalke:
re-ACK c9017ce3bc📙
fjahr:
Code Review Re-ACK c9017ce3bc
ariard:
Code Review ACK c9017ce
ryanofsky:
Code review ACK c9017ce3bc. No changes since last review other than a straight rebase
Tree-SHA512: 3f250d0dc95d4bfd70852ef1e39e081a4a9b71a4453f276e6d474c2ae06ad6ae6a32b4173084fe499e1e9af72dd9007f4a8a375c63ce9ac472ffeaada41ab508
Data files are pre-allocated, and upon flush/finalization, they are trimmed down to their resulting size.
Block (blk) files are written to disk as blocks come in, which is often out of order, whereas undo (rev) files are written sequentially, as each block is added to the top of the chain.
When a block file hits the size limit, the system flushes and trims the file down to its final size, and moves on to the next block file.
Case 1: blocks are added to the chain as they come in (synced up node case) -- in this case, we will flush and finalize the undo file together with the block file.
Case 2: blocks are added to the chain after they have been downloaded (syncing node case) -- in this case, we postpone finalizing the undo file until we know the undo data for the last block in the file has been written to disk.
e57980b473 [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks (John Newbery)
2dd561f361 [validation] Remove pool member from ConnectTrace (John Newbery)
969b65f3f5 [validation] Remove NotifyEntryRemoved callback from ConnectTrace (John Newbery)
5613f9842b [validation] Remove conflictedTxs from PerBlockConnectTrace (John Newbery)
cdb893443c [validation interface] Remove vtxConflicted from BlockConnected (John Newbery)
1168394d75 [wallet] Notify conflicted transactions in TransactionRemovedFromMempool (John Newbery)
Pull request description:
These boost signals were added in #9371, before we had a `TransactionRemovedFromMempool` method in the validation interface. The `NotifyEntryAdded` callback was used by validation to build a vector of conflicted transactions when connecting a block, which the wallet was notified of in the `BlockConnected` CValidationInterface callback.
Now that we have a `TransactionRemovedFromMempool` callback, we can fire that signal directly from the mempool for conflicted transactions.
Note that #9371 was implemented to ensure `-walletnotify` events were fired for these conflicted transaction. We inadvertently stopped sending these notifications in #16624 (Sep 2019 commit 7e89994). We should probably fix that, but in a different PR.
ACKs for top commit:
jonatack:
Re-ACK e57980b
ryanofsky:
Code review ACK e57980b473, no code changes since previous review, but helpful new code comments have been added and the PR description is now more clear about where the old code came from
Tree-SHA512: 3bdbaf1ef2731e788462d4756e69c42a1efdcf168691ce1bbfdaa4b7b55ac3c5b1fd4ab7b90bcdec653703600501b4224d252cfc086aef28f9ce0da3b0563a69
ec30a79f1c Fix UB with bench on genesis block (Gregory Sanders)
Pull request description:
During the loading of the genesis block, the bench print lines in ConnectTip divide by zero due to early return in ConnectBlock.
ACKs for top commit:
practicalswift:
ACK ec30a79f1c
sipa:
utACK ec30a79f1c
promag:
ACK ec30a79, `nBlocksTotal` is only used in logging.
Tree-SHA512: b3bdbb58d10d002a2293d7f99196b227ed9f4ca8c6cd08981e95cc964be47efed98b91fad276ee6da5cf7e6684610998ace7ce9bace172dd6c51c386d985b83c
I'd previously attempted to create a specialized lock for ChainstateManager,
but it turns out that because that lock would be required for functions like
ChainActive() and ChainstateActive(), it created irreconcilable lock inversions
since those functions are used so broadly throughout the codebase.
Instead, I'm just using cs_main to protect the contents of g_chainman.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
ChainstateManager is responsible for creating and managing multiple
chainstates, and will provide a high-level interface for accessing the
appropriate chainstate based upon a certain use.
Incorporates feedback from Marco Falke. Additional documentation written
by Russ Yanofsky.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
