Configure commit delay via setter and not ctor arg

Also make -delay_commit accept duration strings such as 500ms.

cmd/client/txgen/main.go

@@ -100,7 +100,7 @@ func setUpTXGen() *node.Node {
 		fmt.Fprintf(os.Stderr, "Error :%v \n", err)
 		os.Exit(1)
 	}
-	consensusObj, err := consensus.New(myhost, uint32(shardID), p2p.Peer{}, nil, 0)
+	consensusObj, err := consensus.New(myhost, uint32(shardID), p2p.Peer{}, nil)
 	chainDBFactory := &shardchain.MemDBFactory{}
 	txGen := node.New(myhost, consensusObj, chainDBFactory, false) //Changed it : no longer archival node.
 	txGen.Client = client.NewClient(txGen.GetHost(), uint32(shardID))

cmd/harmony/main.go

@@ -95,8 +95,8 @@ var (
 	isGenesis = flag.Bool("is_genesis", true, "true means this node is a genesis node")
 	// isArchival indicates this node is an archival node that will save and archive current blockchain
 	isArchival = flag.Bool("is_archival", false, "true means this node is a archival node")
-	// delayCommit indicate how many ms to delay for harmony node
+	// delayCommit is the commit-delay timer, used by Harmony nodes
-	delayCommit = flag.Int("delay_commit_ms", 500, "how many ms delay for harmony node to send commit message in consensus")
+	delayCommit = flag.String("delay_commit", "500ms", "how long to delay sending commit messages in consensus, ex: 500ms, 1s")
 	//isNewNode indicates this node is a new node
 	isNewNode          = flag.Bool("is_newnode", false, "true means this node is a new node")
 	shardID            = flag.Int("shard_id", -1, "the shard ID of this node")
@@ -300,11 +300,17 @@ func setUpConsensusAndNode(nodeConfig *nodeconfig.ConfigType) *node.Node {
 	// Consensus object.
 	// TODO: consensus object shouldn't start here
 	// TODO(minhdoan): During refactoring, found out that the peers list is actually empty. Need to clean up the logic of consensus later.
-	currentConsensus, err := consensus.New(nodeConfig.Host, nodeConfig.ShardID, nodeConfig.Leader, nodeConfig.ConsensusPriKey, *delayCommit)
+	currentConsensus, err := consensus.New(nodeConfig.Host, nodeConfig.ShardID, nodeConfig.Leader, nodeConfig.ConsensusPriKey)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error :%v \n", err)
 		os.Exit(1)
 	}
+	commitDelay, err := time.ParseDuration(*delayCommit)
+	if err != nil || commitDelay < 0 {
+		_, _ = fmt.Fprintf(os.Stderr, "invalid commit delay %#v", *delayCommit)
+		os.Exit(1)
+	}
+	currentConsensus.SetCommitDelay(commitDelay)
 	currentConsensus.MinPeers = *minPeers
 	if *disableViewChange {
 		currentConsensus.DisableViewChangeForTestingOnly()

consensus/consensus.go

@@ -4,6 +4,7 @@ package consensus // consensus
 import (
 	"math/big"
 	"sync"
+	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -42,8 +43,8 @@ type Consensus struct {
 	// channel to receive consensus message
 	MsgChan chan []byte
 
-	// unit in ms, how many ms to delay for harmony node to send commit message
+	// How long to delay sending commit messages.
-	delayCommit int
+	delayCommit time.Duration
 
 	// 2 types of timeouts: normal and viewchange
 	consensusTimeout map[TimeoutType]*utils.Timeout
@@ -145,6 +146,12 @@ type Consensus struct {
 	disableViewChange bool
 }
 
+// SetCommitDelay sets the commit message delay.  If set to non-zero,
+// validator delays commit message by the amount.
+func (consensus *Consensus) SetCommitDelay(delay time.Duration) {
+	consensus.delayCommit = delay
+}
+
 // StakeInfoFinder returns the stake information finder instance this
 // consensus uses, e.g. for block reward distribution.
 func (consensus *Consensus) StakeInfoFinder() StakeInfoFinder {
@@ -198,10 +205,9 @@ type StakeInfoFinder interface {
 
 // New creates a new Consensus object
 // TODO: put shardId into chain reader's chain config
-func New(host p2p.Host, ShardID uint32, leader p2p.Peer, blsPriKey *bls.SecretKey, delayCommit int) (*Consensus, error) {
+func New(host p2p.Host, ShardID uint32, leader p2p.Peer, blsPriKey *bls.SecretKey) (*Consensus, error) {
 	consensus := Consensus{}
 	consensus.host = host
-	consensus.delayCommit = delayCommit
 	consensus.blockNumLowChan = make(chan struct{})
 
 	// pbft related

consensus/consensus_leader_msg_test.go

@@ -23,7 +23,7 @@ func TestConstructAnnounceMessage(test *testing.T) {
 	if err != nil {
 		test.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		test.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -53,7 +53,7 @@ func TestConstructPreparedMessage(test *testing.T) {
 	if err != nil {
 		test.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		test.Fatalf("Cannot craeate consensus: %v", err)
 	}

consensus/consensus_service_test.go

@@ -24,7 +24,7 @@ func TestGetPeerFromID(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, leaderPriKey, 0)
+	consensus, err := New(host, 0, leader, leaderPriKey)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -43,7 +43,7 @@ func TestPopulateMessageFields(t *testing.T) {
 		t.Fatalf("newhost failure: %v", err)
 	}
 	blsPriKey := bls.RandPrivateKey()
-	consensus, err := New(host, 0, leader, blsPriKey, 0)
+	consensus, err := New(host, 0, leader, blsPriKey)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -77,7 +77,7 @@ func TestSignAndMarshalConsensusMessage(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -103,7 +103,7 @@ func TestSetViewID(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}

consensus/consensus_test.go

@@ -17,7 +17,7 @@ func TestNew(test *testing.T) {
 	if err != nil {
 		test.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		test.Fatalf("Cannot craeate consensus: %v", err)
 	}

consensus/consensus_v2.go

@@ -418,7 +418,7 @@ func (consensus *Consensus) onPrepared(msg *msg_pb.Message) {
 
 	// TODO: genesis account node delay for 1 second, this is a temp fix for allows FN nodes to earning reward
 	if consensus.delayCommit > 0 {
-		time.Sleep(time.Duration(consensus.delayCommit) * time.Millisecond)
+		time.Sleep(consensus.delayCommit)
 	}
 
 	if err := consensus.host.SendMessageToGroups([]p2p.GroupID{p2p.NewGroupIDByShardID(p2p.ShardID(consensus.ShardID))}, host.ConstructP2pMessage(byte(17), msgToSend)); err != nil {

consensus/consensus_validator_msg_test.go

@@ -20,7 +20,7 @@ func TestConstructPrepareMessage(test *testing.T) {
 	if err != nil {
 		test.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		test.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -48,7 +48,7 @@ func TestConstructCommitMessage(test *testing.T) {
 	if err != nil {
 		test.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		test.Fatalf("Cannot craeate consensus: %v", err)
 	}

consensus/pbft_log_test.go

@@ -19,7 +19,7 @@ func constructAnnounceMessage(t *testing.T) []byte {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := New(host, 0, leader, bls.RandPrivateKey(), 0)
+	consensus, err := New(host, 0, leader, bls.RandPrivateKey())
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}

internal/genesis/genesis.go

@@ -36,7 +36,6 @@ func BeaconAccountPriKey() *ecdsa.PrivateKey {
 // FindAccount find the DeployAccount based on the account address, and the account index
 // the account address could be from GenesisAccounts or from GenesisFNAccounts
 // the account index can be used to determin the shard of the account
-// return flag=true means it is GenesisAccount node
 func FindAccount(address string) (int, *DeployAccount) {
 	for i, acc := range GenesisAccounts {
 		if address == acc.Address {

node/node_handler_test.go

@@ -19,7 +19,7 @@ func TestAddNewBlock(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -44,7 +44,7 @@ func TestVerifyNewBlock(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}

node/node_test.go

@@ -31,7 +31,7 @@ func TestNewNode(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -58,7 +58,7 @@ func TestGetSyncingPeers(t *testing.T) {
 		t.Fatalf("newhost failure: %v", err)
 	}
 
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -100,7 +100,7 @@ func TestAddPeers(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -146,7 +146,7 @@ func TestAddBeaconPeer(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}
@@ -220,7 +220,7 @@ func TestPingPongHandler(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}

node/staking_test.go

@@ -31,7 +31,7 @@ func TestUpdateStakingList(t *testing.T) {
 	if err != nil {
 		t.Fatalf("newhost failure: %v", err)
 	}
-	consensus, err := consensus.New(host, 0, leader, nil, 0)
+	consensus, err := consensus.New(host, 0, leader, nil)
 	if err != nil {
 		t.Fatalf("Cannot craeate consensus: %v", err)
 	}