Merge dashpay#5928: backport: Merge bitcoin#20406, 21370, 21395, 21007, 20040, 21418, 21447, 20197, 21613, 21602

daacafb Merge bitcoin#21426: rpc: remove scantxoutset EXPERIMENTAL warning (MarcoFalke)
b8aec5c Merge bitcoin#20197: p2p: protect onions in AttemptToEvictConnection(), add eviction protection test coverage (Wladimir J. van der Laan)
dd92322 Merge bitcoin#21447: Always add -daemonwait to known command line arguments (Wladimir J. van der Laan)
3325620 Merge bitcoin#21418: contrib: Make systemd invoke dependencies only when ready (Wladimir J. van der Laan)
99f09a0 Merge bitcoin#20040: wallet: Refactor OutputGroups to handle fees and spending eligibility on grouping (Samuel Dobson)
6ca0eb1 Merge bitcoin#21007: bitcoind: Add -daemonwait option to wait for initialization (Wladimir J. van der Laan)
666e6e2 Merge bitcoin#20406: util: Avoid invalid integer negation in FormatMoney and ValueFromAmount (Wladimir J. van der Laan)
1a07e04 Merge bitcoin#21370: Use C++11 member initializer in CNodeState (fanquake)
6423f6b Merge bitcoin#21395: Net processing: Remove unused CNode.address member (MarcoFalke)

Pull request description:

  bitcoin backports

Top commit has no ACKs.

Tree-SHA512: dc9490c6c99cc08e8c8ffbd6b71f48e8df8b51dd2d7ca940de44ff47ab28ec3e2d45b7e381a7124d513f7e29ad13300c9df68acd7ddec3022a603eca162b12f3

Author: PastaPastaPasta

configure.ac

@@ -1007,8 +1007,9 @@ AC_CHECK_DECLS([getifaddrs, freeifaddrs],[CHECK_SOCKET],,
 )
 AC_CHECK_DECLS([strnlen])
 
-dnl Check for daemon(3), unrelated to --with-daemon (although used by it)
-AC_CHECK_DECLS([daemon])
+dnl These are used for daemonization in dashd
+AC_CHECK_DECLS([fork])
+AC_CHECK_DECLS([setsid])
 
 AC_CHECK_DECLS([pipe2])
 

contrib/init/dashd.service

@@ -18,7 +18,7 @@ After=network-online.target
 Wants=network-online.target
 
 [Service]
-ExecStart=/usr/bin/dashd -daemon \
+ExecStart=/usr/bin/dashd -daemonwait \
                                                             -pid=/run/dashd/dashd.pid \
                                                             -conf=/etc/dash/dash.conf  \
                                                             -datadir=/var/lib/dashd
@@ -33,6 +33,7 @@ ExecStartPre=/bin/chgrp dashcore /etc/dash
 Type=forking
 PIDFile=/run/dashd/dashd.pid
 Restart=on-failure
+TimeoutStartSec=infinity
 TimeoutStopSec=600
 
 # Directory creation and permissions

src/Makefile.am

@@ -355,6 +355,7 @@ BITCOIN_CORE_H = \
   util/time.h \
   util/thread.h \
   util/threadnames.h \
+  util/tokenpipe.h \
   util/trace.h \
   util/translation.h \
   util/ui_change_type.h \
@@ -792,6 +793,7 @@ libbitcoin_util_a_SOURCES = \
   util/string.cpp \
   util/thread.cpp \
   util/threadnames.cpp \
+  util/tokenpipe.cpp \
   $(BITCOIN_CORE_H)
 
 if USE_LIBEVENT

src/Makefile.test.include

@@ -132,6 +132,7 @@ BITCOIN_TESTS =\
   test/merkleblock_tests.cpp \
   test/miner_tests.cpp \
   test/multisig_tests.cpp \
+  test/net_peer_eviction_tests.cpp \
   test/net_tests.cpp \
   test/netbase_tests.cpp \
   test/pmt_tests.cpp \

src/bench/coin_selection.cpp

@@ -42,19 +42,18 @@ static void CoinSelection(benchmark::Bench& bench)
     }
     addCoin(3 * COIN, wallet, wtxs);
 
-    // Create groups
-    std::vector<OutputGroup> groups;
+    // Create coins
+    std::vector<COutput> coins;
     for (const auto& wtx : wtxs) {
-        COutput output(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */);
-        groups.emplace_back(output.GetInputCoin(), 6, false, 0, 0);
+        coins.emplace_back(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */);
     }
     const CoinEligibilityFilter filter_standard(1, 6, 0);
