Skip to content

Conversation

PastaPastaPasta
Copy link
Member

This backport was really annoying... But yeah, it's done now :)

laanwj and others added 3 commits April 6, 2020 15:51
…e another step towards clarifying internal interfaces

22fddde Avoid calling GetSerializeSize on each tx in a block if !fTxIndex (Matt Corallo)
2862aca Move some additional variables into CChainState private (Matt Corallo)
fd4d80a Create initial CChainState to hold chain state information (Matt Corallo)
e104f0f Move block writing out of AcceptBlock (Matt Corallo)
50701ba Move txindex/undo data disk location stuff out of ConnectBlock (Matt Corallo)
93a34cf Make DisconnectBlock unaware of where undo data resides on disk (Matt Corallo)

Pull request description:

  CChainState should eventually, essentially, be our exposed "libconsensus", but we're probably a few releases away, so the real goal is to clarify our internal interfaces. The main split was a big step, but validation.cpp is still a somewhat ranomly-mixed bag of functions that are pure functions which validate inputs (which should probably either merge with their callers or move into another file in consensus/), read/write data from disk, manipulate our current chain state (which moves into CChainState), and do mempool transaction validation.

  Obviously this is only a small step, but some effort is made to clean up what functions the functions in CChainState call, and obviously as things are added its easy to keep clear "CChainState::* cannot call anything except via callbacks through CValidationInterface, pure functions, or disk read/write things". Right now there are some glaring violations in mempool callbacks, and general flushing logic needs cleaning up (FlushStateToDisk maybe shouldnt be called, and there should be an API towards setDirtyBlockIndex, but I'll leave that for after @sipa's current changesets land).

Tree-SHA512: 69b8ec191b36b19c9492b4dee74c8057621fb6ec98ad219e8da0b2ed5c3ad711b10b5af9ff1117e8807ccf88918eeeab573be8448baecc9a59f099c53095985b
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
@@ -3729,7 +3730,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__));
return false;
}
if (!ReceivedBlockTransactions(block, state, pindex, blockPos))
if (!g_chainstate.ReceivedBlockTransactions(block, state, pindex, blockPos))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see next commit, I realized I didn't need this here and removed it

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed one part, pls see 31e3e10

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@UdjinM6 UdjinM6 added this to the 16 milestone Apr 11, 2020
@UdjinM6 UdjinM6 merged commit 93513d1 into dashpay:develop Apr 11, 2020
@PastaPastaPasta PastaPastaPasta deleted the backport-10279 branch April 11, 2020 20:05
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 8, 2022
* Merge bitcoin#10279: Add a CChainState class to validation.cpp to take another step towards clarifying internal interfaces

22fddde Avoid calling GetSerializeSize on each tx in a block if !fTxIndex (Matt Corallo)
2862aca Move some additional variables into CChainState private (Matt Corallo)
fd4d80a Create initial CChainState to hold chain state information (Matt Corallo)
e104f0f Move block writing out of AcceptBlock (Matt Corallo)
50701ba Move txindex/undo data disk location stuff out of ConnectBlock (Matt Corallo)
93a34cf Make DisconnectBlock unaware of where undo data resides on disk (Matt Corallo)

Pull request description:

  CChainState should eventually, essentially, be our exposed "libconsensus", but we're probably a few releases away, so the real goal is to clarify our internal interfaces. The main split was a big step, but validation.cpp is still a somewhat ranomly-mixed bag of functions that are pure functions which validate inputs (which should probably either merge with their callers or move into another file in consensus/), read/write data from disk, manipulate our current chain state (which moves into CChainState), and do mempool transaction validation.

  Obviously this is only a small step, but some effort is made to clean up what functions the functions in CChainState call, and obviously as things are added its easy to keep clear "CChainState::* cannot call anything except via callbacks through CValidationInterface, pure functions, or disk read/write things". Right now there are some glaring violations in mempool callbacks, and general flushing logic needs cleaning up (FlushStateToDisk maybe shouldnt be called, and there should be an API towards setDirtyBlockIndex, but I'll leave that for after @sipa's current changesets land).

Tree-SHA512: 69b8ec191b36b19c9492b4dee74c8057621fb6ec98ad219e8da0b2ed5c3ad711b10b5af9ff1117e8807ccf88918eeeab573be8448baecc9a59f099c53095985b

* fix

Signed-off-by: Pasta <pasta@dashboost.org>

* fix

Signed-off-by: Pasta <pasta@dashboost.org>

* More of 10279

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants