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

feature(rpc): Migrate from deprecated jsonrpc_* crates to jsonrpsee #9059

Merged
merged 20 commits into from
Dec 21, 2024
Merged

feature(rpc): Migrate from deprecated jsonrpc_* crates to jsonrpsee #9059

merged 20 commits into from
Dec 21, 2024

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Nov 27, 2024 •
edited by arya2
Loading

Motivation

Close #8682

Solution

Upgrade zebra to use jsonrpsee.

Tests

Follow-up Work

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@github-actions github-actions bot added C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Nov 27, 2024
@mpguerra mpguerra linked an issue Dec 2, 2024 that may be closed by this pull request
Replace jsonrpc in zebra-rpc #8682
Closed
@arya2 arya2 mentioned this pull request Dec 5, 2024
Merged
7 tasks
@arya2 arya2 added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels Dec 5, 2024
@oxarbitrage oxarbitrage added A-dependencies Area: Dependency file updates A-rpc Area: Remote Procedure Call interfaces and removed C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Dec 18, 2024
@oxarbitrage oxarbitrage self-assigned this Dec 18, 2024
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Dec 18, 2024
oxarbitrage
oxarbitrage commented Dec 19, 2024
View reviewed changes
zebra-rpc/src/queue.rs Outdated Show resolved Hide resolved
@oxarbitrage oxarbitrage requested a review from arya2 December 19, 2024 13:52
@oxarbitrage oxarbitrage marked this pull request as ready for review December 19, 2024 13:53
@oxarbitrage oxarbitrage requested review from a team as code owners December 19, 2024 13:53
arya2
arya2 reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@arya2 arya2 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is looking excellent so far, thank you for minimizing non-essential changes.

One of the snapshots needs to be updated (the fields were re-ordered), and I'm not sure why the getblocktemplate test is failing yet, but I've reviewed most of the changes now and the only important suggestion I have so far is to return when the queue channel is closed.

zebra-rpc/src/methods/tests/prop.rs Outdated Show resolved Hide resolved
zebra-rpc/src/queue.rs Outdated Show resolved Hide resolved
zebra-rpc/src/config.rs Show resolved Hide resolved
zebra-rpc/src/methods.rs Show resolved Hide resolved
zebra-rpc/src/methods.rs Show resolved Hide resolved
zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs Outdated Show resolved Hide resolved
@arya2
Copy link
Contributor

arya2 commented Dec 20, 2024

"Migrate from deprecated jsonrpc_* crates to jsonrpsee" may be more clear as the PR title.

@oxarbitrage oxarbitrage changed the title feature(rpc): Introduce jsonrpsee crate for our RPC interface feature(rpc): Migrate from deprecated jsonrpc_* crates to jsonrpsee Dec 20, 2024
arya2
arya2 reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This is still looking good, none of these suggestions are blockers and I think they should be addressed in a follow-up PR. I'm reviewing one last file (the vectors tests).

zebra-rpc/src/server/http_request_compatibility.rs Outdated Show resolved Hide resolved
zebra-rpc/src/server/http_request_compatibility.rs Outdated Show resolved Hide resolved
zebra-rpc/src/server/rpc_call_compatibility.rs Show resolved Hide resolved
zebra-rpc/src/server/rpc_call_compatibility.rs Show resolved Hide resolved
@github-actions github-actions bot added C-deprecated extra-reviews This PR needs at least 2 reviews to merge labels Dec 20, 2024
arya2
arya2 previously approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for these changes.

zebra-rpc/src/server/tests/vectors.rs Show resolved Hide resolved
arya2
arya2 approved these changes Dec 21, 2024
View reviewed changes
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

🎉

@oxarbitrage oxarbitrage mentioned this pull request Dec 21, 2024
Open
@oxarbitrage oxarbitrage removed extra-reviews This PR needs at least 2 reviews to merge C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Dec 21, 2024
mergify bot added a commit that referenced this pull request Dec 21, 2024
@mergify
Merge of #9059
dadd5b0
@mergify mergify bot mentioned this pull request Dec 21, 2024
Closed
@mergify mergify bot merged commit 0fe47bb into main Dec 21, 2024
191 checks passed
@mergify mergify bot deleted the jsonrpsee branch December 21, 2024 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arya2 arya2 arya2 approved these changes

Assignees

@oxarbitrage oxarbitrage

Labels
A-dependencies Area: Dependency file updates A-rpc Area: Remote Procedure Call interfaces C-deprecated C-feature Category: New features P-Medium ⚡
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace jsonrpc in zebra-rpc
2 participants
@oxarbitrage @arya2