Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ BITCOIN_CORE_H = \
evo/simplifiedmns.h \
evo/smldiff.h \
evo/snapshot.h \
evo/snapshot_types.h \
evo/specialtx.h \
evo/specialtx_filter.h \
evo/specialtxman.h \
Expand Down Expand Up @@ -558,6 +559,7 @@ libbitcoin_node_a_SOURCES = \
evo/simplifiedmns.cpp \
evo/smldiff.cpp \
evo/snapshot.cpp \
evo/snapshot_chain.cpp \
evo/specialtx.cpp \
evo/specialtx_filter.cpp \
evo/specialtxman.cpp \
Expand Down Expand Up @@ -1293,6 +1295,7 @@ libdashkernel_la_SOURCES = \
evo/providertx_util.cpp \
evo/simplifiedmns.cpp \
evo/smldiff.cpp \
evo/snapshot.cpp \
evo/specialtx.cpp \
evo/specialtx_filter.cpp \
evo/specialtxman.cpp \
Expand Down
10 changes: 9 additions & 1 deletion src/evo/chainhelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <evo/creditpool.h>
#include <evo/deterministicmns.h>
#include <evo/mnhftx.h>
#include <evo/snapshot.h>
#include <evo/specialtxman.h>
#include <governance/superblock.h>
#include <hash.h>
Expand All @@ -27,6 +28,8 @@ CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmn
isman{isman},
mn_sync{mn_sync},
m_dmnman{dmnman},
m_qblockman{qblockman},
m_qsnapman{qsnapman},
credit_pool_manager{std::make_unique<CCreditPoolManager>(evodb, chainman)},
m_chainlocks{chainlocks},
ehf_manager{std::make_unique<CMNHFManager>(evodb, consensus_params)},
Expand Down Expand Up @@ -66,7 +69,12 @@ int32_t CChainstateHelper::GetBestChainLockHeight() const { return m_chainlocks.

uint256 CChainstateHelper::GetDeterministicMNListHash(const CBlockIndex* pindex) const
{
return SerializeHash(m_dmnman.GetListForBlock(Assert(pindex)));
const CBlockIndex* index{Assert(pindex)};
CDeterministicMNList list{m_dmnman.GetListForBlock(index)};
if (list.GetBlockHash().IsNull()) {
list = CDeterministicMNList{index->GetBlockHash(), index->nHeight, 0};
}
return evo::CanonicalMNListHash(list);
}

/** Passthrough functions to CCreditPoolManager */
Expand Down
5 changes: 5 additions & 0 deletions src/evo/chainhelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class CChainstateHelper
llmq::CInstantSendManager& isman;
const CMasternodeSync& mn_sync;
CDeterministicMNManager& m_dmnman;
llmq::CQuorumBlockProcessor& m_qblockman;
llmq::CQuorumSnapshotManager& m_qsnapman;

public:
const std::unique_ptr<CCreditPoolManager> credit_pool_manager;
Expand Down Expand Up @@ -75,6 +77,9 @@ class CChainstateHelper

/** Return a canonical hash of the deterministic MN list derived at a block. */
uint256 GetDeterministicMNListHash(const CBlockIndex* pindex) const;
CDeterministicMNManager& DeterministicMNManager() { return m_dmnman; }
llmq::CQuorumBlockProcessor& QuorumBlockProcessor() { return m_qblockman; }
llmq::CQuorumSnapshotManager& QuorumSnapshotManager() { return m_qsnapman; }

/** Passthrough functions to CCreditPoolManager */
CCreditPool GetCreditPool(const CBlockIndex* const pindex);
Expand Down
19 changes: 13 additions & 6 deletions src/evo/creditpool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <masternode/payments.h>
#include <node/blockstorage.h>
#include <shutdown.h>
#include <util/check.h>
#include <validation.h>

#include <algorithm>
Expand Down Expand Up @@ -125,12 +126,12 @@ std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex& b
return pool;
}
}
if (block_index.nHeight % DISK_SNAPSHOT_PERIOD == 0) {
if (evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
LOCK(cache_mutex);
creditPoolCache.insert(block_hash, pool);
return pool;
}
// Snapshot activation may deliberately seed a full state at a height that
// is not one of the normal periodic checkpoints.
if (evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
LOCK(cache_mutex);
creditPoolCache.insert(block_hash, pool);
return pool;
}
return std::nullopt;
}
Expand All @@ -155,6 +156,12 @@ void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const
}
}

