Skip to content

Commit fce3897

Browse files
committed
Merge bitcoin#36427: [32.x] More Backports
101719c bitcoin-util: print genesis hash in display order (ViniciusCestarii) d66ea38 rpc: Fix OpenRPC hex pattern to allow empty strings (ViniciusCestarii) 1795af9 test: Extract and use sigterm_node in init_fd_overflow_test (MarcoFalke) db5b315 util: cap Sock::WaitMany timeout (Kanan) 54ae12e ci: Use one subnet per container (MarcoFalke) c4c4217 ci: Use one network per container and rm it after success (MarcoFalke) 90db304 net: remove PrivateBroadcast::DidNodeConfirmReception (Andrew Toth) 3c61fa2 net: always complete all initial private broadcast connections (Andrew Toth) f8bd7f0 test: assert HTTP receive throttle from server-side log (Matthew Zipkin) 2475cc8 http: log received bytes per client read (Matthew Zipkin) 6b6f33e fuzz: don't set random state for private broadcast test (Andrew Toth) Pull request description: Backports: * bitcoin#36277 * bitcoin#36303 * bitcoin#36404 * bitcoin#36435 * bitcoin#36340 * bitcoin#36392 * bitcoin#36462 * bitcoin#36471 ACKs for top commit: andrewtoth: ACK 101719c Tree-SHA512: 060a1e72eb40251cb950455cc60baf54fa0c847e915508c264b5941c010068073babeb99158f0d234cb32daf1191dfa2919213c25ddcfdca68af4487b7b84fae
2 parents 2633a1d + 101719c commit fce3897

25 files changed

Lines changed: 479 additions & 197 deletions

‎ci/test/02_run_container.py‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Distributed under the MIT software license, see the accompanying
44
# file COPYING or https://opensource.org/license/mit/.
55

6+
import ipaddress
67
from pathlib import Path
78
import os
89
import shlex
@@ -32,9 +33,22 @@ def main():
3233
# they will not be passed on.
3334
settings.update([
3435
"BASE_BUILD_DIR",
36+
"BIND_TEST_ROUTABLE_IPV4",
37+
"BIND_TEST_ROUTABLE_IPV6",
3538
"CI_FAILFAST_TEST_LEAVE_DANGLING",
3639
])
3740

41+
ci_idx = sorted(path.name for path in Path(__file__).parent.glob("00_setup_env_*.sh")).index(Path(os.environ["FILE_ENV"]).name)
42+
subnet_id = ci_idx % 256
43+
ip4_subnet = ipaddress.IPv4Network(f"11.11.{subnet_id}.0/24")
44+
ip4_addr = ip4_subnet[5]
45+
ip6_subnet = ipaddress.IPv6Network(f"1111:1111:{subnet_id:x}::/112")
46+
ip6_addr = ip6_subnet[5]
47+
48+
if not os.getenv("DANGER_RUN_CI_ON_HOST"):
49+
os.environ["BIND_TEST_ROUTABLE_IPV4"] = str(ip4_addr)
50+
os.environ["BIND_TEST_ROUTABLE_IPV6"] = str(ip6_addr)
51+
3852
# Append $USER to /tmp/env to support multi-user systems and $CONTAINER_NAME
3953
# to allow support starting multiple runs simultaneously by the same user.
4054
env_file = "/tmp/env-{u}-{c}".format(
@@ -114,8 +128,8 @@ def main():
114128
sys.exit(1)
115129
CI_CCACHE_MOUNT = f"type=bind,src={os.environ['CCACHE_DIR']},dst={os.environ['CCACHE_DIR']}"
116130

117-
run(["docker", "network", "create", "--ipv6", "--subnet", "1111:1111::/112", "ci-ip6net"], check=False)
118-
run(["docker", "network", "create", "--subnet", "1.1.1.0/24", "ci-ip4net"], check=False)
131+
network_name = f"{os.environ['CONTAINER_NAME']}-net"
132+
run(["docker", "network", "create", "--ipv6", f"--subnet={ip6_subnet}", f"--subnet={ip4_subnet}", network_name])
119133

120134
if os.getenv("RESTART_CI_DOCKER_BEFORE_RUN"):
121135
print("Restart docker before run to stop and clear all containers started with --rm")
@@ -144,8 +158,9 @@ def main():
144158
*CI_BUILD_MOUNT,
145159
f"--env-file={env_file}",
146160
f"--name={os.environ['CONTAINER_NAME']}",
147-
"--network=ci-ip6net",
148-
"--ip6=1111:1111::5", # Used by some of the tests, don't change it just here (keep them in sync).
161+
f"--network={network_name}",
162+
f"--ip={ip4_addr}",
163+
f"--ip6={ip6_addr}",
149164
f"--platform={os.environ['CI_IMAGE_PLATFORM']}",
150165
os.environ["CONTAINER_NAME"],
151166
]
@@ -156,8 +171,6 @@ def main():
156171
text=True,
157172
).stdout.strip()
158173

159-
run(["docker", "network", "connect", "--ip=1.1.1.5", "ci-ip4net", container_id]) # The IP address is used by some of the tests, don't change it just here (keep them in sync).
160-
161174
def ci_exec(cmd_inner, **kwargs):
162175
if os.getenv("DANGER_RUN_CI_ON_HOST"):
163176
prefix = []
@@ -199,6 +212,7 @@ def ci_exec(cmd_inner, **kwargs):
199212
if not os.getenv("DANGER_RUN_CI_ON_HOST"):
200213
print("Stop and remove CI container by ID")
201214
run(["docker", "container", "kill", container_id])
215+
run(["docker", "network", "rm", network_name])
202216

203217

204218
if __name__ == "__main__":

‎src/bitcoin-util.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ static int GetChainParams(const std::vector<std::string>& args, std::string& str
166166
UniValue result{UniValue::VOBJ};
167167
result.pushKV("chain", params.GetChainTypeString());
168168
result.pushKV("test_chain", params.IsTestChain());
169-
result.pushKV("genesis", HexStr(consensus.hashGenesisBlock));
169+
result.pushKV("genesis", consensus.hashGenesisBlock.GetHex());
170170
result.pushKV("subsidy_halving_interval", consensus.nSubsidyHalvingInterval);
171171

172172
if (consensus.signet_blocks) {

‎src/httpserver.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,16 @@ void HTTPRemoteClient::Receive()
966966
m_recv_buffer.end(),
967967
buf,
968968
buf + nrecv);
969+
970+
m_total_bytes_received += static_cast<uint64_t>(nrecv);
971+
LogDebug(
972+
BCLog::HTTP,
973+
"Received %d bytes from %s (id=%llu): total=%llu buffered=%zu",
974+
nrecv,
975+
m_origin,
976+
m_id,
977+
m_total_bytes_received,
978+
m_recv_buffer.size());
969979
}
970980
}
971981

‎src/httpserver.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,9 @@ class HTTPRemoteClient
563563
*/
564564
std::string m_recv_buffer{};
565565

566+
//! Running total of bytes read from this client's socket.
567+
uint64_t m_total_bytes_received{0};
568+
566569
//! Requests from a client must be processed in the order in which
567570
//! they were received, blocking on a per-client basis. We read
568571
//! one request at a time from the socket buffer then pass it to a worker.

‎src/net_processing.cpp‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,6 @@ static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1};
199199
* based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR
200200
* is exempt from this limit). */
201201
static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND};
202-
/** For private broadcast, send a transaction to this many peers. */
203-
static constexpr size_t NUM_PRIVATE_BROADCAST_PER_TX{3};
204202
/** Private broadcast connections must complete within this time. Disconnect the peer if it takes longer. */
205203
static constexpr auto PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME{3min};
206204

@@ -1745,7 +1743,7 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
17451743

