Rewrite GRC::GetConstantBlockReward

This refactors GetConstantBlockReward to use the blockchain
consensus paramters, removes the lookback limitation for block
v13+ to correspond with the removal of the contract
lookback window for protocol entries, and corrects the behavior
of the lookup if the input index is not at pIndexBest.

src/gridcoin/staking/reward.cpp

@@ -3,7 +3,6 @@
 // file COPYING or https://opensource.org/licenses/mit-license.php.
 
 #include "amount.h"
-#include "gridcoin/appcache.h"
 #include "gridcoin/protocol.h"
 #include "gridcoin/staking/reward.h"
 #include "main.h"
@@ -33,25 +32,64 @@ CAmount GetCoinYearReward(int64_t nTime)
 
 CAmount GRC::GetConstantBlockReward(const CBlockIndex* index)
 {
+    CAmount reward = 0;
+    CAmount MIN_CBR = 0;
+    CAmount MAX_CBR = 0;
+
+    // GetConstantBlockReward is called with a CBlockIndex pointer as an argument, which means it is expected to
+    // return the correct value at that position in the chain. The TryActive call in the protocol registry returns
+    // the CURRENT active value if that exists. This is not what we need if index is not actually pIndexBest.
+    // Here we find the last protocol entry in the historical linked list of protocol entries for the key "blockreward1"
+    // that has a timestamp that is less than or equal to the block time. This will filter for the correct historical
+    // value of blockreward1 that was valid at the index given. One could argue there is some fuzziness here in using
+    // the block time versus the transaction context time implied by the protocol registry entry; however the actual
+    // change of CBR must occur on a block boundary anyway, and as long as there is consistent application for consensus
+    // for V13+ we are safe.
+    ProtocolEntryOption reward_entry = GetProtocolRegistry().TryLastBeforeTimestamp("blockreward1",
+                                                                                    index->GetBlockTime());
+
     // The constant block reward is set to a default, voted on value, but this can
     // be overridden using an admin message. This allows us to change the reward
     // amount without having to release a mandatory with updated rules. In the case
-    // there is a breach or leaked admin keys the rewards are clamped to twice that
-    // of the default value.
-    const CAmount MIN_CBR = 0;
-    const CAmount MAX_CBR = DEFAULT_CBR * 2;
+    // there is a breach or leaked admin keys the rewards are clamped.
 
-    CAmount reward = DEFAULT_CBR;
-    AppCacheEntry oCBReward = GetProtocolRegistry().GetProtocolEntryByKeyLegacy("blockreward1");
+    // Note that the use of the IsV13Enabled test here on the block index does not consider the possibility of
+    // an administrative contract entry prior to v13 that has aged out from the lookback window which could affect consensus.
+    // However this possibility is actually covered because 5.4.8.0 was actually a mandatory due to a problem with
+    // message contracts, and this included the registry for protocol entries, eliminating the lookback window limitation.
+    // There has been no administrative entry to change the CBR in mainnet prior to 5.4.8.0, so in fact this is a moot issue.
+    if (IsV13Enabled(index->nHeight)) {
+        // The default and the clamp for block v13+ is controlled by blockchain consensus parameters.
+        reward = Params().GetConsensus().DefaultConstantBlockReward;
+        MIN_CBR = Params().GetConsensus().ConstantBlockRewardFloor;
+        MAX_CBR = Params().GetConsensus().ConstantBlockRewardCeiling;
+
+        if (reward_entry != nullptr && reward_entry->m_status == ProtocolEntryStatus::ACTIVE) {
+            // There is no contract lookback limitation v13+.
+            if (!ParseInt64(reward_entry->m_value, &reward)) {
+                error("%s: Cannot parse constant block reward from protocol entry: %s",
+                      __func__, reward_entry->m_value);
+            }
+        }
+    } else {
+        // This is the default and clamp pre block v13. Note that an administrative entry to change the CBR prior to block v13
+        // that is outside the pre v13 clamp rules could cause the CBR to change at the v13 height as the new clamp rules above
+        // would then apply on the existing protocol entry.
+        reward = 10 * COIN;
+        MIN_CBR = 0;
+        MAX_CBR = Params().GetConsensus().DefaultConstantBlockReward * 2;
 
-    //TODO: refactor the expire checking to subroutine
-    //Note: time constant is same as GetBeaconPublicKey
-    if ((index->nTime - oCBReward.timestamp) <= (60 * 24 * 30 * 6 * 60)) {
-        // This is a little slippery, because if we ever change CAmount from a int64_t, this will
-        // break. It is unlikely to ever change, however, and avoids an extra copy/implicit cast.
-        if (!ParseInt64(oCBReward.value, &reward)) {
-            error("%s: Cannot parse constant block reward from protocol entry: %s",
-                  __func__, oCBReward.value);
+        if (reward_entry != nullptr && reward_entry->m_status == ProtocolEntryStatus::ACTIVE) {
+            // The contract lookback window here for block version less than v13 is the same as the standard contract
+            // replay lookback.
+            if (index->nTime - reward_entry->m_timestamp <= Params().GetConsensus().StandardContractReplayLookback) {
+                // This is a little slippery, because if we ever change CAmount from a int64_t, this will
+                // break. It is unlikely to ever change, however, and avoids an extra copy/implicit cast.
+                if (!ParseInt64(reward_entry->m_value, &reward)) {
+                    error("%s: Cannot parse constant block reward from protocol entry: %s",
+                          __func__, reward_entry->m_value);
+                }
+            }
         }
     }
 

src/main.h

@@ -47,8 +47,6 @@ class MRC;
 typedef std::optional<Claim> ClaimOption;
 }
 
-static const int64_t DEFAULT_CBR = 10 * COIN;
-
 /** Threshold for nLockTime: below this value it is interpreted as block number, otherwise as UNIX timestamp. */
 static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov  5 00:53:20 1985 UTC
 

src/test/gridcoin_tests.cpp

@@ -18,53 +18,53 @@ extern bool fTestNet;
 extern leveldb::DB *txdb;
 
 namespace {
-   // Arbitrary random characters generated with Python UUID.
-   const std::string TEST_CPID("17c65330c0924259b2f93c31d25b03ac");
-
-   struct GridcoinTestsConfig
-   {
-      GridcoinTestsConfig()
-      {
-      }
-
-      ~GridcoinTestsConfig()
-      {
-      }
-   };
-
-   void AddProtocolEntry(const uint32_t& payload_version, const std::string& key, const std::string& value,
-                         const CBlockIndex index, const bool& reset_registry = false)
-   {
-       GRC::ProtocolRegistry& registry = GRC::GetProtocolRegistry();
-
-       // Make sure the registry is reset.
-       if (reset_registry) registry.Reset();
-
-       CTransaction dummy_tx;
-       dummy_tx.nTime = index.nTime;
-
-       GRC::Contract contract;
-
-       if (payload_version < 2) {
-           contract = GRC::MakeContract<GRC::ProtocolEntryPayload>(
-                       uint32_t {2}, // Contract version (pre v13)
-                       GRC::ContractAction::ADD,
-                       key,
-                       value);
-       } else {
-           contract = GRC::MakeContract<GRC::ProtocolEntryPayload>(
-                       uint32_t {3}, // Contract version (post v13)
-                       GRC::ContractAction::ADD,
-                       payload_version, // Protocol payload version (post v13)
-                       key,
-                       value,
-                       GRC::ProtocolEntryStatus::ACTIVE);
-       }
-
-       dummy_tx.vContracts.push_back(contract);
-
-       registry.Add({contract, dummy_tx, &index});
-   }
+// Arbitrary random characters generated with Python UUID.
+const std::string TEST_CPID("17c65330c0924259b2f93c31d25b03ac");
+
+struct GridcoinTestsConfig
+{
+    GridcoinTestsConfig()
+    {
+    }
+
+    ~GridcoinTestsConfig()
+    {
+    }
+};
+
+void AddProtocolEntry(const uint32_t& payload_version, const std::string& key, const std::string& value,
+                      const CBlockIndex index, const bool& reset_registry = false)
+{
+    GRC::ProtocolRegistry& registry = GRC::GetProtocolRegistry();
+
+           // Make sure the registry is reset.
+    if (reset_registry) registry.Reset();
+
+    CTransaction dummy_tx;
+    dummy_tx.nTime = index.nTime;
+
+    GRC::Contract contract;
+
+    if (payload_version < 2) {
+        contract = GRC::MakeContract<GRC::ProtocolEntryPayload>(
+            uint32_t {2}, // Contract version (pre v13)
+            GRC::ContractAction::ADD,
+            key,
+            value);
+    } else {
+        contract = GRC::MakeContract<GRC::ProtocolEntryPayload>(
+            uint32_t {3}, // Contract version (post v13)
+            GRC::ContractAction::ADD,
+            payload_version, // Protocol payload version (post v13)
+            key,
+            value,
+            GRC::ProtocolEntryStatus::ACTIVE);
+    }
+
+    dummy_tx.vContracts.push_back(contract);
+
+    registry.Add({contract, dummy_tx, &index});
+}
 
 } // anonymous namespace
 
@@ -105,14 +105,30 @@ BOOST_AUTO_TEST_SUITE_END()
 
 BOOST_AUTO_TEST_SUITE(gridcoin_cbr_tests)
 
-BOOST_AUTO_TEST_CASE(gridcoin_DefaultCBRShouldBe10)
+BOOST_AUTO_TEST_CASE(gridcoin_DefaultCBRShouldBe10PreV13)
+{
+    CBlockIndex index;
+    index.nTime = 1538066417;
+
+    auto& registry = GRC::GetProtocolRegistry();
+    registry.Reset();
+
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), 10 * COIN);
+}
+
+BOOST_AUTO_TEST_CASE(gridcoin_DefaultCBRShouldBeDefaultConstantBlockRewardForV13)
 {
     CBlockIndex index;
     index.nTime = 1538066417;
+    index.nHeight = Params().GetConsensus().BlockV13Height;
+
+    auto& registry = GRC::GetProtocolRegistry();
+    registry.Reset();
 
-    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), DEFAULT_CBR);
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), Params().GetConsensus().DefaultConstantBlockReward);
 }
 
