diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7a7ff3ee33..6885499be2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -884,7 +884,7 @@ bool CWallet::IsSpentKey(const uint256& hash, unsigned int n) const return false; } -CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose) +CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose, bool rescanning_old_block) { LOCK(cs_wallet); @@ -914,7 +914,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio wtx.nTimeReceived = chain().getAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); - wtx.nTimeSmart = ComputeTimeSmart(wtx); + wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block); AddToSpends(hash, &batch); } @@ -1031,7 +1031,7 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx return true; } -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate, bool rescanning_old_block) { const CTransaction& tx = *ptx; { @@ -1069,7 +1069,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again - return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false); + return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false, rescanning_old_block); } } return false; @@ -1198,9 +1198,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx) +void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx, bool rescanning_old_block) { - if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx)) + if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx, rescanning_old_block)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1643,7 +1643,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { - SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, fUpdate); + SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, fUpdate, /* rescanning_old_block */ true); } // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; @@ -2384,6 +2384,8 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { * - If sending a transaction, assign its timestamp to the current time. * - If receiving a transaction outside a block, assign its timestamp to the * current time. + * - If receiving a transaction during a rescanning process, assign all its + * (not already known) transactions' timestamps to the block time. * - If receiving a block with a future timestamp, assign all its (not already * known) transactions' timestamps to the current time. * - If receiving a block with a past timestamp, before the most recent known @@ -2398,38 +2400,43 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { * https://bitcointalk.org/?topic=54527, or * https://github.com/bitcoin/bitcoin/pull/1393. */ -unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const +unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const { unsigned int nTimeSmart = wtx.nTimeReceived; if (!wtx.isUnconfirmed() && !wtx.isAbandoned()) { int64_t blocktime; - if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime))) { - int64_t latestNow = wtx.nTimeReceived; - int64_t latestEntry = 0; + int64_t block_max_time; + if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime).maxTime(block_max_time))) { + if (rescanning_old_block) { + nTimeSmart = block_max_time; + } else { + int64_t latestNow = wtx.nTimeReceived; + int64_t latestEntry = 0; - // Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future - int64_t latestTolerated = latestNow + 300; - const TxItems& txOrdered = wtxOrdered; - for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { - CWalletTx* const pwtx = it->second; - if (pwtx == &wtx) { - continue; - } - int64_t nSmartTime; - nSmartTime = pwtx->nTimeSmart; - if (!nSmartTime) { - nSmartTime = pwtx->nTimeReceived; - } - if (nSmartTime <= latestTolerated) { - latestEntry = nSmartTime; - if (nSmartTime > latestNow) { - latestNow = nSmartTime; + // Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future + int64_t latestTolerated = latestNow + 300; + const TxItems& txOrdered = wtxOrdered; + for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { + CWalletTx* const pwtx = it->second; + if (pwtx == &wtx) { + continue; + } + int64_t nSmartTime; + nSmartTime = pwtx->nTimeSmart; + if (!nSmartTime) { + nSmartTime = pwtx->nTimeReceived; + } + if (nSmartTime <= latestTolerated) { + latestEntry = nSmartTime; + if (nSmartTime > latestNow) { + latestNow = nSmartTime; + } + break; } - break; } - } - nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + } } else { WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.m_confirm.hashBlock.ToString()); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6b4bcf31c4..15a5933424 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -271,8 +271,11 @@ private: * abandoned is an indication that it is not safe to be considered abandoned. * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. + * + * Should be called with rescanning_old_block set to true, if the transaction is + * not discovered in real time, but during a rescan of old blocks. */ - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); @@ -284,7 +287,7 @@ private: /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */ - void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true, bool rescanning_old_block = false) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** WalletFlags set on this wallet. */ std::atomic m_wallet_flags{0}; @@ -484,7 +487,7 @@ public: bool EncryptWallet(const SecureString& strWalletPassphrase); void GetKeyBirthTimes(std::map &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; + unsigned int ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const; /** * Increment the next transaction order id @@ -503,7 +506,7 @@ public: //! @return true if wtx is changed and needs to be saved to disk, otherwise false using UpdateWalletTxFn = std::function; - CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true); + CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override; void blockConnected(const CBlock& block, int height) override; diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 528eff0414..2c2aaf6020 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -175,6 +175,7 @@ BASE_SCRIPTS = [ 'rpc_rawtransaction.py --legacy-wallet', 'rpc_rawtransaction.py --descriptors', 'wallet_groups.py --legacy-wallet', + 'wallet_transactiontime_rescan.py', 'p2p_addrv2_relay.py', 'wallet_groups.py --descriptors', 'p2p_compactblocks_hb.py', diff --git a/test/functional/wallet_transactiontime_rescan.py b/test/functional/wallet_transactiontime_rescan.py new file mode 100755 index 0000000000..78859e6131 --- /dev/null +++ b/test/functional/wallet_transactiontime_rescan.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test transaction time during old block rescanning +""" + +import time + +from test_framework.blocktools import COINBASE_MATURITY +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal +) + + +class TransactionTimeRescanTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 3 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + self.log.info('Prepare nodes and wallet') + + minernode = self.nodes[0] # node used to mine BTC and create transactions + usernode = self.nodes[1] # user node with correct time + restorenode = self.nodes[2] # node used to restore user wallet and check time determination in ComputeSmartTime (wallet.cpp) + + # time constant + cur_time = int(time.time()) + ten_days = 10 * 24 * 60 * 60 + + # synchronize nodes and time + self.sync_all() + minernode.setmocktime(cur_time) + usernode.setmocktime(cur_time) + restorenode.setmocktime(cur_time) + + # prepare miner wallet + minernode.createwallet(wallet_name='default') + miner_wallet = minernode.get_wallet_rpc('default') + m1 = miner_wallet.getnewaddress() + + # prepare the user wallet with 3 watch only addresses + wo1 = usernode.getnewaddress() + wo2 = usernode.getnewaddress() + wo3 = usernode.getnewaddress() + + usernode.createwallet(wallet_name='wo', disable_private_keys=True) + wo_wallet = usernode.get_wallet_rpc('wo') + + wo_wallet.importaddress(wo1) + wo_wallet.importaddress(wo2) + wo_wallet.importaddress(wo3) + + self.log.info('Start transactions') + + # check blockcount + assert_equal(minernode.getblockcount(), 200) + + # generate some btc to create transactions and check blockcount + initial_mine = COINBASE_MATURITY + 1 + minernode.generatetoaddress(initial_mine, m1) + assert_equal(minernode.getblockcount(), initial_mine + 200) + + # synchronize nodes and time + self.sync_all() + minernode.setmocktime(cur_time + ten_days) + usernode.setmocktime(cur_time + ten_days) + restorenode.setmocktime(cur_time + ten_days) + # send 10 btc to user's first watch-only address + self.log.info('Send 10 btc to user') + miner_wallet.sendtoaddress(wo1, 10) + + # generate blocks and check blockcount + minernode.generatetoaddress(COINBASE_MATURITY, m1) + assert_equal(minernode.getblockcount(), initial_mine + 300) + + # synchronize nodes and time + self.sync_all() + minernode.setmocktime(cur_time + ten_days + ten_days) + usernode.setmocktime(cur_time + ten_days + ten_days) + restorenode.setmocktime(cur_time + ten_days + ten_days) + # send 5 btc to our second watch-only address + self.log.info('Send 5 btc to user') + miner_wallet.sendtoaddress(wo2, 5) + + # generate blocks and check blockcount + minernode.generatetoaddress(COINBASE_MATURITY, m1) + assert_equal(minernode.getblockcount(), initial_mine + 400) + + # synchronize nodes and time + self.sync_all() + minernode.setmocktime(cur_time + ten_days + ten_days + ten_days) + usernode.setmocktime(cur_time + ten_days + ten_days + ten_days) + restorenode.setmocktime(cur_time + ten_days + ten_days + ten_days) + # send 1 btc to our third watch-only address + self.log.info('Send 1 btc to user') + miner_wallet.sendtoaddress(wo3, 1) + + # generate more blocks and check blockcount + minernode.generatetoaddress(COINBASE_MATURITY, m1) + assert_equal(minernode.getblockcount(), initial_mine + 500) + + self.log.info('Check user\'s final balance and transaction count') + assert_equal(wo_wallet.getbalance(), 16) + assert_equal(len(wo_wallet.listtransactions()), 3) + + self.log.info('Check transaction times') + for tx in wo_wallet.listtransactions(): + if tx['address'] == wo1: + assert_equal(tx['blocktime'], cur_time + ten_days) + assert_equal(tx['time'], cur_time + ten_days) + elif tx['address'] == wo2: + assert_equal(tx['blocktime'], cur_time + ten_days + ten_days) + assert_equal(tx['time'], cur_time + ten_days + ten_days) + elif tx['address'] == wo3: + assert_equal(tx['blocktime'], cur_time + ten_days + ten_days + ten_days) + assert_equal(tx['time'], cur_time + ten_days + ten_days + ten_days) + + # restore user wallet without rescan + self.log.info('Restore user wallet on another node without rescan') + restorenode.createwallet(wallet_name='wo', disable_private_keys=True) + restorewo_wallet = restorenode.get_wallet_rpc('wo') + + restorewo_wallet.importaddress(wo1, rescan=False) + restorewo_wallet.importaddress(wo2, rescan=False) + restorewo_wallet.importaddress(wo3, rescan=False) + + # check user has 0 balance and no transactions + assert_equal(restorewo_wallet.getbalance(), 0) + assert_equal(len(restorewo_wallet.listtransactions()), 0) + + # proceed to rescan, first with an incomplete one, then with a full rescan + self.log.info('Rescan last history part') + restorewo_wallet.rescanblockchain(initial_mine + 350) + self.log.info('Rescan all history') + restorewo_wallet.rescanblockchain() + + self.log.info('Check user\'s final balance and transaction count after restoration') + assert_equal(restorewo_wallet.getbalance(), 16) + assert_equal(len(restorewo_wallet.listtransactions()), 3) + + self.log.info('Check transaction times after restoration') + for tx in restorewo_wallet.listtransactions(): + if tx['address'] == wo1: + assert_equal(tx['blocktime'], cur_time + ten_days) + assert_equal(tx['time'], cur_time + ten_days) + elif tx['address'] == wo2: + assert_equal(tx['blocktime'], cur_time + ten_days + ten_days) + assert_equal(tx['time'], cur_time + ten_days + ten_days) + elif tx['address'] == wo3: + assert_equal(tx['blocktime'], cur_time + ten_days + ten_days + ten_days) + assert_equal(tx['time'], cur_time + ten_days + ten_days + ten_days) + + +if __name__ == '__main__': + TransactionTimeRescanTest().main()