Skip to content

Commit

Permalink
Toughen UpdateGreylistCandidateEntry and AutoGreylist::Reset()
Browse files Browse the repository at this point in the history
  • Loading branch information
jamescowens committed Jan 21, 2025
1 parent 805b083 commit 7960dfa
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 61 deletions.
7 changes: 4 additions & 3 deletions src/gridcoin/project.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "node/ui_interface.h"

#include <algorithm>
#include <atomic>

using namespace GRC;
using LogFlags = BCLog::LogFlags;
Expand Down Expand Up @@ -565,6 +564,8 @@ void AutoGreylist::Reset()
{
if (m_greylist_ptr != nullptr) {
m_greylist_ptr->clear();
} else {
m_greylist_ptr = std::make_shared<Greylist>();
}

m_superblock_hash = Superblock().GetHash(true);
Expand All @@ -586,7 +587,7 @@ WhitelistSnapshot Whitelist::Snapshot(const ProjectEntry::ProjectFilterFlag& fil
return WhitelistSnapshot(std::make_shared<ProjectList>(projects), filter);
}

if (refresh_greylist) {
if (refresh_greylist && m_auto_greylist != nullptr) {
m_auto_greylist->Refresh();
}

Expand All @@ -610,7 +611,7 @@ WhitelistSnapshot Whitelist::Snapshot(const ProjectEntry::ProjectFilterFlag& fil
// applies the current state of the greylist at the time of the construction of the whitelist snapshot, without
// disturbing the underlying projects registry.

bool in_greylist = m_auto_greylist->Contains(iter.first);
bool in_greylist = m_auto_greylist != nullptr ? m_auto_greylist->Contains(iter.first) : false;

// If the project does NOT have a status of auto greylist override, and it is either active or already manually
// greylisted, then if it is in the greylist, mark with the status auto greylisted.
Expand Down
78 changes: 35 additions & 43 deletions src/gridcoin/project.h
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ class AutoGreylist

//!
//! \brief This parameterized constructor is used to construct the initial (baseline) greylist candidate
//! entry with the initial total credit value for the project.
//! entry with the initial total credit value for the project. This also inserts the baseline history entry.
//!
//! \param project_name
//! \param TC_initial_bookmark
Expand All @@ -560,7 +560,7 @@ class AutoGreylist
, m_TC_40_SB_sum(0)
, m_meets_greylisting_crit(false)
, m_TC_initial_bookmark(TC_initial_bookmark)
, m_TC_bookmark(0)
, m_TC_bookmark(TC_initial_bookmark)
, m_sb_from_baseline_processed(0)
, m_update_history()
{
Expand Down Expand Up @@ -639,52 +639,46 @@ class AutoGreylist
//!
void UpdateGreylistCandidateEntry(std::optional<uint64_t> total_credit, uint8_t sb_from_baseline)
{
uint8_t sbs_from_baseline_no_update = sb_from_baseline - m_sb_from_baseline_processed - 1;

m_sb_from_baseline_processed = sb_from_baseline;

// ZCD part. Remember we are going backwards, so if total_credit is greater than or equal to
// the bookmark, then we have zero or even negative project total credit between superblocks, so
// this qualifies as a ZCD.
if (m_sb_from_baseline_processed <= 20) {
// Skipped updates count as ZCDs. We stop counting at 20 superblocks (back) from
// the initial SB baseline. The skipped updates may actually not be used in practice, because we are
// iterating over the entire whitelist for each SB and inserting std::nullopt TC updates for each project.
m_zcd_20_SB_count += sbs_from_baseline_no_update;

// If total credit is greater than the bookmark, this means that (going forward in time) credit actually
// declined, so in addition to the zero credit days recorded for the skipped updates, we must add an
// additional day for the credit decline (going forward in time). If total credit is std::nullopt, then
// this means no statistics available, which also is a ZCD.
if ((total_credit && total_credit >= m_TC_bookmark) || !total_credit){
++m_zcd_20_SB_count;
if (sb_from_baseline > 0) {
// ZCD part. Remember we are going backwards, so if total_credit is greater than or equal to
// the bookmark, then we have zero or even negative project total credit between superblocks, so
// this qualifies as a ZCD.
if (sb_from_baseline <= 20) {
// If total credit is greater than the bookmark, this means that (going forward in time) credit actually
// declined, so we must add a day for the credit decline (going forward in time). If total credit
// is std::nullopt, then this means no statistics available, which also is a ZCD.
if ((total_credit && total_credit >= m_TC_bookmark) || !total_credit){
++m_zcd_20_SB_count;
}
}
}

// WAS part. Here we deal with two numbers, the 40 SB from baseline, and the 7 SB from baseline. We use
// the initial bookmark, and compute the difference, which is the same as adding up the deltas between
// the TC's in each superblock. For updates with no total credit entry (i.e. std::optional is nullopt),
// do not change the sums.
//
// If the initial bookmark TC is not available (i.e. the current SB did not have stats for this project),
// but this update does, then update the initial bookmark to this total credit.
if (!m_TC_initial_bookmark && total_credit) {
m_TC_initial_bookmark = total_credit;
}

if (total_credit && m_TC_initial_bookmark && m_TC_initial_bookmark > total_credit) {
if (m_sb_from_baseline_processed <= 7) {
m_TC_7_SB_sum = *m_TC_initial_bookmark - *total_credit;
// WAS part. Here we deal with two numbers, the 40 SB from baseline, and the 7 SB from baseline. We use
// the initial bookmark, and compute the difference, which is the same as adding up the deltas between
// the TC's in each superblock. For updates with no total credit entry (i.e. std::optional is nullopt),
// do not change the sums.
//
// If the initial bookmark TC is not available (i.e. the current SB did not have stats for this project),
// but this update does, then update the initial bookmark to this total credit.
if (!m_TC_initial_bookmark && total_credit) {
m_TC_initial_bookmark = total_credit;
}

if (m_sb_from_baseline_processed <= 40) {
m_TC_40_SB_sum = *m_TC_initial_bookmark - *total_credit;
if (total_credit && m_TC_initial_bookmark > total_credit) {
if (sb_from_baseline <= 7) {
m_TC_7_SB_sum = *m_TC_initial_bookmark - *total_credit;
}

if (sb_from_baseline <= 40) {
m_TC_40_SB_sum = *m_TC_initial_bookmark - *total_credit;
}
}

m_sb_from_baseline_processed = sb_from_baseline;
}

// Update bookmark.
// Update bookmark only if total_credit is actually there.
if (total_credit) {
m_TC_bookmark = *total_credit;
m_TC_bookmark = total_credit;
}

uint8_t zcd = GetZCD();
Expand Down Expand Up @@ -747,7 +741,7 @@ class AutoGreylist

private:
std::optional<uint64_t> m_TC_initial_bookmark; //!< This is a "reverse" bookmark - we are going backwards in SB's.
uint64_t m_TC_bookmark;
std::optional<uint64_t> m_TC_bookmark;
uint8_t m_sb_from_baseline_processed;

std::vector<UpdateHistoryEntry> m_update_history;
Expand Down Expand Up @@ -832,8 +826,6 @@ class AutoGreylist
//!
void Reset();

//static std::shared_ptr<AutoGreylist> GetAutoGreylistCache();

private:
mutable CCriticalSection autogreylist_lock;

Expand Down
30 changes: 15 additions & 15 deletions src/test/gridcoin/project_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,19 +479,19 @@ BOOST_AUTO_TEST_CASE(it_adds_whitelisted_projects_from_contract_data)
int height = 0;
int64_t time = 0;

BOOST_CHECK(whitelist.Snapshot().size() == 0);
BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == false);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).size() == 0);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == false);

AddProjectEntry(1, "Enigma", "http://enigma.test", false, height, time, false);

BOOST_CHECK(whitelist.Snapshot().size() == 1);
BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == true);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).size() == 1);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == true);

AddProjectEntry(2, "Foo", "http://foo.test", false, height++, time++, false);

BOOST_CHECK(whitelist.Snapshot().size() == 2);
BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == true);
BOOST_CHECK(whitelist.Snapshot().Contains("Foo") == true);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).size() == 2);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == true);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Foo") == true);
}

