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

Sort all msg.GetSignBytes #1563

Merged
merged 6 commits into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*TBD*

BREAKING CHANGES
* msg.GetSignBytes() returns sorted JSON (by key)
* Update Tendermint to v0.22.0
* Default ports changed from 466xx to 266xx
* Amino JSON uses type names instead of prefix bytes
Expand Down
4 changes: 2 additions & 2 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ const msgType3 = "burn"
func (msg testBurnMsg) Type() string { return msgType3 }
func (msg testBurnMsg) GetSignBytes() []byte {
bz, _ := json.Marshal(msg)
return bz
return sdk.MustSortJSON(bz)
}
func (msg testBurnMsg) ValidateBasic() sdk.Error {
if msg.Addr == nil {
Expand All @@ -685,7 +685,7 @@ const msgType4 = "send"
func (msg testSendMsg) Type() string { return msgType4 }
func (msg testSendMsg) GetSignBytes() []byte {
bz, _ := json.Marshal(msg)
return bz
return sdk.MustSortJSON(bz)
}
func (msg testSendMsg) ValidateBasic() sdk.Error {
if msg.Sender == nil || msg.Receiver == nil {
Expand Down
2 changes: 1 addition & 1 deletion docs/core/examples/app1.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (msg MsgSend) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bz
return sdk.MustSortJSON(bz)
}

// Implements Msg. Return the signer.
Expand Down
2 changes: 1 addition & 1 deletion docs/core/examples/app2.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (msg MsgIssue) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bz
return sdk.MustSortJSON(bz)
}

// Implements Msg. Return the signer.
Expand Down
4 changes: 2 additions & 2 deletions examples/democoin/x/cool/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (msg MsgSetTrend) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

//_______________________________________________________________________
Expand Down Expand Up @@ -102,5 +102,5 @@ func (msg MsgQuiz) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}
2 changes: 1 addition & 1 deletion examples/democoin/x/oracle/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (msg Msg) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bz
return sdk.MustSortJSON(bz)
}

// GetSigners implements sdk.Msg
Expand Down
2 changes: 1 addition & 1 deletion examples/democoin/x/pow/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ func (msg MsgMine) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}
2 changes: 1 addition & 1 deletion examples/democoin/x/pow/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestMsgMineGetSignBytes(t *testing.T) {
addr := sdk.Address([]byte("sender"))
msg := MsgMine{addr, 1, 1, 1, []byte("abc")}
res := msg.GetSignBytes()
require.Equal(t, string(res), `{"sender":"73656E646572","difficulty":1,"count":1,"nonce":1,"proof":"YWJj"}`)
require.Equal(t, string(res), `{"count":1,"difficulty":1,"nonce":1,"proof":"YWJj","sender":"73656E646572"}`)
}

func TestMsgMineGetSigners(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion examples/democoin/x/simplestake/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (msg MsgBond) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bz
return sdk.MustSortJSON(bz)
}

//_______________________________________________________________
Expand Down
2 changes: 1 addition & 1 deletion examples/kvstore/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (tx kvstoreTx) GetMemo() string {
}

func (tx kvstoreTx) GetSignBytes() []byte {
return tx.bytes
return sdk.MustSortJSON(tx.bytes)
}

// Should the app be calling this? Or only handlers?
Expand Down
18 changes: 0 additions & 18 deletions server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,24 +141,6 @@ func InsertKeyJSON(cdc *wire.Codec, baseJSON []byte, key string, value json.RawM
return json.RawMessage(bz), err
}

// SortedJSON takes any JSON and returns it sorted by keys. Also, all white-spaces
// are removed.
// This method can be used to canonicalize JSON to be returned by GetSignBytes,
// e.g. for the ledger integration.
// If the passed JSON isn't valid it will return an error.
func SortJSON(toSortJSON []byte) ([]byte, error) {
var c interface{}
err := json.Unmarshal(toSortJSON, &c)
if err != nil {
return nil, err
}
js, err := json.Marshal(c)
if err != nil {
return nil, err
}
return js, nil
}

