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

feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented #4188

Merged
merged 14 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import (
ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
)

var _ porttypes.Middleware = (*IBCMiddleware)(nil)
var (
_ porttypes.Middleware = (*IBCMiddleware)(nil)
_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
)

// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
// ICA controller keeper and the underlying application.
Expand Down Expand Up @@ -253,3 +256,15 @@ func (im IBCMiddleware) WriteAcknowledgement(
func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return im.keeper.GetAppVersion(ctx, portID, channelID)
}

// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
// into an InterchainAccountPacketData. This function implements the optional
// PacketDataUnmarshaler interface required for ADR 008 support.
func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) {
var packetData icatypes.InterchainAccountPacketData
if err := icatypes.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
return nil, err
}

return packetData, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -918,3 +918,26 @@ func (suite *InterchainAccountsTestSuite) TestClosedChannelReopensWithMsgServer(
err = path.EndpointB.ChanOpenConfirm()
suite.Require().NoError(err)
}

func (suite *InterchainAccountsTestSuite) TestPacketDataUnmarshalerInterface() {
path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

expPacketData := icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: []byte("data"),
Memo: "",
}

packetData, err := controller.IBCMiddleware{}.UnmarshalPacketData(expPacketData.GetBytes())
suite.Require().NoError(err)
suite.Require().Equal(expPacketData, packetData)

// test invalid packet data
invalidPacketData := []byte("invalid packet data")
packetData, err = controller.IBCMiddleware{}.UnmarshalPacketData(invalidPacketData)
suite.Require().Error(err)
suite.Require().Nil(packetData)
}
17 changes: 16 additions & 1 deletion modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import (
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

var _ porttypes.Middleware = (*IBCMiddleware)(nil)
var (
_ porttypes.Middleware = (*IBCMiddleware)(nil)
_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
)

// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
// fee keeper and the underlying application.
Expand Down Expand Up @@ -348,3 +351,15 @@ func (im IBCMiddleware) WriteAcknowledgement(
func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return im.keeper.GetAppVersion(ctx, portID, channelID)
}

// UnmarshalPacketData attempts to use the underlying app to unmarshal the packet data.
// If the underlying app does not support the PacketDataUnmarshaler interface, an error is returned.
// This function implements the optional PacketDataUnmarshaler interface required for ADR 008 support.
func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) {
unmarshaler, ok := im.app.(porttypes.PacketDataUnmarshaler)
if !ok {
return nil, errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
}

return unmarshaler.UnmarshalPacketData(bz)
}
26 changes: 26 additions & 0 deletions modules/apps/29-fee/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ package fee_test
import (
"fmt"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
fee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee"
feekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper"
"github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
Expand Down Expand Up @@ -1080,3 +1083,26 @@ func (suite *FeeTestSuite) TestGetAppVersion() {
})
}
}

func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterface() {
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

feeModule, ok := cbs.(porttypes.PacketDataUnmarshaler)
suite.Require().True(ok)

packetData, err := feeModule.UnmarshalPacketData(ibcmock.MockPacketData)
suite.Require().NoError(err)
suite.Require().Equal(ibcmock.MockPacketData, packetData)
}