17461744
void PeerManagerImpl::ReattemptPrivateBroadcast(CScheduler& scheduler)
17471745
{
1748-
// Remove stale transactions that are no longer relevant (e.g. already in
1746+
// Resolve stale transactions that are no longer relevant (e.g. already in
17491747
// the mempool or mined) and count the remaining ones.
17501748
size_t num_for_rebroadcast{0};
17511749
const auto stale_txs = m_tx_for_private_broadcast.GetStale();
@@ -1755,6 +1753,7 @@ void PeerManagerImpl::ReattemptPrivateBroadcast(CScheduler& scheduler)
17551753
LOCK(cs_main);
17561754
auto mempool_acceptable = m_chainman.ProcessTransaction(stale_tx, /*test_accept=*/true);
17571755
if (mempool_acceptable.m_result_type == MempoolAcceptResult::ResultType::VALID) {
1756+
if (!m_tx_for_private_broadcast.TryGrantRetry(stale_tx)) continue;
17581757
LogDebug(BCLog::PRIVBROADCAST,
17591758
"Reattempting broadcast of stale txid=%s wtxid=%s",
17601759
stale_tx->GetHash().ToString(), stale_tx->GetWitnessHash().ToString());
@@ -1763,7 +1762,7 @@ void PeerManagerImpl::ReattemptPrivateBroadcast(CScheduler& scheduler)
17631762
LogDebug(BCLog::PRIVBROADCAST, "Giving up broadcast attempts for txid=%s wtxid=%s: %s",
17641763
stale_tx->GetHash().ToString(), stale_tx->GetWitnessHash().ToString(),
17651764
mempool_acceptable.m_state.ToString());
1766-
m_tx_for_private_broadcast.Remove(stale_tx);
1765+
m_tx_for_private_broadcast.MarkResolved(stale_tx);
17671766
}
17681767
}
17691768

