refactor: rename chainActive #15948

jamesob · 2019-05-03T18:54:56Z

This is part of the assumeutxo project:

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

This change refactors the chainActive reference into a ::ChainActive() call. It also distinguishes CChainState's CChain data member as m_chain instead of the current chainActive, which makes it easily confused with the global data.

The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot.

This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review.

This can't be a scripted-diff due to the confusion of the global chainActive and the CChainState member of the same name. This specific rename makes the following chainActive -> ::ChainActive() diff scriptable.

in preparation for the following scripted-diff commit.

maflcko · 2019-05-03T19:01:02Z

You might have to put the files after sed?

sed ...foobar.. $(git grep -l "chainActive" | grep -E '(h|cpp)$')

Though at the moment ChainActive() simply references `g_chainstate.m_chain`, doing this change now clears the way for multiple chainstate usage and allows us to script the diff. -BEGIN VERIFY SCRIPT- git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/$::$\{0,1\}chainActive/::ChainActive()/g' -END VERIFY SCRIPT-

jamesob · 2019-05-03T19:03:37Z

Oops, missed an xargs. Thanks @MarcoFalke.

DrahtBot · 2019-05-03T19:21:02Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15946 (Allow maintaining the blockfilterindex when using prune by jonasschnelli)
#15933 (Make chain state immutable outside of validation by MarcoFalke)
#15931 (Remove GetDepthInMainChain dependency on locked chain interface by ariard)
#15855 ([refactor] interfaces: Add missing LockAnnotation for cs_main by MarcoFalke)
#14193 (validation: Add missing mempool locks by MarcoFalke)
#13713 (Ignore new blocks when -stopatheight target has been reached by jonasschnelli)
#13582 (Extract AppInitLoadBlockIndex from AppInitMain by Empact)
#13045 ([p2p] getblock for 1-block reorgs in response to compact block message by instagibbs)
#9381 (Remove CWalletTx merging logic from AddToWallet by ryanofsky)

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.

Sjors · 2019-05-04T10:29:04Z

utACK 486c1ee

practicalswift · 2019-05-04T11:44:46Z

@jamesob Should ::ChainActive() be annotated EXCLUSIVE_LOCKS_REQUIRED(cs_main)?

maflcko · 2019-05-06T14:47:05Z

@practicalswift No, I don't think that compiles right now

promag · 2019-05-06T15:09:26Z

utACK 486c1ee.

practicalswift · 2019-05-06T15:48:19Z

utACK 486c1ee

maflcko · 2019-05-06T18:47:58Z

This will be merged tomorrow unless there are objections

