Merge bitcoin/bitcoin#20591: wallet, bugfix: fix ComputeTimeSmart function during rescanning process.

240ea294d5 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami)
d6eb39af21 test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami)
07b44f16e7 wallet: fix ComputeTimeSmart algorithm to use blocktime during old block rescanning (BitcoinTsunami)

Pull request description:

  The function ComputeTimeSmart in wallet.cpp assume that transaction are discovered in the right order.
  Moreover the 'smarttime' determination algorithm is coded with realtime scenario in mind and not rescanning of old block.

  The functional test demonstrate that if the user import a wallet, then rescan only recent history, and then rescan the entire history, the older transaction discovered would have an incorrect time determination.
  In the context of rescanning old block, the only time value that as a meaning is the blocktime.

  That's why I've fixed the problem with a simple separation between rescanning of old block and realtime time determination. The fix is written to have no impact on every realtime scenario and only impact the behaviour during a rescanning process.
  This PR Fixes #20181.

  To be fair, I don't think that this bug could be triggered with the wallet GUI, because it always proceed with a proper rescan.
  But RPC API provide the possibility to trigger it. I've discovered it, because Specter desktop v0.10.0 was impacted. (https://github.com/cryptoadvance/specter-desktop/issues/680).

ACKs for top commit:
  jonatack:
    ACK 240ea294d5 per `git diff b92d552 240ea29`, re-verified rebase to latest master + debug build clean + new test passes on the branch and fails on master, only change since my review a few hours ago is incorporation of latest review suggestions
  meshcollider:
    re-utACK 240ea294d5

Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494

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',

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()