forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: implement Platform Ban PoSe DIP-0031 #6613
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.
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
Open
knst
wants to merge
8
commits into
dashpay:develop
Choose a base branch
from
knst:platform-pose-ban
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d2a5664
test: sort p2p messages list in p2p.py
knst c487ca7
test: removed on_feefilter that has never been implemented for Dash
knst 2c3f7dd
feat!: add new protocol version PLATFORM_BAN_VERSION = 70236
knst fe94d7d
feat!: implementation of Platform PoSe Ban p2p message
knst 9bb1cf4
test: add platforban message to test framework
knst c02ad41
test: functional test p2p_platform_ban.py
knst 0a4ee4c
fix: add missing LOCK for m_platform_ban
knst b94d725
fix: add list of valid mns for mine_quorum helper in p2p_platform_ban.py
knst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -38,6 +38,7 @@ | |||||||||||||||||||||||||||||||||||
#include <util/check.h> | ||||||||||||||||||||||||||||||||||||
#include <util/system.h> | ||||||||||||||||||||||||||||||||||||
#include <util/strencodings.h> | ||||||||||||||||||||||||||||||||||||
#include <util/underlying.h> | ||||||||||||||||||||||||||||||||||||
#include <util/trace.h> | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#include <algorithm> | ||||||||||||||||||||||||||||||||||||
|
@@ -712,6 +713,9 @@ class PeerManagerImpl final : public PeerManager | |||||||||||||||||||||||||||||||||||
const std::vector<CBlockHeader>& headers, | ||||||||||||||||||||||||||||||||||||
bool via_compact_block) | ||||||||||||||||||||||||||||||||||||
EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); | ||||||||||||||||||||||||||||||||||||
PeerMsgRet ProcessPlatformBanMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv) | ||||||||||||||||||||||||||||||||||||
EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/** Various helpers for headers processing, invoked by ProcessHeadersMessage() */ | ||||||||||||||||||||||||||||||||||||
/** Deal with state tracking and headers sync for peers that send the | ||||||||||||||||||||||||||||||||||||
* occasional non-connecting header (this can happen due to BIP 130 headers | ||||||||||||||||||||||||||||||||||||
|
@@ -2256,6 +2260,8 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv) | |||||||||||||||||||||||||||||||||||
#else | ||||||||||||||||||||||||||||||||||||
return m_cj_ctx->server->HasQueue(inv.hash); | ||||||||||||||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||||||||||||||
case MSG_PLATFORM_BAN: | ||||||||||||||||||||||||||||||||||||
return m_mn_metaman.AlreadyHavePlatformBan(inv.hash); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
@@ -2878,6 +2884,13 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic | |||||||||||||||||||||||||||||||||||
push = true; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
if (!push && inv.type == MSG_PLATFORM_BAN) { | ||||||||||||||||||||||||||||||||||||
auto opt_platform_ban = m_mn_metaman.GetPlatformBan(inv.hash); | ||||||||||||||||||||||||||||||||||||
if (opt_platform_ban.has_value()) { | ||||||||||||||||||||||||||||||||||||
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::PLATFORMBAN, *opt_platform_ban)); | ||||||||||||||||||||||||||||||||||||
push = true; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (!push) { | ||||||||||||||||||||||||||||||||||||
vNotFound.push_back(inv); | ||||||||||||||||||||||||||||||||||||
|
@@ -3506,6 +3519,74 @@ void PeerManagerImpl::PostProcessMessage(MessageProcessingResult&& result, NodeI | |||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
PeerMsgRet PeerManagerImpl::ProcessPlatformBanMessage(CNode& pfrom, std::string_view msg_type, CDataStream& vRecv) | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
Comment on lines
+3522
to
+3523
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Consider adding a brief docstring or inline comment for ProcessPlatformBanMessage to clarify its validation logic and the meaning of returned error codes, aiding future maintainers.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||
if (msg_type != NetMsgType::PLATFORMBAN) return {}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Do nothing if node is out of sync | ||||||||||||||||||||||||||||||||||||
if (!m_mn_sync.IsBlockchainSynced()) { | ||||||||||||||||||||||||||||||||||||
return {}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
PlatformBanMessage ban_msg; | ||||||||||||||||||||||||||||||||||||
vRecv >> ban_msg; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const uint256 hash = ban_msg.GetHash(); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- hash: %s protx_hash: %s height: %d peer=%d\n", hash.ToString(), ban_msg.m_protx_hash.ToString(), ban_msg.m_requested_height, pfrom.GetId()); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const auto list = Assert(m_dmnman)->GetListAtChainTip(); | ||||||||||||||||||||||||||||||||||||
auto dmn = list.GetMN(ban_msg.m_protx_hash); | ||||||||||||||||||||||||||||||||||||
if (!dmn) { | ||||||||||||||||||||||||||||||||||||
// small P2P penalty (1), as the evonode may have very recently been removed | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{1}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
if (dmn->nType != MnType::Evo) { | ||||||||||||||||||||||||||||||||||||
// Ban node, P2P penalty (100) if protx_hash is associated with a regular node not an evonode | ||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- hash: %s protx_hash: %s unexpected type of node\n", hash.ToString(), ban_msg.m_protx_hash.ToString()); | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{100}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
const int day_of_blocks = 576; | ||||||||||||||||||||||||||||||||||||
int tipHeight = WITH_LOCK(cs_main, return m_chainman.ActiveChainstate().m_chain.Height()); | ||||||||||||||||||||||||||||||||||||
if (tipHeight < ban_msg.m_requested_height || tipHeight - day_of_blocks > ban_msg.m_requested_height) { | ||||||||||||||||||||||||||||||||||||
// m_requested_height is inside the range [TipHeight - 576 - 5, TipHeight + 5] | ||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- hash: %s protx_hash: %s unexpected height: %d tip: %d\n", hash.ToString(), ban_msg.m_protx_hash.ToString(), ban_msg.m_requested_height, tipHeight); | ||||||||||||||||||||||||||||||||||||
if (tipHeight + 5 < ban_msg.m_requested_height || tipHeight - day_of_blocks - 5 > ban_msg.m_requested_height) { | ||||||||||||||||||||||||||||||||||||
// m_requested_height is outside the range [TipHeight - 576 - 5, TipHeight + 5] | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{10}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{1}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
Consensus::LLMQType llmq_type = Params().GetConsensus().llmqTypePlatform; | ||||||||||||||||||||||||||||||||||||
auto quorum = m_llmq_ctx->qman->GetQuorum(llmq_type, ban_msg.m_quorum_hash); | ||||||||||||||||||||||||||||||||||||
if (!quorum) { | ||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- hash: %s protx_hash: %s missing quorum_hash: %s llmq_type: %d\n", hash.ToString(), ban_msg.m_protx_hash.ToString(), ban_msg.m_quorum_hash.ToString(), ToUnderlying(llmq_type)); | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{100}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const std::string PLATFORM_BAN_REQUESTID_PREFIX = "PlatformPoSeBan"; | ||||||||||||||||||||||||||||||||||||
const auto data = std::make_pair(ban_msg.m_protx_hash, ban_msg.m_requested_height); | ||||||||||||||||||||||||||||||||||||
const uint256 request_id = ::SerializeHash(std::make_pair(PLATFORM_BAN_REQUESTID_PREFIX, data)); | ||||||||||||||||||||||||||||||||||||
const uint256 msg_hash = ::SerializeHash(data); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
auto ret = llmq::VerifyRecoveredSig(llmq_type, m_chainman.ActiveChainstate().m_chain, *m_llmq_ctx->qman, ban_msg.m_requested_height, request_id, msg_hash, ban_msg.m_signature); | ||||||||||||||||||||||||||||||||||||
if (ret != llmq::VerifyRecSigStatus::Valid) { | ||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- hash: %s protx_hash: %s request_id: %s msg_hash: %s sig validation failed: %d\n", hash.ToString(), ban_msg.m_protx_hash.ToString(), request_id.ToString(), msg_hash.ToString(), ToUnderlying(ret)); | ||||||||||||||||||||||||||||||||||||
return tl::unexpected{100}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// At this point, the outgoing message serialization version can't change. | ||||||||||||||||||||||||||||||||||||
const auto meta_info = m_mn_metaman.GetMetaInfo(ban_msg.m_protx_hash); | ||||||||||||||||||||||||||||||||||||
if (meta_info->SetPlatformBan(true, ban_msg.m_requested_height)) { | ||||||||||||||||||||||||||||||||||||
LogPrintf("PLATFORMBAN -- forward message to other nodes\n"); | ||||||||||||||||||||||||||||||||||||
m_mn_metaman.RememberPlatformBan(hash, ban_msg); | ||||||||||||||||||||||||||||||||||||
CInv platform_ban_inv{MSG_PLATFORM_BAN, hash}; | ||||||||||||||||||||||||||||||||||||
RelayInv(platform_ban_inv); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
return {}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
void PeerManagerImpl::ProcessMessage( | ||||||||||||||||||||||||||||||||||||
CNode& pfrom, | ||||||||||||||||||||||||||||||||||||
const std::string& msg_type, | ||||||||||||||||||||||||||||||||||||
|
@@ -5219,7 +5300,6 @@ void PeerManagerImpl::ProcessMessage( | |||||||||||||||||||||||||||||||||||
Misbehaving(pfrom.GetId(), 100, strprintf("received not-requested quorumrotationinfo. peer=%d", pfrom.GetId())); | ||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (msg_type == NetMsgType::NOTFOUND) { | ||||||||||||||||||||||||||||||||||||
// Remove the NOTFOUND transactions from the peer | ||||||||||||||||||||||||||||||||||||
LOCK(cs_main); | ||||||||||||||||||||||||||||||||||||
|
@@ -5273,6 +5353,7 @@ void PeerManagerImpl::ProcessMessage( | |||||||||||||||||||||||||||||||||||
ProcessPeerMsgRet(m_llmq_ctx->qman->ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom); | ||||||||||||||||||||||||||||||||||||
m_llmq_ctx->shareman->ProcessMessage(pfrom, *this, m_sporkman, msg_type, vRecv); | ||||||||||||||||||||||||||||||||||||
ProcessPeerMsgRet(m_llmq_ctx->sigman->ProcessMessage(pfrom, *this, msg_type, vRecv), pfrom); | ||||||||||||||||||||||||||||||||||||
ProcessPeerMsgRet(ProcessPlatformBanMessage(pfrom, msg_type, vRecv), pfrom); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (msg_type == NetMsgType::CLSIG) { | ||||||||||||||||||||||||||||||||||||
if (llmq::AreChainLocksEnabled(m_sporkman)) { | ||||||||||||||||||||||||||||||||||||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify signature validation for platform ban messages.
The PlatformBanMessage class handles serialization correctly, but ensure that signature validation is implemented in the corresponding cpp file. The class would benefit from a method to verify the signature against the hashed message data.
🏁 Script executed:
Length of output: 183
Action: Add Signature Verification Logic for PlatformBanMessage
It appears that the current implementation in
src/masternode/meta.h
only handles serialization and hash generation (viaGetHash()
), but no signature validation method is present in the corresponding source file. To ensure robust security for platform ban messages, please add a dedicated method (for example,bool ValidateSignature() const
) in the corresponding CPP file (likelysrc/masternode/meta.cpp
) that verifies the signature (m_signature
) against the message hash.src/masternode/meta.cpp
(or the appropriate implementation file forPlatformBanMessage
)GetHash()
).CBLSSignature
is correctly integrated.