486c1ee refactoring: remove unused chainActive (James O'Beirne) 631940a scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne) a3a6090 refactoring: introduce unused ChainActive() (James O'Beirne) 1b6e6fc rename: CChainState.chainActive -> m_chain (James O'Beirne) Pull request description: This is part of the assumeutxo project: Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data. The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot. This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review. ACKs for commit 486c1e: Sjors: utACK 486c1ee promag: utACK 486c1ee. practicalswift: utACK 486c1ee Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a

morcos · 2019-05-07T19:17:34Z

@jamesob Can you clean up the comments/help text that got modified by the scripted diff?

jamesob · 2019-05-07T19:21:06Z

@morcos are you saying change the "::ChainActive()" back to something else in comments? Personally I don't mind them that way (since the old comments referred to a particular symbol, i.e. "chainActive"). What were you thinking in terms of a replacement?

morcos · 2019-05-07T19:46:48Z

discussed offline, i guess the comments aren't bad, the help text bugs me slightly, but not sure i have a better solution...

486c1ee refactoring: remove unused chainActive (James O'Beirne) 631940a scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne) a3a6090 refactoring: introduce unused ChainActive() (James O'Beirne) 1b6e6fc rename: CChainState.chainActive -> m_chain (James O'Beirne) Pull request description: This is part of the assumeutxo project: Parent PR: bitcoin#15606 Issue: bitcoin#15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data. The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot. This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review. ACKs for commit 486c1e: Sjors: utACK 486c1ee promag: utACK 486c1ee. practicalswift: utACK 486c1ee Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a

Summary: Though at the moment ChainActive() simply references `g_chainstate.m_chain`, doing this change now clears the way for multiple chainstate usage and allows us to script the diff. -BEGIN VERIFY SCRIPT- `git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/$::$\{0,1\}chainActive/::ChainActive()/g'` -END VERIFY SCRIPT- Depends on D5056 Partial backport of Core [[bitcoin/bitcoin#15948 | PR15948]] bitcoin/bitcoin@631940a bitcoin/bitcoin@486c1ee Test Plan: ninja check Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5373

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

merge bitcoin#15948, bitcoin#15976, bitcoin#16194...: assumeutxo project backports (part 1)

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

jamesob added 2 commits May 3, 2019 14:38

rename: CChainState.chainActive -> m_chain

1b6e6fc

This can't be a scripted-diff due to the confusion of the global chainActive and the CChainState member of the same name. This specific rename makes the following chainActive -> ::ChainActive() diff scriptable.

refactoring: introduce unused ChainActive()

a3a6090

in preparation for the following scripted-diff commit.

jamesob added 2 commits May 3, 2019 15:02

refactoring: remove unused chainActive

486c1ee

jamesob force-pushed the 2019-05-au-chainactive branch from 8e39159 to 486c1ee Compare May 3, 2019 19:03

DrahtBot added GUI Mempool Mining P2P Refactoring RPC/REST/ZMQ Tests TX fees and policy UTXO Db and Indexes Validation Wallet labels May 3, 2019

fanquake removed GUI Mempool Mining P2P RPC/REST/ZMQ TX fees and policy Tests UTXO Db and Indexes Wallet labels May 4, 2019

practicalswift mentioned this pull request May 6, 2019

Add locking annotations for RewindBlockIndex and GetNetworkHashPS. Add missing locks. #15962

Closed

maflcko merged commit 486c1ee into bitcoin:master May 7, 2019

ryanofsky mentioned this pull request May 15, 2019

Remove CWalletTx merging logic from AddToWallet #9381

Merged

maflcko mentioned this pull request Sep 8, 2019

[WIP] [FEATURE] RPC: Add command for pruning past ChainTip #16827

Closed

jamesob mentioned this pull request Aug 5, 2021

assumeutxo #15606

Closed

18 tasks

kwvg added a commit to kwvg/dash that referenced this pull request Oct 9, 2021

merge bitcoin#15948: rename chainActive

4bc579f

kwvg mentioned this pull request Oct 9, 2021

merge bitcoin#15948, #15976, #16194...: assumeutxo project backports (part 1) dashpay/dash#4498

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Oct 10, 2021

merge bitcoin#15948: rename chainActive

c838bb8

kwvg added a commit to kwvg/dash that referenced this pull request Oct 16, 2021

merge bitcoin#15948: rename chainActive

8e5e922

kwvg added a commit to kwvg/dash that referenced this pull request Oct 16, 2021

merge bitcoin#15948: rename chainActive

659f765

kwvg added a commit to kwvg/dash that referenced this pull request Oct 16, 2021

merge bitcoin#15948: rename chainActive

e1203b1

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Oct 21, 2021

merge bitcoin#15948: rename chainActive

00e2f62

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Oct 22, 2021

merge bitcoin#15948: rename chainActive

1f436f8

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 23, 2021

Merge pull request #4498 from kittywhiskers/assumeutxo

29dbe98

merge bitcoin#15948, bitcoin#15976, bitcoin#16194...: assumeutxo project backports (part 1)

pravblockc pushed a commit to pravblockc/dash that referenced this pull request Nov 18, 2021

merge bitcoin#15948: rename chainActive

bc8c423

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: rename chainActive #15948

refactor: rename chainActive #15948

Uh oh!

jamesob commented May 3, 2019

Uh oh!

maflcko commented May 3, 2019

Uh oh!

jamesob commented May 3, 2019

Uh oh!

DrahtBot commented May 3, 2019 •

edited

Loading

Uh oh!

Sjors commented May 4, 2019

Uh oh!

practicalswift commented May 4, 2019

Uh oh!

maflcko commented May 6, 2019

Uh oh!

promag commented May 6, 2019

Uh oh!

practicalswift commented May 6, 2019

Uh oh!

maflcko commented May 6, 2019

Uh oh!

morcos commented May 7, 2019

Uh oh!

jamesob commented May 7, 2019

Uh oh!

morcos commented May 7, 2019

Uh oh!

Uh oh!

refactor: rename chainActive #15948

refactor: rename chainActive #15948

Uh oh!

Conversation

jamesob commented May 3, 2019

Uh oh!

maflcko commented May 3, 2019

Uh oh!

jamesob commented May 3, 2019

Uh oh!

DrahtBot commented May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Sjors commented May 4, 2019

Uh oh!

practicalswift commented May 4, 2019

Uh oh!

maflcko commented May 6, 2019

Uh oh!

promag commented May 6, 2019

Uh oh!

practicalswift commented May 6, 2019

Uh oh!

maflcko commented May 6, 2019

Uh oh!

morcos commented May 7, 2019

Uh oh!

jamesob commented May 7, 2019

Uh oh!

morcos commented May 7, 2019

Uh oh!

Uh oh!

DrahtBot commented May 3, 2019 •

edited

Loading