-
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
Minor refactor: factor out Begin(End)Block calls from simibc #588
Conversation
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.
LGTM, for my understanding could you give an example where executing logic/assertions after EndBlock
but before committing the block is useful?
Yes, if you write a test and what to check the state that was actually going into the blockchain, this is exactly the committed state. But to write the test you probably want to use This is used in the difftests |
tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
) | ||
|
||
func BeginBlock(c *ibctesting.TestChain, dt time.Duration) { |
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.
Add a comment explaining what the function does, especially the dt
param.
|
||
c.LastHeader = c.CurrentTMClientHeader() | ||
|
||
// Store header to be used in UpdateClient |
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.
Why is this comment removed?
} | ||
|
||
// Commit packets emmitted up to this point |
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.
ditto
Description
This is a pure refactors that extracts BeginBlock and EndBlock methods from simibc to make them useable in other contexts
Type of change
Please delete options that are not relevant.
How was the feature tested?
Checklist:
Please delete options that are not relevant.
make test
)