feat!: Add BlocksUntilNextEpoch query #2106
Conversation
github-actions
bot
added
the
C:x/provider
WalkthroughWalkthroughThe recent changes introduce a feature that allows users to query the number of blocks remaining until the next epoch in the Cosmos interchain security framework. This enhancement includes the addition of a new RPC method, related request and response messages, and a CLI command, aimed at improving usability and efficiency of blockchain interactions regarding epoch transitions. Changes
Assessment against linked issues
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional context usedPath-based instructions (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (4)
.changelog/unreleased/features/provider/2106-query-blocks-until-next-epoch.md (1)
1-1: Ensure consistency in changelog entries.Consider adding a period at the end of the sentence for consistency with other changelog entries.
docs/docs/frequently-asked-questions.md (2)
143-143: Surround fenced code blocks with blank lines.According to Markdownlint, fenced code blocks should be surrounded by blank lines.
- ```bash + + ```bashTools
Markdownlint
143-143: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
145-145: Ensure the file ends with a single newline character.According to Markdownlint, files should end with a single newline character.
- interchain-security-pd query provider blocks-until-next-epoch + interchain-security-pd query provider blocks-until-next-epoch +Tools
Markdownlint
145-145: null
Files should end with a single newline character(MD047, single-trailing-newline)
x/ccv/provider/keeper/relay.go (1)
Line range hint
152-161: Simplify the condition for better readability.The condition can be simplified to improve readability.
if k.BlocksUntilNextEpoch(ctx) == 0 { // only queue and send VSCPackets at the boundaries of an epoch // collect validator updates k.QueueVSCPackets(ctx) // try sending VSC packets to all registered consumer chains; // if the CCV channel is not established for a consumer chain, // the updates will remain queued until the channel is established k.SendVSCPackets(ctx) }
| // QueryBlocksUntilNextEpoch returns the number of blocks until the next epoch | ||
| func (k Keeper) QueryBlocksUntilNextEpoch(goCtx context.Context, req *types.QueryBlocksUntilNextEpochRequest) (*types.QueryBlocksUntilNextEpochResponse, error) { | ||
| ctx := sdk.UnwrapSDKContext(goCtx) | ||
|
|
||
| // Calculate the blocks until the next epoch | ||
| blocksUntilNextEpoch := k.BlocksUntilNextEpoch(ctx) | ||
|
|
||
| return &types.QueryBlocksUntilNextEpochResponse{BlocksUntilNextEpoch: uint64(blocksUntilNextEpoch)}, nil |
There was a problem hiding this comment.
Add error handling for null requests.
The function should handle potential null requests to prevent unexpected errors.
func (k Keeper) QueryBlocksUntilNextEpoch(goCtx context.Context, req *types.QueryBlocksUntilNextEpochRequest) (*types.QueryBlocksUntilNextEpochResponse, error) {
+ if req == nil {
+ return nil, status.Error(codes.InvalidArgument, "empty request")
+ }
ctx := sdk.UnwrapSDKContext(goCtx)
// Calculate the blocks until the next epoch
blocksUntilNextEpoch := k.BlocksUntilNextEpoch(ctx)
return &types.QueryBlocksUntilNextEpochResponse{BlocksUntilNextEpoch: uint64(blocksUntilNextEpoch)}, nil
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // QueryBlocksUntilNextEpoch returns the number of blocks until the next epoch | |
| func (k Keeper) QueryBlocksUntilNextEpoch(goCtx context.Context, req *types.QueryBlocksUntilNextEpochRequest) (*types.QueryBlocksUntilNextEpochResponse, error) { | |
| ctx := sdk.UnwrapSDKContext(goCtx) | |
| // Calculate the blocks until the next epoch | |
| blocksUntilNextEpoch := k.BlocksUntilNextEpoch(ctx) | |
| return &types.QueryBlocksUntilNextEpochResponse{BlocksUntilNextEpoch: uint64(blocksUntilNextEpoch)}, nil | |
| // QueryBlocksUntilNextEpoch returns the number of blocks until the next epoch | |
| func (k Keeper) QueryBlocksUntilNextEpoch(goCtx context.Context, req *types.QueryBlocksUntilNextEpochRequest) (*types.QueryBlocksUntilNextEpochResponse, error) { | |
| if req == nil { | |
| return nil, status.Error(codes.InvalidArgument, "empty request") | |
| } | |
| ctx := sdk.UnwrapSDKContext(goCtx) | |
| // Calculate the blocks until the next epoch | |
| blocksUntilNextEpoch := k.BlocksUntilNextEpoch(ctx) | |
| return &types.QueryBlocksUntilNextEpochResponse{BlocksUntilNextEpoch: uint64(blocksUntilNextEpoch)}, nil | |
| } |
| func CmdBlocksUntilNextEpoch() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "blocks-until-next-epoch", | ||
| Short: "Query the number of blocks until the next epoch begins and validator updates are sent to consumer chains", | ||
| Args: cobra.ExactArgs(0), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| clientCtx, err := client.GetClientQueryContext(cmd) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| queryClient := types.NewQueryClient(clientCtx) | ||
|
|
||
| req := &types.QueryBlocksUntilNextEpochRequest{} | ||
| res, err := queryClient.QueryBlocksUntilNextEpoch(cmd.Context(), req) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return clientCtx.PrintProto(res) | ||
| }, | ||
| } | ||
|
|
||
| flags.AddQueryFlagsToCmd(cmd) | ||
|
|
||
| return cmd |
There was a problem hiding this comment.
Add error handling for null responses.
The function should handle potential null responses to prevent unexpected errors.
func CmdBlocksUntilNextEpoch() *cobra.Command {
cmd := &cobra.Command{
Use: "blocks-until-next-epoch",
Short: "Query the number of blocks until the next epoch begins and validator updates are sent to consumer chains",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
queryClient := types.NewQueryClient(clientCtx)
req := &types.QueryBlocksUntilNextEpochRequest{}
res, err := queryClient.QueryBlocksUntilNextEpoch(cmd.Context(), req)
if err != nil {
return err
}
+ if res == nil {
+ return fmt.Errorf("received empty response")
+ }
return clientCtx.PrintProto(res)
},
}
flags.AddQueryFlagsToCmd(cmd)
return cmd
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func CmdBlocksUntilNextEpoch() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "blocks-until-next-epoch", | |
| Short: "Query the number of blocks until the next epoch begins and validator updates are sent to consumer chains", | |
| Args: cobra.ExactArgs(0), | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| clientCtx, err := client.GetClientQueryContext(cmd) | |
| if err != nil { | |
| return err | |
| } | |
| queryClient := types.NewQueryClient(clientCtx) | |
| req := &types.QueryBlocksUntilNextEpochRequest{} | |
| res, err := queryClient.QueryBlocksUntilNextEpoch(cmd.Context(), req) | |
| if err != nil { | |
| return err | |
| } | |
| return clientCtx.PrintProto(res) | |
| }, | |
| } | |
| flags.AddQueryFlagsToCmd(cmd) | |
| return cmd | |
| func CmdBlocksUntilNextEpoch() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "blocks-until-next-epoch", | |
| Short: "Query the number of blocks until the next epoch begins and validator updates are sent to consumer chains", | |
| Args: cobra.ExactArgs(0), | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| clientCtx, err := client.GetClientQueryContext(cmd) | |
| if err != nil { | |
| return err | |
| } | |
| queryClient := types.NewQueryClient(clientCtx) | |
| req := &types.QueryBlocksUntilNextEpochRequest{} | |
| res, err := queryClient.QueryBlocksUntilNextEpoch(cmd.Context(), req) | |
| if err != nil { | |
| return err | |
| } | |
| if res == nil { | |
| return fmt.Errorf("received empty response") | |
| } | |
| return clientCtx.PrintProto(res) | |
| }, | |
| } | |
| flags.AddQueryFlagsToCmd(cmd) | |
| return cmd | |
| } |
| @@ -162,6 +162,13 @@ func (k Keeper) EndBlockVSU(ctx sdk.Context) { | |||
| } | |||
| } | |||
|
|
|||
| // BlocksUntilNextEpoch returns the number of blocks until the next epoch starts | |||
| // Returns 0 if the current block is the last block of the epoch | |||
| func (k Keeper) BlocksUntilNextEpoch(ctx sdk.Context) int64 { | |||
There was a problem hiding this comment.
It seems you could easily move the logic inside the query itself without having to create the BlocksUntilNextEpoch method. Was this added to make testing easier? Couldn't we test the logic from inside grpc_query_test.go?
There was a problem hiding this comment.
To make tests easier and to make the query logic and the logic in relay.go more closely connected with the query. I dont see a downside, do you think this is worse now?
.changelog/unreleased/state-breaking/provider/2106-query-blocks-until-next-epoch.md
Outdated
Show resolved
Hide resolved
Co-authored-by: insumity <[email protected]>
…s-until-next-epoch.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| To query how many blocks are left until the next epoch starts, | ||
| run the following command: | ||
| ```bash |
There was a problem hiding this comment.
Surround fenced code block with blank lines.
Fenced code blocks should be surrounded by blank lines for better readability.
+
```bash
interchain-security-pd query provider blocks-until-next-epoch
<details>
<summary>Tools</summary>
<details>
<summary>Markdownlint</summary><blockquote>
146-146: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
There was a problem hiding this comment.
@p-offtermatt Is it expected that coderabbitai still looks at markdown files?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
This was branched off of main before the config change got in, so on this PR, yes it is
Description
Closes: #1940
Adds a new query that returns the blocks until the next epoch starts.
Slightly refactors the condition for sending VSCPackets in relay.go to achieve this
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if the change is state-machine breakingCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!the type prefix if the change is state-machine breakingSummary by CodeRabbit
New Features
Documentation
Tests