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

change(utils): Add a direct connection mode to zebra-checkpoints #6516

Merged
merged 18 commits into from
Apr 26, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Apr 14, 2023

Instructions

Test the new direct JSON-RPC connections using:

cargo run --release --features=zebra-checkpoints --bin zebra-checkpoints -- --transport direct --addr 127.0.0.1:<zebrad port>

And the refactored existing functionality using:

cargo run --release --features=zebra-checkpoints --bin zebra-checkpoints -- --transport cli --addr 127.0.0.1:<zebrad port>
# automatically uses zcashd
cargo run --release --features=zebra-checkpoints --bin zebra-checkpoints -- --transport cli --backend zcashd
# actually uses zcashd, but in zebrad RPC parameter mode
cargo run --release --features=zebra-checkpoints --bin zebra-checkpoints -- --transport cli --backend zebrad

Motivation

In #6367 we want to generate checkpoints in CI. This is the Rust tool part of that ticket.

Complex Code or Requirements

There are 4 modes in this tool now:

  • zebrad direct
  • zcashd CLI
  • zebrad CLI
  • zcashd CLI in zebrad mode (not supported but it should work)

This means we need to test all of them. (Which I've done.)

Technically this code is async, but it doesn't actually use much concurrency.

Solution

Related Changes:

Unrelated cleanups:

Testing

The rest of the testing will happen as part of the next PR which puts this in CI.

Review

This is part of a routine release process simplification change.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

Add this new tool to CI, so we always have checkpoints ready for releases.
Fully test the generated checkpoints.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces A-compatibility Area: Compatibility with other nodes or wallets, or standard rules labels Apr 14, 2023
@teor2345 teor2345 requested a review from a team as a code owner April 14, 2023 05:40
@teor2345 teor2345 self-assigned this Apr 14, 2023
@teor2345 teor2345 requested a review from a team as a code owner April 14, 2023 05:40
@teor2345 teor2345 requested review from upbqdn and removed request for a team April 14, 2023 05:40
@github-actions github-actions bot added the C-feature Category: New features label Apr 14, 2023
@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Merging #6516 (838a98d) into main (7681da3) will increase coverage by 0.30%.
The diff coverage is 37.73%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6516      +/-   ##
==========================================
+ Coverage   77.72%   78.03%   +0.30%     
==========================================
  Files         307      307              
  Lines       40282    40256      -26     
==========================================
+ Hits        31310    31414     +104     
+ Misses       8972     8842     -130     

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.

Looks great. I left a couple of minor suggestions and questions.

mergify bot added a commit that referenced this pull request Apr 26, 2023
mergify bot added a commit that referenced this pull request Apr 26, 2023
@mergify mergify bot merged commit d3ce022 into main Apr 26, 2023
@mergify mergify bot deleted the checkpoints-direct-rpc branch April 26, 2023 23:35
@oxarbitrage oxarbitrage mentioned this pull request May 9, 2023
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-rpc Area: Remote Procedure Call interfaces A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-feature Category: New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants