From 3799d2dcdd736dd24850b192e1b264bee1cd5e5a Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 16 May 2022 18:43:59 +0200 Subject: [PATCH 1/2] Fix -rpcwait with -netinfo printing negative time durations Fixes negative time duration values in the "send", "recv", and "age" columns (potentially the "txn" and "blk" columns also) for the first run of -rpcwait -netinfo after bitcoind startup. To reproduce, start bitcoind on mainnet and run `bitcoin-cli -rpcwait -netinfo ` where n is 1 or larger. The negative times will be larger/more apparent with a slower CPU speed or e.g. higher checkblocks/checklevel config option settings. --- src/bitcoin-cli.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 88b0e86a369..fd2e09542f9 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -439,7 +439,6 @@ private: if (conn_type == "addr-fetch") return "addr"; return ""; } - const int64_t m_time_now{count_seconds(Now())}; public: static constexpr int ID_PEERINFO = 0; @@ -471,6 +470,7 @@ public: if (networkinfo["version"].get_int() < 209900) { throw std::runtime_error("-netinfo requires bitcoind server to be running v0.21.0 and up"); } + const int64_t time_now{count_seconds(Now())}; // Count peer connection totals, and if DetailsRequested(), store peer data in a vector of structs. for (const UniValue& peer : batch[ID_PEERINFO]["result"].getValues()) { @@ -501,7 +501,7 @@ public: const double min_ping{peer["minping"].isNull() ? -1 : peer["minping"].get_real()}; const double ping{peer["pingtime"].isNull() ? -1 : peer["pingtime"].get_real()}; const std::string addr{peer["addr"].get_str()}; - const std::string age{conn_time == 0 ? "" : ToString((m_time_now - conn_time) / 60)}; + const std::string age{conn_time == 0 ? "" : ToString((time_now - conn_time) / 60)}; const std::string sub_version{peer["subver"].get_str()}; const bool is_addr_relay_enabled{peer["addr_relay_enabled"].isNull() ? false : peer["addr_relay_enabled"].get_bool()}; const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()}; @@ -537,10 +537,10 @@ public: peer.network, PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), - peer.last_send ? ToString(m_time_now - peer.last_send) : "", - peer.last_recv ? ToString(m_time_now - peer.last_recv) : "", - peer.last_trxn ? ToString((m_time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "", - peer.last_blck ? ToString((m_time_now - peer.last_blck) / 60) : "", + peer.last_send ? ToString(time_now - peer.last_send) : "", + peer.last_recv ? ToString(time_now - peer.last_recv) : "", + peer.last_trxn ? ToString((time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "", + peer.last_blck ? ToString((time_now - peer.last_blck) / 60) : "", strprintf("%s%s", peer.is_bip152_hb_to ? "." : " ", peer.is_bip152_hb_from ? "*" : " "), m_max_addr_processed_length, // variable spacing peer.addr_processed ? ToString(peer.addr_processed) : peer.is_addr_relay_enabled ? "" : ".", From 3a998d2e37bf76667b08cd947807ada1305520d7 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 17 May 2022 14:46:13 +0200 Subject: [PATCH 2/2] Use steady_clock in ConnectAndCallRPC and inline time call in loop conditional to avoid unnecessary invocations and an unneeded local variable allocation. --- src/bitcoin-cli.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index fd2e09542f9..640643a695a 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -844,7 +844,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str // Execute and handle connection failures with -rpcwait. const bool fWait = gArgs.GetBoolArg("-rpcwait", false); const int timeout = gArgs.GetIntArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); - const auto deadline{GetTime() + 1s * timeout}; + const auto deadline{std::chrono::steady_clock::now() + 1s * timeout}; do { try { @@ -857,8 +857,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } break; // Connection succeeded, no need to retry. } catch (const CConnectionFailed& e) { - const auto now{GetTime()}; - if (fWait && (timeout <= 0 || now < deadline)) { + if (fWait && (timeout <= 0 || std::chrono::steady_clock::now() < deadline)) { UninterruptibleSleep(1s); } else { throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what()));