BOOST_AUTO_TEST_CASE(it_removes_whitelisted_projects_from_contract_data)
Expand All @@ -503,13 +503,13 @@ BOOST_AUTO_TEST_CASE(it_removes_whitelisted_projects_from_contract_data)

AddProjectEntry(1, "Enigma", "http://enigma.test", false, height, time, true);

BOOST_CHECK(whitelist.Snapshot().size() == 1);
BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == true);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).size() == 1);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == true);

DeleteProjectEntry(1, "Enigma", height++, time++, false);

BOOST_CHECK(whitelist.Snapshot().size() == 0);
BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == false);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).size() == 0);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == false);
}

BOOST_AUTO_TEST_CASE(it_does_not_mutate_existing_snapshots)
Expand All @@ -522,14 +522,14 @@ BOOST_AUTO_TEST_CASE(it_does_not_mutate_existing_snapshots)
AddProjectEntry(1, "Enigma", "http://enigma.test", false, height, time, true);
AddProjectEntry(2, "Foo", "http://foo.test", true, height++, time++, false);

auto snapshot = whitelist.Snapshot();
auto snapshot = whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false);

DeleteProjectEntry(1, "Enigma", height, time, false);

BOOST_CHECK(snapshot.Contains("Enigma") == true);

BOOST_CHECK(whitelist.Snapshot().Contains("Enigma") == false);
BOOST_CHECK(whitelist.Snapshot().Contains("Foo") == true);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Enigma") == false);
BOOST_CHECK(whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false).Contains("Foo") == true);
}

BOOST_AUTO_TEST_CASE(it_overwrites_projects_with_the_same_name)
Expand All @@ -542,7 +542,7 @@ BOOST_AUTO_TEST_CASE(it_overwrites_projects_with_the_same_name)
AddProjectEntry(1, "Enigma", "http://enigma.test", false, height, time, true);
AddProjectEntry(2, "Enigma", "http://new.enigma.test", true, height++, time++, false);

auto snapshot = whitelist.Snapshot();
auto snapshot = whitelist.Snapshot(GRC::ProjectEntry::ProjectFilterFlag::ACTIVE, false, false);
BOOST_CHECK(snapshot.size() == 1);

for (const auto& project : snapshot) {
Expand Down

0 comments on commit 7960dfa

Please sign in to comment.