Skip to content
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

Refactored and completed Path; added query to Chain trait; refactored query code into TendermintChain #174

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

greg-szabo
Copy link
Member

Closes: #157
Closes: #172
Closes: #173

Description

Apologies for the monstre changes again, but the changes are unfortunately dependent on each other.

  • Refactored Path into an enum, implemented all paths. Result is a smaller codebase.
  • Paths now know if they are in the provableStore or not and a check was added to the query to not try to prove something in the privateStore.
  • Implemented query in line with ADR: Domain Decomposition #162 : Chain trait and TendermintChain. This gives us (somewhat) a template on how to implement subscription and transactions. At least it helped me understand the necessities, so I can start working on those in the relayer.
  • Deleted a bunch more obsolete code, where I could.

Tests are lacking, but maybe soon, in another issue. (Local execution worked.)


For contributor use:

  • Unit tests written
  • Added test to CI if applicable
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer

Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff! Left a few suggestions for future consideration but nothing blocking the merge :)

relayer/relay/src/chain.rs Show resolved Hide resolved
modules/src/ics24_host/path.rs Show resolved Hide resolved
modules/src/ics24_host/path.rs Show resolved Hide resolved
relayer/relay/src/error.rs Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #174 into master will increase coverage by 1.9%.
The diff coverage is 19.9%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #174     +/-   ##
========================================
+ Coverage    13.6%   15.6%   +1.9%     
========================================
  Files          69      58     -11     
  Lines        3752    3720     -32     
  Branches     1374    1429     +55     
========================================
+ Hits          513     582     +69     
+ Misses       2618    2430    -188     
- Partials      621     708     +87     
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/client_type.rs 47.6% <ø> (ø)
modules/src/ics03_connection/error.rs 25.0% <0.0%> (-8.4%) ⬇️
modules/src/ics03_connection/exported.rs 42.1% <0.0%> (-9.6%) ⬇️
modules/src/ics04_channel/error.rs 23.0% <0.0%> (-2.0%) ⬇️
modules/src/ics04_channel/packet.rs 0.0% <0.0%> (ø)
modules/src/ics07_tendermint/error.rs 33.3% <0.0%> (+33.3%) ⬆️
modules/src/ics07_tendermint/msgs/create_client.rs 0.0% <0.0%> (ø)
modules/src/ics23_commitment/mod.rs 0.0% <0.0%> (ø)
modules/src/ics24_host/path.rs 0.0% <0.0%> (ø)
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9545ccd...5fe3179. Read the comment docs.

@greg-szabo greg-szabo merged commit 6548786 into master Jul 29, 2020
@greg-szabo greg-szabo deleted the greg/generic-chains branch July 29, 2020 00:27
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
… query code into TendermintChain (informalsystems#174)

* Refactored and completed Path
* added query to Chain trait and implemented it for Tendermint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants