zebra icon indicating copy to clipboard operation
zebra copied to clipboard

feat(rpc): add getblockhash rpc method

Open oxarbitrage opened this issue 3 years ago • 3 comments

Motivation

After the release candidate Zebra will continue the development of more API calls. getblockhash is one of tme methods we will implement almost for sure.

This is low priority however as it is pretty much done i want to push it to zebra reoo as a PR.

This is a draft because is low priority however in other scenario it should be ready for review.

Close #5268.

Solution

Implement getblockhash.

Required Tests

  • [ ] Snapshot the output of the RPC
  • [ ] Test calling the RPC from the mining pool software
  • [ ] Manually compare the zcashd and zebrad RPC output using https://github.com/ZcashFoundation/zebra/blob/main/zebra-utils/zcash-rpc-diff

Review

Reviewer Checklist

  • [x] 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?
  • [ ] How do you know it works? Does it have tests?

Follow Up Work

oxarbitrage avatar Aug 27 '22 23:08 oxarbitrage

Codecov Report

Merging #4967 (fbe223a) into main (f31609e) will decrease coverage by 0.01%. The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4967      +/-   ##
==========================================
- Coverage   79.15%   79.14%   -0.02%     
==========================================
  Files         308      308              
  Lines       39324    39324              
==========================================
- Hits        31128    31123       -5     
- Misses       8196     8201       +5     

codecov[bot] avatar Aug 28 '22 00:08 codecov[bot]

This is blocked by tagging the release we send to the auditors, because it's part of a new feature.

teor2345 avatar Aug 28 '22 00:08 teor2345

We tagged the release candidate, so new features can merge now.

Can you add a priority label to this ticket, so people know how urgent it is?

teor2345 avatar Oct 13 '22 01:10 teor2345

Diffs:

$ ./zcash-rpc-diff 6666 getblockhash 5000
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zcashd is at: 13594
zebrad is at: 68496

Request:
getblockhash 5000

Querying zebrad main chain at height >=68496...

real	0m0,002s
user	0m0,002s
sys	0m0,000s

Querying zcashd main chain at height >=13594...

real	0m0,002s
user	0m0,001s
sys	0m0,000s


Response diff between zcashd and zebrad:
RPC responses were identical

/tmp/tmp.as2xmnOhNF.rpc-diff/zebrad-main-68496-getblockhash.json:
00000000c727e77227f6cb2f1217c457c79ceaac0e6d6db855c4731292f07edd
$ ./zcash-rpc-diff 6666 getblockhash 10000
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zcashd is at: 14775
zebrad is at: 76496

Request:
getblockhash 10000

Querying zebrad main chain at height >=76496...

real	0m0,003s
user	0m0,003s
sys	0m0,000s

Querying zcashd main chain at height >=14775...

real	0m0,003s
user	0m0,003s
sys	0m0,000s


Response diff between zcashd and zebrad:
RPC responses were identical

/tmp/tmp.aArwpungMG.rpc-diff/zebrad-main-76496-getblockhash.json:
000000002c2063bab0acae547bab7e0309db8e1b7003e9c01eb423c0166d5ac9
$ ./zcash-rpc-diff 6666 getblockhash 15000
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zcashd is at: 15103
zebrad is at: 78239

Request:
getblockhash 15000

Querying zebrad main chain at height >=78239...

real	0m0,007s
user	0m0,000s
sys	0m0,004s

Querying zcashd main chain at height >=15103...

real	0m0,003s
user	0m0,000s
sys	0m0,003s


Response diff between zcashd and zebrad:
RPC responses were identical

/tmp/tmp.HTR0I1UwBL.rpc-diff/zebrad-main-78239-getblockhash.json:
00000000b6bc56656812a5b8dcad69d6ad4446dec23b5ec456c18641fb5381ba
$ 

oxarbitrage avatar Oct 16 '22 13:10 oxarbitrage

@mergifyio update

teor2345 avatar Oct 19 '22 06:10 teor2345

update

✅ Branch has been successfully updated

mergify[bot] avatar Oct 19 '22 06:10 mergify[bot]

I think this might be blocked by running getblocktemplate-rpcs code in CI (#5405). What do you think?

teor2345 avatar Oct 19 '22 06:10 teor2345

this could be an existing bug, or bug from PR #5357 that only happens sometimes.

I can't see anything obvious in PR #5357, so here's what it could be:

  • an existing bug in the tests, revealed by timing or code changes
  • an existing bug in our state or database shutdown, revealed by timing or code changes (the errors are similar to #3133]
  • a bug in Rust's compiler

Maybe adding the getblocktemplate tests to CI (#5405) will help us diagnose the issue?

teor2345 avatar Oct 19 '22 19:10 teor2345

Failing in building the zebra-rpc crate individually, ill try to fix tomorrow and the PR should be ready.

oxarbitrage avatar Oct 20 '22 00:10 oxarbitrage

The changes in this PR cause memory corruption in the RPC tests on macOS:

process didn't exit successfully: /Users/runner/work/zebra/zebra/target/debug/deps/zebra_rpc-194938bf1186cc48 --nocapture (signal: 10, SIGBUS: access to undefined memory)

https://github.com/ZcashFoundation/zebra/actions/runs/3284184984/jobs/5409834731#step:14:6231

Since most of these changes are behind a feature, and the new state requests aren't called by the old tests, this could be an existing bug, or bug from PR https://github.com/ZcashFoundation/zebra/pull/5357 that only happens sometimes.

This PR was not ready when you reviewed, i was still working on it. it seems that error is gone. I will remove it from draft state when it is ready, there is still a known issue i will need to fix.

oxarbitrage avatar Oct 20 '22 00:10 oxarbitrage

@mergifyio update

teor2345 avatar Oct 21 '22 02:10 teor2345

update

✅ Branch has been successfully updated

mergify[bot] avatar Oct 21 '22 02:10 mergify[bot]