epoching: Wrapped messages and AnteHandler implementation #18
Conversation
There was a problem hiding this comment.
Cool, I see you decided to go with the version where all the non-BLS related stuff will stay in the epoching module, and only the MsgCreateValidator will appear under checkpointing, and users will just have to pick the right command on the CLI.
So, signing for the MsgCreateValidator will have the validator and delegator signatures over the content - the question will be what the content should be. It will have to include the BLS parts, so nobody can replace a validator's BSL key with something else. That won't be exactly as it is here, where the signature excludes the bytes of the wrapper.
I wonder how the workflow of signing will differ in those two cases. Looking forward to see your next PR on that 👍
|
|
||
| return next(ctx, tx, simulate) | ||
| } | ||
|
|
||
| func (qmd DropValidatorMsgDecorator) IsValidatorRelatedMsg(msg sdk.Msg) error { |
There was a problem hiding this comment.
When I see IsValidatorRelatedMsg, or anything starting with Is, I expect it to return bool.
It would be more idiomatic, and then for AnteHandle to return the error.
| func noOpAnteDecorator() sdk.AnteHandler { | ||
| return func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { | ||
| return ctx, nil | ||
| func TestDropValidatorMsgDecorator(t *testing.T) { |
| func (msg MsgWrappedDelegate) GetSigners() []sdk.AccAddress { | ||
| return msg.Msg.GetSigners() | ||
| } | ||
|
|
||
| // GetSignBytes returns the message bytes to sign over. | ||
| func (msg MsgWrappedDelegate) GetSignBytes() []byte { | ||
| return msg.Msg.GetSignBytes() | ||
| } |
There was a problem hiding this comment.
Okay, so in this case users will have to do the exact same signing procedure, sign just the staking messages, and just send a wrapped version.
Does the outer transaction get signed at all?
There was a problem hiding this comment.
I think so. The outer transaction has to be signed in order to get accepted by Tendermint. Signing inner transactions seems unnecessary as it will be used only in the staking module and the staking module does not need to verify the inner transactions.
|
Thanks Akosh! I have fixed or replied your comments. As this PR blocks CI of other PRs and you have approved this PR, I merged it directly. |
Depends on #15 (forDropValidatorMsgDecorator) and the checkpointing module (forCreateValidatorBLSAPI)Fixes BM-25.
This PR implements the
DropValidatorMsgDecoratorAnteHandler that rejects all non-wrapped validator-related messages, and introduces wrapped variants of these validator-related messages.