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

ica: move Serialize/DeserializeCosmosTx to package types #493

Merged
merged 5 commits into from
Nov 2, 2021
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
26 changes: 0 additions & 26 deletions modules/apps/27-interchain-accounts/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

baseapp "github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
Expand Down Expand Up @@ -55,31 +54,6 @@ func NewKeeper(
}
}

// SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are
// packed into Any's and inserted into the Messages field of a CosmosTx. The proto marshaled CosmosTx
// bytes are returned.
func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (bz []byte, err error) {
msgAnys := make([]*codectypes.Any, len(msgs))

for i, msg := range msgs {
msgAnys[i], err = codectypes.NewAnyWithValue(msg)
if err != nil {
return nil, err
}
}

cosmosTx := &types.CosmosTx{
Messages: msgAnys,
}

bz, err = cdc.Marshal(cosmosTx)
if err != nil {
return nil, err
}

return bz, nil
}

// Logger returns the application logger, scoped to the associated module
func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", host.ModuleName, types.ModuleName))
Expand Down
26 changes: 1 addition & 25 deletions modules/apps/27-interchain-accounts/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,6 @@ func (k Keeper) createOutgoingPacket(
return packet.Sequence, nil
}

// DeserializeCosmosTx unmarshals and unpacks a slice of transaction bytes
// into a slice of sdk.Msg's.
func (k Keeper) DeserializeCosmosTx(_ sdk.Context, data []byte) ([]sdk.Msg, error) {
var cosmosTx types.CosmosTx
if err := k.cdc.Unmarshal(data, &cosmosTx); err != nil {
return nil, err
}

msgs := make([]sdk.Msg, len(cosmosTx.Messages))

for i, any := range cosmosTx.Messages {
var msg sdk.Msg

err := k.cdc.UnpackAny(any, &msg)
if err != nil {
return nil, err
}

msgs[i] = msg
}

return msgs, nil
}

func (k Keeper) AuthenticateTx(ctx sdk.Context, msgs []sdk.Msg, portId string) error {
seen := map[string]bool{}
var signers []sdk.AccAddress
Expand Down Expand Up @@ -176,7 +152,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error

switch data.Type {
case types.EXECUTE_TX:
msgs, err := k.DeserializeCosmosTx(ctx, data.Data)
msgs, err := types.DeserializeCosmosTx(k.cdc, data.Data)
if err != nil {
return err
}
Expand Down
10 changes: 5 additions & 5 deletions modules/apps/27-interchain-accounts/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() {
interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
msg1 := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
msg2 := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
data, err := suite.chainB.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg1, msg2})
data, err := types.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg1, msg2})
suite.Require().NoError(err)
icaPacketData.Data = data
}, true,
Expand Down Expand Up @@ -79,7 +79,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() {
amount, _ := sdk.ParseCoinsNormalized("100stake")
interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
msg := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
data, err := suite.chainB.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg})
data, err := types.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg})
suite.Require().NoError(err)

// default packet data, must be modified in malleate for test cases expected to fail
Expand Down Expand Up @@ -122,7 +122,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
// build packet data
data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
data, err := types.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
suite.Require().NoError(err)

icaPacketData := types.InterchainAccountPacketData{
Expand All @@ -146,7 +146,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
{
"Invalid packet type", func() {
// build packet data
data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{&banktypes.MsgSend{}})
data, err := types.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{&banktypes.MsgSend{}})
suite.Require().NoError(err)

// Type here is an ENUM
Expand Down Expand Up @@ -175,7 +175,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
// Incorrect FromAddress
msg = &banktypes.MsgSend{FromAddress: suite.chainB.SenderAccount.GetAddress().String(), ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
// build packet data
data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
data, err := types.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
suite.Require().NoError(err)
icaPacketData := types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Expand Down
50 changes: 50 additions & 0 deletions modules/apps/27-interchain-accounts/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package types
import (
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

Expand All @@ -22,3 +23,52 @@ func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations((*authtypes.AccountI)(nil), &InterchainAccount{})
}

// SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are
// packed into Any's and inserted into the Messages field of a CosmosTx. The proto marshaled CosmosTx
// bytes are returned.
func SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (bz []byte, err error) {
msgAnys := make([]*codectypes.Any, len(msgs))

for i, msg := range msgs {
msgAnys[i], err = codectypes.NewAnyWithValue(msg)
if err != nil {
return nil, err
}
}

cosmosTx := &CosmosTx{
Messages: msgAnys,
}

bz, err = cdc.Marshal(cosmosTx)
if err != nil {
return nil, err
}

return bz, nil
}

// DeserializeCosmosTx unmarshals and unpacks a slice of transaction bytes
// into a slice of sdk.Msg's.
func DeserializeCosmosTx(cdc codec.BinaryCodec, data []byte) ([]sdk.Msg, error) {
var cosmosTx CosmosTx
if err := cdc.Unmarshal(data, &cosmosTx); err != nil {
return nil, err
}

msgs := make([]sdk.Msg, len(cosmosTx.Messages))

for i, any := range cosmosTx.Messages {
var msg sdk.Msg

err := cdc.UnpackAny(any, &msg)
if err != nil {
return nil, err
}

msgs[i] = msg
}

return msgs, nil
}
130 changes: 130 additions & 0 deletions modules/apps/27-interchain-accounts/types/codec_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
package types_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
"github.com/cosmos/ibc-go/v2/testing/simapp"
)

// caseRawBytes defines a helper struct, used for testing codec operations
type caseRawBytes struct {
name string
bz []byte
expPass bool
}

// mockSdkMsg defines a mock struct, used for testing codec error scenarios
type mockSdkMsg struct{}

// Reset implements sdk.Msg
func (mockSdkMsg) Reset() {
}

// String implements sdk.Msg
func (mockSdkMsg) String() string {
return ""
}

// ProtoMessage implements sdk.Msg
func (mockSdkMsg) ProtoMessage() {
}

// ValidateBasic implements sdk.Msg
func (mockSdkMsg) ValidateBasic() error {
return nil
}

// GetSigners implements sdk.Msg
func (mockSdkMsg) GetSigners() []sdk.AccAddress {
return []sdk.AccAddress{}
}

func (suite *TypesTestSuite) TestSerializeCosmosTx() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adapted these tests from the anys in 02-client. It covers the happy path successfully, but it's seemingly harder to trigger an error than you'd expect. Passing nil for example will just evaluate as an empty list and not fail

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to create an unregistered sdk.Msg type. I think passing in a non pointer sdk.Msg might also work

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried using a value type (non-pointer) sdk.Msg and it can't be done as ProtoMessage() only exists on pointer receiver structs.

I've updated to include a private/unexported struct in the test file - mockSdkMsg and implemented the sdk.Msg interface funcs with some empty boilerplate. This can trigger a failure scenario of an unregistered msg type.


testCases := []struct {
name string
msgs []sdk.Msg
expPass bool
}{
{
"single msg",
[]sdk.Msg{
&banktypes.MsgSend{
FromAddress: TestOwnerAddress,
ToAddress: TestOwnerAddress,
Amount: sdk.NewCoins(sdk.NewCoin("bananas", sdk.NewInt(100))),
},
},
true,
},
{
"multiple msgs, same types",
[]sdk.Msg{
&banktypes.MsgSend{
FromAddress: TestOwnerAddress,
ToAddress: TestOwnerAddress,
Amount: sdk.NewCoins(sdk.NewCoin("bananas", sdk.NewInt(100))),
},
&banktypes.MsgSend{
FromAddress: TestOwnerAddress,
ToAddress: TestOwnerAddress,
Amount: sdk.NewCoins(sdk.NewCoin("bananas", sdk.NewInt(200))),
},
},
true,
},
{
"multiple msgs, different types",
[]sdk.Msg{
&banktypes.MsgSend{
FromAddress: TestOwnerAddress,
ToAddress: TestOwnerAddress,
Amount: sdk.NewCoins(sdk.NewCoin("bananas", sdk.NewInt(100))),
},
&govtypes.MsgSubmitProposal{
InitialDeposit: sdk.NewCoins(sdk.NewCoin("bananas", sdk.NewInt(100))),
Proposer: TestOwnerAddress,
},
},
true,
},
{
"unregistered msg type",
[]sdk.Msg{
&mockSdkMsg{},
},
false,
},
{
"multiple unregistered msg types",
[]sdk.Msg{
&mockSdkMsg{},
&mockSdkMsg{},
&mockSdkMsg{},
},
false,
},
}

testCasesAny := []caseRawBytes{}

for _, tc := range testCases {
bz, err := types.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, tc.msgs)
suite.Require().NoError(err, tc.name)

testCasesAny = append(testCasesAny, caseRawBytes{tc.name, bz, tc.expPass})
}

for i, tc := range testCasesAny {
msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, tc.bz)
if tc.expPass {
suite.Require().NoError(err, tc.name)
suite.Require().Equal(testCases[i].msgs, msgs, tc.name)
} else {
suite.Require().Error(err, tc.name)
}
}
}
3 changes: 0 additions & 3 deletions modules/apps/27-interchain-accounts/types/encoder.go

This file was deleted.

30 changes: 0 additions & 30 deletions modules/apps/27-interchain-accounts/types/hook.go

This file was deleted.