-
Notifications
You must be signed in to change notification settings - Fork 122
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
feat: add consumer genesis time query #2366
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b92e321
add query handler for consumer genesis time
sainoe 9fc6407
add consumer genesis time command to provider CLI
sainoe b14d2fc
update docs
sainoe 39bb17a
typo
sainoe c42eaee
typo #2
sainoe 35ac391
typo #3
sainoe fa02291
add godoc
sainoe fa590f8
nit
sainoe 159c510
Merge branch 'main' into sainoe/genesis-time
sainoe 17971a1
add entry to changelog
sainoe 4a5ee56
fix tests after main merge
sainoe f8a62f0
Update x/ccv/provider/keeper/grpc_query.go
mpoke 4581c86
fix merge conflicts
mpoke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
3 changes: 3 additions & 0 deletions
3
.changelog/unreleased/features/2366-add-genesis-time-query.md
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
- `[x/provider]` Add query for consumer genesis time, | ||
which corresponds to creation time of consumer clients. | ||
([\#2366](https://github.com/cosmos/interchain-security/pull/2366)) |
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't old consensus states pruned? If yes, this query would stop working after some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, needs checking.
However, we can make it so that the query runs for chains that haven't yet started (passed spawntime).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those consumers will not have a client and thus no genesis time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@insumity The reason for this query is starting the consumer chain -- creating the genesis file with a correct timestamp that will not result in expired consumer clients on the provider. The consensus state will not be pruned at least until a new client update is received by the provider (otherwise the client update will fail). By this time, the consumer chain is already running so the query is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought he was saying that after starting a chain (making it run, with relaying enabled) the initial height consensus state would get pruned. Thus, state at block 1 would not be queryable from
GetClientConsensusState
after a while.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consensus states are eventually pruned, but it takes a while. Otherwise client updates would not work. So for sure, the initial consensus state will remain on the provider for a while. The important part is that it will remain there for sure until either the consumer chain launches or the consumer light client on the provider expires.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks for the explanation.