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.

Sign up for GitHub

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

Jump to bottom

5. change(state): split ReadStateService requests into a ReadRequest enum #3866

Merged
merged 4 commits into from
Mar 17, 2022
Merged

5. change(state): split ReadStateService requests into a ReadRequest enum #3866

merged 4 commits into from
Mar 17, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Mar 14, 2022 •
edited
Loading

Motivation

This splits out a state service read request, in preparation for:

Splitting the request type allows us to just implement these methods once in the state.

Solution

  • Split out ReadRequest and ReadResponse state service enums
  • Make zebra-rpc use the new state ReadRequest

Related cleanups:

  • Simplify RPC test vectors

Review

This PR blocks a bunch of tickets, so I've marked it as high priority.

It is based on PR #3865.

Reviewer Checklist

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

Follow Up Work

The tickets listed at the top of this PR.

@teor2345 teor2345 added C-enhancement Category: This is an improvement P-High 🔥 A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes labels Mar 14, 2022
@teor2345 teor2345 requested a review from a team as a code owner March 14, 2022 02:45
@teor2345 teor2345 self-assigned this Mar 14, 2022
@teor2345 teor2345 requested review from dconnolly and removed request for a team March 14, 2022 02:45
@codecov
Copy link

codecov bot commented Mar 14, 2022 •
edited
Loading

Codecov Report

Merging #3866 (76849cb) into main (641f488) will increase coverage by 0.23%.
The diff coverage is 94.87%.

❗ Current head 76849cb differs from pull request most recent head 58b8a72. Consider uploading reports for the commit 58b8a72 to get more accurate results

@@            Coverage Diff             @@
##             main    #3866      +/-   ##
==========================================
+ Coverage   78.68%   78.92%   +0.23%     
==========================================
  Files         296      297       +1     
  Lines       33833    33918      +85     
==========================================
+ Hits        26622    26770     +148     
+ Misses       7211     7148      -63

@teor2345 teor2345 force-pushed the read-state-transaction branch from 72c6226 to 6ee059d Compare March 14, 2022 23:21
@teor2345 teor2345 force-pushed the state-request-split branch from a00c9e5 to eb0de98 Compare March 14, 2022 23:21
@teor2345 teor2345 mentioned this pull request Mar 15, 2022
Merged
3 tasks
@teor2345 teor2345 force-pushed the read-state-transaction branch from 6ee059d to 2e7ff25 Compare March 15, 2022 20:03
@teor2345 teor2345 force-pushed the state-request-split branch from eb0de98 to 945bf42 Compare March 15, 2022 20:03
@teor2345 teor2345 changed the title 5. Split ReadStateService requests into a ReadRequest enum 5. change(state): split ReadStateService requests into a ReadRequest enum Mar 15, 2022
@teor2345 teor2345 force-pushed the read-state-transaction branch from 2e7ff25 to 97e207c Compare March 16, 2022 00:04
@teor2345 teor2345 force-pushed the state-request-split branch from 945bf42 to 76849cb Compare March 16, 2022 00:05
@teor2345 teor2345 force-pushed the read-state-transaction branch from 97e207c to 23f1add Compare March 16, 2022 04:12
@teor2345 teor2345 force-pushed the state-request-split branch from 76849cb to a09cb45 Compare March 16, 2022 04:13
@ftm1000 ftm1000 added the lightwalletd any work associated with lightwalletd label Mar 16, 2022
@teor2345 teor2345 force-pushed the read-state-transaction branch from 23f1add to 7171c59 Compare March 17, 2022 07:10
@teor2345 teor2345 force-pushed the state-request-split branch from a09cb45 to 36f21bb Compare March 17, 2022 07:10
Base automatically changed from read-state-transaction to main March 17, 2022 18:48
@conradoplg conradoplg force-pushed the state-request-split branch from 36f21bb to 58b8a72 Compare March 17, 2022 19:40
conradoplg
conradoplg approved these changes Mar 17, 2022
View reviewed changes
Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

Rebased after the base PR merged; reapproving

mergify bot added a commit that referenced this pull request Mar 17, 2022
@mergify
Merge of #3866
b1a3096
@mergify mergify bot mentioned this pull request Mar 17, 2022
Closed
27 tasks
@jvff jvff mentioned this pull request Mar 17, 2022
Merged
3 tasks
@mergify mergify bot merged commit 39dfca8 into main Mar 17, 2022
@mergify mergify bot deleted the state-request-split branch March 17, 2022 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dconnolly dconnolly dconnolly left review comments

@conradoplg conradoplg conradoplg approved these changes

Assignees

@teor2345 teor2345

Labels
A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes C-enhancement Category: This is an improvement lightwalletd any work associated with lightwalletd
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@teor2345 @conradoplg @dconnolly @ftm1000