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

[gokutheengineer]

Description

This PR aims to decouple the consensus module and pacemaker module by making the pacemaker module a submodule of the consensus module.

In a nutshell, this PR includes and introduces:

• The Pacemaker submodule
• A shared ConsensusPacemaker interface used by the Pacemaker submodule
• Renames some fields to improving code quality

Issue

Fixes (partially as intermediate step) #395

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation

List of changes

• Defined a new shared interface called PaceMakerAccessModule which is implemented by Consensus module
• Removed the consensus module field from Pacemaker struct
• Made pacemaker a separate submodule
• Modified the pacemaker to access the consensus module in a synchronous matter via the PaceMakerAccessModule interface

Testing

• make develop_test
• LocalNet w/ all of the steps outlined in the README

Required Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have tested my changes using the available tooling
• I have updated the corresponding CHANGELOG

If Applicable Checklist

• I have updated the corresponding README(s); local and/or global
• I have added tests that prove my fix is effective or that my feature works
• I have added, or updated, mermaid.js diagrams in the corresponding README(s)
• I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

[Olshansk]

Took a first pass at this. Let's clean it up a bit and I'll look deeper

[Olshansk]

@gokutheengineer Also, please make sure this works with LocalNet

[Olshansk]

@gokutheengineer Would also appreciate if you could look into TestHotstuff4Nodes1BlockHappyPath to add a small/simple unit test. Hopefully it's an opportunity to get your hands dirty with the testing framework.

Will be easier after I merge my PR in though.

[gokutheengineer]

@gokutheengineer Also, please make sure this works with LocalNet

I am facing with problems running LocalNet, even on fresh clone with the latest main branch. I opened this ticket: #433

So as is, this branch doesn't execute correctly with LocalNet, but first we need to understand if what I am facing with the LocalNet as explained in the ticket is a general issue or not. Only after that diagnosis, and possible fix, I will be able to test with LocalNet.

[gokutheengineer]

@gokutheengineer Would also appreciate if you could look into TestHotstuff4Nodes1BlockHappyPath to add a small/simple unit test. Hopefully it's an opportunity to get your hands dirty with the testing framework.

Will be easier after I merge my PR in though.

Okay, since I presume the merge will happen very soon, I will implement after the merge.

[Olshansk]

Looks pretty good to me!

Let's get #404 over the finish line, I followed up on #433, and this should be almost good to go

[Olshansk]

@gokutheengineer Per our discussion offline, here's a video with a bit of an explanation of my approach to merging with main and reviewing PRs: https://drive.google.com/file/d/1wmJHYFt8y-G7uWNur_k7K-D3XzJ4Q6XQ/view?usp=sharing

It's a little long (raw and unedited) but hopefully it helps.

[Olshansk]

@gokutheengineer I tested this locally, made a few minor NITS (and fixed an unresolved merge conflict) to help get this over the finish line.

Let's MERGE IT IN!