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

2a. refactor(rpc): Add the ChainTip and Network to RpcImpl #3863

Merged
merged 4 commits into from
Mar 15, 2022

Conversation

teor2345
Copy link
Contributor

Motivation

We need to estimate the chain height for these RPCs:

This PR adds the fields, but doesn't implement the RPC.

Solution

  • Add the ChainTip and Network to RpcImpl
  • Simplify RPC version field using generics

Review

This PR could cause merge conflicts, so I'm marking it as a high priority.

It is based on PR #3847.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

Implement #3143.

@teor2345 teor2345 added C-enhancement Category: This is an improvement P-High 🔥 A-rpc Area: Remote Procedure Call interfaces labels Mar 13, 2022
@teor2345 teor2345 requested a review from a team as a code owner March 13, 2022 23:08
@teor2345 teor2345 requested review from upbqdn and removed request for a team March 13, 2022 23:08
@teor2345 teor2345 changed the title refactor(rpc): Add the ChainTip and Network to RpcImpl 2. refactor(rpc): Add the ChainTip and Network to RpcImpl Mar 13, 2022
@codecov
Copy link

codecov bot commented Mar 13, 2022

Codecov Report

Merging #3863 (1f2a4c5) into main (5c62dd6) will increase coverage by 0.04%.
The diff coverage is 64.70%.

@@            Coverage Diff             @@
##             main    #3863      +/-   ##
==========================================
+ Coverage   78.80%   78.85%   +0.04%     
==========================================
  Files         296      296              
  Lines       33822    33833      +11     
==========================================
+ Hits        26655    26679      +24     
+ Misses       7167     7154      -13     

upbqdn
upbqdn previously approved these changes Mar 14, 2022
Copy link
Member

@upbqdn upbqdn left a comment

Choose a reason for hiding this comment

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

All looks good.

zebra-rpc/src/methods.rs Show resolved Hide resolved
zebra-rpc/src/methods/tests/prop.rs Show resolved Hide resolved
@teor2345 teor2345 force-pushed the read-only-state-service-impl branch from 6c8b6db to 8870944 Compare March 14, 2022 23:01
@teor2345 teor2345 changed the title 2. refactor(rpc): Add the ChainTip and Network to RpcImpl 2a. refactor(rpc): Add the ChainTip and Network to RpcImpl Mar 14, 2022
Base automatically changed from read-only-state-service-impl to main March 15, 2022 19:50
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Already approved PR with rebase.

mergify bot added a commit that referenced this pull request Mar 15, 2022
@teor2345 teor2345 merged commit 641f488 into main Mar 15, 2022
@teor2345 teor2345 deleted the rpc-chain-tip branch March 15, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants