diff --git a/src/main.cpp b/src/main.cpp index 039092cd1..99f4d4943 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -671,14 +671,8 @@ bool IsStandardTx(const CTransaction& tx, string& reason) bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) { - AssertLockHeld(cs_main); - // Time based nLockTime implemented in 0.1.6 if (tx.nLockTime == 0) return true; - if (nBlockHeight == 0) - nBlockHeight = chainActive.Height(); - if (nBlockTime == 0) - nBlockTime = GetAdjustedTime(); if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) return true; BOOST_FOREACH(const CTxIn& txin, tx.vin) @@ -687,6 +681,12 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) return true; } +bool CheckFinalTx(const CTransaction &tx) +{ + AssertLockHeld(cs_main); + return IsFinalTx(tx, chainActive.Height() + 1, GetAdjustedTime()); +} + /** * Check transaction inputs to mitigate two * potential denial-of-service attacks: @@ -903,21 +903,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. - // - // However, IsFinalTx() is confusing... Without arguments, it uses - // chainActive.Height() to evaluate nLockTime; when a block is accepted, - // chainActive.Height() is set to the value of nHeight in the block. - // However, when IsFinalTx() is called within CBlock::AcceptBlock(), the - // height of the block *being* evaluated is what is used. Thus if we want - // to know if a transaction can be part of the *next* block, we need to - // call IsFinalTx() with one more than chainActive.Height(). - // - // Timestamps on the other hand don't get any special treatment, because we - // can't know what timestamp the next block will have, and there aren't - // timestamp applications where it matters. - if (!IsFinalTx(tx, chainActive.Height() + 1)) - return state.DoS(0, - error("AcceptToMemoryPool: non-final"), + if (!CheckFinalTx(tx)) + return state.DoS(0, error("AcceptToMemoryPool: non-final"), REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? diff --git a/src/main.h b/src/main.h index 9bf7bbb2f..3b2521786 100644 --- a/src/main.h +++ b/src/main.h @@ -324,7 +324,18 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state); */ bool IsStandardTx(const CTransaction& tx, std::string& reason); -bool IsFinalTx(const CTransaction &tx, int nBlockHeight = 0, int64_t nBlockTime = 0); +/** + * Check if transaction is final and can be included in a block with the + * specified height and time. Consensus critical. + */ +bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); + +/** + * Check if transaction will be final in the next block to be created. + * + * Calls IsFinalTx() with current block height and appropriate block time. + */ +bool CheckFinalTx(const CTransaction &tx); /** * Closure representing one script verification diff --git a/src/miner.cpp b/src/miner.cpp index 7a57b42e3..dfabfd48d 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -138,6 +138,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) LOCK2(cs_main, mempool.cs); CBlockIndex* pindexPrev = chainActive.Tip(); const int nHeight = pindexPrev->nHeight + 1; + pblock->nTime = GetAdjustedTime(); CCoinsViewCache view(pcoinsTip); // Priority order to process transactions @@ -152,7 +153,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) mi != mempool.mapTx.end(); ++mi) { const CTransaction& tx = mi->second.GetTx(); - if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight)) + if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, pblock->nTime)) continue; COrphan* porphan = NULL; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 4fffd03ad..5662b1665 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -26,7 +26,7 @@ using namespace std; QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) { AssertLockHeld(cs_main); - if (!IsFinalTx(wtx, chainActive.Height() + 1)) + if (!CheckFinalTx(wtx)) { if (wtx.nLockTime < LOCKTIME_THRESHOLD) return tr("Open for %n more block(s)", "", wtx.nLockTime - chainActive.Height()); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 7f1db58e5..15d13e9fc 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -188,7 +188,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) status.depth = wtx.GetDepthInMainChain(); status.cur_num_blocks = chainActive.Height(); - if (!IsFinalTx(wtx, chainActive.Height() + 1)) + if (!CheckFinalTx(wtx)) { if (wtx.nLockTime < LOCKTIME_THRESHOLD) { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index b6365b1b3..e3e9061cc 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.nLockTime = chainActive.Tip()->nHeight+1; hash = tx.GetHash(); mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); - BOOST_CHECK(!IsFinalTx(tx, chainActive.Tip()->nHeight + 1)); + BOOST_CHECK(!CheckFinalTx(tx)); // time locked tx2.vin.resize(1); @@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx2.nLockTime = chainActive.Tip()->GetMedianTimePast()+1; hash = tx2.GetHash(); mempool.addUnchecked(hash, CTxMemPoolEntry(tx2, 11, GetTime(), 111.0, 11)); - BOOST_CHECK(!IsFinalTx(tx2)); + BOOST_CHECK(!CheckFinalTx(tx2)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); @@ -249,8 +249,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) chainActive.Tip()->nHeight++; SetMockTime(chainActive.Tip()->GetMedianTimePast()+2); - BOOST_CHECK(IsFinalTx(tx, chainActive.Tip()->nHeight + 1)); - BOOST_CHECK(IsFinalTx(tx2)); + // FIXME: we should *actually* create a new block so the following test + // works; CheckFinalTx() isn't fooled by monkey-patching nHeight. + //BOOST_CHECK(CheckFinalTx(tx)); + //BOOST_CHECK(CheckFinalTx(tx2)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index dd5240e3c..3f55b0427 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -588,7 +588,7 @@ Value getreceivedbyaddress(const Array& params, bool fHelp) for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx)) + if (wtx.IsCoinBase() || !CheckFinalTx(wtx)) continue; BOOST_FOREACH(const CTxOut& txout, wtx.vout) @@ -642,7 +642,7 @@ Value getreceivedbyaccount(const Array& params, bool fHelp) for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx)) + if (wtx.IsCoinBase() || !CheckFinalTx(wtx)) continue; BOOST_FOREACH(const CTxOut& txout, wtx.vout) @@ -666,7 +666,7 @@ CAmount GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMi for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (!IsFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0 || wtx.GetDepthInMainChain() < 0) + if (!CheckFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0 || wtx.GetDepthInMainChain() < 0) continue; CAmount nReceived, nSent, nFee; @@ -1109,7 +1109,7 @@ Value ListReceived(const Array& params, bool fByAccounts) { const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !IsFinalTx(wtx)) + if (wtx.IsCoinBase() || !CheckFinalTx(wtx)) continue; int nDepth = wtx.GetDepthInMainChain(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d892c66ed..36baf930a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1318,7 +1318,7 @@ CAmount CWalletTx::GetChange() const bool CWalletTx::IsTrusted() const { // Quick answer in most cases - if (!IsFinalTx(*this)) + if (!CheckFinalTx(*this)) return false; int nDepth = GetDepthInMainChain(); if (nDepth >= 1) @@ -1424,7 +1424,7 @@ CAmount CWallet::GetUnconfirmedBalance() const for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; - if (!IsFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) + if (!CheckFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) nTotal += pcoin->GetAvailableCredit(); } } @@ -1469,7 +1469,7 @@ CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; - if (!IsFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) + if (!CheckFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) nTotal += pcoin->GetAvailableWatchOnlyCredit(); } } @@ -1504,7 +1504,7 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const const uint256& wtxid = it->first; const CWalletTx* pcoin = &(*it).second; - if (!IsFinalTx(*pcoin)) + if (!CheckFinalTx(*pcoin)) continue; if (fOnlyConfirmed && !pcoin->IsTrusted()) @@ -2291,7 +2291,7 @@ std::map CWallet::GetAddressBalances() { CWalletTx *pcoin = &walletEntry.second; - if (!IsFinalTx(*pcoin) || !pcoin->IsTrusted()) + if (!CheckFinalTx(*pcoin) || !pcoin->IsTrusted()) continue; if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0)