Skip to content

Commit

Permalink
feat: add channel version to core app callbacks (#6902)
Browse files Browse the repository at this point in the history 
* feat: add channel version to core app callbacks

* refactor 05-port module interface signature with channel version after ctx

* unwrap app version in ics29

* panic on unwrap app version and update mock apis

* discard unused variables

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
(cherry picked from commit 954d8bd)
  • Loading branch information
@gjermundgaraba @mergify
gjermundgaraba authored and mergify[bot] committed Jul 26, 2024
1 parent 016be29 commit d8da568
Show file tree
Hide file tree
Showing 22 changed files with 216 additions and 170 deletions.
7 changes: 5 additions & 2 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func (im IBCMiddleware) OnChanCloseConfirm(
// OnRecvPacket implements the IBCMiddleware interface
func (IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
_ string,
packet channeltypes.Packet,
_ sdk.AccAddress,
) ibcexported.Acknowledgement {
Expand All @@ -195,6 +196,7 @@ func (IBCMiddleware) OnRecvPacket(
// OnAcknowledgementPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
Expand All @@ -210,7 +212,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(

// call underlying app's OnAcknowledgementPacket callback.
if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, packet.GetSourcePort(), connectionID) {
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
}

return nil
Expand All @@ -219,6 +221,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
// OnTimeoutPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnTimeoutPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
Expand All @@ -236,7 +239,7 @@ func (im IBCMiddleware) OnTimeoutPacket(
}

if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, packet.GetSourcePort(), connectionID) {
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer)
}

return nil
Expand Down
14 changes: 7 additions & 7 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
)

ctx := suite.chainA.GetContext()
ack := cbs.OnRecvPacket(ctx, packet, nil)
ack := cbs.OnRecvPacket(ctx, path.EndpointA.GetChannel().Version, packet, nil)
suite.Require().Equal(tc.expPass, ack.Success())

expectedEvents := sdk.Events{
Expand Down Expand Up @@ -621,7 +621,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
{
"ICA auth module callback fails", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
) error {
return fmt.Errorf("mock ica auth fails")
}
Expand All @@ -637,7 +637,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)

suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
) error {
return fmt.Errorf("error should be unreachable")
}
Expand Down Expand Up @@ -682,7 +682,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, []byte("ack"), nil)
err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, []byte("ack"), nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -718,7 +718,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
{
"ICA auth module callback fails", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnTimeoutPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress,
) error {
return fmt.Errorf("mock ica auth fails")
}
Expand All @@ -734,7 +734,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)

suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnTimeoutPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress,
) error {
return fmt.Errorf("error should be unreachable")
}
Expand Down Expand Up @@ -779,7 +779,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)
err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
55 changes: 29 additions & 26 deletions modules/apps/27-interchain-accounts/host/ibc_module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ func NewIBCModule(k keeper.Keeper) IBCModule {

// OnChanOpenInit implements the IBCModule interface
func (IBCModule) OnChanOpenInit(
ctx sdk.Context,
order channeltypes.Order,
connectionHops []string,
portID string,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
_ sdk.Context,
_ channeltypes.Order,
_ []string,
_ string,
_ string,
_ *capabilitytypes.Capability,
_ channeltypes.Counterparty,
_ string,
) (string, error) {
return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
}
Expand All @@ -69,11 +69,11 @@ func (im IBCModule) OnChanOpenTry(

// OnChanOpenAck implements the IBCModule interface
func (IBCModule) OnChanOpenAck(
ctx sdk.Context,
portID,
channelID string,
counterpartyChannelID string,
counterpartyVersion string,
_ sdk.Context,
_,
_ string,
_ string,
_ string,
) error {
return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
}
Expand All @@ -93,9 +93,9 @@ func (im IBCModule) OnChanOpenConfirm(

// OnChanCloseInit implements the IBCModule interface
func (IBCModule) OnChanCloseInit(
ctx sdk.Context,
portID,
channelID string,
_ sdk.Context,
_ string,
_ string,
) error {
// Disallow user-initiated channel closing for interchain account channels
return errorsmod.Wrap(ibcerrors.ErrInvalidRequest, "user cannot close channel")
Expand All @@ -113,6 +113,7 @@ func (im IBCModule) OnChanCloseConfirm(
// OnRecvPacket implements the IBCModule interface
func (im IBCModule) OnRecvPacket(
ctx sdk.Context,
_ string,
packet channeltypes.Packet,
_ sdk.AccAddress,
) ibcexported.Acknowledgement {
Expand Down Expand Up @@ -140,25 +141,27 @@ func (im IBCModule) OnRecvPacket(

// OnAcknowledgementPacket implements the IBCModule interface
func (IBCModule) OnAcknowledgementPacket(
ctx sdk.Context,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
_ sdk.Context,
_ string,
_ channeltypes.Packet,
_ []byte,
_ sdk.AccAddress,
) error {
return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot receive acknowledgement on a host channel end, a host chain does not send a packet over the channel")
}

// OnTimeoutPacket implements the IBCModule interface
func (IBCModule) OnTimeoutPacket(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
_ sdk.Context,
_ string,
_ channeltypes.Packet,
_ sdk.AccAddress,
) error {
return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot cause a packet timeout on a host channel end, a host chain does not send a packet over the channel")
}

// OnChanUpgradeInit implements the IBCModule interface
func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
func (IBCModule) OnChanUpgradeInit(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) (string, error) {
return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel upgrade handshake must be initiated by controller chain")
}

Expand All @@ -172,12 +175,12 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
}

// OnChanUpgradeAck implements the IBCModule interface
func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
func (IBCModule) OnChanUpgradeAck(_ sdk.Context, _, _, _ string) error {
return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel upgrade handshake must be initiated by controller chain")
}

// OnChanUpgradeOpen implements the IBCModule interface
func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) {
func (IBCModule) OnChanUpgradeOpen(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) {
}

// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
Expand Down
8 changes: 4 additions & 4 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
{
"success with ICA auth module callback failure", func() {
suite.chainB.GetSimApp().ICAAuthModule.IBCApp.OnRecvPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress,
) exported.Acknowledgement {
return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed OnRecvPacket mock callback"))
}
Expand Down Expand Up @@ -503,7 +503,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
suite.Require().True(ok)

ctx := suite.chainB.GetContext()
ack := cbs.OnRecvPacket(ctx, packet, nil)
ack := cbs.OnRecvPacket(ctx, path.EndpointB.GetChannel().Version, packet, nil)

expectedAttributes := []sdk.Attribute{
sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName),
Expand Down Expand Up @@ -587,7 +587,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
0,
)

err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), packet, []byte("ackBytes"), nil)
err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), path.EndpointB.GetChannel().Version, packet, []byte("ackBytes"), nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -642,7 +642,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
0,
)

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)
err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
42 changes: 32 additions & 10 deletions modules/apps/29-fee/ibc_middleware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,16 @@ func (im IBCMiddleware) OnChanCloseConfirm(
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {
if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) {
return im.app.OnRecvPacket(ctx, packet, relayer)
return im.app.OnRecvPacket(ctx, channelVersion, packet, relayer)
}

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

// in case of async acknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement
if ack == nil {
Expand All @@ -241,14 +243,17 @@ func (im IBCMiddleware) OnRecvPacket(
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) {
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
}

appVersion := unwrapAppVersion(channelVersion)

var ack types.IncentivizedAcknowledgement
if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
return errorsmod.Wrapf(err, "cannot unmarshal ICS-29 incentivized packet acknowledgement: %v", ack)
Expand All @@ -263,14 +268,14 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
// for fee enabled channels
//
// Please see ADR 004 for more information.
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

packetID := channeltypes.NewPacketID(packet.SourcePort, packet.SourceChannel, packet.Sequence)
feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
if !found {
// call underlying callback
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel)
Expand All @@ -286,30 +291,37 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
im.keeper.DistributePacketFeesOnAcknowledgement(ctx, ack.ForwardRelayerAddress, payeeAddr, feesInEscrow.PacketFees, packetID)

// call underlying callback
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

// OnTimeoutPacket implements the IBCMiddleware interface
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnTimeoutPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) {
return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer)
}

appVersion := unwrapAppVersion(channelVersion)

// if the fee keeper is locked then fee logic should be skipped
// this may occur in the presence of a severe bug which leads to invalid state
// the fee keeper will be unlocked after manual intervention
//
// Please see ADR 004 for more information.
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) || im.keeper.IsLocked(ctx) {
return im.app.OnTimeoutPacket(ctx, packet, relayer)
if im.keeper.IsLocked(ctx) {
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

packetID := channeltypes.NewPacketID(packet.SourcePort, packet.SourceChannel, packet.Sequence)
feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
if !found {
// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel)
Expand All @@ -325,7 +337,7 @@ func (im IBCMiddleware) OnTimeoutPacket(
im.keeper.DistributePacketFeesOnTimeout(ctx, payeeAddr, feesInEscrow.PacketFees, packetID)

// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

// OnChanUpgradeInit implements the IBCModule interface
Expand Down Expand Up @@ -480,3 +492,13 @@ func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID, channelID s

return unmarshaler.UnmarshalPacketData(ctx, portID, channelID, bz)
}

func unwrapAppVersion(channelVersion string) string {
metadata, err := types.MetadataFromVersion(channelVersion)
if err != nil {
// This should not happen, as it would mean that the channel is broken. Only a severe bug would cause this.
panic(errorsmod.Wrap(err, "failed to unwrap app version from channel version"))
}

return metadata.AppVersion
}
Loading

0 comments on commit d8da568

Please sign in to comment.