// https://stackoverflow.com/questions/23558425/how-do-i-get-the-local-ip-address-in-go
// TODO there must be a better way to get external IP
func externalIP() (string, error) {
Expand Down
31 changes: 0 additions & 31 deletions server/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,3 @@ func TestInsertKeyJSON(t *testing.T) {

require.Equal(t, bar, resBar, "appended: %v", appended)
}

func TestSortJSON(t *testing.T) {
cases := []struct {
unsortedJSON string
want string
wantErr bool
}{
// simple case
{unsortedJSON: `{"cosmos":"foo", "atom":"bar", "tendermint":"foobar"}`,
want: `{"atom":"bar","cosmos":"foo","tendermint":"foobar"}`, wantErr: false},
// failing case (invalid JSON):
{unsortedJSON: `"cosmos":"foo",,,, "atom":"bar", "tendermint":"foobar"}`,
want: "", wantErr: true},
// genesis.json
{unsortedJSON: `{"consensus_params":{"block_size_params":{"max_bytes":22020096,"max_txs":100000,"max_gas":-1},"tx_size_params":{"max_bytes":10240,"max_gas":-1},"block_gossip_params":{"block_part_size_bytes":65536},"evidence_params":{"max_age":100000}},"validators":[{"pub_key":{"type":"AC26791624DE60","value":"c7UMMAbjFuc5GhGPy0E5q5tefy12p9Tq0imXqdrKXwo="},"power":100,"name":""}],"app_hash":"","genesis_time":"2018-05-11T15:52:25.424795506Z","chain_id":"test-chain-Q6VeoW","app_state":{"accounts":[{"address":"718C9C23F98C9642569742ADDD9F9AB9743FBD5D","coins":[{"denom":"Token","amount":1000},{"denom":"steak","amount":50}]}],"stake":{"pool":{"total_supply":50,"bonded_shares":"0","unbonded_shares":"0","bonded_pool":0,"unbonded_pool":0,"inflation_last_time":0,"inflation":"7/100"},"params":{"inflation_rate_change":"13/100","inflation_max":"1/5","inflation_min":"7/100","goal_bonded":"67/100","max_validators":100,"bond_denom":"steak"},"candidates":null,"bonds":null}}}`,
want: `{"app_hash":"","app_state":{"accounts":[{"address":"718C9C23F98C9642569742ADDD9F9AB9743FBD5D","coins":[{"amount":1000,"denom":"Token"},{"amount":50,"denom":"steak"}]}],"stake":{"bonds":null,"candidates":null,"params":{"bond_denom":"steak","goal_bonded":"67/100","inflation_max":"1/5","inflation_min":"7/100","inflation_rate_change":"13/100","max_validators":100},"pool":{"bonded_pool":0,"bonded_shares":"0","inflation":"7/100","inflation_last_time":0,"total_supply":50,"unbonded_pool":0,"unbonded_shares":"0"}}},"chain_id":"test-chain-Q6VeoW","consensus_params":{"block_gossip_params":{"block_part_size_bytes":65536},"block_size_params":{"max_bytes":22020096,"max_gas":-1,"max_txs":100000},"evidence_params":{"max_age":100000},"tx_size_params":{"max_bytes":10240,"max_gas":-1}},"genesis_time":"2018-05-11T15:52:25.424795506Z","validators":[{"name":"","power":100,"pub_key":{"type":"AC26791624DE60","value":"c7UMMAbjFuc5GhGPy0E5q5tefy12p9Tq0imXqdrKXwo="}}]}`,
wantErr: false},
// from the TXSpec:
{unsortedJSON: `{"chain_id":"test-chain-1","sequence":1,"fee_bytes":{"amount":[{"amount":5,"denom":"photon"}],"gas":10000},"msg_bytes":{"inputs":[{"address":"696E707574","coins":[{"amount":10,"denom":"atom"}]}],"outputs":[{"address":"6F7574707574","coins":[{"amount":10,"denom":"atom"}]}]},"alt_bytes":null}`,
want: `{"alt_bytes":null,"chain_id":"test-chain-1","fee_bytes":{"amount":[{"amount":5,"denom":"photon"}],"gas":10000},"msg_bytes":{"inputs":[{"address":"696E707574","coins":[{"amount":10,"denom":"atom"}]}],"outputs":[{"address":"6F7574707574","coins":[{"amount":10,"denom":"atom"}]}]},"sequence":1}`,
wantErr: false},
}

for _, tc := range cases {
got, err := SortJSON([]byte(tc.unsortedJSON))
if tc.wantErr != (err != nil) {
t.Fatalf("got %t, want: %t, err=%s", err != nil, tc.wantErr, err)
}
require.Equal(t, string(got), tc.want)
}
}
2 changes: 1 addition & 1 deletion types/tx_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (msg *TestMsg) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bz
return MustSortJSON(bz)
}
func (msg *TestMsg) ValidateBasic() Error { return nil }
func (msg *TestMsg) GetSigners() []Address {
Expand Down
31 changes: 31 additions & 0 deletions types/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package types

import "encoding/json"

// SortedJSON takes any JSON and returns it sorted by keys. Also, all white-spaces
// are removed.
// This method can be used to canonicalize JSON to be returned by GetSignBytes,
// e.g. for the ledger integration.
// If the passed JSON isn't valid it will return an error.
func SortJSON(toSortJSON []byte) ([]byte, error) {
var c interface{}
err := json.Unmarshal(toSortJSON, &c)
if err != nil {
return nil, err
}
js, err := json.Marshal(c)
if err != nil {
return nil, err
}
return js, nil
}

// MustSortJSON is like SortJSON but panic if an error occurs, e.g., if
// the passed JSON isn't valid.
func MustSortJSON(toSortJSON []byte) []byte {
js, err := SortJSON(toSortJSON)
if err != nil {
panic(err)
}
return js
}
39 changes: 39 additions & 0 deletions types/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package types

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestSortJSON(t *testing.T) {
cases := []struct {
unsortedJSON string
want string
wantErr bool
}{
// simple case
{unsortedJSON: `{"cosmos":"foo", "atom":"bar", "tendermint":"foobar"}`,
want: `{"atom":"bar","cosmos":"foo","tendermint":"foobar"}`, wantErr: false},
// failing case (invalid JSON):
{unsortedJSON: `"cosmos":"foo",,,, "atom":"bar", "tendermint":"foobar"}`,
want: "",
wantErr: true},
// genesis.json
{unsortedJSON: `{"consensus_params":{"block_size_params":{"max_bytes":22020096,"max_txs":100000,"max_gas":-1},"tx_size_params":{"max_bytes":10240,"max_gas":-1},"block_gossip_params":{"block_part_size_bytes":65536},"evidence_params":{"max_age":100000}},"validators":[{"pub_key":{"type":"AC26791624DE60","value":"c7UMMAbjFuc5GhGPy0E5q5tefy12p9Tq0imXqdrKXwo="},"power":100,"name":""}],"app_hash":"","genesis_time":"2018-05-11T15:52:25.424795506Z","chain_id":"test-chain-Q6VeoW","app_state":{"accounts":[{"address":"718C9C23F98C9642569742ADDD9F9AB9743FBD5D","coins":[{"denom":"Token","amount":1000},{"denom":"steak","amount":50}]}],"stake":{"pool":{"total_supply":50,"bonded_shares":"0","unbonded_shares":"0","bonded_pool":0,"unbonded_pool":0,"inflation_last_time":0,"inflation":"7/100"},"params":{"inflation_rate_change":"13/100","inflation_max":"1/5","inflation_min":"7/100","goal_bonded":"67/100","max_validators":100,"bond_denom":"steak"},"candidates":null,"bonds":null}}}`,
want: `{"app_hash":"","app_state":{"accounts":[{"address":"718C9C23F98C9642569742ADDD9F9AB9743FBD5D","coins":[{"amount":1000,"denom":"Token"},{"amount":50,"denom":"steak"}]}],"stake":{"bonds":null,"candidates":null,"params":{"bond_denom":"steak","goal_bonded":"67/100","inflation_max":"1/5","inflation_min":"7/100","inflation_rate_change":"13/100","max_validators":100},"pool":{"bonded_pool":0,"bonded_shares":"0","inflation":"7/100","inflation_last_time":0,"total_supply":50,"unbonded_pool":0,"unbonded_shares":"0"}}},"chain_id":"test-chain-Q6VeoW","consensus_params":{"block_gossip_params":{"block_part_size_bytes":65536},"block_size_params":{"max_bytes":22020096,"max_gas":-1,"max_txs":100000},"evidence_params":{"max_age":100000},"tx_size_params":{"max_bytes":10240,"max_gas":-1}},"genesis_time":"2018-05-11T15:52:25.424795506Z","validators":[{"name":"","power":100,"pub_key":{"type":"AC26791624DE60","value":"c7UMMAbjFuc5GhGPy0E5q5tefy12p9Tq0imXqdrKXwo="}}]}`,
wantErr: false},
// from the TXSpec:
{unsortedJSON: `{"chain_id":"test-chain-1","sequence":1,"fee_bytes":{"amount":[{"amount":5,"denom":"photon"}],"gas":10000},"msg_bytes":{"inputs":[{"address":"696E707574","coins":[{"amount":10,"denom":"atom"}]}],"outputs":[{"address":"6F7574707574","coins":[{"amount":10,"denom":"atom"}]}]},"alt_bytes":null}`,
want: `{"alt_bytes":null,"chain_id":"test-chain-1","fee_bytes":{"amount":[{"amount":5,"denom":"photon"}],"gas":10000},"msg_bytes":{"inputs":[{"address":"696E707574","coins":[{"amount":10,"denom":"atom"}]}],"outputs":[{"address":"6F7574707574","coins":[{"amount":10,"denom":"atom"}]}]},"sequence":1}`,
wantErr: false},
}

for _, tc := range cases {
got, err := SortJSON([]byte(tc.unsortedJSON))
if tc.wantErr != (err != nil) {
t.Fatalf("got %t, want: %t, err=%s", err != nil, tc.wantErr, err)
}
require.Equal(t, string(got), tc.want)
}
}
2 changes: 1 addition & 1 deletion x/auth/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func StdSignBytes(chainID string, accnum int64, sequence int64, fee StdFee, msgs
if err != nil {
panic(err)
}
return bz
return sdk.MustSortJSON(bz)
}

// StdSignMsg is a convenience structure for passing along
Expand Down
8 changes: 4 additions & 4 deletions x/bank/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (msg MsgSend) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// Implements Msg.
Expand Down Expand Up @@ -133,7 +133,7 @@ func (msg MsgIssue) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// Implements Msg.
Expand Down Expand Up @@ -162,7 +162,7 @@ func (in Input) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bin
return sdk.MustSortJSON(bin)
}

