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

feat: enforce intermediate forwarder #69

Closed
wants to merge 1 commit into from
+127 −1
Closed

feat: enforce intermediate forwarder #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions middleware/packet-forward-middleware/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ mocks: $(MOCKS_DIR)
mockgen -package=mock -destination=./test/mock/bank_keeper.go $(GOMOD)/router/types BankKeeper
mockgen -package=mock -destination=./test/mock/ics4_wrapper.go github.com/cosmos/ibc-go/v7/modules/core/05-port/types ICS4Wrapper
mockgen -package=mock -destination=./test/mock/ibc_module.go github.com/cosmos/ibc-go/v7/modules/core/05-port/types IBCModule
mockgen -package=mock -destination=./test/mock/auth_keeper.go $(GOMOD)/router/types AuthKeeper

.PHONY: mocks

Expand Down
6 changes: 6 additions & 0 deletions middleware/packet-forward-middleware/router/ibc_middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ func (im IBCMiddleware) OnRecvPacket(
im.keeper.Logger(ctx).Debug("packetForwardMiddleware OnRecvPacket forward metadata does not exist")
return im.app.OnRecvPacket(ctx, packet, relayer)
}

forwarder := im.keeper.GetForwarderAddress(ctx)
if data.Receiver != forwarder.String() {
return channeltypes.NewErrorAcknowledgement(fmt.Errorf("packetForwardMiddleware invalid receiver address, expected %s, got %s", forwarder, data.Receiver))
}

m := &types.PacketMetadata{}
err = json.Unmarshal([]byte(data.Memo), m)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions middleware/packet-forward-middleware/router/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type Keeper struct {
distrKeeper types.DistributionKeeper
bankKeeper types.BankKeeper
ics4Wrapper porttypes.ICS4Wrapper
authKeeper types.AuthKeeper
}

// NewKeeper creates a new forward Keeper instance
Expand All @@ -66,6 +67,7 @@ func NewKeeper(
distrKeeper types.DistributionKeeper,
bankKeeper types.BankKeeper,
ics4Wrapper porttypes.ICS4Wrapper,
authKeeper types.AuthKeeper,
) *Keeper {
// set KeyTable if it has not already been set
if !paramSpace.HasKeyTable() {
Expand All @@ -81,6 +83,7 @@ func NewKeeper(
distrKeeper: distrKeeper,
bankKeeper: bankKeeper,
ics4Wrapper: ics4Wrapper,
authKeeper: authKeeper,
}
}

Expand Down Expand Up @@ -474,3 +477,7 @@ func (k *Keeper) GetAppVersion(
func (k *Keeper) LookupModuleByChannel(ctx sdk.Context, portID, channelID string) (string, *capabilitytypes.Capability, error) {
return k.channelKeeper.LookupModuleByChannel(ctx, portID, channelID)
}

func (k *Keeper) GetForwarderAddress(ctx sdk.Context) sdk.AccAddress {
return k.authKeeper.GetModuleAddress(types.ModuleName)
}
54 changes: 54 additions & 0 deletions middleware/packet-forward-middleware/router/module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ func TestOnRecvPacket_ForwardNoFee(t *testing.T) {

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packetOrig, senderAccAddr).
Return(acknowledgement),

Expand Down Expand Up @@ -326,6 +328,8 @@ func TestOnRecvPacket_ForwardAmountInt256(t *testing.T) {

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packetOrig, senderAccAddr).
Return(acknowledgement),

Expand Down Expand Up @@ -393,6 +397,8 @@ func TestOnRecvPacket_ForwardWithFee(t *testing.T) {

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packetOrig, senderAccAddr).
Return(acknowledgement),

Expand Down Expand Up @@ -504,6 +510,8 @@ func TestOnRecvPacket_ForwardMultihopStringNext(t *testing.T) {

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packetOrig, senderAccAddr).
Return(acknowledgement),

Expand All @@ -512,6 +520,8 @@ func TestOnRecvPacket_ForwardMultihopStringNext(t *testing.T) {
msgTransfer1,
).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil),

setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr2)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packet2, senderAccAddr2).
Return(acknowledgement),

Expand Down Expand Up @@ -623,6 +633,8 @@ func TestOnRecvPacket_ForwardMultihopJSONNext(t *testing.T) {

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packetOrig, senderAccAddr).
Return(acknowledgement),

Expand All @@ -631,6 +643,8 @@ func TestOnRecvPacket_ForwardMultihopJSONNext(t *testing.T) {
msgTransfer1,
).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil),

setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr2)),

setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packet2, senderAccAddr2).
Return(acknowledgement),

