From c6c2f0fd782ccf607027414012f45c8f48561a30 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 7 Dec 2015 15:44:16 -0500 Subject: Implement SequenceLocks functions SequenceLocks functions are used to evaluate sequence lock times or heights per BIP 68. The majority of this code is copied from maaku in #6312 Further credit: btcdrak, sipa, NicolasDorier --- src/consensus/consensus.h | 5 +- src/main.cpp | 150 ++++++++++++++++++++++++++++++++++++++++- src/main.h | 17 ++++- src/policy/policy.h | 5 +- src/primitives/transaction.cpp | 2 +- src/primitives/transaction.h | 38 ++++++++--- src/script/interpreter.cpp | 2 +- src/test/miner_tests.cpp | 126 +++++++++++++++++++++++++--------- src/test/script_tests.cpp | 4 +- src/txmempool.cpp | 2 +- 10 files changed, 301 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 6d6ce7e099..1b28bb3203 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -13,8 +13,11 @@ static const unsigned int MAX_BLOCK_SIGOPS = MAX_BLOCK_SIZE/50; /** Coinbase transaction outputs can only be spent after this number of new blocks (network rule) */ static const int COINBASE_MATURITY = 100; -/** Flags for LockTime() */ +/** Flags for nSequence and nLockTime locks */ enum { + /* Interpret sequence numbers as relative lock-time constraints. */ + LOCKTIME_VERIFY_SEQUENCE = (1 << 0), + /* Use GetMedianTimePast() instead of nTime for end point timestamp. */ LOCKTIME_MEDIAN_TIME_PAST = (1 << 1), }; diff --git a/src/main.cpp b/src/main.cpp index a0e996ae78..d9bf3bd757 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -667,9 +667,10 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) return true; if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) return true; - BOOST_FOREACH(const CTxIn& txin, tx.vin) - if (!txin.IsFinal()) + BOOST_FOREACH(const CTxIn& txin, tx.vin) { + if (!(txin.nSequence == CTxIn::SEQUENCE_FINAL)) return false; + } return true; } @@ -705,6 +706,128 @@ bool CheckFinalTx(const CTransaction &tx, int flags) return IsFinalTx(tx, nBlockHeight, nBlockTime); } +/** + * Calculates the block height and previous block's median time past at + * which the transaction will be considered final in the context of BIP 68. + * Also removes from the vector of input heights any entries which did not + * correspond to sequence locked inputs as they do not affect the calculation. + */ +static std::pair CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector* prevHeights, const CBlockIndex& block) +{ + assert(prevHeights->size() == tx.vin.size()); + + // Will be set to the equivalent height- and time-based nLockTime + // values that would be necessary to satisfy all relative lock- + // time constraints given our view of block chain history. + // The semantics of nLockTime are the last invalid height/time, so + // use -1 to have the effect of any height or time being valid. + int nMinHeight = -1; + int64_t nMinTime = -1; + + // tx.nVersion is signed integer so requires cast to unsigned otherwise + // we would be doing a signed comparison and half the range of nVersion + // wouldn't support BIP 68. + bool fEnforceBIP68 = static_cast(tx.nVersion) >= 2 + && flags & LOCKTIME_VERIFY_SEQUENCE; + + // Do not enforce sequence numbers as a relative lock time + // unless we have been instructed to + if (!fEnforceBIP68) { + return std::make_pair(nMinHeight, nMinTime); + } + + for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { + const CTxIn& txin = tx.vin[txinIndex]; + + // Sequence numbers with the most significant bit set are not + // treated as relative lock-times, nor are they given any + // consensus-enforced meaning at this point. + if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLE_FLAG) { + // The height of this input is not relevant for sequence locks + (*prevHeights)[txinIndex] = 0; + continue; + } + + int nCoinHeight = (*prevHeights)[txinIndex]; + + if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) { + int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight-1, 0))->GetMedianTimePast(); + // NOTE: Subtract 1 to maintain nLockTime semantics + // BIP 68 relative lock times have the semantics of calculating + // the first block or time at which the transaction would be + // valid. When calculating the effective block time or height + // for the entire transaction, we switch to using the + // semantics of nLockTime which is the last invalid block + // time or height. Thus we subtract 1 from the calculated + // time or height. + + // Time-based relative lock-times are measured from the + // smallest allowed timestamp of the block containing the + // txout being spent, which is the median time past of the + // block prior. + nMinTime = std::max(nMinTime, nCoinTime + (int64_t)((txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) << CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) - 1); + } else { + nMinHeight = std::max(nMinHeight, nCoinHeight + (int)(txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) - 1); + } + } + + return std::make_pair(nMinHeight, nMinTime); +} + +static bool EvaluateSequenceLocks(const CBlockIndex& block, std::pair lockPair) +{ + assert(block.pprev); + int64_t nBlockTime = block.pprev->GetMedianTimePast(); + if (lockPair.first >= block.nHeight || lockPair.second >= nBlockTime) + return false; + + return true; +} + +bool SequenceLocks(const CTransaction &tx, int flags, std::vector* prevHeights, const CBlockIndex& block) +{ + return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block)); +} + +bool CheckSequenceLocks(const CTransaction &tx, int flags) +{ + AssertLockHeld(cs_main); + AssertLockHeld(mempool.cs); + + CBlockIndex* tip = chainActive.Tip(); + CBlockIndex index; + index.pprev = tip; + // CheckSequenceLocks() uses chainActive.Height()+1 to evaluate + // height based locks because when SequenceLocks() 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 + // SequenceLocks() with one more than chainActive.Height(). + index.nHeight = tip->nHeight + 1; + + // pcoinsTip contains the UTXO set for chainActive.Tip() + CCoinsViewMemPool viewMemPool(pcoinsTip, mempool); + std::vector prevheights; + prevheights.resize(tx.vin.size()); + for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { + const CTxIn& txin = tx.vin[txinIndex]; + CCoins coins; + if (!viewMemPool.GetCoins(txin.prevout.hash, coins)) { + return error("%s: Missing input", __func__); + } + if (coins.nHeight == MEMPOOL_HEIGHT) { + // Assume all mempool transaction confirm in the next block + prevheights[txinIndex] = tip->nHeight + 1; + } else { + prevheights[txinIndex] = coins.nHeight; + } + } + + std::pair lockPair = CalculateSequenceLocks(tx, flags, &prevheights, index); + return EvaluateSequenceLocks(index, lockPair); +} + + unsigned int GetLegacySigOpCount(const CTransaction& tx) { unsigned int nSigOps = 0; @@ -949,6 +1072,14 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool view.SetBackend(dummy); + + // Only accept BIP68 sequence locked 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. + // Must keep pool.cs for this unless we change CheckSequenceLocks to take a + // CoinsViewCache instead of create its own + if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) + return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final"); } // Check for non-standard pay-to-script-hash in inputs @@ -2075,6 +2206,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); + std::vector prevheights; + int nLockTimeFlags = 0; CAmount nFees = 0; int nInputs = 0; unsigned int nSigOps = 0; @@ -2098,6 +2231,19 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return state.DoS(100, error("ConnectBlock(): inputs missing/spent"), REJECT_INVALID, "bad-txns-inputs-missingorspent"); + // Check that transaction is BIP68 final + // BIP68 lock checks (as opposed to nLockTime checks) must + // be in ConnectBlock because they require the UTXO set + prevheights.resize(tx.vin.size()); + for (size_t j = 0; j < tx.vin.size(); j++) { + prevheights[j] = view.AccessCoins(tx.vin[j].prevout.hash)->nHeight; + } + + if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { + return state.DoS(100, error("ConnectBlock(): contains a non-BIP68-final transaction", __func__), + REJECT_INVALID, "bad-txns-nonfinal"); + } + if (fStrictPayToScriptHash) { // Add in sigops done by pay-to-script-hash inputs; diff --git a/src/main.h b/src/main.h index 19623f4d96..66b766316e 100644 --- a/src/main.h +++ b/src/main.h @@ -341,7 +341,22 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); */ bool CheckFinalTx(const CTransaction &tx, int flags = -1); -/** +/** + * Check if transaction is final per BIP 68 sequence numbers and can be included in a block. + * Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed. + */ +bool SequenceLocks(const CTransaction &tx, int flags, std::vector* prevHeights, const CBlockIndex& block); + +/** + * Check if transaction will be BIP 68 final in the next block to be created. + * + * Calls SequenceLocks() with data from the tip of the current active chain. + * + * See consensus/consensus.h for flag definitions. + */ +bool CheckSequenceLocks(const CTransaction &tx, int flags); + +/** * Closure representing one script verification * Note that this stores references to the spending transaction */ diff --git a/src/policy/policy.h b/src/policy/policy.h index 31655f2f3a..5034b23863 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -45,8 +45,9 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY /** For convenience, standard but not mandatory verify flags. */ static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; -/** Used as the flags parameter to CheckFinalTx() in non-consensus code */ -static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_MEDIAN_TIME_PAST; +/** Used as the flags parameter to LockTime() in non-consensus code. */ +static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE | + LOCKTIME_MEDIAN_TIME_PAST; bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType); /** diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 46d3cbbe2e..7d0b208398 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -37,7 +37,7 @@ std::string CTxIn::ToString() const str += strprintf(", coinbase %s", HexStr(scriptSig)); else str += strprintf(", scriptSig=%s", HexStr(scriptSig).substr(0, 24)); - if (nSequence != std::numeric_limits::max()) + if (nSequence != SEQUENCE_FINAL) str += strprintf(", nSequence=%u", nSequence); str += ")"; return str; diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index c5d8a64a6d..1dca378b52 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -61,13 +61,40 @@ public: CScript scriptSig; uint32_t nSequence; + /* Setting nSequence to this value for every input in a transaction + * disables nLockTime. */ + static const uint32_t SEQUENCE_FINAL = 0xffffffff; + + /* Below flags apply in the context of BIP 68*/ + /* If this flag set, CTxIn::nSequence is NOT interpreted as a + * relative lock-time. */ + static const uint32_t SEQUENCE_LOCKTIME_DISABLE_FLAG = (1 << 31); + + /* If CTxIn::nSequence encodes a relative lock-time and this flag + * is set, the relative lock-time has units of 512 seconds, + * otherwise it specifies blocks with a granularity of 1. */ + static const uint32_t SEQUENCE_LOCKTIME_TYPE_FLAG = (1 << 22); + + /* If CTxIn::nSequence encodes a relative lock-time, this mask is + * applied to extract that lock-time from the sequence field. */ + static const uint32_t SEQUENCE_LOCKTIME_MASK = 0x0000ffff; + + /* In order to use the same number of bits to encode roughly the + * same wall-clock duration, and because blocks are naturally + * limited to occur every 600s on average, the minimum granularity + * for time-based relative lock-time is fixed at 512 seconds. + * Converting from CTxIn::nSequence to seconds is performed by + * multiplying by 512 = 2^9, or equivalently shifting up by + * 9 bits. */ + static const int SEQUENCE_LOCKTIME_GRANULARITY = 9; + CTxIn() { - nSequence = std::numeric_limits::max(); + nSequence = SEQUENCE_FINAL; } - explicit CTxIn(COutPoint prevoutIn, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits::max()); - CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits::max()); + explicit CTxIn(COutPoint prevoutIn, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=SEQUENCE_FINAL); + CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=SEQUENCE_FINAL); ADD_SERIALIZE_METHODS; @@ -78,11 +105,6 @@ public: READWRITE(nSequence); } - bool IsFinal() const - { - return (nSequence == std::numeric_limits::max()); - } - friend bool operator==(const CTxIn& a, const CTxIn& b) { return (a.prevout == b.prevout && diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 57e0edc4b4..ac753a9d5b 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1147,7 +1147,7 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con // prevent this condition. Alternatively we could test all // inputs, but testing just this input minimizes the data // required to prove correct CHECKLOCKTIMEVERIFY execution. - if (txTo->vin[nIn].IsFinal()) + if (CTxIn::SEQUENCE_FINAL == txTo->vin[nIn].nSequence) return false; return true; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 19ddb5b79c..138ba808e3 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -57,6 +57,20 @@ struct { {2, 0xbbbeb305}, {2, 0xfe1c810a}, }; +CBlockIndex CreateBlockIndex(int nHeight) +{ + CBlockIndex index; + index.nHeight = nHeight; + index.pprev = chainActive.Tip(); + return index; +} + +bool TestSequenceLocks(const CTransaction &tx, int flags) +{ + LOCK(mempool.cs); + return CheckSequenceLocks(tx, flags); +} + // NOTE: These tests rely on CreateNewBlock doing its own self-validation! BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { @@ -79,6 +93,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // We can't make transactions until we have inputs // Therefore, load 100 blocks :) + int baseheight = 0; std::vectortxFirst; for (unsigned int i = 0; i < sizeof(blockinfo)/sizeof(*blockinfo); ++i) { @@ -92,7 +107,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); txCoinbase.vout[0].scriptPubKey = CScript(); pblock->vtx[0] = CTransaction(txCoinbase); - if (txFirst.size() < 2) + if (txFirst.size() == 0) + baseheight = chainActive.Height(); + if (txFirst.size() < 4) txFirst.push_back(new CTransaction(pblock->vtx[0])); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; @@ -240,49 +257,96 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // non-final txs in mempool SetMockTime(chainActive.Tip()->GetMedianTimePast()+1); + int flags = LOCKTIME_VERIFY_SEQUENCE|LOCKTIME_MEDIAN_TIME_PAST; + // height map + std::vector prevheights; - // height locked - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + // relative height locked + tx.nVersion = 2; + tx.vin.resize(1); + prevheights.resize(1); + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); // only 1 transaction + tx.vin[0].prevout.n = 0; tx.vin[0].scriptSig = CScript() << OP_1; - tx.vin[0].nSequence = 0; + tx.vin[0].nSequence = chainActive.Tip()->nHeight + 1; // txFirst[0] is the 2nd block + prevheights[0] = baseheight + 1; + tx.vout.resize(1); tx.vout[0].nValue = 4900000000LL; tx.vout[0].scriptPubKey = CScript() << OP_1; - tx.nLockTime = chainActive.Tip()->nHeight+1; + tx.nLockTime = 0; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(100000000L).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK(!CheckFinalTx(tx, LOCKTIME_MEDIAN_TIME_PAST)); - - // time locked - tx2.vin.resize(1); - tx2.vin[0].prevout.hash = txFirst[1]->GetHash(); - tx2.vin[0].prevout.n = 0; - tx2.vin[0].scriptSig = CScript() << OP_1; - tx2.vin[0].nSequence = 0; - tx2.vout.resize(1); - tx2.vout[0].nValue = 4900000000LL; - tx2.vout[0].scriptPubKey = CScript() << OP_1; - tx2.nLockTime = chainActive.Tip()->GetMedianTimePast()+1; - hash = tx2.GetHash(); - mempool.addUnchecked(hash, entry.Fee(100000000L).Time(GetTime()).SpendsCoinbase(true).FromTx(tx2)); - BOOST_CHECK(!CheckFinalTx(tx2, LOCKTIME_MEDIAN_TIME_PAST)); + BOOST_CHECK(CheckFinalTx(tx, flags)); // Locktime passes + BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail + BOOST_CHECK(SequenceLocks(tx, flags, &prevheights, CreateBlockIndex(chainActive.Tip()->nHeight + 2))); // Sequence locks pass on 2nd block + + // relative time locked + tx.vin[0].prevout.hash = txFirst[1]->GetHash(); + tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | (((chainActive.Tip()->GetMedianTimePast()+1-chainActive[1]->GetMedianTimePast()) >> CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) + 1); // txFirst[1] is the 3rd block + prevheights[0] = baseheight + 2; + hash = tx.GetHash(); + mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + BOOST_CHECK(CheckFinalTx(tx, flags)); // Locktime passes + BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail + + for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) + chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast + BOOST_CHECK(SequenceLocks(tx, flags, &prevheights, CreateBlockIndex(chainActive.Tip()->nHeight + 1))); // Sequence locks pass 512 seconds later + for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) + chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP + + // absolute height locked + tx.vin[0].prevout.hash = txFirst[2]->GetHash(); + tx.vin[0].nSequence = CTxIn::SEQUENCE_FINAL - 1; + prevheights[0] = baseheight + 3; + tx.nLockTime = chainActive.Tip()->nHeight + 1; + hash = tx.GetHash(); + mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + BOOST_CHECK(!CheckFinalTx(tx, flags)); // Locktime fails + BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass + BOOST_CHECK(IsFinalTx(tx, chainActive.Tip()->nHeight + 2, chainActive.Tip()->GetMedianTimePast())); // Locktime passes on 2nd block + + // absolute time locked + tx.vin[0].prevout.hash = txFirst[3]->GetHash(); + tx.nLockTime = chainActive.Tip()->GetMedianTimePast(); + prevheights.resize(1); + prevheights[0] = baseheight + 4; + hash = tx.GetHash(); + mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + BOOST_CHECK(!CheckFinalTx(tx, flags)); // Locktime fails + BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass + BOOST_CHECK(IsFinalTx(tx, chainActive.Tip()->nHeight + 2, chainActive.Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later + + // mempool-dependent transactions (not added) + tx.vin[0].prevout.hash = hash; + prevheights[0] = chainActive.Tip()->nHeight + 1; + tx.nLockTime = 0; + tx.vin[0].nSequence = 0; + BOOST_CHECK(CheckFinalTx(tx, flags)); // Locktime passes + BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass + tx.vin[0].nSequence = 1; + BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail + tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG; + BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass + tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1; + BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey)); - // Neither tx should have make it into the template. - BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1); + // None of the of the absolute height/time locked tx should have made + // it into the template because we still check IsFinalTx in CreateNewBlock, + // but relative locked txs will if inconsistently added to mempool. + // For now these will still generate a valid template until BIP68 soft fork + BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3); delete pblocktemplate; - - // However if we advance height and time by one, both will. + // However if we advance height by 1 and time by 512, all of them should be mined + for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) + chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast chainActive.Tip()->nHeight++; - SetMockTime(chainActive.Tip()->GetMedianTimePast()+2); - - // 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)); + SetMockTime(chainActive.Tip()->GetMedianTimePast() + 1); BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey)); - BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 2); + BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5); delete pblocktemplate; chainActive.Tip()->nHeight--; diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 0059e4a998..90822c71d7 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -63,7 +63,7 @@ CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey) txCredit.vout.resize(1); txCredit.vin[0].prevout.SetNull(); txCredit.vin[0].scriptSig = CScript() << CScriptNum(0) << CScriptNum(0); - txCredit.vin[0].nSequence = std::numeric_limits::max(); + txCredit.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; txCredit.vout[0].scriptPubKey = scriptPubKey; txCredit.vout[0].nValue = 0; @@ -80,7 +80,7 @@ CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMu txSpend.vin[0].prevout.hash = txCredit.GetHash(); txSpend.vin[0].prevout.n = 0; txSpend.vin[0].scriptSig = scriptSig; - txSpend.vin[0].nSequence = std::numeric_limits::max(); + txSpend.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; txSpend.vout[0].scriptPubKey = CScript(); txSpend.vout[0].nValue = 0; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index fea5da8029..67001cf7df 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -504,7 +504,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem list transactionsToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); - if (!CheckFinalTx(tx, flags)) { + if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(tx, flags)) { transactionsToRemove.push_back(tx); } else if (it->GetSpendsCoinbase()) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { -- cgit v1.2.3 From b043c4b746c8199ce948aa5e8b186e0d1a61ad68 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 11 Feb 2016 15:34:04 -0500 Subject: fix sdaftuar's nits again it boggles the mind why these nits can't be delivered on a more timely basis --- src/main.cpp | 10 +++++----- src/main.h | 2 +- src/policy/policy.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index d9bf3bd757..7d75e2ea65 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -799,10 +799,10 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags) index.pprev = tip; // CheckSequenceLocks() uses chainActive.Height()+1 to evaluate // height based locks because when SequenceLocks() 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 - // SequenceLocks() with one more than chainActive.Height(). + // ConnectBlock(), 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 use one more than chainActive.Height() index.nHeight = tip->nHeight + 1; // pcoinsTip contains the UTXO set for chainActive.Tip() @@ -2240,7 +2240,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { - return state.DoS(100, error("ConnectBlock(): contains a non-BIP68-final transaction", __func__), + return state.DoS(100, error("%s: contains a non-BIP68-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal"); } diff --git a/src/main.h b/src/main.h index 66b766316e..a576078f06 100644 --- a/src/main.h +++ b/src/main.h @@ -350,7 +350,7 @@ bool SequenceLocks(const CTransaction &tx, int flags, std::vector* prevHeig /** * Check if transaction will be BIP 68 final in the next block to be created. * - * Calls SequenceLocks() with data from the tip of the current active chain. + * Simulates calling SequenceLocks() with data from the tip of the current active chain. * * See consensus/consensus.h for flag definitions. */ diff --git a/src/policy/policy.h b/src/policy/policy.h index 5034b23863..f25dbf22d5 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -45,7 +45,7 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY /** For convenience, standard but not mandatory verify flags. */ static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; -/** Used as the flags parameter to LockTime() in non-consensus code. */ +/** Used as the flags parameter to sequence and nLocktime checks in non-consensus code. */ static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE | LOCKTIME_MEDIAN_TIME_PAST; -- cgit v1.2.3