+
 // Note that payload versions 1 and 2 alternate here. This is to test the constructors and
 // the machinery to add to the registry. This alternating approach is not what would
 // happen on the real blockchain, but is tolerated by the Registry independent of
@@ -154,7 +170,7 @@ BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldOverrideDefault)
     CBlockIndex index_2;
     index_2.nVersion = 13;
     index_2.nTime = time + 1;
-    index_2.nHeight = 2;
+    index_2.nHeight = Params().GetConsensus().BlockV13Height;
 
     // Protocol payload version 2
 
@@ -176,16 +192,16 @@ BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldOverrideDefault)
     BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index_2), cbr);
 }
 
-BOOST_AUTO_TEST_CASE(gridcoin_NegativeCBRShouldClampTo0)
+BOOST_AUTO_TEST_CASE(gridcoin_NegativeCBRShouldClampTo0PreV13)
 {
     const int64_t time = 123456;
     CBlockIndex index;
-    index.nTime = time + 2;
+    index.nTime = time;
     index.nHeight = 3;
 
     auto& registry = GRC::GetProtocolRegistry();
 
-    AddProtocolEntry(1, "blockreward1", ToString(-1 * COIN), index, false);
+    AddProtocolEntry(1, "blockreward1", ToString(-1 * COIN), index, true);
 
     if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
         LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
@@ -203,16 +219,70 @@ BOOST_AUTO_TEST_CASE(gridcoin_NegativeCBRShouldClampTo0)
     BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), 0);
 }
 
-BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldClampTo2xDefault)
+BOOST_AUTO_TEST_CASE(gridcoin_NegativeCBRShouldClampToConstantBlockRewardFloorForV13)
 {
     const int64_t time = 123456;
     CBlockIndex index;
-    index.nTime = time + 3;
+    index.nTime = time;
+    index.nHeight = Params().GetConsensus().BlockV13Height;
+
+    auto& registry = GRC::GetProtocolRegistry();
+
+    AddProtocolEntry(2, "blockreward1", ToString(-1 * COIN), index, true);
+
+    if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
+        LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
+                                            "m_timestamp %" PRId64 ", m_status string %s",
+                 __func__,
+                 entry->m_key,
+                 entry->m_value,
+                 entry->m_hash.ToString(),
+                 entry->m_previous_hash.ToString(),
+                 entry->m_timestamp,
+                 entry->StatusToString()
+                 );
+    }
+
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), Params().GetConsensus().ConstantBlockRewardFloor);
+}
+
+BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldClampTo2xDefaultPreV13)
+{
+    const int64_t time = 123456;
+    CBlockIndex index;
+    index.nTime = time;
     index.nHeight = 4;
 
     auto& registry = GRC::GetProtocolRegistry();
 
-    AddProtocolEntry(2, "blockreward1", ToString(DEFAULT_CBR * 3), index, false);
+    AddProtocolEntry(2, "blockreward1", ToString(Params().GetConsensus().DefaultConstantBlockReward * 3), index, true);
+
+    if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
+        LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
+                                            "m_timestamp %" PRId64 ", m_status string %s",
+                 __func__,
+                 entry->m_key,
+                 entry->m_value,
+                 entry->m_hash.ToString(),
+                 entry->m_previous_hash.ToString(),
+                 entry->m_timestamp,
+                 entry->StatusToString()
+                 );
+    }
+
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), Params().GetConsensus().DefaultConstantBlockReward * 2);
+}
+
+BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldClampToConstantBlockRewardCeilingForV13)
+{
+    const int64_t time = 123456;
+    CBlockIndex index;
+    index.nTime = time;
+    index.nHeight = Params().GetConsensus().BlockV13Height;
+
+    auto& registry = GRC::GetProtocolRegistry();
+
+    AddProtocolEntry(2, "blockreward1", ToString(Params().GetConsensus().ConstantBlockRewardCeiling * 2), index, true);
 
     if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
         LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
@@ -227,12 +297,10 @@ BOOST_AUTO_TEST_CASE(gridcoin_ConfigurableCBRShouldClampTo2xDefault)
                  );
     }
 
