Merge #18691: test: add wait_for_cookie_credentials() to framework for rpcwait tests

92fe537cf704dfb4ae830c8c8b382f08c4893e65 test: fix intermittent race condition in interface_bitcoin_cli.py (Jon Atack)
c648e636b2f230db5f1d1137088685f74ae42454 test: add wait_for_cookie_credentials() to test framework (Jon Atack)

Pull request description:

  This PR adds a `wait_for_cookie_credentials()` method to the test framework and calls it before the `-rpcwait` tests, to avoid an intermittent race condition on the CI run with Valgrind where the cookie file isn't written yet when the CLI call with `-rpcwait` arrives to `get_auth_cookie()`.

  To reproduce/test, build with
  ```diff
  diff --git a/src/httprpc.cpp b/src/httprpc.cpp
  index 60c4d06f12..3dd06c4758 100644
  --- a/src/httprpc.cpp
  +++ b/src/httprpc.cpp
  @@ -291,6 +291,7 @@ static bool InitRPCAuthentication()
   bool StartHTTPRPC()
   {
       LogPrint(BCLog::RPC, "Starting HTTP RPC server\n");
  +    UninterruptibleSleep(std::chrono::seconds{11});
       if (!InitRPCAuthentication())
  ```
  then run the test normally and with valgrind
  ```
  test/functional/interface_bitcoin_cli.py -l debug
  valgrind test/functional/interface_bitcoin_cli.py -l debug
  ```
  Thanks to Marco Falke for all the help. Closes #18684.

Top commit has no ACKs.

Tree-SHA512: 1b76635b5b1d6b05138affef7ab788aa3bc3fc75b0c69ba778ecdf81063cfe02a8dd7667cfd63a6c6e19b2dac47d7a8b755e334d8af5c0ab9d4026808ee96c83

2 files changed, 29 insertions, 17 deletions

diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
index e7ece8b9b1..2e80d7a248 100755
--- a/test/functional/interface_bitcoin_cli.py
+++ b/test/functional/interface_bitcoin_cli.py
@@ -13,7 +13,6 @@ BLOCKS = 101
 BALANCE = (BLOCKS - 100) * 50
 
 class TestBitcoinCli(BitcoinTestFramework):
-
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
@@ -33,12 +32,12 @@ class TestBitcoinCli(BitcoinTestFramework):
         user, password = get_auth_cookie(self.nodes[0].datadir, self.chain)
 
         self.log.info("Test -stdinrpcpass option")
-        assert_equal(BLOCKS, self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input=password).getblockcount())
-        assert_raises_process_error(1, "Incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input="foo").echo)
+        assert_equal(BLOCKS, self.nodes[0].cli('-rpcuser={}'.format(user), '-stdinrpcpass', input=password).getblockcount())
+        assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword', self.nodes[0].cli('-rpcuser={}'.format(user), '-stdinrpcpass', input='foo').echo)
 
         self.log.info("Test -stdin and -stdinrpcpass")
-        assert_equal(["foo", "bar"], self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input=password + "\nfoo\nbar").echo())
-        assert_raises_process_error(1, "Incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input="foo").echo)
+        assert_equal(['foo', 'bar'], self.nodes[0].cli('-rpcuser={}'.format(user), '-stdin', '-stdinrpcpass', input=password + '\nfoo\nbar').echo())
+        assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword', self.nodes[0].cli('-rpcuser={}'.format(user), '-stdin', '-stdinrpcpass', input='foo').echo)
 
         self.log.info("Test connecting to a non-existing server")
         assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcport=1').echo)
@@ -52,7 +51,7 @@ class TestBitcoinCli(BitcoinTestFramework):
         self.log.info("Test -getinfo returns expected network and blockchain info")
         if self.is_wallet_compiled():
             self.nodes[0].encryptwallet(password)
-        cli_get_info = self.nodes[0].cli('-getinfo').send_cli()
+        cli_get_info = self.nodes[0].cli().send_cli('-getinfo')
         network_info = self.nodes[0].getnetworkinfo()
         blockchain_info = self.nodes[0].getblockchaininfo()
         assert_equal(cli_get_info['version'], network_info['version'])
@@ -76,20 +75,17 @@ class TestBitcoinCli(BitcoinTestFramework):
         else:
             self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped")
 
-        self.stop_node(0)
-
         self.log.info("Test -version with node stopped")
-        cli_response = self.nodes[0].cli("-version").send_cli()
+        self.stop_node(0)
+        cli_response = self.nodes[0].cli().send_cli('-version')
         assert "{} RPC client version".format(self.config['environment']['PACKAGE_NAME']) in cli_response
 
-        self.log.info("Test -rpcwait option waits for RPC connection instead of failing")
-        # Start node without RPC connection.
-        self.nodes[0].start()
-        # Verify failure without -rpcwait.
-        assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('getblockcount').echo)
-        # Verify success using -rpcwait.
-        assert_equal(BLOCKS, self.nodes[0].cli('-rpcwait', 'getblockcount').send_cli())
+        self.log.info("Test -rpcwait option successfully waits for RPC connection")
+        self.nodes[0].start()  # start node without RPC connection
+        self.nodes[0].wait_for_cookie_credentials()  # ensure cookie file is available to avoid race condition
+        blocks = self.nodes[0].cli('-rpcwait').send_cli('getblockcount')
         self.nodes[0].wait_for_rpc_connection()
+        assert_equal(blocks, BLOCKS)
 
 
 if __name__ == '__main__':
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 952f773a8f..64f39b8cfe 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -26,6 +26,7 @@ from .util import (
     MAX_NODES,
     append_config,
     delete_cookie_file,
+    get_auth_cookie,
     get_rpc_proxy,
     rpc_url,
     wait_until,
@@ -237,12 +238,27 @@ class TestNode():
             except OSError as e:
                 if e.errno != errno.ECONNREFUSED:  # Port not yet open?
                     raise  # unknown OS error
-            except ValueError as e:  # cookie file not found and no rpcuser or rpcassword. bitcoind still starting
+            except ValueError as e:  # cookie file not found and no rpcuser or rpcpassword; bitcoind is still starting
                 if "No RPC credentials" not in str(e):
                     raise
             time.sleep(1.0 / poll_per_s)
         self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
 
+    def wait_for_cookie_credentials(self):
+        """Ensures auth cookie credentials can be read, e.g. for testing CLI with -rpcwait before RPC connection is up."""
+        self.log.debug("Waiting for cookie credentials")
+        # Poll at a rate of four times per second.
+        poll_per_s = 4
+        for _ in range(poll_per_s * self.rpc_timeout):
+            try:
+                get_auth_cookie(self.datadir, self.chain)
+                self.log.debug("Cookie credentials successfully retrieved")
+                return
+            except ValueError:  # cookie file not found and no rpcuser or rpcpassword; bitcoind is still starting
+                pass            # so we continue polling until RPC credentials are retrieved
+            time.sleep(1.0 / poll_per_s)
+        self._raise_assertion_error("Unable to retrieve cookie credentials after {}s".format(self.rpc_timeout))
+
     def generate(self, nblocks, maxtries=1000000):
         self.log.debug("TestNode.generate() dispatches `generate` call to `generatetoaddress`")
         return self.generatetoaddress(nblocks=nblocks, address=self.get_deterministic_priv_key().address, maxtries=maxtries)