change(rpc): Simplify getdifficulty RPC implementation

[teor2345]

Motivation

We already have ExpandedDifficulty and Work types that implement the calculations required for the getdifficulty RPC.

Specifications

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

ToTarget() from:
https://zips.z.cash/protocol/protocol.pdf#nbits

Complex Code or Requirements

We're using floating-point here, so the results from zcashd and Zebra will be slightly different.

zcashd does (double)limit.mantissa / (double)bits.mantissa * 256.0^(limit.exponent - bits.exponent):
https://github.com/zcash/zcash/blob/7b28054e8b46eb46a9589d0bdc8e29f9fa1dc82d/src/rpc/blockchain.cpp#L46-L73

This PR makes Zebra just divide the first 128 bits, because floats only have 53 bits of accuracy anyway:

(expanded_pow_limit >> 128) as u128 as f64 / (expanded_difficulty >> 128) as u128 as f64

A previous version of this PR did:
(2^256 / (bits.mantissa * 256^bits.exponent)) as f64 / (2^256 / (limit.mantissa * 256^limit.exponent)) as f64, which was less accurate due to the integer division:

zebra/zebra-chain/src/work/difficulty.rs

Lines 314 to 329 in d67b3b6

	impl TryFrom<ExpandedDifficulty> for Work {
	type Error = ();
	fn try_from(expanded: ExpandedDifficulty) -> Result<Self, Self::Error> {
	// We need to compute `2^256 / (expanded + 1)`, but we can't represent
	// 2^256, as it's too large for a u256. However, as 2^256 is at least as
	// large as `expanded + 1`, it is equal to
	// `((2^256 - expanded - 1) / (expanded + 1)) + 1`, or
	let result = (!expanded.0 / (expanded.0 + 1)) + 1;
	if result <= u128::MAX.into() {
	Ok(Work(result.as_u128()))
	} else {
	Err(())
	}
	}
	}

Solution

• Divide the first 128 bits to simplify the getdifficulty RPC implementation
• Update the tests to ignore small differences, but require 6 significant figures
• Add extra test cases with valid difficulties
• Change all snapshots to use a valid fractional difficulty

Review

This is a suggestion for PR #6099.

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

I added some TODOs to this PR, but we don't need to fix them any time soon. The current code works.

[codecov]

Codecov Report

Merging #6105 (62ad153) into main (00be2da) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6105      +/-   ##
==========================================
+ Coverage   77.98%   78.03%   +0.05%     
==========================================
  Files         304      304              
  Lines       39042    39087      +45     
==========================================
+ Hits        30446    30501      +55     
+ Misses       8596     8586      -10     

[oxarbitrage]

I think the code changes are ok however i am not convinced with the differences, they are pretty big when compared with the original implementation which i suppose is what the users will be expecting to see.

[teor2345]

I think the code changes are ok however i am not convinced with the differences, they are pretty big when compared with the original implementation which i suppose is what the users will be expecting to see.

Sure, I was thinking about that as well. I can try some different calculations that are more accurate:

Just divide the first 128 bits, because floats only have 53 bits of accuracy anyway:

(expanded_pow_limit >> 128) as u128 as f64 / (expanded_difficulty >> 128) as u128 as f64

Just divide the first 64 non-zero bits:

let zero_bits = min(expanded_pow_limit.leading_zeros(), expanded_difficulty.leading_zeros());
(expanded_pow_limit >> (192 - zero_bits)) as u64 as f64 / (expanded_difficulty >> (192 - zero_bits)) as u64 as f64

[teor2345]

Just divide the first 128 bits, because floats only have 53 bits of accuracy anyway:

This worked, and it is accurate to 6 significant figures. I also think it might be more accurate than zcashd, but I'm not sure.

[teor2345]

It looks like actual difficulties are almost identical, which makes sense because we're now doing the same calculation as zcashd. (We just don't bother with the scaling, and we cut off the insignificant bits, because neither of then matter on real networks.)

Mainnet:

$ zcash-rpc-diff 28232 getdifficulty
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 28232) and zcashd (zcash-cli zcash.conf port).

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

Request:
getdifficulty

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

real    0m0.007s
user    0m0.004s
sys     0m0.004s

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

real    0m0.007s
user    0m0.004s
sys     0m0.003s


Response diff between zebrad and zcashd:
RPC responses were identical

/run/user/1000/tmp.rzDcrX0wwL.rpc-diff/zebrad-main-1975206-getdifficulty.json:
87521404.99683589

I also got identical responses for heights 1975207, 1975209, and 1975211.

Testnet:

$ ZCASH_CLI=zcash-cli-testnet zcash-rpc-diff 38232 getdifficulty
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 38232) and zcashd (zcash-cli-testnet zcash.conf port).

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

Request:
getdifficulty

Querying zebrad test chain at height >=2219908...

real    0m0.011s
user    0m0.007s
sys     0m0.004s

Querying zcashd test chain at height >=2219908...

real    0m0.011s
user    0m0.005s
sys     0m0.006s


Response diff between zebrad and zcashd:
--- /run/user/1000/tmp.SXZe2QzSL1.rpc-diff/zebrad-test-2219908-getdifficulty.json       2023-02-07 11:47:27.609433705 +1000
+++ /run/user/1000/tmp.SXZe2QzSL1.rpc-diff/zcashd-test-2219908-getdifficulty.json       2023-02-07 11:47:27.620433601 +1000
@@ -1 +1 @@
-16.902352256844026
+16.90235225684403

I got an identical response for height 2219910.

[oxarbitrage]

looks the same as the current implementation in regards to precision. Thanks.