Skip to content

Commit 002411d

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#25157: Fix -rpcwait with -netinfo returning negative time durations
3a998d2 Use steady_clock in ConnectAndCallRPC and inline time call in loop conditional (Jon Atack) 3799d2d Fix -rpcwait with -netinfo printing negative time durations (Jon Atack) Pull request description: - Fix `bitcoin-cli -rpcwait -netinfo 1` returning negative time durations on its first invocation after node startup in the "send", "recv", and "age" columns (potentially the "txn" and "blk" columns also). To reproduce, start bitcoind on mainnet (for a longer startup time) and run `bitcoin-cli -rpcwait -netinfo <n>` where n is 1 or larger. The negative time durations are larger with a slower CPU speed or e.g. higher `checkblocks`/`checklevel` config option settings. Examples: ``` <-> type net mping ping send recv txn blk hb addrp addrl age id out manual onion -126 -126 -2 0 ms ms sec sec min min min ``` ``` <-> type net mping ping send recv txn blk hb addrp addrl age id out manual cjdns -64 -64 -1 0 ms ms sec sec min min min ``` ``` <-> type net mping ping send recv txn blk hb addrp addrl age id out manual ipv4 -89 -89 * . -1 0 ms ms sec sec min min min ``` ``` <-> type net mping ping send recv txn blk hb addrp addrl age id out manual ipv6 -133 * . -2 0 ms ms sec sec min min min ``` - Use `steady_clock` in ConnectAndCallRPC and inline the time call in the loop conditional to avoid unnecessary invocations and an unneeded local variable allocation. ACKs for top commit: MarcoFalke: cr ACK 3a998d2 Tree-SHA512: 141430d47189ad9f646ce8e51cb31c21b395f6294bb27ba9f7ae4c1e1505a63209a4a19662a0b462806437a9cfd07f1ea114e775adc2872d87397fe823f8b8dc
2 parents 629e250 + 3a998d2 commit 002411d

File tree

1 file changed

+8
-9
lines changed

1 file changed

+8
-9
lines changed

src/bitcoin-cli.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,6 @@ class NetinfoRequestHandler : public BaseRequestHandler
439439
if (conn_type == "addr-fetch") return "addr";
440440
return "";
441441
}
442-
const int64_t m_time_now{count_seconds(Now<CliSeconds>())};
443442

444443
public:
445444
static constexpr int ID_PEERINFO = 0;
@@ -471,6 +470,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
471470
if (networkinfo["version"].get_int() < 209900) {
472471
throw std::runtime_error("-netinfo requires bitcoind server to be running v0.21.0 and up");
473472
}
473+
const int64_t time_now{count_seconds(Now<CliSeconds>())};
474474

475475
// Count peer connection totals, and if DetailsRequested(), store peer data in a vector of structs.
476476
for (const UniValue& peer : batch[ID_PEERINFO]["result"].getValues()) {
@@ -501,7 +501,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
501501
const double min_ping{peer["minping"].isNull() ? -1 : peer["minping"].get_real()};
502502
const double ping{peer["pingtime"].isNull() ? -1 : peer["pingtime"].get_real()};
503503
const std::string addr{peer["addr"].get_str()};
504-
const std::string age{conn_time == 0 ? "" : ToString((m_time_now - conn_time) / 60)};
504+
const std::string age{conn_time == 0 ? "" : ToString((time_now - conn_time) / 60)};
505505
const std::string sub_version{peer["subver"].get_str()};
506506
const bool is_addr_relay_enabled{peer["addr_relay_enabled"].isNull() ? false : peer["addr_relay_enabled"].get_bool()};
507507
const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()};
@@ -537,10 +537,10 @@ class NetinfoRequestHandler : public BaseRequestHandler
537537
peer.network,
538538
PingTimeToString(peer.min_ping),
539539
PingTimeToString(peer.ping),
540-
peer.last_send ? ToString(m_time_now - peer.last_send) : "",
541-
peer.last_recv ? ToString(m_time_now - peer.last_recv) : "",
542-
peer.last_trxn ? ToString((m_time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "",
543-
peer.last_blck ? ToString((m_time_now - peer.last_blck) / 60) : "",
540+
peer.last_send ? ToString(time_now - peer.last_send) : "",
541+
peer.last_recv ? ToString(time_now - peer.last_recv) : "",
542+
peer.last_trxn ? ToString((time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "",
543+
peer.last_blck ? ToString((time_now - peer.last_blck) / 60) : "",
544544
strprintf("%s%s", peer.is_bip152_hb_to ? "." : " ", peer.is_bip152_hb_from ? "*" : " "),
545545
m_max_addr_processed_length, // variable spacing
546546
peer.addr_processed ? ToString(peer.addr_processed) : peer.is_addr_relay_enabled ? "" : ".",
@@ -844,7 +844,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str
844844
// Execute and handle connection failures with -rpcwait.
845845
const bool fWait = gArgs.GetBoolArg("-rpcwait", false);
846846
const int timeout = gArgs.GetIntArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT);
847-
const auto deadline{GetTime<std::chrono::microseconds>() + 1s * timeout};
847+
const auto deadline{std::chrono::steady_clock::now() + 1s * timeout};
848848

849849
do {
850850
try {
@@ -857,8 +857,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str
857857
}
858858
break; // Connection succeeded, no need to retry.
859859
} catch (const CConnectionFailed& e) {
860-
const auto now{GetTime<std::chrono::microseconds>()};
861-
if (fWait && (timeout <= 0 || now < deadline)) {
860+
if (fWait && (timeout <= 0 || std::chrono::steady_clock::now() < deadline)) {
862861
UninterruptibleSleep(1s);
863862
} else {
864863
throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what()));

0 commit comments

Comments
 (0)