Add PacketV2 Protos and CommitmentV2 function #7287
Conversation
chatton
requested review from
colin-axner,
AdityaSripal and
damiannolan
as code owners
| // identifies the receiving chain. | ||
| string destination_id = 3; | ||
| // timeout after which the packet times out | ||
| string timeout = 4; |
There was a problem hiding this comment.
not convinced this is the best idea, @damiannolan also pointed out that string parsing is expensive in an EVM environment.
Is there a good reason not to just use uint64 nanos only?
There was a problem hiding this comment.
I also agree that we should just have uint64
| @@ -26,6 +26,7 @@ func CommitPacket(packet Packet) []byte { | |||
| case IBC_VERSION_UNSPECIFIED, IBC_VERSION_1: | |||
| return commitV1Packet(packet) | |||
| case IBC_VERSION_2: | |||
| // TODO: convert to PacketV2 and commit. | |||
There was a problem hiding this comment.
do we have an issue? 🤔
There was a problem hiding this comment.
don't think so, this'll just be handled as part of PacketV2 internal logic refactor. Can create an issue for this specifically if you think it's worth it!
| // 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 CommitPacketV2(packet PacketV2) []byte { |
There was a problem hiding this comment.
nit: can also be private no?
There was a problem hiding this comment.
I think this'll end up being used in the packet-server package when acting on PacketV2 types and so will be public.
| // specifies the destination port of the packet. | ||
| string destination_port = 2; | ||
| // the payload to be sent to the application. | ||
| Payload payload = 3; |
There was a problem hiding this comment.
I don't think this will ever be nullable no?
|
| @@ -100,6 +101,42 @@ func commitV2Packet(packet Packet) []byte { | |||
| return hash[:] | |||
| } | |||
|
|
|||
| // CommitPacketV2 returns the V2 packet commitment bytes. The commitment consists of: | |||
| // sha256_hash(timeout) + sha256_hash(destinationID) + sha256_hash(packetData) from a given packet. | |||
There was a problem hiding this comment.
| // sha256_hash(timeout) + sha256_hash(destinationID) + sha256_hash(packetData) from a given packet. | |
| // sha256_hash(timeout) + sha256_hash(destinationID) + sha256_hash(packetData) for a given packet. |
There was a problem hiding this comment.
sorry missed this comment, will include this in a follow up!
| @@ -26,6 +26,7 @@ func CommitPacket(packet Packet) []byte { | |||
| case IBC_VERSION_UNSPECIFIED, IBC_VERSION_1: | |||
| return commitV1Packet(packet) | |||
There was a problem hiding this comment.
Can be of course done in a separate PR, but probably we can rename this to commitPacketV1?
Description
This PR adds the PacketV2 protos which we will use internally to convert a PacketV1 to a PacketV2 and commit based on the protocol version field.
I think the final hashing function and structure is not 100% set in stone yet, and might still change based on how the implementation goes.
closes: #XXXX
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.