Merge bitcoin/bitcoin#34353: refactor: Use std::bind_front over std::bind

faa18dceba refactor: Use std::bind_front over std::bind (MarcoFalke)

Pull request description:

  `std::bind` has many issues:

  * It is verbosely listing all placeholders, but in a meaningless way, because it doesn't name the args or their types.
  * It silently ignores args passed to it, when one arg is overridden. For example [1] compiles fine on current master.
  * Accidentally duplicated placeholders compile fine as well.
  * Usually the placeholders aren't even needed.
  * This makes it hard to review, understand, and maintain.

  Fix all issues by using `std::bind_front` from C++20, which allows to drop the brittle `_1, _2, ...` placeholders. The replacement should be correct, if the trailing placeholders are ordered.

  Introducing the same silent bug on top of this pull request [2] will now lead to a compile failure.

  ----

  [1]

  ```diff
  diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
  index 694fb535b5..7661dd361e 100644
  --- a/src/qt/walletmodel.cpp
  +++ b/src/qt/walletmodel.cpp
  @@ -412,3 +412,3 @@ void WalletModel::subscribeToCoreSignals()
       m_handler_status_changed = m_wallet->handleStatusChanged(std::bind(&NotifyKeyStoreStatusChanged, this));
  -    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind(NotifyAddressBookChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5));
  +    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind(NotifyAddressBookChanged, this, CTxDestination{}, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5));
       m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind(NotifyTransactionChanged, this, std::placeholders::_1, std::placeholders::_2));
  ```

  [2]

  ```diff
  diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
  index 578713c0ab..84cced741c 100644
  --- a/src/qt/walletmodel.cpp
  +++ b/src/qt/walletmodel.cpp
  @@ -412,3 +412,3 @@ void WalletModel::subscribeToCoreSignals()
       m_handler_status_changed = m_wallet->handleStatusChanged(std::bind_front(&NotifyKeyStoreStatusChanged, this));
  -    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind_front(NotifyAddressBookChanged, this));
  +    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind_front(NotifyAddressBookChanged, this, CTxDestination{}));
       m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind_front(NotifyTransactionChanged, this));

ACKs for top commit:
  janb84:
    cr ACK faa18dceba
  fjahr:
    Code review ACK faa18dceba
  hebasto:
    ACK faa18dceba, I have reviewed the code and it looks OK.

Tree-SHA512: 9dd13f49527e143a2beafbaae80b1358981f07a2ce20d25cffb1853089a32ff71639e6d718d1d193754522f9ac04e3e168ba017d5fc67a11a5918e79a92b3461

src/qt/walletmodel.cpp

@@ -408,12 +408,12 @@ static void NotifyCanGetAddressesChanged(WalletModel* walletmodel)
 void WalletModel::subscribeToCoreSignals()
 {
     // Connect signals to wallet
-    m_handler_unload = m_wallet->handleUnload(std::bind(&NotifyUnload, this));
+    m_handler_unload = m_wallet->handleUnload(std::bind_front(&NotifyUnload, this));
-    m_handler_status_changed = m_wallet->handleStatusChanged(std::bind(&NotifyKeyStoreStatusChanged, this));
+    m_handler_status_changed = m_wallet->handleStatusChanged(std::bind_front(&NotifyKeyStoreStatusChanged, this));
-    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind(NotifyAddressBookChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5));
+    m_handler_address_book_changed = m_wallet->handleAddressBookChanged(std::bind_front(NotifyAddressBookChanged, this));
-    m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind(NotifyTransactionChanged, this, std::placeholders::_1, std::placeholders::_2));
+    m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind_front(NotifyTransactionChanged, this));
-    m_handler_show_progress = m_wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2));
+    m_handler_show_progress = m_wallet->handleShowProgress(std::bind_front(ShowProgress, this));
-    m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(std::bind(NotifyCanGetAddressesChanged, this));
+    m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(std::bind_front(NotifyCanGetAddressesChanged, this));
 }
 
 void WalletModel::unsubscribeFromCoreSignals()

src/test/scheduler_tests.cpp

@@ -23,7 +23,7 @@ static void microTask(CScheduler& s, std::mutex& mutex, int& counter, int delta,
     }
     auto noTime = std::chrono::steady_clock::time_point::min();
     if (rescheduleTime != noTime) {
-        CScheduler::Function f = std::bind(&microTask, std::ref(s), std::ref(mutex), std::ref(counter), -delta + 1, noTime);
+        CScheduler::Function f = std::bind_front(&microTask, std::ref(s), std::ref(mutex), std::ref(counter), -delta + 1, noTime);
         s.schedule(f, rescheduleTime);
     }
 }
@@ -59,7 +59,7 @@ BOOST_AUTO_TEST_CASE(manythreads)
         auto t = now + std::chrono::microseconds(randomMsec(rng));
         auto tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng));
         int whichCounter = zeroToNine(rng);
-        CScheduler::Function f = std::bind(&microTask, std::ref(microTasks),
+        CScheduler::Function f = std::bind_front(&microTask, std::ref(microTasks),
                                              std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]),
                                              randomDelta(rng), tReschedule);
         microTasks.schedule(f, t);
@@ -73,19 +73,19 @@ BOOST_AUTO_TEST_CASE(manythreads)
     std::vector<std::thread> microThreads;
     microThreads.reserve(10);
     for (int i = 0; i < 5; i++)
-        microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, &microTasks));
+        microThreads.emplace_back(std::bind_front(&CScheduler::serviceQueue, &microTasks));
 
     UninterruptibleSleep(std::chrono::microseconds{600});
     now = std::chrono::steady_clock::now();
 
     // More threads and more tasks:
     for (int i = 0; i < 5; i++)
-        microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, &microTasks));
+        microThreads.emplace_back(std::bind_front(&CScheduler::serviceQueue, &microTasks));
     for (int i = 0; i < 100; i++) {
         auto t = now + std::chrono::microseconds(randomMsec(rng));
         auto tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng));
         int whichCounter = zeroToNine(rng);
-        CScheduler::Function f = std::bind(&microTask, std::ref(microTasks),
+        CScheduler::Function f = std::bind_front(&microTask, std::ref(microTasks),
                                              std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]),
                                              randomDelta(rng), tReschedule);
         microTasks.schedule(f, t);

src/torcontrol.cpp

@@ -332,8 +332,8 @@ TorController::TorController(struct event_base* _base, const std::string& tor_co
     if (!reconnect_ev)
         LogWarning("tor: Failed to create event for reconnection: out of memory?");
     // Start connection attempts immediately
-    if (!conn.Connect(m_tor_control_center, std::bind(&TorController::connected_cb, this, std::placeholders::_1),
+    if (!conn.Connect(m_tor_control_center, std::bind_front(&TorController::connected_cb, this),
-         std::bind(&TorController::disconnected_cb, this, std::placeholders::_1) )) {
+         std::bind_front(&TorController::disconnected_cb, this) )) {
         LogWarning("tor: Initiating connection to Tor control port %s failed", m_tor_control_center);
     }
     // Read service private key if cached
@@ -469,7 +469,7 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
         // Now that we know Tor is running setup the proxy for onion addresses
         // if -onion isn't set to something else.
         if (gArgs.GetArg("-onion", "") == "") {
-            _conn.Command("GETINFO net/listeners/socks", std::bind(&TorController::get_socks_cb, this, std::placeholders::_1, std::placeholders::_2));
+            _conn.Command("GETINFO net/listeners/socks", std::bind_front(&TorController::get_socks_cb, this));
         }
 
         // Finally - now create the service
@@ -479,7 +479,7 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
         // Request onion service, redirect port.
         // Note that the 'virtual' port is always the default port to avoid decloaking nodes using other ports.
         _conn.Command(strprintf("ADD_ONION %s Port=%i,%s", private_key, Params().GetDefaultPort(), m_target.ToStringAddrPort()),
-            std::bind(&TorController::add_onion_cb, this, std::placeholders::_1, std::placeholders::_2));
+            std::bind_front(&TorController::add_onion_cb, this));
     } else {
         LogWarning("tor: Authentication failed");
     }
@@ -538,7 +538,7 @@ void TorController::authchallenge_cb(TorControlConnection& _conn, const TorContr
             }
 
             std::vector<uint8_t> computedClientHash = ComputeResponse(TOR_SAFE_CLIENTKEY, cookie, clientNonce, serverNonce);
-            _conn.Command("AUTHENTICATE " + HexStr(computedClientHash), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
+            _conn.Command("AUTHENTICATE " + HexStr(computedClientHash), std::bind_front(&TorController::auth_cb, this));
         } else {
             LogWarning("tor: Invalid reply to AUTHCHALLENGE");
         }
@@ -589,23 +589,23 @@ void TorController::protocolinfo_cb(TorControlConnection& _conn, const TorContro
             if (methods.contains("HASHEDPASSWORD")) {
                 LogDebug(BCLog::TOR, "Using HASHEDPASSWORD authentication\n");
                 ReplaceAll(torpassword, "\"", "\\\"");
-                _conn.Command("AUTHENTICATE \"" + torpassword + "\"", std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
+                _conn.Command("AUTHENTICATE \"" + torpassword + "\"", std::bind_front(&TorController::auth_cb, this));
             } else {
                 LogWarning("tor: Password provided with -torpassword, but HASHEDPASSWORD authentication is not available");
             }
         } else if (methods.contains("NULL")) {
             LogDebug(BCLog::TOR, "Using NULL authentication\n");
-            _conn.Command("AUTHENTICATE", std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
+            _conn.Command("AUTHENTICATE", std::bind_front(&TorController::auth_cb, this));
         } else if (methods.contains("SAFECOOKIE")) {
             // Cookie: hexdump -e '32/1 "%02x""\n"'  ~/.tor/control_auth_cookie
             LogDebug(BCLog::TOR, "Using SAFECOOKIE authentication, reading cookie authentication from %s\n", cookiefile);
             std::pair<bool,std::string> status_cookie = ReadBinaryFile(fs::PathFromString(cookiefile), TOR_COOKIE_SIZE);
             if (status_cookie.first && status_cookie.second.size() == TOR_COOKIE_SIZE) {
-                // _conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
+                // _conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), std::bind_front(&TorController::auth_cb, this));
                 cookie = std::vector<uint8_t>(status_cookie.second.begin(), status_cookie.second.end());
                 clientNonce = std::vector<uint8_t>(TOR_NONCE_SIZE, 0);
                 GetRandBytes(clientNonce);
-                _conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), std::bind(&TorController::authchallenge_cb, this, std::placeholders::_1, std::placeholders::_2));
+                _conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), std::bind_front(&TorController::authchallenge_cb, this));
             } else {
                 if (status_cookie.first) {
                     LogWarning("tor: Authentication cookie %s is not exactly %i bytes, as is required by the spec", cookiefile, TOR_COOKIE_SIZE);
@@ -627,7 +627,7 @@ void TorController::connected_cb(TorControlConnection& _conn)
 {
     reconnect_timeout = RECONNECT_TIMEOUT_START;
     // First send a PROTOCOLINFO command to figure out what authentication is expected
-    if (!_conn.Command("PROTOCOLINFO 1", std::bind(&TorController::protocolinfo_cb, this, std::placeholders::_1, std::placeholders::_2)))
+    if (!_conn.Command("PROTOCOLINFO 1", std::bind_front(&TorController::protocolinfo_cb, this)))
         LogWarning("tor: Error sending initial protocolinfo command");
 }
 
@@ -656,8 +656,8 @@ void TorController::Reconnect()
     /* Try to reconnect and reestablish if we get booted - for example, Tor
      * may be restarting.
      */
-    if (!conn.Connect(m_tor_control_center, std::bind(&TorController::connected_cb, this, std::placeholders::_1),
+    if (!conn.Connect(m_tor_control_center, std::bind_front(&TorController::connected_cb, this),
-         std::bind(&TorController::disconnected_cb, this, std::placeholders::_1) )) {
+         std::bind_front(&TorController::disconnected_cb, this) )) {
         LogWarning("tor: Re-initiating connection to Tor control port %s failed", m_tor_control_center);
     }
 }