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

Implement z_validateaddress RPC #6083

Closed
arya2 opened this issue Feb 1, 2023 · 8 comments · Fixed by #6185
Closed

Implement z_validateaddress RPC #6083

arya2 opened this issue Feb 1, 2023 · 8 comments · Fixed by #6185
Assignees
Labels
A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement

Comments

@arya2
Copy link
Contributor

arya2 commented Feb 1, 2023

Motivation

Some mining pools use this RPC to check addresses.

Specifications

https://zcash.github.io/rpc/z_validateaddress.html

Designs

We can call the zcash_address crate to validate shielded and unified addresses. (Zebra also has its own transparent address parsing, but we'd be better to just be consistent and use zcash_address for everything.)

We just need to implement these fields:

  • isvalid
  • address
  • type
  • address_type (same as type)
  • ismine - always false, because Zebra doesn't have a wallet

The Address enum added in #6086 can be extended to convert shielded addresses as well for implementing this RPC.

@arya2 arya2 added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces labels Feb 1, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Zebra Feb 1, 2023
@mpguerra mpguerra added this to Zebra Feb 1, 2023
@mpguerra
Copy link
Contributor

mpguerra commented Feb 2, 2023

@upbqdn upbqdn self-assigned this Feb 6, 2023
@teor2345
Copy link
Contributor

teor2345 commented Feb 8, 2023

This is on the critical path for mining pool testing.

@upbqdn
Copy link
Member

upbqdn commented Feb 9, 2023

How do we know we don't have to implement the diversifier and diversifiedtransmissionkey fields described in the spec https://zcash.github.io/rpc/z_validateaddress.html?

@teor2345
Copy link
Contributor

teor2345 commented Feb 9, 2023

How do we know we don't have to implement the diversifier and diversifiedtransmissionkey fields described in the spec https://zcash.github.io/rpc/z_validateaddress.html?

We don't support Sapling shielded coinbase yet (#5472), and those fields are only used for Sapling. So we could add those fields to the Sapling ticket, if they aren't already there?

@upbqdn
Copy link
Member

upbqdn commented Feb 16, 2023

Even though it's not stated in the spec, Zcashd checks unified addresses. Should we also support those?

@teor2345
Copy link
Contributor

Even though it's not stated in the spec, Zcashd checks unified addresses. Should we also support those?

Maybe as a separate ticket once the unified address PR #6171 merges?

@teor2345
Copy link
Contributor

@upbqdn did you want to open that ticket so we don't forget? It doesn't need to be long, we can fill it in later.

@mpguerra
Copy link
Contributor

yes, I'd rather not block testing and the release on unified addresses. We can add them in after.

@mergify mergify bot closed this as completed in #6185 Feb 20, 2023
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Zebra Feb 20, 2023
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 16, 2023
@mpguerra mpguerra marked this as a duplicate of #8445 Jan 10, 2025
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
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants