[Consensus] Decoupling the Pacemaker and Consensus modules (Synchronous method)

consensus/debugging.go

@@ -37,7 +37,7 @@ func (m *consensusModule) GetNodeState() typesCons.ConsensusNodeState {
 		Height:   m.height,
 		Round:    uint8(m.round),
 		Step:     uint8(m.step),
-		IsLeader: m.isLeader(),
+		IsLeader: m.IsLeader(),
 		LeaderId: leaderId,
 	}
 }
@@ -46,7 +46,7 @@ func (m *consensusModule) resetToGenesis(_ *messaging.DebugMessage) {
 	m.nodeLog(typesCons.DebugResetToGenesis)
 
 	m.height = 0
-	m.resetForNewHeight()
+	m.ResetForNewHeight()
 	m.clearLeader()
 	m.clearMessagesPool()
 	m.GetBus().GetPersistenceModule().HandleDebugMessage(&messaging.DebugMessage{
@@ -86,31 +86,3 @@ func (m *consensusModule) togglePacemakerManualMode(_ *messaging.DebugMessage) {
 	}
 	m.paceMaker.SetManualMode(newMode)
 }
-
-// This Pacemaker interface is only used for development & debugging purposes.
-type PacemakerDebug interface {
-	SetManualMode(bool)
-	IsManualMode() bool
-	ForceNextView()
-}
-
-type paceMakerDebug struct {
-	manualMode                bool
-	debugTimeBetweenStepsMsec uint64
-
-	// IMPROVE: Consider renaming to `previousRoundQC`
-	quorumCertificate *typesCons.QuorumCertificate
-}
-
-func (p *paceMaker) IsManualMode() bool {
-	return p.manualMode
-}
-
-func (p *paceMaker) SetManualMode(manualMode bool) {
-	p.manualMode = manualMode
-}
-
-func (p *paceMaker) ForceNextView() {
-	lastQC := p.quorumCertificate
-	p.startNextView(lastQC, true)
-}

consensus/doc/CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.21] - 2023-01-24
+
+- Decouple consensus module and pacemaker module
+- Add `pacemaker` submodule
+- Update pacemaker struct to remove consensus module field, and related functions
+- Create new `pacemaker_consensus.go` source file that consists ConsensusPacemaker function implementations
+
 ## [0.0.0.20] - 2023-01-19
 
 - Rewrite `interface{}` to `any`

consensus/e2e_tests/pacemaker_test.go

@@ -142,7 +142,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) {
 
 	// Starting point
 	testHeight := uint64(3)
-	testStep := consensus.NewRound
+	testStep := uint8(consensus.NewRound)
 
 	// UnitTestNet configs
 	paceMakerTimeoutMsec := uint64(500) // Set a small pacemaker timeout

consensus/helpers.go

@@ -130,13 +130,6 @@ func (m *consensusModule) isOptimisticThresholdMet(numSignatures int, validators
 	return nil
 }
 
-func (m *consensusModule) resetForNewHeight() {
-	m.round = 0
-	m.block = nil
-	m.highPrepareQC = nil
-	m.lockedQC = nil
-}
-
 func protoHash(m proto.Message) string {
 	b, err := codec.GetCodec().Marshal(m)
 	if err != nil {
@@ -208,17 +201,8 @@ func (m *consensusModule) clearMessagesPool() {
 }
 
 /*** Leader Election Helpers ***/
-
-func (m *consensusModule) isLeaderUnknown() bool {
-	return m.leaderId == nil
-}
-
-func (m *consensusModule) isLeader() bool {
-	return m.leaderId != nil && *m.leaderId == m.nodeId
-}
-
 func (m *consensusModule) isReplica() bool {
-	return !m.isLeader()
+	return !m.IsLeader()
 }
 
 func (m *consensusModule) clearLeader() {
@@ -242,7 +226,7 @@ func (m *consensusModule) electNextLeader(message *typesCons.HotstuffMessage) er
 
 	idToValAddrMap := typesCons.NewActorMapper(validators).GetIdToValAddrMap()
 
-	if m.isLeader() {
+	if m.IsLeader() {
 		m.setLogPrefix("LEADER")
 		m.nodeLog(typesCons.ElectedSelfAsNewLeader(idToValAddrMap[*m.leaderId], *m.leaderId, m.height, m.round))
 	} else {
@@ -267,6 +251,7 @@ func (m *consensusModule) nodeLogError(s string, err error) {
 
 func (m *consensusModule) setLogPrefix(logPrefix string) {
 	m.logPrefix = logPrefix
+	m.paceMaker.SetLogPrefix(logPrefix)
 }
 
 func (m *consensusModule) getValidatorsAtHeight(height uint64) ([]*coreTypes.Actor, error) {

consensus/hotstuff_leader.go

@@ -50,7 +50,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM
 	}
 
 	// Likely to be `nil` if blockchain is progressing well.
-	// TECHDEBT: How do we properly validate `highPrepareQC` here?
+	// TECHDEBT: How do we properly validate `prepareQC` here?
 	highPrepareQC := m.findHighQC(m.messagePool[NewRound])
 
 	// TODO: Add test to make sure same block is not applied twice if round is interrupted after being 'Applied'.
@@ -64,7 +64,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM
 		}
 		m.block = block
 	} else {
-		// Leader acts like a replica if `highPrepareQC` is not `nil`
+		// Leader acts like a replica if `prepareQC` is not `nil`
 		// TODO: Do we need to call `validateProposal` here similar to how replicas does it
 		if err := m.applyBlock(highPrepareQC.Block); err != nil {
 			m.nodeLogError(typesCons.ErrApplyBlock.Error(), err)
@@ -118,7 +118,7 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrepareMessage(m *consensusMo
 	}
 
 	m.step = PreCommit
-	m.highPrepareQC = prepareQC
+	m.prepareQC = prepareQC
 	m.messagePool[Prepare] = nil
 
 	preCommitProposeMessage, err := CreateProposeMessage(m.height, m.round, PreCommit, m.block, prepareQC)
@@ -400,19 +400,19 @@ func (m *consensusModule) prepareAndApplyBlock(qc *typesCons.QuorumCertificate)
 // ADDTEST: Add more tests for all the different scenarios here
 func (m *consensusModule) shouldPrepareNewBlock(highPrepareQC *typesCons.QuorumCertificate) bool {
 	if highPrepareQC == nil {
-		m.nodeLog("Preparing a new block - no highPrepareQC found")
+		m.nodeLog("Preparing a new block - no prepareQC found")
 		return true
-	} else if m.isHighPrepareQCFromPast(highPrepareQC) {
-		m.nodeLog("Preparing a new block - highPrepareQC is from the past")
+	} else if m.isPrepareQCFromPast(highPrepareQC) {
+		m.nodeLog("Preparing a new block - prepareQC is from the past")
 		return true
 	} else if highPrepareQC.Block == nil {
-		m.nodeLog("[WARN] Preparing a new block - highPrepareQC SHOULD be used but block is nil")
+		m.nodeLog("[WARN] Preparing a new block - prepareQC SHOULD be used but block is nil")
 		return true
 	}
 	return false
 }
 
-// The `highPrepareQC` is from the past so we can safely ignore it
-func (m *consensusModule) isHighPrepareQCFromPast(highPrepareQC *typesCons.QuorumCertificate) bool {
+// The `prepareQC` is from the past so we can safely ignore it
+func (m *consensusModule) isPrepareQCFromPast(highPrepareQC *typesCons.QuorumCertificate) bool {
 	return highPrepareQC.Height < m.height || highPrepareQC.Round < m.round
 }

consensus/hotstuff_replica.go

@@ -97,7 +97,7 @@ func (handler *HotstuffReplicaMessageHandler) HandlePrecommitMessage(m *consensu
 	}
 
 	m.step = Commit
-	m.highPrepareQC = quorumCert // INVESTIGATE: Why are we never using this for validation?
+	m.prepareQC = quorumCert // INVESTIGATE: Why are we never using this for validation?
 
 	preCommitVoteMessage, err := CreateVoteMessage(m.height, m.round, PreCommit, m.block, m.privateKey)
 	if err != nil {

consensus/module.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/pokt-network/pocket/consensus/leader_election"
+	"github.com/pokt-network/pocket/consensus/pacemaker"
 	consensusTelemetry "github.com/pokt-network/pocket/consensus/telemetry"
 	typesCons "github.com/pokt-network/pocket/consensus/types"
 	"github.com/pokt-network/pocket/runtime/configs"
@@ -49,9 +50,8 @@ type consensusModule struct {
 	// TODO(#315): Move the statefulness of `TxResult` to the persistence module
 	TxResults []modules.TxResult // The current block applied transaction results / voted on; it has not been committed to finality
 
-	// IMPROVE: Consider renaming `highPrepareQC` to simply `prepareQC`
-	highPrepareQC *typesCons.QuorumCertificate // Highest QC for which replica voted PRECOMMIT
-	lockedQC      *typesCons.QuorumCertificate // Highest QC for which replica voted COMMIT
+	prepareQC *typesCons.QuorumCertificate // Highest QC for which replica voted PRECOMMIT
+	lockedQC  *typesCons.QuorumCertificate // Highest QC for which replica voted COMMIT
 
 	// Leader Election
 	leaderId *typesCons.NodeId
@@ -63,7 +63,7 @@ type consensusModule struct {
 	//                to streamline how its accessed in the module (see the ticket).
 
 	utilityContext    modules.UtilityContext
-	paceMaker         Pacemaker
+	paceMaker         pacemaker.Pacemaker
 	leaderElectionMod leader_election.LeaderElectionModule
 
 	// DEPRECATE: Remove later when we build a shared/proper/injected logger
@@ -78,34 +78,42 @@ type consensusModule struct {
 type ConsensusDebugModule interface {
 	SetHeight(uint64)
 	SetRound(uint64)
-	SetStep(typesCons.HotstuffStep)
+	// REFACTOR: This should accept typesCons.HotstuffStep.
+	SetStep(uint8)
 	SetBlock(*coreTypes.Block)
 	SetLeaderId(*typesCons.NodeId)
 	SetUtilityContext(modules.UtilityContext)
 }
 
-func (c *consensusModule) SetHeight(height uint64) {
-	c.height = height
+func (m *consensusModule) SetHeight(height uint64) {
+	m.height = height
 }
 
-func (c *consensusModule) SetRound(round uint64) {
-	c.round = round
+func (m *consensusModule) SetRound(round uint64) {
+	m.round = round
 }
 
-func (c *consensusModule) SetStep(step typesCons.HotstuffStep) {
-	c.step = step
+func (m *consensusModule) SetStep(step uint8) {
+	m.step = typesCons.HotstuffStep(step)
 }
 
-func (c *consensusModule) SetBlock(block *coreTypes.Block) {
-	c.block = block
+func (m *consensusModule) SetBlock(block *coreTypes.Block) {
+	m.block = block
 }
 
-func (c *consensusModule) SetLeaderId(leaderId *typesCons.NodeId) {
-	c.leaderId = leaderId
+func (m *consensusModule) SetLeaderId(leaderId *typesCons.NodeId) {
+	m.leaderId = leaderId
 }
 
-func (c *consensusModule) SetUtilityContext(utilityContext modules.UtilityContext) {
-	c.utilityContext = utilityContext
+func (m *consensusModule) SetUtilityContext(utilityContext modules.UtilityContext) {
+	m.utilityContext = utilityContext
+}
+
+// Implementations of the type PaceMakerAccessModule interface
+// SetHeight, SeetRound, SetStep are implemented for ConsensusDebugModule
+func (m *consensusModule) ClearLeaderMessagesPool() {
+	m.clearLeader()
+	m.clearMessagesPool()
 }
 
 func Create(bus modules.Bus) (modules.Module, error) {
@@ -118,13 +126,12 @@ func (*consensusModule) Create(bus modules.Bus) (modules.Module, error) {
 		return nil, err
 	}
 
-	// TODO(olshansky): Can we make this a submodule?
-	paceMakerMod, err := CreatePacemaker(bus)
+	paceMakerMod, err := pacemaker.CreatePacemaker(bus)
 	if err != nil {
 		return nil, err
 	}
 
-	pacemaker := paceMakerMod.(Pacemaker)
+	pacemaker := paceMakerMod.(pacemaker.Pacemaker)
 	m := &consensusModule{
 		paceMaker:         pacemaker,
 		leaderElectionMod: leaderElectionMod.(leader_election.LeaderElectionModule),
@@ -134,8 +141,8 @@ func (*consensusModule) Create(bus modules.Bus) (modules.Module, error) {
 		step:   NewRound,
 		block:  nil,
 
-		highPrepareQC: nil,
-		lockedQC:      nil,
+		prepareQC: nil,
+		lockedQC:  nil,
 
 		leaderId: nil,
 
@@ -147,9 +154,6 @@ func (*consensusModule) Create(bus modules.Bus) (modules.Module, error) {
 	}
 	bus.RegisterModule(m)
 
-	// TODO(#395): Decouple the pacemaker and consensus modules
-	pacemaker.SetConsensusModule(m)
-
 	runtimeMgr := bus.GetRuntimeMgr()
 
 	consensusCfg := runtimeMgr.GetConfig().Consensus