feat(hard_fork): delayed ack should create commitment after rolling b…

…ack acks (#1383)

app/keepers/keepers.go

@@ -617,7 +617,7 @@ func (a *AppKeepers) SetupHooks() {
 	a.RollappKeeper.SetHooks(rollappmoduletypes.NewMultiRollappHooks(
 		// insert rollapp hooks receivers here
 		a.SequencerKeeper.RollappHooks(),
-		a.delayedAckMiddleware,
+		a.DelayedAckKeeper,
 		a.StreamerKeeper.Hooks(),
 		a.DymNSKeeper.GetRollAppHooks(),
 		a.LightClientKeeper.RollappHooks(),

ibctesting/delayed_ack_test.go

@@ -7,7 +7,12 @@ import (
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
+	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	ibcmerkle "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
+	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
+	"github.com/cosmos/ibc-go/v7/testing/simapp"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -236,4 +241,109 @@ func (s *delayedAckSuite) TestHubToRollappTimeout() {
 	s.Require().Equal(preSendBalance.Amount, postFinalizeBalance.Amount)
 }
 
-// FIXME: test refunds due to hard fork + receipt deletion
+// TestHardFork tests the hard fork handling for outgoing packets from the hub to the rollapp.
+// we assert the packets commitments are restored and the pending packets are ackable after the hard fork.
+func (s *delayedAckSuite) TestHardFork_HubToRollapp() {
+	path := s.newTransferPath(s.hubChain(), s.rollappChain())
+	s.coordinator.Setup(path)
+
+	// Setup endpoints
+	var (
+		hubEndpoint     = path.EndpointA
+		hubIBCKeeper    = s.hubChain().App.GetIBCKeeper()
+		senderAccount   = s.hubChain().SenderAccount.GetAddress()
+		receiverAccount = s.rollappChain().SenderAccount.GetAddress()
+
+		amount, _     = sdk.NewIntFromString("1000000000000000000") // 1DYM
+		coinToSendToB = sdk.NewCoin(sdk.DefaultBondDenom, amount)
+		timeoutHeight = clienttypes.Height{RevisionNumber: 1, RevisionHeight: 50}
+	)
+
+	// Create rollapp and update its initial state
+	s.createRollappWithFinishedGenesis(path.EndpointA.ChannelID)
+	s.setRollappLightClientID(s.rollappCtx().ChainID(), path.EndpointA.ClientID)
+	s.registerSequencer()
+	s.updateRollappState(uint64(s.rollappCtx().BlockHeight()))
+
+	// send from hubChain to rollappChain
+	balanceBefore := s.hubApp().BankKeeper.GetBalance(s.hubCtx(), senderAccount, sdk.DefaultBondDenom)
+	msg := types.NewMsgTransfer(hubEndpoint.ChannelConfig.PortID, hubEndpoint.ChannelID, coinToSendToB, senderAccount.String(), receiverAccount.String(), timeoutHeight, disabledTimeoutTimestamp, "")
+	res, err := s.hubChain().SendMsgs(msg)
+	s.Require().NoError(err)
+	packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
+	s.Require().NoError(err)
+
+	// assert commitments are created
+	found := hubIBCKeeper.ChannelKeeper.HasPacketCommitment(s.hubCtx(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
+	s.Require().True(found)
+
+	// Update the client
+	err = hubEndpoint.UpdateClient()
+	s.Require().NoError(err)
+
+	err = path.RelayPacket(packet)
+	s.Require().NoError(err) // expecting error as no AcknowledgePacket expected to return
+
+	// progress the rollapp chain
+	s.coordinator.CommitNBlocks(s.rollappChain(), 110)
+
+	// Update the client
+	err = hubEndpoint.UpdateClient()
+	s.Require().NoError(err)
+
+	// write ack optimistically
+	err = path.EndpointA.AcknowledgePacket(packet, []byte{0x1})
+	s.Require().NoError(err)
+
+	// assert commitments are no longer available
+	found = hubIBCKeeper.ChannelKeeper.HasPacketCommitment(s.hubCtx(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
+	s.Require().False(found)
+
+	// timeout the packet, can't check for error (ErrNoOp). we assert the balance refund
+	err = path.EndpointA.TimeoutPacket(packet)
+	s.Require().NoError(err)
+	balanceAfter := s.hubApp().BankKeeper.GetBalance(s.hubCtx(), senderAccount, sdk.DefaultBondDenom)
+	s.Require().NotEqual(balanceBefore.String(), balanceAfter.String())
+
+	// hard fork
+	err = s.hubApp().DelayedAckKeeper.OnHardFork(s.hubCtx(), s.rollappCtx().ChainID(), 5)
+	s.Require().NoError(err)
+
+	// assert commitments are created again
+	found = hubIBCKeeper.ChannelKeeper.HasPacketCommitment(s.hubCtx(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
+	s.Require().True(found)
+
+	// Update the client
+	err = hubEndpoint.UpdateClient()
+	s.Require().NoError(err)
+
+	// timeout the packet. we expect for verification error
+	timeoutMsg := getTimeOutPacket(hubEndpoint, packet)
+	_, _, err = simapp.SignAndDeliver(
+		path.EndpointA.Chain.T,
+		path.EndpointA.Chain.TxConfig,
+		path.EndpointA.Chain.App.GetBaseApp(),
+		path.EndpointA.Chain.GetContext().BlockHeader(),
+		[]sdk.Msg{timeoutMsg},
+		path.EndpointA.Chain.ChainID,
+		[]uint64{path.EndpointA.Chain.SenderAccount.GetAccountNumber()},
+		[]uint64{path.EndpointA.Chain.SenderAccount.GetSequence()},
+		true, false, path.EndpointA.Chain.SenderPrivKey,
+	)
+	s.Require().ErrorIs(err, ibcmerkle.ErrInvalidProof)
+}
+
+func getTimeOutPacket(endpoint *ibctesting.Endpoint, packet channeltypes.Packet) *channeltypes.MsgTimeout {
+	packetKey := host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
+	counterparty := endpoint.Counterparty
+	proof, proofHeight := counterparty.QueryProof(packetKey)
+	nextSeqRecv, found := counterparty.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceRecv(counterparty.Chain.GetContext(), counterparty.ChannelConfig.PortID, counterparty.ChannelID)
+	require.True(endpoint.Chain.T, found)
+
+	timeoutMsg := channeltypes.NewMsgTimeout(
+		packet, nextSeqRecv,
+		proof, proofHeight, endpoint.Chain.SenderAccount.GetAddress().String(),
+	)
+
+	return timeoutMsg
+}

testutil/keeper/delayedack.go

@@ -28,6 +28,9 @@ import (
 
 type ChannelKeeperStub struct{}
 
+func (c ChannelKeeperStub) SetPacketCommitment(ctx sdk.Context, portID string, channelID string, sequence uint64, commitmentHash []byte) {
+}
+
 func (c ChannelKeeperStub) SetPacketReceipt(ctx sdk.Context, portID string, channelID string, sequence uint64) {
 }
 

x/delayedack/keeper/fraud.go

@@ -4,17 +4,20 @@ import (
 	"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
+	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 
 	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
+	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
 )
 
-func (k Keeper) HandleHardFork(ctx sdk.Context, rollappID string, height uint64, ibc porttypes.IBCModule) error {
+var _ rollapptypes.RollappHooks = &Keeper{}
+
+func (k Keeper) OnHardFork(ctx sdk.Context, rollappID string, fraudHeight uint64) error {
 	logger := ctx.Logger().With("module", "DelayedAckMiddleware")
 
 	// Get all the pending packets from fork height inclusive
-	rollappPendingPackets := k.ListRollappPackets(ctx, types.PendingByRollappIDFromHeight(rollappID, height))
+	rollappPendingPackets := k.ListRollappPackets(ctx, types.PendingByRollappIDFromHeight(rollappID, fraudHeight))
 
 	// Iterate over all the pending packets and revert them
 	for _, rollappPacket := range rollappPendingPackets {
@@ -26,14 +29,11 @@ func (k Keeper) HandleHardFork(ctx sdk.Context, rollappID string, height uint64,
 			"sequence", rollappPacket.Packet.Sequence,
 		}
 
-		// refund all pending outgoing packets
 		if rollappPacket.Type == commontypes.RollappPacket_ON_ACK || rollappPacket.Type == commontypes.RollappPacket_ON_TIMEOUT {
-			// FIXME: #1380 create packet commitments instead
-			// we don't have access directly to `refundPacketToken` function, so we'll use the `OnTimeoutPacket` function
-			err := ibc.OnTimeoutPacket(ctx, *rollappPacket.Packet, rollappPacket.Relayer)
-			if err != nil {
-				logger.Error("failed to refund reverted packet", append(logContext, "error", err.Error())...)
-			}
+			// for sent packets, we restore the packet commitment
+			// the packet will be handled over the new rollapp revision
+			commitment := channeltypes.CommitPacket(k.cdc, rollappPacket.Packet)
+			k.channelKeeper.SetPacketCommitment(ctx, rollappPacket.Packet.SourcePort, rollappPacket.Packet.SourceChannel, rollappPacket.Packet.Sequence, commitment)
 		} else {
 			// for incoming packets, we need to reset the packet receipt
 			ibcPacket := rollappPacket.Packet

x/delayedack/keeper/fraud_test.go

@@ -1,22 +1,14 @@
 package keeper_test
 
 import (
-	ibctransfer "github.com/cosmos/ibc-go/v7/modules/apps/transfer"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 
 	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
-	damodule "github.com/dymensionxyz/dymension/v3/x/delayedack"
 	"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
 )
 
 func (suite *DelayedAckTestSuite) TestHandleFraud() {
 	keeper, ctx := suite.App.DelayedAckKeeper, suite.Ctx
-	transferStack := damodule.NewIBCMiddleware(
-		damodule.WithIBCModule(ibctransfer.NewIBCModule(suite.App.TransferKeeper)),
-		damodule.WithKeeper(keeper),
-		damodule.WithRollappKeeper(suite.App.RollappKeeper),
-	)
-
 	rollappId := "testRollappId"
 	pkts := generatePackets(rollappId, 10)
 	rollappId2 := "testRollappId2"
@@ -40,7 +32,7 @@ func (suite *DelayedAckTestSuite) TestHandleFraud() {
 	suite.Require().Nil(err)
 
 	// call fraud on the 4 packet
-	err = keeper.HandleHardFork(ctx, rollappId, 4, transferStack)
+	err = keeper.OnHardFork(ctx, rollappId, 4)
 	suite.Require().Nil(err)
 
 	// expected result:
@@ -59,8 +51,6 @@ func (suite *DelayedAckTestSuite) TestHandleFraud() {
 	suite.Require().Equal(9, len(keeper.ListRollappPackets(ctx, prefixPending2)))
 }
 
-// TODO: test refunds of pending packets
-
 /* ---------------------------------- utils --------------------------------- */
 
 func generatePackets(rollappId string, num uint64) []commontypes.RollappPacket {

x/delayedack/keeper/invariants_test.go

@@ -2,25 +2,17 @@ package keeper_test
 
 import (
 	"github.com/cometbft/cometbft/libs/rand"
-	ibctransfer "github.com/cosmos/ibc-go/v7/modules/apps/transfer"
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 
 	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
-	damodule "github.com/dymensionxyz/dymension/v3/x/delayedack"
 	dakeeper "github.com/dymensionxyz/dymension/v3/x/delayedack/keeper"
 	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
 )
 
 func (suite *DelayedAckTestSuite) TestInvariants() {
 	suite.T().Skip("skipping TestInvariants as it's not supported with lazy finalization feature")
 
-	transferStack := damodule.NewIBCMiddleware(
-		damodule.WithIBCModule(ibctransfer.NewIBCModule(suite.App.TransferKeeper)),
-		damodule.WithKeeper(suite.App.DelayedAckKeeper),
-		damodule.WithRollappKeeper(suite.App.RollappKeeper),
-	)
-
 	initialHeight := int64(10)
 	suite.Ctx = suite.Ctx.WithBlockHeight(initialHeight)
 
@@ -82,7 +74,7 @@ func (suite *DelayedAckTestSuite) TestInvariants() {
 
 	// test fraud
 	for rollapp := range seqPerRollapp {
-		err := suite.App.DelayedAckKeeper.HandleHardFork(suite.Ctx, rollapp, uint64(suite.Ctx.BlockHeight()), transferStack)
+		err := suite.App.DelayedAckKeeper.OnHardFork(suite.Ctx, rollapp, uint64(suite.Ctx.BlockHeight()))
 		suite.Require().NoError(err)
 		break
 	}

x/delayedack/keeper/keeper.go

@@ -12,9 +12,12 @@ import (
 	porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
 
 	"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
+	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
 )
 
 type Keeper struct {
+	rollapptypes.StubRollappCreatedHooks
+
 	cdc                   codec.BinaryCodec
 	storeKey              storetypes.StoreKey
 	channelKeeperStoreKey storetypes.StoreKey // we need direct access to the IBC channel store

x/delayedack/keeper/rollapp_packet.go

@@ -174,6 +174,7 @@ func (k Keeper) DeleteRollappPacket(ctx sdk.Context, rollappPacket *commontypes.
 	store.Delete(rollappPacketKey)
 
 	keeperHooks := k.GetHooks()
+	// TODO: can call eIBC directly. shouldn't return error anyway
 	err := keeperHooks.AfterPacketDeleted(ctx, rollappPacket)
 	if err != nil {
 		return err

x/delayedack/types/expected_keepers.go

@@ -14,6 +14,7 @@ import (
 type ChannelKeeper interface {
 	LookupModuleByChannel(ctx sdk.Context, portID, channelID string) (string, *capabilitytypes.Capability, error)
 	SetPacketReceipt(ctx sdk.Context, portID, channelID string, sequence uint64)
+	SetPacketCommitment(ctx sdk.Context, portID, channelID string, sequence uint64, commitmentHash []byte)
 }
 
 type RollappKeeper interface {