From 7aea2e095580a3e073c3de26373cecd448ed939b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 7 Dec 2021 09:01:59 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#23644: wallet: Replace confusing getAdjustedTime() with GetTime() fa37e798b2660d8e44e31c944a257b55aeef5de2 wallet: Replace confusing getAdjustedTime() with GetTime() (MarcoFalke) Pull request description: Setting `nTimeReceived` to the adjusted time has several issues: * `m_best_block_time` is set to the "unadjusted" time, thus a comparison of the two times is like comparing apples to oranges. In the worst case this opens up an attack vector where remote peers can force a premature re-broadcast of wallet txs. * The RPC documentation for `"timereceived"` doesn't mention that the network adjusted time is used, possibly confusing users when the time reported by RPC is off by a few seconds compared to their local timestamp. Fix all issues by replacing the call with `GetTime()`. Also a style fix: Use non-narrowing integer conversion in the RPC method. ACKs for top commit: theStack: Code-review ACK fa37e798b2660d8e44e31c944a257b55aeef5de2 shaavan: crACK fa37e798b2660d8e44e31c944a257b55aeef5de2 Tree-SHA512: 8d020ba400521246b7aed4b6c41319fc70552e8c69e929a5994500375466a9edac02a0ae64b803dbc6695df22276489561a23bd6e030c44c97d288f7b9b2b3fa --- src/interfaces/chain.h | 3 --- src/node/interfaces.cpp | 1 - src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 3 +-- 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 98471db956..85b6c9daee 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -226,9 +226,6 @@ public: //! Check if shutdown requested. virtual bool shutdownRequested() = 0; - //! Get adjusted time. - virtual int64_t getAdjustedTime() = 0; - //! Send init message. virtual void initMessage(const std::string& message) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 34ed833f25..e821709e7f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -785,7 +785,6 @@ public: bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !::ChainstateActive().IsInitialBlockDownload(); } bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); } bool shutdownRequested() override { return ShutdownRequested(); } - int64_t getAdjustedTime() override { return GetAdjustedTime(); } void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } void initWarning(const bilingual_str& message) override { InitWarning(message); } void initError(const bilingual_str& message) override { InitError(message); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 1d85741912..b8b4006cdd 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -183,7 +183,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniVa conflicts.push_back(conflict.GetHex()); entry.pushKV("walletconflicts", conflicts); entry.pushKV("time", wtx.GetTxTime()); - entry.pushKV("timereceived", (int64_t)wtx.nTimeReceived); + entry.pushKV("timereceived", int64_t{wtx.nTimeReceived}); for (const std::pair& item : wtx.mapValue) entry.pushKV(item.first, item.second); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 72f478575d..c87c50e8a2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -864,7 +864,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.BindWallet(this); bool fInsertedNew = ret.second; if (fInsertedNew) { - wtx.nTimeReceived = chain().getAdjustedTime(); + wtx.nTimeReceived = GetTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); @@ -1277,7 +1277,6 @@ void CWallet::UpdatedBlockTip() m_best_block_time = GetTime(); } - void CWallet::BlockUntilSyncedToCurrentChain() const { AssertLockNotHeld(cs_wallet); // Skip the queue-draining stuff if we know we're caught up with