Skip to content

Superblock validation - Part II #1542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 5, 2019
Merged

Superblock validation - Part II #1542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6f15c30
New formulation of per project uncached SB validation
jamescowens Oct 1, 2019
fd4fb62
Merge branch 'development' into integrated_scraper_2
jamescowens Oct 1, 2019
228826f
Tighten up scraper locking
jamescowens Oct 1, 2019
96e128f
Prevent forwarding manifests older than SCRAPER_CMANIFEST_RETENTION_TIME
jamescowens Oct 2, 2019
16d7970
Merge branch 'development' into integrated_scraper_2
jamescowens Oct 2, 2019
037ea52
Use IsManifestCurrent() for qualifying manifests to be sent
jamescowens Oct 4, 2019
ccb7c62
Add missing cs_mapParts lock in reference validation function
jamescowens Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5946,10 +5946,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
pfrom->PushMessage("tx", ss);
}
}
else if(!pushed && MSG_PART == inv.type ) {
else if(!pushed && inv.type == MSG_PART) {
LOCK(CSplitBlob::cs_mapParts);

CSplitBlob::SendPartTo(pfrom, inv.hash);
}
else if(!pushed && MSG_SCRAPERINDEX == inv.type )
else if(!pushed && inv.type == MSG_SCRAPERINDEX)
{
LOCK(CScraperManifest::cs_mapManifest);

Expand All @@ -5971,8 +5973,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
// but it is an out parameter of IsManifestAuthorized.
unsigned int banscore_out = 0;

if (CScraperManifest::IsManifestAuthorized(manifest.pubkey, banscore_out))
// Also don't send a manifest that is not current.
if (CScraperManifest::IsManifestAuthorized(manifest.pubkey, banscore_out) && manifest.IsManifestCurrent())
{
CScraperManifest::SendManifestTo(pfrom, inv.hash);
}
}
}
}
Expand Down
181 changes: 87 additions & 94 deletions src/scraper/scraper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3723,7 +3723,7 @@ bool ScraperSendFileManifestContents(CBitcoinAddress& Address, CKey& Key)

// "Sign" and "send".

LOCK(CScraperManifest::cs_mapManifest);
LOCK2(CScraperManifest::cs_mapManifest, CSplitBlob::cs_mapParts);

bool bAddManifestSuccessful = CScraperManifest::addManifest(std::move(manifest), Key);

Expand Down Expand Up @@ -5014,7 +5014,10 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
}
if (ConvergedScraperStatsCache.NewFormatSuperblock.GetHash() == nNewFormatSuperblockHash) return scraperSBvalidationtype::CurrentCachedConvergence;

// if not validated with current cached contract, then check past ones in the cache.
// if not validated with current cached contract, then check past ones in the cache. Note that it is
// ok that due to a possible key collision for the uint32_t truncated hash hint, only the latest full hash that
// matches the hint would be stored. Any others will be picked up from the uncached case below. This would
// be extremely rare.
auto found = ConvergedScraperStatsCache.PastConvergences.find(nReducedSBContentHash);

if (found != ConvergedScraperStatsCache.PastConvergences.end())
Expand Down Expand Up @@ -5157,15 +5160,10 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
uint256 nManifestHashForConvergedBeaconList = 0;

// We are going to do this for each project in the whitelist.
unsigned int iCountMatchedProjects = 0;
unsigned int iCountSuccessfulConvergedProjects = 0;

_log(logattribute::INFO, "ValidateSuperblock", "Number of Scrapers with manifests = " + std::to_string(nScraperCount));

// Content (Part) Hash - Project
std::map<uint256, std::string> mMatchingProjectPartHashes;


// TODO: Should this be based on the list of projects in the SB? (Equivalent to a cached whitelist state.)
for (const auto& iWhitelistProject : projectWhitelist)
{
Expand All @@ -5177,7 +5175,6 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
std::multimap<int64_t, std::tuple<uint256, uint256, uint256>, greater<int64_t>> mProjectObjectsBinnedByTime;
// and also by project object (content) hash, then scraperID and project.
std::multimap<uint256, std::pair<ScraperID, std::string>> mProjectObjectsBinnedbyContent;
//std::multimap<uint256, std::pair<ScraperID, std::string>>::iterator ProjectConvergence;

{
LOCK(CScraperManifest::cs_mapManifest);
Expand Down Expand Up @@ -5243,104 +5240,75 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
}
}
}
if (fDebug3) _log(logattribute::INFO, "ENDLOCK", "CScraperManifest::cs_mapManifest");
} // Critical section scope for cs_mapManifest.

bool bMatched = false;

