Skip to content

fix: multiple asan's finding #7513

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
PastaPastaPasta merged 7 commits into from
Aug 2, 2026
Merged

fix: multiple asan's finding #7513

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
9ac20b8
fix: suppress intentional unsigned overflow in test/lcg.h
knst Jul 28, 2026
a3a259e
fix: make the block-height narrowing in address index keys explicit
knst Jul 28, 2026
533db58
fix: avoid decrementing past zero to prevent false sanitizer reports
knst Jul 28, 2026
064d07a
fix: hold ChainstateManager, not a Chainstate reference
knst Jul 28, 2026
fb7fa62
fix: hold ChainstateManager in chainlock, instantsend, not a Chainsta…
knst Jul 28, 2026
0b1b0a8
fix: stop capturing wrapper parameters by reference in RPC handlers
knst Jul 28, 2026
11108da
fix: take cs_main when obtaining the active chain
knst Aug 2, 2026
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
4 changes: 2 additions & 2 deletions src/active/context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, *nodeman, qman, sporkman)},
gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, superblocks, *nodeman, chainman, mn_sync)},
ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, sigman, *shareman, qman)},
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), chainlocks, clhandler, isman,
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman, chainlocks, clhandler, isman,
qman, sigman, *shareman, mn_sync)},
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), chainlocks, isman, sigman,
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman, chainlocks, isman, sigman,
*shareman, qman, sporkman, mempool, mn_sync)}
{
}
Expand Down
10 changes: 5 additions & 5 deletions src/chainlock/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
using node::ReadBlockFromDisk;

namespace chainlock {
ChainLockSigner::ChainLockSigner(Chainstate& chainstate, const chainlock::Chainlocks& chainlocks,
ChainLockSigner::ChainLockSigner(const ChainstateManager& chainman, const chainlock::Chainlocks& chainlocks,
ChainlockHandler& clhandler, const llmq::CInstantSendManager& isman,
const llmq::CQuorumManager& qman, llmq::CSigningManager& sigman,
llmq::CSigSharesManager& shareman, const CMasternodeSync& mn_sync) :
m_chainstate{chainstate},
m_chainman{chainman},
m_chainlocks{chainlocks},
m_clhandler{clhandler},
m_isman{isman},
Expand Down Expand Up @@ -88,7 +88,7 @@ void ChainLockSigner::TrySignChainTip()
return;
}

const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip());
const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainman.ActiveChain().Tip());

