From a7fcc8eb59fe51473571661316214156fbdbdcae Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Mon, 1 Feb 2021 17:55:38 +0100 Subject: [PATCH 1/4] rpc: Add a `-rpcwaittimeout` parameter to limit time spent waiting Adds a new numeric `-rpcwaittimeout` that can be used to limit the time we spend waiting on the RPC server to appear. This is used by downstream projects to provide a bit of slack when `bitcoind`s RPC interface is not available right away. --- src/bitcoin-cli.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index c5840b2098..3f96963d94 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -40,6 +40,7 @@ UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; +static constexpr int DEFAULT_WAIT_CLIENT_TIMEOUT = 0; static const bool DEFAULT_NAMED=false; static const int CONTINUE_EXECUTION=-1; static constexpr int8_t UNKNOWN_NETWORK{-1}; @@ -73,6 +74,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-rpcport=", strprintf("Connect to JSON-RPC on (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcuser=", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwait", "Wait for RPC server to start", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-rpcwaittimeout=", strprintf("Timeout in seconds to wait for the RPC server to start, or 0 for no timeout. (default: %d)", DEFAULT_WAIT_CLIENT_TIMEOUT), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwallet=", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind). This changes the RPC endpoint used, e.g. http://127.0.0.1:8332/wallet/", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdinrpcpass", "Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password. When combined with -stdinwalletpassphrase, -stdinrpcpass consumes the first line, and -stdinwalletpassphrase consumes the second.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -794,6 +796,9 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str UniValue response(UniValue::VOBJ); // Execute and handle connection failures with -rpcwait. const bool fWait = gArgs.GetBoolArg("-rpcwait", false); + const int timeout = gArgs.GetArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); + const int64_t deadline = GetTime().count() + timeout; + do { try { response = CallRPC(rh, strMethod, args, rpcwallet); @@ -805,8 +810,9 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } break; // Connection succeeded, no need to retry. } catch (const CConnectionFailed&) { - if (fWait) { - UninterruptibleSleep(std::chrono::milliseconds{1000}); + const int64_t now = GetTime().count(); + if (fWait && (timeout <= 0 || now < deadline)) { + UninterruptibleSleep(std::chrono::seconds{1}); } else { throw; } From c490e17ef698a1695050f82ef6567b3b87a21861 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 16 Feb 2021 15:07:13 +0100 Subject: [PATCH 2/4] doc: Add release notes for the `-rpcwaittimeout` cli parameter --- doc/release-notes-21056.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doc/release-notes-21056.md diff --git a/doc/release-notes-21056.md b/doc/release-notes-21056.md new file mode 100644 index 0000000000..2201a8cdae --- /dev/null +++ b/doc/release-notes-21056.md @@ -0,0 +1,6 @@ +New bitcoin-cli settings +------------------------ + +- A new `-rpcwaittimeout` argument to `bitcoin-cli` sets the timeout + in seconds to use with `-rpcwait`. If the timeout expires, + `bitcoin-cli` will report a failure. (#21056) From f76cb10d7dc9a7b0c55d28011161606399417664 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 23 Mar 2021 14:39:25 +0100 Subject: [PATCH 3/4] rpc: Prefix rpcwaittimeout error with details on its nature As proposed by @laanwj the error message is now prefixed with the "timeout on transient error:" prefix, to explain why the error is suddenly considered terminal. --- src/bitcoin-cli.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 3f96963d94..adc485983f 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -809,12 +809,12 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } } break; // Connection succeeded, no need to retry. - } catch (const CConnectionFailed&) { + } catch (const CConnectionFailed& e) { const int64_t now = GetTime().count(); if (fWait && (timeout <= 0 || now < deadline)) { UninterruptibleSleep(std::chrono::seconds{1}); } else { - throw; + throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what())); } } } while (fWait); From b9e76f1bf08c52fcd402b2314e00db4ad247ebc8 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 6 Apr 2021 13:06:41 +0200 Subject: [PATCH 4/4] rpc: Add test for -rpcwaittimeout Suggested-by: Jon Atack <@jonatack> --- test/functional/interface_bitcoin_cli.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 30cd499b3f..22eec59600 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -10,10 +10,12 @@ from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than_or_equal, assert_raises_process_error, assert_raises_rpc_error, get_auth_cookie, ) +import time # The block reward of coinbaseoutput.nValue (50) BTC/block matures after # COINBASE_MATURITY (100) blocks. Therefore, after mining 101 blocks we expect @@ -248,6 +250,12 @@ class TestBitcoinCli(BitcoinTestFramework): self.nodes[0].wait_for_rpc_connection() assert_equal(blocks, BLOCKS + 25) + self.log.info("Test -rpcwait option waits at most -rpcwaittimeout seconds for startup") + self.stop_node(0) # stop the node so we time out + start_time = time.time() + assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcwait', '-rpcwaittimeout=5').echo) + assert_greater_than_or_equal(time.time(), start_time + 5) + if __name__ == '__main__': TestBitcoinCli().main()