Expand Down Expand Up @@ -662,3 +676,43 @@ func TestOnRecvPacket_ForwardMultihopJSONNext(t *testing.T) {
err = forwardMiddleware.OnAcknowledgementPacket(ctx, packet2, successAck, senderAccAddr)
require.NoError(t, err)
}

func TestOnRecvPacket_WrongHostAddr(t *testing.T) {
ctl := gomock.NewController(t)
defer ctl.Finish()
setup := test.NewTestSetup(t, ctl)
ctx := setup.Initializer.Ctx
cdc := setup.Initializer.Marshaler
forwardMiddleware := setup.ForwardMiddleware

// Test data
const (
hostAddr = "cosmos1vzxkv3lxccnttr9rs0002s93sgw72h7ghukuhs"
hostAddr2 = "cosmos1q4p4gx889lfek5augdurrjclwtqvjhuntm6j4m"
destAddr = "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k"
port = "transfer"
channel = "channel-0"
)
senderAccAddr := test.AccAddress()

packetOrig := transferPacket(t, hostAddr2, &types.PacketMetadata{
Forward: &types.ForwardMetadata{
Receiver: destAddr,
Port: port,
Channel: channel,
},
})

// Expected mocks
gomock.InOrder(
setup.Mocks.AuthKeeperMock.EXPECT().GetModuleAddress(types.ModuleName).Return(sdk.MustAccAddressFromBech32(hostAddr)),
)

ack := forwardMiddleware.OnRecvPacket(ctx, packetOrig, senderAccAddr)
require.False(t, ack.Success())

expectedAck := &channeltypes.Acknowledgement{}
err := cdc.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
require.NoError(t, err)
require.Equal(t, "ABCI code: 1: error handling packet: see events for details", expectedAck.GetError())
}
4 changes: 4 additions & 0 deletions middleware/packet-forward-middleware/router/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ type BankKeeper interface {
SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
}

type AuthKeeper interface {
GetModuleAddress(name string) sdk.AccAddress
}
49 changes: 49 additions & 0 deletions middleware/packet-forward-middleware/test/mock/auth_keeper.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion middleware/packet-forward-middleware/test/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ func NewTestSetup(t *testing.T, ctl *gomock.Controller) *Setup {
bankKeeperMock := mock.NewMockBankKeeper(ctl)
ibcModuleMock := mock.NewMockIBCModule(ctl)
ics4WrapperMock := mock.NewMockICS4Wrapper(ctl)
authKeeperMock := mock.NewMockAuthKeeper(ctl)

paramsKeeper := initializer.paramsKeeper()
routerKeeper := initializer.routerKeeper(paramsKeeper, transferKeeperMock, channelKeeperMock, distributionKeeperMock, bankKeeperMock, ics4WrapperMock)
routerKeeper := initializer.routerKeeper(paramsKeeper, transferKeeperMock, channelKeeperMock, distributionKeeperMock, bankKeeperMock, ics4WrapperMock, authKeeperMock)
// routerModule := initializer.routerModule(routerKeeper)

require.NoError(t, initializer.StateStore.LoadLatestVersion())
Expand All @@ -58,6 +59,7 @@ func NewTestSetup(t *testing.T, ctl *gomock.Controller) *Setup {
DistributionKeeperMock: distributionKeeperMock,
IBCModuleMock: ibcModuleMock,
ICS4WrapperMock: ics4WrapperMock,
AuthKeeperMock: authKeeperMock,
},

ForwardMiddleware: initializer.forwardMiddleware(ibcModuleMock, routerKeeper, 0, keeper.DefaultForwardTransferPacketTimeoutTimestamp, keeper.DefaultRefundTransferPacketTimeoutTimestamp),
Expand All @@ -83,6 +85,7 @@ type testMocks struct {
DistributionKeeperMock *mock.MockDistributionKeeper
IBCModuleMock *mock.MockIBCModule
ICS4WrapperMock *mock.MockICS4Wrapper
AuthKeeperMock *mock.MockAuthKeeper
}

type initializer struct {
Expand Down Expand Up @@ -133,6 +136,7 @@ func (i initializer) routerKeeper(
distributionKeeper types.DistributionKeeper,
bankKeeper types.BankKeeper,
ics4Wrapper porttypes.ICS4Wrapper,
authKeeper types.AuthKeeper,
) *keeper.Keeper {
storeKey := sdk.NewKVStoreKey(types.StoreKey)
i.StateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, i.DB)
Expand All @@ -147,6 +151,7 @@ func (i initializer) routerKeeper(
distributionKeeper,
bankKeeper,
ics4Wrapper,
authKeeper,
)

return routerKeeper
Expand Down