Skip to content

Commit

Permalink
Correct Quorum::Hash on v3 serialized superblocks
Browse files Browse the repository at this point in the history
Implement v3 superblock tests. Also fix potential deadlock issue
with cs_main lock.
  • Loading branch information
jamescowens committed Feb 3, 2025
1 parent 554e0f8 commit b69ecc1
Show file tree
Hide file tree
Showing 3 changed files with 346 additions and 8 deletions.
11 changes: 4 additions & 7 deletions src/gridcoin/superblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,9 @@ class ScraperStatsSuperblockBuilder
end_build_from_stats_loop:
m_superblock.m_verified_beacons.Reset(stats_verified_beacons_tc.mVerifiedMap);

{
LOCK(cs_main);

if (IsSuperblockV3Enabled(nBestHeight)) {
m_superblock.m_projects_all_cpids_total_credits.Reset(stats_verified_beacons_tc.m_total_credit_map);
}
// This is equivalent to superblock v3+ and doesn't require a lock on cs_main.
if (!stats_verified_beacons_tc.m_total_credit_map.empty()) {
m_superblock.m_projects_all_cpids_total_credits.Reset(stats_verified_beacons_tc.m_total_credit_map);
}
}
private:
Expand Down Expand Up @@ -1110,7 +1107,7 @@ QuorumHash::QuorumHash(const std::vector<unsigned char>& bytes) : QuorumHash()
QuorumHash QuorumHash::Hash(const Superblock& superblock)
{
if (superblock.m_version > 1) {
return QuorumHash(SerializeHash(superblock));
return QuorumHash(SerializeHash(SuperblockForHash(superblock)));
}

std::string input;
Expand Down
38 changes: 38 additions & 0 deletions src/gridcoin/superblock.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ class QuorumHash
//!
//! \brief Hash the provided superblock.
//!
//! Note the m_project_status map is NOT hashed. While this data is serialized, the hash is computed from a specialization
//! of the Superblock class (SuperblockForHash) with the serialization of that map removed. The reason for this is twofold.
//! 1) The project status is populated from the AutoGreylist class, which when used in the miner context is using a candidate
//! superblock generated from the scraper. The QuorumHasher proxies which use the scraper statistics do not have the project
//! status information available for all call paths. 2) The project status map is extra information that is not required to
//! assure uniqueness and/or validation of a superblock, as it can (and is) derived by the AutoGreylist class. This map is
//! provided in the superblock as a convenience so that this does not have to be recalculated if looking at statistics over
//! a large block range in reporting.
//!
//! \param superblock Superblock object containing the data to hash.
//!
//! \return The appropriate quorum hash variant digest depending on the
Expand Down Expand Up @@ -1424,6 +1433,35 @@ class Superblock
mutable QuorumHash m_hash_cache;
}; // Superblock

class SuperblockForHash : public Superblock
{
public:
SuperblockForHash(const Superblock& superblock)
: Superblock(superblock)
{}

ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action)
{
if (!(s.GetType() & SER_GETHASH)) {
READWRITE(m_version);
READWRITE(m_convergence_hint);
READWRITE(m_manifest_content_hint);
}

READWRITE(m_cpids);
READWRITE(m_projects);
READWRITE(m_verified_beacons);

if (m_version > 2) {
// Note that project status is left out of the serialization here for hashing purposes.
READWRITE(m_projects_all_cpids_total_credits);
}
}
};

//!
//! \brief A smart pointer that wraps a superblock object for shared ownership
//! with context of its containing block.
Expand Down
Loading

0 comments on commit b69ecc1

Please sign in to comment.