-
Notifications
You must be signed in to change notification settings - Fork 587
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
Update WriteAcknowledgementV2 to not depend on channels #7238
Changes from 12 commits
ac275b2
01d4bc8
6c075f5
1234b0c
7534d2a
6287cad
296f26c
52081b8
29f9ee3
829d3bd
b5a687a
f02c818
7c52580
075f7d8
a105608
039cd36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -283,7 +283,7 @@ func (k *Keeper) RecvPacketV2( | |
return errorsmod.Wrap(err, "couldn't verify counterparty packet commitment") | ||
} | ||
|
||
//if err := k.applyReplayProtection(ctx, packet, channel); err != nil { | ||
// if err := k.applyReplayProtection(ctx, packet, channel); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you know why this is commented out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it may have been commented out to keep diff low, it'll require a duplicate v2 version of all the functions down that code path. I think we can probably come up with a cleaner solution in the end where the v2 code path is the only path anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also just realised this entire fn doesn't need to exist, channelKeeper.RecvPacketV2 was the impl that existed on the port router branch, we can remove this since the v2 flow will always go through the PacketServerKeeper |
||
// return err | ||
//} | ||
|
||
|
@@ -298,7 +298,7 @@ func (k *Keeper) RecvPacketV2( | |
) | ||
|
||
// emit an event that the relayer can query for | ||
//emitRecvPacketEvent(ctx, packet, channel) | ||
// emitRecvPacketEvent(ctx, packet, channel) | ||
|
||
return nil | ||
} | ||
|
@@ -456,112 +456,6 @@ func (k *Keeper) WriteAcknowledgement( | |
return nil | ||
} | ||
|
||
// WriteAcknowledgementAsyncV2 TODO: this has not been tested at all. | ||
func (k *Keeper) WriteAcknowledgementAsyncV2( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved to packetServerKeeper |
||
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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,13 @@ package keeper | |
|
||
import ( | ||
"context" | ||
errorsmod "cosmossdk.io/errors" | ||
"errors" | ||
"fmt" | ||
|
||
"golang.org/x/exp/slices" | ||
|
||
errorsmod "cosmossdk.io/errors" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" | ||
|
@@ -614,14 +616,13 @@ func (k *Keeper) recvPacketV2(goCtx context.Context, msg *channeltypes.MsgRecvPa | |
if err := k.PacketServerKeeper.WriteAcknowledgementV2(ctx, msg.PacketV2, multiAck); err != nil { | ||
return nil, err | ||
} | ||
} else { | ||
// TODO; move to PacketServerKeeper this will blow up with no channels | ||
k.ChannelKeeper.SetMultiAcknowledgement(ctx, msg.PacketV2.GetDestinationPort(), msg.PacketV2.GetDestinationChannel(), msg.PacketV2.GetSequence(), multiAck) | ||
return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil | ||
} | ||
|
||
//defer telemetry.ReportRecvPacket(msg.Packet) | ||
// TODO; move to PacketServerKeeper this will blow up with no channels | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
k.ChannelKeeper.SetMultiAcknowledgement(ctx, msg.PacketV2.GetDestinationPort(), msg.PacketV2.GetDestinationChannel(), msg.PacketV2.GetSequence(), multiAck) | ||
|
||
//ctx.Logger().Info("receive packet callback succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) | ||
// ctx.Logger().Info("receive packet callback succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) | ||
|
||
return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil | ||
} | ||
|
@@ -854,14 +855,13 @@ func (k *Keeper) acknowledgementV2(goCtx context.Context, msg *channeltypes.MsgA | |
for _, pd := range msg.PacketV2.Data { | ||
cb := k.PortKeeper.AppRouter.Route(pd.AppName) | ||
err = cb.OnAcknowledgementPacketV2(ctx, msg.PacketV2, pd.Payload, recvResults[pd.AppName], relayer) | ||
|
||
if err != nil { | ||
ctx.Logger().Error("acknowledgement failed", "port-id", msg.PacketV2.SourcePort, "channel-id", msg.PacketV2.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet callback failed")) | ||
return nil, errorsmod.Wrap(err, "acknowledge packet callback failed") | ||
} | ||
} | ||
|
||
//defer telemetry.ReportAcknowledgePacket(msg.PacketV2) | ||
// defer telemetry.ReportAcknowledgePacket(msg.PacketV2) | ||
|
||
ctx.Logger().Info("acknowledgement succeeded", "port-id", msg.PacketV2.SourcePort, "channel-id", msg.PacketV2.SourceChannel, "result", channeltypes.SUCCESS.String()) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ref #7248