Fix the difficulty adjustment off-by-one; slight initial diff adjustment; BREAKS CONSENSUS

include/constants.h

@@ -33,8 +33,26 @@
 #define DIFFICULTY_ADJUSTMENT_INTERVAL 3840 // Every 3840 blocks (roughly every 4 days with a 90 second block time)
                                            // Max adjustment per is x2. So if blocks are coming in too fast, the difficulty will at most double every 24 hours, and vice versa if they're coming in too slow.
 #define TARGET_BLOCK_TIME 90 // Target block time in seconds
-//#define INITIAL_DIFFICULTY 0x1f0c1422 // Default compact target used by Autolykos2 PoW (This is ridiculously low)
-#define INITIAL_DIFFICULTY 0x1f1b7c51 // This takes 90s on my machine with a single thread, good for testing
+// The retarget measures the span between the FIRST and LAST block of the window, which is one fewer
+// interval than the window has blocks, and divides by it. Two blocks is the minimum that leaves a
+// non-zero span. See Chain_ComputeTargetAtHeight.
+static_assert(DIFFICULTY_ADJUSTMENT_INTERVAL >= 2,
+    "DIFFICULTY_ADJUSTMENT_INTERVAL must span at least one block interval");
+#define INITIAL_DIFFICULTY 0x1f0c1422 // Default compact target used by Autolykos2 PoW (This is ridiculously low)
+//#define INITIAL_DIFFICULTY 0x1f1b7c51 // Ridiculously low difficulty for testing.
+
+// Mining
+// The timestamp lives in the header the PoW hashes, so the miner restamps it while searching rather
+// than keeping the one stamped when the search started. Two things fall out of that: a block carries
+// the time it was actually found instead of a timestamp that is a whole block time stale on average,
+// and every restamp is a fresh search space, so the nonce sweep starts over from 0 and never has to
+// walk out to keep finding untried candidates. It costs nothing to throw the old nonce range away --
+// each attempt is independent, so the work already done was never getting any closer.
+static const uint64_t MINING_TIMESTAMP_REFRESH_MS = 2ULL; // Don't restamp for a drift smaller than this
+// Reading the clock once per hash would be wasted work next to a memory-hard hash, so the check is
+// batched. Note this, not the refresh interval, is what actually bounds accuracy once a batch of
+// hashes takes longer than MINING_TIMESTAMP_REFRESH_MS -- keep it small enough that it doesn't.
+static const uint64_t MINING_TIMESTAMP_CHECK_NONCES = 16ULL;
 
 // Sync / Reorg tuning constants
 // Timeouts and retry/backoff behavior for block fetches during sync (milliseconds)

src/block/chain.c

@@ -1681,7 +1681,13 @@ uint32_t Chain_ComputeTargetAtHeight(blockchain_t* chain, uint64_t height, uint3
         return currentTarget; // Invalid/non-increasing time window; keep current target
     }
 
-    const uint64_t targetTime = (uint64_t)TARGET_BLOCK_TIME * 1000ULL * (uint64_t)DIFFICULTY_ADJUSTMENT_INTERVAL;
+    // The window holds DIFFICULTY_ADJUSTMENT_INTERVAL blocks, but the two timestamps subtracted
+    // above are its FIRST and LAST, so what was measured is one fewer interval than there are
+    // blocks. Bitcoin compares that span against the full interval count anyway and carries a
+    // permanent ~1/(interval-1) fast bias for it; we compare against the span we actually measured,
+    // so the steady state is TARGET_BLOCK_TIME rather than TARGET_BLOCK_TIME * n/(n-1).
+    const uint64_t measuredIntervals = (uint64_t)DIFFICULTY_ADJUSTMENT_INTERVAL - 1ULL;
+    const uint64_t targetTime = (uint64_t)TARGET_BLOCK_TIME * 1000ULL * measuredIntervals;
 
     // Clamp per-epoch target movement: at most x2 easier or x2 harder. Clamping the measured span
     // is equivalent to clamping the ratio, but stays in integers.

src/main.c

@@ -101,14 +101,35 @@ static bool MineBlock(blockchain_t* chain, block_t* block) {
             (unsigned long long)epochIndex, dagBytes);
     }
 
-    for (uint64_t nonce = 0;; ++nonce) {
-        block->header.nonce = nonce;
-        if (Block_HasValidProofOfWorkWithParams(block, epochIndex, dagBytes, seed)) {
-            return true;
-        }
+    // Whatever BuildNextBlock stamped is only the time the search STARTED, so the header timestamp
+    // is refreshed as we go and the nonce sweep restarts against the new header. See
+    // MINING_TIMESTAMP_REFRESH_MS.
+    uint64_t stampedAt = block->header.timestamp;
 
-        if (nonce == UINT64_MAX) {
-            return false;
+    for (;;) {
+        for (uint64_t nonce = 0;; ++nonce) {
+            block->header.nonce = nonce;
+            if (Block_HasValidProofOfWorkWithParams(block, epochIndex, dagBytes, seed)) {
+                return true;
+            }
+
+            if (nonce == UINT64_MAX) {
+                return false;
+            }
+
+            if (((nonce + 1) % MINING_TIMESTAMP_CHECK_NONCES) != 0) {
+                continue;
+            }
+
+            // Only ever move the timestamp forward. CLOCK_REALTIME can step backwards under NTP,
+            // and backdating the block we are mining is exactly what the median-time-past rule
+            // treats as a node faking being behind.
+            const uint64_t now = get_current_time_ms();
+            if (now > stampedAt && (now - stampedAt) >= MINING_TIMESTAMP_REFRESH_MS) {
+                block->header.timestamp = now;
+                stampedAt = now;
+                break; // New header, so the nonces tried against the old one are worth retrying
+            }
         }
     }
 }