bool CCreditPoolManager::SeedSnapshot(const CBlockIndex* block, const CCreditPool& pool)
{
if (!Assume(block != nullptr)) return false;
return evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block->GetBlockHash()), pool);
}
Comment on lines +159 to +163

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💬 Nitpick: raw assert() on seed input compiles out in release

SeedSnapshot uses raw assert(block != nullptr) before dereferencing. The assert disappears in release builds, and a null here (e.g. a snapshot referencing an unknown block index during load) is input-driven, so it must be rejected with normal error handling that returns false rather than crashing. This also matches SeedMinedCommitment, which returns false on unknown indexes instead of asserting.

Suggested change
bool CCreditPoolManager::SeedSnapshot(const CBlockIndex* block, const CCreditPool& pool)
{
assert(block != nullptr);
return evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block->GetBlockHash()), pool);
}
bool CCreditPoolManager::SeedSnapshot(const CBlockIndex* block, const CCreditPool& pool)
{
if (block == nullptr) return false;
return evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block->GetBlockHash()), pool);
}

source: muse-spark-1.3-contributor (phase1-reviewer: dash-core-commit-history)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in b1921b3 as if (!Assume(block != nullptr)) return false;. A null here is an internal caller bug (the seeder resolves block indexes before calling), so Assume keeps debug/fuzz builds loud while release returns the failure to the caller. SeedSnapshotForBlock in llmq/snapshot.cpp had the same unguarded dereference and got the same treatment.


🤖 Posted autonomously by Claude on behalf of pasta.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Resolved (re-reviewed at b1921b3d): Confirmed your null guards in both SeedSnapshot and SeedSnapshotForBlock return false before dereferencing the block index in release builds. This addresses the unguarded dereference while retaining the internal-caller diagnostic.


CCreditPool CCreditPoolManager::ConstructCreditPool(const gsl::not_null<const CBlockIndex*> block_index, CCreditPool prev)
{
std::optional<CreditPoolDataPerBlock> opt_block_data = GetCreditDataFromBlock(block_index, m_chainman.GetConsensus());
Expand Down
2 changes: 2 additions & 0 deletions src/evo/creditpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ class CCreditPoolManager
* it can happen if there limits of withdrawal (unlock) exceed
*/
CCreditPool GetCreditPool(const CBlockIndex* block) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);
/** Seed a full pool snapshot in the current EvoDB transaction. */
bool SeedSnapshot(const CBlockIndex* block, const CCreditPool& pool) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);

private:
std::optional<CCreditPool> GetFromCache(const CBlockIndex& block_index) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);
Expand Down
13 changes: 13 additions & 0 deletions src/evo/deterministicmns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,18 @@ CDeterministicMNManager::CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaM

CDeterministicMNManager::~CDeterministicMNManager() = default;

bool CDeterministicMNManager::SeedListForBlock(const CDeterministicMNList& list)
{
return m_evoDb.WriteDerived(std::make_pair(DB_LIST_SNAPSHOT, list.GetBlockHash()), list);
}

void CDeterministicMNManager::InvalidateListCacheForBlock(const uint256& block_hash)
{
LOCK(cs);
mnListsCache.erase(block_hash);
mnListDiffsCache.erase(block_hash);
}

bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex,
BlockValidationState& state, const CDeterministicMNList& newList,
MNListUpdates& updatesRet)
Expand Down Expand Up @@ -851,6 +863,7 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n
mnListsCache.emplace(pindex->GetBlockHash(), snapshot);
break;
}
if (m_list_snapshot_miss_hook) m_list_snapshot_miss_hook(pindex);

