change(rpc): Match zcashd's block template exactly

[teor2345]

Motivation

It's easier to compare Zebra and zcashd's RPC output if we use exactly the same coinbase transaction version, contents, and transaction order.

Part of testing #5686.

Depends-On: #5862

Complex Code or Requirements

These changes still obey the consensus rules, but they do it in a way that matches what zcashd does.

Solution

Add a constant and function argument that makes Zebra's block template match zcashd's, and turn it on for now.

Like zcashd's getblocktemplate RPC responses:

• Sort transactions based on their serialized data bytes
• Use v4 coinbase transactions, with:
  • an extra null byte at the end of the coinbase data
  • a u32::MAX sequence number
  • outputs in miner, ECC, MG, ZF order

Update snapshots.

Review

This is an optional change, but it makes testing much easier, because if the mempool is the same, then zcash-rpc-diff should match exactly.

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?

[codecov]

Codecov Report

Merging #5867 (5385774) into main (80a6d3c) will increase coverage by 0.15%.
The diff coverage is 50.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5867      +/-   ##
==========================================
+ Coverage   78.11%   78.27%   +0.15%     
==========================================
  Files         310      310              
  Lines       38838    38838              
==========================================
+ Hits        30340    30402      +62     
+ Misses       8498     8436      -62     

[teor2345]

When there were differences between the coinbase transactions, I used zcash-cli decoderawtransaction (hex data) to compare their parsed forms. We might want to add this to zcash-rpc-diff.

It is impossible to configure Zebra with the same miner address as zcashd, because zcashd generates a new address for each template.

I checked if the mempools were in sync using the coinbase fee and transaction diff as quick checks. Once they were in sync, I got pretty similar output. (Only the miner address, coinbase transaction, roots, long poll id, and extra Zebra fields were different.)

[teor2345]

This is odd, I didn't change any of the request code in this PR:

---- methods::tests::vectors::rpc_getblocktemplate stdout ----

The application panicked (crashed).
Message:  timeout while waiting for a request
 in zebra_test::mock_service::MockService<zebra_state::request::ReadRequest, zebra_state::response::ReadResponse, zebra_test::mock_service::PanicAssertion, alloc::boxed::Box<dyn core::error::Error+core::marker::Send+core::marker::Sync>>
Location: /opt/zebrad/zebra-test/src/mock_service.rs:449

https://github.com/ZcashFoundation/zebra/actions/runs/3702131038/jobs/6272900780#step:3:2224

Before the failure I see:

2022-12-15T09:37:26.315316Z  INFO Zebra has not synced to the chain tip. Hint: check your network connection, clock, and time zone settings. estimated_distance_to_chain_tip=200 local_tip_height=Height(1687104)
2022-12-15T09:37:26.315374Z  INFO Zebra has not synced to the chain tip. Hint: check your network connection, clock, and time zone settings. estimated_distance_to_chain_tip=0 local_tip_height=Height(1687104)
2022-12-15T09:37:26.315382Z  INFO Zebra has not synced to the chain tip. Hint: check your network connection, clock, and time zone settings. estimated_distance_to_chain_tip=200 local_tip_height=Height(1687104)
2022-12-15T09:37:26.315396Z  INFO Zebra has not synced to the chain tip. Hint: check your network connection, clock, and time zone settings. estimated_distance_to_chain_tip=200 local_tip_height=Height(1687104)
test methods::tests::vectors::rpc_getblocktemplate ... FAILED

https://github.com/ZcashFoundation/zebra/actions/runs/3702131038/jobs/6272900780#step:3:1869

I don't know if this is the normal test output.

[str4d]

It is impossible to configure Zebra with the same miner address as zcashd, because zcashd generates a new address for each template.

Use -mineraddress=X on zcashd to pin its coinbase address.

[teor2345]

It is impossible to configure Zebra with the same miner address as zcashd, because zcashd generates a new address for each template.

Use -mineraddress=X on zcashd to pin its coinbase address.

Thanks, I'm trying this now.

I got the output order slightly different from zcashd, so I'll also need to fix that.

[teor2345]

I might also have the transaction order slightly different, I've seen some shielded transactions in a different order,

[teor2345]

It seems like zcashd's transaction order is sometimes arbitrary. (Or I can't work out how they are sorted.)

But on testnet, and on mainnet when the order matches up, I can get the Zebra and zcashd block templates to match exactly. This means that Zebra can create valid blocks. (We still need to test to make sure it always creates valid blocks.)

$ ZCASH_CLI="zcash-cli -testnet" zcash-rpc-diff 38232 getblocktemplate                                                                                               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:
getblocktemplate

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

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

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

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


Response diff between zebrad and zcashd:
--- /run/user/1000/tmp.2HCTThHXya.rpc-diff/zebrad-test-2153481-getblocktemplate.json    2022-12-16 13:44:35.074629890 +1000
+++ /run/user/1000/tmp.2HCTThHXya.rpc-diff/zcashd-test-2153481-getblocktemplate.json    2022-12-16 13:44:35.092629752 +1000
@@ -1,5 +1,6 @@
 {
   "capabilities": [
+    "proposal"
   ],
   "version": 4,
   "previousblockhash": "00357e387d134477dbcdaa721b51cbdcdfda2ddfd792d71d8d413a0868c1a5be",
@@ -24,9 +25,9 @@
     "sigops": 1,
     "required": true
   },
-  "longpollid": "000215348150723e951671167237000000000000000000",
+  "longpollid": "00357e387d134477dbcdaa721b51cbdcdfda2ddfd792d71d8d413a0868c1a5be287",
   "target": "005fd45000000000000000000000000000000000000000000000000000000000",
-  "mintime": 1671160202,
+  "mintime": 1671161838,
   "mutable": [
     "time",
     "transactions",
@@ -37,6 +38,5 @@
   "sizelimit": 2000000,
   "curtime": 1671162275,
   "bits": "1f5fd450",
-  "height": 2153482,
-  "maxtime": 1671167237
+  "height": 2153482
 }

There's a weird difference in the mintime field, but it's only on testnet, I opened ticket #5871.

[oxarbitrage]

I think it looks good. I think this might became either the default or a configuration option.

The only problem i see on making it the default is the v4 coinbase instead of v5 which i think zcashd should update at some point.