Skip to content

Commit 72322ce

Browse files
committed
merge bitcoin#20591: fix ComputeTimeSmart function during rescanning process
1 parent 99c9880 commit 72322ce

File tree

4 files changed

+209
-36
lines changed

4 files changed

+209
-36
lines changed

src/wallet/wallet.cpp

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ bool CWallet::IsSpentKey(const uint256& hash, unsigned int n) const
879879
return false;
880880
}
881881

882-
CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose)
882+
CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose, bool rescanning_old_block)
883883
{
884884
LOCK(cs_wallet);
885885

@@ -910,7 +910,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio
910910
wtx.nTimeReceived = GetTime();
911911
wtx.nOrderPos = IncOrderPosNext(&batch);
912912
wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx));
913-
wtx.nTimeSmart = ComputeTimeSmart(wtx);
913+
wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block);
914914
AddToSpends(wtx, &batch);
915915
candidates = AddWalletUTXOs(wtx.tx, /*ret_dups=*/true);
916916
}
@@ -1042,7 +1042,7 @@ std::set<COutPoint> CWallet::AddWalletUTXOs(CTransactionRef tx, bool ret_dups)
10421042
return ret;
10431043
}
10441044

1045-
bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate)
1045+
bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate, bool rescanning_old_block)
10461046
{
10471047
const CTransaction& tx = *ptx;
10481048
{
@@ -1087,7 +1087,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co
10871087

10881088
// Block disconnection override an abandoned tx as unconfirmed
10891089
// which means user may have to call abandontransaction again
1090-
return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false);
1090+
return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false, rescanning_old_block);
10911091
}
10921092
}
10931093
return false;
@@ -1241,9 +1241,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
12411241
fAnonymizableTallyCachedNonDenom = false;
12421242
}
12431243

1244-
void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx)
1244+
void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx, bool rescanning_old_block)
12451245
{
1246-
if (!AddToWalletIfInvolvingMe(ptx, confirm, batch, update_tx))
1246+
if (!AddToWalletIfInvolvingMe(ptx, confirm, batch, update_tx, rescanning_old_block))
12471247
return; // Not one of ours
12481248

12491249
// If a transaction changes 'conflicted' state, that changes the balance
@@ -1725,7 +1725,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
17251725
break;
17261726
}
17271727
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
1728-
SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, batch, fUpdate);
1728+
SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, batch, fUpdate, /*rescanning_old_block=*/true);
17291729
}
17301730
// scan succeeded, record block as most recent successfully scanned
17311731
result.last_scanned_block = block_hash;
@@ -2559,6 +2559,8 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const {
25592559
* - If sending a transaction, assign its timestamp to the current time.
25602560
* - If receiving a transaction outside a block, assign its timestamp to the
25612561
* current time.
2562+
* - If receiving a transaction during a rescanning process, assign all its
2563+
* (not already known) transactions' timestamps to the block time.
25622564
* - If receiving a block with a future timestamp, assign all its (not already
25632565
* known) transactions' timestamps to the current time.
25642566
* - If receiving a block with a past timestamp, before the most recent known
@@ -2573,38 +2575,43 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const {
25732575
* https://bitcointalk.org/?topic=54527, or
25742576
* https://github.com/bitcoin/bitcoin/pull/1393.
25752577
*/
2576-
unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const
2578+
unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const
25772579
{
25782580
unsigned int nTimeSmart = wtx.nTimeReceived;
25792581
if (!wtx.isUnconfirmed() && !wtx.isAbandoned()) {
25802582
int64_t blocktime;
2581-
if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime))) {
2582-
int64_t latestNow = wtx.nTimeReceived;
2583-
int64_t latestEntry = 0;
2584-
2585-
// Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future
2586-
int64_t latestTolerated = latestNow + 300;
2587-
const TxItems& txOrdered = wtxOrdered;
2588-
for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) {
2589-
CWalletTx* const pwtx = it->second;
2590-
if (pwtx == &wtx) {
2591-
continue;
2592-
}
2593-
int64_t nSmartTime;
2594-
nSmartTime = pwtx->nTimeSmart;
2595-
if (!nSmartTime) {
2596-
nSmartTime = pwtx->nTimeReceived;
2597-
}
2598-
if (nSmartTime <= latestTolerated) {
2599-
latestEntry = nSmartTime;
2600-
if (nSmartTime > latestNow) {
2601-
latestNow = nSmartTime;
2583+
int64_t block_max_time;
2584+
if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime).maxTime(block_max_time))) {
2585+
if (rescanning_old_block) {
2586+
nTimeSmart = block_max_time;
2587+
} else {
2588+
int64_t latestNow = wtx.nTimeReceived;
2589+
int64_t latestEntry = 0;
2590+
2591+
// Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future
2592+
int64_t latestTolerated = latestNow + 300;
2593+
const TxItems& txOrdered = wtxOrdered;
2594+
for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) {
2595+
CWalletTx* const pwtx = it->second;
2596+
if (pwtx == &wtx) {
2597+
continue;
2598+
}
2599+
int64_t nSmartTime;
2600+
nSmartTime = pwtx->nTimeSmart;
2601+
if (!nSmartTime) {
2602+
nSmartTime = pwtx->nTimeReceived;
2603+
}
2604+
if (nSmartTime <= latestTolerated) {
2605+
latestEntry = nSmartTime;
2606+
if (nSmartTime > latestNow) {
2607+
latestNow = nSmartTime;
2608+
}
2609+
break;
26022610
}
2603-
break;
26042611
}
2605-
}
26062612

2607-
nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow));
2613+
nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow));
2614+
}
26082615
} else {
26092616
WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.m_confirm.hashBlock.ToString());
26102617
}