// no snapshot found yet, check diffs
auto itDiffs = mnListDiffsCache.find(pindex->GetBlockHash());
Expand Down
17 changes: 17 additions & 0 deletions src/evo/deterministicmns.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include <algorithm>
#include <atomic>
#include <functional>
#include <limits>
#include <numeric>
#include <stdexcept>
Expand Down Expand Up @@ -778,6 +779,7 @@ class CDeterministicMNManager

Uint256HashMap<CDeterministicMNList> mnListsCache GUARDED_BY(cs);
Uint256HashMap<CDeterministicMNListDiff> mnListDiffsCache GUARDED_BY(cs);
std::function<void(const CBlockIndex*)> m_list_snapshot_miss_hook GUARDED_BY(cs);
const CBlockIndex* tipIndex GUARDED_BY(cs) {nullptr};
const CBlockIndex* m_initial_snapshot_index GUARDED_BY(cs) {nullptr};

Expand All @@ -801,6 +803,21 @@ class CDeterministicMNManager
};
CDeterministicMNList GetListAtChainTip() EXCLUSIVE_LOCKS_REQUIRED(!cs);

/** Seed a canonical full-list snapshot in the current EvoDB transaction. */
bool SeedListForBlock(const CDeterministicMNList& list) EXCLUSIVE_LOCKS_REQUIRED(!cs);

/** Invalidate cached list data so the next lookup reloads it from EvoDB. */
void InvalidateListCacheForBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);

/** Test-only guard invoked after a full-list cache/EvoDB miss, before
* ordinary diff-chain reconstruction can access earlier NORMAL state. */
void SetListSnapshotMissHookForTesting(std::function<void(const CBlockIndex*)> hook)
EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
m_list_snapshot_miss_hook = std::move(hook);
}

void SetListForBlockForTesting(const CDeterministicMNList& list) EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
Expand Down
49 changes: 48 additions & 1 deletion src/evo/evodb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ CEvoDB::CEvoDB(const util::DbWrapperParams& db_params) :

CEvoDB::~CEvoDB() = default;

bool CEvoDB::HasActiveTransaction()
{
LOCK(cs);
return active_transaction.has_value();
}

CEvoDB::TransactionContext& CEvoDB::GetContext(EvoDbIdentity identity)
{
auto it = transaction_contexts.find(identity);
Expand Down Expand Up @@ -143,13 +149,28 @@ bool CEvoDB::HasDualChainstateMarker()
return db->Exists(EVODB_DUAL_CHAINSTATE);
}

// Reader is the raw DB or a transaction; Eraser a batch or the same transaction.
template <typename Reader, typename Eraser>
static void EraseHistoricalMNListMarkers(Reader& reader, Eraser& eraser)
{
std::vector<uint256> required;
if (reader.Read(EVODB_REQUIRED_WORK_MNLISTS, required)) {
for (const auto& block_hash : required) {
eraser.Erase(std::make_pair(EVODB_BACKGROUND_WORK_MNLIST_HASH, block_hash));
}
}
eraser.Erase(EVODB_REQUIRED_WORK_MNLISTS);
}

void CEvoDB::EraseSnapshotMarkers()
{
LOCK(cs);
auto& transaction = GetContext(GetCurrentIdentity()).cur_transaction;
transaction.Erase(std::make_pair(EVODB_BEST_BLOCK, uint8_t{1}));
transaction.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
transaction.Erase(EVODB_BACKGROUND_MNLIST_HASH);
EraseHistoricalMNListMarkers(transaction, transaction);
transaction.Erase(EVODB_SNAPSHOT_EVO_SECTION);
transaction.Erase(EVODB_DUAL_CHAINSTATE);
}

Expand Down Expand Up @@ -182,6 +203,26 @@ bool CEvoDB::ReadBackgroundMNListHash(uint256& block_hash, uint256& mn_list_hash
return true;
}

void CEvoDB::WriteRequiredWorkMNListHashes(const std::vector<uint256>& block_hashes)
{
Write(EVODB_REQUIRED_WORK_MNLISTS, block_hashes);
}

bool CEvoDB::ReadRequiredWorkMNListHashes(std::vector<uint256>& block_hashes)
{
return Read(EVODB_REQUIRED_WORK_MNLISTS, block_hashes);
}

void CEvoDB::WriteBackgroundWorkMNListHash(const uint256& block_hash, const uint256& mn_list_hash)
{
Write(std::make_pair(EVODB_BACKGROUND_WORK_MNLIST_HASH, block_hash), mn_list_hash);
}

bool CEvoDB::ReadBackgroundWorkMNListHash(const uint256& block_hash, uint256& mn_list_hash)
{
return Read(std::make_pair(EVODB_BACKGROUND_WORK_MNLIST_HASH, block_hash), mn_list_hash);
}

bool CEvoDB::PromoteSnapshotMarkers(const uint256& expected_snapshot_tip)
{
LOCK(cs);
Expand All @@ -198,7 +239,9 @@ bool CEvoDB::PromoteSnapshotMarkers(const uint256& expected_snapshot_tip)
uint256 normal_tip;
const bool already_promoted = db->Read(EVODB_BEST_BLOCK, normal_tip) && normal_tip == expected_snapshot_tip &&
!db->Exists(EVODB_DUAL_CHAINSTATE) && !db->Exists(EVODB_SNAPSHOT_MNLIST_HASH) &&
!db->Exists(EVODB_BACKGROUND_MNLIST_HASH);
!db->Exists(EVODB_BACKGROUND_MNLIST_HASH) &&
!db->Exists(EVODB_REQUIRED_WORK_MNLISTS) &&
!db->Exists(EVODB_SNAPSHOT_EVO_SECTION);
if (already_promoted) m_default_identity = EvoDbIdentity::NORMAL;
return already_promoted;
}
Expand All @@ -209,6 +252,8 @@ bool CEvoDB::PromoteSnapshotMarkers(const uint256& expected_snapshot_tip)
batch.Erase(snapshot_key);
batch.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
batch.Erase(EVODB_BACKGROUND_MNLIST_HASH);
EraseHistoricalMNListMarkers(*db, batch);
batch.Erase(EVODB_SNAPSHOT_EVO_SECTION);
batch.Erase(EVODB_DUAL_CHAINSTATE);
if (!db->WriteBatch(batch, /*fSync=*/true)) return false;
// The dual-chainstate run is over: the promoted state is the NORMAL
Expand All @@ -231,6 +276,8 @@ bool CEvoDB::DiscardSnapshotMarkers()
batch.Erase(std::make_pair(EVODB_BEST_BLOCK, uint8_t{1}));
batch.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
batch.Erase(EVODB_BACKGROUND_MNLIST_HASH);
EraseHistoricalMNListMarkers(*db, batch);
batch.Erase(EVODB_SNAPSHOT_EVO_SECTION);
batch.Erase(EVODB_DUAL_CHAINSTATE);
if (!db->WriteBatch(batch, /*fSync=*/true)) return false;
// The snapshot chainstate is gone; transaction-less access must resolve
Expand Down
8 changes: 8 additions & 0 deletions src/evo/evodb.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ static const std::string EVODB_BEST_BLOCK = "b_b4";
static const std::string EVODB_DUAL_CHAINSTATE = "b_dcs";
static const std::string EVODB_SNAPSHOT_MNLIST_HASH = "b_dcs_mn";
static const std::string EVODB_BACKGROUND_MNLIST_HASH = "b_dcs_bg_mn";
static const std::string EVODB_REQUIRED_WORK_MNLISTS = "b_dcs_req_mn";
static const std::string EVODB_BACKGROUND_WORK_MNLIST_HASH = "b_dcs_bg_work_mn";
static const std::string EVODB_SNAPSHOT_EVO_SECTION = "b_dcs_evo";