// Get an iterator pointing to the project in the project index map and match to the hint.
// Walk the time map (backwards in time because the sort order is descending), and select the first
// Project Part (Object) content hash that meets the convergence rule.
for (const auto& iter : mProjectObjectsBinnedByTime)
{
// Here we only consider those objects from manifests that have a time that is equal to or before the superblock to be validated,
// and which matches the project (part) hint.
uint32_t nReducedProjectObjectContentHash = std::get<0>(iter.second).Get64() >> 32;

if (const auto SBProjectIter = NewFormatSuperblock.m_projects.Try(iWhitelistProject.m_name))
{
// Pull the convergence hint (reduced content hash) for the project object.
uint32_t nReducedSBProjectObjectContentHash = SBProjectIter->m_convergence_hint;

for (const auto& iter : mProjectObjectsBinnedbyContent)
if (iter.first <= NewFormatSuperblock.m_timestamp && nReducedProjectObjectContentHash == nReducedSBProjectObjectContentHash)
Copy link
Member

@cyrossignol cyrossignol Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a node's time is behind, this could result in a false validation failure. A scraper could publish a manifest with a time ahead of the perceived time that a node received the superblock. As in...

  1. Scraper detects new etag, pulls the stats, and publishes a new manifest
  2. 10 seconds later, a node 20 seconds behind stakes a superblock with that manifest
  3. Another node immediately receives the manifest and the superblock...
    manifest time: t - 10,
    superblock time: t - 20,
    where t is roughly the node's current time.
  4. Validation fails because the manifest was not considered when reconstructing the convergence

The superblock timestamp will be set to the timestamp of its containing block when received by a node. I don't think we need to check the timestamp here because the content hash should adequately filter out ineligible candidates.

Copy link
Member Author

@jamescowens jamescowens Oct 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I put this in there to try and conform the validation to the existing implementation of the formation of a convergence from the source node, while trying to deal with the exact opposite case that you describe above, which is that the convergence that is hinted by the incoming superblock may not be the latest by project convergence that can be formed on the receiving node.

Copy link
Member

@cyrossignol cyrossignol Oct 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to check every combination of the matching project parts (breadth-and-most-recent-first) to also protect against temporary network partitions or delays during the superblock window. Not a pretty algorithm...

There will only be one combination almost all of the time, though.

Copy link
Member Author

@jamescowens jamescowens Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. And it is 99.9% likely the head of the list. But it is the 0.1% I am worried about. I do not want to get into an 2^n (n the number of projects) algorithm here for a 0.1% corner case. I have an idea. I will have to draw it out on a picture.

Copy link
Member Author

@jamescowens jamescowens Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
image
image

Copy link
Member

@cyrossignol cyrossignol Oct 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rare

To say the least 🙂

And this only matters in a fallback-to-project-level convergence.

Copy link
Member Author

@jamescowens jamescowens Oct 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the node has only been running for a short enough while that it is not cached...

But we will do the final algo anyway, because I like to be thorough. I don't think we should delay the interim release over this. Let's merge this tracking PR as is and continue our work on this on integrated_scraper_2.

//if (manifest.nTime <= NewFormatSuperblock.m_timestamp && nReducedProjectObjectContentHash == nReducedSBProjectObjectContentHash)
{
uint32_t nReducedProjectObjectContentHash = iter.first.Get64() >> 32;

// This has the effect of only storing the first one of the series of matching project parts (from different scrapers) with the same
// full hash that match the hint, because of the insert. Below we will count the others matching to check for a supermajority.
// Note that two different full hashes that match the hint for the same project would be included.
if (nReducedProjectObjectContentHash == nReducedSBProjectObjectContentHash)
// Notice the below is NOT using the time. We switch to the content only. The time is only used to make sure
// we test the convergence of the project objects in time order, but once a content hash is selected based on the time,
// only the content hash is used to count occurrences in the multimap, because the times for the same
// project object (part hash) will be different across different manifests and different scrapers.
unsigned int nIdenticalContentManifestCount = mProjectObjectsBinnedbyContent.count(std::get<0>(iter.second));
if (nIdenticalContentManifestCount >= NumScrapersForSupermajority(nScraperCount))
{
// ------------------------------------------ full content hash ---- project name
mMatchingProjectPartHashes.insert(std::make_pair(iter.first, iWhitelistProject.m_name));
bMatched = true;
}
}
}
LOCK(CSplitBlob::cs_mapParts);

// Keep track of the number of matched projects.
if (bMatched) ++iCountMatchedProjects;

if (fDebug3) _log(logattribute::INFO, "ENDLOCK", "CScraperManifest::cs_mapManifest");
}
// Get the actual part ----------------- by object hash.
auto iPart = CSplitBlob::mapParts.find(std::get<0>(iter.second));

