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.

Sign up for GitHub

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

Jump to bottom

ica: move Serialize/DeserializeCosmosTx to package types #493

Merged
merged 5 commits into from
Nov 2, 2021
Merged

ica: move Serialize/DeserializeCosmosTx to package types #493

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
997115a
moving SerializeCosmosTx and DeserializeCosmosTx to types pkg
damiannolan Oct 13, 2021
b19b590
removing dead code
damiannolan Oct 13, 2021
2218408
adding mockSdkMsg message type for failing codec test scenarios
damiannolan Nov 2, 2021
8fe0101
Update modules/apps/27-interchain-accounts/types/codec_test.go
damiannolan Nov 2, 2021
06bd2b3
Merge branch 'interchain-accounts' into damian/458-move-tx-serdes
damiannolan Nov 2, 2021
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 for testing codec error scenarios
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop

This file was deleted.

30 changes: 0 additions & 30 deletions modules/apps/27-interchain-accounts/types/hook.go
View file
Open in desktop

This file was deleted.