[DEV-77] Wire converage

* [DEV-77] Added tests for ScriptFreeList

* [DEV-77] Fixed formatting in BlockHeader .SelectedPrevBlock and IsGenesis

* [DEV-77] Restructured returnScriptBuffers to be more idiomatic and testable

* [DEV-77] Added tests for binaryFreeList

* [DEV-77] Added test-cases to TestVarIntWireErrors to check for errors when writing the varint length

* [DEV-77] Added MsgSendHeaders and MsgFeeFilter test-cases for TestMessage

* [DEV-77] Added test for BlockHeader.IsGenesis()

* [DEV-77] using binaryFreeListMaxItems instead of freeListMaxItems in TestBinaryFreeList

* [DEV-77] Fixed error message copy-paste typo

wire/blockheader.go

@@ -66,17 +66,18 @@ func (h *BlockHeader) BlockHash() daghash.Hash {
 	return daghash.DoubleHashH(buf.Bytes())
 }
 
-// BestPrevBlock returns the hash of the selected block header.
+// SelectedPrevBlock returns the hash of the selected block header.
-func (header *BlockHeader) SelectedPrevBlock() *daghash.Hash {
+func (h *BlockHeader) SelectedPrevBlock() *daghash.Hash {
-	if header.NumPrevBlocks == 0 {
+	if h.NumPrevBlocks == 0 {
 		return nil
 	}
 
-	return &header.PrevBlocks[0]
+	return &h.PrevBlocks[0]
 }
 
-func (header *BlockHeader) IsGenesis() bool {
+// IsGenesis returns true iff this block is a genesis block
-	return header.NumPrevBlocks == 0
+func (h *BlockHeader) IsGenesis() bool {
+	return h.NumPrevBlocks == 0
 }
 
 // BtcDecode decodes r using the bitcoin protocol encoding into the receiver.

wire/blockheader_test.go

@@ -315,3 +315,45 @@ func TestBlockHeaderSerializeSize(t *testing.T) {
 		}
 	}
 }
+
+func TestIsGenesis(t *testing.T) {
+	nonce := uint32(123123) // 0x1e0f3
+	bits := uint32(0x1d00ffff)
+	timestamp := time.Unix(0x495fab29, 0) // 2009-01-03 12:15:05 -0600 CST
+
+	baseBlockHdr := &BlockHeader{
+		Version:       1,
+		NumPrevBlocks: 2,
+		PrevBlocks:    []daghash.Hash{mainNetGenesisHash, simNetGenesisHash},
+		MerkleRoot:    mainNetGenesisMerkleRoot,
+		Timestamp:     timestamp,
+		Bits:          bits,
+		Nonce:         nonce,
+	}
+	genesisBlockHdr := &BlockHeader{
+		Version:       1,
+		NumPrevBlocks: 0,
+		PrevBlocks:    []daghash.Hash{},
+		MerkleRoot:    mainNetGenesisMerkleRoot,
+		Timestamp:     timestamp,
+		Bits:          bits,
+		Nonce:         nonce,
+	}
+
+	tests := []struct {
+		in        *BlockHeader // Block header to encode
+		isGenesis bool         // Expected result for call of .IsGenesis
+	}{
+		{genesisBlockHdr, true},
+		{baseBlockHdr, false},
+	}
+
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		isGenesis := test.in.IsGenesis()
+		if isGenesis != test.isGenesis {
+			t.Errorf("BlockHeader.IsGenesis: #%d got: %t, want: %t",
+				i, isGenesis, test.isGenesis)
+		}
+	}
+}

wire/common_test.go

@@ -334,11 +334,14 @@ func TestVarIntWireErrors(t *testing.T) {
 		// Force errors on discriminant.
 		{0, []byte{0x00}, pver, 0, io.ErrShortWrite, io.EOF},
 		// Force errors on 2-byte read/write.
-		{0xfd, []byte{0xfd}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF},
+		{0xfd, []byte{0xfd}, pver, 0, io.ErrShortWrite, io.EOF},              // error on writing length
+		{0xfd, []byte{0xfd}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF}, // error on writing actual data
 		// Force errors on 4-byte read/write.
-		{0x10000, []byte{0xfe}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF},
+		{0x10000, []byte{0xfe}, pver, 0, io.ErrShortWrite, io.EOF},              // error on writing length
+		{0x10000, []byte{0xfe}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF}, // error on writing actual data
 		// Force errors on 8-byte read/write.
-		{0x100000000, []byte{0xff}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF},
+		{0x100000000, []byte{0xff}, pver, 0, io.ErrShortWrite, io.EOF},              // error on writing length
+		{0x100000000, []byte{0xff}, pver, 2, io.ErrShortWrite, io.ErrUnexpectedEOF}, // error on writing actual data
 	}
 
 	t.Logf("Running %d tests", len(tests))
@@ -766,3 +769,51 @@ func TestRandomUint64Errors(t *testing.T) {
 		t.Errorf("Nonce is not 0 [%v]", nonce)
 	}
 }
+
+func TestBinaryFreeList(t *testing.T) {
+	var list binaryFreeList = make(chan []byte, binaryFreeListMaxItems)
+
+	expectedCapacity := 8
+	expectedLength := 8
+
+	first := list.Borrow()
+	if cap(first) != expectedCapacity {
+		t.Errorf("MsgTx.TestBinaryFreeList: Expected capacity for first %d, but got %d",
+			expectedCapacity, cap(first))
+	}
+	if len(first) != expectedLength {
+		t.Errorf("MsgTx.TestBinaryFreeList: Expected length for first %d, but got %d",
+			expectedLength, len(first))
+	}
+	list.Return(first)
+
+	// Borrow again, and check that the underlying array is re-used for second
+	second := list.Borrow()
+	if cap(second) != expectedCapacity {
+		t.Errorf("TestBinaryFreeList: Expected capacity for second %d, but got %d",
+			expectedCapacity, cap(second))
+	}
+	if len(second) != expectedLength {
+		t.Errorf("TestBinaryFreeList: Expected length for second %d, but got %d",
+			expectedLength, len(second))
+	}
+
+	firstArrayAddress := underlyingArrayAddress(first)
+	secondArrayAddress := underlyingArrayAddress(second)
+
+	if firstArrayAddress != secondArrayAddress {
+		t.Errorf("First underlying array is at address %d and second at address %d, "+
+			"which means memory was not re-used", firstArrayAddress, secondArrayAddress)
+	}
+
+	list.Return(second)
+
+	// test there's no crash when channel is full because borrowed too much
+	buffers := make([][]byte, binaryFreeListMaxItems+1)
+	for i := 0; i < binaryFreeListMaxItems+1; i++ {
+		buffers[i] = list.Borrow()
+	}
+	for i := 0; i < binaryFreeListMaxItems+1; i++ {
+		list.Return(buffers[i])
+	}
+}

wire/message_test.go

@@ -60,6 +60,8 @@ func TestMessage(t *testing.T) {
 	msgPing := NewMsgPing(123123)
 	msgPong := NewMsgPong(123123)
 	msgGetHeaders := NewMsgGetHeaders()
+	msgSendHeaders := NewMsgSendHeaders()
+	msgFeeFilter := NewMsgFeeFilter(123456)
 	msgHeaders := NewMsgHeaders()
 	msgAlert := NewMsgAlert([]byte("payload"), []byte("signature"))
 	msgMemPool := NewMsgMemPool()
@@ -97,6 +99,8 @@ func TestMessage(t *testing.T) {
 		{msgPing, msgPing, pver, MainNet, 32},
 		{msgPong, msgPong, pver, MainNet, 32},
 		{msgGetHeaders, msgGetHeaders, pver, MainNet, 61},
+		{msgSendHeaders, msgSendHeaders, pver, MainNet, 24},
+		{msgFeeFilter, msgFeeFilter, pver, MainNet, 32},
 		{msgHeaders, msgHeaders, pver, MainNet, 25},
 		{msgAlert, msgAlert, pver, MainNet, 42},
 		{msgMemPool, msgMemPool, pver, MainNet, 24},

wire/msgtx.go

@@ -365,13 +365,10 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32) error {
 	// returns them.
 	returnScriptBuffers := func() {
 		for _, txIn := range msg.TxIn {
-			if txIn == nil {
+			if txIn == nil || txIn.SignatureScript == nil {
 				continue
 			}
-
+			scriptPool.Return(txIn.SignatureScript)
-			if txIn.SignatureScript != nil {
-				scriptPool.Return(txIn.SignatureScript)
-			}
 		}
 		for _, txOut := range msg.TxOut {
 			if txOut == nil || txOut.PkScript == nil {

wire/msgtx_test.go

@@ -11,6 +11,7 @@ import (
 	"math"
 	"reflect"
 	"testing"
+	"unsafe"
 
 	"github.com/daglabs/btcd/dagconfig/daghash"
 	"github.com/davecgh/go-spew/spew"
@@ -637,6 +638,75 @@ func TestTxSerializeSize(t *testing.T) {
 	}
 }
 
+func TestScriptFreeList(t *testing.T) {
+	var list scriptFreeList = make(chan []byte, freeListMaxItems)
+
+	expectedCapacity := 512
+	expectedLengthFirst := 12
+	expectedLengthSecond := 13
+
+	first := list.Borrow(uint64(expectedLengthFirst))
+	if cap(first) != expectedCapacity {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected capacity for first %d, but got %d",
+			expectedCapacity, cap(first))
+	}
+	if len(first) != expectedLengthFirst {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected length for first %d, but got %d",
+			expectedLengthFirst, len(first))
+	}
+	list.Return(first)
+
+	// Borrow again, and check that the underlying array is re-used for second
+	second := list.Borrow(uint64(expectedLengthSecond))
+	if cap(second) != expectedCapacity {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected capacity for second %d, but got %d",
+			expectedCapacity, cap(second))
+	}
+	if len(second) != expectedLengthSecond {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected length for second %d, but got %d",
+			expectedLengthSecond, len(second))
+	}
+
+	firstArrayAddress := underlyingArrayAddress(first)
+	secondArrayAddress := underlyingArrayAddress(second)
+
+	if firstArrayAddress != secondArrayAddress {
+		t.Errorf("First underlying array is at address %d and second at address %d, "+
+			"which means memory was not re-used", firstArrayAddress, secondArrayAddress)
+	}
+
+	list.Return(second)
+
+	// test for buffers bigger than freeListMaxScriptSize
+	expectedCapacityBig := freeListMaxScriptSize + 1
+	expectedLengthBig := expectedCapacityBig
+	big := list.Borrow(uint64(expectedCapacityBig))
+
+	if cap(big) != expectedCapacityBig {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected capacity for second %d, but got %d",
+			expectedCapacityBig, cap(big))
+	}
+	if len(big) != expectedLengthBig {
+		t.Errorf("MsgTx.TestScriptFreeList: Expected length for second %d, but got %d",
+			expectedLengthBig, len(big))
+	}
+
+	list.Return(big)
+
+	// test there's no crash when channel is full because borrowed too much
+	buffers := make([][]byte, freeListMaxItems+1)
+	for i := 0; i < freeListMaxItems+1; i++ {
+		buffers[i] = list.Borrow(1)
+	}
+	for i := 0; i < freeListMaxItems+1; i++ {
+		list.Return(buffers[i])
+	}
+}
+
+func underlyingArrayAddress(buf []byte) uint64 {
+	return uint64((*reflect.SliceHeader)(unsafe.Pointer(&buf)).Data)
+}
+
 // multiTx is a MsgTx with an input and output and used in various tests.
 var multiTx = &MsgTx{
 	Version: 1,