Skip to content

Conversation

@thanethomson
Copy link
Contributor

@thanethomson thanethomson commented Dec 17, 2021

Closes #1021

The important changes are in:

The reason for the massive line count in this PR:

  1. I removed the rpc/tests/support folder and its fixtures, since those were manually generated, in favour of automatically generated fixtures from the rpc-probe (this PR also introduces a small query plan for interacting with a Gaia node).
  2. I generated some fixtures from a local Gaia node. These will be replaced when I can get access to the WebSocket endpoint of a production Gaia node.
  3. The output of curl http://18.191.147.51:26657/block_results?height=4555980 is added in here as a specific test case to ensure it parses.

One simple way of verifying this code is to check out this branch locally and then, from the root of the repository, run:

# Fetch the block_results response at height 4555980 (previously failed to parse),
# output it to stdout, and pass it through jq for readability.
cargo run -p tendermint-rpc \
    --features cli \
    --bin tendermint-rpc \
    -- \
    -u http://18.191.147.51:26657/ \
    block-results 4555980 | jq
  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
This introduces a (somewhat hacky, yet temporary) approach to being able
to deserialize public keys in validator updates until such time that
Tendermint addresses the problem.

Signed-off-by: Thane Thomson <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #1061 (ec10cf9) into v0.23.x (8354fec) will decrease coverage by 0.9%.
The diff coverage is 70.7%.

Impacted file tree graph

@@            Coverage Diff            @@
##           v0.23.x   #1061     +/-   ##
=========================================
- Coverage     66.1%   65.1%   -1.0%     
=========================================
  Files          209     210      +1     
  Lines        21023   20744    -279     
=========================================
- Hits         13913   13522    -391     
- Misses        7110    7222    +112     
Impacted Files Coverage Δ
rpc/src/request.rs 53.5% <0.0%> (-9.0%) ⬇️
tools/rpc-probe/src/common.rs 0.0% <0.0%> (ø)
tendermint/src/public_key.rs 75.0% <72.7%> (-0.1%) ⬇️
rpc/tests/gaia_fixtures.rs 98.4% <98.4%> (ø)
rpc/src/client/transport/mock.rs 88.1% <100.0%> (+<0.1%) ⬆️
rpc/src/client/transport/router.rs 74.5% <100.0%> (+0.4%) ⬆️
rpc/src/client/transport/websocket.rs 64.8% <100.0%> (+0.1%) ⬆️
tendermint/src/validator.rs 73.6% <100.0%> (+1.5%) ⬆️
rpc/src/endpoint/net_info.rs 50.0% <0.0%> (-25.0%) ⬇️
proto/src/serializers/evidence.rs 51.6% <0.0%> (-22.6%) ⬇️
... and 16 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 8354fec...ec10cf9. Read the comment docs.

Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
@thanethomson thanethomson marked this pull request as ready for review December 17, 2021 21:54
Copy link
Contributor

@adizere adizere left a comment

Choose a reason for hiding this comment

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

Amazing work Thane!

Confirm this works with the provided tests.

One last thought: Would it be worthwhile to make a PR on Hermes pinned with a dependency on this branch, and then pass it to Injective folks for further end-to-end testing? It seems unlikely we can uncover anything with this additional step, but not sure.

@thanethomson thanethomson merged commit e562d4d into v0.23.x Dec 20, 2021
@thanethomson thanethomson deleted the thane/1021-rpc-block_results-v0.23.x branch December 20, 2021 16:27
thanethomson added a commit that referenced this pull request Jan 19, 2022
* Rename kvstore module to common, since it provides common RPC requests

Signed-off-by: Thane Thomson <[email protected]>

* Rename quick module to kvstore, since it caters exclusively for the kvstore

Signed-off-by: Thane Thomson <[email protected]>

* Move attribute after comment

Signed-off-by: Thane Thomson <[email protected]>

* Add support for simple query plan for Gaia

Signed-off-by: Thane Thomson <[email protected]>

* Enable rustls for wss support

Signed-off-by: Thane Thomson <[email protected]>

* Fix clippy lint

Signed-off-by: Thane Thomson <[email protected]>

* Add changelog entry

Signed-off-by: Thane Thomson <[email protected]>
thanethomson added a commit that referenced this pull request Jan 31, 2022
* Add convenience method to decode RPC requests from strings

Signed-off-by: Thane Thomson <[email protected]>

* Add deserialization workaround for validator updates

This introduces a (somewhat hacky, yet temporary) approach to being able
to deserialize public keys in validator updates until such time that
Tendermint addresses the problem.

Signed-off-by: Thane Thomson <[email protected]>

* Add changelog entry

Signed-off-by: Thane Thomson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants