refactor: move LoadChainTip/RelayBlocks under CChainState

jamesob commented at 5:07 pm on August 28, 2019: member

Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

Move more chainstate-related functionality to methods on CChainState. Nothing too interesting here, but needed to work with multiple chainstates. And brief to review. :)

Also fixes doc on ActivateBestChain.

DrahtBot added the label Refactoring on Aug 28, 2019

DrahtBot added the label Validation on Aug 28, 2019

DrahtBot commented at 10:34 pm on August 28, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16224 (gui: Bilingual GUI error messages by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake added this to the "In progress" column in a project

in src/init.cpp:1548 in 70e9694951 outdated

1544@@ -1545,7 +1545,7 @@ bool AppInitMain(InitInterfaces& interfaces)
1545                 }
1546 
1547                 // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
1548-                if (!ReplayBlocks(chainparams, &::ChainstateActive().CoinsDB())) {
1549+                if (!g_chainstate->ReplayBlocks(chainparams)) {

ryanofsky commented at 6:36 pm on August 29, 2019:

In commit “refactoring: move ReplayBlocks under CChainState” (70e969495191a89070a1308393a5749b37b11e8d)

Should maybe use ChainstateActive instead of g_chainstate to be consistent with surrounding code

ryanofsky approved

ryanofsky commented at 6:38 pm on August 29, 2019: member

utACK 70e969495191a89070a1308393a5749b37b11e8d

in src/validation.h:680 in 295ab2cc02 outdated

677+     * we avoid holding cs_main for an extended period of time; the length of this
678+     * call may be quite long during reindexing or a substantial reorg.
679+     *
680+     * May not be called with cs_main held. May not be called in a
681+     * validationinterface callback.
682+     */

promag commented at 10:30 am on August 30, 2019:

295ab2cc02f3ed5a1dc7ffc5c3f0e2d6015dd483

Could also document return value.

jamesob commented at 7:08 pm on September 9, 2019:

This is taken care of in #16757 so I won’t duplicate effort here.

in src/init.cpp:1560 in ab350cabd5 outdated

1556@@ -1557,8 +1557,8 @@ bool AppInitMain(InitInterfaces& interfaces)
1557                 is_coinsview_empty = fReset || fReindexChainState ||
1558                     ::ChainstateActive().CoinsTip().GetBestBlock().IsNull();
1559                 if (!is_coinsview_empty) {
1560-                    // LoadChainTip sets ::ChainActive() based on CoinsTip()'s best block
1561-                    if (!LoadChainTip(chainparams)) {
1562+                    // LoadChainTip initializes the chain based on CoinsTip()'s best block

promag commented at 10:35 am on August 30, 2019:

ab350cabd5e078ce09378258e35d6051a2d2d3ce

Maybe move&merge this comment with the one in the header?

in src/validation.cpp:4098 in 70e9694951 outdated

4050+bool CChainState::ReplayBlocks(const CChainParams& params)
4051 {
4052     LOCK(cs_main);
4053 
4054-    CCoinsViewCache cache(view);
4055+    CCoinsView& db = this->CoinsDB();

promag commented at 10:40 am on August 30, 2019:

70e969495191a89070a1308393a5749b37b11e8d

this-> 👀

jamesob commented at 7:04 pm on September 9, 2019:

Hah yeah, I guess we have yet to formalize anything around this but I prefer prefixing method calls with this-> for clarity. See the related IRC discussion.

ryanofsky commented at 1:40 pm on September 19, 2019:

re: #16743 (review)

At some point I tried to allow lowerCase() for object methods as way to distinguish calls that use the current object from calls that don’t (#14635), but agree that this-> helps without that option.

jamesob force-pushed on Sep 9, 2019

jamesob commented at 7:26 pm on September 9, 2019: member

Thanks for the reviews so far. Have addressed feedback from @ryanofsky, @promag.

DrahtBot added the label Needs rebase on Sep 16, 2019

Sjors commented at 2:14 pm on September 16, 2019: member

Code review ACK d5ec57c, modulo (documentation) rebase.

doc: fix CChainState::ActivateBestChain doc f5809d5b13

refactoring: move LoadChainTip to CChainState method bcf73d3b84

refactoring: move ReplayBlocks under CChainState 3cf36736e5

jamesob force-pushed on Sep 17, 2019

DrahtBot removed the label Needs rebase on Sep 17, 2019

ryanofsky approved

ryanofsky commented at 1:42 pm on September 19, 2019: member

Nothing too interesting here

Can confirm. utACK 3cf36736e540cf06250701f0934a7946836d000d. Removes wrapper functions and removes more ::ChainActive() and ::ChainstateActive() calls than it adds, so seems good.

MarcoFalke commented at 2:44 pm on September 19, 2019: member

ACK 3cf36736e540cf06250701f0934a7946836d000d

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK 3cf36736e540cf06250701f0934a7946836d000d
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUgWggwAk5LJRQCCsph2Kzd8DOd0PsGc1rbpdt/XaoH4u+PbxXElKrFQMTKN86iD
 8dGeChxEFLVb6k/p9VXBm7ZL6fAw6WpINbJV4yMys7ZakJ9w3taZiH96SkKp5wPm4
 983oH9jtSgIM3TV+0Y6mSDtBsEpHZz0RT+MjLdUTaUq/1Lh1ZESsokYX61rX9guth
10l6ca5P7KVlb4iOrQSeEnUHXwkmOUQKFWORra1ZDkwgBxCv8PjNXnYUF7d8WIJzIl
11sdDtEaJsONcdHbguz8Jh11U/hdzqV07v/scJYIFxCoTx7poAIiUvLG4tX0MM8yZW
12a8qYnmY1Jbq+XKRc6+T9tDL6LLXDwQiW0Y2fG2Oz2y2eVccp+16HG5CmDCTIpw/H
13POD9NkR/YIxSA6eWzP2ee1hKJJu63Bae7HCWE57jCLaC0Hla3FHYrdTbpIo70yIZ
14rHigWoZCLVmgmCRomB9IMbDpsROvG/MXH0b/0hgeRVWIuCj1tRBSetYCm1RiQGCL
15DTb1hvTS
16=7wt1
17-----END PGP SIGNATURE-----

Timestamp of file with hash e56693f3040a22575c73812bb2cbe85f4068287b758fc92f82ffa57153a7f872 -

MarcoFalke referenced this in commit 7d4bc60f1f on Sep 19, 2019

MarcoFalke merged this on Sep 19, 2019

MarcoFalke closed this on Sep 19, 2019

fanquake moved this from the "In progress" to the "Done" column in a project

sidhujag referenced this in commit d8a09acbc9 on Sep 23, 2019

jasonbcox referenced this in commit d44cb2f967 on Oct 16, 2020

kittywhiskers referenced this in commit d869ace523 on Oct 10, 2021

kittywhiskers referenced this in commit abc40527b5 on Oct 10, 2021

kittywhiskers referenced this in commit 70c72f3de3 on Oct 16, 2021

kittywhiskers referenced this in commit d361b1bbc3 on Oct 16, 2021

kittywhiskers referenced this in commit 11a91691c5 on Oct 16, 2021

kittywhiskers referenced this in commit d5f3d97feb on Oct 21, 2021

kittywhiskers referenced this in commit 510ee3581a on Oct 22, 2021

pravblockc referenced this in commit 26cc6ccf5a on Nov 18, 2021

DrahtBot locked this on Dec 16, 2021

refactor: move LoadChainTip/RelayBlocks under CChainState #16743

Conflicts