Merge #2923: [Bug] Fix wallet data race

f9a6ebd fix: solve data race by making nTimeBestReceived atomic (ale)
4331c30 refactor: move nTimeBestReceived to CWallet (ale)

Pull request description:

  First commit is a small refactor:  the global variable `nTimeBestReceived` is now a private member of the `CWallet` class, as it was used only there.

  Second commit: fix the following data race by making `nTimeBestReceived` atomic

  ```
  WARNING: ThreadSanitizer: data race (pid=18270)
    Write of size 8 at 0x7b6800000ed8 by thread T16:
      #0 CWallet::UpdatedBlockTip(CBlockIndex const*, CBlockIndex const*, bool) wallet/wallet.cpp:2094 (pivxd+0x5058c9)
      #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2ee1c0)
      #2 std::__invoke_result<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>::type std::__invoke<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2ee25a)
      #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>, std::_Placeholder<2>, std::_Placeholder<3>))(CBlockIndex const*, CBlockIndex const*, bool)>::__call<void, CBlockIndex const*&&, CBlockIndex const*&&, bool&&, 0ul, 1ul, 2ul, 3ul>(std::tuple<CBlockIndex const*&&, CBlockIndex const*&&, bool&&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/12/functional:484 (pivxd+0x2ee25a)
      ...

    Previous read of size 8 at 0x7b6800000ed8 by thread T35 (mutexes: write M134298, write M132880):
      #0 CWallet::ResendWalletTransactions(CConnman*) wallet/wallet.cpp:2065 (pivxd+0x515b25)
      #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2eef45)
      #2 std::__invoke_result<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>::type std::__invoke<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2eefb9)
      #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>))(CConnman*)>::__call<void, CConnman*&&, 0ul, 1ul>(std::tuple<CConnman*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/functional:484 (pivxd+0x2eefb9)
      ...
  ```

ACKs for top commit: f9a6ebd
  Duddino:
    utACK f9a6ebd
  Liquid369:
    tACK f9a6ebd
  Fuzzbawls:
    ACK f9a6ebd

Tree-SHA512: c1de883956bf0958204b05c338831c27f694e6453e74b77e3347ae12a3cbd69df2fe1120bae3fca72dab14587c0c499c4f92d5c53943305855e66fc1275ab4fa

src/net_processing.cpp

@@ -34,7 +34,6 @@
 
 using namespace std::chrono_literals;
 
-int64_t nTimeBestReceived = 0;  // Used only to inform the wallet of when we last received a block
 
 static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8]
 
@@ -757,8 +756,6 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex* pindexNew, const CB
             }
         });
     }
-
-    nTimeBestReceived = GetTime();
 }
 
 void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state)

src/validation.h

@@ -129,7 +129,6 @@ extern BlockMap mapBlockIndex;
 extern PrevBlockMap mapPrevBlockIndex;
 extern uint64_t nLastBlockTx;
 extern uint64_t nLastBlockSize;
-extern int64_t nTimeBestReceived;
 
 // Best block section
 extern Mutex g_best_block_mutex;

src/wallet/wallet.cpp

@@ -2088,6 +2088,11 @@ void CWallet::ResendWalletTransactions(CConnman* connman)
     }
 }
 
+void CWallet::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
+{
+    nTimeBestReceived = GetTime();
+}
+
 /** @} */ // end of mapWallet
 
 

src/wallet/wallet.h

@@ -628,6 +628,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
 
     int64_t nNextResend;
     int64_t nLastResend;
+    std::atomic<int64_t> nTimeBestReceived{0}; // Used only to inform the wallet of when we last received a block
 
     /**
      * Used to keep track of spent outpoints, and
@@ -1042,6 +1043,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
     void ReacceptWalletTransactions(bool fFirstLoad = false);
     void ResendWalletTransactions(CConnman* connman) override;
+    void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;
 
     struct Balance {
         CAmount m_mine_trusted{0};               //!< Trusted, at depth=GetBalance.min_depth or more