func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterfaceError() {
// test the case when the underlying application cannot be casted to a PacketDataUnmarshaler
mockFeeMiddleware := fee.NewIBCMiddleware(nil, feekeeper.Keeper{})

_, err := mockFeeMiddleware.UnmarshalPacketData(ibcmock.MockPacketData)
suite.Require().ErrorIs(err, errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil)))
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions modules/apps/29-fee/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ var (
ErrFeeNotEnabled = errorsmod.Register(ModuleName, 9, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled")
ErrRelayerNotFoundForAsyncAck = errorsmod.Register(ModuleName, 10, "relayer address must be stored for async WriteAcknowledgement")
ErrFeeModuleLocked = errorsmod.Register(ModuleName, 11, "the fee module is currently locked, a severe bug has been detected")
ErrUnsupportedAction = errorsmod.Register(ModuleName, 12, "unsupported action")
)
17 changes: 17 additions & 0 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ import (
ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
)

var (
_ porttypes.IBCModule = (*IBCModule)(nil)
_ porttypes.PacketDataUnmarshaler = (*IBCModule)(nil)
)

// IBCModule implements the ICS26 interface for transfer given the transfer keeper.
type IBCModule struct {
keeper keeper.Keeper
Expand Down Expand Up @@ -301,3 +306,15 @@ func (im IBCModule) OnTimeoutPacket(

return nil
}

// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
// into a FungibleTokenPacketData. This function implements the optional
// PacketDataUnmarshaler interface required for ADR 008 support.
func (im IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
var packetData types.FungibleTokenPacketData
if err := types.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
return nil, err
}

return packetData, nil
}
60 changes: 60 additions & 0 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package transfer_test

import (
"fmt"
"math"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
Expand Down Expand Up @@ -238,3 +242,59 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
})
}
}

func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
var (
sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
srcCallbackAddr = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
destCallbackAddr = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
denom = "transfer/channel-0/atom"
amount = "100"
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

data []byte
expPacketData types.FungibleTokenPacketData
)

testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success: both callbacks",
func() {
expPacketData = types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: sender,
Receiver: receiver,
Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, srcCallbackAddr, destCallbackAddr),
}
data = expPacketData.GetBytes()
},
true,
},
{
"failure: invalid packet data",
func() {
data = []byte("invalid packet data")
},
false,
},
}

for _, tc := range testCases {
tc.malleate()

packetData, err := transfer.IBCModule{}.UnmarshalPacketData(data)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().Equal(expPacketData, packetData)
} else {
suite.Require().Error(err)
suite.Require().Nil(packetData)
}
}
}
7 changes: 7 additions & 0 deletions modules/core/05-port/types/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,10 @@ type Middleware interface {
IBCModule
ICS4Wrapper
}

// PacketDataUnmarshaler defines an optional interface which allows a middleware to
// request the packet data to be unmarshaled by the base application.
type PacketDataUnmarshaler interface {
// UnmarshalPacketData unmarshals the packet data into a concrete type
UnmarshalPacketData([]byte) (interface{}, error)
}
16 changes: 16 additions & 0 deletions testing/mock/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,24 @@ package mock
import (
"bytes"
"fmt"
"reflect"
"strconv"
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

var (
_ porttypes.IBCModule = (*IBCModule)(nil)
_ porttypes.PacketDataUnmarshaler = (*IBCModule)(nil)
)

// IBCModule implements the ICS26 callbacks for testing/mock.
type IBCModule struct {
appModule *AppModule
Expand Down Expand Up @@ -162,6 +169,15 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,
return nil
}

// UnmarshalPacketData returns the MockPacketData. This function implements the optional
// PacketDataUnmarshaler interface required for ADR 008 support.
func (im IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
if reflect.DeepEqual(bz, MockPacketData) {
return MockPacketData, nil
}
return nil, ErrorMock
}

// GetMockRecvCanaryCapabilityName generates a capability name for testing OnRecvPacket functionality.
func GetMockRecvCanaryCapabilityName(packet channeltypes.Packet) string {
return fmt.Sprintf("%s%s%s%s", MockRecvCanaryCapabilityName, packet.GetDestPort(), packet.GetDestChannel(), strconv.Itoa(int(packet.GetSequence())))
Expand Down
1 change: 1 addition & 0 deletions testing/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const (
var (
MockAcknowledgement = channeltypes.NewResultAcknowledgement([]byte("mock acknowledgement"))
MockFailAcknowledgement = channeltypes.NewErrorAcknowledgement(fmt.Errorf("mock failed acknowledgement"))
ErrorMock = fmt.Errorf("mock failed acknowledgement")
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
MockPacketData = []byte("mock packet data")
MockFailPacketData = []byte("mock failed packet data")
MockAsyncPacketData = []byte("mock async packet data")
Expand Down