test for invalid rlp encoding of block in BlocksMsg

- rename Validate -> ValidateFields not to confure consensus block validation
- add nil transaction and nil uncle header validation
- remove bigint field checks: rlp already decodes *big.Int to big.NewInt(0)
- add test for nil header, nil transaction

3 files changed, 44 insertions, 22 deletions

diff --git a/core/types/block.go b/core/types/block.go
index c04beae5a..a40bac42c 100644
--- a/core/types/block.go
+++ b/core/types/block.go
@@ -148,22 +148,19 @@ func NewBlockWithHeader(header *Header) *Block {
 	return &Block{header: header}
 }
 
-func (self *Block) Validate() error {
+func (self *Block) ValidateFields() error {
 	if self.header == nil {
 		return fmt.Errorf("header is nil")
 	}
-	// check *big.Int fields
-	if self.header.Difficulty == nil {
-		return fmt.Errorf("Difficulty undefined")
-	}
-	if self.header.GasLimit == nil {
-		return fmt.Errorf("GasLimit undefined")
-	}
-	if self.header.GasUsed == nil {
-		return fmt.Errorf("GasUsed undefined")
+	for i, transaction := range self.transactions {
+		if transaction == nil {
+			return fmt.Errorf("transaction %d is nil", i)
+		}
 	}
-	if self.header.Number == nil {
-		return fmt.Errorf("Number undefined")
+	for i, uncle := range self.uncles {
+		if uncle == nil {
+			return fmt.Errorf("uncle %d is nil", i)
+		}
 	}
 	return nil
 }
@@ -253,10 +250,10 @@ func (self *Block) AddReceipt(receipt *Receipt) {
 }
 
 func (self *Block) RlpData() interface{} {
-	return []interface{}{self.header, self.transactions, self.uncles}
-}
+	// 	return []interface{}{self.header, self.transactions, self.uncles}
+	// }
 
-func (self *Block) RlpDataForStorage() interface{} {
+	// func (self *Block) RlpDataForStorage() interface{} {
 	return []interface{}{self.header, self.transactions, self.uncles, self.Td /* TODO receipts */}
 }
 
diff --git a/eth/protocol.go b/eth/protocol.go
index 0a3f67b62..f0a749d33 100644
--- a/eth/protocol.go
+++ b/eth/protocol.go
@@ -268,6 +268,9 @@ func (self *ethProtocol) handle() error {
 					return self.protoError(ErrDecode, "msg %v: %v", msg, err)
 				}
 			}
+			if err := block.ValidateFields(); err != nil {
+				return self.protoError(ErrDecode, "block validation %v: %v", msg, err)
+			}
 			self.blockPool.AddBlock(&block, self.id)
 		}
 
@@ -276,7 +279,7 @@ func (self *ethProtocol) handle() error {
 		if err := msg.Decode(&request); err != nil {
 			return self.protoError(ErrDecode, "%v: %v", msg, err)
 		}
-		if err := request.Block.Validate(); err != nil {
+		if err := request.Block.ValidateFields(); err != nil {
 			return self.protoError(ErrDecode, "block validation %v: %v", msg, err)
 		}
 		hash := request.Block.Hash()
diff --git a/eth/protocol_test.go b/eth/protocol_test.go
index d5ac21755..6a8eedadd 100644
--- a/eth/protocol_test.go
+++ b/eth/protocol_test.go
@@ -1,7 +1,6 @@
 package eth
 
 import (
-	"fmt"
 	"log"
 	"math/big"
 	"os"
@@ -227,12 +226,11 @@ func TestStatusMsgErrors(t *testing.T) {
 }
 
 func TestNewBlockMsg(t *testing.T) {
-	logInit()
+	// logInit()
 	eth := newEth(t)
 
 	var disconnected bool
 	eth.blockPool.removePeer = func(peerId string) {
-		fmt.Printf("peer <%s> is disconnected\n", peerId)
 		disconnected = true
 	}
 
@@ -293,7 +291,7 @@ func TestNewBlockMsg(t *testing.T) {
 }
 
 func TestBlockMsg(t *testing.T) {
-	logInit()
+	// logInit()
 	eth := newEth(t)
 	blocks := make(chan *types.Block)
 	eth.blockPool.addBlock = func(block *types.Block, peerId string) (err error) {
@@ -303,7 +301,6 @@ func TestBlockMsg(t *testing.T) {
 
 	var disconnected bool
 	eth.blockPool.removePeer = func(peerId string) {
-		fmt.Printf("peer <%s> is disconnected\n", peerId)
 		disconnected = true
 	}
 
@@ -320,7 +317,9 @@ func TestBlockMsg(t *testing.T) {
 	newblock := func(i int64) *types.Block {
 		return types.NewBlock(common.Hash{byte(i)}, common.Address{byte(i)}, common.Hash{byte(i)}, big.NewInt(i), uint64(i), string(i))
 	}
-	go p2p.Send(eth, BlocksMsg, types.Blocks{newblock(0), newblock(1), newblock(2)})
+	b := newblock(0)
+	b.Header().Difficulty = nil // check if nil as *big.Int decodes as 0
+	go p2p.Send(eth, BlocksMsg, types.Blocks{b, newblock(1), newblock(2)})
 	timer := time.After(delay)
 	for i := int64(0); i < 3; i++ {
 		select {
@@ -328,6 +327,9 @@ func TestBlockMsg(t *testing.T) {
 			if (block.ParentHash() != common.Hash{byte(i)}) {
 				t.Errorf("incorrect block %v, expected %v", block.ParentHash(), common.Hash{byte(i)})
 			}
+			if block.Difficulty().Cmp(big.NewInt(i)) != 0 {
+				t.Errorf("incorrect block %v, expected %v", block.Difficulty(), big.NewInt(i))
+			}
 		case <-timer:
 			t.Errorf("no td recorded after %v", delay)
 			return
@@ -336,4 +338,24 @@ func TestBlockMsg(t *testing.T) {
 			return
 		}
 	}
+
+	go p2p.Send(eth, BlocksMsg, []interface{}{[]interface{}{}})
+	eth.checkError(ErrDecode, delay)
+	if !disconnected {
+		t.Errorf("peer not disconnected after error")
+	}
+
+	// test empty transaction
+	eth.reset()
+	go eth.run()
+	eth.handshake(t, true)
+	err = p2p.ExpectMsg(eth, TxMsg, []interface{}{})
+	if err != nil {
+		t.Errorf("transactions expected, got %v", err)
+	}
+	b = newblock(0)
+	b.AddTransaction(nil)
+	go p2p.Send(eth, BlocksMsg, types.Blocks{b})
+	eth.checkError(ErrDecode, delay)
+
 }