highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-29 03:02:33 +01:00
Code Issues Packages Projects Releases Wiki Activity

p2p: Drop roles from sendtxrcncl

Browse Source 
This feature was currently redundant (although could have provided
more flexibility in the future), and already been causing confusion.
This commit is contained in:
Gleb Naumenko 2022-10-20 16:38:50 +03:00
parent 6772cbf69c
commit a60f729e29
7 changed files with 29 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/net_processing.cpp
View File

@ -3279,11 +3279,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// - we are not in -blocksonly mode.
if (pfrom.m_relays_txs && !m_ignore_incoming_txs) {
const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId());
// We suggest our txreconciliation role (initiator/responder) based on
// the connection direction.
m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL,
!pfrom.IsInboundConn(),
pfrom.IsInboundConn(),
TXRECONCILIATION_VERSION, recon_salt));
}
}
@ -3514,11 +3510,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}
bool is_peer_initiator, is_peer_responder;
uint32_t peer_txreconcl_version;
uint64_t remote_salt;
vRecv >> is_peer_initiator >> is_peer_responder >> peer_txreconcl_version >> remote_salt;
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());
@ -3526,10 +3517,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
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(),
is_peer_initiator, is_peer_responder,
peer_txreconcl_version,
remote_salt);
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,

31
src/node/txreconciliation.cpp
View File

@ -39,7 +39,8 @@ public:
* the following commits.
*
* Reconciliation protocol assumes using one role consistently: either a reconciliation
* initiator (requesting sketches), or responder (sending sketches). This defines our role.
* initiator (requesting sketches), or responder (sending sketches). This defines our role,
* based on the direction of the p2p connection.
*
*/
bool m_we_initiate;
@ -93,9 +94,8 @@ public:
return local_salt;
}
ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator,
bool is_peer_recon_responder, uint32_t peer_recon_version,
uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex)
ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, uint32_t peer_recon_version,
uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex)
{
AssertLockNotHeld(m_txreconciliation_mutex);
LOCK(m_txreconciliation_mutex);
@ -116,24 +116,11 @@ public:
// v1 is the lowest version, so suggesting something below must be a protocol violation.
if (recon_version < 1) return PROTOCOL_VIOLATION;
// Must match SENDTXRCNCL logic.
const bool they_initiate = is_peer_recon_initiator && is_peer_inbound;
const bool we_initiate = !is_peer_inbound && is_peer_recon_responder;
// If we ever announce support for both requesting and responding, this will need
// tie-breaking. For now, this is mutually exclusive because both are based on the
// inbound flag.
assert(!(they_initiate && we_initiate));
// The peer set both flags to false, we treat it as a protocol violation.
if (!(they_initiate || we_initiate)) return PROTOCOL_VIOLATION;
LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d with the following params: " /* Continued */
"we_initiate=%i, they_initiate=%i.\n",
peer_id, we_initiate, they_initiate);
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)};
recon_state->second = TxReconciliationState(we_initiate, full_salt.GetUint64(0), full_salt.GetUint64(1));
recon_state->second = TxReconciliationState(!is_peer_inbound, full_salt.GetUint64(0), full_salt.GetUint64(1));
return SUCCESS;
}
@ -166,11 +153,9 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id)
}
ReconciliationRegisterResult TxReconciliationTracker::RegisterPeer(NodeId peer_id, bool is_peer_inbound,
bool is_peer_recon_initiator, bool is_peer_recon_responder,
uint32_t peer_recon_version, uint64_t remote_salt)
{
return m_impl->RegisterPeer(peer_id, is_peer_inbound, is_peer_recon_initiator, is_peer_recon_responder,
peer_recon_version, remote_salt);
return m_impl->RegisterPeer(peer_id, is_peer_inbound, peer_recon_version, remote_salt);
}
void TxReconciliationTracker::ForgetPeer(NodeId peer_id)

4
src/node/txreconciliation.h
View File

@ -72,8 +72,8 @@ public:
* Step 0. Once the peer agreed to reconcile txs with us, generate the state required to track
* ongoing reconciliations. Must be called only after pre-registering the peer and only once.
*/
ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator,
bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt);
ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound,
uint32_t peer_recon_version, uint64_t remote_salt);
/**
* Attempts to forget txreconciliation-related state of the peer (if we previously stored any).

4
src/protocol.h
View File

@ -259,9 +259,7 @@ extern const char* CFCHECKPT;
*/
extern const char* WTXIDRELAY;
/**
* Contains 2 1-byte bools, a 4-byte version number and an 8-byte salt.
* The 2 booleans indicate that a node is willing to participate in transaction
* reconciliation, respectively as an initiator or as a receiver.
* Contains a 4-byte version number and an 8-byte salt.
* The salt is used to compute short txids needed for efficient
* txreconciliation, as described by BIP 330.
*/

26
src/test/txreconciliation_tests.cpp
View File

@ -18,33 +18,23 @@ BOOST_AUTO_TEST_CASE(RegisterPeerTest)
// Prepare a peer for reconciliation.
tracker.PreRegisterPeer(0);
// Both roles are false, don't register.
BOOST_CHECK(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true,
/*is_peer_recon_initiator=*/false,
/*is_peer_recon_responder=*/false,
/*peer_recon_version=*/1, salt) ==
ReconciliationRegisterResult::PROTOCOL_VIOLATION);
// Invalid roles for the given connection direction.
BOOST_CHECK(tracker.RegisterPeer(0, true, false, true, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION);
BOOST_CHECK(tracker.RegisterPeer(0, false, true, false, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION);
// Invalid version.
BOOST_CHECK(tracker.RegisterPeer(0, true, true, false, 0, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION);
BOOST_CHECK(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true,
/*peer_recon_version=*/0, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION);
// Valid registration.
BOOST_REQUIRE(!tracker.IsPeerRegistered(0));
BOOST_REQUIRE(tracker.RegisterPeer(0, true, true, false, 1, salt) == ReconciliationRegisterResult::SUCCESS);
BOOST_REQUIRE(tracker.RegisterPeer(0, true, 1, salt) == ReconciliationRegisterResult::SUCCESS);
BOOST_CHECK(tracker.IsPeerRegistered(0));
// Reconciliation version is higher than ours, should be able to register.
BOOST_REQUIRE(!tracker.IsPeerRegistered(1));
tracker.PreRegisterPeer(1);
BOOST_REQUIRE(tracker.RegisterPeer(1, true, true, false, 2, salt) == ReconciliationRegisterResult::SUCCESS);
BOOST_REQUIRE(tracker.RegisterPeer(1, true, 2, salt) == ReconciliationRegisterResult::SUCCESS);
BOOST_CHECK(tracker.IsPeerRegistered(1));
// Do not register if there were no pre-registration for the peer.
BOOST_REQUIRE(tracker.RegisterPeer(100, true, true, false, 1, salt) == ReconciliationRegisterResult::NOT_FOUND);
BOOST_REQUIRE(tracker.RegisterPeer(100, true, 1, salt) == ReconciliationRegisterResult::NOT_FOUND);
BOOST_CHECK(!tracker.IsPeerRegistered(100));
}
@ -56,12 +46,12 @@ BOOST_AUTO_TEST_CASE(ForgetPeerTest)
// Removing peer after pre-registring works and does not let to register the peer.
tracker.PreRegisterPeer(peer_id0);
tracker.ForgetPeer(peer_id0);
BOOST_CHECK(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::NOT_FOUND);
BOOST_CHECK(tracker.RegisterPeer(peer_id0, true, 1, 1) == ReconciliationRegisterResult::NOT_FOUND);
// Removing peer after it is registered works.
tracker.PreRegisterPeer(peer_id0);
BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0));
BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS);
BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, 1, 1) == ReconciliationRegisterResult::SUCCESS);
BOOST_CHECK(tracker.IsPeerRegistered(peer_id0));
tracker.ForgetPeer(peer_id0);
BOOST_CHECK(!tracker.IsPeerRegistered(peer_id0));
@ -76,7 +66,7 @@ BOOST_AUTO_TEST_CASE(IsPeerRegisteredTest)
tracker.PreRegisterPeer(peer_id0);
BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0));
BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS);
BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, 1, 1) == ReconciliationRegisterResult::SUCCESS);
BOOST_CHECK(tracker.IsPeerRegistered(peer_id0));
tracker.ForgetPeer(peer_id0);

34
test/functional/p2p_sendtxrcncl.py
View File

@ -54,10 +54,8 @@ class PeerTrackMsgOrder(P2PInterface):
super().on_message(message)
self.messages.append(message)
def create_sendtxrcncl_msg(initiator=True):
def create_sendtxrcncl_msg():
sendtxrcncl_msg = msg_sendtxrcncl()
sendtxrcncl_msg.initiator = initiator
sendtxrcncl_msg.responder = not initiator
sendtxrcncl_msg.version = 1
sendtxrcncl_msg.salt = 2
return sendtxrcncl_msg
@ -71,8 +69,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
self.log.info('SENDTXRCNCL sent to an inbound')
peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True)
assert peer.sendtxrcncl_msg_received
assert not peer.sendtxrcncl_msg_received.initiator
assert peer.sendtxrcncl_msg_received.responder
assert_equal(peer.sendtxrcncl_msg_received.version, 1)
self.nodes[0].disconnect_p2ps()
@ -117,22 +113,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
peer.send_message(create_sendtxrcncl_msg())
peer.wait_for_disconnect()
self.log.info('SENDTXRCNCL with initiator=responder=0 triggers a disconnect')
sendtxrcncl_no_role = create_sendtxrcncl_msg()
sendtxrcncl_no_role.initiator = False
sendtxrcncl_no_role.responder = False
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]):
peer.send_message(sendtxrcncl_no_role)
peer.wait_for_disconnect()
self.log.info('SENDTXRCNCL with initiator=0 and responder=1 from inbound triggers a disconnect')
sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=False)
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]):
peer.send_message(sendtxrcncl_wrong_role)
peer.wait_for_disconnect()
self.log.info('SENDTXRCNCL with version=0 triggers a disconnect')
sendtxrcncl_low_version = create_sendtxrcncl_msg()
sendtxrcncl_low_version.version = 0
@ -158,8 +138,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
peer = self.nodes[0].add_outbound_p2p_connection(
SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay")
assert peer.sendtxrcncl_msg_received
assert peer.sendtxrcncl_msg_received.initiator
assert not peer.sendtxrcncl_msg_received.responder
assert_equal(peer.sendtxrcncl_msg_received.version, 1)
self.nodes[0].disconnect_p2ps()
@ -178,15 +156,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
peer = self.nodes[0].add_outbound_p2p_connection(
PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="block-relay-only")
with self.nodes[0].assert_debug_log(["we indicated no tx relay; disconnecting"]):
peer.send_message(create_sendtxrcncl_msg(initiator=False))
peer.wait_for_disconnect()
self.log.info('SENDTXRCNCL with initiator=1 and responder=0 from outbound triggers a disconnect')
sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=True)
peer = self.nodes[0].add_outbound_p2p_connection(
PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="outbound-full-relay")
with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]):
peer.send_message(sendtxrcncl_wrong_role)
peer.send_message(create_sendtxrcncl_msg())
peer.wait_for_disconnect()
self.log.info('SENDTXRCNCL not sent if -txreconciliation flag is not set')

12
test/functional/test_framework/messages.py
View File

@ -1840,29 +1840,23 @@ class msg_cfcheckpt:
self.filter_type, self.stop_hash)
class msg_sendtxrcncl:
__slots__ = ("initiator", "responder", "version", "salt")
__slots__ = ("version", "salt")
msgtype = b"sendtxrcncl"
def __init__(self):
self.initiator = False
self.responder = False
self.version = 0
self.salt = 0
def deserialize(self, f):
self.initiator = struct.unpack("<?", f.read(1))[0]
self.responder = struct.unpack("<?", f.read(1))[0]
self.version = struct.unpack("<I", f.read(4))[0]
self.salt = struct.unpack("<Q", f.read(8))[0]
def serialize(self):
r = b""
r += struct.pack("<?", self.initiator)
r += struct.pack("<?", self.responder)
r += struct.pack("<I", self.version)
r += struct.pack("<Q", self.salt)
return r
def __repr__(self):
return "msg_sendtxrcncl(initiator=%i, responder=%i, version=%lu, salt=%lu)" %\
(self.initiator, self.responder, self.version, self.salt)
return "msg_sendtxrcncl(version=%lu, salt=%lu)" %\
(self.version, self.salt)