feat(sequencer): set/update reward addr #510
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I realised I overcomplicated a lot before @mtsitrin , it's much simpler now, and still secure |
|
|
||
| msgs := make([]sdk.Msg, 1) | ||
|
|
||
| msg, err := types.BuildMsgCreateSequencer(func(msg []byte) ([]byte, cryptotypes.PubKey, error) { |
There was a problem hiding this comment.
msg []byte is actaully the payload to sign right? not to be confused with the msg created
|
|
||
| consAddr, err := v.GetConsAddr() | ||
| if err != nil { | ||
| panic(err) // it must be ok because we used it to check sig |
There was a problem hiding this comment.
why not return err?
I think it checked in validateBasic anyway
| func NewUpdateCmd() *cobra.Command { | ||
| short := "Update a sequencer object, to claim rewards etc." | ||
| long := strings.TrimSpace(short + | ||
| `Requires signature from consensus address public key. Specify consensus key in keyring uid. |
There was a problem hiding this comment.
I think it's wrong long. we don't require sig over consesus pubkey here.
| ) | ||
|
|
||
| // NewHandler returns a new msg handler. | ||
| func NewHandler(k keeper.Keeper) sdk.Handler { |
There was a problem hiding this comment.
I believe this file is redundant. in sdk 0.46 you simply need to simply register your msg server infront of your keeper and implement the corresponding methods in the keeper.
| // The reasoning is as follows: | ||
| // | ||
| // We know from the SDK TX signing mechanism that the account originates from the operator, and on this chain ID. | ||
| // Therefore, we just need to check that the consensus private key also signed over this account and chain ID. |
There was a problem hiding this comment.
I blv this godoc is wrong as we're only checking if signed on validator address.
|
to not block testing, gonna merge it and open technical debt with @mtsitrin and myself comments. |
omritoptix
deleted the
danwt/509-do-not-require-rewards-addr-on-genesis
branch
|
followup #526 |
PR Standards
Closes #509 #508
I will follow up with some improvements in other PRs
Opening a pull request should be able to meet the following requirements
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
For Author:
godoccommentsFor Reviewer:
After reviewer approval: