feat(callbacks): adr8 implementation (backport #3939)

.github/CODEOWNERS

@@ -39,4 +39,8 @@
 /proto/ibc/applications/fee/ @AdityaSripal @charleenfei @colin-axner @damiannolan
 
 # CODEOWNERS for docs
-/docs/ @colin-axner @AdityaSripal @crodriguezvega @charleenfei @damiannolan @chatton @tmsdkeys
+/docs/ @colin-axner @AdityaSripal @crodriguezvega @charleenfei @damiannolan @chatton @DimitrisJim @srdtrk
+
+# CODEOWNERS for callbacks middleware
+
+/modules/apps/callbacks/ @colin-axner @AdityaSripal @damiannolan @srdtrk

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -12,9 +12,9 @@ import (
 	controllerkeeper "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/keeper"
 	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
 	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
-	fee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee"
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/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"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 )
@@ -839,7 +839,7 @@ func (suite *InterchainAccountsTestSuite) TestGetAppVersion() {
 	cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
 	suite.Require().True(ok)
 
-	controllerStack := cbs.(fee.IBCMiddleware)
+	controllerStack := cbs.(porttypes.Middleware)
 	appVersion, found := controllerStack.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 	suite.Require().True(found)
 	suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion)

modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go

@@ -7,7 +7,6 @@ import (
 
 	genesistypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/genesis/types"
 	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
-	ibcfeekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper"
 	channelkeeper "github.com/cosmos/ibc-go/v7/modules/core/04-channel/keeper"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
@@ -261,11 +260,9 @@ func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
 func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
 	suite.SetupTest()
 
-	// test if the ics4 wrapper is the fee keeper initially
+	// test if the ics4 wrapper is the channel keeper initially
 	ics4Wrapper := suite.chainA.GetSimApp().ICAControllerKeeper.GetICS4Wrapper()
 
-	_, isFeeKeeper := ics4Wrapper.(ibcfeekeeper.Keeper)
-	suite.Require().True(isFeeKeeper)
 	_, isChannelKeeper := ics4Wrapper.(channelkeeper.Keeper)
 	suite.Require().False(isChannelKeeper)
 
@@ -275,6 +272,4 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
 
 	_, isChannelKeeper = ics4Wrapper.(channelkeeper.Keeper)
 	suite.Require().True(isChannelKeeper)
-	_, isFeeKeeper = ics4Wrapper.(ibcfeekeeper.Keeper)
-	suite.Require().False(isFeeKeeper)
 }

modules/apps/29-fee/ibc_middleware_test.go

@@ -1067,7 +1067,7 @@ func (suite *FeeTestSuite) TestGetAppVersion() {
 			cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
 			suite.Require().True(ok)
 
-			feeModule := cbs.(fee.IBCMiddleware)
+			feeModule := cbs.(porttypes.Middleware)
 
 			appVersion, found := feeModule.GetAppVersion(suite.chainA.GetContext(), portID, channelID)
 

modules/apps/callbacks/callbacks_test.go

@@ -0,0 +1,301 @@
+package ibccallbacks_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+
+	sdkmath "cosmossdk.io/math"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+
+	icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
+	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+	feetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types"
+	"github.com/cosmos/ibc-go/v7/modules/apps/callbacks/types"
+	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
+	ibctesting "github.com/cosmos/ibc-go/v7/testing"
+	ibcmock "github.com/cosmos/ibc-go/v7/testing/mock"
+	simapp "github.com/cosmos/ibc-go/v7/testing/simapp"
+)
+
+const maxCallbackGas = uint64(1000000)
+
+// CallbacksTestSuite defines the needed instances and methods to test callbacks
+type CallbacksTestSuite struct {
+	suite.Suite
+
+	coordinator *ibctesting.Coordinator
+
+	chainA *ibctesting.TestChain
+	chainB *ibctesting.TestChain
+
+	path *ibctesting.Path
+}
+
+// setupChains sets up a coordinator with 2 test chains.
+func (s *CallbacksTestSuite) setupChains() {
+	s.coordinator = ibctesting.NewCoordinator(s.T(), 2)
+	s.chainA = s.coordinator.GetChain(ibctesting.GetChainID(1))
+	s.chainB = s.coordinator.GetChain(ibctesting.GetChainID(2))
+	s.path = ibctesting.NewPath(s.chainA, s.chainB)
+}
+
+// SetupTransferTest sets up a transfer channel between chainA and chainB
+func (s *CallbacksTestSuite) SetupTransferTest() {
+	s.setupChains()
+
+	s.path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort
+	s.path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort
+	s.path.EndpointA.ChannelConfig.Version = transfertypes.Version
+	s.path.EndpointB.ChannelConfig.Version = transfertypes.Version
+
+	s.coordinator.Setup(s.path)
+}
+
+// SetupFeeTransferTest sets up a fee middleware enabled transfer channel between chainA and chainB
+func (s *CallbacksTestSuite) SetupFeeTransferTest() {
+	s.setupChains()
+
+	feeTransferVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: transfertypes.Version}))
+	s.path.EndpointA.ChannelConfig.Version = feeTransferVersion
+	s.path.EndpointB.ChannelConfig.Version = feeTransferVersion
+	s.path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
+	s.path.EndpointB.ChannelConfig.PortID = transfertypes.PortID
+
+	s.coordinator.Setup(s.path)
+}
+
+func (s *CallbacksTestSuite) SetupMockFeeTest() {
+	s.setupChains()
+
+	mockFeeVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: ibcmock.Version}))
+	s.path.EndpointA.ChannelConfig.Version = mockFeeVersion
+	s.path.EndpointB.ChannelConfig.Version = mockFeeVersion
+	s.path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
+	s.path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
+}
+
+// SetupICATest sets up an interchain accounts channel between chainA (controller) and chainB (host).
+// It funds and returns the interchain account address owned by chainA's SenderAccount.
+func (s *CallbacksTestSuite) SetupICATest() string {
+	s.setupChains()
+	s.coordinator.SetupConnections(s.path)
+
+	icaOwner := s.chainA.SenderAccount.GetAddress().String()
+	// ICAVersion defines a interchain accounts version string
+	icaVersion := icatypes.NewDefaultMetadataString(s.path.EndpointA.ConnectionID, s.path.EndpointB.ConnectionID)
+	icaControllerPortID, err := icatypes.NewControllerPortID(icaOwner)
+	s.Require().NoError(err)
+
+	s.path.SetChannelOrdered()
+	s.path.EndpointA.ChannelConfig.PortID = icaControllerPortID
+	s.path.EndpointB.ChannelConfig.PortID = icatypes.HostPortID
+	s.path.EndpointA.ChannelConfig.Version = icaVersion
+	s.path.EndpointB.ChannelConfig.Version = icaVersion
+
+	s.RegisterInterchainAccount(icaOwner)
+	// open chan init must be skipped. So we cannot use .CreateChannels()
+	err = s.path.EndpointB.ChanOpenTry()
+	s.Require().NoError(err)
+	err = s.path.EndpointA.ChanOpenAck()
+	s.Require().NoError(err)
+	err = s.path.EndpointB.ChanOpenConfirm()
+	s.Require().NoError(err)
+
+	interchainAccountAddr, found := s.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(s.chainB.GetContext(), s.path.EndpointA.ConnectionID, s.path.EndpointA.ChannelConfig.PortID)
+	s.Require().True(found)
+
+	// fund the interchain account on chainB
+	msgBankSend := &banktypes.MsgSend{
+		FromAddress: s.chainB.SenderAccount.GetAddress().String(),
+		ToAddress:   interchainAccountAddr,
+		Amount:      sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100000))),
+	}
+	res, err := s.chainB.SendMsgs(msgBankSend)
+	s.Require().NotEmpty(res)
+	s.Require().NoError(err)
+
+	return interchainAccountAddr
+}
+
+// RegisterInterchainAccount submits a MsgRegisterInterchainAccount and updates the controller endpoint with the
+// channel created.
+func (s *CallbacksTestSuite) RegisterInterchainAccount(owner string) {
+	msgRegister := icacontrollertypes.NewMsgRegisterInterchainAccount(s.path.EndpointA.ConnectionID, owner, s.path.EndpointA.ChannelConfig.Version)
+
+	res, err := s.chainA.SendMsgs(msgRegister)
+	s.Require().NotEmpty(res)
+	s.Require().NoError(err)
+
+	channelID, err := ibctesting.ParseChannelIDFromEvents(res.GetEvents())
+	s.Require().NoError(err)
+
+	s.path.EndpointA.ChannelID = channelID
+}
+
+// AssertHasExecutedExpectedCallback checks the stateful entries and counters based on callbacktype.
+// It assumes that the source chain is chainA and the destination chain is chainB.
+func (s *CallbacksTestSuite) AssertHasExecutedExpectedCallback(callbackType types.CallbackType, expSuccess bool) {
+	var expStatefulEntries uint8
+	if expSuccess {
+		// if the callback is expected to be successful,
+		// we expect at least one state entry
+		expStatefulEntries = 1
+	}
+
+	sourceStatefulCounter := s.chainA.GetSimApp().MockContractKeeper.GetStateEntryCounter(s.chainA.GetContext())
+	destStatefulCounter := s.chainB.GetSimApp().MockContractKeeper.GetStateEntryCounter(s.chainB.GetContext())
+
+	switch callbackType {
+	case "none":
+		s.Require().Equal(uint8(0), sourceStatefulCounter)
+		s.Require().Equal(uint8(0), destStatefulCounter)
+
+	case types.CallbackTypeSendPacket:
+		s.Require().Equal(expStatefulEntries, sourceStatefulCounter, "unexpected stateful entry amount for source send packet callback")
+		s.Require().Equal(uint8(0), destStatefulCounter)
+
+	case types.CallbackTypeAcknowledgementPacket, types.CallbackTypeTimeoutPacket:
+		expStatefulEntries *= 2 // expect OnAcknowledgement/OnTimeout to be successful as well as the initial SendPacket
+		s.Require().Equal(expStatefulEntries, sourceStatefulCounter, "unexpected stateful entry amount for source acknowledgement/timeout callbacks")
+		s.Require().Equal(uint8(0), destStatefulCounter)
+
+	case types.CallbackTypeReceivePacket:
+		s.Require().Equal(uint8(0), sourceStatefulCounter)
+		s.Require().Equal(expStatefulEntries, destStatefulCounter)
+
+	default:
+		s.FailNow(fmt.Sprintf("invalid callback type %s", callbackType))
+	}
+
+	s.AssertCallbackCounters(callbackType)
+}
+
+func (s *CallbacksTestSuite) AssertCallbackCounters(callbackType types.CallbackType) {
+	sourceCounters := s.chainA.GetSimApp().MockContractKeeper.Counters
+	destCounters := s.chainB.GetSimApp().MockContractKeeper.Counters
+
+	switch callbackType {
+	case "none":
+		s.Require().Len(sourceCounters, 0)
+		s.Require().Len(destCounters, 0)
+
+	case types.CallbackTypeSendPacket:
+		s.Require().Len(sourceCounters, 1)
+		s.Require().Equal(1, sourceCounters[types.CallbackTypeSendPacket])
+
+	case types.CallbackTypeAcknowledgementPacket:
+		s.Require().Len(sourceCounters, 2)
+		s.Require().Equal(1, sourceCounters[types.CallbackTypeSendPacket])
+		s.Require().Equal(1, sourceCounters[types.CallbackTypeAcknowledgementPacket])
+
+		s.Require().Len(destCounters, 0)
+
+	case types.CallbackTypeReceivePacket:
+		s.Require().Len(sourceCounters, 0)
+		s.Require().Len(destCounters, 1)
+		s.Require().Equal(1, destCounters[types.CallbackTypeReceivePacket])
+
+	case types.CallbackTypeTimeoutPacket:
+		s.Require().Len(sourceCounters, 2)
+		s.Require().Equal(1, sourceCounters[types.CallbackTypeSendPacket])
+		s.Require().Equal(1, sourceCounters[types.CallbackTypeTimeoutPacket])
+
+		s.Require().Len(destCounters, 0)
+
+	default:
+		s.FailNow(fmt.Sprintf("invalid callback type %s", callbackType))
+	}
+}
+
+func TestIBCCallbacksTestSuite(t *testing.T) {
+	suite.Run(t, new(CallbacksTestSuite))
+}
+
+// AssertHasExecutedExpectedCallbackWithFee checks if only the expected type of callback has been executed
+// and that the expected ics-29 fee has been paid.
+func (s *CallbacksTestSuite) AssertHasExecutedExpectedCallbackWithFee(
+	callbackType types.CallbackType, isSuccessful bool, isTimeout bool,
+	originalSenderBalance sdk.Coins, fee feetypes.Fee,
+) {
+	// Recall that:
+	// - the source chain is chainA
+	// - forward relayer is chainB.SenderAccount
+	// - reverse relayer is chainA.SenderAccount
+	// - The counterparty payee of the forward relayer in chainA is chainB.SenderAccount (as a chainA account)
+
+	// We only check if the fee is paid if the callback is successful.
+	if !isTimeout && isSuccessful {
+		// check forward relay balance
+		s.Require().Equal(
+			fee.RecvFee,
+			sdk.NewCoins(s.chainA.GetSimApp().BankKeeper.GetBalance(s.chainA.GetContext(), s.chainB.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)),
+		)
+
+		s.Require().Equal(
+			fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded
+			sdk.NewCoins(
+				s.chainA.GetSimApp().BankKeeper.GetBalance(
+					s.chainA.GetContext(), s.chainA.SenderAccount.GetAddress(),
+					ibctesting.TestCoin.Denom),
+			).Sub(originalSenderBalance[0]),
+		)
+	} else if isSuccessful {
+		// forward relay balance should be 0
+		s.Require().Equal(
+			sdk.NewCoin(ibctesting.TestCoin.Denom, sdkmath.ZeroInt()),
+			s.chainA.GetSimApp().BankKeeper.GetBalance(s.chainA.GetContext(), s.chainB.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom),
+		)
+
+		// all fees should be returned as sender is the reverse relayer
+		s.Require().Equal(
+			fee.Total(),
+			sdk.NewCoins(
+				s.chainA.GetSimApp().BankKeeper.GetBalance(
+					s.chainA.GetContext(), s.chainA.SenderAccount.GetAddress(),
+					ibctesting.TestCoin.Denom),
+			).Sub(originalSenderBalance[0]),
+		)
+	}
+	s.AssertHasExecutedExpectedCallback(callbackType, isSuccessful)
+}
+
+// OverrideSendMsgWithAssertion overrides both chains' SendMsgsOverride function to assert whether the
+// transaction is successful or not.
+func OverrideSendMsgWithAssertion(chain *ibctesting.TestChain, expPass bool) {
+	chain.SendMsgsOverride = func(msgs ...sdk.Msg) (*sdk.Result, error) {
+		// ensure the chain has the latest time
+		chain.Coordinator.UpdateTimeForChain(chain)
+
+		_, r, err := simapp.SignAndDeliver(
+			chain.T,
+			chain.TxConfig,
+			chain.App.GetBaseApp(),
+			chain.GetContext().BlockHeader(),
+			msgs,
+			chain.ChainID,
+			[]uint64{chain.SenderAccount.GetAccountNumber()},
+			[]uint64{chain.SenderAccount.GetSequence()},
+			true, expPass, chain.SenderPrivKey,
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		// NextBlock calls app.Commit()
+		chain.NextBlock()
+
+		// increment sequence for successful transaction execution
+		err = chain.SenderAccount.SetSequence(chain.SenderAccount.GetSequence() + 1)
+		if err != nil {
+			return nil, err
+		}
+
+		chain.Coordinator.IncrementTime()
+
+		return r, nil
+	}
+}

modules/apps/callbacks/export_test.go

@@ -0,0 +1,25 @@
+package ibccallbacks
+
+/*
+	This file is to allow for unexported functions and fields to be accessible to the testing package.
+*/
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/v7/modules/apps/callbacks/types"
+	porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
+)
+
+// ProcessCallback is a wrapper around processCallback to allow the function to be directly called in tests.
+func (im IBCMiddleware) ProcessCallback(
+	ctx sdk.Context, callbackType types.CallbackType,
+	callbackData types.CallbackData, callbackExecutor func(sdk.Context) error,
+) error {
+	return im.processCallback(ctx, callbackType, callbackData, callbackExecutor)
+}
+
+// GetICS4Wrapper is a getter for the IBCMiddleware's ICS4Wrapper.
+func (im *IBCMiddleware) GetICS4Wrapper() porttypes.ICS4Wrapper {
+	return im.ics4Wrapper
+}