forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge bitcoin#20591: wallet, bugfix: fix ComputeTimeSmart function du…
…ring rescanning process. 240ea29 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami) d6eb39a test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami) 07b44f1 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 bitcoin#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. (cryptoadvance/specter-desktop#680). ACKs for top commit: jonatack: ACK 240ea29 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 240ea29 Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494
- Loading branch information
1 parent
08ccc15
commit e810337
Showing
4 changed files
with
208 additions
and
36 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() |