fix(delayedack): wrong delayedack channels for onTimeout and OnAck (#815

)

Co-authored-by: danwt <30197399+danwt@users.noreply.github.com>

x/common/types/key_rollapp_packet.go

@@ -22,15 +22,30 @@ var (
 	keySeparatorBytes = []byte("/")
 )
 
-// RollappPacketKey constructs a key for a specific RollappPacket
-// status/rollappID/proofHeight/packetUID
+// RollappPacketKey constructs a key for a specific RollappPacket of the form:
+// status/rollappID/proofHeight/packetType/packetSourceChannel/packetSequence.
+//
+// In order to build a packet UID We need to take the source channel + packet + packet type as the packet UID
+// otherwise we're not guaranteed with uniqueness as we could have:
+// Same rollapp id, same status, same proof height same sequence (as it refers to the source chain) and same channel.
+// Example would be, both rollapp and hub have channel-0 and we have at the same proof height of the rollapp
+// AckPacket with sequence 1 (originated on the hub) and OnRecvPacket with sequence 1 (originated on the rollapp).
+// Adding the packet type guarantees uniqueness as the type differentiate the source.
 func RollappPacketKey(rollappPacket *RollappPacket) []byte {
+	// Get the bytes rep
 	srppPrefix := RollappPacketByStatusByRollappIDByProofHeightPrefix(rollappPacket.RollappId, rollappPacket.Status, rollappPacket.ProofHeight)
+	packetTypeBytes := []byte(rollappPacket.Type.String())
 	packetSequenceBytes := sdk.Uint64ToBigEndian(rollappPacket.Packet.Sequence)
-	packetDestinationChannelBytes := []byte(rollappPacket.Packet.DestinationChannel)
-	packetUIDBytes := append(packetDestinationChannelBytes, packetSequenceBytes...)
+	packetSourceChannelBytes := []byte(rollappPacket.Packet.SourceChannel)
+	// Construct the key
 	result := append(srppPrefix, keySeparatorBytes...)
-	return append(result, packetUIDBytes...)
+	result = append(result, packetTypeBytes...)
+	result = append(result, keySeparatorBytes...)
+	result = append(result, packetSourceChannelBytes...)
+	result = append(result, keySeparatorBytes...)
+	result = append(result, packetSequenceBytes...)
+
+	return result
 }
 
 // RollappPacketByStatusByRollappIDByProofHeightPrefix constructs a key prefix for a specific RollappPacket

x/common/types/packet_uid.go

@@ -0,0 +1,26 @@
+package types
+
+import fmt "fmt"
+
+// PacketUID is a unique identifier for an Rollapp IBC packet on the hub
+type PacketUID struct {
+	Type              RollappPacket_Type
+	RollappHubPort    string
+	RollappHubChannel string
+	Sequence          uint64
+}
+
+// NewPacketUID creates a new PacketUID with the provided details.
+func NewPacketUID(packetType RollappPacket_Type, hubPort string, hubChannel string, sequence uint64) PacketUID {
+	return PacketUID{
+		Type:              packetType,
+		RollappHubPort:    hubPort,
+		RollappHubChannel: hubChannel,
+		Sequence:          sequence,
+	}
+}
+
+// String returns a string representation of the PacketUID
+func (p PacketUID) String() string {
+	return fmt.Sprintf("%s-%s-%s-%d", p.Type, p.RollappHubChannel, p.RollappHubPort, p.Sequence)
+}

x/delayedack/eibc.go

@@ -55,10 +55,10 @@ func (im IBCMiddleware) eIBCDemandOrderHandler(ctx sdk.Context, rollappPacket co
 		}
 		// Calculate the fee by multiplying the fee by the price
 		var feeMultiplier sdk.Dec
