Rename Id Fields to Channel for Packet V2 #7428
Conversation
AdityaSripal
requested review from
colin-axner,
damiannolan,
srdtrk,
chatton,
DimitrisJim,
gjermundgaraba and
bznein
as code owners
| merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) | ||
|
|
||
| if err := k.ClientKeeper.VerifyNonMembership( | ||
| ctx, | ||
| packet.SourceId, | ||
| packet.SourceChannel, |
There was a problem hiding this comment.
This should be looking up the v2 channelEnd (now counterparty) and taking the clientID from it and using it here!
Not sure if you want to do that in this PR or not
| merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) | ||
|
|
||
| commitment := channeltypesv2.CommitPacket(packet) | ||
|
|
||
| if err := k.ClientKeeper.VerifyMembership( | ||
| ctx, | ||
| packet.DestinationId, | ||
| packet.DestinationChannel, |
There was a problem hiding this comment.
Likewise, lookup v2 channelEnd and use the clientID rather than channelID
There was a problem hiding this comment.
yes, changed this in packetserver/relay.go recently. In general, there might be some tweaks necessary in these files before tests for it pass.
There was a problem hiding this comment.
nice catch!! Though it seems this was already wrong before
Co-authored-by: Damian Nolan <[email protected]>
| } | ||
| } | ||
| if counterparty.ClientId != packet.SourceId { | ||
| if counterparty.ClientId != packet.SourceChannel { |
There was a problem hiding this comment.
not related to this but should be CounterpartyChannelId. Unsure if this applies elsewhere but feel free to fix in follow-up.
| merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path) | ||
|
|
||
| commitment := channeltypesv2.CommitPacket(packet) | ||
|
|
||
| if err := k.ClientKeeper.VerifyMembership( | ||
| ctx, | ||
| packet.DestinationId, | ||
| packet.DestinationChannel, |
There was a problem hiding this comment.
yes, changed this in packetserver/relay.go recently. In general, there might be some tweaks necessary in these files before tests for it pass.
|
|
I think the above should be fixed in a separate PR. Will create a tracking issue |
Description
closes: #7416
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.