Merge branch 'main' into deps/bump-comet

modules/core/04-channel/keeper/export_test.go

@@ -29,3 +29,8 @@ func (k Keeper) CheckForUpgradeCompatibility(ctx sdk.Context, upgradeFields, cou
 func (k Keeper) SetUpgradeErrorReceipt(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt) {
 	k.setUpgradeErrorReceipt(ctx, portID, channelID, errorReceipt)
 }
+
+// SetRecvStartSequence is a wrapper around setRecvStartSequence to allow the function to be directly called in tests.
+func (k Keeper) SetRecvStartSequence(ctx sdk.Context, portID, channelID string, sequence uint64) {
+	k.setRecvStartSequence(ctx, portID, channelID, sequence)
+}

modules/core/04-channel/keeper/keeper.go

@@ -623,17 +623,20 @@ func (k Keeper) HasInflightPackets(ctx sdk.Context, portID, channelID string) bo
 	return iterator.Valid()
 }
 
-// SetPruningSequenceEnd sets the channel's pruning sequence end to the store.
-func (k Keeper) SetPruningSequenceEnd(ctx sdk.Context, portID, channelID string, sequence uint64) {
+// setRecvStartSequence sets the channel's recv start sequence to the store.
+func (k Keeper) setRecvStartSequence(ctx sdk.Context, portID, channelID string, sequence uint64) {
 	store := ctx.KVStore(k.storeKey)
 	bz := sdk.Uint64ToBigEndian(sequence)
-	store.Set(host.PruningSequenceEndKey(portID, channelID), bz)
+	store.Set(host.RecvStartSequenceKey(portID, channelID), bz)
 }
 
-// GetPruningSequenceEnd gets a channel's pruning sequence end from the store.
-func (k Keeper) GetPruningSequenceEnd(ctx sdk.Context, portID, channelID string) (uint64, bool) {
+// GetRecvStartSequence gets a channel's recv start sequence from the store.
+// The recv start sequence will be set to the counterparty's next sequence send
+// upon a successful channel upgrade. It will be used for replay protection of
+// historical packets and as the upper bound for pruning stale packet receives.
+func (k Keeper) GetRecvStartSequence(ctx sdk.Context, portID, channelID string) (uint64, bool) {
 	store := ctx.KVStore(k.storeKey)
-	bz := store.Get(host.PruningSequenceEndKey(portID, channelID))
+	bz := store.Get(host.RecvStartSequenceKey(portID, channelID))
 	if len(bz) == 0 {
 		return 0, false
 	}
@@ -675,9 +678,9 @@ func (k Keeper) PruneAcknowledgements(ctx sdk.Context, portID, channelID string,
 	if !found {
 		return 0, 0, errorsmod.Wrapf(types.ErrPruningSequenceStartNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
 	}
-	pruningSequenceEnd, found := k.GetPruningSequenceEnd(ctx, portID, channelID)
+	pruningSequenceEnd, found := k.GetRecvStartSequence(ctx, portID, channelID)
 	if !found {
-		return 0, 0, errorsmod.Wrapf(types.ErrPruningSequenceEndNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
+		return 0, 0, errorsmod.Wrapf(types.ErrRecvStartSequenceNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
 	}
 
 	start := pruningSequenceStart

modules/core/04-channel/keeper/keeper_test.go

@@ -580,7 +580,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
 				// Assert that PruneSequenceStart and PruneSequenceEnd are both set to 1.
 				start, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPruningSequenceStart(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
-				end, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPruningSequenceEnd(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				end, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetRecvStartSequence(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
 
 				suite.Require().Equal(uint64(1), start)
@@ -600,7 +600,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
 			},
 			func() {},
 			func(pruned, left uint64) {
-				sequenceEnd, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPruningSequenceEnd(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				sequenceEnd, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetRecvStartSequence(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
 
 				// We expect nothing to be left and sequenceStart == sequenceEnd.
@@ -672,7 +672,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
 				limit = 15
 			},
 			func(pruned, left uint64) {
-				sequenceEnd, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPruningSequenceEnd(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				sequenceEnd, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetRecvStartSequence(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
 
 				// We expect nothing to be left and sequenceStart == sequenceEnd.
@@ -825,10 +825,10 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
 			func() {},
 			func() {
 				store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(exported.StoreKey))
-				store.Delete(host.PruningSequenceEndKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
+				store.Delete(host.RecvStartSequenceKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
 			},
 			func(_, _ uint64) {},
-			types.ErrPruningSequenceEndNotFound,
+			types.ErrRecvStartSequenceNotFound,
 		},
 	}
 

modules/core/04-channel/keeper/packet.go

@@ -118,8 +118,8 @@ func (k Keeper) RecvPacket(
 		return errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestChannel())
 	}
 
-	if !slices.Contains([]types.State{types.OPEN, types.FLUSHING}, channel.State) {
-		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of [%s, %s], but got %s", types.OPEN, types.FLUSHING, channel.State)
+	if !slices.Contains([]types.State{types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE}, channel.State) {
+		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of [%s, %s, %s], but got %s", types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE, channel.State)
 	}
 
 	// If counterpartyUpgrade is stored we need to ensure that the
@@ -197,9 +197,20 @@ func (k Keeper) RecvPacket(
 		return errorsmod.Wrap(err, "couldn't verify counterparty packet commitment")
 	}
 
+	// REPLAY PROTECTION: The recvStartSequence will prevent historical proofs from allowing replay
+	// attacks on packets processed in previous lifecycles of a channel. After a successful channel
+	// upgrade all packets under the recvStartSequence will have been processed and thus should be
+	// rejected.
+	recvStartSequence, _ := k.GetRecvStartSequence(ctx, packet.GetDestPort(), packet.GetDestChannel())
+	if packet.GetSequence() < recvStartSequence {
+		return errorsmod.Wrap(types.ErrPacketReceived, "packet already processed in previous channel upgrade")
+	}
+
 	switch channel.Ordering {
 	case types.UNORDERED:
-		// check if the packet receipt has been received already for unordered channels
+		// REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received
+		// on unordered channels. Packet receipts must not be pruned, unless it has been marked stale
+		// by the increase of the recvStartSequence.
 		_, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 		if found {
 			emitRecvPacketEvent(ctx, packet, channel)
@@ -212,7 +223,7 @@ func (k Keeper) RecvPacket(
 		// All verification complete, update state
 		// For unordered channels we must set the receipt so it can be verified on the other side.
 		// This receipt does not contain any data, since the packet has not yet been processed,
-		// it's just a single store key set to an empty string to indicate that the packet has been received
+		// it's just a single store key set to a single byte to indicate that the packet has been received
 		k.SetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 
 	case types.ORDERED:
@@ -233,6 +244,8 @@ func (k Keeper) RecvPacket(
 			return types.ErrNoOpMsg
 		}
 
+		// REPLAY PROTECTION: Ordered channels require packets to be received in a strict order.
+		// Any out of order or previously received packets are rejected.
 		if packet.GetSequence() != nextSequenceRecv {
 			return errorsmod.Wrapf(
 				types.ErrPacketSequenceOutOfOrder,
@@ -287,11 +300,8 @@ func (k Keeper) WriteAcknowledgement(
 		return errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestChannel())
 	}
 
-	if !channel.IsOpen() {
-		return errorsmod.Wrapf(
-			types.ErrInvalidChannelState,
-			"channel state is not OPEN (got %s)", channel.State.String(),
-		)
+	if !slices.Contains([]types.State{types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE}, channel.State) {
+		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected one of [%s, %s, %s], got %s", types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE, channel.State)
 	}
 
 	// Authenticate capability to ensure caller has authority to receive packet on this channel

modules/core/04-channel/keeper/packet_test.go

@@ -323,6 +323,38 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			},
 			nil,
 		},
+		{
+			"success UNORDERED channel in FLUSHING",
+			func() {
+				// setup uses an UNORDERED channel
+				suite.coordinator.Setup(path)
+				channel := path.EndpointB.GetChannel()
+				channel.State = types.FLUSHING
+				path.EndpointB.SetChannel(channel)
+
+				sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
+				suite.Require().NoError(err)
+				packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
+				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			},
+			nil,
+		},
+		{
+			"success UNORDERED channel in FLUSHCOMPLETE",
+			func() {
+				// setup uses an UNORDERED channel
+				suite.coordinator.Setup(path)
+				channel := path.EndpointB.GetChannel()
+				channel.State = types.FLUSHCOMPLETE
+				path.EndpointB.SetChannel(channel)
+
+				sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
+				suite.Require().NoError(err)
+				packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
+				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			},
+			nil,
+		},
 		{
 			"success with out of order packet: UNORDERED channel",
 			func() {
@@ -395,21 +427,6 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			},
 			types.ErrInvalidPacket,
 		},
-		{
-			"failure while upgrading channel, channel in flush complete state",
-			func() {
-				suite.coordinator.Setup(path)
-				sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
-				suite.Require().NoError(err)
-				packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
-				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
-
-				channel := path.EndpointB.GetChannel()
-				channel.State = types.FLUSHCOMPLETE
-				path.EndpointB.SetChannel(channel)
-			},
-			types.ErrInvalidChannelState,
-		},
 		{
 			"packet already relayed ORDERED channel (no-op)",
 			func() {
@@ -604,6 +621,21 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			},
 			types.ErrSequenceReceiveNotFound,
 		},
+		{
+			"packet already received",
+			func() {
+				suite.coordinator.Setup(path)
+
+				sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
+				suite.Require().NoError(err)
+				packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
+				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+
+				// set recv seq start to indicate packet was processed in previous upgrade
+				suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetRecvStartSequence(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sequence+1)
+			},
+			types.ErrPacketReceived,
+		},
 		{
 			"receipt already stored",
 			func() {
@@ -687,6 +719,32 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
 			},
 			true,
 		},
+		{
+			"success: channel flushing",
+			func() {
+				suite.coordinator.Setup(path)
+				packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
+				ack = ibcmock.MockAcknowledgement
+
+				err := path.EndpointB.SetChannelState(types.FLUSHING)
+				suite.Require().NoError(err)
+				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			},
+			true,
+		},
+		{
+			"success: channel flush complete",
+			func() {
+				suite.coordinator.Setup(path)
+				packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
+				ack = ibcmock.MockAcknowledgement
+
+				err := path.EndpointB.SetChannelState(types.FLUSHCOMPLETE)
+				suite.Require().NoError(err)
+				channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			},
+			true,
+		},
 		{"channel not found", func() {
 			// use wrong channel naming
 			suite.coordinator.Setup(path)

modules/core/04-channel/keeper/upgrade.go

@@ -581,8 +581,10 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin
 		k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextSequenceSend)
 	}
 
-	// set the counterparty next sequence send as pruning sequence end in order to have upper bound to prune to
-	k.SetPruningSequenceEnd(ctx, portID, channelID, counterpartyUpgrade.NextSequenceSend)
+	// Set the counterparty next sequence send as the recv start sequence.
+	// This will be the upper bound for pruning and it will allow for replay
+	// protection of historical packets.
+	k.setRecvStartSequence(ctx, portID, channelID, counterpartyUpgrade.NextSequenceSend)
 
 	// First upgrade for this channel will set the pruning sequence to 1, the starting sequence for pruning.
 	// Subsequent upgrades will not modify the pruning sequence thereby allowing pruning to continue from the last

modules/core/04-channel/keeper/upgrade_test.go

@@ -1404,8 +1404,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() {
 				// Assert that pruning sequence start has not been initialized.
 				suite.Require().False(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
 
-				// Assert that pruning sequence end has not been set
-				counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				// Assert that recv start sequence has not been set
+				counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetRecvStartSequence(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().False(found)
 				suite.Require().Equal(uint64(0), counterpartyNextSequenceSend)
 			},
@@ -1433,8 +1433,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() {
 				suite.Require().True(found)
 				suite.Require().Equal(uint64(1), pruningSeq)
 
-				// Assert that pruning sequence end has been set correctly
-				counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				// Assert that the recv start sequence has been set correctly
+				counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetRecvStartSequence(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
 				suite.Require().Equal(uint64(2), counterpartySequenceSend)
 			},
@@ -1464,8 +1464,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() {
 				// Assert that pruning sequence start has not been initialized.
 				suite.Require().False(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
 
-				// Assert that pruning sequence end has not been set
-				counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				// Assert that recv start sequence has not been set
+				counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetRecvStartSequence(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().False(found)
 				suite.Require().Equal(uint64(0), counterpartyNextSequenceSend)
 			},
@@ -1494,8 +1494,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() {
 				suite.Require().True(found)
 				suite.Require().Equal(uint64(1), pruningSeq)
 
-				// Assert that pruning sequence end has been set correctly
-				counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				// Assert that the recv start sequence has been set correctly
+				counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetRecvStartSequence(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				suite.Require().True(found)
 				suite.Require().Equal(uint64(2), counterpartySequenceSend)
 			},

modules/core/04-channel/types/errors.go

@@ -55,5 +55,5 @@ var (
 	ErrTimeoutNotReached               = errorsmod.Register(SubModuleName, 39, "timeout not reached")
 	ErrTimeoutElapsed                  = errorsmod.Register(SubModuleName, 40, "timeout elapsed")
 	ErrPruningSequenceStartNotFound    = errorsmod.Register(SubModuleName, 41, "pruning sequence start not found")
-	ErrPruningSequenceEndNotFound      = errorsmod.Register(SubModuleName, 42, "pruning sequence end not found")
+	ErrRecvStartSequenceNotFound       = errorsmod.Register(SubModuleName, 42, "recv start sequence not found")
 )

modules/core/24-host/packet_keys.go

@@ -11,7 +11,7 @@ const (
 	KeyPacketAckPrefix        = "acks"
 	KeyPacketReceiptPrefix    = "receipts"
 	KeyPruningSequenceStart   = "pruningSequenceStart"
-	KeyPruningSequenceEnd     = "pruningSequenceEnd"
+	KeyRecvStartSequence      = "recvStartSequence"
 )
 
 // ICS04
@@ -103,14 +103,14 @@ func PruningSequenceStartKey(portID, channelID string) []byte {
 	return []byte(PruningSequenceStartPath(portID, channelID))
 }
 
-// PruningSequenceEndPath defines the path under which the pruning sequence end is stored
-func PruningSequenceEndPath(portID, channelID string) string {
-	return fmt.Sprintf("%s/%s", KeyPruningSequenceEnd, channelPath(portID, channelID))
+// RecvStartSequencePath defines the path under which the recv start sequence is stored
+func RecvStartSequencePath(portID, channelID string) string {
+	return fmt.Sprintf("%s/%s", KeyRecvStartSequence, channelPath(portID, channelID))
 }
 
-// PruningSequenceEndKey returns the store key for the pruning sequence end of a particular channel
-func PruningSequenceEndKey(portID, channelID string) []byte {
-	return []byte(PruningSequenceEndPath(portID, channelID))
+// RecvStartSequenceKey returns the store key for the recv start sequence of a particular channel
+func RecvStartSequenceKey(portID, channelID string) []byte {
+	return []byte(RecvStartSequencePath(portID, channelID))
 }
 
 func sequencePath(sequence uint64) string {