-
Notifications
You must be signed in to change notification settings - Fork 170
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
BM-15: feat(epoching): add protobuf messages #10
Changes from all commits
b1ff2fe
16d6a73
0ea9c57
45f32aa
9fb06d0
09cebfe
1f062dc
b168cf2
b37bb2f
a332b9a
29a8c3b
19f8f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
syntax = "proto3"; | ||
package babylon.epoching.v1; | ||
|
||
import "gogoproto/gogo.proto"; | ||
import "cosmos/staking/v1beta1/tx.proto"; | ||
|
||
option go_package = "github.com/babylonchain/babylon/x/epoching/types"; | ||
|
||
// QueuedMessage is a message that can change the validator set and is delayed to the epoch boundary | ||
message QueuedMessage { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can imagine that this will at some point require something like an "original transaction ID" so we have a way to emit an event that users can tie back to the transaction they sent. |
||
// msg_id is the original message ID | ||
bytes msg_id = 1; | ||
// msg is the actual message that is sent by a user and is queued by the epoching module | ||
oneof msg { | ||
cosmos.staking.v1beta1.MsgCreateValidator msg_create_validator = 2; | ||
cosmos.staking.v1beta1.MsgDelegate msg_delegate = 3; | ||
cosmos.staking.v1beta1.MsgUndelegate msg_undelegate = 4; | ||
cosmos.staking.v1beta1.MsgBeginRedelegate msg_begin_redelegate = 5; | ||
// TODO: after we bump to Cosmos SDK v0.46, add MsgCancelUnbondingDelegation | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,7 @@ | ||
syntax = "proto3"; | ||
package babylon.epoching.v1; | ||
|
||
// this line is used by starport scaffolding # proto/tx/import | ||
|
||
option go_package = "github.com/babylonchain/babylon/x/epoching/types"; | ||
|
||
// Msg defines the Msg service. | ||
service Msg { | ||
// this line is used by starport scaffolding # proto/tx/rpc | ||
} | ||
|
||
// this line is used by starport scaffolding # proto/tx/message | ||
service Msg {} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,34 @@ | ||
package keeper | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/babylonchain/babylon/x/epoching/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"google.golang.org/grpc/codes" | ||
"google.golang.org/grpc/status" | ||
) | ||
|
||
var _ types.QueryServer = Keeper{} | ||
// Querier is used as Keeper will have duplicate methods if used directly, and gRPC names take precedence over keeper | ||
type Querier struct { | ||
Keeper | ||
} | ||
|
||
var _ types.QueryServer = Querier{} | ||
|
||
func (k Keeper) Params(c context.Context, req *types.QueryParamsRequest) (*types.QueryParamsResponse, error) { | ||
if req == nil { | ||
return nil, status.Error(codes.InvalidArgument, "invalid request") | ||
} | ||
ctx := sdk.UnwrapSDKContext(c) | ||
|
||
return &types.QueryParamsResponse{Params: k.GetParams(ctx)}, nil | ||
} | ||
|
||
func (k Keeper) CurrentEpoch(c context.Context, req *types.QueryCurrentEpochRequest) (*types.QueryCurrentEpochResponse, error) { | ||
panic("TODO: unimplemented") | ||
} | ||
|
||
func (k Keeper) EpochMsgs(c context.Context, req *types.QueryEpochMsgsRequest) (*types.QueryEpochMsgsResponse, error) { | ||
panic("TODO: unimplemented") | ||
} |
This file was deleted.
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.
Do you intend this to hold on to these messages forever, or only until they are dequeued?
I think there is no reason to support query the messages that were enqueued for an epoch in the past, it just takes up space in the ledger. The other queues in the staking module are drained as well.
If we don't have to support historical queries, then I think we shouldn't need
epoch_num
in the query, since we will only have a single delayed queue, which we completely drain at the end of the current epoch iff we are in a position to do so. I suggested that if the checkpoint for the previous epoch is not stable yet than we don't process these request, and do another epoch with the same validator set. It would be easier to always just dequeue everything, rather than having to index/filter them by epoch number.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.
When there exists multiple epochs that haven't been checkpointed yet, the epoching module has to store messages queued in each of these epochs. Otherwise, without such indexing, if one of these uncheckpointed epochs becomes checkpointed, the epoching module cannot know which of the queued messages should be forwarded to the staking module.
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.
Right, we understand things differently then: the delayed staking messages don't need a checkpoint before they can be forwarded to the staking module, we do that unconditionally at the end of the current epoch. Therefore we don't need indexing, as we always forward all of them at once.
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.
Agree, after the meeting this is clear to me now. Then this query will be returning msgs queued in the current epoch. This query can even be merged with
CurrentEpoch
, with an extra field on whether to return queued msgs. Will change accordingly next Monday. Thanks for pointing this out!