Merge bitcoin/bitcoin#29503: 26.x: backport #28784 ("rpc: keep .cookie if it was not generated")

1e956439eb86de7a455560c349370684f4e54561 rpc: keep .cookie if it was not generated (Roman Zeyde)

Pull request description:

  v26 introduced a regression in that starting a `bitcoind` twice may have the second instance delete the cookie file of the first, making it impossible to communicate with it.

  Not a big deal but it's annoying, only an issue for 26.0, and the patch is trivial.

ACKs for top commit:
  glozow:
    lgtm ACK 1e956439eb86de7a455560c349370684f4e54561

Tree-SHA512: 0e4b18aebaaf284944f1709b238c8c0acce5e8997409e0c278a5a30ac221ac1ff1d3ad31fbf2ac15b03bf7582891e07a7a2cf00f13cb596aa9512566b9320c23

2 files changed, 10 insertions, 1 deletions

diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp
index 4c67da8b70..b7acd62ee3 100644
--- a/src/rpc/request.cpp
+++ b/src/rpc/request.cpp
@@ -80,6 +80,8 @@ static fs::path GetAuthCookieFile(bool temp=false)
     return AbsPathForConfigVal(gArgs, arg);
 }
 
+static bool g_generated_cookie = false;
+
 bool GenerateAuthCookie(std::string *cookie_out)
 {
     const size_t COOKIE_SIZE = 32;
@@ -105,6 +107,7 @@ bool GenerateAuthCookie(std::string *cookie_out)
         LogPrintf("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
         return false;
     }
+    g_generated_cookie = true;
     LogPrintf("Generated RPC authentication cookie %s\n", fs::PathToString(filepath));
 
     if (cookie_out)
@@ -131,7 +134,10 @@ bool GetAuthCookie(std::string *cookie_out)
 void DeleteAuthCookie()
 {
     try {
-        fs::remove(GetAuthCookieFile());
+        if (g_generated_cookie) {
+            // Delete the cookie file if it was generated by this process
+            fs::remove(GetAuthCookieFile());
+        }
     } catch (const fs::filesystem_error& e) {
         LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, fsbridge::get_filesystem_error_message(e));
     }
diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py
index 24a68a04bf..0f4be54d0e 100755
--- a/test/functional/feature_filelock.py
+++ b/test/functional/feature_filelock.py
@@ -30,6 +30,9 @@ class FilelockTest(BitcoinTestFramework):
         expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running."
         self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg)
 
+        cookie_file = datadir / ".cookie"
+        assert cookie_file.exists()  # should not be deleted during the second bitcoind instance shutdown
+
         if self.is_wallet_compiled():
             def check_wallet_filelock(descriptors):
                 wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])