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

feat!: Add BlocksUntilNextEpoch query #2106

Merged
merged 11 commits into from
Jul 30, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add a query to get the blocks until the next epoch begins
([\#2106](https://github.com/cosmos/interchain-security/pull/2106))
13 changes: 12 additions & 1 deletion docs/docs/frequently-asked-questions.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,15 @@ By setting the commission rate ahead of time, they can make sure that they immed
Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal).

## Can a Top N consumer chain become Opt-In or vice versa?
Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal).
Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal).

p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
## How do I know when the next validator update will be sent to the consumer chain?
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
Validator updates are sent to the consumer chain every `BlocksPerEpoch` blocks.
Keep in mind that depending on the status of relayers between the Hub and the consumer chain,
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
it might take a while for the validator update to be processed and applied on the consumer chain.

To query how many blocks are left until the next epoch starts,
run the following command:
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p-offtermatt Is it expected that coderabbitai still looks at markdown files?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was branched off of main before the config change got in, so on this PR, yes it is

```bash
interchain-security-pd query provider blocks-until-next-epoch
```
15 changes: 15 additions & 0 deletions proto/interchain_security/ccv/provider/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ service Query {
option (google.api.http).get =
"/interchain_security/ccv/provider/consumer_validators/{chain_id}";
}

// QueryBlocksUntilNextEpoch returns the number of blocks until the next epoch
// starts and validator updates are sent to the consumer chains
rpc QueryBlocksUntilNextEpoch(QueryBlocksUntilNextEpochRequest)
returns (QueryBlocksUntilNextEpochResponse) {
option (google.api.http).get =
"/interchain_security/ccv/provider/blocks_until_next_epoch";
}
}

message QueryConsumerGenesisRequest { string chain_id = 1; }
Expand Down Expand Up @@ -326,3 +334,10 @@ message QueryOldestUnconfirmedVscResponse {
interchain_security.ccv.provider.v1.VscSendTimestamp vsc_send_timestamp = 1
[ (gogoproto.nullable) = false ];
}

message QueryBlocksUntilNextEpochRequest { }

message QueryBlocksUntilNextEpochResponse {
// The number of blocks until the next epoch starts
uint64 blocks_until_next_epoch = 1;
}
28 changes: 28 additions & 0 deletions x/ccv/provider/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func NewQueryCmd() *cobra.Command {
cmd.AddCommand(CmdConsumerChainsValidatorHasToValidate())
cmd.AddCommand(CmdValidatorConsumerCommissionRate())
cmd.AddCommand(CmdOldestUnconfirmedVsc())
cmd.AddCommand(CmdBlocksUntilNextEpoch())
return cmd
}

Expand Down Expand Up @@ -596,3 +597,30 @@ func CmdOldestUnconfirmedVsc() *cobra.Command {

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
}

return clientCtx.PrintProto(res)
},
}

flags.AddQueryFlagsToCmd(cmd)

return cmd
Comment on lines +601 to +625
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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
}

}
10 changes: 10 additions & 0 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,3 +488,13 @@ func (k Keeper) QueryOldestUnconfirmedVsc(goCtx context.Context, req *types.Quer

return &types.QueryOldestUnconfirmedVscResponse{VscSendTimestamp: ts}, nil
}

// 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
Comment on lines +492 to +499
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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
}

}
9 changes: 8 additions & 1 deletion x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (k Keeper) EndBlockVSU(ctx sdk.Context) {
// notify the staking module to complete all matured unbonding ops
k.completeMaturedUnbondingOps(ctx)

if ctx.BlockHeight()%k.GetBlocksPerEpoch(ctx) == 0 {
if k.BlocksUntilNextEpoch(ctx) == k.GetBlocksPerEpoch(ctx) {
// only queue and send VSCPackets at the boundaries of an epoch

// collect validator updates
Expand All @@ -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
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) BlocksUntilNextEpoch(ctx sdk.Context) int64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@p-offtermatt p-offtermatt Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

epochLength := k.GetBlocksPerEpoch(ctx)
return int64(epochLength - (ctx.BlockHeight()%epochLength))
}

// SendVSCPackets iterates over all registered consumers and sends pending
// VSC packets to the chains with established CCV channels.
// If the CCV channel is not established for a consumer chain,
Expand Down
27 changes: 27 additions & 0 deletions x/ccv/provider/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -924,3 +924,30 @@ func TestQueueVSCPacketsWithPowerCapping(t *testing.T) {

require.Equal(t, expectedQueuedVSCPackets, actualQueuedVSCPackets)
}

// TestBlocksUntilNextEpoch tests the `BlocksUntilNextEpoch` method
func TestBlocksUntilNextEpoch(t *testing.T) {
providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

// 10 blocks constitute an epoch
params := providertypes.DefaultParams()
params.BlocksPerEpoch = 10
providerKeeper.SetParams(ctx, params)

// with block height of 1 we expect 9 blocks until the next epoch
ctx = ctx.WithBlockHeight(1)
require.Equal(t, uint64(9), providerKeeper.BlocksUntilNextEpoch(ctx))

// with block height of 5 we expect 5 blocks until the next epoch
ctx = ctx.WithBlockHeight(5)
require.Equal(t, uint64(5), providerKeeper.BlocksUntilNextEpoch(ctx))

// with block height of 10 we expect 10 blocks until the next epoch, since the last epoch ends with this block
ctx = ctx.WithBlockHeight(10)
require.Equal(t, uint64(10), providerKeeper.BlocksUntilNextEpoch(ctx))

// with block height of 15 we expect 5 blocks until the next epoch
ctx = ctx.WithBlockHeight(15)
require.Equal(t, uint64(5), providerKeeper.BlocksUntilNextEpoch(ctx))
}
Loading
Loading