uint256 nLatestProjectPartMatchingHash;
int64_t nLatestProjectPartMatchingTime = 0;
uint256 nLatestProjectPartMatchingConsensusBlock;
uint256 nLatestProjectPartMatchingManifestForBeaconList;
uint256 nContentHashCheck = Hash(iPart->second.data.begin(), iPart->second.data.end());

for (const auto& iter : mMatchingProjectPartHashes)
{
// We want the most recent (latest) part, constrained to the above map. This is slightly different from.
// but similar to the construction of the convergence without constraint.
for (const auto& iter_inner : mProjectObjectsBinnedByTime)
{
// Only go until the first match (latest), then break.
if (std::get<0>(iter_inner.second) == iter.first)
{
nLatestProjectPartMatchingHash = iter.first;
nLatestProjectPartMatchingTime = iter_inner.first;
nLatestProjectPartMatchingConsensusBlock = get<1>(iter_inner.second);
nLatestProjectPartMatchingManifestForBeaconList = get<2>(iter_inner.second);

break;
}
}
}

unsigned int nIdenticalContentManifestCount = mProjectObjectsBinnedbyContent.count(nLatestProjectPartMatchingHash);

if (nIdenticalContentManifestCount >= NumScrapersForSupermajority(nScraperCount))
{
LOCK(CScraperManifest::cs_mapParts);

// Get the actual part ----------------- by object hash.
auto iPart = CSplitBlob::mapParts.find(nLatestProjectPartMatchingHash);

uint256 nContentHashCheck = Hash(iPart->second.data.begin(), iPart->second.data.end());
if (nContentHashCheck != iPart->first)
{
_log(logattribute::ERR, "ScraperConstructConvergedManifestByProject", "Selected Converged Project Object content hash check failed! nContentHashCheck = "
+ nContentHashCheck.GetHex() + " and nContentHash = " + iPart->first.GetHex());
break;
}

if (nContentHashCheck != iPart->first)
{
_log(logattribute::ERR, "ValidateSuperblock", "Selected Converged Project Object content hash check failed! nContentHashCheck = "
+ nContentHashCheck.GetHex() + " and nContentHash = " + iPart->first.GetHex());
// Put Project Object (Part) in StructConvergedManifest keyed by project.
StructDummyConvergedManifest.ConvergedManifestPartsMap.insert(std::make_pair(iWhitelistProject.m_name, iPart->second.data));

// Bail.
return scraperSBvalidationtype::Invalid;
}
// If the indirectly referenced manifest has a consensus time that is greater than already recorded, replace with that time, and also
// change the consensus block to the referred to consensus block. (Note that this is scoped at even above the individual project level, so
// the result after iterating through all projects will be the latest manifest time and consensus block that corresponds to any of the
// parts that meet convergence.) We will also get the manifest hash too, so we can retrieve the associated BeaconList that was used.
if (iter.first > nConvergedConsensusTime)
{
nConvergedConsensusTime = iter.first;
nConvergedConsensusBlock = std::get<1>(iter.second);
nManifestHashForConvergedBeaconList = std::get<2>(iter.second);
}

// Put Project Object (Part) in Dummy StructConvergedManifest keyed by project.
StructDummyConvergedManifest.ConvergedManifestPartsMap.insert(std::make_pair(iWhitelistProject.m_name, iPart->second.data));
iCountSuccessfulConvergedProjects++;

// If the indirectly referenced manifest has a consensus time that is greater than already recorded, replace with that time, and also
// change the consensus block to the referred to consensus block. (Note that this is scoped at even above the individual project level, so
// the result after iterating through all projects will be the latest manifest time and consensus block that corresponds to any of the
// parts that meet convergence.) We will also get the manifest hash too, so we can retrieve the associated BeaconList that was used.
// This version is subject to the constraints of the hint by the above code, but operates similar to a normal convergence construction.
if (nLatestProjectPartMatchingTime > nConvergedConsensusTime)
{
nConvergedConsensusTime = nLatestProjectPartMatchingTime;
nConvergedConsensusBlock = nLatestProjectPartMatchingConsensusBlock;
nManifestHashForConvergedBeaconList = nLatestProjectPartMatchingManifestForBeaconList;
}
// Note this break is VERY important, it prevents considering essentially the same project object that meets convergence multiple times.
break;
} // Test for supermajority (per project).
} // Test for timestamp <= superblock timestamp and hint matches.
} // Test whether project exists in whitelist and provide iterator.
} // For loop over mProjectObjectsBinnedByTime.
} // For loop over projectWhitelist.

// Keep track of the number of matched projects that go in the convergence.
++iCountSuccessfulConvergedProjects;
}
}