enum class EvoDbIdentity {
NORMAL,
Expand Down Expand Up @@ -224,6 +227,7 @@ class CEvoDB
bool CommitRootTransaction(EvoDbIdentity identity = EvoDbIdentity::NORMAL, bool sync = false) EXCLUSIVE_LOCKS_REQUIRED(!cs);

bool IsEmpty() { return db->IsEmpty(); }
bool HasActiveTransaction() EXCLUSIVE_LOCKS_REQUIRED(!cs);

//! Set the identity used by reads/writes outside any transaction. Must
//! track the active chainstate: snapshot activation sets SNAPSHOT;
Expand All @@ -250,6 +254,10 @@ class CEvoDB
bool ReadSnapshotBaseMNListHash(uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
void WriteBackgroundMNListHash(const uint256& block_hash, const uint256& mn_list_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ReadBackgroundMNListHash(uint256& block_hash, uint256& mn_list_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
void WriteRequiredWorkMNListHashes(const std::vector<uint256>& block_hashes) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ReadRequiredWorkMNListHashes(std::vector<uint256>& block_hashes) EXCLUSIVE_LOCKS_REQUIRED(!cs);
void WriteBackgroundWorkMNListHash(const uint256& block_hash, const uint256& mn_list_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ReadBackgroundWorkMNListHash(const uint256& block_hash, uint256& mn_list_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs);

/**
* Atomically promote the surviving snapshot marker to the legacy NORMAL key
Expand Down
7 changes: 7 additions & 0 deletions src/evo/mnhftx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <llmq/signhash.h>
#include <node/blockstorage.h>
#include <shutdown.h>
#include <util/check.h>
#include <util/std23.h>

#include <chain.h>
Expand Down Expand Up @@ -382,6 +383,12 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p
}
}

bool CMNHFManager::SeedSignals(const CBlockIndex* pindex, const Signals& signals)
{
if (!Assume(pindex != nullptr)) return false;
return m_evoDb.WriteDerived(std::make_pair(DB_SIGNALS_v2, pindex->GetBlockHash()), signals);
}
Comment on lines +386 to +390

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💬 Nitpick: raw assert() on SeedSignals input

SeedSignals uses raw assert(pindex != nullptr) before dereferencing. Same as the credit-pool seed path: the assert compiles out in release and a null is input-driven, so return false instead of asserting.

Suggested change
bool CMNHFManager::SeedSignals(const CBlockIndex* pindex, const Signals& signals)
{
assert(pindex != nullptr);
return m_evoDb.WriteDerived(std::make_pair(DB_SIGNALS_v2, pindex->GetBlockHash()), signals);
}
bool CMNHFManager::SeedSignals(const CBlockIndex* pindex, const Signals& signals)
{
if (pindex == nullptr) return false;
return m_evoDb.WriteDerived(std::make_pair(DB_SIGNALS_v2, pindex->GetBlockHash()), signals);
}

source: muse-spark-1.3-contributor (phase1-reviewer: dash-core-commit-history)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in b1921b3 as if (!Assume(pindex != nullptr)) return false;, same reasoning as the credit-pool seed path.


🤖 Posted autonomously by Claude on behalf of pasta.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Resolved (re-reviewed at b1921b3d): Confirmed your SeedSignals change returns false before dereferencing a null pindex in release builds. Assume retains the debug/fuzz diagnostic without leaving the release path unguarded.


void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit)
{
auto signals = GetForBlock(pindex->pprev);
Expand Down
2 changes: 2 additions & 0 deletions src/evo/mnhftx.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class CMNHFManager : public AbstractEHFManager
void AddSignal(const CBlockIndex* const pindex, int bit) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache);

bool ForceSignalDBUpdate(const CBlockIndex* tip) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !cs_cache);
/** Seed the signals at a block in the current EvoDB transaction. */
bool SeedSignals(const CBlockIndex* pindex, const Signals& signals) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache);

private:
void AddToCache(const Signals& signals, const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache);
Expand Down
Loading
Loading