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

fix(rpc): Make the verbose argument of the getrawtransaction RPC optional #8076

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

upbqdn
Copy link
Member

@upbqdn upbqdn commented Dec 8, 2023

Motivation

The verbose argument of the getrawtransaction RPC is specified as optional in the documentation: https://zcash.github.io/rpc/getrawtransaction.html. It's also specified as optional in Zebra's docs:

/// - `verbose`: (numeric, optional, default=0) If 0, return a string of hex-encoded data, otherwise return a JSON object.
, but not implemented as such.

PR Author Checklist

Check before marking the PR as ready for review:

  • Will the PR name make sense to users?
  • Does the PR have a priority label?
  • Have you added or updated tests?
  • Is the documentation up to date?

Specifications

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

Solution

  • Use Option<u8> instead of u8 for the verbose argument.

Testing

  • I adjusted existing tests and didn't add any new tests.
  • I manually tested that the argument is optional with this PR and that its value defaults to 0.
  • I also manually tested that Zebra required this argument without this PR, which didn't comply with the spec.

Review

Reviewer Checklist

Check before approving the PR:

  • Does the PR scope match the ticket?
  • Are there enough tests to make sure it works? Do the tests cover the PR motivation?
  • Are all the PR blockers dealt with?
    PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

The `verbose` argument of the `getrawtransaction` RPC is specified as
optional in the documentation:
https://zcash.github.io/rpc/getrawtransaction.html.
@upbqdn upbqdn added C-bug Category: This is a bug A-rust Area: Updates to Rust code P-Low ❄️ A-rpc Area: Remote Procedure Call interfaces labels Dec 8, 2023
@upbqdn upbqdn self-assigned this Dec 8, 2023
@upbqdn upbqdn requested a review from a team as a code owner December 8, 2023 16:26
@upbqdn upbqdn requested review from teor2345 and removed request for a team December 8, 2023 16:26
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thanks!

mergify bot added a commit that referenced this pull request Dec 10, 2023
@mergify mergify bot merged commit 9acdf0e into main Dec 10, 2023
125 of 126 checks passed
@mergify mergify bot deleted the fix-getrawtransaction-rpc branch December 10, 2023 21:44
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 A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants