Merge pull request #2008 from jamescowens/beacon_storage

beacon: Change beacon map to pointers.

Author: jamescowens

src/gridcoin/beacon.cpp

@@ -46,16 +46,15 @@ uint256 HashBeaconPayload(const BeaconPayload& payload)
 //!
 bool TryRenewal(BeaconRegistry::BeaconMap& beacons,
                 BeaconRegistry::HistoricalBeaconMap& historical_beacons,
-                const BeaconPayload& payload,
-                const uint256& ctx_hash)
+                const BeaconPayload& payload)
 {
     auto beacon_pair_iter = beacons.find(payload.m_cpid);
 
     if (beacon_pair_iter == beacons.end()) {
         return false;
     }
 
-    Beacon& current_beacon = beacon_pair_iter->second;
+    Beacon& current_beacon = *beacon_pair_iter->second;
 
     if (current_beacon.Expired(payload.m_beacon.m_timestamp)) {
         return false;
@@ -65,12 +64,15 @@ bool TryRenewal(BeaconRegistry::BeaconMap& beacons,
         return false;
     }
 
-    // Insert current beacon into historical map.
-    historical_beacons.emplace(std::make_pair(ctx_hash, current_beacon));
+    // Insert copy of current beacon into historical map.
+    historical_beacons.emplace(std::make_pair(current_beacon.m_ctx_hash, current_beacon));
 
-    // Update current beacon record.
+    // Set the previous transaction hash to the current.
+    current_beacon.m_prev_beacon_ctx_hash = current_beacon.m_ctx_hash;
+    // Update current beacon record timestamp to the incoming payload transaction hash.
     current_beacon.m_timestamp = payload.m_beacon.m_timestamp;
-    current_beacon.m_prev_beacon_txn_hash = ctx_hash;
+    // Update the current transaction hash to the incoming payload transaction hash.
+    current_beacon.m_ctx_hash = payload.m_beacon.m_ctx_hash;
 
     return true;
 }
@@ -89,19 +91,20 @@ BeaconRegistry& GRC::GetBeaconRegistry()
 // Class: Beacon
 // -----------------------------------------------------------------------------
 
-Beacon::Beacon() : m_public_key(), m_timestamp(0)
+Beacon::Beacon() : m_public_key(), m_timestamp(0), m_ctx_hash()
 {
 }
 
 Beacon::Beacon(CPubKey public_key)
-    : Beacon(std::move(public_key), 0)
+    : Beacon(std::move(public_key), 0, uint256 {})
 {
 }
 
-Beacon::Beacon(CPubKey public_key, int64_t timestamp)
+Beacon::Beacon(CPubKey public_key, int64_t timestamp, uint256 hash)
     : m_public_key(std::move(public_key))
-    , m_timestamp(timestamp)
-    , m_prev_beacon_txn_hash()
+    , m_timestamp(timestamp),
+      m_ctx_hash(hash)
+    , m_prev_beacon_ctx_hash()
 {
 }
 
@@ -169,7 +172,7 @@ bool Beacon::Renewable(const int64_t now) const
 bool Beacon::Renewed() const
 {
     // If not empty, the beacon was a renewal.
-    return (m_prev_beacon_txn_hash != uint256());
+    return (m_prev_beacon_ctx_hash != uint256());
 }
 
 CKeyID Beacon::GetId() const
@@ -304,7 +307,7 @@ BeaconOption BeaconRegistry::Try(const Cpid& cpid) const
         return nullptr;
     }
 
-    return &iter->second;
+    return iter->second;
 }
 
 BeaconOption BeaconRegistry::TryActive(const Cpid& cpid, const int64_t now) const
@@ -363,18 +366,21 @@ void BeaconRegistry::Add(const ContractContext& ctx)
 {
     BeaconPayload payload = ctx->CopyPayloadAs<BeaconPayload>();
     payload.m_beacon.m_timestamp = ctx.m_tx.nTime;
+    payload.m_beacon.m_ctx_hash = ctx.m_tx.GetHash();
+    payload.m_beacon.m_prev_beacon_ctx_hash = uint256 {};
 
     // Legacy beacon contracts before block version 11--just load the beacon:
     //
     if (ctx->m_version == 1) {
-        m_beacons[payload.m_cpid] = std::move(payload.m_beacon);
+        m_historical[ctx.m_tx.GetHash()] = std::move(payload.m_beacon);
+        m_beacons[payload.m_cpid] = std::make_shared<Beacon>(m_historical[ctx.m_tx.GetHash()]);
         return;
     }
 
     // For beacon renewals, check that the new beacon contains the same public
     // key. If it matches, we don't need to verify it again:
     //
-    if (TryRenewal(m_beacons, m_historical, payload, ctx.m_tx.GetHash())) {
+    if (TryRenewal(m_beacons, m_historical, payload)) {
         return;
     }
 
@@ -384,6 +390,8 @@ void BeaconRegistry::Add(const ContractContext& ctx)
     PendingBeacon pending(payload.m_cpid, std::move(payload.m_beacon));
 
     m_pending.emplace(pending.GetId(), std::move(pending));
+
+    // Note that pending beacons do not get added to the historical map.
 }
 
 void BeaconRegistry::Delete(const ContractContext& ctx)
@@ -395,6 +403,7 @@ void BeaconRegistry::Delete(const ContractContext& ctx)
     }
 
     m_beacons.erase(payload->m_cpid);
+    m_historical.erase(ctx.m_tx.GetHash());
 }
 
 bool BeaconRegistry::Validate(const Contract& contract, const CTransaction& tx) const
@@ -466,15 +475,18 @@ void BeaconRegistry::ActivatePending(
     const std::vector<uint160>& beacon_ids,
     const int64_t superblock_time)
 {
+    // Activate the pending beacons that are not expired with respect to pending age.
     for (const auto& id : beacon_ids) {
         auto iter_pair = m_pending.find(id);
 
         if (iter_pair != m_pending.end()) {
-            m_beacons[iter_pair->second.m_cpid] = std::move(iter_pair->second);
+            m_historical.emplace(std::make_pair(iter_pair->second.m_ctx_hash, iter_pair->second));
+            m_beacons[iter_pair->second.m_cpid] = std::make_shared<Beacon>(m_historical[iter_pair->second.m_ctx_hash]);
             m_pending.erase(iter_pair);
         }
     }
 
+    // Discard pending beacons that are expired with respect to pending age.
     for (auto iter = m_pending.begin(); iter != m_pending.end(); /* no-op */) {
         if (iter->second.Expired(superblock_time)) {
             iter = m_pending.erase(iter);
@@ -487,10 +499,10 @@ void BeaconRegistry::ActivatePending(
 void BeaconRegistry::Deactivate(const int64_t superblock_time)
 {
     for (auto iter = m_beacons.begin(); iter != m_beacons.end(); /* no-op */) {
-        if (iter->second.m_timestamp >= superblock_time) {
-            PendingBeacon pending(iter->first, std::move(iter->second));
+        if (iter->second->m_timestamp >= superblock_time) {
+            PendingBeacon pending(iter->first, *iter->second);
             m_pending.emplace(pending.GetId(), std::move(pending));
-
+            m_historical.erase(iter->second->m_ctx_hash);
             iter = m_beacons.erase(iter);
         } else {
             ++iter;

src/gridcoin/beacon.h

@@ -11,6 +11,7 @@
 #include "gridcoin/cpid.h"
 #include "serialize.h"
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -66,14 +67,16 @@ class Beacon
     static constexpr int64_t RENEWAL_AGE = 60 * 60 * 24 * 30 * 5;
 
     CPubKey m_public_key;      //!< Verifies blocks that claim research rewards.
-    int64_t m_timestamp;       //!< Time of the the beacon contract transaction.
+    int64_t m_timestamp;       //!< Time of the beacon contract transaction.
+
+    uint256 m_ctx_hash;        //!< The hash of the transaction that advertised the beacon.
 
     // The reason the transaction hash is included here instead of a pointer
     // to the previous beacon, is that given the limited lookback scope, the
     // previous beacon may not be in the map. This makes the linked list harder
     // to deal with, but relieves us of dealing with the leading edge of the
     // lookback window.
-    uint256 m_prev_beacon_txn_hash; //!< If a renewal contains the txn hash of the previous beacon.
+    uint256 m_prev_beacon_ctx_hash; //!< If a renewal contains the txn hash of the previous beacon.
 
     //!
     //! \brief Initialize an empty, invalid beacon instance.
@@ -93,7 +96,7 @@ class Beacon
     //! \param public_key   Used to verify blocks that claim research rewards.
     //! \param tx_timestamp Time of the transaction with the beacon contract.
     //!
-    Beacon(CPubKey public_key, int64_t tx_timestamp);
+    Beacon(CPubKey public_key, int64_t tx_timestamp, uint256 hash);
 
     //!
     //! \brief Initialize a beacon instance from a contract that contains beacon
@@ -209,9 +212,14 @@ class Beacon
 };
 
 //!
-//! \brief A type that either points to to some beacon or does not.
+//! \brief The type that defines a shared pointer to a Beacon
+//!
+typedef std::shared_ptr<Beacon> Beacon_ptr;
+
+//!
+//! \brief A type that either points to some beacon or does not.
 //!
-typedef const Beacon* BeaconOption;
+typedef const Beacon_ptr BeaconOption;
 
 //!
 //! \brief The body of a beacon contract advertised in a transaction.
@@ -393,6 +401,8 @@ class PendingBeacon : public Beacon
 
     Cpid m_cpid; // Identifies the researcher that advertised the beacon.
 
+    // There is no previous transaction hash here because renewed beacons are never
+    // pending.
     //!
     //! \brief Initialize a pending beacon.
     //!
@@ -421,7 +431,7 @@ class BeaconRegistry : public IContractHandler
     //!
     //! \brief The type that associates beacons with CPIDs in the registry.
     //!
-    typedef std::unordered_map<Cpid, Beacon> BeaconMap;
+    typedef std::unordered_map<Cpid, Beacon_ptr> BeaconMap;
 
     //!
     //! \brief The type that associates historical beacons with the ctx hash.

src/gridcoin/scraper/scraper.cpp

@@ -204,7 +204,7 @@ const CBlockIndex* GetBeaconConsensusHeight()
 BeaconConsensus GetConsensusBeaconList()
 {
     BeaconConsensus consensus;
-    std::vector<std::pair<Cpid, Beacon>> beacons;
+    std::vector<std::pair<Cpid, Beacon_ptr>> beacon_ptrs;
     std::vector<std::pair<CKeyID, PendingBeacon>> pending_beacons;
     int64_t max_time;
 
@@ -223,16 +223,16 @@ BeaconConsensus GetConsensusBeaconList()
         // Copy the set of beacons out of the registry so we can release the
         // lock on cs_main before stringifying them:
         //
-        beacons.reserve(beacon_map.size());
-        beacons.assign(beacon_map.begin(), beacon_map.end());
+        beacon_ptrs.reserve(beacon_map.size());
+        beacon_ptrs.assign(beacon_map.begin(), beacon_map.end());
         pending_beacons.reserve(pending_beacon_map.size());
         pending_beacons.assign(pending_beacon_map.begin(), pending_beacon_map.end());
     }
 
-    for (const auto& beacon_pair : beacons)
+    for (const auto& beacon_pair : beacon_ptrs)
     {
         const Cpid& cpid = beacon_pair.first;
-        const Beacon& beacon = beacon_pair.second;
+        const Beacon& beacon = *beacon_pair.second;
 
         if (beacon.Expired(max_time) || beacon.m_timestamp >= max_time)
         {
@@ -3926,8 +3926,8 @@ bool IsScraperMaximumManifestPublishingRateExceeded(int64_t& nTime, CPubKey& Pub
     nAvgTimeBetweenManifests = nTotalTime / nIntervals;
 
     // nScraperSleep is in milliseconds. If the average interval is less than 25% of nScraperSleep in seconds, ban the scraper.
-    // Note that this is at least a factor of 12 faster than the expected rate given usual project update velocity.
-    if (nAvgTimeBetweenManifests < nScraperSleep / 4000)
+    // Note that this is a factor of 24 faster than the expected rate given usual project update velocity.
+    if (nAvgTimeBetweenManifests < nScraperSleep / 8000)
     {
         _log(logattribute::CRITICAL, "IsScraperMaximumManifestPublishingRateExceeded", "Scraper " + sManifestAddress +
              " has published too many manifests in too short a time:\n" +

src/gridcoin/tally.cpp

@@ -1034,25 +1034,25 @@ CAmount Tally::GetNewbieSuperblockAccrualCorrection(const Cpid& cpid, const Supe
 
     LogPrint(BCLog::LogFlags::ACCRUAL, "INFO %s: beacon registry size = %u", __func__, beacons.Beacons().size());
 
-    if (beacon)
+    // Bail if there is no active beacon.
+    if (!beacon)
     {
-        // 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->Renewed())
-        {
-            beacon = &beacons.HistoricalBeacons().find(beacon->m_prev_beacon_txn_hash)->second;
-        }
-    }
-    else
-    {
-
         LogPrint(BCLog::LogFlags::ACCRUAL, "ERROR: %s: No active beacon for cpid %s.",
                  __func__, cpid.ToString());
 
         return accrual;
     }
 
+    Beacon_ptr beacon_ptr = beacon;
+
+    // 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 = std::make_shared<Beacon>(beacons.HistoricalBeacons().find(beacon->m_prev_beacon_ctx_hash)->second);
+    }
+
     const CBlockIndex* pindex_baseline = GRC::Tally::GetBaseline();
 
     // Start at the tip.

src/rpc/blockchain.cpp

@@ -745,25 +745,25 @@ UniValue beaconreport(const UniValue& params, bool fHelp)
 
     UniValue results(UniValue::VARR);
 
-    std::vector<std::pair<GRC::Cpid, GRC::Beacon>> active_beacons;
+    std::vector<std::pair<GRC::Cpid, GRC::Beacon_ptr>> active_beacon_ptrs;
 
     // Minimize the lock on cs_main.
     {
         LOCK(cs_main);
 
         const auto& beacon_map = GRC::GetBeaconRegistry().Beacons();
 
-        active_beacons.reserve(beacon_map.size());
-        active_beacons.assign(beacon_map.begin(), beacon_map.end());
+        active_beacon_ptrs.reserve(beacon_map.size());
+        active_beacon_ptrs.assign(beacon_map.begin(), beacon_map.end());
     }
 
-    for (const auto& beacon_pair : active_beacons)
+    for (const auto& beacon_pair : active_beacon_ptrs)
     {
         UniValue entry(UniValue::VOBJ);
 
         entry.pushKV("cpid", beacon_pair.first.ToString());
-        entry.pushKV("address", beacon_pair.second.GetAddress().ToString());
-        entry.pushKV("timestamp", beacon_pair.second.m_timestamp);
+        entry.pushKV("address", beacon_pair.second->GetAddress().ToString());
+        entry.pushKV("timestamp", beacon_pair.second->m_timestamp);
 
         results.push_back(entry);
     }