Revert "Add rules--presently disabled--for using GetMedianTimePast as endpoint for lock-time calculations"

This reverts commit 9d55050773.

As noted by Luke-Jr, under some conditions this will accept transactions which are invalid by the network
 rules.  This happens when the current block time is head of the median time past and a transaction's
 locktime is in the middle.

This could be addressed by changing the rule to MAX(this_block_time, MTP+offset) but this solution and
 the particular offset used deserve some consideration.
This commit is contained in:
Gregory Maxwell 2015-11-01 20:05:18 +00:00
parent 8537ecdfc4
commit 40cd32e835
5 changed files with 7 additions and 54 deletions

View File

@ -13,10 +13,4 @@ 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) */ /** Coinbase transaction outputs can only be spent after this number of new blocks (network rule) */
static const int COINBASE_MATURITY = 100; 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),
};
#endif // BITCOIN_CONSENSUS_CONSENSUS_H #endif // BITCOIN_CONSENSUS_CONSENSUS_H

View File

@ -650,35 +650,10 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
return true; return true;
} }
bool CheckFinalTx(const CTransaction &tx, int flags) bool CheckFinalTx(const CTransaction &tx)
{ {
AssertLockHeld(cs_main); 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);
} }
unsigned int GetLegacySigOpCount(const CTransaction& tx) unsigned int GetLegacySigOpCount(const CTransaction& tx)
@ -822,7 +797,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
// Only accept nLockTime-using transactions that can be mined in the next // 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 // block; we don't want our mempool filled up with transactions that can't
// be mined yet. // be mined yet.
if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) if (!CheckFinalTx(tx))
return state.DoS(0, false, REJECT_NONSTANDARD, "non-final"); return state.DoS(0, false, REJECT_NONSTANDARD, "non-final");
// is it already in the memory pool? // is it already in the memory pool?
@ -2748,15 +2723,10 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
const Consensus::Params& consensusParams = Params().GetConsensus(); const Consensus::Params& consensusParams = Params().GetConsensus();
// Check that all transactions are finalized // Check that all transactions are finalized
BOOST_FOREACH(const CTransaction& tx, block.vtx) { BOOST_FOREACH(const CTransaction& tx, block.vtx)
int nLockTimeFlags = 0; if (!IsFinalTx(tx, nHeight, block.GetBlockTime())) {
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"); 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 // 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): // if 750 of the last 1,000 blocks are version 2 or greater (51/100 if testnet):

View File

@ -308,10 +308,8 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime);
* Check if transaction will be final in the next block to be created. * Check if transaction will be final in the next block to be created.
* *
* Calls IsFinalTx() with current block height and appropriate block time. * Calls IsFinalTx() with current block height and appropriate block time.
*
* See consensus/consensus.h for flag definitions.
*/ */
bool CheckFinalTx(const CTransaction &tx, int flags = -1); bool CheckFinalTx(const CTransaction &tx);
/** /**
* Closure representing one script verification * Closure representing one script verification

View File

@ -148,7 +148,6 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
CBlockIndex* pindexPrev = chainActive.Tip(); CBlockIndex* pindexPrev = chainActive.Tip();
const int nHeight = pindexPrev->nHeight + 1; const int nHeight = pindexPrev->nHeight + 1;
pblock->nTime = GetAdjustedTime(); pblock->nTime = GetAdjustedTime();
const int64_t nMedianTimePast = pindexPrev->GetMedianTimePast();
CCoinsViewCache view(pcoinsTip); CCoinsViewCache view(pcoinsTip);
// Priority order to process transactions // Priority order to process transactions
@ -163,12 +162,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
mi != mempool.mapTx.end(); ++mi) mi != mempool.mapTx.end(); ++mi)
{ {
const CTransaction& tx = mi->GetTx(); const CTransaction& tx = mi->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; continue;
COrphan* porphan = NULL; COrphan* porphan = NULL;

View File

@ -43,9 +43,6 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY
/** For convenience, standard but not mandatory verify flags. */ /** 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; 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 = 0;
bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType); bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType);
/** /**
* Check for standard transaction types * Check for standard transaction types