p2p: first addr self-announcement in separate msg
This makes sure the initial address self-announcement a node sends to a peer happends in a separate P2P message. This has benefits for both inbound and outbound connections: For inbound connections from a peer to us, previously, we might send the self-announcement along with our response to a GETADDR request. However, the self-announcement might replace an address from the GETADDR response. This isn't clean. For outbound connections from us to a peer, previously, it could have happend that we send the self-announcement along with other addresses. Since shortly after connection open, the peer might only have one rate-limiting token for us, and the addresses are shuffeld on arrival, it's possible that the self-announcement gets rate-limited. However, note that these rate-limitings seem to be rare in practice. This is inspired by and based on https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3462287763 Co-Authored-By: Anthony Towns <aj@erisian.com.au>
This commit is contained in:
2 files changed
+26
-3
No files matched your search
@@ -5328,8 +5328,21 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
|
|||||||
}
|
}
|
||||||
if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) {
|
if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) {
|
||||||
CAddress local_addr{*local_service, peer.m_our_services, Now<NodeSeconds>()};
|
CAddress local_addr{*local_service, peer.m_our_services, Now<NodeSeconds>()};
|
||||||
|
if (peer.m_next_local_addr_send == 0us) {
|
||||||
|
// Send the initial self-announcement in its own message. This makes sure
|
||||||
|
// rate-limiting with limited start-tokens doesn't ignore it if the first
|
||||||
|
// message ends up containing multiple addresses.
|
||||||
|
std::vector<CAddress> self_announcement {local_addr};
|
||||||
|
if (peer.m_wants_addrv2) {
|
||||||
|
MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement));
|
||||||
|
} else {
|
||||||
|
MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(self_announcement));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// All later self-announcements are sent together with the other addresses.
|
||||||
PushAddress(peer, local_addr);
|
PushAddress(peer, local_addr);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
peer.m_next_local_addr_send = current_time + m_rng.rand_exp_duration(AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
|
peer.m_next_local_addr_send = current_time + m_rng.rand_exp_duration(AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -5,6 +5,9 @@
|
|||||||
"""
|
"""
|
||||||
Test that a node sends a self-announcement with its external IP to
|
Test that a node sends a self-announcement with its external IP to
|
||||||
in- and outbound peers after connection open and again sometime later.
|
in- and outbound peers after connection open and again sometime later.
|
||||||
|
|
||||||
|
Additionally, this checks that the first self-announcement arrives
|
||||||
|
in its own message and that this message is the first we receive.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import time
|
import time
|
||||||
@@ -42,6 +45,13 @@ class SelfAnnouncementReceiver(P2PInterface):
|
|||||||
self.addresses_received += 1
|
self.addresses_received += 1
|
||||||
if addr == self.expected:
|
if addr == self.expected:
|
||||||
self.self_announcements_received += 1
|
self.self_announcements_received += 1
|
||||||
|
if self.self_announcements_received == 1:
|
||||||
|
# If it's the first self-announcement, check that it is
|
||||||
|
# in the first addr message we receive, and that this message
|
||||||
|
# only contains one address. This also implies that it is
|
||||||
|
# the first address we receive.
|
||||||
|
assert_equal(self.addr_messages_received, 1)
|
||||||
|
assert_equal(len(message.addrs), 1)
|
||||||
|
|
||||||
def on_addrv2(self, message):
|
def on_addrv2(self, message):
|
||||||
assert (self.addrv2_test)
|
assert (self.addrv2_test)
|
||||||
@@ -69,10 +79,10 @@ class AddrSelfAnnouncementTest(BitcoinTestFramework):
|
|||||||
self.self_announcement_test(outbound=True, addrv2=True)
|
self.self_announcement_test(outbound=True, addrv2=True)
|
||||||
|
|
||||||
def inbound_connection_open_assertions(self, addr_receiver):
|
def inbound_connection_open_assertions(self, addr_receiver):
|
||||||
# We expect one self-announcement and multiple other addresses in
|
# In response to a GETADDR, we expect a message with the self-announcement
|
||||||
# response to a GETADDR in a single addr / addrv2 message.
|
# and an addr message containing the GETADDR response.
|
||||||
assert_equal(addr_receiver.self_announcements_received, 1)
|
assert_equal(addr_receiver.self_announcements_received, 1)
|
||||||
assert_equal(addr_receiver.addr_messages_received, 1)
|
assert_equal(addr_receiver.addr_messages_received, 2)
|
||||||
assert_greater_than(addr_receiver.addresses_received, 1)
|
assert_greater_than(addr_receiver.addresses_received, 1)
|
||||||
|
|
||||||
def outbound_connection_open_assertions(self, addr_receiver):
|
def outbound_connection_open_assertions(self, addr_receiver):
|
||||||
|
|||||||
Reference in new issue
Block a user