From bc84e24a4f0736919ea4a76f7d45085587625aba Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Mon, 14 Nov 2022 11:37:28 +0200 Subject: [PATCH] p2p, refactor: Switch to enum class for ReconciliationRegisterResult While doing this, add a new value: ALREADY_REGISTERED. --- src/net_processing.cpp | 21 +++++++++------------ src/node/txreconciliation.cpp | 18 ++++++++++-------- src/node/txreconciliation.h | 9 +++++---- src/test/txreconciliation_tests.cpp | 3 +++ 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 71bf48798d9..e90951ae1fc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3510,24 +3510,21 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (m_txreconciliation->IsPeerRegistered(pfrom.GetId())) { - // A peer is already registered, meaning we already received SENDTXRCNCL from them. - LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId()); - pfrom.fDisconnect = true; - return; - } - uint32_t peer_txreconcl_version; uint64_t remote_salt; vRecv >> peer_txreconcl_version >> remote_salt; const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(), peer_txreconcl_version, remote_salt); - - // If it's a protocol violation, disconnect. - // If the peer was not found (but something unexpected happened) or it was registered, - // nothing to be done. - if (result == ReconciliationRegisterResult::PROTOCOL_VIOLATION) { + switch (result) { + case ReconciliationRegisterResult::NOT_FOUND: + case ReconciliationRegisterResult::SUCCESS: + break; + case ReconciliationRegisterResult::ALREADY_REGISTERED: + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId()); + pfrom.fDisconnect = true; + return; + case ReconciliationRegisterResult::PROTOCOL_VIOLATION: LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index 3552cfd8f29..7b50b41dc74 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -101,11 +101,13 @@ public: LOCK(m_txreconciliation_mutex); auto recon_state = m_states.find(peer_id); - // A peer should be in the pre-registered state to proceed here. - if (recon_state == m_states.end()) return NOT_FOUND; - uint64_t* local_salt = std::get_if(&recon_state->second); - // A peer is already registered. This should be checked by the caller. - Assume(local_salt); + if (recon_state == m_states.end()) return ReconciliationRegisterResult::NOT_FOUND; + + if (std::holds_alternative(recon_state->second)) { + return ReconciliationRegisterResult::ALREADY_REGISTERED; + } + + uint64_t local_salt = *std::get_if(&recon_state->second); // If the peer supports the version which is lower than ours, we downgrade to the version // it supports. For now, this only guarantees that nodes with future reconciliation @@ -114,14 +116,14 @@ public: // satisfactory (e.g. too low). const uint32_t recon_version{std::min(peer_recon_version, m_recon_version)}; // v1 is the lowest version, so suggesting something below must be a protocol violation. - if (recon_version < 1) return PROTOCOL_VIOLATION; + if (recon_version < 1) return ReconciliationRegisterResult::PROTOCOL_VIOLATION; LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d (inbound=%i)\n", peer_id, is_peer_inbound); - const uint256 full_salt{ComputeSalt(*local_salt, remote_salt)}; + const uint256 full_salt{ComputeSalt(local_salt, remote_salt)}; recon_state->second = TxReconciliationState(!is_peer_inbound, full_salt.GetUint64(0), full_salt.GetUint64(1)); - return SUCCESS; + return ReconciliationRegisterResult::SUCCESS; } void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index caaf1777e9b..4591dd5df7c 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -16,10 +16,11 @@ static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false}; /** Supported transaction reconciliation protocol version */ static constexpr uint32_t TXRECONCILIATION_VERSION{1}; -enum ReconciliationRegisterResult { - NOT_FOUND = 0, - SUCCESS = 1, - PROTOCOL_VIOLATION = 2, +enum class ReconciliationRegisterResult { + NOT_FOUND, + SUCCESS, + ALREADY_REGISTERED, + PROTOCOL_VIOLATION, }; /** diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp index 6b72e5d0f04..1d6d4840c1d 100644 --- a/src/test/txreconciliation_tests.cpp +++ b/src/test/txreconciliation_tests.cpp @@ -33,6 +33,9 @@ BOOST_AUTO_TEST_CASE(RegisterPeerTest) BOOST_REQUIRE(tracker.RegisterPeer(1, true, 2, salt) == ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(1)); + // Try registering for the second time. + BOOST_REQUIRE(tracker.RegisterPeer(1, false, 1, salt) == ReconciliationRegisterResult::ALREADY_REGISTERED); + // Do not register if there were no pre-registration for the peer. BOOST_REQUIRE(tracker.RegisterPeer(100, true, 1, salt) == ReconciliationRegisterResult::NOT_FOUND); BOOST_CHECK(!tracker.IsPeerRegistered(100));