Keep block period on the current epoch during resync

consensus/consensus_service.go

@@ -19,6 +19,7 @@ import (
 	"github.com/harmony-one/harmony/crypto/hash"
 	"github.com/harmony-one/harmony/internal/chain"
 	nodeconfig "github.com/harmony-one/harmony/internal/configs/node"
+	"github.com/harmony-one/harmony/internal/params"
 	"github.com/harmony-one/harmony/internal/utils"
 	"github.com/harmony-one/harmony/multibls"
 	"github.com/harmony-one/harmony/p2p"
@@ -338,6 +339,38 @@ func (consensus *Consensus) UpdateConsensusInformation(reason string) Mode {
 	return consensus.updateConsensusInformation(reason)
 }
 
+func blockPeriodForEpoch(config *params.ChainConfig, epoch *big.Int) time.Duration {
+	blockPeriod := 5 * time.Second
+	if config.IsTwoSeconds(epoch) {
+		blockPeriod = 2 * time.Second
+	}
+	if config.IsOneSecond(epoch) {
+		blockPeriod = time.Second
+	}
+	return blockPeriod
+}
+
+func blockPeriodForConsensusUpdate(
+	config *params.ChainConfig, currentEpoch, nextEpoch *big.Int, isLastBlockInEpoch bool,
+) time.Duration {
+	// Startup and resync must retain the current epoch's cadence. Once the last
+	// block is committed, the next proposal belongs to the next epoch.
+	epoch := currentEpoch
+	if isLastBlockInEpoch {
+		epoch = nextEpoch
+	}
+	return blockPeriodForEpoch(config, epoch)
+}
+
+func (consensus *Consensus) setNextBlockDue(
+	now time.Time, config *params.ChainConfig, currentEpoch, nextEpoch *big.Int, isLastBlockInEpoch bool,
+) {
+	consensus.BlockPeriod = blockPeriodForConsensusUpdate(
+		config, currentEpoch, nextEpoch, isLastBlockInEpoch,
+	)
+	consensus.NextBlockDue = now.Add(consensus.BlockPeriod)
+}
+
 func (consensus *Consensus) updateConsensusInformation(reason string) Mode {
 	curHeader := consensus.Blockchain().CurrentHeader()
 	curEpoch := curHeader.Epoch()
@@ -358,15 +391,9 @@ func (consensus *Consensus) updateConsensusInformation(reason string) Mode {
 		}
 	}
 
-	consensus.BlockPeriod = 5 * time.Second
-
-	// Enable 2s block time at the twoSecondsEpoch
-	if consensus.Blockchain().Config().IsTwoSeconds(nextEpoch) {
-		consensus.BlockPeriod = 2 * time.Second
-	}
-	if consensus.Blockchain().Config().IsOneSecond(nextEpoch) {
-		consensus.BlockPeriod = 1 * time.Second
-	}
+	consensus.BlockPeriod = blockPeriodForConsensusUpdate(
+		consensus.Blockchain().Config(), curEpoch, nextEpoch, curHeader.IsLastBlockInEpoch(),
+	)
 
 	isFirstTimeStaking := consensus.Blockchain().Config().IsStaking(nextEpoch) &&
 		curHeader.IsLastBlockInEpoch() && !consensus.Blockchain().Config().IsStaking(curEpoch)

consensus/consensus_service_test.go

@@ -1,9 +1,12 @@
 package consensus
 
 import (
+	"math/big"
 	"testing"
+	"time"
 
 	"github.com/harmony-one/harmony/crypto/bls"
+	"github.com/harmony-one/harmony/internal/params"
 	"github.com/harmony-one/harmony/internal/registry"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
@@ -90,3 +93,121 @@ func TestErrors(t *testing.T) {
 		require.True(t, errors.Is(e2, e1))
 	})
 }
+
+func TestBlockPeriodForConsensusUpdate(t *testing.T) {
+	config := *params.TestnetChainConfig
+	config.TwoSecondsEpoch = big.NewInt(10)
+	config.IsOneSecondEpoch = big.NewInt(20)
+
+	tests := []struct {
+		name               string
+		currentEpoch       int64
+		nextEpoch          int64
+		isLastBlockInEpoch bool
+		want               time.Duration
+	}{
+		{
+			name:         "before two second activation",
+			currentEpoch: 9,
+			nextEpoch:    10,
+			want:         5 * time.Second,
+		},
+		{
+			name:               "two second activation boundary",
+			currentEpoch:       9,
+			nextEpoch:          10,
+			isLastBlockInEpoch: true,
+			want:               2 * time.Second,
+		},
+		{
+			name:         "mid epoch before one second activation",
+			currentEpoch: 19,
+			nextEpoch:    20,
+			want:         2 * time.Second,
+		},
+		{
+			name:               "one second activation boundary",
+			currentEpoch:       19,
+			nextEpoch:          20,
+			isLastBlockInEpoch: true,
+			want:               time.Second,
+		},
+		{
+			name:         "after one second activation",
+			currentEpoch: 20,
+			nextEpoch:    21,
+			want:         time.Second,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := blockPeriodForConsensusUpdate(
+				&config,
+				big.NewInt(test.currentEpoch),
+				big.NewInt(test.nextEpoch),
+				test.isLastBlockInEpoch,
+			)
+			require.Equal(t, test.want, got)
+		})
+	}
+}
+
+func TestSetNextBlockDueUsesPeriodForNextConsensusEpoch(t *testing.T) {
+	config := *params.TestnetChainConfig
+	config.TwoSecondsEpoch = big.NewInt(10)
+	config.IsOneSecondEpoch = big.NewInt(20)
+
+	tests := []struct {
+		name               string
+		currentEpoch       int64
+		nextEpoch          int64
+		isLastBlockInEpoch bool
+		want               time.Duration
+	}{
+		{
+			name:         "mid epoch before two second activation",
+			currentEpoch: 9,
+			nextEpoch:    10,
+			want:         5 * time.Second,
+		},
+		{
+			name:               "first consensus at two second activation",
+			currentEpoch:       9,
+			nextEpoch:          10,
+			isLastBlockInEpoch: true,
+			want:               2 * time.Second,
+		},
+		{
+			name:         "mid epoch before one second activation",
+			currentEpoch: 19,
+			nextEpoch:    20,
+			want:         2 * time.Second,
+		},
+		{
+			name:               "first consensus at one second activation",
+			currentEpoch:       19,
+			nextEpoch:          20,
+			isLastBlockInEpoch: true,
+			want:               time.Second,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			consensus := &Consensus{BlockPeriod: 99 * time.Second}
+			now := time.Unix(100, 0)
+
+			consensus.setNextBlockDue(
+				now,
+				&config,
+				big.NewInt(test.currentEpoch),
+				big.NewInt(test.nextEpoch),
+				test.isLastBlockInEpoch,
+			)
+
+			require.Equal(t, test.want, consensus.BlockPeriod)
+			require.Equal(t, now.Add(test.want), consensus.NextBlockDue)
+		})
+	}
+}

consensus/consensus_v2.go

@@ -468,8 +468,13 @@ func (consensus *Consensus) BlockChannel(newBlock *types.Block) {
 		consensus.StartFinalityCount()
 		consensus.mutex.Lock()
 		defer consensus.mutex.Unlock()
-		// Update time due for next block
-		consensus.NextBlockDue = time.Now().Add(consensus.BlockPeriod)
+		// Set the cadence for the epoch of the consensus that starts now. This is
+		// especially important for the first block after an epoch-boundary fork.
+		currentEpoch := newBlock.Epoch()
+		nextEpoch := new(big.Int).Add(currentEpoch, common.Big1)
+		consensus.setNextBlockDue(
+			time.Now(), consensus.Blockchain().Config(), currentEpoch, nextEpoch, newBlock.IsLastBlockInEpoch(),
+		)
 
 		startTime = time.Now()
 		consensus.msgSender.Reset(newBlock.NumberU64())
@@ -881,7 +886,7 @@ func (consensus *Consensus) setupForNewConsensus(blk *types.Block, committedMsg
 
 			if consensus.isLeader() && newLeader && !wasLeader {
 				// leader changed
-				blockPeriod := consensus.BlockPeriod
+				blockPeriod := blockPeriodForEpoch(consensus.Blockchain().Config(), epoch)
 				go func() {
 					<-time.After(blockPeriod)
 					consensus.ReadySignal(NewProposal(SyncProposal, blk.NumberU64()+1), "setupForNewConsensus", "I am the new leader")