// If we have a three way match, then proceed with the construction of local candidate SB
// If we have a matched project count match, then proceed with the construction of local candidate SB
// to validate, otherwise there is no validation.
if (iCountSuccessfulConvergedProjects == iCountMatchedProjects && iCountMatchedProjects == NewFormatSuperblock.m_projects.size())
if (iCountSuccessfulConvergedProjects == NewFormatSuperblock.m_projects.size())
{
// Fill out the the rest of the ConvergedManifest structure. Note this assumes one-to-one part to project statistics BLOB. Needs to
// be fixed for more than one part per BLOB. This is easy in this case, because it is all from/referring to one manifest.
Expand All @@ -5367,7 +5335,7 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
ss << iter.second;

StructDummyConvergedManifest.nContentHash = Hash(ss.begin(), ss.end());
StructDummyConvergedManifest.timestamp = GetAdjustedTime();
StructDummyConvergedManifest.timestamp = nConvergedConsensusTime;
StructDummyConvergedManifest.bByParts = true;

_log(logattribute::INFO, "ValidateSuperblock", "Successful convergence by project: "
Expand All @@ -5376,7 +5344,6 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb
+ DateTimeStrFormat("%x %H:%M:%S", StructDummyConvergedManifest.timestamp));


if (fDebug3) _log(logattribute::INFO, "ENDLOCK", "CScraperManifest::cs_mapManifest");

ScraperStats mScraperstats = GetScraperStatsByConvergedManifest(StructDummyConvergedManifest);

Expand All @@ -5403,6 +5370,8 @@ scraperSBvalidationtype ValidateSuperblock(const NN::Superblock& NewFormatSuperb

NN::QuorumHash nCandidateSuperblockHash = superblock.GetHash();

if (fDebug3) _log(logattribute::INFO, "ENDLOCK", "CScraperManifest::cs_mapManifest");

if (nCandidateSuperblockHash == nNewFormatSuperblockHash) return scraperSBvalidationtype::ProjectLevelConvergence;
} // If you fall out of this if statement... no validation.
}
Expand Down Expand Up @@ -5698,15 +5667,35 @@ UniValue testnewsb(const UniValue& params, bool fHelp)

if (PastConvergencesSize > 1)
{
std::default_random_engine generator;
std::uniform_int_distribution<int> distribution(0, PastConvergencesSize - 1);

std::advance(iPastSB, distribution(generator));
/*
std::default_random_engine generator(static_cast<unsigned int>(GetAdjustedTime()));
std::uniform_int_distribution<unsigned int> distribution(0, PastConvergencesSize - 1);

_log(logattribute::INFO, "testnewsb", "NN::ValidateSuperblock random past distribution limits: " + std::to_string(distribution.a())
+ " , " + std::to_string(distribution.b()));

unsigned int i = distribution(generator);
*/

int i = GetRandInt(PastConvergencesSize - 1);

_log(logattribute::INFO, "testnewsb", "NN::ValidateSuperblock random past RandomPastConvergedManifest index " + std::to_string(i) + " selected.");
res.pushKV("NN::ValidateSuperblock random past RandomPastConvergedManifest index selected", i);

std::advance(iPastSB, i);

RandomPastConvergedManifest = iPastSB->second.second;

bPastConvergencesEmpty = false;
}
else if (PastConvergencesSize == 1)
{
//Use the first and only element.
RandomPastConvergedManifest = iPastSB->second.second;

bPastConvergencesEmpty = false;
}

}

Expand All @@ -5718,7 +5707,7 @@ UniValue testnewsb(const UniValue& params, bool fHelp)

// This should really be done in the superblock class as an overload on NN::Superblock::FromConvergence.
RandomPastSB.m_convergence_hint = RandomPastConvergedManifest.nContentHash.Get64() >> 32;
RandomPastSB.m_manifest_content_hint = RandomPastConvergedManifest.nUnderlyingManifestContentHash.Get64() >> 32;
RandomPastSB.m_timestamp = RandomPastConvergedManifest.timestamp;

if (RandomPastConvergedManifest.bByParts)
{
Expand All @@ -5732,9 +5721,13 @@ UniValue testnewsb(const UniValue& params, bool fHelp)
const std::string& project_name = part_pair.first;
const CSerializeData& part_data = part_pair.second;

projects.SetHint(project_name, part_data);
projects.SetHint(project_name, part_data); // This also sets m_converged_by_project to true.
}
}
else
{
RandomPastSB.m_manifest_content_hint = RandomPastConvergedManifest.nUnderlyingManifestContentHash.Get64() >> 32;
}

//
// ValidateSuperblock() reference function tests (past convergence)
Expand Down
Loading