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

ics4 callbacks fee middleware #580

Merged
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
22 changes: 20 additions & 2 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

- [ibc/applications/fee/v1/genesis.proto](#ibc/applications/fee/v1/genesis.proto)
- [FeeEnabledChannel](#ibc.applications.fee.v1.FeeEnabledChannel)
- [ForwardRelayerAddress](#ibc.applications.fee.v1.ForwardRelayerAddress)
- [GenesisState](#ibc.applications.fee.v1.GenesisState)
- [RegisteredRelayerAddress](#ibc.applications.fee.v1.RegisteredRelayerAddress)

Expand Down Expand Up @@ -749,7 +750,7 @@ and an optional list of relayers that are permitted to receive the fee.
<a name="ibc.applications.fee.v1.FeeEnabledChannel"></a>

### FeeEnabledChannel
Contains the PortID & ChannelID for a fee enabled channel
FeeEnabledChannel contains the PortID & ChannelID for a fee enabled channel


| Field | Type | Label | Description |
Expand All @@ -762,6 +763,22 @@ Contains the PortID & ChannelID for a fee enabled channel



<a name="ibc.applications.fee.v1.ForwardRelayerAddress"></a>

### ForwardRelayerAddress
ForwardRelayerAddress contains the forward relayer address and packetId used for async acknowledgements


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `address` | [string](#string) | | |
| `packet_id` | [ibc.core.channel.v1.PacketId](#ibc.core.channel.v1.PacketId) | | |






<a name="ibc.applications.fee.v1.GenesisState"></a>

### GenesisState
Expand All @@ -773,6 +790,7 @@ GenesisState defines the fee middleware genesis state
| `identified_fees` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | repeated | |
| `fee_enabled_channels` | [FeeEnabledChannel](#ibc.applications.fee.v1.FeeEnabledChannel) | repeated | |
| `registered_relayers` | [RegisteredRelayerAddress](#ibc.applications.fee.v1.RegisteredRelayerAddress) | repeated | |
| `forward_relayers` | [ForwardRelayerAddress](#ibc.applications.fee.v1.ForwardRelayerAddress) | repeated | |



Expand All @@ -782,7 +800,7 @@ GenesisState defines the fee middleware genesis state
<a name="ibc.applications.fee.v1.RegisteredRelayerAddress"></a>

### RegisteredRelayerAddress
Contains the address and counterparty address for a specific relayer (for distributing fees)
RegisteredRelayerAddress contains the address and counterparty address for a specific relayer (for distributing fees)


| Field | Type | Label | Description |
Expand Down
13 changes: 6 additions & 7 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ require (
gopkg.in/yaml.v2 v2.4.0
)

require (
github.com/gin-gonic/gin v1.7.0 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/runc v1.0.3 // indirect
)

require (
filippo.io/edwards25519 v1.0.0-beta.2 // indirect
github.com/99designs/keyring v1.1.6 // indirect
Expand Down Expand Up @@ -117,10 +123,3 @@ require (
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
nhooyr.io/websocket v1.8.6 // indirect
)

require (
github.com/cosmos/ibc-go v1.2.5
github.com/gin-gonic/gin v1.7.0 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/runc v1.0.3 // indirect
)
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY
github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw=
github.com/cosmos/iavl v0.17.3 h1:s2N819a2olOmiauVa0WAhoIJq9EhSXE9HDBAoR9k+8Y=
github.com/cosmos/iavl v0.17.3/go.mod h1:prJoErZFABYZGDHka1R6Oay4z9PrNeFFiMKHDAMOi4w=
github.com/cosmos/ibc-go v1.2.5 h1:BiA48yKEDUcabBRkmp7qqSX41ZrgXTSNCtdjDURbLwE=
github.com/cosmos/ibc-go v1.2.5/go.mod h1:wkGkkX8Ou6yXgE8lO2xP9NOwo+Tl5x1dJaTTE6jBDpg=
github.com/cosmos/ledger-cosmos-go v0.11.1 h1:9JIYsGnXP613pb2vPjFeMMjBI5lEDsEaF6oYorTy6J4=
github.com/cosmos/ledger-cosmos-go v0.11.1/go.mod h1:J8//BsAGTo3OC/vDLjMRFLW6q0WAaXvHnVc7ZmE8iUY=
github.com/cosmos/ledger-go v0.9.2 h1:Nnao/dLwaVTk1Q5U9THldpUMMXU94BOTWPddSmVB6pI=
Expand Down Expand Up @@ -630,7 +628,6 @@ github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:F
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.2/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.3 h1:OVowDSCllw/YjdLkam3/sm7wEtOy59d8ndGgCcyj8cs=
github.com/mitchellh/mapstructure v1.4.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/moby/sys/mountinfo v0.4.1/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A=
Expand Down Expand Up @@ -1458,7 +1455,6 @@ gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMy
gopkg.in/gcfg.v1 v1.2.3/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o=
gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.63.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.66.2 h1:XfR1dOYubytKy4Shzc2LHrrGhU0lDCfDGG1yLPmpgsI=
gopkg.in/ini.v1 v1.66.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce/go.mod h1:5AcXVHNjg+BDxry382+8OKon8SEWiKktQR07RKPsv1c=
Expand Down
11 changes: 5 additions & 6 deletions modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,13 @@ func (im IBCModule) OnRecvPacket(
ack := im.app.OnRecvPacket(ctx, packet, relayer)

forwardRelayer, found := im.keeper.GetCounterpartyAddress(ctx, relayer.String())
if !found {
forwardRelayer = ""
}

return types.IncentivizedAcknowledgement{
Result: ack.Acknowledgement(),
ForwardRelayerAddress: forwardRelayer,
// incase of async aknowledgement (ack == nil) store the ForwardRelayer address for use later
if ack == nil && found {
im.keeper.SetForwardRelayerAddress(ctx, channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence()), forwardRelayer)
}

return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement())
}

// OnAcknowledgementPacket implements the IBCModule interface
Expand Down
1 change: 0 additions & 1 deletion modules/apps/29-fee/keeper/escrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee *types.Identified
}

// Store fee in state for reference later
// feeInEscrow/<port-id>/<channel-id>/packet/<sequence-id>/ -> Fee (timeout, ack, onrecv)
k.SetFeeInEscrow(ctx, identifiedFee)
return nil
}
Expand Down
5 changes: 5 additions & 0 deletions modules/apps/29-fee/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
k.SetCounterpartyAddress(ctx, addr.Address, addr.CounterpartyAddress)
}

for _, forwardAddr := range state.ForwardRelayers {
k.SetForwardRelayerAddress(ctx, forwardAddr.PacketId, forwardAddr.Address)
}

for _, enabledChan := range state.FeeEnabledChannels {
k.SetFeeEnabled(ctx, enabledChan.PortId, enabledChan.ChannelId)
}
Expand All @@ -27,5 +31,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
IdentifiedFees: k.GetAllIdentifiedPacketFees(ctx),
FeeEnabledChannels: k.GetAllFeeEnabledChannels(ctx),
RegisteredRelayers: k.GetAllRelayerAddresses(ctx),
ForwardRelayers: k.GetAllForwardRelayerAddresses(ctx),
}
}
7 changes: 7 additions & 0 deletions modules/apps/29-fee/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
// set counterparty address
suite.chainA.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainA.GetContext(), sender, counterparty)

// set forward relayer address
suite.chainA.GetSimApp().IBCFeeKeeper.SetForwardRelayerAddress(suite.chainA.GetContext(), packetId, sender)

// export genesis
genesisState := suite.chainA.GetSimApp().IBCFeeKeeper.ExportGenesis(suite.chainA.GetContext())

Expand All @@ -110,4 +113,8 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
// check registered relayer addresses
suite.Require().Equal(sender, genesisState.RegisteredRelayers[0].Address)
suite.Require().Equal(counterparty, genesisState.RegisteredRelayers[0].CounterpartyAddress)

// check registered relayer addresses
suite.Require().Equal(sender, genesisState.ForwardRelayers[0].Address)
suite.Require().Equal(packetId, genesisState.ForwardRelayers[0].PacketId)
}
72 changes: 55 additions & 17 deletions modules/apps/29-fee/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper

import (
"strconv"
"strings"

"github.com/cosmos/cosmos-sdk/codec"
Expand All @@ -12,7 +13,6 @@ import (
"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// Middleware must implement types.ChannelKeeper and types.PortKeeper expected interfaces
Expand Down Expand Up @@ -77,22 +77,6 @@ func (k Keeper) GetFeeModuleAddress() sdk.AccAddress {
return k.authKeeper.GetModuleAddress(types.ModuleName)
}

// SendPacket wraps IBC ChannelKeeper's SendPacket function
func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) error {
return k.ics4Wrapper.SendPacket(ctx, chanCap, packet)
}

// WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
func (k Keeper) WriteAcknowledgement(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
packet ibcexported.PacketI,
acknowledgement []byte,
) error {
return nil
// return k.channelKeeper.WriteAcknowledgement(ctx, chanCap, packet, acknowledgement)
}

// SetFeeEnabled sets a flag to determine if fee handling logic should run for the given channel
// identified by channel and port identifiers.
func (k Keeper) SetFeeEnabled(ctx sdk.Context, portID, channelID string) {
Expand Down Expand Up @@ -169,6 +153,7 @@ func (k Keeper) GetCounterpartyAddress(ctx sdk.Context, address string) (string,
return addr, true
}

// 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))
Expand All @@ -189,6 +174,59 @@ func (k Keeper) GetAllRelayerAddresses(ctx sdk.Context) []*types.RegisteredRelay
return registeredAddrArr
}

// SetForwardRelayerAddress sets the forward relayer address during OnRecvPacket in case of async acknowledgement
func (k Keeper) SetForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId, address string) {
store := ctx.KVStore(k.storeKey)
store.Set(types.KeyForwardRelayerAddress(packetId), []byte(address))
}

// GetForwardRelayerAddress gets forward relayer address for a particular packet
func (k Keeper) GetForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := types.KeyForwardRelayerAddress(packetId)
if !store.Has(key) {
return "", false
}

addr := string(store.Get(key))
return addr, true
}

AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// GetAllForwardRelayerAddresses returns all forward relayer addresses stored for async acknowledgements
func (k Keeper) GetAllForwardRelayerAddresses(ctx sdk.Context) []*types.ForwardRelayerAddress {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a unit test for this? A followup pr is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll create an issue. Thanks

store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(types.ForwardRelayerPrefix))
defer iterator.Close()

var forwardRelayerAddr []*types.ForwardRelayerAddress
for ; iterator.Valid(); iterator.Next() {
keySplit := strings.Split(string(iterator.Key()), "/")

seq, err := strconv.ParseUint(keySplit[3], 0, 64)
if err != nil {
panic("failed to parse packet sequence in forward relayer address mapping")
}

packetId := channeltypes.NewPacketId(keySplit[2], keySplit[1], seq)

addr := &types.ForwardRelayerAddress{
Address: string(iterator.Value()),
PacketId: packetId,
}

forwardRelayerAddr = append(forwardRelayerAddr, addr)
}

return forwardRelayerAddr
}

// Deletes the forwardRelayerAddr associated with the packetId
func (k Keeper) DeleteForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId) {
store := ctx.KVStore(k.storeKey)
key := types.KeyForwardRelayerAddress(packetId)
store.Delete(key)
}

// Stores a Fee for a given packet in state
func (k Keeper) SetFeeInEscrow(ctx sdk.Context, fee *types.IdentifiedPacketFee) {
store := ctx.KVStore(k.storeKey)
Expand Down
31 changes: 31 additions & 0 deletions modules/apps/29-fee/keeper/relay.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// SendPacket wraps IBC ChannelKeeper's SendPacket function
func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) error {
return k.ics4Wrapper.SendPacket(ctx, chanCap, packet)
}

// WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
// ICS29 WriteAcknowledgement is used for asynchronous acknowledgements
func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement []byte) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AdityaSripal I guess I need the OnRecvPacket functionality finished before I can write a test for this?

#262

Copy link
Member

Choose a reason for hiding this comment

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

Yea because OnRecvPacket should ONLY set ForwardRelayerAddress if the underlying application returns a nil acknowledgement. cc: @charleenfei

No need to store it if you're going to send it out immediately

// retrieve the forward relayer that was stored in `onRecvPacket`
packetId := channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence())
relayer, _ := k.GetForwardRelayerAddress(ctx, packetId)

k.DeleteForwardRelayerAddress(ctx, packetId)

AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
ack := types.NewIncentivizedAcknowledgement(relayer, acknowledgement)
bz := ack.Acknowledgement()

// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, bz)
}
66 changes: 66 additions & 0 deletions modules/apps/29-fee/keeper/relay_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package keeper_test

import (
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
)

func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add a test case for the forward relayer address being set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added a check to ensure it gets successfully deleted if set, and no-ops otherwise.

func() {},
true,
},
{
"forward relayer address is successfully deleted",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetForwardRelayerAddress(suite.chainB.GetContext(), channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, 1), suite.chainA.SenderAccount.GetAddress().String())
},
true,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()

// open incentivized channel
suite.coordinator.Setup(suite.path)

// build packet
timeoutTimestamp := ^uint64(0)
packet := channeltypes.NewPacket(
[]byte("packetData"),
1,
suite.path.EndpointA.ChannelConfig.PortID,
suite.path.EndpointA.ChannelID,
suite.path.EndpointB.ChannelConfig.PortID,
suite.path.EndpointB.ChannelID,
clienttypes.ZeroHeight(),
timeoutTimestamp,
)

ack := []byte("ack")
chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)

// malleate test case
tc.malleate()

err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)

if tc.expPass {
suite.Require().NoError(err)
_, found := suite.chainB.GetSimApp().IBCFeeKeeper.GetForwardRelayerAddress(suite.chainB.GetContext(), channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, 1))
suite.Require().False(found)
} else {
suite.Require().Error(err)
}
})
}
}
8 changes: 8 additions & 0 deletions modules/apps/29-fee/types/ack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// NewIncentivizedAcknowledgement creates a new instance of IncentivizedAcknowledgement
func NewIncentivizedAcknowledgement(relayer string, ack []byte) IncentivizedAcknowledgement {
return IncentivizedAcknowledgement{
Result: ack,
ForwardRelayerAddress: relayer,
}
}

Copy link
Member

Choose a reason for hiding this comment

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

You also need to add a Success method here. Just return false if ForwardRelayerAddress is empty, and true otherwise

Copy link
Contributor Author

@seantking seantking Dec 2, 2021

Choose a reason for hiding this comment

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

I think @charleenfei has this on her branch. I'll copy it over

// Success implements the Acknowledgement interface. The acknowledgement is
// considered successful if the forward relayer address is empty. Otherwise it is
// considered a failed acknowledgement.
Expand Down
Loading