-		if t == commontypes.RollappPacket_ON_TIMEOUT {
+		switch t {
+		case commontypes.RollappPacket_ON_TIMEOUT:
 			feeMultiplier = im.keeper.TimeoutFee(ctx)
-		}
-		if t == commontypes.RollappPacket_ON_ACK {
+		case commontypes.RollappPacket_ON_ACK:
 			feeMultiplier = im.keeper.ErrAckFee(ctx)
 		}
 
@@ -147,7 +147,8 @@ func (im IBCMiddleware) createDemandOrderFromIBCPacket(fungibleTokenPacketData t
 	case commontypes.RollappPacket_ON_TIMEOUT:
 		fallthrough
 	case commontypes.RollappPacket_ON_ACK:
-		demandOrderDenom = fungibleTokenPacketData.Denom      // It's what we tried to send
+		trace := transfertypes.ParseDenomTrace(fungibleTokenPacketData.Denom)
+		demandOrderDenom = trace.IBCDenom()
 		demandOrderRecipient = fungibleTokenPacketData.Sender // and who tried to send it (refund because it failed)
 	case commontypes.RollappPacket_ON_RECV:
 		demandOrderDenom = im.getEIBCTransferDenom(*rollappPacket.Packet, fungibleTokenPacketData)

x/delayedack/ibc_middleware.go

@@ -46,10 +46,11 @@ func (im IBCMiddleware) OnRecvPacket(
 	if !im.keeper.IsRollappsEnabled(ctx) {
 		return im.IBCModule.OnRecvPacket(ctx, packet, relayer)
 	}
-
 	logger := ctx.Logger().With("module", "DelayedAckMiddleware")
 
-	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet)
+	rollappPortOnHub, rollappChannelOnHub := packet.DestinationPort, packet.DestinationChannel
+
+	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to extract rollapp id from packet", "err", err)
 		return channeltypes.NewErrorAcknowledgement(err)
@@ -60,13 +61,13 @@ func (im IBCMiddleware) OnRecvPacket(
 		return im.IBCModule.OnRecvPacket(ctx, packet, relayer)
 	}
 
-	err = im.keeper.ValidateRollappId(ctx, rollappID, packet.GetDestPort(), packet.GetDestChannel())
+	err = im.keeper.ValidateRollappId(ctx, rollappID, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to validate rollappID", "rollappID", rollappID, "err", err)
 		return channeltypes.NewErrorAcknowledgement(err)
 	}
 
-	proofHeight, err := im.GetProofHeight(ctx, packet)
+	proofHeight, err := im.GetProofHeight(ctx, commontypes.RollappPacket_ON_RECV, rollappPortOnHub, rollappChannelOnHub, packet.Sequence)
 	if err != nil {
 		logger.Error("Failed to get proof height from packet", "err", err)
 		return channeltypes.NewErrorAcknowledgement(err)
@@ -116,13 +117,15 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 	}
 	logger := ctx.Logger().With("module", "DelayedAckMiddleware")
 
+	rollappPortOnHub, rollappChannelOnHub := packet.SourcePort, packet.SourceChannel
+
 	var ack channeltypes.Acknowledgement
 	if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
 		logger.Error("Unmarshal acknowledgement", "err", err)
 		return errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "unmarshal ICS-20 transfer packet acknowledgement: %v", err)
 	}
 
-	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet)
+	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to extract rollapp id from channel", "err", err)
 		return err
@@ -132,13 +135,13 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 		logger.Debug("Skipping IBC transfer OnAcknowledgementPacket for non-rollapp chain")
 		return im.IBCModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
 	}
-	err = im.keeper.ValidateRollappId(ctx, rollappID, packet.GetDestPort(), packet.GetDestChannel())
+	err = im.keeper.ValidateRollappId(ctx, rollappID, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to validate rollappID", "rollappID", rollappID, "err", err)
 		return err
 	}
 
-	proofHeight, err := im.GetProofHeight(ctx, packet)
+	proofHeight, err := im.GetProofHeight(ctx, commontypes.RollappPacket_ON_ACK, rollappPortOnHub, rollappChannelOnHub, packet.Sequence)
 	if err != nil {
 		logger.Error("Failed to get proof height from packet", "err", err)
 		return err
@@ -200,7 +203,9 @@ func (im IBCMiddleware) OnTimeoutPacket(
 	}
 	logger := ctx.Logger().With("module", "DelayedAckMiddleware")
 
-	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet)
+	rollappPortOnHub, rollappChannelOnHub := packet.SourcePort, packet.SourceChannel
+
+	rollappID, transferPacketData, err := im.ExtractRollappIDAndTransferPacket(ctx, packet, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to extract rollapp id from channel", "err", err)
 		return err
@@ -211,13 +216,13 @@ func (im IBCMiddleware) OnTimeoutPacket(
 		return im.IBCModule.OnTimeoutPacket(ctx, packet, relayer)
 	}
 
-	err = im.keeper.ValidateRollappId(ctx, rollappID, packet.DestinationPort, packet.DestinationChannel)
+	err = im.keeper.ValidateRollappId(ctx, rollappID, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		logger.Error("Failed to validate rollappID", "rollappID", rollappID, "err", err)
 		return err
 	}
 
-	proofHeight, err := im.GetProofHeight(ctx, packet)
+	proofHeight, err := im.GetProofHeight(ctx, commontypes.RollappPacket_ON_TIMEOUT, rollappPortOnHub, rollappChannelOnHub, packet.Sequence)
 	if err != nil {
 		logger.Error("Failed to get proof height from packet", "err", err)
 		return err
@@ -294,14 +299,14 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string)
 }
 
 // ExtractRollappIDAndTransferPacket extracts the rollapp ID from the packet