// ValidateBasic - validate transaction input
Expand Down Expand Up @@ -209,7 +209,7 @@ func (out Output) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return bin
return sdk.MustSortJSON(bin)
}

// ValidateBasic - validate transaction output
Expand Down
4 changes: 2 additions & 2 deletions x/bank/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func TestMsgSendGetSignBytes(t *testing.T) {
}
res := msg.GetSignBytes()

expected := `{"inputs":[{"address":"cosmosaccaddr1d9h8qat5e4ehc5","coins":[{"denom":"atom","amount":"10"}]}],"outputs":[{"address":"cosmosaccaddr1da6hgur4wse3jx32","coins":[{"denom":"atom","amount":"10"}]}]}`
expected := `{"inputs":[{"address":"cosmosaccaddr1d9h8qat5e4ehc5","coins":[{"amount":"10","denom":"atom"}]}],"outputs":[{"address":"cosmosaccaddr1da6hgur4wse3jx32","coins":[{"amount":"10","denom":"atom"}]}]}`
require.Equal(t, expected, string(res))
}

Expand Down Expand Up @@ -257,7 +257,7 @@ func TestMsgIssueGetSignBytes(t *testing.T) {
}
res := msg.GetSignBytes()

expected := `{"banker":"cosmosaccaddr1d9h8qat5e4ehc5","outputs":[{"address":"cosmosaccaddr1d3hkzm3dveex7mfdvfsku6cwsauqd","coins":[{"denom":"atom","amount":"10"}]}]}`
expected := `{"banker":"cosmosaccaddr1d9h8qat5e4ehc5","outputs":[{"address":"cosmosaccaddr1d3hkzm3dveex7mfdvfsku6cwsauqd","coins":[{"amount":"10","denom":"atom"}]}]}`
require.Equal(t, expected, string(res))
}

Expand Down
6 changes: 3 additions & 3 deletions x/gov/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (msg MsgSubmitProposal) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// Implements Msg.
Expand Down Expand Up @@ -149,7 +149,7 @@ func (msg MsgDeposit) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// Implements Msg.
Expand Down Expand Up @@ -213,7 +213,7 @@ func (msg MsgVote) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// Implements Msg.
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p IBCPacket) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// validator the ibc packey
Expand Down Expand Up @@ -131,5 +131,5 @@ func (msg IBCReceiveMsg) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}
2 changes: 1 addition & 1 deletion x/slashing/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (msg MsgUnrevoke) GetSignBytes() []byte {
if err != nil {
panic(err)
}
return b
return sdk.MustSortJSON(b)
}

// quick validity check
Expand Down
Loading