-    const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0);
+    const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0, false);
     bench.run([&] {
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;
         bool bnb_used;
-        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
+        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
         assert(success);
         assert(nValueRet == 1003 * COIN);
         assert(setCoinsRet.size() == 2);
@@ -74,7 +73,8 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>
     tx.vout.resize(nInput + 1);
     tx.vout[nInput].nValue = nValue;
     std::unique_ptr<CWalletTx> wtx = std::make_unique<CWalletTx>(&testWallet, MakeTransactionRef(std::move(tx)));
-    set.emplace_back(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0);
+    set.emplace_back();
+    set.back().Insert(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false);
     wtxn.emplace_back(std::move(wtx));
 }
 // Copied from src/wallet/test/coinselector_tests.cpp

src/bitcoind.cpp

@@ -21,6 +21,7 @@
 #include <util/system.h>
 #include <util/strencodings.h>
 #include <util/threadnames.h>
+#include <util/tokenpipe.h>
 #include <util/translation.h>
 #include <stacktraces.h>
 #include <util/url.h>
@@ -35,6 +36,79 @@ UrlDecodeFn* const URL_DECODE = urlDecode;
 //
 // Start
 //
+#if HAVE_DECL_FORK
+
+/** Custom implementation of daemon(). This implements the same order of operations as glibc.
+ * Opens a pipe to the child process to be able to wait for an event to occur.
+ *
+ * @returns 0 if successful, and in child process.
+ *          >0 if successful, and in parent process.
+ *          -1 in case of error (in parent process).
+ *
+ *          In case of success, endpoint will be one end of a pipe from the child to parent process,
+ *          which can be used with TokenWrite (in the child) or TokenRead (in the parent).
+ */
+int fork_daemon(bool nochdir, bool noclose, TokenPipeEnd& endpoint)
+{
+    // communication pipe with child process
+    std::optional<TokenPipe> umbilical = TokenPipe::Make();
+    if (!umbilical) {
+        return -1; // pipe or pipe2 failed.
+    }
+
+    int pid = fork();
+    if (pid < 0) {
+        return -1; // fork failed.
+    }
+    if (pid != 0) {
+        // Parent process gets read end, closes write end.
+        endpoint = umbilical->TakeReadEnd();
+        umbilical->TakeWriteEnd().Close();
+
+        int status = endpoint.TokenRead();
+        if (status != 0) { // Something went wrong while setting up child process.
+            endpoint.Close();
+            return -1;
+        }
+
+        return pid;
+    }
+    // Child process gets write end, closes read end.
+    endpoint = umbilical->TakeWriteEnd();
+    umbilical->TakeReadEnd().Close();
+
+#if HAVE_DECL_SETSID
+    if (setsid() < 0) {
+        exit(1); // setsid failed.
+    }
+#endif
+
+    if (!nochdir) {
+        if (chdir("/") != 0) {
+            exit(1); // chdir failed.
+        }
+    }
+    if (!noclose) {
+        // Open /dev/null, and clone it into STDIN, STDOUT and STDERR to detach
+        // from terminal.
+        int fd = open("/dev/null", O_RDWR);
+        if (fd >= 0) {
+            bool err = dup2(fd, STDIN_FILENO) < 0 || dup2(fd, STDOUT_FILENO) < 0 || dup2(fd, STDERR_FILENO) < 0;
+            // Don't close if fd<=2 to try to handle the case where the program was invoked without any file descriptors open.
+            if (fd > 2) close(fd);
+            if (err) {
+                exit(1); // dup2 failed.
+            }
+        } else {
+            exit(1); // open /dev/null failed.
+        }
+    }
+    endpoint.TokenWrite(0); // Success
+    return 0;
+}
+
+#endif
+
 static bool AppInit(int argc, char* argv[])
 {
     NodeContext node;
@@ -72,6 +146,14 @@ static bool AppInit(int argc, char* argv[])
     }
 
     CoreContext context{node};
+#if HAVE_DECL_FORK
+    // Communication with parent after daemonizing. This is used for signalling in the following ways:
+    // - a boolean token is sent when the initialization process (all the Init* functions) have finished to indicate
+    // that the parent process can quit, and whether it was successful/unsuccessful.
+    // - an unexpected shutdown of the child process creates an unexpected end of stream at the parent
+    // end, which is interpreted as failure to start.
+    TokenPipeEnd daemon_ep;
+#endif
     try
     {
         if (!CheckDataDirOption()) {
@@ -117,24 +199,34 @@ static bool AppInit(int argc, char* argv[])
             // InitError will have been called with detailed error, which ends up on console
             return false;
         }
-        if (args.GetBoolArg("-daemon", false)) {
-#if HAVE_DECL_DAEMON
-#if defined(MAC_OSX)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
+        if (args.GetBoolArg("-daemon", DEFAULT_DAEMON) || args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
+#if HAVE_DECL_FORK
             tfm::format(std::cout, PACKAGE_NAME " starting\n");
 
             // Daemonize
-            if (daemon(1, 0)) { // don't chdir (1), do close FDs (0)
-                return InitError(Untranslated(strprintf("daemon() failed: %s\n", strerror(errno))));
+            switch (fork_daemon(1, 0, daemon_ep)) { // don't chdir (1), do close FDs (0)
+            case 0: // Child: continue.
+                // If -daemonwait is not enabled, immediately send a success token the parent.
+                if (!args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
+                    daemon_ep.TokenWrite(1);
+                    daemon_ep.Close();
+                }
+                break;
+            case -1: // Error happened.
+                return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", strerror(errno))));
+            default: { // Parent: wait and exit.
+                int token = daemon_ep.TokenRead();
+                if (token) { // Success
+                    exit(EXIT_SUCCESS);
+                } else { // fRet = false or token read error (premature exit).
+                    tfm::format(std::cerr, "Error during initializaton - check debug.log for details\n");
+                    exit(EXIT_FAILURE);
+                }
+            }
             }
-#if defined(MAC_OSX)
-#pragma GCC diagnostic pop
-#endif
 #else
             return InitError(Untranslated("-daemon is not supported on this operating system\n"));
-#endif // HAVE_DECL_DAEMON
+#endif // HAVE_DECL_FORK
         }
         // Lock data directory after daemonization
         if (!AppInitLockDataDirectory())
@@ -147,6 +239,13 @@ static bool AppInit(int argc, char* argv[])
         PrintExceptionContinue(std::current_exception(), "AppInit()");
     }
 
+#if HAVE_DECL_FORK
+    if (daemon_ep.IsOpen()) {
+        // Signal initialization status to parent, then close pipe.
+        daemon_ep.TokenWrite(fRet);
+        daemon_ep.Close();
+    }
+#endif
     if (fRet) {
         WaitForShutdown();
     }

src/core_io.h

@@ -41,7 +41,7 @@ std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strN
 int ParseSighashString(const UniValue& sighash);
 
 // core_write.cpp
-UniValue ValueFromAmount(const CAmount& amount);
+UniValue ValueFromAmount(const CAmount amount);
 std::string FormatScript(const CScript& script);
 std::string EncodeHexTx(const CTransaction& tx);
 std::string SighashToStr(unsigned char sighash_type);

src/core_write.cpp

@@ -15,6 +15,7 @@
 #include <univalue.h>
 #include <util/check.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 
 #include <addressindex.h>
 #include <spentindex.h>
@@ -26,14 +27,17 @@
 #include <evo/specialtx.h>
 #include <llmq/commitment.h>
 
-UniValue ValueFromAmount(const CAmount& amount)
+UniValue ValueFromAmount(const CAmount amount)
 {
-    bool sign = amount < 0;
-    int64_t n_abs = (sign ? -amount : amount);
-    int64_t quotient = n_abs / COIN;
-    int64_t remainder = n_abs % COIN;
+    static_assert(COIN > 1);
+    int64_t quotient = amount / COIN;
+    int64_t remainder = amount % COIN;
+    if (amount < 0) {
+        quotient = -quotient;
+        remainder = -remainder;
+    }
     return UniValue(UniValue::VNUM,
-            strprintf("%s%d.%08d", sign ? "-" : "", quotient, remainder));
+            strprintf("%s%d.%08d", amount < 0 ? "-" : "", quotient, remainder));
 }
 
 std::string FormatScript(const CScript& script)

src/evo/cbtx.h

@@ -24,7 +24,7 @@ class CChainLocksHandler;
 }// namespace llmq
 
 // Forward declaration from core_io to get rid of circular dependency
-UniValue ValueFromAmount(const CAmount& amount);
+UniValue ValueFromAmount(const CAmount amount);
 
 // coinbase transaction
 class CCbTx

src/init.cpp

@@ -779,10 +779,12 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-statsport=<port>", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsns=<ns>", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_NAMESPACE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsperiod=<seconds>", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
-#if HAVE_DECL_DAEMON
-    argsman.AddArg("-daemon", "Run in the background as a daemon and accept commands", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+#if HAVE_DECL_FORK
+    argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
+    argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
 #else
     hidden_args.emplace_back("-daemon");
+    hidden_args.emplace_back("-daemonwait");
 #endif
 
     // Add the hidden options