From e6dac1da3d8669166c60887148adcd500a5b0b97 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Tue, 26 Jan 2021 15:03:22 +0000 Subject: [PATCH] ibc: MsgTransfer amino JSON, commits from @fedekunze (#8440) From: #8437 Closes: #8266 Thanks: @fedekunze --- CHANGELOG.md | 4 ++++ x/auth/client/rest/rest_test.go | 3 +-- x/ibc/applications/transfer/module.go | 4 +++- x/ibc/applications/transfer/types/codec.go | 18 +++++++++++++++++- x/ibc/applications/transfer/types/msgs.go | 5 ++--- x/ibc/applications/transfer/types/msgs_test.go | 10 ++++++++++ 6 files changed, 37 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c598bbb43bc5..c20d838b323d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### State Machine Breaking + +* (x/ibc) [\#8266](https://github.com/cosmos/cosmos-sdk/issues/8266) Add amino JSON for IBC messages in order to support Ledger text signing. + ### Improvements * (x/ibc) [\#8404](https://github.com/cosmos/cosmos-sdk/pull/8404) Reorder IBC `ChanOpenAck` and `ChanOpenConfirm` handler execution to perform core handler first, followed by application callbacks. diff --git a/x/auth/client/rest/rest_test.go b/x/auth/client/rest/rest_test.go index e20874f6e716..0ba55cea67df 100644 --- a/x/auth/client/rest/rest_test.go +++ b/x/auth/client/rest/rest_test.go @@ -186,8 +186,7 @@ func (s *IntegrationTestSuite) TestBroadcastIBCTxRequest() { res, err := rest.PostRequest(fmt.Sprintf("%s/txs", val.APIAddress), "application/json", []byte(req)) s.Require().NoError(err) - // Make sure the error message is correct. - s.Require().Contains(string(res), "this transaction cannot be broadcasted via legacy REST endpoints") + s.Require().NotContains(string(res), "this transaction cannot be broadcasted via legacy REST endpoints", string(res)) } // Helper function to test querying txs. We will use it to query StdTx and service `Msg`s. diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go index bf324e2c9b36..039597afe5e4 100644 --- a/x/ibc/applications/transfer/module.go +++ b/x/ibc/applications/transfer/module.go @@ -46,7 +46,9 @@ func (AppModuleBasic) Name() string { } // RegisterLegacyAminoCodec implements AppModuleBasic interface -func (AppModuleBasic) RegisterLegacyAminoCodec(*codec.LegacyAmino) {} +func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + types.RegisterLegacyAminoCodec(cdc) +} // RegisterInterfaces registers module concrete types into protobuf Any. func (AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry) { diff --git a/x/ibc/applications/transfer/types/codec.go b/x/ibc/applications/transfer/types/codec.go index 18b594f8a77a..24ad7e5a9023 100644 --- a/x/ibc/applications/transfer/types/codec.go +++ b/x/ibc/applications/transfer/types/codec.go @@ -7,6 +7,12 @@ import ( "github.com/cosmos/cosmos-sdk/types/msgservice" ) +// RegisterLegacyAminoCodec registers the necessary x/ibc transfer interfaces and concrete types +// on the provided LegacyAmino codec. These types are used for Amino JSON serialization. +func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + cdc.RegisterConcrete(&MsgTransfer{}, "cosmos-sdk/MsgTransfer", nil) +} + // RegisterInterfaces register the ibc transfer module interfaces to protobuf // Any. func RegisterInterfaces(registry codectypes.InterfaceRegistry) { @@ -16,10 +22,20 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { } var ( + amino = codec.NewLegacyAmino() + // ModuleCdc references the global x/ibc-transfer module codec. Note, the codec // should ONLY be used in certain instances of tests and for JSON encoding. // - // The actual codec used for serialization should be provided to x/ibc-transfer and + // The actual codec used for serialization should be provided to x/ibc transfer and // defined at the application level. ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) + + // AminoCdc is a amino codec created to support amino json compatible msgs. + AminoCdc = codec.NewAminoCodec(amino) ) + +func init() { + RegisterLegacyAminoCodec(amino) + amino.Seal() +} diff --git a/x/ibc/applications/transfer/types/msgs.go b/x/ibc/applications/transfer/types/msgs.go index 065482671375..cf2293213a82 100644 --- a/x/ibc/applications/transfer/types/msgs.go +++ b/x/ibc/applications/transfer/types/msgs.go @@ -70,10 +70,9 @@ func (msg MsgTransfer) ValidateBasic() error { return ValidateIBCDenom(msg.Token.Denom) } -// GetSignBytes implements sdk.Msg. The function will panic since it is used -// for amino transaction verification which IBC does not support. +// GetSignBytes implements sdk.Msg. func (msg MsgTransfer) GetSignBytes() []byte { - panic("IBC messages do not support amino") + return sdk.MustSortJSON(AminoCdc.MustMarshalJSON(&msg)) } // GetSigners implements sdk.Msg diff --git a/x/ibc/applications/transfer/types/msgs_test.go b/x/ibc/applications/transfer/types/msgs_test.go index 89bd39524a51..1fc70c543bbe 100644 --- a/x/ibc/applications/transfer/types/msgs_test.go +++ b/x/ibc/applications/transfer/types/msgs_test.go @@ -1,6 +1,7 @@ package types import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -51,6 +52,15 @@ func TestMsgTransferType(t *testing.T) { require.Equal(t, "transfer", msg.Type()) } +func TestMsgTransferGetSignBytes(t *testing.T) { + msg := NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0) + expected := fmt.Sprintf(`{"type":"cosmos-sdk/MsgTransfer","value":{"receiver":"%s","sender":"%s","source_channel":"testchannel","source_port":"testportid","timeout_height":{"revision_height":"10"},"token":{"amount":"100","denom":"atom"}}}`, addr2, addr1) + require.NotPanics(t, func() { + res := msg.GetSignBytes() + require.Equal(t, expected, string(res)) + }) +} + // TestMsgTransferValidation tests ValidateBasic for MsgTransfer func TestMsgTransferValidation(t *testing.T) { testCases := []struct {