Merge pull request gridcoin-community#2731 from jamescowens/fix_beaco…

…n_history_stall

beacon, registry: Fix beacon history stall

src/chainparams.h

@@ -153,7 +153,7 @@ inline bool IsV12Enabled(int nHeight)
     return nHeight >= Params().GetConsensus().BlockV12Height;
 }
 
-inline bool IsV13Enabled(int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+inline bool IsV13Enabled(int nHeight)
 {
     // The argument driven override temporarily here to facilitate testing.
 

src/gridcoin/beacon.h

@@ -13,7 +13,6 @@
 #include "gridcoin/contract/registry_db.h"
 #include "gridcoin/cpid.h"
 #include "gridcoin/support/enumbytes.h"
-
 #include <memory>
 #include <string>
 #include <vector>
@@ -545,13 +544,10 @@ class BeaconRegistry : public IContractHandler
     //! Version 0: <= 5.2.0.0
     //! Version 1: = 5.2.1.0
     //! Version 2: 5.2.1.0 with hotfix and > 5.2.1.0
-    //!
-    //! The current version of the beacon db is 2. No changes to the underlying storage have
-    //! occurred during the refactor to the registry db template, so this version remains unchanged
-    //! through 5.4.2.0+
+    //! Version 3: 5.4.5.5+
     //!
     BeaconRegistry()
-        : m_beacon_db(2)
+        : m_beacon_db(3)
     {
     };
 
@@ -589,6 +585,12 @@ class BeaconRegistry : public IContractHandler
     //!
     const PendingBeaconMap& PendingBeacons() const;
 
+    //!
+    //! \brief Get the set of beacons that have expired while pending (awaiting verification)
+    //! \return A reference to the expired pending beacon set.
+    //!
+    const std::set<Beacon_ptr>& ExpiredBeacons() const;
+
     //!
     //! \brief Get the beacon for the specified CPID.
     //!
@@ -768,6 +770,18 @@ class BeaconRegistry : public IContractHandler
     //!
     uint64_t PassivateDB();
 
+    //!
+    //! \brief This function walks the linked beacon entries back (using the m_previous_hash member) from a provided
+    //! beacon to find the initial advertisement. Note that this does NOT traverse non-continuous beacon ownership,
+    //! which occurs when a beacon is allowed to expire and must be reverified under a new key.
+    //!
+    //! \param beacon smart shared pointer to beacon entry to begin walking back
+    //! \param beacon_chain_out shared pointer to UniValue beacon chain out report array
+    //! \return root (advertisement) beacon entry smart shared pointer
+    //!
+    Beacon_ptr GetBeaconChainletRoot(Beacon_ptr beacon,
+                                     std::shared_ptr<std::vector<std::pair<uint256, int64_t>>> beacon_chain_out = nullptr);
+
     //!
     //! \brief Returns whether IsContract correction is needed in ReplayContracts during initialization
     //! \return
@@ -794,6 +808,7 @@ class BeaconRegistry : public IContractHandler
                        BeaconStatusForStorage,
                        BeaconMap,
                        PendingBeaconMap,
+                       std::set<Beacon_ptr>,
                        HistoricalBeaconMap> BeaconDB;
 
 private:
@@ -805,6 +820,22 @@ class BeaconRegistry : public IContractHandler
     BeaconMap m_beacons;        //!< Contains the active registered beacons.
     PendingBeaconMap m_pending; //!< Contains beacons awaiting verification.
 
+    //!
+    //! \brief Contains pending beacons that have expired.
+    //!
+    //! Contains pending beacons that have expired but need to be retained until the next SB (activation) to ensure a
+    //! reorganization will successfully resurrect expired pending beacons back into pending ones up to the depth equal to one SB to
+    //! the next, which is about 960 blocks. The reason this is necessary is two fold: 1) it makes the lookup for expired
+    //! pending beacons in the deactivate method much simpler in the case of a reorg across a SB boundary, and 2) it holds
+    //! a reference to the pending beacon shared pointer object in the history map, which prevents it from being passivated.
+    //! Otherwise, a passivation event, which would remove the pending deleted beacons, followed by a reorganization across
+    //! SB boundary could have a small possibility of removing a pending beacon that could be verified in the alternative SB
+    //! eventually staked.
+    //!
+    //! This set is cleared and repopulated at each SB accepted by the node with the current expired pending beacons.
+    //!
+    std::set<Beacon_ptr> m_expired_pending;
+
     //!
     //! \brief The member variable that is the instance of the beacon database. This is private to the
     //! beacon registry and is only accessible by beacon registry functions.

src/gridcoin/contract/registry_db.h

@@ -26,9 +26,11 @@ namespace GRC {
 //! M:  the map type for the entries
 //! P:  the map type for pending entries. This is really only used for beacons. In all other registries it is typedef'd to
 //!     the same as M.
+//! X:  the map type for expired pending entries. This is really only used for beacons. In all other registries it is typedef'd to
+//!     the same as M.
 //! H:  the historical map type for historical entries
 //!
-template<class E, class SE, class S, class M, class P, class H>
+template<class E, class SE, class S, class M, class P, class X, class H>
 class RegistryDB
 {
 public:
@@ -62,10 +64,12 @@ class RegistryDB
     //! \param entries The map of current entries.
     //! \param pending_entries. The map of pending entries. This is not used in the general template, only in the beacons
     //! specialization.
+    //! \param expired_entries. The map of expired pending entries. This is not used in the geenral template, only in the
+    //! beacons specialization.
     //!
     //! \return block height up to and including which the entry records were stored.
     //!
-    int Initialize(M& entries, P& pending_entries)
+    int Initialize(M& entries, P& pending_entries, X& expired_entries)
     {
         bool status = true;
         int height = 0;
@@ -169,7 +173,7 @@ class RegistryDB
             m_historical[iter.second.m_hash] = std::make_shared<E>(entry);
             entry_ptr& historical_entry_ptr = m_historical[iter.second.m_hash];
 
-            HandleCurrentHistoricalEntries(entries, pending_entries, entry,
+            HandleCurrentHistoricalEntries(entries, pending_entries, expired_entries, entry,
                                                 historical_entry_ptr, recnum, key_type);
 
             number_passivated += (uint64_t) HandlePreviousHistoricalEntries(historical_entry_ptr);
@@ -199,7 +203,7 @@ class RegistryDB
     //! \param recnum
     //! \param key_type
     //!
-    void HandleCurrentHistoricalEntries(M& entries, P& pending_entries, const E& entry,
+    void HandleCurrentHistoricalEntries(M& entries, P& pending_entries, X& expired_entries, const E& entry,
                                         entry_ptr& historical_entry_ptr, const uint64_t& recnum,
                                         const std::string& key_type)
     {
@@ -326,8 +330,8 @@ class RegistryDB
 
         LogPrint(BCLog::LogFlags::CONTRACT, "INFO: %s: Passivated %" PRId64 " elements from %s entry db.",
                  __func__,
-                 KeyType(),
-                 number_passivated);
+                 number_passivated,
+                 KeyType());
 
         // Set needs passivation flag to false after passivating the db.
         m_needs_passivation = false;

src/gridcoin/project.cpp

@@ -495,7 +495,7 @@ int Whitelist::Initialize()
 {
     LOCK(cs_lock);
 
-    int height = m_project_db.Initialize(m_project_entries, m_pending_project_entries);
+    int height = m_project_db.Initialize(m_project_entries, m_pending_project_entries, m_expired_project_entries);
 
     LogPrint(LogFlags::CONTRACT, "INFO: %s: m_project_db size after load: %u", __func__, m_project_db.size());
     LogPrint(LogFlags::CONTRACT, "INFO: %s: m_project_entries size after load: %u", __func__, m_project_entries.size());

src/gridcoin/project.h

@@ -618,13 +618,15 @@ class Whitelist : public IContractHandler
     static void RunDBPassivation();
 
     //!
-    //! \brief Specializes the template RegistryDB for the ScraperEntry class
+    //! \brief Specializes the template RegistryDB for the ScraperEntry class. Note that std::set<ProjectEntry> is not
+    //! actually used.
     //!
     typedef RegistryDB<ProjectEntry,
                        ProjectEntry,
                        ProjectEntryStatus,
                        ProjectEntryMap,
                        PendingProjectEntryMap,
+                       std::set<ProjectEntry>,
                        HistoricalProjectEntryMap> ProjectEntryDB;
 
 private:
@@ -644,6 +646,8 @@ class Whitelist : public IContractHandler
     ProjectEntryMap m_project_entries;                   //!< The set of whitelisted projects.
     PendingProjectEntryMap m_pending_project_entries {}; //!< Not actually used. Only to satisfy the template.
 
+    std::set<ProjectEntry> m_expired_project_entries {}; //!< Not actually used. Only to satisfy the template.
+
     ProjectEntryDB m_project_db; //!< The project db member
 public:
 

src/gridcoin/protocol.cpp

@@ -482,7 +482,7 @@ int ProtocolRegistry::Initialize()
 {
     LOCK(cs_lock);
 
-    int height = m_protocol_db.Initialize(m_protocol_entries, m_pending_protocol_entries);
+    int height = m_protocol_db.Initialize(m_protocol_entries, m_pending_protocol_entries, m_expired_protocol_entries);
 
     LogPrint(LogFlags::CONTRACT, "INFO: %s: m_protocol_db size after load: %u", __func__, m_protocol_db.size());
     LogPrint(LogFlags::CONTRACT, "INFO: %s: m_protocol_entries size after load: %u", __func__, m_protocol_entries.size());

src/gridcoin/protocol.h

@@ -567,13 +567,15 @@ class ProtocolRegistry : public IContractHandler
     static void RunDBPassivation();
 
     //!
-    //! \brief Specializes the template RegistryDB for the ProtocolEntry class
+    //! \brief Specializes the template RegistryDB for the ProtocolEntry class. Note that std::set<ProtocolEntry>
+    //! is not actually used.
     //!
     typedef RegistryDB<ProtocolEntry,
                        ProtocolEntry,
                        ProtocolEntryStatus,
                        ProtocolEntryMap,
                        PendingProtocolEntryMap,
+                       std::set<ProtocolEntry>,
                        HistoricalProtocolEntryMap> ProtocolEntryDB;
 
 private:
@@ -593,6 +595,8 @@ class ProtocolRegistry : public IContractHandler
     ProtocolEntryMap m_protocol_entries;                   //!< Contains the current protocol entries including entries marked DELETED.
     PendingProtocolEntryMap m_pending_protocol_entries {}; //!< Not used. Only to satisfy the template.
 
+    std::set<ProtocolEntry> m_expired_protocol_entries {}; //!< Not used. Only to satisfy the template.
+
     ProtocolEntryDB m_protocol_db;
 
 public:

src/gridcoin/scraper/scraper_registry.cpp

@@ -527,7 +527,7 @@ int ScraperRegistry::Initialize()
 {
     LOCK(cs_lock);
 
-    int height = m_scraper_db.Initialize(m_scrapers, m_pending_scrapers);
+    int height = m_scraper_db.Initialize(m_scrapers, m_pending_scrapers, m_expired_scraper_entries);
 
     LogPrint(LogFlags::SCRAPER, "INFO: %s: m_scraper_db size after load: %u", __func__, m_scraper_db.size());
     LogPrint(LogFlags::SCRAPER, "INFO: %s: m_scrapers size after load: %u", __func__, m_scrapers.size());

src/gridcoin/scraper/scraper_registry.h

@@ -606,13 +606,15 @@ class ScraperRegistry : public IContractHandler
     static void RunDBPassivation();
 
     //!
-    //! \brief Specializes the template RegistryDB for the ScraperEntry class
+    //! \brief Specializes the template RegistryDB for the ScraperEntry class. Note that std::set<ScraperEntry> is
+    //! not actually used.
     //!
     typedef RegistryDB<ScraperEntry,
                        ScraperEntry,
                        ScraperEntryStatus,
                        ScraperMap,
                        PendingScraperMap,
+                       std::set<ScraperEntry>,
                        HistoricalScraperMap> ScraperEntryDB;
 
 private:
@@ -632,6 +634,8 @@ class ScraperRegistry : public IContractHandler
     ScraperMap m_scrapers;                   //!< Contains the current scraper entries, including entries marked DELETED.
     PendingScraperMap m_pending_scrapers {}; //!< Not actually used for scrapers. To satisfy the template only.
 
+    std::set<ScraperEntry> m_expired_scraper_entries {}; //!< Not actually used for scrapers. To satisfy the template only.
+
     ScraperEntryDB m_scraper_db;
 
 public:

src/gridcoin/sidestake.cpp

@@ -712,7 +712,7 @@ int SideStakeRegistry::Initialize()
 {
     LOCK(cs_lock);
 
-    int height = m_sidestake_db.Initialize(m_mandatory_sidestake_entries, m_pending_sidestake_entries);
+    int height = m_sidestake_db.Initialize(m_mandatory_sidestake_entries, m_pending_sidestake_entries, m_expired_sidestake_entries);
 
     SubscribeToCoreSignals();
 

src/gridcoin/sidestake.h

@@ -783,13 +783,15 @@ class SideStakeRegistry : public IContractHandler
     static void RunDBPassivation();
 
     //!
-    //! \brief Specializes the template RegistryDB for the SideStake class
+    //! \brief Specializes the template RegistryDB for the SideStake class. Note that std::set<MandatorySideStake>
+    //! is not actually used.
     //!
     typedef RegistryDB<MandatorySideStake,
                        MandatorySideStake,
                        MandatorySideStake::MandatorySideStakeStatus,
                        MandatorySideStakeMap,
                        PendingSideStakeMap,
+                       std::set<SideStake>,
                        HistoricalSideStakeMap> SideStakeDB;
 
 private:
@@ -827,6 +829,8 @@ class SideStakeRegistry : public IContractHandler
     MandatorySideStakeMap m_mandatory_sidestake_entries;  //!< Contains the mandatory sidestake entries, including DELETED.
     PendingSideStakeMap m_pending_sidestake_entries {};   //!< Not used. Only to satisfy the template.
 
+    std::set<SideStake> m_expired_sidestake_entries {};   //!< Not used. Only to satisfy the template.
+
     SideStakeDB m_sidestake_db;                           //!< The internal sidestake db object for leveldb persistence.
 
     bool m_local_entry_already_saved_to_config = false;   //!< Flag to prevent reload on signal if individual entry saved already.

src/gridcoin/tally.cpp

@@ -1228,14 +1228,15 @@ CAmount Tally::GetNewbieSuperblockAccrualCorrection(const Cpid& cpid, const Supe
         return accrual;
     }
 
-    Beacon_ptr beacon_ptr = beacon;
+    Beacon_ptr beacon_ptr;
 
     // Walk back the entries in the historical beacon map linked by renewal prev tx hash until the first
     // beacon in the renewal chain is found (the original advertisement). The accrual starts no earlier
     // than here.
-    while (beacon_ptr->Renewed())
-    {
-        beacon_ptr = beacons.GetBeaconDB().find(beacon_ptr->m_previous_hash)->second;
+    try {
+        beacon_ptr = beacons.GetBeaconChainletRoot(beacon);
+    } catch (std::runtime_error& e) {
+        std::abort();
     }
 
     const CBlockIndex* pindex_baseline = GRC::Tally::GetBaseline();

src/qt/mrcmodel.cpp

@@ -255,8 +255,6 @@ void MRCModel::refresh() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 
     // This is similar to createmrcrequest in many ways, but the state tracking is more complicated.
 
-    LOCK(cs_main);
-
     // Record initial block height during init run.
     if (!m_init_block_height) {
         m_init_block_height = pindexBest->nHeight;

src/rpc/mining.cpp

@@ -300,47 +300,29 @@ UniValue auditsnapshotaccrual(const UniValue& params, bool fHelp)
 
     GRC::Beacon_ptr beacon_ptr = beacon_try;
 
-    LogPrint(BCLog::LogFlags::ACCRUAL, "INFO %s: active beacon: timestamp = %" PRId64 ", ctx_hash = %s,"
-                                       " prev_beacon_ctx_hash = %s",
-             __func__,
-             beacon_ptr->m_timestamp,
-             beacon_ptr->m_hash.GetHex(),
-             beacon_ptr->m_previous_hash.GetHex());
+    std::vector<std::pair<uint256, int64_t>> beacon_chain_out {};
+
+    std::shared_ptr<std::vector<std::pair<uint256, int64_t>>> beacon_chain_out_ptr
+        = std::make_shared<std::vector<std::pair<uint256, int64_t>>>(beacon_chain_out);
 
     UniValue beacon_chain(UniValue::VARR);
-    UniValue beacon_chain_entry(UniValue::VOBJ);
-
-    beacon_chain_entry.pushKV("ctx_hash", beacon_ptr->m_hash.GetHex());
-    beacon_chain_entry.pushKV("timestamp",  beacon_ptr->m_timestamp);
-    beacon_chain.push_back(beacon_chain_entry);
-
-    // This walks back the entries in the historical beacon map linked by renewal prev tx hash until the first
-    // beacon in the renewal chain is found (the original advertisement). The accrual starts no earlier than here.
-    uint64_t renewals = 0;
-    // The renewals <= 100 is simply to prevent an infinite loop if there is a problem with the beacon chain in the registry. This
-    // was an issue in post Fern beacon db work, but has been resolved and not encountered since. Still makes sense to leave the
-    // limit in, which represents 41 years worth of beacon chain at the 150 day standard auto-renewal cycle.
-    while (beacon_ptr->Renewed() && renewals <= 100)
-    {
-        auto iter = beacons.GetBeaconDB().find(beacon_ptr->m_previous_hash);
 
-        beacon_ptr = iter->second;
+    try {
+        beacon_ptr = beacons.GetBeaconChainletRoot(beacon_ptr, beacon_chain_out_ptr);
+    } catch (std::runtime_error& e) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, e.what());
+    }
 
-        LogPrint(BCLog::LogFlags::ACCRUAL, "INFO %s: renewal %u beacon: timestamp = %" PRId64 ", ctx_hash = %s,"
-                                           " prev_beacon_ctx_hash = %s.",
-                 __func__,
-                 renewals,
-                 beacon_ptr->m_timestamp,
-                 beacon_ptr->m_hash.GetHex(),
-                 beacon_ptr->m_previous_hash.GetHex());
+    for (const auto& iter : *beacon_chain_out_ptr) {
+        UniValue beacon_chain_entry(UniValue::VOBJ);
 
-        beacon_chain_entry.pushKV("ctx_hash", beacon_ptr->m_hash.GetHex());
-        beacon_chain_entry.pushKV("timestamp", beacon_ptr->m_timestamp);
+        beacon_chain_entry.pushKV("ctx_hash", iter.first.GetHex());
+        beacon_chain_entry.pushKV("timestamp",  iter.second);
         beacon_chain.push_back(beacon_chain_entry);
-
-        ++renewals;
     }
 
+    int64_t renewals = beacon_chain_out_ptr->size() - 1;
+
     bool retry_from_baseline = false;
 
     // Up to two passes. The first is from the start of the current beacon chain for the CPID, the second from the Fern baseline.