From a1d3c6fb9de8a3fc3e36bc3753a6831cb1dfdbcb Mon Sep 17 00:00:00 2001 From: Mark Friedenbach Date: Wed, 3 Jun 2015 12:55:45 -0700 Subject: [PATCH 1/2] Add rules--presently disabled--for using GetMedianTimePast as endpoint for lock-time calculations The lock-time code currently uses CBlock::nTime as the cutoff point for time based locked transactions. This has the unfortunate outcome of creating a perverse incentive for miners to lie about the time of a block in order to collect more fees by including transactions that by wall clock determination have not yet matured. By using CBlockIndex::GetMedianTimePast from the prior block instead, the self-interested miner no longer gains from generating blocks with fraudulent timestamps. Users can compensate for this change by simply adding an hour (3600 seconds) to their time-based lock times. If enforced, this would be a soft-fork change. This commit only adds the functionality on an unexecuted code path, without changing the behaviour of Bitcoin Core. --- src/consensus/consensus.h | 9 ++++++++ src/main.cpp | 43 ++++++++++++++++++++++++++++++++------- src/main.h | 4 +++- src/miner.cpp | 8 +++++++- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index f937844e9..6195d6a79 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -13,4 +13,13 @@ 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() */ +enum { + /* Use GetMedianTimePast() instead of nTime for end point timestamp. */ + LOCKTIME_MEDIAN_TIME_PAST = (1 << 1), +}; + +/** Used as the flags parameter to CheckFinalTx() in non-consensus code */ +static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = 0; + #endif // BITCOIN_CONSENSUS_CONSENSUS_H diff --git a/src/main.cpp b/src/main.cpp index 621fd720e..af907d06b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -709,10 +709,35 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) return true; } -bool CheckFinalTx(const CTransaction &tx) +bool CheckFinalTx(const CTransaction &tx, int flags) { AssertLockHeld(cs_main); - return IsFinalTx(tx, chainActive.Height() + 1, GetAdjustedTime()); + + // By convention a negative value for flags indicates that the + // current network-enforced consensus rules should be used. In + // a future soft-fork scenario that would mean checking which + // rules would be enforced for the next block and setting the + // appropriate flags. At the present time no soft-forks are + // scheduled, so no flags are set. + flags = std::max(flags, 0); + + // CheckFinalTx() uses chainActive.Height()+1 to evaluate + // nLockTime because 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(). + const int nBlockHeight = chainActive.Height() + 1; + + // 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. + // However this changes once median past time-locks are enforced: + const int64_t nBlockTime = (flags & LOCKTIME_MEDIAN_TIME_PAST) + ? chainActive.Tip()->GetMedianTimePast() + : GetAdjustedTime(); + + return IsFinalTx(tx, nBlockHeight, nBlockTime); } /** @@ -931,9 +956,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. - if (!CheckFinalTx(tx)) - return state.DoS(0, error("AcceptToMemoryPool: non-final"), - REJECT_NONSTANDARD, "non-final"); + if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) + return state.DoS(0, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? uint256 hash = tx.GetHash(); @@ -2809,10 +2833,15 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn const Consensus::Params& consensusParams = Params().GetConsensus(); // Check that all transactions are finalized - BOOST_FOREACH(const CTransaction& tx, block.vtx) - if (!IsFinalTx(tx, nHeight, block.GetBlockTime())) { + BOOST_FOREACH(const CTransaction& tx, block.vtx) { + int nLockTimeFlags = 0; + int64_t nLockTimeCutoff = (nLockTimeFlags & LOCKTIME_MEDIAN_TIME_PAST) + ? pindexPrev->GetMedianTimePast() + : block.GetBlockTime(); + if (!IsFinalTx(tx, nHeight, nLockTimeCutoff)) { return state.DoS(10, error("%s: contains a non-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal"); } + } // Enforce block.nVersion=2 rule that the coinbase starts with serialized block height // if 750 of the last 1,000 blocks are version 2 or greater (51/100 if testnet): diff --git a/src/main.h b/src/main.h index 58c717c27..74911f6e6 100644 --- a/src/main.h +++ b/src/main.h @@ -338,8 +338,10 @@ 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. + * + * See consensus/consensus.h for flag definitions. */ -bool CheckFinalTx(const CTransaction &tx); +bool CheckFinalTx(const CTransaction &tx, int flags = -1); /** * Closure representing one script verification diff --git a/src/miner.cpp b/src/miner.cpp index f5919ca3a..5972f7421 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -139,6 +139,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CBlockIndex* pindexPrev = chainActive.Tip(); const int nHeight = pindexPrev->nHeight + 1; pblock->nTime = GetAdjustedTime(); + const int64_t nMedianTimePast = pindexPrev->GetMedianTimePast(); CCoinsViewCache view(pcoinsTip); // Priority order to process transactions @@ -153,7 +154,12 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) mi != mempool.mapTx.end(); ++mi) { const CTransaction& tx = mi->second.GetTx(); - if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, pblock->nTime)) + + int64_t nLockTimeCutoff = (STANDARD_LOCKTIME_VERIFY_FLAGS & LOCKTIME_MEDIAN_TIME_PAST) + ? nMedianTimePast + : pblock->GetBlockTime(); + + if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, nLockTimeCutoff)) continue; COrphan* porphan = NULL; From f720c5fc9f34899ec82e23d12f369c9bf37003e7 Mon Sep 17 00:00:00 2001 From: Mark Friedenbach Date: Wed, 3 Jun 2015 15:01:47 -0700 Subject: [PATCH 2/2] Enable policy enforcing GetMedianTimePast as the end point of lock-time constraints Transactions are not allowed in the memory pool or selected for inclusion in a block until their lock times exceed chainActive.Tip()->GetMedianTimePast(). However blocks including transactions which are only mature under the old rules are still accepted; this is *not* the soft-fork required to actually rely on the new constraint in production. --- src/consensus/consensus.h | 2 +- src/test/miner_tests.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 6195d6a79..fc2d01f91 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -20,6 +20,6 @@ enum { }; /** Used as the flags parameter to CheckFinalTx() in non-consensus code */ -static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = 0; +static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_MEDIAN_TIME_PAST; #endif // BITCOIN_CONSENSUS_CONSENSUS_H diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9e4fd8513..e419d61e7 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(!CheckFinalTx(tx)); + BOOST_CHECK(!CheckFinalTx(tx, LOCKTIME_MEDIAN_TIME_PAST)); // 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(!CheckFinalTx(tx2)); + BOOST_CHECK(!CheckFinalTx(tx2, LOCKTIME_MEDIAN_TIME_PAST)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); @@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) //BOOST_CHECK(CheckFinalTx(tx2)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); - BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3); + BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 2); delete pblocktemplate; chainActive.Tip()->nHeight--;