feat: add acknowledgePacket handler to channel/v2 #7412
Conversation
| @@ -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 | |||
There was a problem hiding this comment.
moved from relay.go to keeper.go
There was a problem hiding this comment.
renamed relay.go to packet.go
There was a problem hiding this comment.
created commitment.go to house commitment funcs
| } | ||
|
|
||
| path := hostv2.PacketAcknowledgementKey(packet.DestinationId, sdk.Uint64ToBigEndian(packet.Sequence)) | ||
| merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) |
There was a problem hiding this comment.
BuildMerklePath, used here and other funcs in this file is imported from the packet-server package.
We should move this to the channel/v2 types pkg afaik. We should not depend on anything in packet-server (which will ultimately be removed).
I'd also like some insight into the implementation of BuildMerklePath.
Some questions I have are:
Why are we not using the MerklePrefix type for the "ibc" prefix used by counterparties - instead we're using MerklePath.
Secondly, implementation looks complex and kinda funky, haven't stepped through to see exactly why that is, was hoping someone could explain it for me.
There was a problem hiding this comment.
re dependencies: I think we can do a pass at the end and make sure there is no dependency on v1 anything within the channel/v2 package.
| 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) |
There was a problem hiding this comment.
this will become a v2 channelEnd, and below clientID := counterparty.ClientID will read better... right now its slightly confusing.
The channelEnd.ClientID would suggest to me that it's the clientID of the client representing the counterparty state machine. Rather than a counterparty.ClientID which would suggest to me its the clientID on the counterparty which repesents this chain (self host where this code is being executed)
There was a problem hiding this comment.
yeah the naming is a bit all over the place at the moment with regards to counterparty/channel/ids etc, hopefully we'll be able to sort this out quite soon!
damiannolan
requested review from
colin-axner,
AdityaSripal and
chatton
as code owners
| k.Logger(ctx).Info("packet acknowledged", "sequence", strconv.FormatUint(packet.GetSequence(), 10), "source_id", packet.GetSourceId(), "destination_id", packet.GetDestinationId()) | ||
|
|
||
| EmitAcknowledgePacketEvents(ctx, packet, acknowledgement) |
There was a problem hiding this comment.
I guess these logs and events should be at the msg server handler layer. wdyt?
There was a problem hiding this comment.
I think that would make the most sense yeah, can do in a follow up though I think!
| 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) |
There was a problem hiding this comment.
yeah the naming is a bit all over the place at the moment with regards to counterparty/channel/ids etc, hopefully we'll be able to sort this out quite soon!
| k.Logger(ctx).Info("packet acknowledged", "sequence", strconv.FormatUint(packet.GetSequence(), 10), "source_id", packet.GetSourceId(), "destination_id", packet.GetDestinationId()) | ||
|
|
||
| EmitAcknowledgePacketEvents(ctx, packet, acknowledgement) |
There was a problem hiding this comment.
I think that would make the most sense yeah, can do in a follow up though I think!
| } | ||
|
|
||
| path := hostv2.PacketAcknowledgementKey(packet.DestinationId, sdk.Uint64ToBigEndian(packet.Sequence)) | ||
| merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) |
There was a problem hiding this comment.
re dependencies: I think we can do a pass at the end and make sure there is no dependency on v1 anything within the channel/v2 package.
|
Description
acknowledgePackethandler to channel/v2commitment.goin channel/v2/typescloses: #7369
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.