From 72b221f7453ee34873fe3f41944629407e52f711 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 21 Sep 2017 16:58:23 +0300 Subject: [PATCH] Revert "fix BIP68 granularity and mask (#1641)" (#1647) This reverts commit d3829e55b1e8fd0da0f0285daf0b4fd26fd00f15. Reverting and postponing the fix till later. Probably should write a DIP and explore different options first. --- qa/rpc-tests/bip68-sequence.py | 4 ++-- src/primitives/transaction.h | 24 +++++++----------------- src/script/interpreter.cpp | 2 +- src/test/miner_tests.cpp | 12 ++++++------ src/validation.cpp | 4 ++-- 5 files changed, 18 insertions(+), 28 deletions(-) diff --git a/qa/rpc-tests/bip68-sequence.py b/qa/rpc-tests/bip68-sequence.py index ce9afddba8cfa..33e05dfc51a09 100755 --- a/qa/rpc-tests/bip68-sequence.py +++ b/qa/rpc-tests/bip68-sequence.py @@ -15,8 +15,8 @@ SEQUENCE_LOCKTIME_DISABLE_FLAG = (1<<31) SEQUENCE_LOCKTIME_TYPE_FLAG = (1<<22) # this means use time (0 means height) -SEQUENCE_LOCKTIME_GRANULARITY = 7 # this is a bit-shift -SEQUENCE_LOCKTIME_MASK = 0x0003ffff +SEQUENCE_LOCKTIME_GRANULARITY = 9 # this is a bit-shift +SEQUENCE_LOCKTIME_MASK = 0x0000ffff # RPC error for non-BIP68 final transactions NOT_FINAL_ERROR = "64: non-BIP68-final" diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 806d912f4fda8..f87b45e728ac3 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -73,32 +73,22 @@ class CTxIn 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 128 seconds, + * 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 LEGACY_SEQUENCE_LOCKTIME_MASK = 0x0000ffff; - static const uint32_t DIP0001_SEQUENCE_LOCKTIME_MASK = 0x0003ffff; - static const uint32_t GetSequenceLocktimeMask(bool fDIP0001Active /*= false */) - { - return fDIP0001Active ? DIP0001_SEQUENCE_LOCKTIME_MASK : LEGACY_SEQUENCE_LOCKTIME_MASK; - } + 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 150s on average, the minimum granularity - * for time-based relative lock-time is fixed at 128 seconds. + * 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 128 = 2^7, or equivalently shifting up by - * 7 bits. */ - static const int LEGACY_SEQUENCE_LOCKTIME_GRANULARITY = 9; - static const int DIP0001_SEQUENCE_LOCKTIME_GRANULARITY = 7; - static const int GetSequenceLocktimeGranularity(bool fDIP0001Active /*= false */) - { - return fDIP0001Active ? DIP0001_SEQUENCE_LOCKTIME_GRANULARITY : LEGACY_SEQUENCE_LOCKTIME_GRANULARITY; - } + * multiplying by 512 = 2^9, or equivalently shifting up by + * 9 bits. */ + static const int SEQUENCE_LOCKTIME_GRANULARITY = 9; CTxIn() { diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 994f565404623..149a4f0156315 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1213,7 +1213,7 @@ bool TransactionSignatureChecker::CheckSequence(const CScriptNum& nSequence) con // Mask off any bits that do not have consensus-enforced meaning // before doing the integer comparisons - const uint32_t nLockTimeMask = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | CTxIn::LEGACY_SEQUENCE_LOCKTIME_MASK; + const uint32_t nLockTimeMask = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | CTxIn::SEQUENCE_LOCKTIME_MASK; const int64_t txToSequenceMasked = txToSequence & nLockTimeMask; const CScriptNum nSequenceMasked = nSequence & nLockTimeMask; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index d9db299b6f586..5bbbafa071fb8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -311,7 +311,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // 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::GetSequenceLocktimeGranularity(fDIP0001ActiveAtTip)) + 1); // txFirst[1] is the 3rd block + 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)); @@ -319,10 +319,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) - chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += fDIP0001ActiveAtTip ? 128 : 512; //Trick the MedianTimePast - BOOST_CHECK(SequenceLocks(tx, flags, &prevheights, CreateBlockIndex(chainActive.Tip()->nHeight + 1))); // Sequence locks pass 128/512 seconds later + 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 -= fDIP0001ActiveAtTip ? 128 : 512; //undo tricked MTP + chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP // absolute height locked tx.vin[0].prevout.hash = txFirst[2]->GetHash(); @@ -368,9 +368,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // 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 by 1 and time by 128/512, all of them should be mined + // 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 += fDIP0001ActiveAtTip ? 128 : 512; //Trick the MedianTimePast + chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast chainActive.Tip()->nHeight++; SetMockTime(chainActive.Tip()->GetMedianTimePast() + 1); diff --git a/src/validation.cpp b/src/validation.cpp index 4aa369e4ba927..6995b1a297d9d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -301,9 +301,9 @@ static std::pair CalculateSequenceLocks(const CTransaction &tx, in // 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::GetSequenceLocktimeMask(fDIP0001ActiveAtTip)) << CTxIn::GetSequenceLocktimeGranularity(fDIP0001ActiveAtTip)) - 1); + 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::GetSequenceLocktimeMask(fDIP0001ActiveAtTip)) - 1); + nMinHeight = std::max(nMinHeight, nCoinHeight + (int)(txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) - 1); } }