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

rpc: Add temporary fix and tests for block_results serialization #1061

Merged
merged 16 commits into from
Dec 20, 2021

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 <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
…vstore

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
…eInfo over ABCI and RPC

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
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 <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
@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 <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
@thanethomson thanethomson marked this pull request as ready for review December 17, 2021 21:54
Copy link
Member

@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 <connect@thanethomson.com>

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

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Move attribute after comment

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add support for simple query plan for Gaia

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Enable rustls for wss support

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix clippy lint

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
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 <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
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.

3 participants