Resolve the checkpoints <-> validation circular dependency #15655

pull l2a5b1 wants to merge 1 commits into bitcoin:master from l2a5b1:patch/resolve-checkpoints-validation-cd changing 9 files +18 −68

l2a5b1 commented at 4:56 PM on March 23, 2019: contributor

This pull request attempts to resolve the checkpoints -> validation -> checkpoints circular dependency.

The circular dependency is resolved by moving the CheckPoints::GetLastCheckpoint(const CCheckpointData& data) function to validation.cpp where it used exclusively by the private function ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime).

Resolve the checkpoints <-> validation CD.

This commit resolves the checkpoints -> validation -> checkpoints
cirular dependency by moving
`CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` from
`checkpoints.cpp` to `validation.cpp`.

418d3230f8

practicalswift commented at 5:07 PM on March 23, 2019: contributor

Concept ACK
DrahtBot commented at 6:14 PM on March 23, 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:
- #14045 (refactor: Fix the chainparamsbase -> util circular dependency by Empact)
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.
promag commented at 7:12 PM on March 23, 2019: member

utACK 418d323, only GetLastCheckpoint usage is in validation.cpp and so makes sense to move it there.
in src/validation.cpp:3204 in 418d3230f8
```
3195 | @@ -3196,6 +3196,22 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc
3196 |      return commitment;
3197 |  }
3198 |  
3199 | +//! Returns last CBlockIndex* that is a checkpoint
3200 | +static CBlockIndex* GetLastCheckpoint(const CCheckpointData& data)
3201 | +{
3202 | +    const MapCheckpoints& checkpoints = data.mapCheckpoints;
3203 | +
3204 | +    for (const MapCheckpoints::value_type& i : reverse_iterate(checkpoints))
```
skwp commented at 7:34 PM on March 23, 2019:

Might be a nitpick (sorry, new to reviewing here), but single-char var names are not great. Why not use checkpoint here instead of i. Getting to more readable source will get more devs involved. Doesn't make a huge difference in this tiny loop but it sets an example for others.

promag commented at 7:39 PM on March 23, 2019:

@skwp this code was moved from src/checkpoints.cpp and we try to not change moved code.

l2a5b1 commented at 7:20 AM on March 25, 2019:

Thanks @promag, exactly this!

skwp commented at 7:28 PM on May 6, 2019:

would it be ok to create var renaming PRs if keeping code in place?
fanquake added the label Refactoring on Mar 23, 2019
practicalswift commented at 7:28 AM on March 24, 2019: contributor

utACK 418d3230f86f77dde6e817f502baff8a54b707fa

Very nice to get rid of yet another circular dependency! Thanks!
MarcoFalke commented at 1:55 PM on March 25, 2019: member

utACK 418d3230f86f77dde6e817f502baff8a54b707fa

The checkpoint-data has long been moved to chainparams, so not worth to keep two files for a single function at this point.
sipa commented at 11:14 PM on March 25, 2019: member

utACK 418d3230f86f77dde6e817f502baff8a54b707fa
MarcoFalke merged this on Apr 19, 2019
MarcoFalke closed this on Apr 19, 2019
MarcoFalke referenced this in commit ae2c19f578 on Apr 19, 2019
jasonbcox referenced this in commit 54155de050 on Sep 22, 2020
Munkybooty referenced this in commit 3eba3800b9 on Oct 1, 2021
Munkybooty referenced this in commit 52c6127dd1 on Oct 1, 2021
Munkybooty referenced this in commit 9ec6d55045 on Oct 7, 2021
Munkybooty referenced this in commit d09ab06ff4 on Oct 7, 2021
Munkybooty referenced this in commit c880f82eea on Oct 12, 2021
Munkybooty referenced this in commit 9284c3fab2 on Oct 16, 2021
Munkybooty referenced this in commit 758885edcb on Oct 20, 2021
Munkybooty referenced this in commit e9f4cec8c5 on Oct 21, 2021
Munkybooty referenced this in commit e9ab8a2033 on Oct 23, 2021
PastaPastaPasta referenced this in commit 09c6991a03 on Oct 24, 2021
PastaPastaPasta referenced this in commit 02c6ecb02f on Oct 25, 2021
pravblockc referenced this in commit 7fb916d686 on Nov 18, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Resolve the checkpoints <-> validation circular dependency #15655

Conflicts