core: verify blocks, fix #12

This adds the following verifications:
 * merkleroot check
 * index check
 * timestamp check
 * witnesses verification

VerifyWitnesses is also renamed to verifyTxWitnesses here to not confuse it
with verifyBlockWitnesse and to hide it from external access (no users at the
moment).

pkg/core/block.go

@@ -9,7 +9,6 @@ import (
 	"github.com/CityOfZion/neo-go/pkg/io"
 	"github.com/CityOfZion/neo-go/pkg/util"
 	"github.com/Workiva/go-datastructures/queue"
-	log "github.com/sirupsen/logrus"
 )
 
 // Block represents one block in the chain.
@@ -31,14 +30,18 @@ func (b *Block) Header() *Header {
 	}
 }
 
-// rebuildMerkleRoot rebuild the merkleroot of the block.
-func (b *Block) rebuildMerkleRoot() error {
-	hashes := make([]util.Uint256, len(b.Transactions))
-	for i, tx := range b.Transactions {
+func merkleTreeFromTransactions(txes []*transaction.Transaction) (*crypto.MerkleTree, error) {
+	hashes := make([]util.Uint256, len(txes))
+	for i, tx := range txes {
 		hashes[i] = tx.Hash()
 	}
 
-	merkle, err := crypto.NewMerkleTree(hashes)
+	return crypto.NewMerkleTree(hashes)
+}
+
+// rebuildMerkleRoot rebuild the merkleroot of the block.
+func (b *Block) rebuildMerkleRoot() error {
+	merkle, err := merkleTreeFromTransactions(b.Transactions)
 	if err != nil {
 		return err
 	}
@@ -48,7 +51,7 @@ func (b *Block) rebuildMerkleRoot() error {
 }
 
 // Verify the integrity of the block.
-func (b *Block) Verify(full bool) error {
+func (b *Block) Verify() error {
 	// There has to be some transaction inside.
 	if len(b.Transactions) == 0 {
 		return errors.New("no transactions")
@@ -63,9 +66,12 @@ func (b *Block) Verify(full bool) error {
 			return fmt.Errorf("miner transaction %s is not the first one", tx.Hash().ReverseString())
 		}
 	}
-	// TODO: When full is true, do a full verification.
-	if full {
-		log.Warn("full verification of blocks is not yet implemented")
+	merkle, err := merkleTreeFromTransactions(b.Transactions)
+	if err != nil {
+		return err
+	}
+	if !b.MerkleRoot.Equals(merkle.Root()) {
+		return errors.New("MerkleRoot mismatch")
 	}
 	return nil
 }

pkg/core/block_base.go

@@ -40,8 +40,11 @@ type BlockBase struct {
 	// Script used to validate the block
 	Script *transaction.Witness `json:"script"`
 
-	// hash of this block, created when binary encoded.
+	// Hash of this block, created when binary encoded (double SHA256).
 	hash util.Uint256
+
+	// Hash of the block used to verify it (single SHA256).
+	verificationHash util.Uint256
 }
 
 // Verify verifies the integrity of the BlockBase.
@@ -58,6 +61,16 @@ func (b *BlockBase) Hash() util.Uint256 {
 	return b.hash
 }
 
+// VerificationHash returns the hash of the block used to verify it.
+func (b *BlockBase) VerificationHash() util.Uint256 {
+	if b.verificationHash.Equals(util.Uint256{}) {
+		if b.createHash() != nil {
+			panic("failed to compute hash!")
+		}
+	}
+	return b.verificationHash
+}
+
 // DecodeBinary implements Serializable interface.
 func (b *BlockBase) DecodeBinary(br *io.BinReader) {
 	b.decodeHashableFields(br)
@@ -80,6 +93,16 @@ func (b *BlockBase) EncodeBinary(bw *io.BinWriter) {
 	b.Script.EncodeBinary(bw)
 }
 
+// getHashableData returns serialized hashable data of the block.
+func (b *BlockBase) getHashableData() ([]byte, error) {
+	buf := io.NewBufBinWriter()
+	b.encodeHashableFields(buf.BinWriter)
+	if buf.Err != nil {
+		return nil, buf.Err
+	}
+	return buf.Bytes(), nil
+}
+
 // createHash creates the hash of the block.
 // When calculating the hash value of the block, instead of calculating the entire block,
 // only first seven fields in the block head will be calculated, which are
@@ -87,12 +110,12 @@ func (b *BlockBase) EncodeBinary(bw *io.BinWriter) {
 // Since MerkleRoot already contains the hash value of all transactions,
 // the modification of transaction will influence the hash value of the block.
 func (b *BlockBase) createHash() error {
-	buf := io.NewBufBinWriter()
-	b.encodeHashableFields(buf.BinWriter)
-	if buf.Err != nil {
-		return buf.Err
+	bb, err := b.getHashableData()
+	if err != nil {
+		return err
 	}
-	b.hash = hash.DoubleSha256(buf.Bytes())
+	b.hash = hash.DoubleSha256(bb)
+	b.verificationHash = hash.Sha256(bb)
 
 	return nil
 }

pkg/core/block_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/CityOfZion/neo-go/pkg/core/transaction"
 	"github.com/CityOfZion/neo-go/pkg/crypto"
+	"github.com/CityOfZion/neo-go/pkg/crypto/hash"
 	"github.com/CityOfZion/neo-go/pkg/io"
 	"github.com/stretchr/testify/assert"
 )
@@ -76,30 +77,59 @@ func TestTrimmedBlock(t *testing.T) {
 	}
 }
 
+func newDumbBlock() *Block {
+	return &Block{
+		BlockBase: BlockBase{
+			Version:       0,
+			PrevHash:      hash.Sha256([]byte("a")),
+			MerkleRoot:    hash.Sha256([]byte("b")),
+			Timestamp:     uint32(100500),
+			Index:         1,
+			ConsensusData: 1111,
+			NextConsensus: hash.Hash160([]byte("a")),
+			Script: &transaction.Witness{
+				VerificationScript: []byte{0x51}, // PUSH1
+				InvocationScript:   []byte{0x61}, // NOP
+			},
+		},
+		Transactions: []*transaction.Transaction{
+			{Type: transaction.MinerType},
+			{Type: transaction.IssueType},
+		},
+	}
+}
+
 func TestHashBlockEqualsHashHeader(t *testing.T) {
-	block := newBlock(0)
+	block := newDumbBlock()
+
 	assert.Equal(t, block.Hash(), block.Header().Hash())
 }
 
 func TestBlockVerify(t *testing.T) {
-	block := newBlock(
-		0,
-		newMinerTX(),
-		newIssueTX(),
-	)
-	assert.Nil(t, block.Verify(false))
+	block := newDumbBlock()
+	assert.NotNil(t, block.Verify())
+	assert.Nil(t, block.rebuildMerkleRoot())
+	assert.Nil(t, block.Verify())
 
 	block.Transactions = []*transaction.Transaction{
 		{Type: transaction.IssueType},
 		{Type: transaction.MinerType},
 	}
-	assert.NotNil(t, block.Verify(false))
+	assert.NoError(t, block.rebuildMerkleRoot())
+	assert.NotNil(t, block.Verify())
 
 	block.Transactions = []*transaction.Transaction{
 		{Type: transaction.MinerType},
 		{Type: transaction.MinerType},
 	}
-	assert.NotNil(t, block.Verify(false))
+	assert.NoError(t, block.rebuildMerkleRoot())
+	assert.NotNil(t, block.Verify())
+	block.Transactions = []*transaction.Transaction{
+		{Type: transaction.MinerType},
+		{Type: transaction.IssueType},
+		{Type: transaction.IssueType},
+	}
+	assert.NotNil(t, block.Verify())
 }
 
 func TestBinBlockDecodeEncode(t *testing.T) {

pkg/core/blockchain.go

@@ -206,7 +206,10 @@ func (bc *Blockchain) AddBlock(block *Block) error {
 		return fmt.Errorf("expected block %d, but passed block %d", expectedHeight, block.Index)
 	}
 	if bc.config.VerifyBlocks {
-		err := block.Verify(false)
+		err := block.Verify()
+		if err == nil {
+			err = bc.VerifyBlock(block)
+		}
 		if err != nil {
 			return fmt.Errorf("block %s is invalid: %s", block.Hash().ReverseString(), err)
 		}
@@ -840,6 +843,21 @@ func (bc *Blockchain) GetMemPool() MemPool {
 	return bc.memPool
 }
 
+// VerifyBlock verifies block against its current state.
+func (bc *Blockchain) VerifyBlock(block *Block) error {
+	prevHeader, err := bc.GetHeader(block.PrevHash)
+	if err != nil {
+		return errors.Wrap(err, "unable to get previous header")
+	}
+	if prevHeader.Index+1 != block.Index {
+		return errors.New("previous header index doesn't match")
+	}
+	if prevHeader.Timestamp >= block.Timestamp {
+		return errors.New("block is not newer than the previous one")
+	}
+	return bc.verifyBlockWitnesses(block, prevHeader)
+}
+
 // VerifyTx verifies whether a transaction is bonafide or not. Block parameter
 // is used for easy interop access and can be omitted for transactions that are
 // not yet added into any block.
@@ -870,7 +888,7 @@ func (bc *Blockchain) VerifyTx(t *transaction.Transaction, block *Block) error {
 		}
 	}
 
-	return bc.VerifyWitnesses(t, block)
+	return bc.verifyTxWitnesses(t, block)
 }
 
 func (bc *Blockchain) verifyInputs(t *transaction.Transaction) bool {
@@ -1094,14 +1112,63 @@ func (bc *Blockchain) GetScriptHashesForVerifying(t *transaction.Transaction) ([
 
 }
 
-// VerifyWitnesses verify the scripts (witnesses) that come with a given
+// verifyHashAgainstScript verifies given hash against the given witness.
+func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, checkedHash util.Uint256, interopCtx *interopContext) error {
+	verification := witness.VerificationScript
+
+	if len(verification) == 0 {
+		bb := new(bytes.Buffer)
+		err := vm.EmitAppCall(bb, hash, false)
+		if err != nil {
+			return err
+		}
+		verification = bb.Bytes()
+	} else {
+		if h := witness.ScriptHash(); hash != h {
+			return errors.New("witness hash mismatch")
+		}
+	}
+
+	vm := vm.New(vm.ModeMute)
+	vm.SetCheckedHash(checkedHash.Bytes())
+	vm.SetScriptGetter(func(hash util.Uint160) []byte {
+		cs := bc.GetContractState(hash)
+		if cs == nil {
+			return nil
+		}
+		return cs.Script
+	})
+	vm.RegisterInteropFuncs(interopCtx.getSystemInteropMap())
+	vm.RegisterInteropFuncs(interopCtx.getNeoInteropMap())
+	vm.LoadScript(verification)
+	vm.LoadScript(witness.InvocationScript)
+	vm.Run()
+	if vm.HasFailed() {
+		return errors.Errorf("vm failed to execute the script")
+	}
+	resEl := vm.Estack().Pop()
+	if resEl != nil {
+		res, err := resEl.TryBool()
+		if err != nil {
+			return err
+		}
+		if !res {
+			return errors.Errorf("signature check failed")
+		}
+	} else {
+		return errors.Errorf("no result returned from the script")
+	}
+	return nil
+}
+
+// verifyTxWitnesses verify the scripts (witnesses) that come with a given
 // transaction. It can reorder them by ScriptHash, because that's required to
 // match a slice of script hashes from the Blockchain. Block parameter
 // is used for easy interop access and can be omitted for transactions that are
 // not yet added into any block.
 // Golang implementation of VerifyWitnesses method in C# (https://github.com/neo-project/neo/blob/master/neo/SmartContract/Helper.cs#L87).
 // Unfortunately the IVerifiable interface could not be implemented because we can't move the References method in blockchain.go to the transaction.go file
-func (bc *Blockchain) VerifyWitnesses(t *transaction.Transaction, block *Block) error {
+func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *Block) error {
 	hashes, err := bc.GetScriptHashesForVerifying(t)
 	if err != nil {
 		return err
@@ -1113,57 +1180,30 @@ func (bc *Blockchain) VerifyWitnesses(t *transaction.Transaction, block *Block)
 	}
 	sort.Slice(hashes, func(i, j int) bool { return hashes[i].Less(hashes[j]) })
 	sort.Slice(witnesses, func(i, j int) bool { return witnesses[i].ScriptHash().Less(witnesses[j].ScriptHash()) })
+	interopCtx := newInteropContext(0, bc, block, t)
 	for i := 0; i < len(hashes); i++ {
-		verification := witnesses[i].VerificationScript
-
-		if len(verification) == 0 {
-			bb := new(bytes.Buffer)
-			err = vm.EmitAppCall(bb, hashes[i], false)
-			if err != nil {
-				return err
-			}
-			verification = bb.Bytes()
-		} else {
-			if h := witnesses[i].ScriptHash(); hashes[i] != h {
-				return errors.Errorf("hash mismatch for script #%d", i)
-			}
-		}
-
-		vm := vm.New(vm.ModeMute)
-		vm.SetCheckedHash(t.VerificationHash().Bytes())
-		vm.SetScriptGetter(func(hash util.Uint160) []byte {
-			cs := bc.GetContractState(hash)
-			if cs == nil {
-				return nil
-			}
-			return cs.Script
-		})
-		systemInterop := newInteropContext(0, bc, block, t)
-		vm.RegisterInteropFuncs(systemInterop.getSystemInteropMap())
-		vm.RegisterInteropFuncs(systemInterop.getNeoInteropMap())
-		vm.LoadScript(verification)
-		vm.LoadScript(witnesses[i].InvocationScript)
-		vm.Run()
-		if vm.HasFailed() {
-			return errors.Errorf("vm failed to execute the script")
-		}
-		resEl := vm.Estack().Pop()
-		if resEl != nil {
-			res, err := resEl.TryBool()
-			if err != nil {
-				return err
-			}
-			if !res {
-				return errors.Errorf("signature check failed")
-			}
-		} else {
-			return errors.Errorf("no result returned from the script")
+		err := bc.verifyHashAgainstScript(hashes[i], witnesses[i], t.VerificationHash(), interopCtx)
+		if err != nil {
+			numStr := fmt.Sprintf("witness #%d", i)
+			return errors.Wrap(err, numStr)
 		}
 	}
 
 	return nil
 }
 
+// verifyBlockWitnesses is a block-specific implementation of VerifyWitnesses logic.
+func (bc *Blockchain) verifyBlockWitnesses(block *Block, prevHeader *Header) error {
+	var hash util.Uint160
+	if prevHeader == nil && block.PrevHash.Equals(util.Uint256{}) {
+		hash = block.Script.ScriptHash()
+	} else {
+		hash = prevHeader.NextConsensus
+	}
+	interopCtx := newInteropContext(0, bc, nil, nil)
+	return bc.verifyHashAgainstScript(hash, block.Script, block.VerificationHash(), interopCtx)
+}
+
 func hashAndIndexToBytes(h util.Uint256, index uint32) []byte {
 	buf := io.NewBufBinWriter()
 	buf.WriteLE(h.BytesReverse())

pkg/core/blockchain_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/CityOfZion/neo-go/config"
 	"github.com/CityOfZion/neo-go/pkg/core/storage"
 	"github.com/CityOfZion/neo-go/pkg/io"
 	"github.com/stretchr/testify/assert"
@@ -137,8 +136,9 @@ func TestGetTransaction(t *testing.T) {
 	block := getDecodedBlock(t, 2)
 	bc := newTestChain(t)
 
-	assert.Nil(t, bc.AddBlock(b1))
-	assert.Nil(t, bc.AddBlock(block))
+	// These are from some kind of different chain, so can't be added via AddBlock().
+	assert.Nil(t, bc.storeBlock(b1))
+	assert.Nil(t, bc.storeBlock(block))
 
 	// Test unpersisted and persisted access
 	for j := 0; j < 2; j++ {
@@ -154,16 +154,3 @@ func TestGetTransaction(t *testing.T) {
 		assert.NoError(t, bc.persist(context.Background()))
 	}
 }
-
-func newTestChain(t *testing.T) *Blockchain {
-	cfg, err := config.Load("../../config", config.ModeUnitTestNet)
-	if err != nil {
-		t.Fatal(err)
-	}
-	chain, err := NewBlockchain(storage.NewMemoryStore(), cfg.ProtocolConfiguration)
-	if err != nil {
-		t.Fatal(err)
-	}
-	go chain.Run(context.Background())
-	return chain
-}

pkg/core/helper_test.go

@@ -1,6 +1,7 @@
 package core
 
 import (
+	"context"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
@@ -8,32 +9,90 @@ import (
 	"testing"
 	"time"
 
+	"github.com/CityOfZion/neo-go/config"
+	"github.com/CityOfZion/neo-go/pkg/core/storage"
 	"github.com/CityOfZion/neo-go/pkg/core/transaction"
-	"github.com/CityOfZion/neo-go/pkg/crypto/hash"
+	"github.com/CityOfZion/neo-go/pkg/crypto/keys"
 	"github.com/CityOfZion/neo-go/pkg/io"
+	"github.com/CityOfZion/neo-go/pkg/smartcontract"
 	"github.com/CityOfZion/neo-go/pkg/util"
+	"github.com/stretchr/testify/require"
 )
 
+var newBlockPrevHash util.Uint256
+var unitTestNetCfg config.Config
+
+var privNetKeys = []string{
+	"KxyjQ8eUa4FHt3Gvioyt1Wz29cTUrE4eTqX3yFSk1YFCsPL8uNsY",
+	"KzfPUYDC9n2yf4fK5ro4C8KMcdeXtFuEnStycbZgX3GomiUsvX6W",
+	"KzgWE3u3EDp13XPXXuTKZxeJ3Gi8Bsm8f9ijY3ZsCKKRvZUo1Cdn",
+	"L2oEXKRAAMiPEZukwR5ho2S6SMeQLhcK9mF71ZnF7GvT8dU4Kkgz",
+}
+
+// newTestChain should be called before newBlock invocation to properly setup
+// global state.
+func newTestChain(t *testing.T) *Blockchain {
+	var err error
+	unitTestNetCfg, err = config.Load("../../config", config.ModeUnitTestNet)
+	if err != nil {
+		t.Fatal(err)
+	}
+	chain, err := NewBlockchain(storage.NewMemoryStore(), unitTestNetCfg.ProtocolConfiguration)
+	if err != nil {
+		t.Fatal(err)
+	}
+	go chain.Run(context.Background())
+	zeroHash, err := chain.GetHeader(chain.GetHeaderHash(0))
+	require.Nil(t, err)
+	newBlockPrevHash = zeroHash.Hash()
+	return chain
+}
+
 func newBlock(index uint32, txs ...*transaction.Transaction) *Block {
+	validators, _ := getValidators(unitTestNetCfg.ProtocolConfiguration)
+	vlen := len(validators)
+	valScript, _ := smartcontract.CreateMultiSigRedeemScript(
+		vlen-(vlen-1)/3,
+		validators,
+	)
+	witness := &transaction.Witness{
+		VerificationScript: valScript,
+	}
 	b := &Block{
 		BlockBase: BlockBase{
 			Version:       0,
-			PrevHash:      hash.Sha256([]byte("a")),
-			MerkleRoot:    hash.Sha256([]byte("b")),
-			Timestamp:     uint32(time.Now().UTC().Unix()),
+			PrevHash:      newBlockPrevHash,
+			Timestamp:     uint32(time.Now().UTC().Unix()) + index,
 			Index:         index,
 			ConsensusData: 1111,
-			NextConsensus: util.Uint160{},
-			Script: &transaction.Witness{
-				VerificationScript: []byte{0x0},
-				InvocationScript:   []byte{0x1},
-			},
+			NextConsensus: witness.ScriptHash(),
+			Script:        witness,
 		},
 		Transactions: txs,
 	}
-
+	_ = b.rebuildMerkleRoot()
 	b.createHash()
+	newBlockPrevHash = b.Hash()
 
+	invScript := make([]byte, 0)
+	for _, wif := range privNetKeys {
+		pKey, err := keys.NewPrivateKeyFromWIF(wif)
+		if err != nil {
+			panic(err)
+		}
+		b, err := b.getHashableData()
+		if err != nil {
+			panic(err)
+		}
+		sig, err := pKey.Sign(b)
+		if err != nil || len(sig) != 64 {
+			panic(err)
+		}
+		// 0x40 is PUSHBYTES64
+		invScript = append(invScript, 0x40)
+		invScript = append(invScript, sig...)
+	}
+	b.Script.InvocationScript = invScript
 	return b
 }
 
@@ -52,13 +111,6 @@ func newMinerTX() *transaction.Transaction {
 	}
 }
 
-func newIssueTX() *transaction.Transaction {
-	return &transaction.Transaction{
-		Type: transaction.IssueType,
-		Data: &transaction.IssueTX{},
-	}
-}
-
 func getDecodedBlock(t *testing.T, i int) *Block {
 	data, err := getBlockData(i)
 	if err != nil {

pkg/rpc/server_helper_test.go

@@ -3,18 +3,16 @@ package rpc
 import (
 	"context"
 	"net/http"
+	"os"
 	"testing"
-	"time"
 
 	"github.com/CityOfZion/neo-go/config"
 	"github.com/CityOfZion/neo-go/pkg/core"
 	"github.com/CityOfZion/neo-go/pkg/core/storage"
-	"github.com/CityOfZion/neo-go/pkg/core/transaction"
-	"github.com/CityOfZion/neo-go/pkg/crypto/hash"
+	"github.com/CityOfZion/neo-go/pkg/io"
 	"github.com/CityOfZion/neo-go/pkg/network"
 	"github.com/CityOfZion/neo-go/pkg/rpc/result"
 	"github.com/CityOfZion/neo-go/pkg/rpc/wrappers"
-	"github.com/CityOfZion/neo-go/pkg/util"
 	"github.com/stretchr/testify/require"
 )
 
@@ -142,6 +140,8 @@ type GetAccountStateResponse struct {
 }
 
 func initServerWithInMemoryChain(ctx context.Context, t *testing.T) (*core.Blockchain, http.HandlerFunc) {
+	var nBlocks uint32
+
 	net := config.ModeUnitTestNet
 	configPath := "../../config"
 	cfg, err := config.Load(configPath, net)
@@ -152,7 +152,18 @@ func initServerWithInMemoryChain(ctx context.Context, t *testing.T) (*core.Block
 	require.NoError(t, err, "could not create chain")
 
 	go chain.Run(ctx)
-	initBlocks(t, chain)
+
+	f, err := os.Open("testdata/50testblocks.acc")
+	require.Nil(t, err)
+	br := io.NewBinReaderFromIO(f)
+	br.ReadLE(&nBlocks)
+	require.Nil(t, br.Err)
+	for i := 0; i < int(nBlocks); i++ {
+		block := &core.Block{}
+		block.DecodeBinary(br)
+		require.Nil(t, br.Err)
+		require.NoError(t, chain.AddBlock(block))
+	}
 
 	serverConfig := network.NewServerConfig(cfg)
 	server := network.NewServer(serverConfig, chain)
@@ -161,45 +172,3 @@ func initServerWithInMemoryChain(ctx context.Context, t *testing.T) (*core.Block
 
 	return chain, handler
 }
-
-func initBlocks(t *testing.T, chain *core.Blockchain) {
-	blocks := makeBlocks(10)
-	for i := 0; i < len(blocks); i++ {
-		require.NoError(t, chain.AddBlock(blocks[i]))
-	}
-}
-
-func makeBlocks(n int) []*core.Block {
-	blocks := make([]*core.Block, n)
-	for i := 0; i < n; i++ {
-		blocks[i] = newBlock(uint32(i+1), newMinerTX())
-	}
-	return blocks
-}
-
-func newMinerTX() *transaction.Transaction {
-	return &transaction.Transaction{
-		Type: transaction.MinerType,
-		Data: &transaction.MinerTX{},
-	}
-}
-
-func newBlock(index uint32, txs ...*transaction.Transaction) *core.Block {
-	b := &core.Block{
-		BlockBase: core.BlockBase{
-			Version:       0,
-			PrevHash:      hash.Sha256([]byte("a")),
-			MerkleRoot:    hash.Sha256([]byte("b")),
-			Timestamp:     uint32(time.Now().UTC().Unix()),
-			Index:         index,
-			ConsensusData: 1111,
-			NextConsensus: util.Uint160{},
-			Script: &transaction.Witness{
-				VerificationScript: []byte{0x0},
-				InvocationScript:   []byte{0x1},
-			},
-		},
-		Transactions: txs,
-	}
-	return b
-}