Age | Commit message (Collapse) | Author |
|
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
84716b134e9afca2fba7731de4af1f22fa1b1518 Add "index/blockfilterindex -> validation -> index/blockfilterindex" to expected circular dependencies (Jonas Schnelli)
ab3a0a2fb915d8b8384c30a8b38b4b5cc35236fd Add functional test for blockfilterindex in prune-mode (Jonas Schnelli)
c286a22f7b63a8bd336d5d7606c339053ee0054b Add debug startup parameter -fastprune for more effective pruning tests (Jonas Schnelli)
5e112269c311a559bfded814d3c3c438349a1986 Avoid pruning below the blockfilterindex sync height (Jonas Schnelli)
00d57ff76854938ead800767fb673a8af46eac8e Avoid accessing nullpointer in BaseIndex::GetSummary() (Jonas Schnelli)
6abe9f5b11cd4a5ecb6caca8443fe2950a417842 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 84716b134e9afca2fba7731de4af1f22fa1b1518. Only changes since last review were suggested new FindFilesToPrune argument and test.
benthecarman:
tACK 84716b134e9afca2fba7731de4af1f22fa1b1518
Tree-SHA512: 91d832c6c562c463f7ec7655c08956385413a99a896640b9737bda0183607fac530435d03d87c3c0e70c61ccdfe73fe8f3639bc7d26d33ca7e60925ebb97d77a
|
|
|
|
|
|
Don't return null snapshotblockhash values to avoid caller complexity/confusion.
|
|
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.
|
|
log message to debug category
25f899cc23a791c08e19acae91bebda6c3538d37 log: Move "Pre-allocating up to position 0x[...] in [...].dat" log message to debug category (practicalswift)
acd7980b37b5a71f324f7772d72175c8bd7ab900 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 25f899cc23a791c08e19acae91bebda6c3538d37
Tree-SHA512: 5970798c41b041527ebdcbd843c5e136c257c28c3b21fc74102da8970406ca5c0c7e406305c5e6e67de5c1708dc1858af07a77a2e05f44159b7103423e8ab32f
|
|
53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408)
174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408)
f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408)
9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408)
Pull request description:
This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things:
- It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error.
- Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param.
- We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`.
- We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it.
ACKs for top commit:
MarcoFalke:
ACK 53e716ea119658c28935fee24eb50090907c500e 💿
jnewbery:
Code review ACK 53e716ea119658c28935fee24eb50090907c500e
ariard:
Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes.
Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f
|
|
|
|
|
|
ATMPArgs should contain const arguments for validation.
The Workspace should contain state that may change
throughout validation.
|
|
This creates a cleaner interface with ATMP, allows us to make results const,
and makes accessing values that don't make sense (e.g. fee when tx is
invalid) an error.
|
|
Since these chainstates are:
1. Also vulnerable to the race condition described in the previous
commit
2. Documented as having similar semantics as m_active_chainstate
we should also protect them with ::cs_main.
|
|
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong)
b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong)
0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong)
ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong)
e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong)
5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong)
2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong)
9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong)
0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong)
d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong)
f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong)
e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong)
b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong)
3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong)
eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong)
15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong)
f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
jnewbery:
utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
laanwj:
re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
ryanofsky:
Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review:
Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
|
|
c943282b5e6312537f885c811d43120ce2f5b766 validation: remove redundant check on pindex (jarolrod)
Pull request description:
This removes a redundant check on `pindex` being a `nullptr`. By the time we get to this step `pindex` is always a `nullptr` as the branch where it has been set would have already returned.
Closes #19223
ACKs for top commit:
Zero-1729:
re-ACK c943282
ajtowns:
ACK c943282b5e6312537f885c811d43120ce2f5b766 - code review only
MarcoFalke:
review ACK c943282b5e6312537f885c811d43120ce2f5b766 📨
theStack:
re-ACK c943282b5e6312537f885c811d43120ce2f5b766
Tree-SHA512: d2dc58206be61d2897b0703ee93af67abed492a80e84ea03dcfbf7116833173da3bdafa18ff80422d5296729d5254d57cc1db03fdaf817c8f57c62c3abef673c
|
|
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
LoadExternalBlockFile mainly acts on CChainState.
|
|
Instead use CChainState::ActivateBestChain, which is what the global one
calls anyway.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
GetLastCheckPoint mainly acts on BlockManager.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
GetSpendHeight only acts on BlockManager.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
FindForkInGlobalIndex only acts on BlockManager.
Note to reviewers: Since FindForkInGlobalIndex is always called with
::ChainActive() as its first parameter, it is possible to move
FindForkInGlobalIndex to CChainState and remove this const CChain&
parameter to instead use m_chain. However, it seems like the original
intention was for FindForkInGlobalIndex to work with _any_ chain, not
just the current active chain. Let me know if this should be changed.
|
|
|
|
[META] This commit should be followed up by a scripted-diff commit which
fixes calls to LookupBlockIndex tree-wide.
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
LookupBlockIndex only acts on BlockManager.
|
|
This avoids a potential race-condition where a thread is reading the
ChainstateManager::m_active_chainstate pointer while another one is
writing to it. There is no portable guarantee that reading/writing the
pointer is thread-safe.
This is also done in way that mimics ::ChainstateActive(), so the
transition from that function to this method is easy.
More discussion:
1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027
2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961
3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522
4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695
|
|
bb6fcc75d1ec94b733d1477c816351c50be5faf9 refactor: Drop boost::thread stuff in CCheckQueue (Hennadii Stepanov)
6784ac471bb32b6bb8e2de60986f123eb4990706 bench: Use CCheckQueue local thread pool (Hennadii Stepanov)
dba30695fc42f45828db008e7e5b81cb2b5d8551 test: Use CCheckQueue local thread pool (Hennadii Stepanov)
01511776acb0c7ec216dc9c8112531067763f1cb Add local thread pool to CCheckQueue (Hennadii Stepanov)
0ef938685b5c079a6f5a98daf0e3865d718d817b refactor: Use member initializers in CCheckQueue (Hennadii Stepanov)
Pull request description:
This PR:
- gets rid of `boost::thread_group` in the `CCheckQueue` class
- allows thread safety annotation usage in the `CCheckQueue` class
- is alternative to #14464 (https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-616618525, https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-617291612)
Also, with this PR (I hope) it could be easier to resurrect a bunch of brilliant ideas from #9938.
Related: #17307
ACKs for top commit:
laanwj:
Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9
LarryRuane:
ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9
jonatack:
Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9 and verified rebase to master builds cleanly with unit/functional tests green
Tree-SHA512: fddeb720d5a391b48bb4c6fa58ed34ccc3f57862fdb8e641745c021841c8340e35c5126338271446cbd98f40bd5484f27926aa6c3e76fa478ba1efafe72e73c1
|
|
This removes a conditional that checks if pindex is equal to nullptr.
This check is redundant because the branch where pindex is set returns at an earlier time. Additionaly, The independence of the earlier and later pindex is made clearer.
|
|
|
|
We should already have the mempool lock when entering
CheckInputsFromMempoolAndCache
|