-    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), DEFAULT_CBR * 2);
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index), Params().GetConsensus().ConstantBlockRewardCeiling);
 }
 
-// TODO: when the 180 day lookback is removed, this should be removed as a test as it will
-// be irrelevant and invalid.
-BOOST_AUTO_TEST_CASE(gridcoin_ObsoleteConfigurableCBRShouldResortToDefault)
+BOOST_AUTO_TEST_CASE(gridcoin_ObsoleteConfigurableCBRShouldResortToDefaultPreV13)
 {
     CBlockIndex index_check;
     index_check.nTime = 1538066417;
@@ -245,7 +313,7 @@ BOOST_AUTO_TEST_CASE(gridcoin_ObsoleteConfigurableCBRShouldResortToDefault)
 
     auto& registry = GRC::GetProtocolRegistry();
 
-    AddProtocolEntry(2, "blockreward1", ToString(3 * COIN), index_add, false);
+    AddProtocolEntry(1, "blockreward1", ToString(3 * COIN), index_add, true);
 
     if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
         LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
@@ -260,7 +328,39 @@ BOOST_AUTO_TEST_CASE(gridcoin_ObsoleteConfigurableCBRShouldResortToDefault)
                  );
     }
 
-    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index_check), DEFAULT_CBR);
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index_check), Params().GetConsensus().DefaultConstantBlockReward);
 }
 
+BOOST_AUTO_TEST_CASE(gridcoin_PriorObsoleteConfigurableCBRShouldNotResortToDefaultforV13)
+{
+    CBlockIndex index_check;
+    index_check.nTime = 1538066417;
+    index_check.nHeight = Params().GetConsensus().BlockV13Height;
+    const int64_t max_message_age = 60 * 60 * 24 * 180;
+
+    CBlockIndex index_add;
+    index_add.nTime = index_check.nTime - max_message_age - 1;
+    index_add.nHeight = 5;
+
+    auto& registry = GRC::GetProtocolRegistry();
+
+    AddProtocolEntry(2, "blockreward1", ToString(3 * COIN), index_add, true);
+
+    if (GRC::ProtocolEntryOption entry = registry.TryActive("blockreward1")) {
+        LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Protocol entry: m_key %s, m_value %s, m_hash %s, m_previous_hash %s, "
+                                            "m_timestamp %" PRId64 ", m_status string %s",
+                 __func__,
+                 entry->m_key,
+                 entry->m_value,
+                 entry->m_hash.ToString(),
+                 entry->m_previous_hash.ToString(),
+                 entry->m_timestamp,
+                 entry->StatusToString()
+                 );
+    }
+
+    BOOST_CHECK_EQUAL(GRC::GetConstantBlockReward(&index_check), 3 * COIN);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()