[DAA] Add ASERT DAA with height-based fork

src/consensus/activation.cpp

@@ -71,3 +71,15 @@ bool IsPhononEnabled(const Consensus::Params &params,
     return pindexPrev->GetMedianTimePast() >=
            gArgs.GetArg("-phononactivationtime", params.phononActivationTime);
 }
+
+bool IsASERTEnabled(const Consensus::Params &params, int32_t nHeight) {
+    return nHeight >= gArgs.GetArg("-asertactivationheight", INT_MAX);
+}
+
+bool IsASERTEnabled(const Consensus::Params &params, const CBlockIndex *pindexPrev) {
+    if (pindexPrev == nullptr) {
+        return false;
+    }
+
+    return IsASERTEnabled(params, pindexPrev->nHeight);
+}
\ No newline at end of file

src/consensus/activation.h

@@ -35,4 +35,8 @@ bool IsGravitonEnabled(const Consensus::Params &params,
 bool IsPhononEnabled(const Consensus::Params &params,
                      const CBlockIndex *pindexPrev);
 
+/** Check if the proposed ASERT DAA change has activated. */
+bool IsASERTEnabled(const Consensus::Params &params,
+                     const CBlockIndex *pindexPrev);
+
 #endif // BITCOIN_CONSENSUS_ACTIVATION_H

src/pow.cpp

@@ -102,6 +102,10 @@ uint32_t GetNextWorkRequired(const CBlockIndex *pindexPrev,
         return pindexPrev->nBits;
     }
 
+    if (IsASERTEnabled(params, pindexPrev)) {
+        return GetNextASERTWorkRequired(pindexPrev, pblock, params, gArgs.GetArg("-asertactivationheight", INT_MAX));
+    }
+
     if (IsDAAEnabled(params, pindexPrev)) {
         return GetNextCashWorkRequired(pindexPrev, pblock, params);
     }
@@ -282,3 +286,79 @@ uint32_t GetNextCashWorkRequired(const CBlockIndex *pindexPrev,
 
     return nextTarget.GetCompact();
 }
+
+/**
+ * Compute the next required proof of work using an absolutely scheduled 
+ * exponentially weighted target (ASERT).
+ *
+ * With ASERT, we define an ideal schedule for block issuance (e.g. 1 block every 600 seconds), and we calculate the
+ * difficulty based on how far the most recent block's timestamp is ahead of or behind that schedule.
+ * We set our targets (difficulty) exponentially. For every [tau] seconds ahead of or behind schedule we get, we
+ * double or halve the difficulty. 
+ */
+uint32_t GetNextASERTWorkRequired(const CBlockIndex *pindexPrev,
+                                  const CBlockHeader *pblock,
+                                  const Consensus::Params &params,
+                                  const int32_t nforkHeight) {
+    // This cannot handle the genesis block and early blocks in general.
+    assert(pindexPrev);
+
+    // Special difficulty rule for testnet:
+    // If the new block's timestamp is more than 2* 10 minutes then allow
+    // mining of a min-difficulty block.
+    if (params.fPowAllowMinDifficultyBlocks &&
+        (pblock->GetBlockTime() >
+         pindexPrev->GetBlockTime() + 2 * params.nPowTargetSpacing)) {
+        return UintToArith256(params.powLimit).GetCompact();
+    }
+
+    // Diff halves/doubles for every 2 days behind/ahead of schedule we get
+    const uint64_t tau = 2*24*60*60; 
+
+    // This algorithm uses fixed-point math. The lowest rbits bits are after
+    // the radix, and represent the "decimal" (or binary) portion of the value
+    const uint8_t rbits = 16; 
+
+    const CBlockIndex *pforkBlock = &pindexPrev[nforkHeight];
+
+    assert(pforkBlock != nullptr);
+    assert(pindexPrev->nHeight >= params.DifficultyAdjustmentInterval());
+
+    int32_t nTimeDiff = pindexPrev->nTime - pforkBlock->GetBlockHeader().nTime;
+    int32_t nHeightDiff = pindexPrev->nHeight - pforkBlock->nHeight;
+
+    const arith_uint256 origTarget = arith_uint256().SetCompact(pforkBlock->nBits);
+    arith_uint256 nextTarget = origTarget;
+
+    // Ultimately, we want to approximate the following ASERT formula, using only integer (fixed-point) math:
+    //     new_target = old_target * 2^((blocks_time - IDEAL_BLOCK_TIME*(height_diff+1)) / tau)
+
+    // First, we'll calculate the exponent:
+    int64_t exponent = ((nTimeDiff - params.nPowTargetSpacing * (nHeightDiff+1)) << rbits) / tau;
+
+    // Next, we use the 2^x = 2 * 2(x-1) identity to shift our exponent into the [0, 1) interval.
+    // The truncated exponent tells us how many shifts we need to do
+    // Note: This needs to be a right shift. Right shift rounds downward, whereas division rounds towards zero.
+    int8_t shifts = exponent >> rbits;
+    if (shifts < 0) {
+        nextTarget = nextTarget >> -shifts;
+    } else {
+        nextTarget = nextTarget << shifts;
+    }
+    exponent -= (shifts << rbits);
+
+    // Now we compute an approximated target * 2^(exponent)
+    // 2^x ~= (1 + 0.695502049*x + 0.2262698*x**2 + 0.0782318*x**3) for 0 <= x < 1
+    // Error versus actual 2^x is less than 0.013%.
+    uint64_t factor = (195766423245049*exponent + 
+                       971821376*exponent*exponent + 
+                       5127*exponent*exponent*exponent + (1ll<<47))>>(rbits*3);
+    nextTarget += (nextTarget * factor) >> rbits;
+
+    const arith_uint256 powLimit = UintToArith256(params.powLimit);
+    if (nextTarget > powLimit) {
+        return powLimit.GetCompact();
+    }
+
+    return nextTarget.GetCompact();
+}

src/pow.h

@@ -38,4 +38,8 @@ uint32_t GetNextCashWorkRequired(const CBlockIndex *pindexPrev,
                                  const CBlockHeader *pblock,
                                  const Consensus::Params &params);
 
+uint32_t GetNextASERTWorkRequired(const CBlockIndex *pindexPrev,
+                                  const CBlockHeader *pblock,
+                                  const Consensus::Params &params,
+                                  const int32_t nforkHeight);
 #endif // BITCOIN_POW_H

[Jonathan Toomim]

As per mtrycz's comments in BCHN code-review slack, this should be nHeight > gArgs ..., not >=. The fork block can't be both the reference block and the first block whose difficulty is set by the new DAA.

[Jonathan Toomim]

On second thought, I'm going to fix this by changing ASERT to get the parent of the fork block. That way the activation code has the same >= form as the activation code for other forks.

[freetrader]

I've done something similar in my branch where I've switched away from height-based to MTP activation. I'm not longer passing a height in to GetNextASERTWorkRequired, I'm passing a reference (activation) block. The code to do that 'searching' would be replaced by some hardcoded reference the activation block once the upgrade is past, checkpointed and under sufficient work.

freetrader/bitcoin-cash-node@e48e009e

Still need to test more.

[freetrader]

This function can be declared as noexcept.

[freetrader]

constexpr

[freetrader]

constexpr

[freetrader]

I believe this should be

const CBlockIndex *pforkBlock = pindexPrev->GetAncestor(nforkHeight);

[Jonathan Toomim]

Yes, I believe you are correct.

[freetrader]

Not really sure why a check is made here that the height >= 2016.

That number seems to have no special significance to ASERT implementation that I can see.

Perhaps this is a remnant of assertion code from some other algorithm that had a dependency on a window of certain size being available.

[Jonathan Toomim]

I copy-pasted this from other DAA code, and didn't think about it too much. I don't have any objections to it being removed.

[freetrader]

const

[freetrader]

This is just my preference maybe, but I would also suggest increasing this type to int64_t , even if block timestamps are currently 32 bits. This variable is intended for the most part to hold a positive value, so range is up to 2^31-1 which is "only" ~68 years.

[freetrader]

const

Would also assert that nHeightDiff > 0 just to be sure input is sane

I mean, there is no reason to apply the algorithm to blocks before the pforkBlock, is there?

[Jonathan Toomim]

I agree on both points. There is no need to do retroactive difficulty calculations in the current Bitcoin logic or expected unit tests.

[freetrader]

I suspect we have to be very careful about the left shift here.

For negative a, the behavior of a << b is undefined. (until C++20)

[Jonathan Toomim]

I think the most reasonable approach is to rely on the undefined arithmetic shift behavior, and add unit tests to ensure that the undefined behavior is as expected. The alternative is to use multiplication here, which isn't too bad, and to use division with special if statements to switch the rounding direction elsewhere.

[freetrader]

Nit:

There is an exponentiation symbol missing in the identity:

=> 2^(x-1)

This nit applies to various implementations of this algo which share this 'identity' comment.

[Jonathan Toomim]

Yep.

[Jonathan Toomim]

int8_t isn't large enough. Target can be as big as 1<<224, so +127/-128 shifts is insufficient to cover the full range unless the target of the reference (fork) block is between 1<<96 and 1<<128.

[freetrader]

Similar caveat about left-shifting negative values being UB. (until C++20)

[freetrader]

powLimit can be defined as a static local object. (static const arith_uint256 powLimit = ...)