-func (im IBCMiddleware) ExtractRollappIDAndTransferPacket(ctx sdk.Context, packet channeltypes.Packet) (string, *transfertypes.FungibleTokenPacketData, error) {
+func (im IBCMiddleware) ExtractRollappIDAndTransferPacket(ctx sdk.Context, packet channeltypes.Packet, rollappPortOnHub string, rollappChannelOnHub string) (string, *transfertypes.FungibleTokenPacketData, error) {
 	// no-op if the packet is not a fungible token packet
 	var data transfertypes.FungibleTokenPacketData
 	if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
 		return "", nil, err
 	}
 	// Check if the packet is destined for a rollapp
-	chainID, err := im.keeper.ExtractChainIDFromChannel(ctx, packet.DestinationPort, packet.DestinationChannel)
+	chainID, err := im.keeper.ExtractChainIDFromChannel(ctx, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		return "", &data, err
 	}
@@ -313,19 +318,21 @@ func (im IBCMiddleware) ExtractRollappIDAndTransferPacket(ctx sdk.Context, packe
 		return "", &data, errorsmod.Wrapf(rollapptypes.ErrGenesisEventNotTriggered, "empty channel id: rollap id: %s", chainID)
 	}
 	// check if the channelID matches the rollappID's channelID
-	if rollapp.ChannelId != packet.GetDestChannel() {
+	if rollapp.ChannelId != rollappChannelOnHub {
 		return "", &data, errorsmod.Wrapf(
 			rollapptypes.ErrMismatchedChannelID,
-			"channel id mismatch: expect: %s: got: %s", rollapp.ChannelId, packet.GetDestChannel(),
+			"channel id mismatch: expect: %s: got: %s", rollapp.ChannelId, rollappChannelOnHub,
 		)
 	}
 
 	return chainID, &data, nil
 }
 
 // GetProofHeight returns the proof height of the packet
-func (im IBCMiddleware) GetProofHeight(ctx sdk.Context, packet channeltypes.Packet) (uint64, error) {
-	packetId := channeltypes.NewPacketID(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
+func (im IBCMiddleware) GetProofHeight(ctx sdk.Context, packetType commontypes.RollappPacket_Type,
+	rollappPortOnHub string, rollappChannelOnHub string, sequence uint64,
+) (uint64, error) {
+	packetId := commontypes.NewPacketUID(packetType, rollappPortOnHub, rollappChannelOnHub, sequence)
 	height, ok := types.FromIBCProofContext(ctx, packetId)
 	if ok {
 		return height.RevisionHeight, nil

x/delayedack/keeper/fraud.go

@@ -35,7 +35,6 @@ func (k Keeper) HandleFraud(ctx sdk.Context, rollappID string, ibc porttypes.IBC
 				logger.Error("failed to refund reverted packet", append(logContext, "error", err.Error())...)
 			}
 		}
-
 		// Update status to reverted
 		_, err := k.UpdateRollappPacketWithStatus(ctx, rollappPacket, commontypes.Status_REVERTED)
 		if err != nil {

x/delayedack/keeper/keeper.go

@@ -187,7 +187,7 @@ func (k *Keeper) LookupModuleByChannel(ctx sdk.Context, portID, channelID string
 }
 
 // ValidateRollappId checks that the rollapp id from the ibc connection matches the rollapp, checking the sequencer registered with the consensus state validator set
-func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollappID, portID, channelID string) error {
+func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollappID, rollappPortOnHub string, rollappChannelOnHub string) error {
 	// Get the sequencer from the latest state info update and check the validator set hash
 	// from the headers match with the sequencer for the rollappID
 	// As the assumption the sequencer is honest we don't check the packet proof height.
@@ -203,7 +203,7 @@ func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollappID, portID, channelID
 	// TODO (srene): We compare the validator set of the last consensus height, because it fails to  get consensus for a different height,
 	// but we should compare the validator set at the height of the last state info, because sequencer may have changed after that.
 	// If the sequencer is changed, then the validation will fail till the new sequencer sends a new state info update.
-	tmConsensusState, err := k.getTmConsensusState(ctx, portID, channelID)
+	tmConsensusState, err := k.getTmConsensusState(ctx, rollappPortOnHub, rollappChannelOnHub)
 	if err != nil {
 		k.Logger(ctx).Error("error consensus state", err)
 		return err

x/delayedack/keeper/rollapp_packet.go

@@ -12,7 +12,7 @@ import (
 // The key for the packet is generated using the rollappID, proofHeight and the packet itself.
 func (k Keeper) SetRollappPacket(ctx sdk.Context, rollappPacket commontypes.RollappPacket) error {
 	logger := ctx.Logger()
-	logger.Debug("Saving rollapp packet", "rollappID", rollappPacket.RollappId, "channel", rollappPacket.Packet.DestinationChannel,
+	logger.Debug("Saving rollapp packet", "rollappID", rollappPacket.RollappId, "src channel", rollappPacket.Packet.SourceChannel,
 		"sequence", rollappPacket.Packet.Sequence, "proofHeight", rollappPacket.ProofHeight, "type", rollappPacket.Type)
 	store := ctx.KVStore(k.storeKey)
 	rollappPacketKey := commontypes.RollappPacketKey(&rollappPacket)
@@ -73,6 +73,7 @@ func (k Keeper) UpdateRollappPacketTransferAddress(
 	case commontypes.RollappPacket_ON_ACK:
 		sender = address
 	}
+	// Create a new packet data with the updated recipient and sender
 	newPacketData := transfertypes.NewFungibleTokenPacketData(
 		transferPacketData.Denom,
 		transferPacketData.Amount,

x/delayedack/proof_height_ante.go

@@ -5,6 +5,7 @@ import (
 
 	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
+	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
 	delayedacktypes "github.com/dymensionxyz/dymension/v3/x/delayedack/types"
 )
 
@@ -18,26 +19,40 @@ func (rrd IBCProofHeightDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
 	for _, m := range tx.GetMsgs() {
 		var (
 			height   clienttypes.Height
-			packetId channeltypes.PacketId
+			packetId commontypes.PacketUID
 		)
 		switch msg := m.(type) {
 		case *channeltypes.MsgRecvPacket:
 			height = msg.ProofHeight
-			packetId = channeltypes.NewPacketID(msg.Packet.GetDestPort(), msg.Packet.GetDestChannel(), msg.Packet.GetSequence())
+			packetId = commontypes.NewPacketUID(
+				commontypes.RollappPacket_ON_RECV,
+				msg.Packet.DestinationPort,
+				msg.Packet.DestinationChannel,
+				msg.Packet.Sequence,
+			)
 
 		case *channeltypes.MsgAcknowledgement:
 			height = msg.ProofHeight
-			packetId = channeltypes.NewPacketID(msg.Packet.GetDestPort(), msg.Packet.GetDestChannel(), msg.Packet.GetSequence())
+			packetId = commontypes.NewPacketUID(
+				commontypes.RollappPacket_ON_ACK,
+				msg.Packet.SourcePort,
+				msg.Packet.SourceChannel,
+				msg.Packet.Sequence,
+			)
 
 		case *channeltypes.MsgTimeout:
 			height = msg.ProofHeight
-			packetId = channeltypes.NewPacketID(msg.Packet.GetDestPort(), msg.Packet.GetDestChannel(), msg.Packet.GetSequence())
+			packetId = commontypes.NewPacketUID(
+				commontypes.RollappPacket_ON_TIMEOUT,
+				msg.Packet.SourcePort,
+				msg.Packet.SourceChannel,
+				msg.Packet.Sequence,
+			)
 		default:
 			continue
 		}
 
 		ctx = delayedacktypes.NewIBCProofContext(ctx, packetId, height)
 	}
-
 	return next(ctx, tx, simulate)
 }