From 6a12e77ebb020d339d189ed396f9e67578439224 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 8 Oct 2024 12:13:01 +0200 Subject: [PATCH 1/4] chore: add commitment.go with temporary commit acknowledgement func --- .../core/04-channel/v2/types/commitment.go | 55 +++++++++++++++++++ modules/core/04-channel/v2/types/packet.go | 39 ------------- 2 files changed, 55 insertions(+), 39 deletions(-) create mode 100644 modules/core/04-channel/v2/types/commitment.go diff --git a/modules/core/04-channel/v2/types/commitment.go b/modules/core/04-channel/v2/types/commitment.go new file mode 100644 index 00000000000..41fa17a4551 --- /dev/null +++ b/modules/core/04-channel/v2/types/commitment.go @@ -0,0 +1,55 @@ +package types + +import ( + "crypto/sha256" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// CommitPacket returns the V2 packet commitment bytes. The commitment consists of: +// sha256_hash(timeout) + sha256_hash(destinationID) + sha256_hash(packetData) from a given packet. +// This results in a fixed length preimage. +// NOTE: A fixed length preimage is ESSENTIAL to prevent relayers from being able +// to malleate the packet fields and create a commitment hash that matches the original packet. +func CommitPacket(packet Packet) []byte { + buf := sdk.Uint64ToBigEndian(packet.GetTimeoutTimestamp()) + + destIDHash := sha256.Sum256([]byte(packet.DestinationId)) + buf = append(buf, destIDHash[:]...) + + for _, data := range packet.Data { + buf = append(buf, hashPacketData(data)...) + } + + hash := sha256.Sum256(buf) + return hash[:] +} + +// hashPacketData returns the hash of the packet data. +func hashPacketData(data PacketData) []byte { + var buf []byte + sourceHash := sha256.Sum256([]byte(data.SourcePort)) + buf = append(buf, sourceHash[:]...) + destHash := sha256.Sum256([]byte(data.DestinationPort)) + buf = append(buf, destHash[:]...) + payloadValueHash := sha256.Sum256(data.Payload.Value) + buf = append(buf, payloadValueHash[:]...) + payloadEncodingHash := sha256.Sum256([]byte(data.Payload.Encoding)) + buf = append(buf, payloadEncodingHash[:]...) + payloadVersionHash := sha256.Sum256([]byte(data.Payload.Version)) + buf = append(buf, payloadVersionHash[:]...) + hash := sha256.Sum256(buf) + return hash[:] +} + +// CommitAcknowledgement returns the hash of the acknowledgement data. +func CommitAcknowledgement(acknowledgement Acknowledgement) []byte { + var buf []byte + for _, ack := range acknowledgement.GetAcknowledgementResults() { + hash := sha256.Sum256(ack.RecvPacketResult.GetAcknowledgement()) + buf = append(buf, hash[:]...) + } + + hash := sha256.Sum256(buf) + return hash[:] +} diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go index 85924dca020..8b075335eb5 100644 --- a/modules/core/04-channel/v2/types/packet.go +++ b/modules/core/04-channel/v2/types/packet.go @@ -1,13 +1,10 @@ package types import ( - "crypto/sha256" "strings" errorsmod "cosmossdk.io/errors" - sdk "github.com/cosmos/cosmos-sdk/types" - host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ) @@ -87,39 +84,3 @@ func (p Payload) Validate() error { } return nil } - -// CommitPacket returns the V2 packet commitment bytes. The commitment consists of: -// sha256_hash(timeout) + sha256_hash(destinationID) + sha256_hash(packetData) from a given packet. -// This results in a fixed length preimage. -// NOTE: A fixed length preimage is ESSENTIAL to prevent relayers from being able -// to malleate the packet fields and create a commitment hash that matches the original packet. -func CommitPacket(packet Packet) []byte { - buf := sdk.Uint64ToBigEndian(packet.GetTimeoutTimestamp()) - - destIDHash := sha256.Sum256([]byte(packet.DestinationId)) - buf = append(buf, destIDHash[:]...) - - for _, data := range packet.Data { - buf = append(buf, hashPacketData(data)...) - } - - hash := sha256.Sum256(buf) - return hash[:] -} - -// hashPacketData returns the hash of the packet data. -func hashPacketData(data PacketData) []byte { - var buf []byte - sourceHash := sha256.Sum256([]byte(data.SourcePort)) - buf = append(buf, sourceHash[:]...) - destHash := sha256.Sum256([]byte(data.DestinationPort)) - buf = append(buf, destHash[:]...) - payloadValueHash := sha256.Sum256(data.Payload.Value) - buf = append(buf, payloadValueHash[:]...) - payloadEncodingHash := sha256.Sum256([]byte(data.Payload.Encoding)) - buf = append(buf, payloadEncodingHash[:]...) - payloadVersionHash := sha256.Sum256([]byte(data.Payload.Version)) - buf = append(buf, payloadVersionHash[:]...) - hash := sha256.Sum256(buf) - return hash[:] -} From 288f2d340ddb34181cbe1490623d11f866a9d157 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 8 Oct 2024 12:13:25 +0200 Subject: [PATCH 2/4] feat: impl acknowledgePacket handler in channel/v2 --- modules/core/04-channel/v2/keeper/keeper.go | 12 ++-- .../core/04-channel/v2/keeper/msg_server.go | 42 +++++++++++- modules/core/04-channel/v2/keeper/relay.go | 64 +++++++++++++++++-- 3 files changed, 104 insertions(+), 14 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/keeper.go b/modules/core/04-channel/v2/keeper/keeper.go index d367855475a..4f91a3ace6f 100644 --- a/modules/core/04-channel/v2/keeper/keeper.go +++ b/modules/core/04-channel/v2/keeper/keeper.go @@ -75,7 +75,7 @@ func (k *Keeper) GetCounterparty(ctx context.Context, clientID string) (types.Co } // GetPacketReceipt returns the packet receipt from the packet receipt path based on the sourceID and sequence. -func (k *Keeper) GetPacketReceipt(ctx context.Context, sourceID string, sequence uint64) (string, bool) { +func (k *Keeper) GetPacketReceipt(ctx context.Context, sourceID string, sequence uint64) []byte { store := k.storeService.OpenKVStore(ctx) bigEndianBz := sdk.Uint64ToBigEndian(sequence) bz, err := store.Get(hostv2.PacketReceiptKey(sourceID, bigEndianBz)) @@ -83,9 +83,9 @@ func (k *Keeper) GetPacketReceipt(ctx context.Context, sourceID string, sequence panic(err) } if len(bz) == 0 { - return "", false + return nil } - return string(bz), true + return bz } // SetPacketReceipt writes the packet receipt under the receipt path @@ -121,7 +121,7 @@ func (k *Keeper) HasPacketAcknowledgement(ctx context.Context, sourceID string, } // GetPacketCommitment returns the packet commitment hash under the commitment path. -func (k *Keeper) GetPacketCommitment(ctx context.Context, sourceID string, sequence uint64) (string, bool) { +func (k *Keeper) GetPacketCommitment(ctx context.Context, sourceID string, sequence uint64) []byte { store := k.storeService.OpenKVStore(ctx) bigEndianBz := sdk.Uint64ToBigEndian(sequence) bz, err := store.Get(hostv2.PacketCommitmentKey(sourceID, bigEndianBz)) @@ -129,9 +129,9 @@ func (k *Keeper) GetPacketCommitment(ctx context.Context, sourceID string, seque panic(err) } if len(bz) == 0 { - return "", false + return nil } - return string(bz), true + return bz } // SetPacketCommitment writes the commitment hash under the commitment path. diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index 0455fd7adbe..129097d4360 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -43,8 +43,46 @@ func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPack return &channeltypesv2.MsgSendPacketResponse{Sequence: sequence}, nil } -func (*Keeper) Acknowledgement(ctx context.Context, acknowledgement *channeltypesv2.MsgAcknowledgement) (*channeltypesv2.MsgAcknowledgementResponse, error) { - panic("implement me") +func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAcknowledgement) (*channeltypesv2.MsgAcknowledgementResponse, error) { + sdkCtx := sdk.UnwrapSDKContext(ctx) + relayer, err := sdk.AccAddressFromBech32(msg.Signer) + if err != nil { + sdkCtx.Logger().Error("acknowledgement failed", "error", errorsmod.Wrap(err, "Invalid address for msg Signer")) + return nil, errorsmod.Wrap(err, "Invalid address for msg Signer") + } + + cacheCtx, writeFn := sdkCtx.CacheContext() + err = k.acknowledgePacket(cacheCtx, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight) + + switch err { + case nil: + writeFn() + case channeltypesv1.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored + sdkCtx.Logger().Debug("no-op on redundant relay", "source-id", msg.Packet.SourceId) + return &channeltypesv2.MsgAcknowledgementResponse{Result: channeltypesv1.NOOP}, nil + default: + sdkCtx.Logger().Error("acknowledgement failed", "source-id", msg.Packet.SourceId, "error", errorsmod.Wrap(err, "acknowledge packet verification failed")) + return nil, errorsmod.Wrap(err, "acknowledge packet verification failed") + } + + // k.Logger(ctx).Info("packet acknowledged", "sequence", strconv.FormatUint(packet.GetSequence(), 10), "src_port", packet.GetSourcePort(), "src_channel", packet.GetSourceChannel(), "dst_port", packet.GetDestPort(), "dst_channel", packet.GetDestChannel()) + + // channelkeeper.EmitAcknowledgePacketEvent(ctx, packet, nil) + + _ = relayer + + // TODO: implement once app router is wired up. + // https://github.com/cosmos/ibc-go/issues/7384 + // for _, pd := range msg.PacketData { + // cbs := k.PortKeeper.AppRouter.Route(pd.SourcePort) + // err := cbs.OnSendPacket(ctx, msg.SourceId, sequence, msg.TimeoutTimestamp, pd, signer) + // if err != nil { + // return nil, err + // } + // } + + return nil, nil } // RecvPacket implements the PacketMsgServer RecvPacket method. diff --git a/modules/core/04-channel/v2/keeper/relay.go b/modules/core/04-channel/v2/keeper/relay.go index df034cc2f6a..ce079fa69ba 100644 --- a/modules/core/04-channel/v2/keeper/relay.go +++ b/modules/core/04-channel/v2/keeper/relay.go @@ -137,8 +137,9 @@ func (k Keeper) recvPacket( // 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.DestinationId, packet.Sequence) - if found { + // TODO: change to HasReceipt + receipt := k.GetPacketReceipt(ctx, packet.DestinationId, packet.Sequence) + if receipt != nil { EmitRecvPacketEvents(ctx, packet) // This error indicates that the packet has already been relayed. Core IBC will // treat this error as a no-op in order to prevent an entire relay transaction @@ -173,6 +174,58 @@ func (k Keeper) recvPacket( return nil } +func (k Keeper) acknowledgePacket(ctx context.Context, packet channeltypesv2.Packet, acknowledgement channeltypesv2.Acknowledgement, proof []byte, proofHeight exported.Height) error { + // Lookup counterparty associated with our channel and ensure + // that the packet was indeed sent by our counterparty. + counterparty, ok := k.GetCounterparty(ctx, packet.SourceId) + if !ok { + return errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.SourceId) + } + + if counterparty.ClientId != packet.DestinationId { + return channeltypes.ErrInvalidChannelIdentifier + } + clientID := counterparty.ClientId + + commitment := k.GetPacketCommitment(ctx, packet.SourceId, packet.Sequence) + if len(commitment) == 0 { + // TODO: emit events + // k.EmitAcknowledgePacketEvent(ctx, packet, nil) + + // This error indicates that the acknowledgement has already been relayed + // or there is a misconfigured relayer attempting to prove an acknowledgement + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + return channeltypes.ErrNoOpMsg + } + + packetCommitment := channeltypesv2.CommitPacket(packet) + + // verify we sent the packet and haven't cleared it out yet + if !bytes.Equal(commitment, packetCommitment) { + return errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "commitment bytes are not equal: got (%v), expected (%v)", packetCommitment, commitment) + } + + path := hostv2.PacketAcknowledgementKey(packet.DestinationId, sdk.Uint64ToBigEndian(packet.Sequence)) + merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) + + if err := k.ClientKeeper.VerifyMembership( + ctx, + clientID, + proofHeight, + 0, 0, + proof, + merklePath, + channeltypesv2.CommitAcknowledgement(acknowledgement), + ); err != nil { + return errorsmod.Wrapf(err, "failed packet acknowledgement verification for client (%s)", clientID) + } + + k.DeletePacketCommitment(ctx, packet.SourceId, packet.Sequence) + + return nil +} + // timeoutPacket implements the timeout logic required by a packet handler. // The packet is checked for correctness including asserting that the packet was // sent and received on clients which are counterparties for one another. @@ -209,9 +262,8 @@ func (k Keeper) timeoutPacket( } // check that the commitment has not been cleared and that it matches the packet sent by relayer - commitment, ok := k.GetPacketCommitment(ctx, packet.SourceId, packet.Sequence) - - if !ok { + commitment := k.GetPacketCommitment(ctx, packet.SourceId, packet.Sequence) + if len(commitment) == 0 { EmitTimeoutPacketEvents(ctx, packet) // This error indicates that the timeout has already been relayed // or there is a misconfigured relayer attempting to prove a timeout @@ -222,7 +274,7 @@ func (k Keeper) timeoutPacket( packetCommitment := channeltypesv2.CommitPacket(packet) // verify we sent the packet and haven't cleared it out yet - if !bytes.Equal([]byte(commitment), packetCommitment) { + if !bytes.Equal(commitment, packetCommitment) { return errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) } From f56cd9f547ade5ae3c83c4cb6a94398117569450 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 8 Oct 2024 12:23:43 +0200 Subject: [PATCH 3/4] nit: vanity nits --- modules/core/04-channel/v2/keeper/keeper.go | 12 ++++++++++++ .../04-channel/v2/keeper/{relay.go => packet.go} | 12 ------------ 2 files changed, 12 insertions(+), 12 deletions(-) rename modules/core/04-channel/v2/keeper/{relay.go => packet.go} (95%) diff --git a/modules/core/04-channel/v2/keeper/keeper.go b/modules/core/04-channel/v2/keeper/keeper.go index 4f91a3ace6f..159efe8aada 100644 --- a/modules/core/04-channel/v2/keeper/keeper.go +++ b/modules/core/04-channel/v2/keeper/keeper.go @@ -199,3 +199,15 @@ func (k *Keeper) AliasV1Channel(ctx context.Context, portID, channelID string) ( } return counterparty, true } + +// getV1Counterparty attempts to retrieve a v1 channel from the channel keeper if it exists, then converts it +// to a v2 counterparty and stores it in the v2 channel keeper for future use +func (k *Keeper) getV1Counterparty(ctx context.Context, port, id string) (types.Counterparty, bool) { + if counterparty, ok := k.AliasV1Channel(ctx, port, id); ok { + // we can key on just the channel here since channel ids are globally unique + k.SetCounterparty(ctx, id, counterparty) + return counterparty, true + } + + return types.Counterparty{}, false +} diff --git a/modules/core/04-channel/v2/keeper/relay.go b/modules/core/04-channel/v2/keeper/packet.go similarity index 95% rename from modules/core/04-channel/v2/keeper/relay.go rename to modules/core/04-channel/v2/keeper/packet.go index ce079fa69ba..d3f8c62822a 100644 --- a/modules/core/04-channel/v2/keeper/relay.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -17,18 +17,6 @@ import ( "github.com/cosmos/ibc-go/v9/modules/core/packet-server/types" ) -// getV1Counterparty attempts to retrieve a v1 channel from the channel keeper if it exists, then converts it -// to a v2 counterparty and stores it in the v2 channel keeper for future use -func (k *Keeper) getV1Counterparty(ctx context.Context, port, id string) (channeltypesv2.Counterparty, bool) { - if counterparty, ok := k.AliasV1Channel(ctx, port, id); ok { - // we can key on just the channel here since channel ids are globally unique - k.SetCounterparty(ctx, id, counterparty) - return counterparty, true - } - - return channeltypesv2.Counterparty{}, false -} - // sendPacket constructs a packet from the input arguments, writes a packet commitment to state // in order for the packet to be sent to the counterparty. func (k *Keeper) sendPacket( From 11980bcfe2ff30344a5c9f171f29555105f734ed Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 8 Oct 2024 13:50:16 +0200 Subject: [PATCH 4/4] chore: add placeholder event func and logs --- modules/core/04-channel/v2/keeper/events.go | 13 +++++++++---- modules/core/04-channel/v2/keeper/msg_server.go | 4 ---- modules/core/04-channel/v2/keeper/packet.go | 4 ++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/events.go b/modules/core/04-channel/v2/keeper/events.go index e1ffea106ff..ced49356045 100644 --- a/modules/core/04-channel/v2/keeper/events.go +++ b/modules/core/04-channel/v2/keeper/events.go @@ -11,12 +11,17 @@ func EmitSendPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { // TODO: https://github.com/cosmos/ibc-go/issues/7386 } -// EmitTimeoutPacketEvents emits events for the TimeoutPacket handler. -func EmitTimeoutPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { +// EmitRecvPacketEvents emits events for the RecvPacket handler. +func EmitRecvPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { // TODO: https://github.com/cosmos/ibc-go/issues/7386 } -// EmitRecvPacketEvents emits events for the RecvPacket handler. -func EmitRecvPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { +// EmitAcknowledgePacketEvents emits events for the AcknowledgePacket handler. +func EmitAcknowledgePacketEvents(ctx context.Context, packet channeltypesv2.Packet, acknowledgement channeltypesv2.Acknowledgement) { + // TODO: https://github.com/cosmos/ibc-go/issues/7386 +} + +// EmitTimeoutPacketEvents emits events for the TimeoutPacket handler. +func EmitTimeoutPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { // TODO: https://github.com/cosmos/ibc-go/issues/7386 } diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index 129097d4360..27e36c14cc5 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -66,10 +66,6 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAck return nil, errorsmod.Wrap(err, "acknowledge packet verification failed") } - // k.Logger(ctx).Info("packet acknowledged", "sequence", strconv.FormatUint(packet.GetSequence(), 10), "src_port", packet.GetSourcePort(), "src_channel", packet.GetSourceChannel(), "dst_port", packet.GetDestPort(), "dst_channel", packet.GetDestChannel()) - - // channelkeeper.EmitAcknowledgePacketEvent(ctx, packet, nil) - _ = relayer // TODO: implement once app router is wired up. diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index d3f8c62822a..07d172219b5 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -211,6 +211,10 @@ func (k Keeper) acknowledgePacket(ctx context.Context, packet channeltypesv2.Pac k.DeletePacketCommitment(ctx, packet.SourceId, packet.Sequence) + k.Logger(ctx).Info("packet acknowledged", "sequence", strconv.FormatUint(packet.GetSequence(), 10), "source_id", packet.GetSourceId(), "destination_id", packet.GetDestinationId()) + + EmitAcknowledgePacketEvents(ctx, packet, acknowledgement) + return nil }