-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make CommitmentOp generic over all ics23 specs #6331
Make CommitmentOp generic over all ics23 specs #6331
Conversation
Codecov Report
@@ Coverage Diff @@
## aditya/iavl-ics-query #6331 +/- ##
=========================================================
- Coverage 55.62% 55.61% -0.02%
=========================================================
Files 447 447
Lines 26995 27007 +12
=========================================================
+ Hits 15017 15020 +3
- Misses 10907 10916 +9
Partials 1071 1071 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!! WIll move the CommitmentOp to a different package on my PR, but this works great
Great. Please do move the code around and refactor in your branch. The gist of what you did was great, just saw a way to save duplicate code. |
* switch iavl store to use ics proof * fix proofs to return for correct height * appease linter * Register commitment op correctly * Make CommitmentOp generic over all ics23 specs (#6331) * Make the CommitmentOp generic over all ics23 ProofSpecs, using Type to distinguish * Register SimpleMerkle ics23 proof op as well * Addressed linter issues * move commitment proof to types * Apply suggestions from code review Co-authored-by: colin axner <25233464+colin-axner@users.noreply.github.com> * address review comments: * Apply suggestions from code review Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * allow proofs against empty store * address review comments * Update store/types/proof.go Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> * add changelog Co-authored-by: Ethan Frey <ethanfrey@users.noreply.github.com> Co-authored-by: colin axner <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Marko <marbar3778@yahoo.com>
Description
This extends the great work in #6324 and allows the same code to be reused for ics23 proofs derived from
merkle.SimpleProof
as well as the currentiavl.RangeProof
. It just adds some settings to use the serialized type to select a registered ProofSpec.If you like this, please merge into your PR before finalizing it. Once that is done, and #6323 is merged, then adding ics23 support for the root multistore proof should be quite easy with no code duplication.
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.
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer