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

add(rpc): getblock: return transaction details with verbosity=2 #9083

Merged
merged 6 commits into from
Dec 14, 2024

Conversation

conradoplg
Copy link
Collaborator

Motivation

We want getblock to match zcashd. With verbosity=2 we returned a list of hashes, but a list of transaction objects is expected.

Closes #9024

Specifications & References

https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L735

Solution

If verbosity=2, call getrawtransaction for each tx in the block internally and use its output to fill the transaction details.

There are tons of fields missing in getrawtransaction, but that will be another issue. When they are filled, they will be automatically returned by getblock too.

zcashd seems to not return the heigth and confirmations fields in the transactions details inside getblock (which are indeed redundant) but I think it's harmless to return them.

Tests

Expanded existing test. Also tested manually with zcash-cli.

Follow-up Work

See #8446 for other getblock related stuff, and we'll need an issue for completing getrawtransaction

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.

@conradoplg conradoplg requested a review from a team as a code owner December 12, 2024 01:29
@conradoplg conradoplg requested review from oxarbitrage and removed request for a team December 12, 2024 01:29
oxarbitrage
oxarbitrage previously approved these changes Dec 12, 2024
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks good, it might conflict with #9049

zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
@oxarbitrage
Copy link
Contributor

It seems some test is still locked somewhere. I am unsure if it is caused by this code but it seems so.

@arya2
Copy link
Contributor

arya2 commented Dec 13, 2024

This looks good, I opened a suggestion PR (#9084) for reducing db queries and service calls.

…erbosity=2" (#9084)

* Replaces multiple service calls (per transaction) with a single call to the state service for all of a block's transactions.

* adjustments to reuse code from getrawtransaction

---------

Co-authored-by: Conrado Gouvea <[email protected]>
@mergify mergify bot merged commit 543f066 into main Dec 14, 2024
171 checks passed
@mergify mergify bot deleted the get-block-tx-objs branch December 14, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getblock: return transactions details with verbosity=2
3 participants