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

epoching: API: epoch_msgs/{epoch_num} -> all events during this epoch #108

Merged
merged 14 commits into from
Sep 4, 2022

Conversation

SebastianElvis
Copy link
Member

Fixes BM-119

This PR provides the API that given an epoch number, outputs the information of queued msgs during this epoch, including the msg type, submitted block height, validator address and amount for each queued msg.

Specifically,

  • msg type can be found by using Protobuf's oneof feature
  • submitted block height is block_height in QueuedMessage
  • validator address and amount can be found in msg of QueuedMessage

@SebastianElvis SebastianElvis changed the title epoching: API: epoch_msgs/{epoch_num} -> [(event_type, block_height, val_addr, amount), ...] epoching: API: epoch_msgs/{epoch_num} -> all events during this epoch Aug 31, 2022
Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

Looks good! Just minor comments and clarifications.

// init epoch msg queue
k.InitMsgQueue(ctx)
// if epoch 1, then copy all queued msgs in epoch 0 to epoch 1
// TODO: more elegant way to do this? e.g., reject all msgs submitted in epoch 0?
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand epoch 0 is the genesis block. There the genutil module executes the MsgCreateValidator methods directly, we have no delays. Why is this copying necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

There the genutil module executes the MsgCreateValidator methods directly, we have no delays.

Yeah MsgCreateValidator does not go through the msg queue so is not delayed.

Why is this copying necessary?

Cosmos SDK does not execute BeginBlock and EndBlock for the genesis block. If someone submits a wrapped msg (e.g., MsgWrappedDelegate) at genesis block (i.e., in epoch 0), then it will be put in epoch 0's msg queue. However, since epoch 0 does not have EndBlock invocation, the queued msg will never be handled. This corner case was tested by fuzzing tests in epoch_msg_queue_test.go.

This PR works around this issue by copying all queued msgs in epoch 0 to epoch 1 upon BeginBlock of block 1. Another fix I can think of is to reject all wrapped msgs submitted during epoch 0. Do you have any better alternatives?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. I just don't understand how anyone can send anything in the genesis block. The genesis block is created purely based on the genesis.json file, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's weird that the system handles messages during block 0. So perhaps let's reject validator-related msgs during block 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it was just an artifact of the simulation? Rejection sounds good to me.

// InitQueueLength initialises the msg queue length to 0
func (k Keeper) InitQueueLength(ctx sdk.Context) {
store := ctx.KVStore(k.storeKey)
// InitQueueLength initialises the msg queue length of the current epoch to 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// InitQueueLength initialises the msg queue length of the current epoch to 0
// InitMsgQueue initialises the msg queue length of the current epoch to 0

x/epoching/keeper/epoch_msg_queue_test.go Show resolved Hide resolved
x/epoching/keeper/epoch_msg_queue.go Outdated Show resolved Hide resolved
@SebastianElvis
Copy link
Member Author

Thanks for the comments Akosh! Now this PR rejects any of wrapped msg types during genesis block (i.e., when block height is 0). It also makes a number of minor fixes that do not affect the functionality, and rebases to the latest main branch. Feel free to have a look again.

Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

Solid as always 👏

@SebastianElvis SebastianElvis merged commit 0501cb1 into main Sep 4, 2022
@SebastianElvis SebastianElvis deleted the epoching-epoch-events-api branch September 4, 2022 23:30
vitsalis pushed a commit that referenced this pull request Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants