ValidatorBlock() do more checks (ethereum#44)

* ValidatorBlock() do more checks

* direct compare nonce

Co-authored-by: Qi Zhou <qzhou64@gmail.com>

consensus/tendermint/adapter/store.go

@@ -2,6 +2,7 @@ package adapter
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	pbft "github.com/QuarkChain/go-minimal-pbft/consensus"
@@ -78,12 +79,77 @@ func (s *Store) SaveBlock(block *types.FullBlock, commit *types.Commit) {
 	s.mux.Post(core.NewMinedBlockEvent{Block: block.WithCommit(commit).Block})
 }
 
+// Validate a block without Commit and with LastCommit.
 func (s *Store) ValidateBlock(state pbft.ChainState, block *types.FullBlock) (err error) {
 	err = s.verifyHeaderFunc(s.chain, block.Header(), false)
 	if err != nil {
 		return
 	}
 
+	// Validate if the block matches current state.
+	if state.LastBlockHeight == 0 && block.NumberU64() != state.InitialHeight {
+		return fmt.Errorf("wrong Block.Header.Height. Expected %v for initial block, got %v",
+			block.NumberU64(), state.InitialHeight)
+	}
+	if state.LastBlockHeight > 0 && block.NumberU64() != state.LastBlockHeight+1 {
+		return fmt.Errorf("wrong Block.Header.Height. Expected %v, got %v",
+			state.LastBlockHeight+1,
+			block.NumberU64(),
+		)
+	}
+	// Validate prev block info.
+	if block.ParentHash() != state.LastBlockID {
+		return fmt.Errorf("wrong Block.Header.LastBlockID.  Expected %v, got %v",
+			state.LastBlockID,
+			block.ParentHash(),
+		)
+	}
+
+	// Validate basic info without Commit.
+	// Validate block LastCommit.
+	if block.NumberU64() == state.InitialHeight {
+		if len(block.LastCommit.Signatures) != 0 {
+			return errors.New("initial block can't have LastCommit signatures")
+		}
+	} else {
+		// LastCommit.Signatures length is checked in VerifyCommit.
+		if err := state.LastValidators.VerifyCommit(
+			state.ChainID, state.LastBlockID, block.NumberU64()-1, block.LastCommit); err != nil {
+			return err
+		}
+	}
+
+	// Validate block Time with LastCommit
+	switch {
+	case block.NumberU64() > state.InitialHeight:
+		if block.TimeMs() <= state.LastBlockTime {
+			return fmt.Errorf("block time %v not greater than last block time %v",
+				block.TimeMs(),
+				state.LastBlockTime,
+			)
+		}
+		medianTime := pbft.MedianTime(block.LastCommit, state.LastValidators)
+		if block.TimeMs() != medianTime {
+			return fmt.Errorf("invalid block time. Expected %v, got %v",
+				medianTime,
+				block.TimeMs(),
+			)
+		}
+
+	case block.NumberU64() == state.InitialHeight:
+		genesisTime := state.LastBlockTime + 1000
+		if block.TimeMs() != genesisTime {
+			return fmt.Errorf("block time %v is not equal to genesis time %v",
+				block.TimeMs(),
+				genesisTime,
+			)
+		}
+
+	default:
+		return fmt.Errorf("block height %v lower than initial height %v",
+			block.NumberU64(), state.InitialHeight)
+	}
+
 	err = s.chain.PreExecuteBlock(block.Block)
 	return
 }
@@ -163,22 +229,36 @@ func updateState(
 	}, nil
 }
 
-func (s *Store) MakeBlock(state *pbft.ChainState, height uint64,
+func (s *Store) MakeBlock(
+	state *pbft.ChainState,
+	height uint64,
 	commit *pbft.Commit,
-	proposerAddress common.Address) *types.FullBlock {
+	proposerAddress common.Address,
+) *types.FullBlock {
 
 	// Set time.
-	var timestamp uint64
+	var timestampMs uint64
 	if height == state.InitialHeight {
-		timestamp = state.LastBlockTime + 1 // genesis time + 1
+		timestampMs = state.LastBlockTime + 1000 // genesis time + 1sec
 	} else {
-		timestamp = pbft.MedianTime(commit, state.LastValidators)
+		timestampMs = pbft.MedianTime(commit, state.LastValidators)
 	}
+	var timestamp = timestampMs / 1000
 
 	block, err := s.makeBlock(state.LastBlockID, proposerAddress, timestamp)
 	if err != nil {
-		panic("failed to make a block")
+		log.Crit("failed to make a block", "err", err)
+	}
+
+	// Make a copy of header, and setup TM-related fields
+	header := block.Header()
+	if header.Time != timestamp {
+		log.Crit("make block does not setup header.Time correctly")
 	}
+	header.TimeMs = timestampMs
+	header.LastCommitHash = commit.Hash()
+
+	block = block.WithSeal(header)
 
 	return &types.FullBlock{Block: block, LastCommit: commit}
 }

consensus/tendermint/gov/gov.go

@@ -54,20 +54,6 @@ func (g *Governance) GetValidatorSet(height uint64, lastVals *types.ValidatorSet
 	return epochVals
 }
 
-// EpochValidators returns the current epoch validators that height belongs to
-func (g *Governance) EpochValidators(height uint64) []common.Address {
-	// TODO: get real validators by calling contract
-	header := g.chain.GetHeaderByNumber(0)
-	return header.NextValidators
-}
-
-// EpochValidatorPowers returns the current epoch validator powers that height belongs to
-func (g *Governance) EpochValidatorPowers(height uint64) []uint64 {
-	// TODO: get real validators by calling contract
-	header := g.chain.GetHeaderByNumber(0)
-	return header.NextValidatorPowers
-}
-
 func (g *Governance) NextValidators(height uint64) []common.Address {
 	if height%g.config.Epoch != 0 {
 		return []common.Address{}

consensus/tendermint/tendermint.go

@@ -18,7 +18,6 @@
 package tendermint
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -32,7 +31,6 @@ import (
 	libp2p "github.com/QuarkChain/go-minimal-pbft/p2p"
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/consensus/misc"
 	"github.com/ethereum/go-ethereum/consensus/tendermint/adapter"
@@ -49,11 +47,11 @@ import (
 	"github.com/libp2p/go-libp2p-core/peer"
 )
 
-// Clique proof-of-authority protocol constants.
+// Tendermint proof-of-authority/stake BFT protocol constants.
 var (
 	epochLength = uint64(30000) // Default number of blocks after which to checkpoint and reset the pending votes
 
-	nonceDefault = hexutil.MustDecode("0x0000000000000000") // Magic nonce number to vote on removing a signer.
+	nonceDefault = types.BlockNonce{} // Default nonce number.
 
 	uncleHash = types.CalcUncleHash(nil) // Always Keccak256(RLP([])) as uncles are meaningless outside of PoW.
 
@@ -342,7 +340,7 @@ func (c *Tendermint) verifyHeader(chain consensus.ChainHeaderReader, header *typ
 	if len(header.NextValidatorPowers) != len(header.NextValidators) {
 		return errors.New("NextValidators must have the same len as powers")
 	}
-	if !bytes.Equal(header.Nonce[:], nonceDefault) {
+	if header.Nonce != nonceDefault {
 		return errors.New("invalid nonce")
 	}
 	// Ensure that the mix digest is zero as we don't have fork protection currently
@@ -363,6 +361,30 @@ func (c *Tendermint) verifyHeader(chain consensus.ChainHeaderReader, header *typ
 	if header.GasLimit > params.MaxGasLimit {
 		return fmt.Errorf("invalid gasLimit: have %v, max %v", header.GasLimit, params.MaxGasLimit)
 	}
+
+	// Check if TimeMs matches Time.
+	// Note that we will not check acutal Time since we don't have LastCommit.
+	if header.TimeMs/1000 != header.Time {
+		return fmt.Errorf("inccorect timestamp")
+	}
+
+	epochHeader := c.getEpochHeader(chain, header)
+	if epochHeader == nil {
+		return fmt.Errorf("epochHeader not found, height:%d", number)
+	}
+
+	vs := types.NewValidatorSet(epochHeader.NextValidators, types.U64ToI64Array(epochHeader.NextValidatorPowers), int64(c.config.ProposerRepetition))
+
+	// NOTE: We can't actually verify it's the right proposer because we don't
+	// know what round the block was first proposed. So just check that it's
+	// a legit address and a known validator.
+	// The length is checked in ValidateBasic above.
+	if !vs.HasAddress(header.Coinbase) {
+		return fmt.Errorf("block.Header.ProposerAddress %X is not a validator",
+			header.Coinbase,
+		)
+	}
+
 	// If all checks passed, validate any special fields for hard forks
 	if err := misc.VerifyForkHashes(chain.Config(), header, false); err != nil {
 		return err
@@ -372,12 +394,6 @@ func (c *Tendermint) verifyHeader(chain consensus.ChainHeaderReader, header *typ
 		return nil
 	}
 
-	epochHeader := c.getEpochHeader(chain, header)
-	if epochHeader == nil {
-		return fmt.Errorf("epochHeader not found, height:%d", number)
-	}
-
-	vs := types.NewValidatorSet(epochHeader.NextValidators, types.U64ToI64Array(epochHeader.NextValidatorPowers), int64(c.config.ProposerRepetition))
 	return vs.VerifyCommit(c.config.NetworkID, header.Hash(), number, header.Commit)
 }
 
@@ -405,31 +421,17 @@ func (c *Tendermint) VerifyUncles(chain consensus.ChainReader, block *types.Bloc
 
 // Prepare implements consensus.Engine, preparing all the consensus fields of the
 // header for running the transactions on top.
+// This method should be called by store.MakeBlock() -> worker.getSealingBlock() -> engine.Prepare().
 func (c *Tendermint) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error {
 	number := header.Number.Uint64()
-	epochHeader := c.getEpochHeader(chain, header)
-	if epochHeader == nil {
-		return fmt.Errorf("epochHeader not found, height:%d", number)
-	}
-	parentHeader := chain.GetHeaderByHash(header.ParentHash)
-	if epochHeader == nil {
-		return fmt.Errorf("parentHeader not found, height:%d", number)
-	}
 
-	header.LastCommitHash = parentHeader.Commit.Hash()
-	var timestamp uint64
-	if number == 1 {
-		timestamp = parentHeader.TimeMs // genesis time
-	} else {
-		timestamp = pbftconsensus.MedianTime(
-			parentHeader.Commit,
-			types.NewValidatorSet(epochHeader.NextValidators, types.U64ToI64Array(epochHeader.NextValidatorPowers), int64(c.config.ProposerRepetition)),
-		)
-	}
-
-	header.TimeMs = timestamp
-	header.Time = timestamp / 1000
 	header.Difficulty = big.NewInt(1)
+	// Use constant nonce at the monent
+	header.Nonce = nonceDefault
+	// Mix digest is reserved for now, set to empty
+	header.MixDigest = common.Hash{}
+
+	// Timestamp should be already set in store.MakeBlock()
 
 	governance := gov.New(c.config, chain)
 	header.NextValidators = governance.NextValidators(number)