if (!pindex || !pindex->pprev) {
return;
Expand Down Expand Up @@ -232,7 +232,7 @@ ChainLockSigner::BlockTxs::mapped_type ChainLockSigner::GetBlockTxs(const uint25
uint32_t blockTime;
{
LOCK(::cs_main);
const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(blockHash);
const auto* pindex = m_chainman.m_blockman.LookupBlockIndex(blockHash);
if (!pindex) {
return nullptr;
}
Expand Down Expand Up @@ -305,7 +305,7 @@ void ChainLockSigner::Cleanup()
std::vector<std::shared_ptr<Uint256HashSet>> removed;
LOCK2(::cs_main, cs_signer);
for (auto it = blockTxs.begin(); it != blockTxs.end();) {
const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first);
const auto* pindex = m_chainman.m_blockman.LookupBlockIndex(it->first);
if (!pindex) {
it = blockTxs.erase(it);
} else if (m_chainlocks.HasChainLock(pindex->nHeight, pindex->GetBlockHash())) {
Expand Down
5 changes: 3 additions & 2 deletions src/chainlock/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

class CScheduler;
class CMasternodeSync;
class ChainstateManager;
namespace llmq {
class CInstantSendManager;
class CRecoveredSig;
Expand All @@ -25,7 +26,7 @@ class ChainlockHandler;
class ChainLockSigner final : public llmq::CRecoveredSigsListener, public CValidationInterface
{
private:
Chainstate& m_chainstate;
const ChainstateManager& m_chainman;
const chainlock::Chainlocks& m_chainlocks;
ChainlockHandler& m_clhandler;
const llmq::CInstantSendManager& m_isman;
Expand Down Expand Up @@ -58,7 +59,7 @@ class ChainLockSigner final : public llmq::CRecoveredSigsListener, public CValid
ChainLockSigner() = delete;
ChainLockSigner(const ChainLockSigner&) = delete;
ChainLockSigner& operator=(const ChainLockSigner&) = delete;
explicit ChainLockSigner(Chainstate& chainstate, const chainlock::Chainlocks& chainlocks,
explicit ChainLockSigner(const ChainstateManager& chainman, const chainlock::Chainlocks& chainlocks,
ChainlockHandler& clhandler, const llmq::CInstantSendManager& isman,
const llmq::CQuorumManager& qman, llmq::CSigningManager& sigman,
llmq::CSigSharesManager& shareman, const CMasternodeSync& mn_sync);
Expand Down
10 changes: 5 additions & 5 deletions src/index/addressindex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,9 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i
pindex->GetBlockHash().ToString(), block.vtx.size() - 1, blockundo.vtxundo.size());
}

for (size_t i = blockundo.vtxundo.size(); i-- > 0;) {
const CTransactionRef& tx = block.vtx[i + 1];
const CTxUndo& txundo = blockundo.vtxundo[i];
for (size_t i = blockundo.vtxundo.size(); i > 0; --i) {
const CTransactionRef& tx = block.vtx[i];
const CTxUndo& txundo = blockundo.vtxundo[i-1];
const uint256 txhash = tx->GetHash();

// Undo outputs (remove from unspent index and transaction history)
Expand All @@ -324,7 +324,7 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i

// Remove receiving activity from history
addressIndex.push_back(std::make_pair(CAddressIndexKey(address_type, address_bytes, pindex->nHeight,
i + 1, txhash, k, false),
i, txhash, k, false),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
out.nValue));

// Remove from unspent index (mark for deletion)
Expand Down Expand Up @@ -352,7 +352,7 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i

// Remove spending activity from history
addressIndex.push_back(
std::make_pair(CAddressIndexKey(address_type, address_bytes, pindex->nHeight, i + 1, txhash, j, true),
std::make_pair(CAddressIndexKey(address_type, address_bytes, pindex->nHeight, i, txhash, j, true),
prevout.nValue * -1));

// Restore to unspent index
Expand Down
4 changes: 2 additions & 2 deletions src/index/addressindex_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ struct CAddressIndexKey {
{
m_address_type = static_cast<AddressType>(ser_readdata8(s));
m_address_bytes.Unserialize(s);
m_block_height = ser_readdata32be(s);
m_block_height = static_cast<int32_t>(ser_readdata32be(s));
m_block_tx_pos = ser_readdata32be(s);
m_tx_hash.Unserialize(s);
m_tx_index = ser_readdata32(s);
Expand Down Expand Up @@ -228,7 +228,7 @@ struct CAddressIndexIteratorHeightKey {
{
m_address_type = static_cast<AddressType>(ser_readdata8(s));
m_address_bytes.Unserialize(s);
m_block_height = ser_readdata32be(s);
m_block_height = static_cast<int32_t>(ser_readdata32be(s));
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

// ********************************************************* Step 7d: Setup other Dash services

node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, node.active_ctx ? node.active_ctx->is_signer.get() : nullptr, *node.llmq_ctx->sigman, *node.llmq_ctx->qman, *node.chainlocks, chainman.ActiveChainstate(), *node.mempool, *node.mn_sync));
node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, node.active_ctx ? node.active_ctx->is_signer.get() : nullptr, *node.llmq_ctx->sigman, *node.llmq_ctx->qman, *node.chainlocks, chainman, *node.mempool, *node.mn_sync));
node.peerman->AddExtraHandler(std::make_unique<llmq::NetSigning>(node.peerman.get(), *node.llmq_ctx->sigman, node.active_ctx ? node.active_ctx->shareman.get() : nullptr, *node.sporkman));

{
Expand Down
20 changes: 11 additions & 9 deletions src/instantsend/net_instantsend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ bool NetInstantSend::ValidateIncomingISLock(const instantsend::InstantSendLock&

std::optional<int> NetInstantSend::ResolveCycleHeight(const uint256& cycle_hash)
{
auto cycle_height = GetBlockHeight(m_is_manager, m_chainstate, cycle_hash);
auto cycle_height = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), cycle_hash);
if (cycle_height) {
return cycle_height;
}

const auto block_index = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(cycle_hash));
const auto block_index = WITH_LOCK(::cs_main, return m_chainman.m_blockman.LookupBlockIndex(cycle_hash));
if (block_index == nullptr) {
return std::nullopt;
}
Expand Down Expand Up @@ -108,6 +108,8 @@ std::unique_ptr<NetInstantSend::BatchVerificationData> NetInstantSend::BuildVeri
{
auto data = std::make_unique<BatchVerificationData>();

const CChain& active_chain = *WITH_LOCK(::cs_main, return &m_chainman.ActiveChain());

for (const auto& pending : pend) {
const auto& hash = pending.islock_hash;
auto nodeId = pending.node_id;
Expand All @@ -131,7 +133,7 @@ std::unique_ptr<NetInstantSend::BatchVerificationData> NetInstantSend::BuildVeri
continue;
}

auto cycleHeightOpt = GetBlockHeight(m_is_manager, m_chainstate, islock->cycleHash);
auto cycleHeightOpt = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), islock->cycleHash);
if (!cycleHeightOpt) {
data->batchVerifier.badSources.emplace(nodeId);
continue;
Expand All @@ -145,7 +147,7 @@ std::unique_ptr<NetInstantSend::BatchVerificationData> NetInstantSend::BuildVeri
nSignHeight = cycleHeight + dkgInterval - 1;
}
// For RegTest non-rotating quorum cycleHash has directly quorum hash
auto quorum = llmq_params.useRotation ? llmq::SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, m_qman,
auto quorum = llmq_params.useRotation ? llmq::SelectQuorumForSigning(llmq_params, active_chain, m_qman,
id, nSignHeight, signOffset)
: m_qman.GetQuorum(llmq_params.type, islock->cycleHash);

Expand Down Expand Up @@ -375,7 +377,7 @@ void NetInstantSend::ProcessInstantSendLock(NodeId from, const uint256& hash, co
auto tx = GetTransaction(nullptr, &m_mempool, islock->txid, Params().GetConsensus(), hashBlock);
const bool found_transaction{tx != nullptr};
// we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally
const auto minedHeight = GetBlockHeight(m_is_manager, m_chainstate, hashBlock);
const auto minedHeight = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), hashBlock);
if (found_transaction) {
// Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes,
// we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain
Expand Down Expand Up @@ -601,8 +603,8 @@ void NetInstantSend::ResolveBlockConflicts(const uint256& islockHash, const inst

BlockValidationState state;
// need non-const pointer
auto pindex2 = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(pindex->GetBlockHash()));
if (!m_chainstate.InvalidateBlock(state, pindex2)) {
auto pindex2 = WITH_LOCK(::cs_main, return m_chainman.ActiveChainstate().m_blockman.LookupBlockIndex(pindex->GetBlockHash()));
if (!m_chainman.ActiveChainstate().InvalidateBlock(state, pindex2)) {
LogPrintf("NetInstantSend::%s -- InvalidateBlock failed: %s\n", __func__, state.ToString());
// This should not have happened and we are in a state were it's not safe to continue anymore
assert(false);
Expand All @@ -612,13 +614,13 @@ void NetInstantSend::ResolveBlockConflicts(const uint256& islockHash, const inst
} else {
LogPrintf("NetInstantSend::%s -- resetting block %s\n", __func__, pindex2->GetBlockHash().ToString());
LOCK(::cs_main);
m_chainstate.ResetBlockFailureFlags(pindex2);
m_chainman.ActiveChainstate().ResetBlockFailureFlags(pindex2);
}
}

if (activateBestChain) {
BlockValidationState state;
if (!m_chainstate.ActivateBestChain(state)) {
if (!m_chainman.ActiveChainstate().ActivateBestChain(state)) {
LogPrintf("NetInstantSend::%s -- ActivateBestChain failed: %s\n", __func__, state.ToString());
// This should not have happened and we are in a state were it's not safe to continue anymore
assert(false);
Expand Down
8 changes: 4 additions & 4 deletions src/instantsend/net_instantsend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <thread>
#include <vector>

class Chainstate;
class ChainstateManager;

namespace Consensus {
struct LLMQParams;
Expand Down Expand Up @@ -44,15 +44,15 @@ class NetInstantSend final : public NetHandler, public CValidationInterface
public:
NetInstantSend(PeerManagerInternal* peer_manager, llmq::CInstantSendManager& is_manager,
instantsend::InstantSendSigner* signer, llmq::CSigningManager& sigman, llmq::CQuorumManager& qman,
const chainlock::Chainlocks& chainlocks, Chainstate& chainstate, CTxMemPool& mempool,
const chainlock::Chainlocks& chainlocks, const ChainstateManager& chainman, CTxMemPool& mempool,
const CMasternodeSync& mn_sync) :
NetHandler(peer_manager),
m_is_manager{is_manager},
m_signer{signer},
m_sigman{sigman},
m_qman(qman),
m_chainlocks{chainlocks},
m_chainstate{chainstate},
m_chainman{chainman},
m_mempool{mempool},
m_mn_sync{mn_sync}
{
Expand Down Expand Up @@ -113,7 +113,7 @@ class NetInstantSend final : public NetHandler, public CValidationInterface
llmq::CSigningManager& m_sigman;
llmq::CQuorumManager& m_qman;
const chainlock::Chainlocks& m_chainlocks;
Chainstate& m_chainstate;
const ChainstateManager& m_chainman;
CTxMemPool& m_mempool;
const CMasternodeSync& m_mn_sync;

Expand Down
9 changes: 5 additions & 4 deletions src/instantsend/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ using node::fReindex;
using node::GetTransaction;

namespace instantsend {
InstantSendSigner::InstantSendSigner(Chainstate& chainstate, const chainlock::Chainlocks& chainlocks,
InstantSendSigner::InstantSendSigner(const ChainstateManager& chainman, const chainlock::Chainlocks& chainlocks,
llmq::CInstantSendManager& isman, llmq::CSigningManager& sigman,
llmq::CSigSharesManager& shareman, llmq::CQuorumManager& qman,
CSporkManager& sporkman, CTxMemPool& mempool, const CMasternodeSync& mn_sync) :
m_chainstate{chainstate},
m_chainman{chainman},
m_chainlocks{chainlocks},
m_isman{isman},
m_sigman{sigman},
Expand Down Expand Up @@ -211,7 +211,7 @@ bool InstantSendSigner::CheckCanLock(const COutPoint& outpoint, bool printDebug,
if (auto ret = m_isman.GetCachedHeight(hashBlock)) {
blockHeight = *ret;
} else {
const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));
const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainman.m_blockman.LookupBlockIndex(hashBlock));
if (pindex == nullptr) {
if (printDebug) {
LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: failed to determine mined height for parent TX %s\n",
Expand Down Expand Up @@ -390,7 +390,8 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx)

const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
assert(llmq_params_opt);
const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, m_qman, id);
const CChain& active_chain = *WITH_LOCK(::cs_main, return &m_chainman.ActiveChain());
const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), active_chain, m_qman, id);

if (!quorum) {
LogPrint(BCLog::INSTANTSEND, "%s -- failed to select quorum. islock id=%s, txid=%s\n", __func__, id.ToString(),
Expand Down
5 changes: 3 additions & 2 deletions src/instantsend/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <threadsafety.h>

class CMasternodeSync;
class ChainstateManager;
class CSporkManager;
class CTxMemPool;

Expand All @@ -33,7 +34,7 @@ namespace instantsend {
class InstantSendSigner final : public llmq::CRecoveredSigsListener
{
private:
Chainstate& m_chainstate;
const ChainstateManager& m_chainman;
const chainlock::Chainlocks& m_chainlocks;
llmq::CInstantSendManager& m_isman;
llmq::CSigningManager& m_sigman;
Expand Down Expand Up @@ -66,7 +67,7 @@ class InstantSendSigner final : public llmq::CRecoveredSigsListener
InstantSendSigner() = delete;
InstantSendSigner(const InstantSendSigner&) = delete;
InstantSendSigner& operator=(const InstantSendSigner&) = delete;
explicit InstantSendSigner(Chainstate& chainstate, const chainlock::Chainlocks& chainlocks,
explicit InstantSendSigner(const ChainstateManager& chainman, const chainlock::Chainlocks& chainlocks,
llmq::CInstantSendManager& isman, llmq::CSigningManager& sigman,
llmq::CSigSharesManager& shareman, llmq::CQuorumManager& qman, CSporkManager& sporkman,
CTxMemPool& mempool, const CMasternodeSync& mn_sync);
Expand Down
Loading
Loading