From 0b334bc841e5ee8fc8deda8f6a72a86c3e344134 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Sun, 6 Oct 2013 13:01:54 -0500 Subject: [PATCH] Correct reading of serialized height in coinbase. This commit corrects the reading of the serialized height in coinbase transactions for block height of version 2 or greater. On mainnet, the serialized height is always 3 bytes and will continue to be so for something like another ~159 years, so there was no issue with mainnet. However on testnet, there are some version 2 blocks which are low enough in the chain to only take 2 bytes to serialize. In addition, this commit adds a full tests for the relavant function including negative tests and variable length serialized lengths for block heights. Closes #1. --- internal_test.go | 7 +++++++ test_coverage.txt | 30 ++++++++++++++-------------- validate.go | 19 +++++++++++++----- validate_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 20 deletions(-) diff --git a/internal_test.go b/internal_test.go index 0ad1f9d4..b919d543 100644 --- a/internal_test.go +++ b/internal_test.go @@ -13,6 +13,7 @@ package btcchain import ( "github.com/conformal/btcutil" + "github.com/conformal/btcwire" "time" ) @@ -33,3 +34,9 @@ func TstSetCoinbaseMaturity(maturity int64) { func TstTimeSorter(times []time.Time) timeSorter { return timeSorter(times) } + +// TstCheckSerializedHeight makes the internal checkSerializedHeight function +// available to the test package. +func TstCheckSerializedHeight(coinbaseTx *btcwire.MsgTx, wantHeight int64) error { + return checkSerializedHeight(coinbaseTx, wantHeight) +} diff --git a/test_coverage.txt b/test_coverage.txt index e0681766..12a4ee8a 100644 --- a/test_coverage.txt +++ b/test_coverage.txt @@ -1,27 +1,28 @@ +github.com/conformal/btcchain/validate.go checkSerializedHeight 100.00% (17/17) github.com/conformal/btcchain/chain.go BlockChain.removeOrphanBlock 100.00% (16/16) github.com/conformal/btcchain/validate.go countSigOps 100.00% (8/8) github.com/conformal/btcchain/checkpoints.go init 100.00% (6/6) github.com/conformal/btcchain/difficulty.go ShaHashToBig 100.00% (5/5) github.com/conformal/btcchain/merkle.go hashMerkleBranches 100.00% (5/5) -github.com/conformal/btcchain/merkle.go nextPowerOfTwo 100.00% (4/4) github.com/conformal/btcchain/chain.go newBlockNode 100.00% (4/4) +github.com/conformal/btcchain/merkle.go nextPowerOfTwo 100.00% (4/4) github.com/conformal/btcchain/process.go BlockChain.blockExists 100.00% (3/3) github.com/conformal/btcchain/checkpoints.go newShaHashFromStr 100.00% (2/2) github.com/conformal/btcchain/chain.go New 100.00% (2/2) -github.com/conformal/btcchain/log.go DisableLog 100.00% (1/1) -github.com/conformal/btcchain/validate.go calcBlockSubsidy 100.00% (1/1) +github.com/conformal/btcchain/log.go init 100.00% (1/1) github.com/conformal/btcchain/timesorter.go timeSorter.Less 100.00% (1/1) github.com/conformal/btcchain/timesorter.go timeSorter.Swap 100.00% (1/1) -github.com/conformal/btcchain/timesorter.go timeSorter.Len 100.00% (1/1) -github.com/conformal/btcchain/log.go init 100.00% (1/1) github.com/conformal/btcchain/params.go BlockChain.chainParams 100.00% (1/1) github.com/conformal/btcchain/checkpoints.go BlockChain.DisableCheckpoints 100.00% (1/1) +github.com/conformal/btcchain/validate.go calcBlockSubsidy 100.00% (1/1) +github.com/conformal/btcchain/timesorter.go timeSorter.Len 100.00% (1/1) +github.com/conformal/btcchain/log.go DisableLog 100.00% (1/1) github.com/conformal/btcchain/merkle.go BuildMerkleTreeStore 94.12% (16/17) github.com/conformal/btcchain/txlookup.go disconnectTransactions 93.75% (15/16) github.com/conformal/btcchain/txlookup.go fetchTxListMain 93.33% (14/15) -github.com/conformal/btcchain/process.go BlockChain.processOrphans 92.86% (13/14) github.com/conformal/btcchain/chain.go BlockChain.getReorganizeNodes 92.86% (13/14) +github.com/conformal/btcchain/process.go BlockChain.processOrphans 92.86% (13/14) github.com/conformal/btcchain/chain.go BlockChain.connectBestChain 90.00% (27/30) github.com/conformal/btcchain/chain.go BlockChain.getPrevNodeFromBlock 88.89% (8/9) github.com/conformal/btcchain/scriptval.go ValidateTransactionScripts 88.24% (30/34) @@ -64,27 +65,26 @@ github.com/conformal/btcchain/blocklocator.go BlockChain.BlockLocatorFromHash github.com/conformal/btcchain/checkpoints.go BlockChain.IsCheckpointCandidate 0.00% (0/32) github.com/conformal/btcchain/validate.go countP2SHSigOps 0.00% (0/24) github.com/conformal/btcchain/chain.go BlockChain.GenerateInitialIndex 0.00% (0/17) -github.com/conformal/btcchain/txlookup.go BlockChain.FetchTransactionStore 0.00% (0/15) github.com/conformal/btcchain/difficulty.go BlockChain.calcEasiestDifficulty 0.00% (0/15) -github.com/conformal/btcchain/validate.go checkSerializedHeight 0.00% (0/12) +github.com/conformal/btcchain/txlookup.go BlockChain.FetchTransactionStore 0.00% (0/15) github.com/conformal/btcchain/difficulty.go BlockChain.findPrevTestNetDifficulty 0.00% (0/12) github.com/conformal/btcchain/chain.go BlockChain.removeBlockNode 0.00% (0/12) github.com/conformal/btcchain/chain.go BlockChain.GetOrphanRoot 0.00% (0/11) github.com/conformal/btcchain/chain.go BlockChain.IsCurrent 0.00% (0/9) github.com/conformal/btcchain/chain.go removeChildNode 0.00% (0/8) -github.com/conformal/btcchain/chain.go BlockChain.HaveInventory 0.00% (0/7) github.com/conformal/btcchain/log.go SetLogWriter 0.00% (0/7) +github.com/conformal/btcchain/chain.go BlockChain.HaveInventory 0.00% (0/7) github.com/conformal/btcchain/blocklocator.go BlockChain.LatestBlockLocator 0.00% (0/6) -github.com/conformal/btcchain/chain.go BlockChain.IsKnownOrphan 0.00% (0/5) github.com/conformal/btcchain/checkpoints.go isNonstandardTransaction 0.00% (0/5) -github.com/conformal/btcchain/checkpoints.go BlockChain.checkpointData 0.00% (0/4) +github.com/conformal/btcchain/chain.go BlockChain.IsKnownOrphan 0.00% (0/5) github.com/conformal/btcchain/validate.go isTransactionSpent 0.00% (0/4) -github.com/conformal/btcchain/notifications.go NotificationType.String 0.00% (0/3) +github.com/conformal/btcchain/checkpoints.go BlockChain.checkpointData 0.00% (0/4) github.com/conformal/btcchain/chain.go addChildrenWork 0.00% (0/3) -github.com/conformal/btcchain/log.go newLogClosure 0.00% (0/1) +github.com/conformal/btcchain/notifications.go NotificationType.String 0.00% (0/3) github.com/conformal/btcchain/log.go UseLogger 0.00% (0/1) -github.com/conformal/btcchain/chain.go BlockChain.DisableVerify 0.00% (0/1) +github.com/conformal/btcchain/log.go newLogClosure 0.00% (0/1) github.com/conformal/btcchain/log.go logClosure.String 0.00% (0/1) github.com/conformal/btcchain/process.go RuleError.Error 0.00% (0/1) -github.com/conformal/btcchain ------------------------------------- 53.97% (625/1158) +github.com/conformal/btcchain/chain.go BlockChain.DisableVerify 0.00% (0/1) +github.com/conformal/btcchain ------------------------------------- 55.20% (642/1163) diff --git a/validate.go b/validate.go index b1e9a354..b22a54fc 100644 --- a/validate.go +++ b/validate.go @@ -498,17 +498,26 @@ func (b *BlockChain) checkBlockSanity(block *btcutil.Block) error { // transaction starts with the serialized block height of wantHeight. func checkSerializedHeight(coinbaseTx *btcwire.MsgTx, wantHeight int64) error { sigScript := coinbaseTx.TxIn[0].SignatureScript - if len(sigScript) < 4 { + if len(sigScript) < 1 { str := "the coinbase signature script for blocks of " + "version %d or greater must start with the " + - "serialized block height" + "length of the serialized block height" str = fmt.Sprintf(str, serializedHeightVersion) return RuleError(str) } - serializedHeightBytes := make([]byte, 4, 4) - copy(serializedHeightBytes, sigScript[1:4]) - serializedHeight := binary.LittleEndian.Uint32(serializedHeightBytes) + serializedLen := int(sigScript[0]) + if len(sigScript[1:]) < serializedLen { + str := "the coinbase signature script for blocks of " + + "version %d or greater must start with the " + + "serialized block height" + str = fmt.Sprintf(str, serializedLen) + return RuleError(str) + } + + serializedHeightBytes := make([]byte, 8, 8) + copy(serializedHeightBytes, sigScript[1:serializedLen+1]) + serializedHeight := binary.LittleEndian.Uint64(serializedHeightBytes) if int64(serializedHeight) != wantHeight { str := fmt.Sprintf("the coinbase signature script serialized "+ "block height is %d when %d was expected", diff --git a/validate_test.go b/validate_test.go index 3718049a..1b2998cd 100644 --- a/validate_test.go +++ b/validate_test.go @@ -9,7 +9,9 @@ import ( "github.com/conformal/btcdb" "github.com/conformal/btcutil" "github.com/conformal/btcwire" + "math" "os" + "reflect" "testing" "time" ) @@ -36,6 +38,54 @@ func TestCheckBlockSanity(t *testing.T) { } } +// TestCheckSerializedHeight tests the checkSerializedHeight function with +// various serialized heights and also does negative tests to ensure errors +// and handled properly. +func TestCheckSerializedHeight(t *testing.T) { + // Create an empty coinbase template to be used in the tests below. + coinbaseOutpoint := btcwire.NewOutPoint(&btcwire.ShaHash{}, math.MaxUint32) + coinbaseTx := btcwire.NewMsgTx() + coinbaseTx.Version = 2 + coinbaseTx.AddTxIn(btcwire.NewTxIn(coinbaseOutpoint, nil)) + + // + tests := []struct { + sigScript []byte // Serialized data + wantHeight int64 // Expected height + err error // Expected error type + }{ + // No serialized height length. + {[]byte{}, 0, btcchain.RuleError("")}, + // Serialized height length with no height bytes. + {[]byte{0x02}, 0, btcchain.RuleError("")}, + // Serialized height length with too few height bytes. + {[]byte{0x02, 0x4a}, 0, btcchain.RuleError("")}, + // Serialized height that needs 2 bytes to encode. + {[]byte{0x02, 0x4a, 0x52}, 21066, nil}, + // Serialized height that needs 2 bytes to encode, but backwards + // endianness. + {[]byte{0x02, 0x4a, 0x52}, 19026, btcchain.RuleError("")}, + // Serialized height that needs 3 bytes to encode. + {[]byte{0x03, 0x40, 0x0d, 0x03}, 200000, nil}, + // Serialized height that needs 3 bytes to encode, but backwards + // endianness. + {[]byte{0x03, 0x40, 0x0d, 0x03}, 1074594560, btcchain.RuleError("")}, + } + + t.Logf("Running %d tests", len(tests)) + for i, test := range tests { + tx := coinbaseTx.Copy() + tx.TxIn[0].SignatureScript = test.sigScript + + err := btcchain.TstCheckSerializedHeight(tx, test.wantHeight) + if reflect.TypeOf(err) != reflect.TypeOf(test.err) { + t.Errorf("checkSerializedHeight #%d wrong error type "+ + "got: %v <%T>, want: %T", i, err, err, test.err) + continue + } + } +} + // Block100000 defines block 100,000 of the block chain. It is used to // test Block operations. var Block100000 = btcwire.MsgBlock{