-
Notifications
You must be signed in to change notification settings - Fork 68
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
quantumagi
wants to merge
14
commits into
stratisproject:release/1.6.0.0
Choose a base branch
from
quantumagi:refchainindexer
base: release/1.6.0.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[TODO] Remove ChainState: (3) Improve tracking of Consensus tip with ChainIndexer tip #921
quantumagi
wants to merge
14
commits into
stratisproject:release/1.6.0.0
from
quantumagi:refchainindexer
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:and
However there are instances where this is not being done as rigorously:
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 theConsensusTip
would make it easier to get rid of the corresponding field held withinChainState
. Some preliminary tests have shown that the substitution works and these changes make the approach safer.