refactor: replace usage of verification funcs in 03-connection #1647
Conversation
damiannolan
requested review from
colin-axner,
fedekunze and
AdityaSripal
as code owners
|
|
||
| if err := targetClient.VerifyMembership( | ||
| ctx, clientStore, k.cdc, height, | ||
| 0, 0, |
There was a problem hiding this comment.
might be nice to add an inline comment for future readers:
0, 0, // skip delay period checks for non-packet processing verificationor something along those lines
There was a problem hiding this comment.
Do you think we should add test cases for each of the verify functions with the time/block delay periods set to 0. That is, add test cases which will normally fail on the delay period not being passed (if 0, 0, isn't passed in). Thoughts?
There was a problem hiding this comment.
Great idea, I'll add the inline comment.
add test cases which will normally fail on the delay period not being passed (if 0, 0, isn't passed in)
So if 0, 0, isn't passed in here it won't fail? there's nothing at the lightclient level which would return an error, and it needs to be generic to support both zero values and non zero values for delay periods, right?
There was a problem hiding this comment.
Let's address this with a follow up if needs be.
* removing legacy verify funcs from clientstate interface * chore: remove legacy verify funcs tm client (#1651) * removing legacy verification methods from tendermint client * removing commented out tests
…s#1647) * replacing 03-connection verifications with generic membership methods * adding inline comments for skipping delay period checks * chore: removing legacy verify funcs from `ClientState` interface (cosmos#1649) * removing legacy verify funcs from clientstate interface * chore: remove legacy verify funcs tm client (cosmos#1651) * removing legacy verification methods from tendermint client * removing commented out tests * fixing interface definition
Description
Replacing usage of:
VerifyClientStateVerifyConsensusStateVerifyConnectionVerifyChannelVerifyPacketCommitmentVerifyAcknowledgementVerifyPacketReceiptVerifyPacketReceiptAbsencewith the generic
VerifyMembershipandVerifyNonMembershipfunctions.closes: #1293
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/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes