Modify locktime thresholds to accomodate 64 bits and millisecond timestamps (#1770)

* Change SequenceLockTimeDisabled to 1 << 63

* Move LockTimeThreshold to constants

* Update locktime constants according to new proposal

* Fix opcodeCheckSequenceVerify and failed tests

* Disallow numbers above 8 bytes in makeScriptNum

* Use littleEndian.Uint64 for sequence instead of ScriptNum

* Update comments on constants

* Update some more comments

domain/consensus/processes/blockvalidator/block_body_in_context.go

@@ -4,12 +4,11 @@ import (
 	"math"
 
 	"github.com/kaspanet/kaspad/domain/consensus/model"
-	"github.com/kaspanet/kaspad/infrastructure/logger"
-
 	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
 	"github.com/kaspanet/kaspad/domain/consensus/ruleerrors"
 	"github.com/kaspanet/kaspad/domain/consensus/utils/consensushashing"
-	"github.com/kaspanet/kaspad/domain/consensus/utils/txscript"
+	"github.com/kaspanet/kaspad/domain/consensus/utils/constants"
+	"github.com/kaspanet/kaspad/infrastructure/logger"
 	"github.com/pkg/errors"
 )
 
@@ -161,7 +160,7 @@ func (v *blockValidator) isFinalizedTransaction(tx *externalapi.DomainTransactio
 	// value is before the txscript.LockTimeThreshold. When it is under the
 	// threshold it is a block blue score.
 	blockTimeOrBlueScore := uint64(0)
-	if lockTime < txscript.LockTimeThreshold {
+	if lockTime < constants.LockTimeThreshold {
 		blockTimeOrBlueScore = blockBlueScore
 	} else {
 		blockTimeOrBlueScore = uint64(blockTime)

domain/consensus/processes/transactionvalidator/transaction_in_context.go

@@ -294,7 +294,7 @@ func (v *transactionValidator) calcTxSequenceLockFromReferencedUTXOEntries(stagi
 			// amount to convert to the proper relative time-lock. We also
 			// subtract one from the relative lock to maintain the original
 			// lockTime semantics.
-			timeLockMilliseconds := (relativeLock << constants.SequenceLockTimeGranularity) - 1
+			timeLockMilliseconds := (relativeLock * constants.SequenceLockTimeGranularity) - 1
 			timeLock := medianTime + timeLockMilliseconds
 			if timeLock > sequenceLock.Milliseconds {
 				sequenceLock.Milliseconds = timeLock

domain/consensus/utils/constants/constants.go

@@ -26,21 +26,24 @@ const (
 	// SequenceLockTimeDisabled is a flag that if set on a transaction
 	// input's sequence number, the sequence number will not be interpreted
 	// as a relative locktime.
-	SequenceLockTimeDisabled = 1 << 31
+	SequenceLockTimeDisabled uint64 = 1 << 63
 
 	// SequenceLockTimeIsSeconds is a flag that if set on a transaction
-	// input's sequence number, the relative locktime has units of 512
-	// seconds.
-	SequenceLockTimeIsSeconds = 1 << 22
+	// input's sequence number, the relative locktime has units of 1 second.
+	// If the flag is not set, the relative lockatime is according to DAA score.
+	SequenceLockTimeIsSeconds uint64 = 1 << 62
 
 	// SequenceLockTimeMask is a mask that extracts the relative locktime
 	// when masked against the transaction input sequence number.
-	SequenceLockTimeMask = 0x0000ffff
+	SequenceLockTimeMask uint64 = 0x00000000ffffffff
 
 	// SequenceLockTimeGranularity is the defined time based granularity
 	// for milliseconds-based relative time locks. When converting from milliseconds
-	// to a sequence number, the value is right shifted by this amount,
-	// therefore the granularity of relative time locks in 524288 or 2^19
-	// seconds. Enforced relative lock times are multiples of 524288 milliseconds.
-	SequenceLockTimeGranularity = 19
+	// to a sequence number, the value is multiplied by this amount,
+	// therefore the granularity of relative time locks is 1000 milliseconds or 1 second.
+	SequenceLockTimeGranularity = 1000
+
+	// LockTimeThreshold is the number below which a lock time is
+	// interpreted to be a block number.
+	LockTimeThreshold = 5e8 // Tue Nov 5 00:53:20 1985 UTC
 )

domain/consensus/utils/txscript/consensus.go

@@ -1,13 +0,0 @@
-// Copyright (c) 2015-2016 The btcsuite developers
-// Use of this source code is governed by an ISC
-// license that can be found in the LICENSE file.
-
-package txscript
-
-const (
-	// LockTimeThreshold is the number below which a lock time is
-	// interpreted to be a block number. Since an average of one block
-	// is generated per 10 minutes, this allows blocks for about 9,512
-	// years.
-	LockTimeThreshold = 5e8 // Tue Nov 5 00:53:20 1985 UTC
-)

domain/consensus/utils/txscript/data/script_tests.json

@@ -2883,11 +2883,11 @@
     "OK"
   ],
   [
-    "TRUE 2147483648",
+    "TRUE DATA_8 0x0000000000000080",
     "CHECKSEQUENCEVERIFY",
     "",
     "OK",
-    "CSV passes if stack top bit 1 << 31 is set"
+    "CSV passes if stack top bit 1 << 63 is set"
   ],
   [
     "",
@@ -5272,20 +5272,6 @@
     "INVALID_STACK_OPERATION",
     "CSV automatically fails on a empty stack"
   ],
-  [
-    "-1",
-    "CHECKSEQUENCEVERIFY",
-    "",
-    "NEGATIVE_LOCKTIME",
-    "CSV automatically fails if stack top is negative"
-  ],
-  [
-    "0x0100",
-    "CHECKSEQUENCEVERIFY",
-    "",
-    "UNKNOWN_ERROR",
-    "CSV fails if stack top is not minimally encoded"
-  ],
   [
     "0",
     "CHECKSEQUENCEVERIFY",

domain/consensus/utils/txscript/opcode.go

@@ -1121,7 +1121,7 @@ func opcodeCheckLockTimeVerify(op *parsedOpcode, vm *Engine) error {
 	// which the transaction is finalized or a timestamp depending on if the
 	// value is before the txscript.LockTimeThreshold. When it is under the
 	// threshold it is a block height.
-	err = verifyLockTime(vm.tx.LockTime, LockTimeThreshold,
+	err = verifyLockTime(vm.tx.LockTime, constants.LockTimeThreshold,
 		uint64(lockTime))
 	if err != nil {
 		return err
@@ -1153,39 +1153,32 @@ func opcodeCheckLockTimeVerify(op *parsedOpcode, vm *Engine) error {
 // LockTime field of the transaction containing the script signature
 // validating if the transaction outputs are spendable yet.
 func opcodeCheckSequenceVerify(op *parsedOpcode, vm *Engine) error {
-
-	// The current transaction sequence is a uint64 resulting in a maximum
-	// sequence of 2^63-1. However, scriptNums are signed and therefore a
-	// standard 4-byte scriptNum would only support up to a maximum of
-	// 2^31-1. Thus, a 5-byte scriptNum is used here since it will support
-	// up to 2^39-1 which allows sequences beyond the current sequence
-	// limit.
-	//
-	// PopByteArray is used here instead of PopInt because we do not want
-	// to be limited to a 4-byte integer for reasons specified above.
-	so, err := vm.dstack.PopByteArray()
-	if err != nil {
-		return err
-	}
-	stackSequence, err := makeScriptNum(so, 5)
+	sequenceBytes, err := vm.dstack.PopByteArray()
 	if err != nil {
 		return err
 	}
 
-	// In the rare event that the argument needs to be < 0 due to some
-	// arithmetic being done first, you can always use
-	// 0 OP_MAX OP_CHECKSEQUENCEVERIFY.
-	if stackSequence < 0 {
-		str := fmt.Sprintf("negative sequence: %d", stackSequence)
-		return scriptError(ErrNegativeLockTime, str)
+	// Make sure sequenceBytes is exactly 8 bytes.
+	// If more - return ErrNumberTooBig
+	// If less - pad with 0's
+	if len(sequenceBytes) > 8 {
+		str := fmt.Sprintf("sequence value represented as %x is longer then 8 bytes", sequenceBytes)
+		return scriptError(ErrNumberTooBig, str)
+	}
+	if len(sequenceBytes) < 8 {
+		paddedSequenceBytes := make([]byte, 8)
+		copy(paddedSequenceBytes[8-len(sequenceBytes):], sequenceBytes)
+		sequenceBytes = paddedSequenceBytes
 	}
 
-	sequence := uint64(stackSequence)
+	// Don't use makeScriptNum here, since sequence is not an actual number, minimal encoding rules don't apply to it,
+	// and is more convenient to be represented as an unsigned int.
+	stackSequence := binary.LittleEndian.Uint64(sequenceBytes)
 
 	// To provide for future soft-fork extensibility, if the
 	// operand has the disabled lock-time flag set,
 	// CHECKSEQUENCEVERIFY behaves as a NOP.
-	if sequence&uint64(constants.SequenceLockTimeDisabled) != 0 {
+	if stackSequence&constants.SequenceLockTimeDisabled != 0 {
 		return nil
 	}
 
@@ -1201,10 +1194,10 @@ func opcodeCheckSequenceVerify(op *parsedOpcode, vm *Engine) error {
 	}
 
 	// Mask off non-consensus bits before doing comparisons.
-	lockTimeMask := uint64(constants.SequenceLockTimeIsSeconds |
-		constants.SequenceLockTimeMask)
-	return verifyLockTime(txSequence&lockTimeMask,
-		constants.SequenceLockTimeIsSeconds, sequence&lockTimeMask)
+	lockTimeMask := constants.SequenceLockTimeIsSeconds | constants.SequenceLockTimeMask
+	maskedTxSequence := txSequence & lockTimeMask
+	maskedStackSequence := stackSequence & lockTimeMask
+	return verifyLockTime(maskedTxSequence, constants.SequenceLockTimeIsSeconds, maskedStackSequence)
 }
 
 // opcodeToAltStack removes the top item from the main data stack and pushes it

domain/consensus/utils/txscript/scriptnum.go

@@ -190,6 +190,12 @@ func makeScriptNum(v []byte, scriptNumLen int) (scriptNum, error) {
 		return 0, scriptError(ErrNumberTooBig, str)
 	}
 
+	// Disallow any numerical value larger than 8 bytes, so that it fits in int64.
+	if len(v) > 8 {
+		str := fmt.Sprintf("numeric value encoded as %x is longer than 8 bytes", v)
+		return 0, scriptError(ErrNumberTooBig, str)
+	}
+
 	if err := checkMinimalDataEncoding(v); err != nil {
 		return 0, err
 	}

domain/consensus/utils/txscript/scriptnum_test.go

@@ -136,10 +136,6 @@ func TestMakeScriptNum(t *testing.T) {
 		{hexToBytes("ffffffffff"), -549755813887, 5, nil},
 		{hexToBytes("ffffffffffffff7f"), 9223372036854775807, 8, nil},
 		{hexToBytes("ffffffffffffffff"), -9223372036854775807, 8, nil},
-		{hexToBytes("ffffffffffffffff7f"), -1, 9, nil},
-		{hexToBytes("ffffffffffffffffff"), 1, 9, nil},
-		{hexToBytes("ffffffffffffffffff7f"), -1, 10, nil},
-		{hexToBytes("ffffffffffffffffffff"), 1, 10, nil},
 
 		// Minimally encoded values that are out of range for data that
 		// is interpreted as script numbers with the minimal encoding
@@ -173,6 +169,9 @@ func TestMakeScriptNum(t *testing.T) {
 		{hexToBytes("00000800"), 0, defaultScriptNumLen, errMinimalData}, // 524288
 		{hexToBytes("00007000"), 0, defaultScriptNumLen, errMinimalData}, // 7340032
 		{hexToBytes("0009000100"), 0, 5, errMinimalData},                 // 16779520
+		// Values above 8 bytes should always return error
+		{hexToBytes("ffffffffffffffffff"), 0, 9, errNumTooBig},
+		{hexToBytes("00000000000000000000"), 0, 10, errNumTooBig},
 	}
 
 	for _, test := range tests {