Skip to content

Commit

Permalink
ics29:fix: counterparty addr must contain channelID (#937)
Browse files Browse the repository at this point in the history
* fix: counterparty address must chain channelID

* nit: updating var name

* test: adding validation check for channelID

* nit: fn names
  • Loading branch information
seantking authored Feb 22, 2022
1 parent 74afccd commit 6928af7
Show file tree
Hide file tree
Showing 18 changed files with 228 additions and 113 deletions.
2 changes: 2 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ RegisteredRelayerAddress contains the address and counterparty address for a spe
| ----- | ---- | ----- | ----------- |
| `address` | [string](#string) | | |
| `counterparty_address` | [string](#string) | | |
| `channel_id` | [string](#string) | | |



Expand Down Expand Up @@ -1041,6 +1042,7 @@ MsgRegisterCounterpartyAddress is the request type for registering the counterpa
| ----- | ---- | ----- | ----------- |
| `address` | [string](#string) | | |
| `counterparty_address` | [string](#string) | | |
| `channel_id` | [string](#string) | | |



Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (im IBCModule) OnRecvPacket(

ack := im.app.OnRecvPacket(ctx, packet, relayer)

forwardRelayer, found := im.keeper.GetCounterpartyAddress(ctx, relayer.String())
forwardRelayer, found := im.keeper.GetCounterpartyAddress(ctx, relayer.String(), packet.DestinationChannel)

// incase of async aknowledgement (ack == nil) store the ForwardRelayer address for use later
if ack == nil && found {
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
{
"forward address is not found",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "", suite.path.EndpointB.ChannelID)
},
false,
true,
Expand All @@ -514,7 +514,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String())
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID)

// malleate test case
tc.malleate()
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
k.SetFeeInEscrow(ctx, fee)
}

for _, addr := range state.RegisteredRelayers {
k.SetCounterpartyAddress(ctx, addr.Address, addr.CounterpartyAddress)
for _, relayer := range state.RegisteredRelayers {
k.SetCounterpartyAddress(ctx, relayer.Address, relayer.CounterpartyAddress, relayer.ChannelId)
}

for _, forwardAddr := range state.ForwardRelayers {
Expand Down
5 changes: 3 additions & 2 deletions modules/apps/29-fee/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
{
Address: sender,
CounterpartyAddress: counterparty,
ChannelId: ibctesting.FirstChannelID,
},
},
}
Expand All @@ -59,7 +60,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
suite.Require().True(isEnabled)

// check relayers
addr, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(suite.chainA.GetContext(), sender)
addr, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(suite.chainA.GetContext(), sender, ibctesting.FirstChannelID)
suite.Require().True(found)
suite.Require().Equal(genesisState.RegisteredRelayers[0].CounterpartyAddress, addr)
}
Expand All @@ -84,7 +85,7 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
sender := suite.chainA.SenderAccount.GetAddress().String()
counterparty := suite.chainB.SenderAccount.GetAddress().String()
// set counterparty address
suite.chainA.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainA.GetContext(), sender, counterparty)
suite.chainA.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainA.GetContext(), sender, counterparty, ibctesting.FirstChannelID)

// set forward relayer address
suite.chainA.GetSimApp().IBCFeeKeeper.SetForwardRelayerAddress(suite.chainA.GetContext(), packetID, sender)
Expand Down
11 changes: 6 additions & 5 deletions modules/apps/29-fee/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,15 @@ func (k Keeper) DisableAllChannels(ctx sdk.Context) {

// SetCounterpartyAddress maps the destination chain relayer address to the source relayer address
// The receiving chain must store the mapping from: address -> counterpartyAddress for the given channel
func (k Keeper) SetCounterpartyAddress(ctx sdk.Context, address, counterpartyAddress string) {
func (k Keeper) SetCounterpartyAddress(ctx sdk.Context, address, counterpartyAddress, channelID string) {
store := ctx.KVStore(k.storeKey)
store.Set(types.KeyRelayerAddress(address), []byte(counterpartyAddress))
store.Set(types.KeyCounterpartyRelayer(address, channelID), []byte(counterpartyAddress))
}

// GetCounterpartyAddress gets the relayer counterparty address given a destination relayer address
func (k Keeper) GetCounterpartyAddress(ctx sdk.Context, address string) (string, bool) {
func (k Keeper) GetCounterpartyAddress(ctx sdk.Context, address, channelID string) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := types.KeyRelayerAddress(address)
key := types.KeyCounterpartyRelayer(address, channelID)

if !store.Has(key) {
return "", false
Expand All @@ -156,7 +156,7 @@ func (k Keeper) GetCounterpartyAddress(ctx sdk.Context, address string) (string,
// GetAllRelayerAddresses returns all registered relayer addresses
func (k Keeper) GetAllRelayerAddresses(ctx sdk.Context) []types.RegisteredRelayerAddress {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(types.RelayerAddressKeyPrefix))
iterator := sdk.KVStorePrefixIterator(store, []byte(types.CounterpartyRelayerAddressKeyPrefix))
defer iterator.Close()

var registeredAddrArr []types.RegisteredRelayerAddress
Expand All @@ -166,6 +166,7 @@ func (k Keeper) GetAllRelayerAddresses(ctx sdk.Context) []types.RegisteredRelaye
addr := types.RegisteredRelayerAddress{
Address: keySplit[1],
CounterpartyAddress: string(iterator.Value()),
ChannelId: keySplit[2],
}

registeredAddrArr = append(registeredAddrArr, addr)
Expand Down
3 changes: 2 additions & 1 deletion modules/apps/29-fee/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,13 @@ func (suite *KeeperTestSuite) TestGetAllRelayerAddresses() {
sender := suite.chainA.SenderAccount.GetAddress().String()
counterparty := suite.chainB.SenderAccount.GetAddress().String()

suite.chainA.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainA.GetContext(), sender, counterparty)
suite.chainA.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainA.GetContext(), sender, counterparty, ibctesting.FirstChannelID)

expectedAddr := []types.RegisteredRelayerAddress{
{
Address: sender,
CounterpartyAddress: counterparty,
ChannelId: ibctesting.FirstChannelID,
},
}

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (k Keeper) RegisterCounterpartyAddress(goCtx context.Context, msg *types.Ms
ctx := sdk.UnwrapSDKContext(goCtx)

k.SetCounterpartyAddress(
ctx, msg.Address, msg.CounterpartyAddress,
ctx, msg.Address, msg.CounterpartyAddress, msg.ChannelId,
)

k.Logger(ctx).Info("Registering counterparty address for relayer.", "Address:", msg.Address, "Counterparty Address:", msg.CounterpartyAddress)
Expand Down
5 changes: 3 additions & 2 deletions modules/apps/29-fee/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper_test
import (
"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
Expand Down Expand Up @@ -35,14 +36,14 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
sender = suite.chainA.SenderAccount.GetAddress().String()
counterparty = suite.chainB.SenderAccount.GetAddress().String()
tc.malleate()
msg := types.NewMsgRegisterCounterpartyAddress(sender, counterparty)
msg := types.NewMsgRegisterCounterpartyAddress(sender, counterparty, ibctesting.FirstChannelID)

_, err := suite.chainA.SendMsgs(msg)

if tc.expPass {
suite.Require().NoError(err) // message committed

counterpartyAddress, _ := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(ctx, suite.chainA.SenderAccount.GetAddress().String())
counterpartyAddress, _ := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(ctx, suite.chainA.SenderAccount.GetAddress().String(), ibctesting.FirstChannelID)
suite.Require().Equal(counterparty, counterpartyAddress)
} else {
suite.Require().Error(err)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (suite *FeeTestSuite) TestFeeTransfer() {
// to differentiate from the chainA.SenderAccount for checking successful relay payouts
relayerAddress := suite.chainB.SenderAccount.GetAddress()

msgRegister := types.NewMsgRegisterCounterpartyAddress(suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String())
msgRegister := types.NewMsgRegisterCounterpartyAddress(suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String(), ibctesting.FirstChannelID)
_, err = suite.chainB.SendMsgs(msgRegister)
suite.Require().NoError(err) // message committed

Expand Down
126 changes: 89 additions & 37 deletions modules/apps/29-fee/types/genesis.pb.go

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

10 changes: 5 additions & 5 deletions modules/apps/29-fee/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const (
// FeeEnabledPrefix is the key prefix for storing fee enabled flag
FeeEnabledKeyPrefix = "feeEnabled"

// RelayerAddressKeyPrefix is the key prefix for relayer address mapping
RelayerAddressKeyPrefix = "relayerAddress"
// CounterpartyRelayerAddressKeyPrefix is the key prefix for relayer address mapping
CounterpartyRelayerAddressKeyPrefix = "relayerAddress"

// FeeInEscrowPrefix is the key prefix for fee in escrow mapping
FeeInEscrowPrefix = "feeInEscrow"
Expand All @@ -47,9 +47,9 @@ func FeeEnabledKey(portID, channelID string) []byte {
return []byte(fmt.Sprintf("%s/%s/%s", FeeEnabledKeyPrefix, portID, channelID))
}

// KeyRelayerAddress returns the key for relayer address -> counteryparty address mapping
func KeyRelayerAddress(address string) []byte {
return []byte(fmt.Sprintf("%s/%s", RelayerAddressKeyPrefix, address))
// KeyCounterpartyRelayer returns the key for relayer address -> counteryparty address mapping
func KeyCounterpartyRelayer(address, channelID string) []byte {
return []byte(fmt.Sprintf("%s/%s/%s", CounterpartyRelayerAddressKeyPrefix, address, channelID))
}

// KeyForwardRelayerAddress returns the key for packetID -> forwardAddress mapping
Expand Down
Loading

0 comments on commit 6928af7

Please sign in to comment.