Merge PR #1563: Sort all msg.GetSignBytes

CHANGELOG.md

@@ -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

baseapp/baseapp_test.go

@@ -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 {
@@ -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 {

docs/core/examples/app1.go

@@ -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.

docs/core/examples/app2.go

@@ -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.

examples/democoin/x/cool/types.go

@@ -58,7 +58,7 @@ func (msg MsgSetTrend) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 //_______________________________________________________________________
@@ -102,5 +102,5 @@ func (msg MsgQuiz) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }

examples/democoin/x/oracle/types.go

@@ -18,7 +18,7 @@ func (msg Msg) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return bz
+	return sdk.MustSortJSON(bz)
 }
 
 // GetSigners implements sdk.Msg

examples/democoin/x/pow/types.go

@@ -76,5 +76,5 @@ func (msg MsgMine) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }

examples/democoin/x/pow/types_test.go

@@ -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) {

examples/democoin/x/simplestake/msgs.go

@@ -48,7 +48,7 @@ func (msg MsgBond) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return bz
+	return sdk.MustSortJSON(bz)
 }
 
 //_______________________________________________________________

examples/kvstore/tx.go

@@ -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?

server/util.go

@@ -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) {

server/util_test.go

@@ -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)
-	}
-}

types/tx_msg.go

@@ -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 {

types/utils.go

@@ -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
+}

types/utils_test.go

@@ -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)
+	}
+}

x/auth/stdtx.go

@@ -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

x/bank/msgs.go

@@ -72,7 +72,7 @@ func (msg MsgSend) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // Implements Msg.
@@ -133,7 +133,7 @@ func (msg MsgIssue) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // Implements Msg.
@@ -162,7 +162,7 @@ func (in Input) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return bin
+	return sdk.MustSortJSON(bin)
 }
 
 // ValidateBasic - validate transaction input
@@ -209,7 +209,7 @@ func (out Output) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return bin
+	return sdk.MustSortJSON(bin)
 }
 
 // ValidateBasic - validate transaction output

x/bank/msgs_test.go

@@ -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))
 }
 
@@ -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))
 }
 

x/gov/msgs.go

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

x/ibc/types.go

@@ -59,7 +59,7 @@ func (p IBCPacket) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // validator the ibc packey
@@ -131,5 +131,5 @@ func (msg IBCReceiveMsg) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }

x/slashing/msg.go

@@ -38,7 +38,7 @@ func (msg MsgUnrevoke) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check

x/stake/types/msg.go

@@ -63,7 +63,7 @@ func (msg MsgCreateValidator) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -114,7 +114,7 @@ func (msg MsgEditValidator) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -166,7 +166,7 @@ func (msg MsgDelegate) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -226,7 +226,7 @@ func (msg MsgBeginRedelegate) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -286,7 +286,7 @@ func (msg MsgCompleteRedelegate) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -338,7 +338,7 @@ func (msg MsgBeginUnbonding) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check
@@ -387,7 +387,7 @@ func (msg MsgCompleteUnbonding) GetSignBytes() []byte {
 	if err != nil {
 		panic(err)
 	}
-	return b
+	return sdk.MustSortJSON(b)
 }
 
 // quick validity check