Update WriteAcknowledgementV2 to not depend on channels (#7238)

modules/apps/callbacks/testing/simapp/app.go

@@ -470,7 +470,7 @@ func NewSimApp(
 		appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
 		app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
 		app.IBCKeeper.ChannelKeeper, app.IBCKeeper.PortKeeper,
-		app.AccountKeeper, app.BankKeeper, scopedTransferKeeper,
+		app.AccountKeeper, app.BankKeeper, app.PacketServer, scopedTransferKeeper,
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
 

modules/apps/transfer/ibc_module_v2.go

@@ -31,7 +31,7 @@ func NewIBCModuleV2(k keeper.Keeper) IBCModuleV2 {
 }
 
 // OnSendPacketV2 implements the IBCModuleV2 interface.
-func (im IBCModuleV2) OnSendPacketV2(
+func (IBCModuleV2) OnSendPacketV2(
 	ctx sdk.Context,
 	portID string,
 	channelID string,

modules/apps/transfer/keeper/forwarding.go

@@ -102,7 +102,7 @@ func (k Keeper) acknowledgeForwardedPacketV2(ctx sdk.Context, forwardedPacket, p
 		recvResult.Status = channeltypes.PacketStatus_Failure
 	}
 
-	if err := k.channelKeeper.WriteAcknowledgementAsyncV2(ctx, forwardedPacket, types.ModuleName, recvResult); err != nil {
+	if err := k.packetServerKeeper.WriteAcknowledgementAsyncV2(ctx, forwardedPacket, types.ModuleName, recvResult); err != nil {
 		return err
 	}
 

modules/apps/transfer/keeper/keeper.go

@@ -22,6 +22,7 @@ import (
 	porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v9/modules/core/exported"
+	packetserver "github.com/cosmos/ibc-go/v9/modules/core/packet-server/keeper"
 )
 
 // Keeper defines the IBC fungible transfer keeper
@@ -30,12 +31,13 @@ type Keeper struct {
 	cdc            codec.BinaryCodec
 	legacySubspace types.ParamSubspace
 
-	ics4Wrapper   porttypes.ICS4Wrapper
-	channelKeeper types.ChannelKeeper
-	portKeeper    types.PortKeeper
-	authKeeper    types.AccountKeeper
-	bankKeeper    types.BankKeeper
-	scopedKeeper  exported.ScopedKeeper
+	ics4Wrapper        porttypes.ICS4Wrapper
+	channelKeeper      types.ChannelKeeper
+	packetServerKeeper *packetserver.Keeper
+	portKeeper         types.PortKeeper
+	authKeeper         types.AccountKeeper
+	bankKeeper         types.BankKeeper
+	scopedKeeper       exported.ScopedKeeper
 
 	// the address capable of executing a MsgUpdateParams message. Typically, this
 	// should be the x/gov module account.
@@ -52,6 +54,7 @@ func NewKeeper(
 	portKeeper types.PortKeeper,
 	authKeeper types.AccountKeeper,
 	bankKeeper types.BankKeeper,
+	packetServerKeeper *packetserver.Keeper,
 	scopedKeeper exported.ScopedKeeper,
 	authority string,
 ) Keeper {
@@ -65,16 +68,17 @@ func NewKeeper(
 	}
 
 	return Keeper{
-		cdc:            cdc,
-		storeKey:       key,
-		legacySubspace: legacySubspace,
-		ics4Wrapper:    ics4Wrapper,
-		channelKeeper:  channelKeeper,
-		portKeeper:     portKeeper,
-		authKeeper:     authKeeper,
-		bankKeeper:     bankKeeper,
-		scopedKeeper:   scopedKeeper,
-		authority:      authority,
+		cdc:                cdc,
+		storeKey:           key,
+		legacySubspace:     legacySubspace,
+		ics4Wrapper:        ics4Wrapper,
+		channelKeeper:      channelKeeper,
+		portKeeper:         portKeeper,
+		authKeeper:         authKeeper,
+		bankKeeper:         bankKeeper,
+		scopedKeeper:       scopedKeeper,
+		packetServerKeeper: packetServerKeeper,
+		authority:          authority,
 	}
 }
 

modules/apps/transfer/keeper/keeper_test.go

@@ -64,6 +64,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().IBCKeeper.PortKeeper,
 				suite.chainA.GetSimApp().AccountKeeper,
 				suite.chainA.GetSimApp().BankKeeper,
+				suite.chainA.GetSimApp().PacketServer,
 				suite.chainA.GetSimApp().ScopedTransferKeeper,
 				suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
 			)
@@ -78,6 +79,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().IBCKeeper.PortKeeper,
 				authkeeper.AccountKeeper{}, // empty account keeper
 				suite.chainA.GetSimApp().BankKeeper,
+				suite.chainA.GetSimApp().PacketServer,
 				suite.chainA.GetSimApp().ScopedTransferKeeper,
 				suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
 			)
@@ -92,6 +94,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().IBCKeeper.PortKeeper,
 				suite.chainA.GetSimApp().AccountKeeper,
 				suite.chainA.GetSimApp().BankKeeper,
+				suite.chainA.GetSimApp().PacketServer,
 				suite.chainA.GetSimApp().ScopedTransferKeeper,
 				"", // authority
 			)

modules/apps/transfer/types/expected_keepers.go

@@ -40,12 +40,6 @@ type ChannelKeeper interface {
 	GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool)
 	GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) []channeltypes.IdentifiedChannel
 	HasChannel(ctx sdk.Context, portID, channelID string) bool
-	WriteAcknowledgementAsyncV2(
-		ctx sdk.Context,
-		packet channeltypes.PacketV2,
-		appName string,
-		recvResult channeltypes.RecvPacketResult,
-	) error
 }
 
 // ClientKeeper defines the expected IBC client keeper

modules/core/02-client/types/msgs_test.go

@@ -27,6 +27,10 @@ import (
 	ibctesting "github.com/cosmos/ibc-go/v9/testing"
 )
 
+const (
+	invalidSigner = "invalid"
+)
+
 type TypesTestSuite struct {
 	testifysuite.Suite
 
@@ -640,7 +644,7 @@ func (suite *TypesTestSuite) TestMsgRecoverClientValidateBasic() {
 		{
 			"failure: invalid signer address",
 			func() {
-				msg.Signer = "invalid"
+				msg.Signer = invalidSigner
 			},
 			ibcerrors.ErrInvalidAddress,
 		},
@@ -703,7 +707,7 @@ func (suite *TypesTestSuite) TestMsgProvideCounterpartyValidateBasic() {
 		{
 			"failure: invalid signer address",
 			func() {
-				msg.Signer = "invalid"
+				msg.Signer = invalidSigner
 			},
 			ibcerrors.ErrInvalidAddress,
 		},
@@ -883,7 +887,7 @@ func (suite *TypesTestSuite) TestMsgIBCSoftwareUpgrade_ValidateBasic() {
 		{
 			"failure: invalid authority address",
 			func() {
-				signer = "invalid"
+				signer = invalidSigner
 			},
 			ibcerrors.ErrInvalidAddress,
 		},
@@ -982,7 +986,7 @@ func (suite *TypesTestSuite) TestMsgUpdateParamsValidateBasic() {
 		},
 		{
 			"failure: invalid signer address",
-			types.NewMsgUpdateParams("invalid", types.DefaultParams()),
+			types.NewMsgUpdateParams(invalidSigner, types.DefaultParams()),
 			false,
 		},
 		{

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

@@ -326,7 +326,7 @@ func EmitAcknowledgePacketEventV2(ctx sdk.Context, packet types.PacketV2, channe
 	})
 }
 
-// emitTimeoutPacketEvent emits a timeout packet event. It will be emitted both the first time a packet
+// EmitTimeoutPacketEvent emits a timeout packet event. It will be emitted both the first time a packet
 // is timed out for a certain sequence and for all duplicate timeouts.
 func EmitTimeoutPacketEvent(ctx sdk.Context, packet types.Packet, channel types.Channel) {
 	ctx.EventManager().EmitEvents(sdk.Events{

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

@@ -208,101 +208,6 @@ func (k *Keeper) RecvPacket(
 	return channel.Version, nil
 }
 
-// RecvPacketV2 is called by a module in order to receive & process an IBC packet
-// sent on the corresponding channel end on the counterparty chain.
-func (k *Keeper) RecvPacketV2(
-	ctx sdk.Context,
-	packet types.PacketV2,
-	proof []byte,
-	proofHeight exported.Height,
-) error {
-	channel, found := k.GetChannel(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel())
-	if !found {
-		return errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestinationChannel())
-	}
-
-	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
-	// packet sequence is < counterparty next sequence send. If the
-	// counterparty is implemented correctly, this may only occur
-	// when we are in FLUSHCOMPLETE and the counterparty has already
-	// completed the channel upgrade.
-	counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel())
-	if found {
-		counterpartyNextSequenceSend := counterpartyUpgrade.NextSequenceSend
-		if packet.GetSequence() >= counterpartyNextSequenceSend {
-			return errorsmod.Wrapf(types.ErrInvalidPacket, "cannot flush packet at sequence greater than or equal to counterparty next sequence send (%d) ≥ (%d).", packet.GetSequence(), counterpartyNextSequenceSend)
-		}
-	}
-
-	// packet must come from the channel's counterparty
-	if packet.GetSourcePort() != channel.Counterparty.PortId {
-		return errorsmod.Wrapf(
-			types.ErrInvalidPacket,
-			"packet source port doesn't match the counterparty's port (%s ≠ %s)", packet.GetSourcePort(), channel.Counterparty.PortId,
-		)
-	}
-
-	if packet.GetSourceChannel() != channel.Counterparty.ChannelId {
-		return errorsmod.Wrapf(
-			types.ErrInvalidPacket,
-			"packet source channel doesn't match the counterparty's channel (%s ≠ %s)", packet.GetSourceChannel(), channel.Counterparty.ChannelId,
-		)
-	}
-
-	// Connection must be OPEN to receive a packet. It is possible for connection to not yet be open if packet was
-	// sent optimistically before connection and channel handshake completed. However, to receive a packet,
-	// connection and channel must both be open
-	connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0])
-	if !found {
-		return errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0])
-	}
-
-	if connectionEnd.State != connectiontypes.OPEN {
-		return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectionEnd.State)
-	}
-
-	// check if packet timed out by comparing it with the latest height of the chain
-	selfHeight, selfTimestamp := clienttypes.GetSelfHeight(ctx), uint64(ctx.BlockTime().UnixNano())
-	timeout := types.NewTimeout(packet.GetTimeoutHeight(), packet.GetTimeoutTimestamp())
-	if timeout.Elapsed(selfHeight, selfTimestamp) {
-		return errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed")
-	}
-
-	commitment := types.CommitPacketV2(packet)
-
-	// verify that the counterparty did commit to sending this packet
-	if err := k.connectionKeeper.VerifyPacketCommitment(
-		ctx, connectionEnd, proofHeight, proof,
-		packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(),
-		commitment,
-	); err != nil {
-		return errorsmod.Wrap(err, "couldn't verify counterparty packet commitment")
-	}
-
-	// if err := k.applyReplayProtection(ctx, packet, channel); err != nil {
-	//	return err
-	//}
-
-	// log that a packet has been received & executed
-	k.Logger(ctx).Info(
-		"packet received",
-		"sequence", strconv.FormatUint(packet.GetSequence(), 10),
-		"src_port", packet.GetSourcePort(),
-		"src_channel", packet.GetSourceChannel(),
-		"dst_port", packet.GetDestinationPort(),
-		"dst_channel", packet.GetDestinationChannel(),
-	)
-
-	// emit an event that the relayer can query for
-	// emitRecvPacketEvent(ctx, packet, channel)
-
-	return nil
-}
-
 // applyReplayProtection ensures a packet has not already been received
 // and performs the necessary state changes to ensure it cannot be received again.
 func (k *Keeper) applyReplayProtection(ctx sdk.Context, packet types.Packet, channel types.Channel) error {
@@ -456,110 +361,6 @@ func (k *Keeper) WriteAcknowledgement(
 	return nil
 }
 
-// WriteAcknowledgementAsyncV2 TODO: this has not been tested at all.
-func (k *Keeper) WriteAcknowledgementAsyncV2(
-	ctx sdk.Context,
-	packet types.PacketV2,
-	appName string,
-	recvResult types.RecvPacketResult,
-) error {
-	// we should have stored the multi ack structure in OnRecvPacket
-	ackResults, found := k.GetMultiAcknowledgement(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel(), packet.GetSequence())
-	if !found {
-		return errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "multi-acknowledgement not found for %s", appName)
-	}
-
-	// find the index that corresponds to the app.
-	index := slices.IndexFunc(ackResults.AcknowledgementResults, func(result types.AcknowledgementResult) bool {
-		return result.AppName == appName
-	})
-
-	if index == -1 {
-		return errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "acknowledgement not found for %s", appName)
-	}
-
-	existingResult := ackResults.AcknowledgementResults[index]
-
-	// ensure that the existing status is async.
-	if existingResult.RecvPacketResult.Status != types.PacketStatus_Async {
-		return errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "acknowledgement for %s is not async", appName)
-	}
-
-	// modify the result and set it back.
-	ackResults.AcknowledgementResults[index].RecvPacketResult = recvResult
-	k.SetMultiAcknowledgement(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel(), packet.GetSequence(), ackResults)
-
-	// check if all acknowledgements are now sync.
-	isAsync := slices.ContainsFunc(ackResults.AcknowledgementResults, func(ackResult types.AcknowledgementResult) bool {
-		return ackResult.RecvPacketResult.Status == types.PacketStatus_Async
-	})
-
-	if !isAsync {
-		// if there are no more async acks, we can write the final multi ack.
-		return k.WriteAcknowledgementV2(ctx, packet, ackResults)
-	}
-
-	// we have updated one app's result, but there are still async results pending acknowledgement.
-	return nil
-}
-
-func (k *Keeper) WriteAcknowledgementV2(
-	ctx sdk.Context,
-	packet types.PacketV2,
-	multiAck types.MultiAcknowledgement,
-) error {
-	channel, found := k.GetChannel(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel())
-	if !found {
-		return errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestinationChannel())
-	}
-
-	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)
-	}
-
-	// 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. Any asynchronous acknowledgement writes from packets processed in a previous lifecycle of a channel
-	// will also be rejected.
-	recvStartSequence, _ := k.GetRecvStartSequence(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel())
-	if packet.GetSequence() < recvStartSequence {
-		return errorsmod.Wrap(types.ErrPacketReceived, "packet already processed in previous channel upgrade")
-	}
-
-	// NOTE: IBC app modules might have written the acknowledgement synchronously on
-	// the OnRecvPacket callback so we need to check if the acknowledgement is already
-	// set on the store and return an error if so.
-	if k.HasPacketAcknowledgementV2(ctx, packet.GetDestinationPort(), packet.GetDestinationChannel(), packet.GetSequence()) {
-		return types.ErrAcknowledgementExists
-	}
-
-	if len(multiAck.AcknowledgementResults) == 0 {
-		return errorsmod.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be empty")
-	}
-
-	multiAckBz := k.cdc.MustMarshal(&multiAck)
-	// set the acknowledgement so that it can be verified on the other side
-	k.SetPacketAcknowledgementV2(
-		ctx, packet.GetDestinationPort(), packet.GetDestinationChannel(), packet.GetSequence(),
-		types.CommitAcknowledgement(multiAckBz),
-	)
-
-	// log that a packet acknowledgement has been written
-	k.Logger(ctx).Info(
-		"acknowledgement written",
-		"sequence", strconv.FormatUint(packet.GetSequence(), 10),
-		"src_port", packet.GetSourcePort(),
-		"src_channel", packet.GetSourceChannel(),
-		"dst_port", packet.GetDestinationPort(),
-		"dst_channel", packet.GetDestinationChannel(),
-	)
-
-	EmitWriteAcknowledgementEventV2(ctx, packet, channel, multiAck)
-
-	return nil
-}
-
 // AcknowledgePacket is called by a module to process the acknowledgement of a
 // packet previously sent by the calling module on a channel to a counterparty
 // module on the counterparty chain. Its intended usage is within the ante