From a27ef7f7d165eb3c284515984e1a488aabc11c7d Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Wed, 23 Jan 2019 03:46:41 -0500 Subject: [PATCH] Merge PR #3347: Ensure Canonical Message Signature Bytes --- PENDING.md | 2 + x/bank/msgs.go | 46 ++------------- x/bank/msgs_test.go | 2 +- x/distribution/types/msg.go | 21 ++----- x/gov/msgs.go | 21 ++----- x/slashing/msg.go | 7 +-- x/staking/types/msg.go | 115 +++++++++++++++++------------------- 7 files changed, 76 insertions(+), 138 deletions(-) diff --git a/PENDING.md b/PENDING.md index a3b72242a..602895123 100644 --- a/PENDING.md +++ b/PENDING.md @@ -144,6 +144,8 @@ BUG FIXES * \#3223 Fix unset governance proposal queues when importing state from old chain * [#3187](https://github.com/cosmos/cosmos-sdk/issues/3187) Fix `gaiad export` by resetting each validator's slashing period. + * [##3336](https://github.com/cosmos/cosmos-sdk/issues/3336) Ensure all SDK + messages have their signature bytes contain canonical fields `value` and `type`. * SDK diff --git a/x/bank/msgs.go b/x/bank/msgs.go index e73dec3b3..059b49525 100644 --- a/x/bank/msgs.go +++ b/x/bank/msgs.go @@ -1,8 +1,6 @@ package bank import ( - "encoding/json" - sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -43,24 +41,8 @@ func (msg MsgSend) ValidateBasic() sdk.Error { // Implements Msg. func (msg MsgSend) GetSignBytes() []byte { - var inputs, outputs []json.RawMessage - for _, input := range msg.Inputs { - inputs = append(inputs, input.GetSignBytes()) - } - for _, output := range msg.Outputs { - outputs = append(outputs, output.GetSignBytes()) - } - b, err := msgCdc.MarshalJSON(struct { - Inputs []json.RawMessage `json:"inputs"` - Outputs []json.RawMessage `json:"outputs"` - }{ - Inputs: inputs, - Outputs: outputs, - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -81,15 +63,6 @@ type Input struct { Coins sdk.Coins `json:"coins"` } -// Return bytes to sign for Input -func (in Input) GetSignBytes() []byte { - bin, err := msgCdc.MarshalJSON(in) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(bin) -} - // ValidateBasic - validate transaction input func (in Input) ValidateBasic() sdk.Error { if len(in.Address) == 0 { @@ -106,11 +79,10 @@ func (in Input) ValidateBasic() sdk.Error { // NewInput - create a transaction input, used with MsgSend func NewInput(addr sdk.AccAddress, coins sdk.Coins) Input { - input := Input{ + return Input{ Address: addr, Coins: coins, } - return input } //---------------------------------------- @@ -122,15 +94,6 @@ type Output struct { Coins sdk.Coins `json:"coins"` } -// Return bytes to sign for Output -func (out Output) GetSignBytes() []byte { - bin, err := msgCdc.MarshalJSON(out) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(bin) -} - // ValidateBasic - validate transaction output func (out Output) ValidateBasic() sdk.Error { if len(out.Address) == 0 { @@ -147,11 +110,10 @@ func (out Output) ValidateBasic() sdk.Error { // NewOutput - create a transaction output, used with MsgSend func NewOutput(addr sdk.AccAddress, coins sdk.Coins) Output { - output := Output{ + return Output{ Address: addr, Coins: coins, } - return output } // ---------------------------------------------------------------------------- diff --git a/x/bank/msgs_test.go b/x/bank/msgs_test.go index 040d12bda..3f8b68fd4 100644 --- a/x/bank/msgs_test.go +++ b/x/bank/msgs_test.go @@ -179,7 +179,7 @@ func TestMsgSendGetSignBytes(t *testing.T) { } res := msg.GetSignBytes() - expected := `{"inputs":[{"address":"cosmos1d9h8qat57ljhcm","coins":[{"amount":"10","denom":"atom"}]}],"outputs":[{"address":"cosmos1da6hgur4wsmpnjyg","coins":[{"amount":"10","denom":"atom"}]}]}` + expected := `{"type":"cosmos-sdk/Send","value":{"inputs":[{"address":"cosmos1d9h8qat57ljhcm","coins":[{"amount":"10","denom":"atom"}]}],"outputs":[{"address":"cosmos1da6hgur4wsmpnjyg","coins":[{"amount":"10","denom":"atom"}]}]}}` require.Equal(t, expected, string(res)) } diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index 8100e5156..9c977ea91 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -34,11 +34,8 @@ func (msg MsgSetWithdrawAddress) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgSetWithdrawAddress) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -75,11 +72,8 @@ func (msg MsgWithdrawDelegatorReward) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgWithdrawDelegatorReward) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -114,11 +108,8 @@ func (msg MsgWithdrawValidatorCommission) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgWithdrawValidatorCommission) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check diff --git a/x/gov/msgs.go b/x/gov/msgs.go index bec99f078..3be7ab583 100644 --- a/x/gov/msgs.go +++ b/x/gov/msgs.go @@ -73,11 +73,8 @@ func (msg MsgSubmitProposal) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgSubmitProposal) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -134,11 +131,8 @@ func (msg MsgDeposit) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgDeposit) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -192,11 +186,8 @@ func (msg MsgVote) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgVote) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. diff --git a/x/slashing/msg.go b/x/slashing/msg.go index 6721f8e9b..551258627 100644 --- a/x/slashing/msg.go +++ b/x/slashing/msg.go @@ -30,11 +30,8 @@ func (msg MsgUnjail) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgUnjail) GetSignBytes() []byte { - b, err := cdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := cdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 7fe7418dd..9ef062104 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -2,14 +2,21 @@ package types import ( "bytes" + "encoding/json" "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" ) -// Verify interface at compile time -var _, _, _ sdk.Msg = &MsgCreateValidator{}, &MsgEditValidator{}, &MsgDelegate{} +// ensure Msg interface compliance at compile time +var ( + _ sdk.Msg = &MsgCreateValidator{} + _ sdk.Msg = &MsgEditValidator{} + _ sdk.Msg = &MsgDelegate{} + _ sdk.Msg = &MsgUndelegate{} + _ sdk.Msg = &MsgBeginRedelegate{} +) //______________________________________________________________________ @@ -23,6 +30,15 @@ type MsgCreateValidator struct { Value sdk.Coin `json:"value"` } +type msgCreateValidatorJSON struct { + Description Description `json:"description"` + Commission CommissionMsg `json:"commission"` + DelegatorAddr sdk.AccAddress `json:"delegator_address"` + ValidatorAddr sdk.ValAddress `json:"validator_address"` + PubKey string `json:"pubkey"` + Value sdk.Coin `json:"value"` +} + // Default way to create validator. Delegator address and validator address are the same func NewMsgCreateValidator(valAddr sdk.ValAddress, pubkey crypto.PubKey, selfDelegation sdk.Coin, description Description, commission CommissionMsg) MsgCreateValidator { @@ -62,26 +78,41 @@ func (msg MsgCreateValidator) GetSigners() []sdk.AccAddress { return addrs } -// TODO Remove use of custom struct (no longer necessary) -// get the bytes for the message signer to sign on -func (msg MsgCreateValidator) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - Description Description `json:"description"` - Commission CommissionMsg `json:"commission"` - DelegatorAddr sdk.AccAddress `json:"delegator_address"` - ValidatorAddr sdk.ValAddress `json:"validator_address"` - PubKey string `json:"pubkey"` - Value sdk.Coin `json:"value"` - }{ +// MarshalJSON implements the json.Marshaler interface to provide custom JSON +// serialization of the MsgCreateValidator type. +func (msg MsgCreateValidator) MarshalJSON() ([]byte, error) { + return json.Marshal(msgCreateValidatorJSON{ Description: msg.Description, + Commission: msg.Commission, + DelegatorAddr: msg.DelegatorAddr, ValidatorAddr: msg.ValidatorAddr, PubKey: sdk.MustBech32ifyConsPub(msg.PubKey), Value: msg.Value, }) - if err != nil { - panic(err) +} + +// UnmarshalJSON implements the json.Unmarshaler interface to provide custom +// JSON deserialization of the MsgCreateValidator type. +func (msg *MsgCreateValidator) UnmarshalJSON(bz []byte) error { + var msgCreateValJSON msgCreateValidatorJSON + if err := json.Unmarshal(bz, &msgCreateValJSON); err != nil { + return err } - return sdk.MustSortJSON(b) + + msg.Description = msgCreateValJSON.Description + msg.Commission = msgCreateValJSON.Commission + msg.DelegatorAddr = msgCreateValJSON.DelegatorAddr + msg.ValidatorAddr = msgCreateValJSON.ValidatorAddr + msg.PubKey = sdk.MustGetConsPubKeyBech32(msgCreateValJSON.PubKey) + msg.Value = msgCreateValJSON.Value + + return nil +} + +// GetSignBytes returns the message bytes to sign over. +func (msg MsgCreateValidator) GetSignBytes() []byte { + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -137,17 +168,8 @@ func (msg MsgEditValidator) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgEditValidator) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - Description - ValidatorAddr sdk.ValAddress `json:"address"` - }{ - Description: msg.Description, - ValidatorAddr: msg.ValidatorAddr, - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -189,11 +211,8 @@ func (msg MsgDelegate) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgDelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -240,21 +259,8 @@ func (msg MsgBeginRedelegate) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgBeginRedelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - DelegatorAddr sdk.AccAddress `json:"delegator_addr"` - ValidatorSrcAddr sdk.ValAddress `json:"validator_src_addr"` - ValidatorDstAddr sdk.ValAddress `json:"validator_dst_addr"` - SharesAmount string `json:"shares"` - }{ - DelegatorAddr: msg.DelegatorAddr, - ValidatorSrcAddr: msg.ValidatorSrcAddr, - ValidatorDstAddr: msg.ValidatorDstAddr, - SharesAmount: msg.SharesAmount.String(), - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -298,19 +304,8 @@ func (msg MsgUndelegate) GetSigners() []sdk.AccAddress { return []sdk.AccAddress // get the bytes for the message signer to sign on func (msg MsgUndelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - DelegatorAddr sdk.AccAddress `json:"delegator_addr"` - ValidatorAddr sdk.ValAddress `json:"validator_addr"` - SharesAmount string `json:"shares_amount"` - }{ - DelegatorAddr: msg.DelegatorAddr, - ValidatorAddr: msg.ValidatorAddr, - SharesAmount: msg.SharesAmount.String(), - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check