Improve tests after the refactoring

blockchain/mod.go

@@ -4,7 +4,6 @@ import (
 	"context"
 
 	"github.com/golang/protobuf/proto"
-	"go.dedis.ch/fabric/crypto"
 	"go.dedis.ch/fabric/encoding"
 	mino "go.dedis.ch/fabric/mino"
 )
@@ -27,8 +26,6 @@ type Block interface {
 // verified from the genesis block.
 type VerifiableBlock interface {
 	Block
-
-	Verify(crypto.Verifier) error
 }
 
 // BlockFactory provides primitives to create blocks from a untrusted source.

blockchain/skipchain/block.go

@@ -2,10 +2,8 @@ package skipchain
 
 import (
 	"bytes"
-	"crypto/sha256"
 	"encoding/binary"
 	fmt "fmt"
-	"hash"
 
 	proto "github.com/golang/protobuf/proto"
 	"go.dedis.ch/fabric/blockchain"
@@ -28,17 +26,6 @@ func (d Digest) String() string {
 	return fmt.Sprintf("%x", d[:])[:16]
 }
 
-// sha256Factory is a factory for SHA256 digests.
-//
-// - implements crypto.HashFactory
-type sha256Factory struct{}
-
-// New implements crypto.HashFactory. It returns a new instance of a SHA256
-// hash.
-func (f sha256Factory) New() hash.Hash {
-	return sha256.New()
-}
-
 // SkipBlock is a representation of the data held by a block. It contains the
 // information to build a skipchain.
 //
@@ -152,16 +139,6 @@ type VerifiableBlock struct {
 	Chain consensus.Chain
 }
 
-// Verify implements blockchain.VerifiableBlock. It makes sure the integrity of
-// the chain is valid.
-func (vb VerifiableBlock) Verify(v crypto.Verifier) error {
-	if !bytes.Equal(vb.GetHash(), vb.Chain.GetLastHash()) {
-		return xerrors.Errorf("mismatch target %x != %x", vb.GetHash(), vb.Chain.GetLastHash())
-	}
-
-	return nil
-}
-
 // Pack implements encoding.Packable. It returns the protobuf message for a
 // verifiable block.
 func (vb VerifiableBlock) Pack(enc encoding.ProtoMarshaler) (proto.Message, error) {
@@ -187,9 +164,9 @@ func (vb VerifiableBlock) Pack(enc encoding.ProtoMarshaler) (proto.Message, erro
 //
 // - implements blockchain.BlockFactory
 type blockFactory struct {
-	*Skipchain
 	encoder     encoding.ProtoMarshaler
 	hashFactory crypto.HashFactory
+	consensus   consensus.Consensus
 }
 
 func (f blockFactory) prepareBlock(block *SkipBlock) error {
@@ -273,22 +250,18 @@ func (f blockFactory) FromVerifiable(src proto.Message) (blockchain.Block, error
 
 	chainFactory, err := f.consensus.GetChainFactory()
 	if err != nil {
-		return nil, err
+		return nil, xerrors.Errorf("couldn't get the chain factory: %v", err)
 	}
 
+	// Integrity of the chain is verifier during decoding.
 	chain, err := chainFactory.FromProto(in.GetChain())
 	if err != nil {
 		return nil, xerrors.Errorf("couldn't decode the chain: %v", err)
 	}
 
-	vb := VerifiableBlock{
-		SkipBlock: block,
-		Chain:     chain,
-	}
-
-	err = vb.Verify(nil)
-	if err != nil {
-		return nil, xerrors.Errorf("couldn't verify: %v", err)
+	// Only the link between the chain and the block needs to be verified.
+	if !bytes.Equal(chain.GetLastHash(), block.hash[:]) {
+		return nil, xerrors.Errorf("mismatch hashes: %#x != %#x", chain.GetLastHash(), block.hash)
 	}
 
 	return block, nil

blockchain/skipchain/block_test.go

@@ -119,7 +119,7 @@ func TestSkipBlock_HashUniqueness(t *testing.T) {
 
 	enc := encoding.NewProtoEncoder()
 
-	prevHash, err := block.computeHash(sha256Factory{}, enc)
+	prevHash, err := block.computeHash(crypto.NewSha256Factory(), enc)
 	require.NoError(t, err)
 
 	value := reflect.ValueOf(&block)
@@ -137,7 +137,7 @@ func TestSkipBlock_HashUniqueness(t *testing.T) {
 		field.Set(reflect.Zero(value.Elem().Field(i).Type()))
 		newBlock := value.Interface()
 
-		hash, err := newBlock.(*SkipBlock).computeHash(sha256Factory{}, enc)
+		hash, err := newBlock.(*SkipBlock).computeHash(crypto.NewSha256Factory(), enc)
 		require.NoError(t, err)
 
 		errMsg := fmt.Sprintf("field %#v produced same hash", fieldName)
@@ -180,7 +180,7 @@ func TestBlockFactory_FromPrevious(t *testing.T) {
 	f := func(prev SkipBlock) bool {
 		factory := blockFactory{
 			encoder:     encoding.NewProtoEncoder(),
-			hashFactory: sha256Factory{},
+			hashFactory: crypto.NewSha256Factory(),
 		}
 
 		block, err := factory.fromPrevious(prev, &empty.Empty{})
@@ -205,10 +205,7 @@ func TestBlockFactory_DecodeBlock(t *testing.T) {
 	f := func(block SkipBlock) bool {
 		factory := blockFactory{
 			encoder:     encoding.NewProtoEncoder(),
-			hashFactory: sha256Factory{},
-			Skipchain: &Skipchain{
-				mino: fake.Mino{},
-			},
+			hashFactory: crypto.NewSha256Factory(),
 		}
 
 		packed, err := block.Pack(encoding.NewProtoEncoder())
@@ -236,11 +233,8 @@ func TestBlockFactory_FromVerifiable(t *testing.T) {
 	f := func(block SkipBlock) bool {
 		factory := blockFactory{
 			encoder:     encoding.NewProtoEncoder(),
-			hashFactory: sha256Factory{},
-			Skipchain: &Skipchain{
-				mino:      fake.Mino{},
-				consensus: fakeConsensus{hash: block.hash},
-			},
+			hashFactory: crypto.NewSha256Factory(),
+			consensus:   fakeConsensus{hash: block.hash},
 		}
 
 		packed, err := block.Pack(encoding.NewProtoEncoder())
@@ -263,11 +257,20 @@ func TestBlockFactory_FromVerifiable(t *testing.T) {
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "couldn't decode the block: ")
 
-		factory.hashFactory = sha256Factory{}
+		factory.hashFactory = crypto.NewSha256Factory()
+		factory.consensus = fakeConsensus{errFactory: xerrors.New("oops")}
+		_, err = factory.FromVerifiable(pb)
+		require.EqualError(t, err, "couldn't get the chain factory: oops")
+
 		factory.consensus = fakeConsensus{err: xerrors.New("oops")}
 		_, err = factory.FromVerifiable(pb)
 		require.EqualError(t, err, "couldn't decode the chain: oops")
 
+		factory.consensus = fakeConsensus{hash: Digest{}}
+		_, err = factory.FromVerifiable(pb)
+		require.EqualError(t, err,
+			fmt.Sprintf("mismatch hashes: %#x != %#x", [32]byte{}, block.hash))
+
 		return true
 	}
 
@@ -301,7 +304,7 @@ func (s SkipBlock) Generate(rand *rand.Rand, size int) reflect.Value {
 		Payload:   &empty.Empty{},
 	}
 
-	hash, _ := block.computeHash(sha256Factory{}, encoding.NewProtoEncoder())
+	hash, _ := block.computeHash(crypto.NewSha256Factory(), encoding.NewProtoEncoder())
 	block.hash = hash
 
 	return reflect.ValueOf(block)
@@ -338,17 +341,18 @@ func (f fakeChainFactory) FromProto(proto.Message) (consensus.Chain, error) {
 
 type fakeConsensus struct {
 	consensus.Consensus
-	hash     Digest
-	err      error
-	errChain error
+	hash       Digest
+	err        error
+	errChain   error
+	errFactory error
 }
 
 func (c fakeConsensus) GetChainFactory() (consensus.ChainFactory, error) {
 	return fakeChainFactory{
 		hash:     c.hash,
 		err:      c.err,
 		errChain: c.errChain,
-	}, nil
+	}, c.errFactory
 }
 
 func (c fakeConsensus) GetChain(id []byte) (consensus.Chain, error) {

blockchain/skipchain/handler.go

@@ -31,9 +31,7 @@ func newHandler(sc *Skipchain, proc blockchain.PayloadProcessor) handler {
 func (h handler) Process(req mino.Request) (proto.Message, error) {
 	switch in := req.Message.(type) {
 	case *PropagateGenesis:
-		factory := h.GetBlockFactory().(blockFactory)
-
-		genesis, err := factory.decodeBlock(in.GetGenesis())
+		genesis, err := h.blockFactory.decodeBlock(in.GetGenesis())
 		if err != nil {
 			return nil, xerrors.Errorf("couldn't decode the block: %v", err)
 		}

blockchain/skipchain/handler_test.go

@@ -8,8 +8,8 @@ import (
 	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/stretchr/testify/require"
+	"go.dedis.ch/fabric/crypto"
 	"go.dedis.ch/fabric/encoding"
-	"go.dedis.ch/fabric/internal/testing/fake"
 	"go.dedis.ch/fabric/mino"
 	"golang.org/x/xerrors"
 )
@@ -19,11 +19,12 @@ func TestHandler_Process(t *testing.T) {
 		proc := &fakePayloadProc{}
 		watcher := &fakeWatcher{}
 		h := newHandler(&Skipchain{
-			encoder:   encoding.NewProtoEncoder(),
-			db:        &fakeDatabase{},
-			mino:      fake.Mino{},
-			consensus: fakeConsensus{},
-			watcher:   watcher,
+			blockFactory: blockFactory{
+				encoder:     encoding.NewProtoEncoder(),
+				hashFactory: crypto.NewSha256Factory(),
+			},
+			db:      &fakeDatabase{},
+			watcher: watcher,
 		}, proc)
 
 		block.Payload = &wrappers.BoolValue{Value: true}

blockchain/skipchain/mod.go

@@ -7,7 +7,6 @@ package skipchain
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -35,35 +34,41 @@ const (
 // - implements blockchain.Blockchain
 // - implements fmt.Stringer
 type Skipchain struct {
-	logger    zerolog.Logger
-	mino      mino.Mino
-	db        Database
-	consensus consensus.Consensus
-	watcher   blockchain.Observable
-	encoder   encoding.ProtoMarshaler
+	logger       zerolog.Logger
+	mino         mino.Mino
+	db           Database
+	consensus    consensus.Consensus
+	watcher      blockchain.Observable
+	encoder      encoding.ProtoMarshaler
+	blockFactory blockFactory
 }
 
 // NewSkipchain returns a new instance of Skipchain.
 func NewSkipchain(m mino.Mino, consensus consensus.Consensus) *Skipchain {
 	db := NewInMemoryDatabase()
+	encoder := encoding.NewProtoEncoder()
 
 	return &Skipchain{
 		logger:    fabric.Logger,
 		mino:      m,
 		db:        db,
 		consensus: consensus,
 		watcher:   blockchain.NewWatcher(),
-		encoder:   encoding.NewProtoEncoder(),
+		encoder:   encoder,
+		blockFactory: blockFactory{
+			encoder:     encoder,
+			consensus:   consensus,
+			hashFactory: crypto.NewSha256Factory(),
+		},
 	}
 }
 
 // Listen implements blockchain.Blockchain. It registers the RPC and starts the
 // consensus module.
 func (s *Skipchain) Listen(proc blockchain.PayloadProcessor) (blockchain.Actor, error) {
 	actor := skipchainActor{
-		Skipchain:   s,
-		hashFactory: sha256Factory{},
-		rand:        crypto.CryptographicRandomGenerator{},
+		Skipchain: s,
+		rand:      crypto.CryptographicRandomGenerator{},
 	}
 
 	var err error
@@ -83,11 +88,7 @@ func (s *Skipchain) Listen(proc blockchain.PayloadProcessor) (blockchain.Actor,
 // GetBlockFactory implements blockchain.Blockchain. It returns the block
 // factory for skipchains.
 func (s *Skipchain) GetBlockFactory() blockchain.BlockFactory {
-	return blockFactory{
-		Skipchain:   s,
-		encoder:     s.encoder,
-		hashFactory: sha256Factory{},
-	}
+	return s.blockFactory
 }
 
 // GetBlock implements blockchain.Blockchain. It returns the latest block.
@@ -142,21 +143,14 @@ func (s *Skipchain) Watch(ctx context.Context) <-chan blockchain.Block {
 	return ch
 }
 
-// String implements fmt.Stringer. It returns a simple representation of the
-// skipchain instance to easily identify it.
-func (s *Skipchain) String() string {
-	return fmt.Sprintf("skipchain@%v", s.mino.GetAddress())
-}
-
 // skipchainActor provides the primitives of a blockchain actor.
 //
 // - implements blockchain.Actor
 type skipchainActor struct {
 	*Skipchain
-	hashFactory crypto.HashFactory
-	rand        crypto.RandGenerator
-	consensus   consensus.Actor
-	rpc         mino.RPC
+	rand      crypto.RandGenerator
+	consensus consensus.Actor
+	rpc       mino.RPC
 }
 
 // InitChain implements blockchain.Actor. It creates a genesis block if none
@@ -205,7 +199,7 @@ func (a skipchainActor) newChain(data proto.Message, conodes mino.Players) error
 		Payload:   data,
 	}
 
-	err = a.GetBlockFactory().(blockFactory).prepareBlock(&genesis)
+	err = a.blockFactory.prepareBlock(&genesis)
 	if err != nil {
 		return xerrors.Errorf("couldn't create block: %v", err)
 	}
@@ -234,14 +228,12 @@ func (a skipchainActor) newChain(data proto.Message, conodes mino.Players) error
 // Store implements blockchain.Actor. It will append a new block to chain filled
 // with the data.
 func (a skipchainActor) Store(data proto.Message, players mino.Players) error {
-	factory := a.GetBlockFactory().(blockFactory)
-
 	previous, err := a.db.ReadLast()
 	if err != nil {
 		return xerrors.Errorf("couldn't read the latest block: %v", err)
 	}
 
-	block, err := factory.fromPrevious(previous, data)
+	block, err := a.blockFactory.fromPrevious(previous, data)
 	if err != nil {
 		return xerrors.Errorf("couldn't create next block: %v", err)
 	}