src/wallet/wallet.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,11 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
312312
* abandoned is an indication that it is not safe to be considered abandoned.
313313
* Abandoned state should probably be more carefully tracked via different
314314
* chain notifications or by checking mempool presence when necessary.
315+
*
316+
* Should be called with rescanning_old_block set to true, if the transaction is
317+
* not discovered in real time, but during a rescan of old blocks.
315318
*/
316-
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
319+
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
317320

318321
/** Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */
319322
void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx);
@@ -323,7 +326,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
323326

324327
void SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator>) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
325328

326-
void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
329+
void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx = true, bool rescanning_old_block = false) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
327330

328331
/** WalletFlags set on this wallet. */
329332
std::atomic<uint64_t> m_wallet_flags{0};
@@ -588,7 +591,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
588591
bool EncryptWallet(const SecureString& strWalletPassphrase);
589592

590593
void GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
591-
unsigned int ComputeTimeSmart(const CWalletTx& wtx) const;
594+
unsigned int ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const;
592595

593596
/**
594597
* Increment the next transaction order id
@@ -607,7 +610,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
607610
//! @return true if wtx is changed and needs to be saved to disk, otherwise false
608611
using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>;
609612

610-
CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true);
613+
CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
611614
bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
612615
void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime, uint64_t mempool_sequence) override;
613616
void blockConnected(const CBlock& block, int height) override;

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@
205205
'rpc_signrawtransaction.py --descriptors',
206206
'rpc_rawtransaction.py --legacy-wallet',
207207
'rpc_rawtransaction.py --descriptors',
208+
'wallet_transactiontime_rescan.py',
208209
'p2p_addrv2_relay.py',
209210
'wallet_groups.py --legacy-wallet',
210211
'wallet_groups.py --descriptors',
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2018-2019 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test transaction time during old block rescanning
6+
"""
7+
8+
import time
9+
10+
from test_framework.blocktools import COINBASE_MATURITY
11+
from test_framework.test_framework import BitcoinTestFramework
12+
from test_framework.util import (
13+
assert_equal
14+
)
15+
16+
17+
class TransactionTimeRescanTest(BitcoinTestFramework):
18+
def set_test_params(self):
19+
self.disable_mocktime = True
20+
self.setup_clean_chain = False
21+
self.num_nodes = 3
22+
23+
def skip_test_if_missing_module(self):
24+
self.skip_if_no_wallet()
25+
26+
def run_test(self):
27+
self.log.info('Prepare nodes and wallet')
28+
29+
minernode = self.nodes[0] # node used to mine DASH and create transactions
30+
usernode = self.nodes[1] # user node with correct time
31+
restorenode = self.nodes[2] # node used to restore user wallet and check time determination in ComputeSmartTime (wallet.cpp)
32+
33+
# time constant
34+
cur_time = int(time.time())
35+
ten_days = 10 * 24 * 60 * 60
36+
37+
# synchronize nodes and time
38+
self.sync_all()
39+
minernode.setmocktime(cur_time)
40+
usernode.setmocktime(cur_time)
41+
restorenode.setmocktime(cur_time)
42+
43+
# prepare miner wallet
44+
minernode.createwallet(wallet_name='default')
45+
miner_wallet = minernode.get_wallet_rpc('default')
46+
m1 = miner_wallet.getnewaddress()
47+
48+
# prepare the user wallet with 3 watch only addresses
49+
wo1 = usernode.getnewaddress()
50+
wo2 = usernode.getnewaddress()
51+
wo3 = usernode.getnewaddress()
52+
53+
usernode.createwallet(wallet_name='wo', disable_private_keys=True)
54+
wo_wallet = usernode.get_wallet_rpc('wo')
55+
56+
wo_wallet.importaddress(wo1)
57+
wo_wallet.importaddress(wo2)
58+
wo_wallet.importaddress(wo3)
59+
60+
self.log.info('Start transactions')
61+
62+
# check blockcount
63+
assert_equal(minernode.getblockcount(), 200)
64+
65+
# generate some DASH to create transactions and check blockcount
66+
initial_mine = COINBASE_MATURITY + 1
67+
self.generatetoaddress(minernode, initial_mine, m1)
68+
assert_equal(minernode.getblockcount(), initial_mine + 200)
69+
70+
# synchronize nodes and time
71+
self.sync_all()
72+
minernode.setmocktime(cur_time + ten_days)
73+
usernode.setmocktime(cur_time + ten_days)
74+
restorenode.setmocktime(cur_time + ten_days)
75+
# send 10 DASH to user's first watch-only address
76+
self.log.info('Send 10 DASH to user')
77+
miner_wallet.sendtoaddress(wo1, 10)
78+
79+
# generate blocks and check blockcount
80+
self.generatetoaddress(minernode, COINBASE_MATURITY, m1)
81+
assert_equal(minernode.getblockcount(), initial_mine + 300)
82+
83+
# synchronize nodes and time
84+
self.sync_all()
85+
minernode.setmocktime(cur_time + ten_days + ten_days)
86+
usernode.setmocktime(cur_time + ten_days + ten_days)
87+
restorenode.setmocktime(cur_time + ten_days + ten_days)
88+
# send 5 DASH to our second watch-only address
89+
self.log.info('Send 5 DASH to user')
90+
miner_wallet.sendtoaddress(wo2, 5)
91+
92+
# generate blocks and check blockcount
93+
self.generatetoaddress(minernode, COINBASE_MATURITY, m1)
94+
assert_equal(minernode.getblockcount(), initial_mine + 400)
95+
96+
# synchronize nodes and time
97+
self.sync_all()
98+
minernode.setmocktime(cur_time + ten_days + ten_days + ten_days)
99+
usernode.setmocktime(cur_time + ten_days + ten_days + ten_days)
100+
restorenode.setmocktime(cur_time + ten_days + ten_days + ten_days)
101+
# send 1 DASH to our third watch-only address
102+
self.log.info('Send 1 DASH to user')
103+
miner_wallet.sendtoaddress(wo3, 1)
104+
105+
# generate more blocks and check blockcount
106+
self.generatetoaddress(minernode, COINBASE_MATURITY, m1)
107+
assert_equal(minernode.getblockcount(), initial_mine + 500)
108+
109+
self.log.info('Check user\'s final balance and transaction count')
110+
assert_equal(wo_wallet.getbalance(), 16)
111+
assert_equal(len(wo_wallet.listtransactions()), 3)
112+
113+
self.log.info('Check transaction times')
114+
for tx in wo_wallet.listtransactions():
115+
if tx['address'] == wo1:
116+
assert_equal(tx['blocktime'], cur_time + ten_days)
117+
assert_equal(tx['time'], cur_time + ten_days)
118+
elif tx['address'] == wo2:
119+
assert_equal(tx['blocktime'], cur_time + ten_days + ten_days)
120+
assert_equal(tx['time'], cur_time + ten_days + ten_days)
121+
elif tx['address'] == wo3:
122+
assert_equal(tx['blocktime'], cur_time + ten_days + ten_days + ten_days)
123+
assert_equal(tx['time'], cur_time + ten_days + ten_days + ten_days)
124+
125+
# restore user wallet without rescan
126+
self.log.info('Restore user wallet on another node without rescan')
127+
restorenode.createwallet(wallet_name='wo', disable_private_keys=True)
128+
restorewo_wallet = restorenode.get_wallet_rpc('wo')
129+
130+
restorewo_wallet.importaddress(wo1, rescan=False)
131+
restorewo_wallet.importaddress(wo2, rescan=False)
132+
restorewo_wallet.importaddress(wo3, rescan=False)
133+
134+
# check user has 0 balance and no transactions
135+
assert_equal(restorewo_wallet.getbalance(), 0)
136+
assert_equal(len(restorewo_wallet.listtransactions()), 0)
137+
138+
# proceed to rescan, first with an incomplete one, then with a full rescan
139+
self.log.info('Rescan last history part')
140+
restorewo_wallet.rescanblockchain(initial_mine + 350)
141+
self.log.info('Rescan all history')
142+
restorewo_wallet.rescanblockchain()
143+
144+
self.log.info('Check user\'s final balance and transaction count after restoration')
145+
assert_equal(restorewo_wallet.getbalance(), 16)
146+
assert_equal(len(restorewo_wallet.listtransactions()), 3)
147+
148+
self.log.info('Check transaction times after restoration')
149+
for tx in restorewo_wallet.listtransactions():
150+
if tx['address'] == wo1:
151+
assert_equal(tx['blocktime'], cur_time + ten_days)
152+
assert_equal(tx['time'], cur_time + ten_days)
153+
elif tx['address'] == wo2:
154+
assert_equal(tx['blocktime'], cur_time + ten_days + ten_days)
155+
assert_equal(tx['time'], cur_time + ten_days + ten_days)
156+
elif tx['address'] == wo3:
157+
assert_equal(tx['blocktime'], cur_time + ten_days + ten_days + ten_days)
158+
assert_equal(tx['time'], cur_time + ten_days + ten_days + ten_days)
159+
160+
161+
if __name__ == '__main__':
162+
TransactionTimeRescanTest().main()

0 commit comments

Comments
 (0)