@@ -1843,11 +1842,18 @@ void PeerManagerImpl::FinalizeNode(const CNode& node)
18431842
LOCK(m_headers_presync_mutex);
18441843
m_headers_presync_stats.erase(nodeid);
18451844
}
1846-
if (node.IsPrivateBroadcastConn() &&
1847-
!m_tx_for_private_broadcast.DidNodeConfirmReception(nodeid) &&
1848-
m_tx_for_private_broadcast.HavePendingTransactions()) {
1849-
1850-
m_connman.m_private_broadcast.NumToOpenAdd(1);
1845+
if (node.IsPrivateBroadcastConn()) {
1846+
m_tx_for_private_broadcast.NodeDisconnected(nodeid);
1847+
// We consider a transaction sent to a peer if we sent them an INV.
1848+
// Normally they should request the transaction with GETDATA, but this may
1849+
// not happen if they are already aware of the transaction.
1850+
// If we didn't send them an INV, schedule a new connection to compensate
1851+
// for this send failure. Whether we sent them an INV is not readily
1852+
// available here, so use fSuccessfullyConnected because INV is sent right
1853+
// after a successful handshake when there is a pending transaction.
1854+
if (!node.fSuccessfullyConnected && m_tx_for_private_broadcast.HavePendingTransactions()) {
1855+
m_connman.m_private_broadcast.NumToOpenAdd(1);
1856+
}
18511857
}
18521858
LogDebug(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid);
18531859
}
@@ -1988,11 +1994,9 @@ std::vector<CTransactionRef> PeerManagerImpl::AbortPrivateBroadcast(const uint25
19881994
for (const auto& tx_info : snapshot) {
19891995
const CTransactionRef& tx{tx_info.tx};
19901996
if (tx->GetHash().ToUint256() != id && tx->GetWitnessHash().ToUint256() != id) continue;
1991-
if (const auto peer_acks{m_tx_for_private_broadcast.Remove(tx)}) {
1997+
if (const auto remaining_sends{m_tx_for_private_broadcast.Remove(tx)}) {
19921998
removed_txs.push_back(tx);
1993-
if (NUM_PRIVATE_BROADCAST_PER_TX > *peer_acks) {
1994-
connections_cancelled += (NUM_PRIVATE_BROADCAST_PER_TX - *peer_acks);
1995-
}
1999+
connections_cancelled += *remaining_sends;
19962000
}
19972001
}
19982002
m_connman.m_private_broadcast.NumToOpenSub(connections_cancelled);
@@ -2495,8 +2499,8 @@ node::TransactionError PeerManagerImpl::InitiateTxBroadcastPrivate(const CTransa
24952499
const auto txstr{strprintf("txid=%s, wtxid=%s", tx->GetHash().ToString(), tx->GetWitnessHash().ToString())};
24962500
switch (m_tx_for_private_broadcast.Add(tx)) {
24972501
case PrivateBroadcast::AddResult::Added:
2498-
LogDebug(BCLog::PRIVBROADCAST, "Requesting %d new connections due to %s", NUM_PRIVATE_BROADCAST_PER_TX, txstr);
2499-
m_connman.m_private_broadcast.NumToOpenAdd(NUM_PRIVATE_BROADCAST_PER_TX);
2502+
LogDebug(BCLog::PRIVBROADCAST, "Requesting %d new connections due to %s", PrivateBroadcast::INITIAL_CONNECTION_COUNT, txstr);
2503+
m_connman.m_private_broadcast.NumToOpenAdd(PrivateBroadcast::INITIAL_CONNECTION_COUNT);
25002504
return node::TransactionError::OK;
25012505
case PrivateBroadcast::AddResult::AlreadyPresent:
25022506
LogDebug(BCLog::PRIVBROADCAST, "Ignoring unnecessary request to schedule an already scheduled transaction: %s", txstr);
@@ -4722,15 +4726,10 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
47224726
const uint256& hash = peer.m_wtxid_relay ? wtxid.ToUint256() : txid.ToUint256();
47234727
AddKnownTx(peer, hash);
47244728

4725-
if (const auto num_broadcasted{m_tx_for_private_broadcast.Remove(ptx)}) {
4729+
if (m_tx_for_private_broadcast.MarkResolved(ptx)) {
47264730
LogDebug(BCLog::PRIVBROADCAST, "Received our privately broadcast transaction (txid=%s) from the "
4727-
"network from %s; stopping private broadcast attempts",
4731+
"network from %s",
47284732
txid.ToString(), pfrom.LogPeer());
4729-
if (NUM_PRIVATE_BROADCAST_PER_TX > num_broadcasted.value()) {
4730-
// Not all of the initial NUM_PRIVATE_BROADCAST_PER_TX connections were needed.
4731-
// Tell CConnman it does not need to start the remaining ones.
4732-
m_connman.m_private_broadcast.NumToOpenSub(NUM_PRIVATE_BROADCAST_PER_TX - num_broadcasted.value());
4733-
}
47344733
}
47354734

47364735
LOCK2(cs_main, m_tx_download_mutex);

‎src/private_broadcast.cpp‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,21 @@ PrivateBroadcast::AddResult PrivateBroadcast::Add(const CTransactionRef& tx)
1414
EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
1515
{
1616
LOCK(m_mutex);
17+
// Cleanup finished transactions
18+
std::erase_if(m_transactions, [this](const auto& entry) {
19+
const auto& state{entry.second};
20+
return state.resolved && !IsPending(state) &&
21+
std::ranges::all_of(state.send_statuses, [](const auto& status) { return status.disconnected; });
22+
});
23+
1724
if (const auto it{m_transactions.find(tx)}; it != m_transactions.end()) {
18-
if (IsPending(it->second)) return AddResult::AlreadyPresent;
25+
if (it->second.send_statuses.size() < m_max_send_attempts) return AddResult::AlreadyPresent;
1926

20-
// An exhausted transaction can be explicitly retried by adding it again.
27+
// A transaction that has reached m_max_send_attempts can be explicitly retried by adding it again.
2128
it->second.time_added = NodeClock::now();
2229
it->second.send_statuses.clear();
30+
it->second.planned_sends = INITIAL_CONNECTION_COUNT;
31+
it->second.resolved = false;
2332
return AddResult::Added;
2433
}
2534

@@ -35,12 +44,43 @@ std::optional<size_t> PrivateBroadcast::Remove(const CTransactionRef& tx)
3544
LOCK(m_mutex);
3645
const auto handle{m_transactions.extract(tx)};
3746
if (handle) {
38-
const auto p{DerivePriority(handle.mapped().send_statuses)};
39-
return p.num_confirmed;
47+
const auto& state{handle.mapped()};
48+
const size_t planned{std::min(state.planned_sends, m_max_send_attempts)};
49+
const size_t actual{state.send_statuses.size()};
50+
return planned > actual ? planned - actual : 0;
4051
}
4152
return std::nullopt;
4253
}
4354

55+
bool PrivateBroadcast::MarkResolved(const CTransactionRef& tx)
56+
{
57+
LOCK(m_mutex);
58+
const auto it{m_transactions.find(tx)};
59+
if (it == m_transactions.end()) return false;
60+
it->second.resolved = true;
61+
return true;
62+
}
63+
64+
void PrivateBroadcast::NodeDisconnected(NodeId nodeid)
65+
{
66+
LOCK(m_mutex);
67+
if (const auto entry{GetSendStatusByNode(nodeid)}) {
68+
entry->send_status.disconnected = true;
69+
}
70+
}
71+
72+
bool PrivateBroadcast::TryGrantRetry(const CTransactionRef& tx)
73+
{
74+
LOCK(m_mutex);
75+
const auto it{m_transactions.find(tx)};
76+
if (it == m_transactions.end() ||
77+
it->second.resolved ||
78+
IsPending(it->second) ||
79+
it->second.planned_sends >= m_max_send_attempts) return false;
80+
++it->second.planned_sends;
81+
return true;
82+
}
83+
4484
std::optional<CTransactionRef> PrivateBroadcast::PickTxForSend(const NodeId& will_send_to_nodeid, const CService& will_send_to_address)
4585
EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
4686
{
@@ -87,17 +127,6 @@ void PrivateBroadcast::NodeConfirmedReception(const NodeId& nodeid)
87127
}
88128
}
89129

90-
bool PrivateBroadcast::DidNodeConfirmReception(const NodeId& nodeid)
91-
EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
92-
{
93-
LOCK(m_mutex);
94-
const auto tx_and_status{GetSendStatusByNode(nodeid)};
95-
if (tx_and_status.has_value()) {
96-
return tx_and_status.value().send_status.confirmed.has_value();
97-
}
98-
return false;
99-
}
100-
101130
bool PrivateBroadcast::HavePendingTransactions()
102131
EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
103132
{
@@ -112,7 +141,7 @@ std::vector<CTransactionRef> PrivateBroadcast::GetStale() const
112141
const auto now{NodeClock::now()};
113142
std::vector<CTransactionRef> stale;
114143
for (const auto& [tx, state] : m_transactions) {
115-
if (!IsPending(state)) continue;
144+
if (state.resolved || state.planned_sends >= m_max_send_attempts) continue;
116145
const Priority p{DerivePriority(state.send_statuses)};
117146
if (p.num_confirmed == 0) {
118147
if (state.time_added < now - INITIAL_STALE_DURATION) stale.push_back(tx);
@@ -131,6 +160,7 @@ std::vector<PrivateBroadcast::TxBroadcastInfo> PrivateBroadcast::GetBroadcastInf
131160
entries.reserve(m_transactions.size());
132161

133162
for (const auto& [tx, state] : m_transactions) {
163+
if (state.resolved) continue;
134164
std::vector<PeerSendInfo> peers;
135165
peers.reserve(state.send_statuses.size());
136166
for (const auto& status : state.send_statuses) {
@@ -145,7 +175,9 @@ std::vector<PrivateBroadcast::TxBroadcastInfo> PrivateBroadcast::GetBroadcastInf
145175

146176
bool PrivateBroadcast::IsPending(const TxSendStatus& status) const
147177
{
148-
return status.send_statuses.size() < m_max_send_attempts;
178+
// Deliberately ignore resolved so all initially scheduled connections can complete.
179+
const size_t limit{std::min(status.planned_sends, m_max_send_attempts)};
180+
return status.send_statuses.size() < limit;
149181
}
150182

151183
PrivateBroadcast::Priority PrivateBroadcast::DerivePriority(const std::vector<SendStatus>& sent_to)

0 commit comments

Comments
 (0)