Skip to content
Merged
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
4 changes: 4 additions & 0 deletions doc/release-notes-6877.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
P2P and Network Changes
-----------------------

`MIN_PEER_PROTO_VERSION` has been bumped to `70221`
19 changes: 2 additions & 17 deletions src/evo/mnauth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode
}
auto pk = mn_activeman.GetPubKey();
const CBLSPublicKey pubKey(pk);
uint256 signHash = [&]() {
if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
return ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
} else {
return ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
}
}();
const uint256 signHash{::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion))};

mnauth.proRegTxHash = mn_activeman.GetProTxHash();

Expand Down Expand Up @@ -97,18 +91,9 @@ MessageProcessingResult CMNAuth::ProcessMessage(CNode& peer, ServiceFlags node_s
return MisbehavingError{10, "missing mnauth masternode"};
}

uint256 signHash;
int nOurNodeVersion{PROTOCOL_VERSION};
if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
nOurNodeVersion = gArgs.GetIntArg("-pushversion", PROTOCOL_VERSION);
}
const CBLSPublicKey pubKey(dmn->pdmnState->pubKeyOperator.Get());
// See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection)
if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
signHash = ::SerializeHash(std::make_tuple(pubKey, peer.GetSentMNAuthChallenge(), !peer.IsInboundConn()));
} else {
signHash = ::SerializeHash(std::make_tuple(pubKey, peer.GetSentMNAuthChallenge(), !peer.IsInboundConn(), peer.nVersion.load()));
}
const uint256 signHash{::SerializeHash(std::make_tuple(pubKey, peer.GetSentMNAuthChallenge(), !peer.IsInboundConn(), peer.nVersion.load()))};
LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, peer.nVersion, peer.GetId());

if (!mnauth.sig.VerifyInsecure(dmn->pdmnState->pubKeyOperator.Get(), signHash, false)) {
Expand Down
13 changes: 0 additions & 13 deletions src/governance/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,19 +1005,6 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c
continue;
}

if (peer.nVersion < GOVSCRIPT_PROTO_VERSION && govobj.GetObjectType() == GovernanceObject::PROPOSAL) {
// We know this proposal is valid locally, otherwise we would not store it.
// But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh
// because they won't accept it anyway and will simply ban us eventually.
CProposalValidator validator(govobj.GetDataAsHexString(), false /* no script */);
if (!validator.Validate(false /* ignore expiration */)) {
// The only way we could get here is when proposal is valid but payment_address is actually p2sh.
LogPrintf("CGovernanceManager::%s -- not syncing p2sh govobj to older node: %s, peer=%d\n", __func__,
strHash, peer.GetId());
continue;
}
}

// Push the inventory budget proposal message over to the other client
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing govobj: %s, peer=%d\n", __func__, strHash, peer.GetId());
ret.m_inventory.emplace_back(MSG_GOVERNANCE_OBJECT, nHash);
Expand Down
15 changes: 1 addition & 14 deletions src/governance/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -602,21 +602,8 @@ void CGovernanceObject::Relay(PeerManager& peerman, const CMasternodeSync& mn_sy
return;
}

int minProtoVersion = MIN_PEER_PROTO_VERSION;
if (m_obj.type == GovernanceObject::PROPOSAL) {
// We know this proposal is valid locally, otherwise we would not get to the point we should relay it.
// But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh
// because they won't accept it anyway and will simply ban us eventually.
CProposalValidator validator(GetDataAsHexString(), false /* no script */);
if (!validator.Validate(false /* ignore expiration */)) {
// The only way we could get here is when proposal is valid but payment_address is actually p2sh.
LogPrint(BCLog::GOBJECT, "CGovernanceObject::Relay -- won't relay %s to older peers\n", GetHash().ToString());
minProtoVersion = GOVSCRIPT_PROTO_VERSION;
}
}

CInv inv(MSG_GOVERNANCE_OBJECT, GetHash());
peerman.RelayInv(inv, minProtoVersion);
peerman.RelayInv(inv);
}

void CGovernanceObject::UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list)
Expand Down
4 changes: 0 additions & 4 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2193,10 +2193,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

// ********************************************************* Step 8: start indexers
if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) {
return InitError(*error);
}

g_txindex = std::make_unique<TxIndex>(cache_sizes.tx_index, false, fReindex);
if (!g_txindex->Start(chainman.ActiveChainstate())) {
return false;
Expand Down
4 changes: 0 additions & 4 deletions src/llmq/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,6 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, CQuorumC
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid pfrom: nullptr\n", __func__);
return false;
}
if (pfrom->nVersion < LLMQ_DATA_MESSAGES_VERSION) {
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Version must be %d or greater.\n", __func__, LLMQ_DATA_MESSAGES_VERSION);
return false;
}
if (pfrom->GetVerifiedProRegTxHash().IsNull()) {
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pfrom is not a verified masternode\n", __func__);
return false;
Expand Down
17 changes: 8 additions & 9 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,13 +650,13 @@ class PeerManagerImpl final : public PeerManager
void AskPeersForTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);

/** Relay inventories to peers that find it relevant */
void RelayInvFiltered(const CInv& inv, const CTransaction& relatedTx, const int minProtoVersion) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayInvFiltered(const CInv& inv, const CTransaction& relatedTx) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);

/**
* This overload will not update node filters, use it only for the cases
* when other messages will update related transaction data in filters
*/
void RelayInvFiltered(const CInv& inv, const uint256& relatedTxHash, const int minProtoVersion) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayInvFiltered(const CInv& inv, const uint256& relatedTxHash) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);

void EraseObjectRequest(NodeId nodeid, const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

Expand Down Expand Up @@ -2402,15 +2402,15 @@ void PeerManagerImpl::RelayDSQ(const CCoinJoinQueue& queue)
}
}

void PeerManagerImpl::RelayInvFiltered(const CInv& inv, const CTransaction& relatedTx, const int minProtoVersion)
void PeerManagerImpl::RelayInvFiltered(const CInv& inv, const CTransaction& relatedTx)
{
// TODO: Migrate to iteration through m_peer_map
m_connman.ForEachNode([&](CNode* pnode) {
PeerRef peer = GetPeerRef(pnode->GetId());
if (peer == nullptr) return;

auto tx_relay = peer->GetTxRelay();
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || tx_relay == nullptr) {
if (!pnode->CanRelay() || tx_relay == nullptr) {
return;
}

Expand All @@ -2427,15 +2427,15 @@ void PeerManagerImpl::RelayInvFiltered(const CInv& inv, const CTransaction& rela
});
}

void PeerManagerImpl::RelayInvFiltered(const CInv& inv, const uint256& relatedTxHash, const int minProtoVersion)
void PeerManagerImpl::RelayInvFiltered(const CInv& inv, const uint256& relatedTxHash)
{
// TODO: Migrate to iteration through m_peer_map
m_connman.ForEachNode([&](CNode* pnode) {
PeerRef peer = GetPeerRef(pnode->GetId());
if (peer == nullptr) return;

auto tx_relay = peer->GetTxRelay();
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || tx_relay == nullptr) {
if (!pnode->CanRelay() || tx_relay == nullptr) {
return;
}

Expand Down Expand Up @@ -3525,9 +3525,9 @@ void PeerManagerImpl::PostProcessMessage(MessageProcessingResult&& result, NodeI
if (result.m_inv_filter) {
const auto& [inv, filter] = result.m_inv_filter.value();
if (std::holds_alternative<CTransactionRef>(filter)) {
RelayInvFiltered(inv, *std::get<CTransactionRef>(filter), ISDLOCK_PROTO_VERSION);
RelayInvFiltered(inv, *std::get<CTransactionRef>(filter));
} else if (std::holds_alternative<uint256>(filter)) {
RelayInvFiltered(inv, std::get<uint256>(filter), ISDLOCK_PROTO_VERSION);
RelayInvFiltered(inv, std::get<uint256>(filter));
} else {
Comment on lines +3528 to 3531
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Avoid potential null dereference of CTransactionRef.

std::get(filter) can be empty; dereferencing would crash. Guard it.

-            RelayInvFiltered(inv, *std::get<CTransactionRef>(filter));
+            const auto& txref = std::get<CTransactionRef>(filter);
+            if (txref) {
+                RelayInvFiltered(inv, *txref);
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RelayInvFiltered(inv, *std::get<CTransactionRef>(filter));
} else if (std::holds_alternative<uint256>(filter)) {
RelayInvFiltered(inv, std::get<uint256>(filter), ISDLOCK_PROTO_VERSION);
RelayInvFiltered(inv, std::get<uint256>(filter));
} else {
const auto& txref = std::get<CTransactionRef>(filter);
if (txref) {
RelayInvFiltered(inv, *txref);
}
} else if (std::holds_alternative<uint256>(filter)) {
RelayInvFiltered(inv, std::get<uint256>(filter));
} else {
🤖 Prompt for AI Agents
In src/net_processing.cpp around lines 3528-3531, the code directly dereferences
std::get<CTransactionRef>(filter) which can be a null CTransactionRef and crash;
retrieve the CTransactionRef into a local variable, check it for non-null (e.g.
if (tx) ...) before dereferencing and calling RelayInvFiltered, and handle the
null case (skip relay or log/return) instead of dereferencing unconditionally.

assert(false);
}
Expand Down Expand Up @@ -6211,7 +6211,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

const auto islock = m_llmq_ctx->isman->GetInstantSendLockByTxid(hash);
if (islock == nullptr) continue;
if (pto->nVersion < ISDLOCK_PROTO_VERSION) continue;
uint256 isLockHash{::SerializeHash(*islock)};
tx_relay->m_tx_inventory_known_filter.insert(isLockHash);
queueAndMaybePushInv(CInv(MSG_ISDLOCK, isLockHash));
Expand Down
28 changes: 7 additions & 21 deletions src/txdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@

#include <chain.h>
#include <pow.h>
#include <primitives/transaction.h>
#include <random.h>
#include <shutdown.h>
#include <uint256.h>
#include <util/system.h>
#include <util/translation.h>
#include <util/vector.h>

#include <stdint.h>
#include <cassert>
#include <cstdlib>
#include <iterator>

static constexpr uint8_t DB_COIN{'C'};
static constexpr uint8_t DB_BLOCK_FILES{'f'};
Expand All @@ -32,26 +35,9 @@ static constexpr uint8_t DB_LAST_BLOCK{'l'};

// Keys used in previous version that might still be found in the DB:
static constexpr uint8_t DB_COINS{'c'};
static constexpr uint8_t DB_TXINDEX_BLOCK{'T'};
// uint8_t DB_TXINDEX{'t'}

std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db)
{
CBlockLocator ignored{};
if (block_tree_db.Read(DB_TXINDEX_BLOCK, ignored)) {
return _("The -txindex upgrade started by a previous version cannot be completed. Restart with the previous version or run a full -reindex.");
}
bool txindex_legacy_flag{false};
block_tree_db.ReadFlag("txindex", txindex_legacy_flag);
if (txindex_legacy_flag) {
// Disable legacy txindex and warn once about occupied disk space
if (!block_tree_db.WriteFlag("txindex", false)) {
return Untranslated("Failed to write block index db flag 'txindex'='0'");
}
return _("The block index db contains a legacy 'txindex'. To clear the occupied disk space, run a full -reindex, otherwise ignore this error. This error message will not be displayed again.");
}
return std::nullopt;
}
// CBlockTreeDB::DB_TXINDEX_BLOCK{'T'};
// CBlockTreeDB::DB_TXINDEX{'t'}
// CBlockTreeDB::ReadFlag("txindex")

bool CCoinsViewDB::NeedsUpgrade()
{
Expand Down
2 changes: 0 additions & 2 deletions src/txdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,4 @@ class CBlockTreeDB : public CDBWrapper
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
};

std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);

#endif // BITCOIN_TXDB_H
14 changes: 1 addition & 13 deletions src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,11 @@ static const int PROTOCOL_VERSION = 70238;
static const int INIT_PROTO_VERSION = 209;

//! disconnect from peers older than this proto version
static const int MIN_PEER_PROTO_VERSION = 70216;
static const int MIN_PEER_PROTO_VERSION = 70221;

//! minimum proto version of masternode to accept in DKGs
static const int MIN_MASTERNODE_PROTO_VERSION = 70238;

//! protocol version is included in MNAUTH starting with this version
static const int MNAUTH_NODE_VER_VERSION = 70218;

//! introduction of QGETDATA/QDATA messages
static const int LLMQ_DATA_MESSAGES_VERSION = 70219;

//! introduction of instant send deterministic lock (ISDLOCK)
static const int ISDLOCK_PROTO_VERSION = 70220;

//! GOVSCRIPT was activated in this version
static const int GOVSCRIPT_PROTO_VERSION = 70221;

//! ADDRV2 was introduced in this version
static const int ADDRV2_PROTO_VERSION = 70223;

Expand Down
5 changes: 4 additions & 1 deletion test/functional/feature_llmq_simplepose.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
)
from test_framework.util import assert_equal, force_finish_mnsync

# See version.h
MIN_MASTERNODE_PROTO_VERSION = 70238

class LLMQSimplePoSeTest(DashTestFramework):
def set_test_params(self):
# rotating quorums add instability for this functional tests
Expand Down Expand Up @@ -87,7 +90,7 @@ def close_mn_port(self, mn: MasternodeInfo):

def force_old_mn_proto(self, mn: MasternodeInfo):
self.stop_node(mn.nodeIdx)
self.start_masternode(mn, ["-pushversion=70216"])
self.start_masternode(mn, [f"-pushversion={MIN_MASTERNODE_PROTO_VERSION - 1}"])
self.connect_nodes(mn.nodeIdx, 0)
self.reset_probe_timeouts()
return False, True
Expand Down
97 changes: 0 additions & 97 deletions test/functional/feature_txindex_compatibility.py

This file was deleted.

1 change: 0 additions & 1 deletion test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@
'p2p_ping.py',
'p2p_sendtxrcncl.py',
'rpc_scantxoutset.py',
'feature_txindex_compatibility.py',
'feature_unsupported_utxo_db.py',
'feature_logging.py',
'feature_anchors.py',
Expand Down
Loading