Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TODO] Remove ChainState: (3) Improve tracking of Consensus tip with ChainIndexer tip #921

Open
wants to merge 14 commits into
base: release/1.6.0.0
Choose a base branch
from

Conversation

quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Mar 6, 2022

https://app.clickup.com/t/1zvm0j2

The purpose of ChainIndexer is to index the consensus blocks. As such we want to reliably synchronise the updating of the chain indexer tip with the updating of the consensus tip. This is already being accomplished to some extent via twin calls such as these:

                lock (this.peerLock)
                {
                    this.SetConsensusTipInternalLocked(current.Previous);
                    this.chainIndexer.Remove(current);
                }

and

                    lock (this.peerLock)
                    {
                        this.SetConsensusTipInternalLocked(lastValidatedBlockHeader);
                        this.chainIndexer.Add(lastValidatedBlockHeader);
                    }

However there are instances where this is not being done as rigorously:

            // After successfully connecting all blocks set the tree tip and claim the branch.
            List<int> peersToResync = this.SetConsensusTip(lastValidatedBlockHeader, blockMined);

This PR fixes that by updating the chain indexer tip within SetConsensusTip.

In fact it may be a good idea to only update the chain indexer tip from within SetConsensusTip. Tests may be an exception.

Having a ChainIndexer tip that reliably tracks the ConsensusTip would make it easier to get rid of the corresponding field held within ChainState. Some preliminary tests have shown that the substitution works and these changes make the approach safer.

@quantumagi quantumagi requested a review from zeptin March 6, 2022 05:16
@quantumagi quantumagi changed the title Improve tracking of Consensus tip with ChainIndexer tip [TODO] Improve tracking of Consensus tip with ChainIndexer tip Mar 6, 2022
@quantumagi quantumagi changed the title [TODO] Improve tracking of Consensus tip with ChainIndexer tip [TODO] Remove ChainState: (3) Improve tracking of Consensus tip with ChainIndexer tip Mar 15, 2022
@quantumagi quantumagi changed the base branch from release/1.3.0.0 to release/1.4.0.0 March 21, 2022 12:49
@quantumagi quantumagi removed the request for review from noescape00 August 20, 2022 06:50
@fassadlr fassadlr changed the base branch from release/1.4.0.0 to release/1.5.0.0 October 21, 2022 12:42
@fassadlr fassadlr added 1.6.0.0 and removed 1.4.0.0 labels Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants