Merge bitcoin/bitcoin#20541: Move special CAddress-without-nTime logic to net_processing

75290ae61e Drop us=... message in net debug for sending version message (Pieter Wuille)
5d47860334 refactor: move CAddress-without-nTime logic to net_processing (Pieter Wuille)

Pull request description:

  Historically, the VERSION message contains an "addrMe" and an "addrYou". As these are sent before version negotiation is complete, the protocol version is INIT_PROTO_VERSION (209), and in that protocol, CAddress is serialized without nTime.

  This is in fact the only situation left where a CAddress is (de)serialized without nTime. As it's such a simple structure (CService for ip/port + uint64_t for nServices), just inline that structure in the few places where it occurs, and remove the logic for dealing with missing nTime from CAddress.

ACKs for top commit:
  Zero-1729:
    crACK 75290ae61e
  jonatack:
    ACK 75290ae61e
  vasild:
    ACK 75290ae61e

Tree-SHA512: ccd9f478e1766fb2ad845d512b090e6297b82100640545ca490d930785801da3b429d40400bc2eb2cbe9057d5d12362ab33f8d650a1ca9e9e85857aef36ec414
This commit is contained in:
MarcoFalke 2021-08-23 18:19:25 +02:00
commit dbcb5742c4
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
2 changed files with 21 additions and 25 deletions

View file

@ -1087,25 +1087,25 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, int64_t nTime)
// Note that pnode->GetLocalServices() is a reflection of the local
// services we were offering when the CNode object was created for this
// peer.
ServiceFlags nLocalNodeServices = pnode.GetLocalServices();
uint64_t my_services{pnode.GetLocalServices()};
uint64_t nonce = pnode.GetLocalNonce();
const int nNodeStartingHeight{m_best_height};
NodeId nodeid = pnode.GetId();
CAddress addr = pnode.addr;
CAddress addrYou = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ?
addr :
CAddress(CService(), addr.nServices);
CAddress addrMe = CAddress(CService(), nLocalNodeServices);
CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService();
uint64_t your_services{addr.nServices};
const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr;
m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe,
m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime,
your_services, addr_you, // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime)
my_services, CService(), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime)
nonce, strSubVersion, nNodeStartingHeight, tx_relay));
if (fLogIPs) {
LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), tx_relay, nodeid);
LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, them=%s, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addr_you.ToString(), tx_relay, nodeid);
} else {
LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), tx_relay, nodeid);
LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, tx_relay, nodeid);
}
}
@ -2487,21 +2487,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
int64_t nTime;
CAddress addrMe;
CAddress addrFrom;
CService addrMe;
uint64_t nNonce = 1;
uint64_t nServiceInt;
ServiceFlags nServices;
int nVersion;
std::string cleanSubVer;
int starting_height = -1;
bool fRelay = true;
vRecv >> nVersion >> nServiceInt >> nTime >> addrMe;
vRecv >> nVersion >> Using<CustomUintFormatter<8>>(nServices) >> nTime;
if (nTime < 0) {
nTime = 0;
}
nServices = ServiceFlags(nServiceInt);
vRecv.ignore(8); // Ignore the addrMe service bits sent by the peer
vRecv >> addrMe;
if (!pfrom.IsInboundConn())
{
m_addrman.SetServices(pfrom.addr, nServices);
@ -2520,8 +2519,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}
if (!vRecv.empty())
vRecv >> addrFrom >> nNonce;
if (!vRecv.empty()) {
// The version message includes information about the sending node which we don't use:
// - 8 bytes (service bits)
// - 16 bytes (ipv6 address)
// - 2 bytes (port)
vRecv.ignore(26);
vRecv >> nNonce;
}
if (!vRecv.empty()) {
std::string strSubVer;
vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH);

View file

@ -396,7 +396,6 @@ public:
// ambiguous what that would mean. Make sure no code relying on that is introduced:
assert(!(s.GetType() & SER_GETHASH));
bool use_v2;
bool store_time;
if (s.GetType() & SER_DISK) {
// In the disk serialization format, the encoding (v1 or v2) is determined by a flag version
// that's part of the serialization itself. ADDRV2_FORMAT in the stream version only determines
@ -413,24 +412,16 @@ public:
} else {
throw std::ios_base::failure("Unsupported CAddress disk format version");
}
store_time = true;
} else {
// In the network serialization format, the encoding (v1 or v2) is determined directly by
// the value of ADDRV2_FORMAT in the stream version, as no explicitly encoded version
// exists in the stream.
assert(s.GetType() & SER_NETWORK);
use_v2 = s.GetVersion() & ADDRV2_FORMAT;
// The only time we serialize a CAddress object without nTime is in
// the initial VERSION messages which contain two CAddress records.
// At that point, the serialization version is INIT_PROTO_VERSION.
// After the version handshake, serialization version is >=
// MIN_PEER_PROTO_VERSION and all ADDR messages are serialized with
// nTime.
store_time = s.GetVersion() != INIT_PROTO_VERSION;
}
SER_READ(obj, obj.nTime = TIME_INIT);
if (store_time) READWRITE(obj.nTime);
READWRITE(obj.nTime);
// nServices is serialized as CompactSize in V2; as uint64_t in V1.
if (use_v2) {
uint64_t services_tmp;