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 getblocktemplate min and max times on testnet #5871

Closed
teor2345 opened this issue Dec 16, 2022 · 2 comments · Fixed by #5925
Closed

Fix getblocktemplate min and max times on testnet #5871

teor2345 opened this issue Dec 16, 2022 · 2 comments · Fixed by #5925
Assignees
Labels
A-consensus Area: Consensus rule updates A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug I-consensus Zebra breaks a Zcash consensus rule

Comments

@teor2345
Copy link
Contributor

Motivation

On testnet only, there is a 1679 second difference between the mintime produced by Zebra and zcashd for the same height.

This amount doesn't seem to correspond to any particular consensus rule.

Specifications

Define the median-time-past of a block to be the median (as defined in § 7.7.3 ‘Difficulty adjustment’ on p. 131) of the nTime fields of the preceding PoWMedianBlockSpan blocks (or all preceding blocks if there are fewer than PoWMedianBlockSpan). The median-time-past of a genesis block is not defined.
For each block other than the genesis block , nTime MUST be strictly greater than the median-time-past of that block.
For each block at block height 2 or greater on Mainnet , or block height 653606 or greater on Testnet , nTime MUST be less than or equal to the median-time-past of that block plus 90 · 60 seconds.

https://zips.z.cash/protocol/protocol.pdf#blockheader

This [difficulty] algorithm changed on Testnet, starting from block 299188, to allow "minimum-difficulty" blocks. If the block time of a block from this height onward is greater than 15 minutes after that of the preceding block, then the block is a minimum-difficulty block. In that case its nBits field MUST be set to ToCompact(PoWLimit)

https://zips.z.cash/zip-0205#change-to-difficulty-adjustment-on-testnet

Complex Code or Requirements

Zebra adjusts the mintime or maxtime on testnet, so the block time range is either for a standard difficulty block, or a minimum difficulty block.

Analysis

This is not a minimum difficulty block, so the maxtime should get adjusted to 15 minutes after the mintime. But instead it is 93.8 minutes later. But it should be 15m after the mintime for standard difficulty, or 75m (90m-15m) for minimum difficulty.

This seems like a bug in Zebra's testnet time adjustment. These comments are also wrong and don't match the spec:

// The max time is always a minimum difficulty block, because the minimum difficulty
// gap is 7.5 minutes, but the maximum gap is 90 minutes. This means that testnet blocks
// have two valid time ranges with different difficulties:
// * 1s - 7m30s: standard difficulty
// * 7m31s - 90m: minimum difficulty

It is also possible that zcashd sets a higher minimum than the time permitted by the consensus rules.

Related Work

This was discovered as part of:

Logs

$ 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
 }
@teor2345 teor2345 added C-bug Category: This is a bug A-consensus Area: Consensus rule updates S-needs-triage Status: A bug report needs triage P-Medium ⚡ I-consensus Zebra breaks a Zcash consensus rule A-rpc Area: Remote Procedure Call interfaces labels Dec 16, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 5, 2023

I added some raw pre-adjustment fields in PR #5917.

$ 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 >=2178714...                                                                                                                                                          
                                                                                                                                                                                                           
real    0m0.012s                                                                                                                                                                                           
user    0m0.006s                                                                                                                                                                                           
sys     0m0.005s                                                                                                                                                                                           
                                                                                                                                                                                                           
Querying zcashd test chain at height >=2178714...                                                                                                                                                          
                                                                                                                                                                                                           
real    0m0.020s                                                                                                                                                                                           
user    0m0.006s                                                                                                                                                                                           
sys     0m0.005s                                                                                                                                                                                           
                                       
Response diff between zebrad and zcashd:                                                                                                                                                                   
--- /run/user/1000/tmp.KiMUrRRqrS.rpc-diff/zebrad-test-2178714-getblocktemplate.json    2023-01-05 16:48:52.640399231 +1000                                                                                
+++ /run/user/1000/tmp.KiMUrRRqrS.rpc-diff/zcashd-test-2178714-getblocktemplate.json    2023-01-05 16:48:52.661398986 +1000                                                                                
@@ -1,5 +1,6 @@                                                                                                                                                                                            
 {                                                                                                                                                                                                         
   "capabilities": [                                                                                                                                                                                       
+    "proposal"                                                                                                                                                                                            
   ],                                                                                                                                                                                                      
   "version": 4,                                                                                                                                                                                           
   "previousblockhash": "0059837ee6454e239ee3ea18075d03322bb0706eb2c88b3b47a6789a61dad147",                                                                                                                
@@ -24,9 +25,9 @@                                                                                                                                                                                          
     "sigops": 1,                                                                                                                                                                                          
     "required": true                                                                                                                                                                                      
   },                                                                                                                                                                                                      
-  "longpollid": "0002178714c0a676ff1672906243000000000000000000",                                                                                                                                         
+  "longpollid": "0059837ee6454e239ee3ea18075d03322bb0706eb2c88b3b47a6789a61dad14725227",                                                                                                                  
   "target": "00f0cc0000000000000000000000000000000000000000000000000000000000",                     
-  "mintime": 1672899473,
+  "mintime": 1672900844,            
   "mutable": [                                                                                      
     "time",                         
     "transactions",  
@@ -37,11 +38,5 @@                                                                                   
   "sizelimit": 2000000, 
   "curtime": 1672901332,
   "bits": "2000f0cc",
-  "height": 2178715,
-  "maxtime": 1672906243,
-  "raw_target": "00f0cc0000000000000000000000000000000000000000000000000000000000",
-  "raw_bits": "2000f0cc",
-  "raw_curtime": 1672901332,
-  "raw_mintime": 1672900844,
-  "raw_maxtime": 1672906243
+  "height": 2178715
 }

This diff shows that Zebra's mintime field is wrong on testnet, but the raw_mintime (before the testnet minimum difficulty adjustment) matches zcashd.

So we might be subtracting rather than adding, or some other wrong calculation. I'll try to fix it tomorrow.

@mpguerra
Copy link
Contributor

mpguerra commented Jan 9, 2023

@teor2345 can you please add an estimate to this issue?

@mergify mergify bot closed this as completed in #5925 Jan 17, 2023
@mpguerra mpguerra moved this to 🛑 Won't Fix in Zebra Jan 19, 2023
@mpguerra mpguerra added this to Zebra Jan 19, 2023
@mpguerra mpguerra moved this from 🛑 Won't Fix to ✅ Done in Zebra Jan 19, 2023
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug I-consensus Zebra breaks a Zcash consensus rule
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants