Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: rm event emission after context caching (backport #2662) #2727

Merged
merged 1 commit into from Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/relay.go
Expand Up @@ -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)
Expand Down
12 changes: 0 additions & 12 deletions modules/apps/29-fee/keeper/escrow.go
Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()

Expand Down
23 changes: 8 additions & 15 deletions modules/core/keeper/msg_server.go
Expand Up @@ -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")
Expand All @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down