From b1e041e1bbdb19f2a908007b3312fe437f93d3fa Mon Sep 17 00:00:00 2001 From: Charly Date: Thu, 10 Nov 2022 18:18:55 +0100 Subject: [PATCH] chore: rm event emission after context caching (#2662) (cherry picked from commit c912fd9d1be3a13470c217d21af40f4a67c230c3) --- .../host/keeper/relay.go | 2 -- modules/apps/29-fee/keeper/escrow.go | 12 ---------- modules/core/keeper/msg_server.go | 23 +++++++------------ 3 files changed, 8 insertions(+), 29 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay.go b/modules/apps/27-interchain-accounts/host/keeper/relay.go index 5aa767284c7..76170ea31b4 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay.go @@ -73,8 +73,6 @@ func (k Keeper) executeTx(ctx sdk.Context, sourcePort, destPort, destChannel str txMsgData.MsgResponses[i] = any } - // NOTE: The context returned by CacheContext() creates a new EventManager, so events must be correctly propagated back to the current context - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) writeCache() txResponse, err := proto.Marshal(txMsgData) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 95f79ee18d9..3a8e40f1718 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -74,9 +74,6 @@ func (k Keeper) DistributePacketFeesOnAcknowledgement(ctx sdk.Context, forwardRe k.distributePacketFeeOnAcknowledgement(cacheCtx, refundAddr, forwardAddr, reverseRelayer, packetFee) } - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - // write the cache writeFn() @@ -130,9 +127,6 @@ func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, timeoutRelayer sd k.distributePacketFeeOnTimeout(cacheCtx, refundAddr, timeoutRelayer, packetFee) } - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - // write the cache writeFn() @@ -177,9 +171,6 @@ func (k Keeper) distributeFee(ctx sdk.Context, receiver, refundAccAddress sdk.Ac // write the cache writeFn() - - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) } // RefundFeesOnChannelClosure will refund all fees associated with the given port and channel identifiers. @@ -228,9 +219,6 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st } } - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - // write the cache writeFn() diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 500d1dd8cf7..6d65b5a99af 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -393,13 +393,11 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke cacheCtx, writeFn := ctx.CacheContext() err = k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight) - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - switch err { case nil: writeFn() case channeltypes.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.NOOP}, nil default: return nil, sdkerrors.Wrap(err, "receive packet verification failed") @@ -410,12 +408,13 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. cacheCtx, writeFn = ctx.CacheContext() ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer) - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - // Events from callback are emitted regardless of acknowledgement success - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) if ack == nil || ack.Success() { // write application state changes for asynchronous and successful acknowledgements writeFn() + } else { + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + // Events should still be emitted from failed acks and asynchronous acks + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) } // Set packet acknowledgement only if the acknowledgement is not nil. @@ -471,13 +470,11 @@ func (k Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*c cacheCtx, writeFn := ctx.CacheContext() err = k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv) - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - switch err { case nil: writeFn() case channeltypes.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored return &channeltypes.MsgTimeoutResponse{Result: channeltypes.NOOP}, nil default: return nil, sdkerrors.Wrap(err, "timeout packet verification failed") @@ -539,13 +536,11 @@ func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeo cacheCtx, writeFn := ctx.CacheContext() err = k.ChannelKeeper.TimeoutOnClose(cacheCtx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv) - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - switch err { case nil: writeFn() case channeltypes.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored return &channeltypes.MsgTimeoutOnCloseResponse{Result: channeltypes.NOOP}, nil default: return nil, sdkerrors.Wrap(err, "timeout on close packet verification failed") @@ -610,13 +605,11 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn cacheCtx, writeFn := ctx.CacheContext() err = k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight) - // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. - ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) - switch err { case nil: writeFn() case channeltypes.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored return &channeltypes.MsgAcknowledgementResponse{Result: channeltypes.NOOP}, nil default: return nil, sdkerrors.Wrap(err, "acknowledge packet verification failed")