IOTEX-126 Remove dummy block handling logic (iotexproject#241)

Author: zjshen14

blockchain/block.go

@@ -130,14 +130,6 @@ func NewSecretBlock(
 	return block
 }
 
-// IsDummyBlock checks whether block is a dummy block
-func (b *Block) IsDummyBlock() bool {
-	return b.Header.height > 0 &&
-		len(b.Header.blockSig) == 0 &&
-		b.Header.Pubkey == keypair.ZeroPublicKey &&
-		len(b.Actions) == 0
-}
-
 // Height returns the height of this block
 func (b *Block) Height() uint64 {
 	return b.Header.height
@@ -168,10 +160,7 @@ func (b *Block) ByteStreamHeader() []byte {
 	tmp8B := make([]byte, 8)
 	enc.MachineEndian.PutUint64(tmp8B, b.Header.height)
 	stream = append(stream, tmp8B...)
-	// TODO: exclude timestamp from block hash because dummy block needs to have a consistent hash no matter which
-	// node produces it at a given height. Once we get rid of the dummy block concept, we need to include it into
-	// the hash block hash again
-	//enc.MachineEndian.PutUint64(tmp8B, b.Header.timestamp)
+	enc.MachineEndian.PutUint64(tmp8B, b.Header.timestamp)
 	stream = append(stream, tmp8B...)
 	stream = append(stream, b.Header.prevBlockHash[:]...)
 	stream = append(stream, b.Header.txRoot[:]...)

blockchain/blockchain.go

@@ -107,8 +107,6 @@ type Blockchain interface {
 	// MintNewSecretBlock creates a new DKG secret block with given DKG secrets and witness
 	MintNewSecretBlock(secretProposals []*action.SecretProposal, secretWitness *action.SecretWitness,
 		producer *iotxaddress.Address) (*Block, error)
-	// MintDummyNewBlock creates a new dummy block, used for unreached consensus
-	MintNewDummyBlock() *Block
 	// CommitBlock validates and appends a block to the chain
 	CommitBlock(blk *Block) error
 	// ValidateBlock validates a new block before adding it to the blockchain
@@ -728,30 +726,6 @@ func (bc *blockchain) MintNewSecretBlock(
 	return blk, nil
 }
 
-// MintDummyNewBlock creates a new dummy block, used for unreached consensus
-func (bc *blockchain) MintNewDummyBlock() *Block {
-	bc.mu.RLock()
-	defer bc.mu.RUnlock()
-
-	blk := NewBlock(bc.config.Chain.ID, bc.tipHeight+1, bc.tipHash, bc.now(), nil)
-	blk.Header.Pubkey = keypair.ZeroPublicKey
-	blk.Header.blockSig = []byte{}
-
-	// run execution and update state trie root hash
-	ws, err := bc.sf.NewWorkingSet()
-	if err != nil {
-		return nil
-	}
-	root, err := bc.runActions(blk, ws, false)
-	if err != nil {
-		return nil
-	}
-	blk.Header.stateRoot = root
-	// attach working set to be committed to state factory
-	blk.workingSet = ws
-	return blk
-}
-
 //  CommitBlock validates and appends a block to the chain
 func (bc *blockchain) CommitBlock(blk *Block) error {
 	bc.mu.Lock()
@@ -818,13 +792,8 @@ func (bc *blockchain) validateBlock(blk *Block, containCoinbase bool) error {
 		logger.Panic().Msg("no block validator")
 	}
 
-	tipHeight, tipHash, err := bc.replaceHeightAndHash(blk)
-	if err != nil {
-		return errors.Wrap(err, "failed to get the tip height and tip hash of blockchain")
-	}
-
-	if err := bc.validator.Validate(blk, tipHeight, tipHash, containCoinbase); err != nil {
-		return errors.Wrapf(err, "Failed to validate block on height %d", tipHeight)
+	if err := bc.validator.Validate(blk, bc.tipHeight, bc.tipHash, containCoinbase); err != nil {
+		return errors.Wrapf(err, "Failed to validate block on height %d", bc.tipHeight)
 	}
 	// run actions and update state factory
 	ws, err := bc.sf.NewWorkingSet()
@@ -833,7 +802,7 @@ func (bc *blockchain) validateBlock(blk *Block, containCoinbase bool) error {
 	}
 	// TODO: disable validation before resolve the state root doesn't match issue
 	if _, err := bc.runActions(blk, ws, false); err != nil {
-		logger.Panic().Err(err).Msgf("Failed to update state on height %d", tipHeight)
+		logger.Panic().Err(err).Msgf("Failed to update state on height %d", bc.tipHeight)
 	}
 	// attach working set to be committed to state factory
 	blk.workingSet = ws
@@ -891,28 +860,6 @@ func (bc *blockchain) runActions(blk *Block, ws state.WorkingSet, verify bool) (
 	return root, nil
 }
 
-func (bc *blockchain) replaceHeightAndHash(blk *Block) (uint64, hash.Hash32B, error) {
-	tipHeight := bc.tipHeight
-	tipHash := bc.tipHash
-	// replacement logic, used to replace a fake old dummy block
-	if blk.Height() != 0 && blk.Height() <= bc.tipHeight {
-		oldDummyBlock, err := bc.GetBlockByHeight(blk.Height())
-		if err != nil {
-			return 0, hash.ZeroHash32B, errors.Wrapf(err, "The height of the new block is invalid")
-		}
-		if !oldDummyBlock.IsDummyBlock() {
-			return 0, hash.ZeroHash32B, errors.New("The replaced block is not a dummy block")
-		}
-		lastBlock, err := bc.GetBlockByHeight(blk.Height() - 1)
-		if err != nil {
-			return 0, hash.ZeroHash32B, errors.Wrapf(err, "Failed to get the last block when replacing the dummy block")
-		}
-		tipHeight = lastBlock.Height()
-		tipHash = lastBlock.HashBlock()
-	}
-	return tipHeight, tipHash, nil
-}
-
 func (bc *blockchain) SubscribeBlockCreation(ch chan *Block) error {
 	bc.blocklistener = append(bc.blocklistener, ch)
 	return nil

blockchain/blockchain_test.go

@@ -11,9 +11,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"math/big"
-	"strings"
 	"testing"
-	"time"
 
 	"github.com/facebookgo/clock"
 	"github.com/stretchr/testify/require"
@@ -644,40 +642,6 @@ func TestBlockchain_Validator(t *testing.T) {
 	require.NotNil(t, bc.Validator())
 }
 
-func TestBlockchain_MintNewDummyBlock(t *testing.T) {
-	cfg := &config.Default
-	testutil.CleanupPath(t, cfg.Chain.TrieDBPath)
-	defer testutil.CleanupPath(t, cfg.Chain.TrieDBPath)
-	testutil.CleanupPath(t, cfg.Chain.ChainDBPath)
-	defer testutil.CleanupPath(t, cfg.Chain.ChainDBPath)
-	require := require.New(t)
-	sf, err := state.NewFactory(cfg, state.DefaultTrieOption())
-	require.NoError(err)
-	require.NoError(sf.Start(context.Background()))
-	val := validator{sf, ""}
-
-	ctx := context.Background()
-	bc := NewBlockchain(cfg, InMemDaoOption(), InMemStateFactoryOption())
-	require.NoError(bc.Start(ctx))
-	defer func() {
-		err := bc.Stop(ctx)
-		require.Nil(err)
-	}()
-	require.NotNil(bc)
-
-	blk := bc.MintNewDummyBlock()
-	require.Equal(uint64(1), blk.Height())
-	tipHash := bc.TipHash()
-	require.NoError(val.Validate(blk, 0, tipHash, true))
-	tsf, _ := action.NewTransfer(1, big.NewInt(1), "", "", []byte{}, uint64(100000), big.NewInt(10))
-	blk.Actions = []action.Action{tsf}
-	err = val.Validate(blk, 0, tipHash, true)
-	require.Error(err)
-	require.True(
-		strings.Contains(err.Error(), "failed to verify block's signature"),
-	)
-}
-
 func TestBlockchainInitialCandidate(t *testing.T) {
 	require := require.New(t)
 
@@ -870,85 +834,6 @@ func TestActions(t *testing.T) {
 	require.Nil(val.Validate(blk, 0, blk.PrevHash(), true))
 }
 
-func TestDummyReplacement(t *testing.T) {
-	require := require.New(t)
-	cfg := config.Default
-
-	testutil.CleanupPath(t, testTriePath)
-	defer testutil.CleanupPath(t, testTriePath)
-	testutil.CleanupPath(t, testDBPath)
-	defer testutil.CleanupPath(t, testDBPath)
-
-	cfg.Chain.TrieDBPath = testTriePath
-	cfg.Chain.ChainDBPath = testDBPath
-
-	sf, _ := state.NewFactory(&cfg, state.InMemTrieOption())
-	require.NoError(sf.Start(context.Background()))
-	require.NoError(addCreatorToFactory(sf))
-
-	// Create a blockchain from scratch
-	bc := NewBlockchain(&cfg, PrecreatedStateFactoryOption(sf), BoltDBDaoOption())
-	require.NoError(bc.Start(context.Background()))
-	dummy := bc.MintNewDummyBlock()
-	require.Nil(bc.ValidateBlock(dummy, true))
-	require.NoError(bc.CommitBlock(dummy))
-	actualDummyBlock, err := bc.GetBlockByHeight(1)
-	require.NoError(err)
-	require.Equal(dummy.HashBlock(), actualDummyBlock.HashBlock())
-
-	chain := bc.(*blockchain)
-	chain.tipHeight = 0
-	realBlock, err := bc.MintNewBlock(nil, ta.Addrinfo["producer"],
-		nil, nil, "")
-	require.NotNil(realBlock)
-	require.NoError(err)
-	require.Nil(bc.ValidateBlock(realBlock, true))
-	require.NoError(bc.CommitBlock(realBlock))
-	actualRealBlock, err := bc.GetBlockByHeight(1)
-	require.NoError(err)
-	require.Equal(realBlock.HashBlock(), actualRealBlock.HashBlock())
-
-	block2, err := bc.MintNewBlock(nil, ta.Addrinfo["producer"],
-		nil, nil, "")
-	require.NoError(err)
-	require.Nil(bc.ValidateBlock(block2, true))
-	require.NoError(bc.CommitBlock(block2))
-	dummyBlock3 := bc.MintNewDummyBlock()
-	require.Nil(bc.ValidateBlock(dummyBlock3, true))
-	require.NoError(bc.CommitBlock(dummyBlock3))
-	block4, err := bc.MintNewBlock(nil, ta.Addrinfo["producer"],
-		nil, nil, "")
-	require.NoError(err)
-	require.NoError(bc.ValidateBlock(block4, true))
-	require.NoError(bc.CommitBlock(block4))
-	actualDummyBlock3, err := bc.GetBlockByHeight(3)
-	require.NoError(err)
-	require.True(actualDummyBlock3.IsDummyBlock())
-	chain.tipHeight = 2
-	block3, err := bc.MintNewBlock(nil, ta.Addrinfo["producer"],
-		nil, nil, "")
-	require.NotNil(block3)
-	require.NoError(err)
-	require.NoError(bc.ValidateBlock(block3, true))
-	require.NoError(bc.CommitBlock(block3))
-	actualBlock3, err := bc.GetBlockByHeight(3)
-	require.NoError(err)
-	require.Equal(block3.HashBlock(), actualBlock3.HashBlock())
-}
-
-func TestMintNewBlock(t *testing.T) {
-	t.Parallel()
-	cfg := config.Default
-	clk := clock.NewMock()
-	chain := NewBlockchain(&cfg, InMemDaoOption(), InMemStateFactoryOption(), ClockOption(clk))
-	require.NoError(t, chain.Start(context.Background()))
-	blk1 := chain.MintNewDummyBlock()
-	clk.Add(2 * time.Second)
-	blk2 := chain.MintNewDummyBlock()
-	require.Equal(t, uint64(2), blk2.Header.timestamp-blk1.Header.timestamp)
-	require.Equal(t, blk1.HashBlock(), blk2.HashBlock())
-}
-
 func TestMintDKGBlock(t *testing.T) {
 	require := require.New(t)
 	lastSeed, _ := hex.DecodeString("9de6306b08158c423330f7a27243a1a5cbe39bfd764f07818437882d21241567")
@@ -1015,10 +900,6 @@ func TestMintDKGBlock(t *testing.T) {
 	}
 
 	// Generate dkg signature for each block
-	require.NoError(err)
-	dummy := chain.MintNewDummyBlock()
-	require.NoError(chain.ValidateBlock(dummy, true))
-	require.NoError(chain.CommitBlock(dummy))
 	for i := 1; i < numNodes; i++ {
 		iotxAddr := iotxaddress.Address{
 			PublicKey:  ec283PKList[i],
@@ -1036,7 +917,7 @@ func TestMintDKGBlock(t *testing.T) {
 	}
 	height, err := chain.GetFactory().Height()
 	require.NoError(err)
-	require.Equal(uint64(21), height)
+	require.Equal(uint64(20), height)
 	candidates, err := chain.CandidatesByHeight(height)
 	require.NoError(err)
 	require.Equal(21, len(candidates))

blockchain/blockvalidator.go

@@ -56,9 +56,6 @@ func (v *validator) Validate(blk *Block, tipHeight uint64, tipHash hash.Hash32B,
 	if err := verifyHeightAndHash(blk, tipHeight, tipHash); err != nil {
 		return errors.Wrap(err, "failed to verify block's height and hash")
 	}
-	if blk.IsDummyBlock() {
-		return nil
-	}
 	if err := verifySigAndRoot(blk); err != nil {
 		return errors.Wrap(err, "failed to verify block's signature and merkle root")
 	}

blocksync/blocksync.go

@@ -79,10 +79,7 @@ func (bs *blockSyncer) P2P() network.Overlay {
 // Start starts a block syncer
 func (bs *blockSyncer) Start(ctx context.Context) error {
 	logger.Debug().Msg("Starting block syncer")
-	startHeight, err := findSyncStartHeight(bs.bc)
-	if err != nil {
-		return err
-	}
+	startHeight := bs.bc.TipHeight() + 1
 	bs.buf.startHeight = startHeight
 	bs.buf.confirmedHeight = startHeight - 1
 	return bs.worker.Start(ctx)

blocksync/buffer.go

@@ -51,10 +51,6 @@ func (b *blockBuffer) Flush(blk *blockchain.Block) (bool, bCheckinResult) {
 		return moved, bCheckinLower
 	}
 	if h < b.startHeight {
-		// try to replace a dummy block
-		if blk.IsDummyBlock() {
-			return moved, bCheckinLower
-		}
 		if err := commitBlock(b.bc, b.ap, blk); err != nil {
 			return moved, bCheckinLower
 		}
@@ -78,11 +74,8 @@ func (b *blockBuffer) Flush(blk *blockchain.Block) (bool, bCheckinResult) {
 		}
 		if err := commitBlock(b.bc, b.ap, b.blocks[syncHeight]); err == nil {
 			syncedHeight = syncHeight
-			if !b.blocks[syncedHeight].IsDummyBlock() {
-				b.confirmedHeight = syncedHeight
-			}
-			l.Info().Uint64("syncedHeight", syncedHeight).Bool("dummy", b.blocks[syncedHeight].IsDummyBlock()).
-				Msg("Successfully committed block.")
+			b.confirmedHeight = syncedHeight
+			l.Info().Uint64("syncedHeight", syncedHeight).Msg("Successfully committed block.")
 			delete(b.blocks, syncHeight)
 			continue
 		} else {
@@ -94,14 +87,7 @@ func (b *blockBuffer) Flush(blk *blockchain.Block) (bool, bCheckinResult) {
 			if blk.HashBlock() == b.blocks[syncHeight].HashBlock() {
 				// same existing block
 				syncedHeight = syncHeight
-				if !b.blocks[syncedHeight].IsDummyBlock() {
-					b.confirmedHeight = syncedHeight
-				}
-			} else if blk.IsDummyBlock() {
-				// existing dummy but get a bad block
-				l.Error().Uint64("syncHeight", syncHeight).
-					Uint64("syncedHeight", syncedHeight).
-					Msg("Failed to replace dummy block.")
+				b.confirmedHeight = syncedHeight
 			}
 			delete(b.blocks, syncHeight)
 		} else {

blocksync/util.go

@@ -9,8 +9,6 @@ package blocksync
 import (
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/iotexproject/iotex-core/actpool"
 	"github.com/iotexproject/iotex-core/blockchain"
 	"github.com/iotexproject/iotex-core/config"
@@ -28,29 +26,6 @@ func commitBlock(bc blockchain.Blockchain, ap actpool.ActPool, blk *blockchain.B
 	return nil
 }
 
-// findSyncStartHeight needs to find a reasonable start point to sync
-// 1. current height + 1 if current height is not dummy
-// 2. current height remove all dummy on top + 1
-// 3. FIXME this node may still has issue, if it was following the wrong chain, this is actually a general version of 2, but in 3, we need to rollback blockchain first
-func findSyncStartHeight(bc blockchain.Blockchain) (uint64, error) {
-	var next uint64
-	h := bc.TipHeight()
-	for ; ; h-- {
-		blk, err := bc.GetBlockByHeight(h)
-		if err != nil {
-			return next, err
-		}
-		if !blk.IsDummyBlock() {
-			next = h + 1
-			break
-		}
-		if h == 0 { // 0 height is dummy
-			return next, errors.New("0 height block is dummy block")
-		}
-	}
-	return next, nil
-}
-
 // syncTaskInterval returns the recurring sync task interval, or 0 if this config should not need to run sync task
 func syncTaskInterval(cfg *config.Config) time.Duration {
 	if cfg.IsLightweight() {