From c6b82d1db54e21cd7fb1b961e16b4bd547ba5766 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 10 Feb 2017 14:27:27 -0500 Subject: [PATCH 1/4] Add tests for CWalletTx::nTimeSmart --- src/wallet/test/wallet_tests.cpp | 53 ++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 294920add..19c1e9f29 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -453,4 +453,57 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 50*COIN); } +static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) +{ + CMutableTransaction tx; + tx.nLockTime = lockTime; + SetMockTime(mockTime); + CBlockIndex* block = nullptr; + if (blockTime > 0) { + auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex); + assert(inserted.second); + const uint256& hash = inserted.first->first; + block = inserted.first->second; + block->nTime = blockTime; + block->phashBlock = &hash; + } + + CWalletTx wtx(&wallet, MakeTransactionRef(tx)); + if (block) { + wtx.SetMerkleBranch(block, 0); + } + wallet.AddToWallet(wtx); + return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; +} + +// Simple test to verify assignment of CWalletTx::nSmartTime value. Could be +// expanded to cover more corner cases of smart time logic. +BOOST_AUTO_TEST_CASE(ComputeTimeSmart) +{ + CWallet wallet; + + // New transaction should use clock time if lower than block time. + BOOST_CHECK_EQUAL(AddTx(wallet, 1, 100, 120), 100); + + // Test that updating existing transaction does not change smart time. + BOOST_CHECK_EQUAL(AddTx(wallet, 1, 200, 220), 100); + + // New transaction should use clock time if there's no block time. + BOOST_CHECK_EQUAL(AddTx(wallet, 2, 300, 0), 300); + + // New transaction should use block time if lower than clock time. + BOOST_CHECK_EQUAL(AddTx(wallet, 3, 420, 400), 400); + + // New transaction should use latest entry time if higher than + // min(block time, clock time). + BOOST_CHECK_EQUAL(AddTx(wallet, 4, 500, 390), 400); + + // If there are future entries, new transaction should use time of the + // newest entry that is no more than 300 seconds ahead of the clock time. + BOOST_CHECK_EQUAL(AddTx(wallet, 5, 50, 600), 300); + + // Reset mock time for other tests. + SetMockTime(0); +} + BOOST_AUTO_TEST_SUITE_END() From 1f98abe47b585217f6290fadfe30a5da240d30a4 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 16 Dec 2016 10:00:26 -0500 Subject: [PATCH 2/4] Factor out CWallet::nTimeSmart computation into a method. No change in behavior, this change just pulls some code out of CWallet::AddToWallet that was making it very long into a separate method. --- src/wallet/wallet.cpp | 95 +++++++++++++++++++++++-------------------- src/wallet/wallet.h | 1 + 2 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ce3f1fb54..aefd15284 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -896,51 +896,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.nTimeReceived = GetAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&walletdb); wtxOrdered.insert(make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0))); - - wtx.nTimeSmart = wtx.nTimeReceived; - if (!wtxIn.hashUnset()) - { - if (mapBlockIndex.count(wtxIn.hashBlock)) - { - 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 (TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) - { - CWalletTx *const pwtx = (*it).second.first; - if (pwtx == &wtx) - continue; - CAccountingEntry *const pacentry = (*it).second.second; - int64_t nSmartTime; - if (pwtx) - { - nSmartTime = pwtx->nTimeSmart; - if (!nSmartTime) - nSmartTime = pwtx->nTimeReceived; - } - else - nSmartTime = pacentry->nTime; - if (nSmartTime <= latestTolerated) - { - latestEntry = nSmartTime; - if (nSmartTime > latestNow) - latestNow = nSmartTime; - break; - } - } - } - - int64_t blocktime = mapBlockIndex[wtxIn.hashBlock]->GetBlockTime(); - wtx.nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); - } - else - LogPrintf("AddToWallet(): found %s in block %s not in index\n", - wtxIn.GetHash().ToString(), - wtxIn.hashBlock.ToString()); - } + wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash); } @@ -3498,6 +3454,55 @@ void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) c mapKeyBirth[it->first] = it->second->GetBlockTime() - 7200; // block times can be 2h off } +unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const +{ + unsigned int nTimeSmart = wtx.nTimeReceived; + if (!wtx.hashUnset()) + { + if (mapBlockIndex.count(wtx.hashBlock)) + { + 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 (TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) + { + CWalletTx *const pwtx = (*it).second.first; + if (pwtx == &wtx) + continue; + CAccountingEntry *const pacentry = (*it).second.second; + int64_t nSmartTime; + if (pwtx) + { + nSmartTime = pwtx->nTimeSmart; + if (!nSmartTime) + nSmartTime = pwtx->nTimeReceived; + } + else + nSmartTime = pacentry->nTime; + if (nSmartTime <= latestTolerated) + { + latestEntry = nSmartTime; + if (nSmartTime > latestNow) + latestNow = nSmartTime; + break; + } + } + } + + int64_t blocktime = mapBlockIndex[wtx.hashBlock]->GetBlockTime(); + nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + } + else + LogPrintf("AddToWallet(): found %s in block %s not in index\n", + wtx.GetHash().ToString(), + wtx.hashBlock.ToString()); + } + return nTimeSmart; +} + bool CWallet::AddDestData(const CTxDestination &dest, const std::string &key, const std::string &value) { if (boost::get(&dest)) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 176063c27..110dedcad 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -776,6 +776,7 @@ public: bool EncryptWallet(const SecureString& strWalletPassphrase); void GetKeyBirthTimes(std::map &mapKeyBirth) const; + unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; /** * Increment the next transaction order id From 6c996c2df7d3bebadda528c6585aa709579d1953 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 19 Dec 2016 10:51:45 -0500 Subject: [PATCH 3/4] Add documentation describing CWallet::nTimeSmart. Most of the text comes from the 2012 Luke Dashjr c3f95ef commit message. --- src/wallet/wallet.cpp | 21 +++++++++++++++++++++ src/wallet/wallet.h | 9 +++++++++ 2 files changed, 30 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index aefd15284..dc085d0df 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3454,6 +3454,27 @@ void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) c mapKeyBirth[it->first] = it->second->GetBlockTime() - 7200; // block times can be 2h off } +/** + * Compute smart timestamp for a transaction being added to the wallet. + * + * Logic: + * - 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 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 + * transaction (that we care about), assign all its (not already known) + * transactions' timestamps to the same timestamp as that most-recent-known + * transaction. + * - If receiving a block with a past timestamp, but after the most recent known + * transaction, assign all its (not already known) transactions' timestamps to + * the block time. + * + * For more information see CWalletTx::nTimeSmart, + * https://bitcointalk.org/?topic=54527, or + * https://github.com/bitcoin/bitcoin/pull/1393. + */ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { unsigned int nTimeSmart = wtx.nTimeReceived; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 110dedcad..99f6f43a4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -260,6 +260,15 @@ public: std::vector > vOrderForm; unsigned int fTimeReceivedIsTxTime; unsigned int nTimeReceived; //!< time received by this node + /** + * Stable timestamp that never changes, and reflects the order a transaction + * was added to the wallet. Timestamp is based on the block time for a + * transaction added as part of a block, or else the time when the + * transaction was received if it wasn't part of a block, with the timestamp + * adjusted in both cases so timestamp order matches the order transactions + * were added to the wallet. More details can be found in + * CWallet::ComputeTimeSmart(). + */ unsigned int nTimeSmart; /** * From me flag is set to 1 for transactions that were created by the wallet From 630fc549e28cb33eb11df1b4f951339bf8152e4f Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 10 Feb 2017 15:00:30 -0500 Subject: [PATCH 4/4] Clean up braces in CWallet::ComputeTimeSmart --- src/wallet/wallet.cpp | 59 ++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dc085d0df..0e95699bb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3478,48 +3478,43 @@ void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) c unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { unsigned int nTimeSmart = wtx.nTimeReceived; - if (!wtx.hashUnset()) - { - if (mapBlockIndex.count(wtx.hashBlock)) - { + if (!wtx.hashUnset()) { + if (mapBlockIndex.count(wtx.hashBlock)) { 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 (TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) - { - CWalletTx *const pwtx = (*it).second.first; - if (pwtx == &wtx) - continue; - CAccountingEntry *const pacentry = (*it).second.second; - int64_t nSmartTime; - if (pwtx) - { - nSmartTime = pwtx->nTimeSmart; - if (!nSmartTime) - nSmartTime = pwtx->nTimeReceived; + + // 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.first; + if (pwtx == &wtx) { + continue; + } + CAccountingEntry* const pacentry = it->second.second; + int64_t nSmartTime; + if (pwtx) { + nSmartTime = pwtx->nTimeSmart; + if (!nSmartTime) { + nSmartTime = pwtx->nTimeReceived; } - else - nSmartTime = pacentry->nTime; - if (nSmartTime <= latestTolerated) - { - latestEntry = nSmartTime; - if (nSmartTime > latestNow) - latestNow = nSmartTime; - break; + } else { + nSmartTime = pacentry->nTime; + } + if (nSmartTime <= latestTolerated) { + latestEntry = nSmartTime; + if (nSmartTime > latestNow) { + latestNow = nSmartTime; } + break; } } int64_t blocktime = mapBlockIndex[wtx.hashBlock]->GetBlockTime(); nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + } else { + LogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.hashBlock.ToString()); } - else - LogPrintf("AddToWallet(): found %s in block %s not in index\n", - wtx.GetHash().ToString(), - wtx.hashBlock.ToString()); } return nTimeSmart; }