[SPIKE][Consensus] Remove validatorMap from the consensusModule struct - (Issue #331) (#425)

## Description

This PR builds upon #402 and removes `validatorMap` from the
`consensusModule` struct, encapsulating the logic so that the validator
set is dynamic and retrieved from the `persistence` module.

Also, since the change introduced the natural dependency on
`persistence`, it made me question and revisit the way the modules
register themselves with the `bus` and also how the modules are
instantiated.

In an effort to simply the codebase and testing, while we improve the
way we do dependency injection, the modules now are instantiated with a
`bus` that will allow them to access to all the dependencies they need.

Previously the modules required a `runtimeMgr` in their constructor and
the bus registration was handled like an after-thought.

## Issue

Fixes #331 

## Type of change

Please mark the relevant option(s):

- [x] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [x] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- Removed `validatorMap` from `consensusModule` and encapsulated the
logic so that it's dynamic, per-height and sourced from `persistence`
- Refactored module registration in `bus`
- Refactored modules instantiation and DI (`runtimeMgr` -> `bus`)
- The `runtimeMgr` is now a dependency and it can be accessed via the
bus
- Minor improvements in tests

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## Required Checklist

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

### If Applicable Checklist
- [x] I have updated the corresponding README(s); local and/or global
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added, or updated,
[mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding
README(s)
- [ ] I have added, or updated, documentation and
[mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*`
if I updated `shared/*`README(s)

Signed-off-by: Alessandro De Blasis <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>

Author: deblasis

app/client/cli/debug.go

@@ -58,7 +58,7 @@ func NewDebugCommand() *cobra.Command {
 		Args:  cobra.ExactArgs(0),
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			var err error
-			runtimeMgr := runtime.NewManagerFromFiles(defaultConfigPath, defaultGenesisPath, runtime.WithRandomPK())
+			runtimeMgr := runtime.NewManagerFromFiles(defaultConfigPath, defaultGenesisPath, runtime.WithClientDebugMode(), runtime.WithRandomPK())
 
 			// HACK(#416): this is a temporary solution that guarantees backward compatibility while we implement peer discovery.
 			validators = runtimeMgr.GetGenesis().Validators
@@ -74,7 +74,8 @@ func NewDebugCommand() *cobra.Command {
 
 			debugCurrentHeightProvider := debugCHP.NewDebugCurrentHeightProvider(0)
 
-			p2pM, err := p2p.CreateWithProviders(runtimeMgr, debugAddressBookProvider, debugCurrentHeightProvider)
+			// TODO(#429): refactor injecting the dependencies into the bus so that they can be consumed in an updated `P2PModule.Create()` implementation
+			p2pM, err := p2p.CreateWithProviders(runtimeMgr.GetBus(), debugAddressBookProvider, debugCurrentHeightProvider)
 			if err != nil {
 				log.Fatalf("[ERROR] Failed to create p2p module: %v", err.Error())
 			}

app/client/cli/doc/CHANGELOG.md

@@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [0.0.0.4] - 2023-01-09
+## [0.0.0.4] - 2023-01-10
 
 - The `client` (i.e. CLI) no longer instantiates a `P2P` module along with a bus of optional modules. Instead, it instantiates a `client-only` `P2P` module that is disconnected from consensus and persistence. Interactions with the persistence & consensus layer happen via RPC.
 - Replaced previous implementation, reliant on `ValidatorMap`, with a temporary fetch from genesis. This will be replaced with a lightweight peer discovery mechanism in #416

app/pocket/doc/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.1] - 2023-01-10
+
+- Updated module constructor to accept a `bus` and not a `runtimeMgr` anymore
+
 ## [0.0.0.0] - 2022-11-02
 
 ### Added

app/pocket/main.go

@@ -22,8 +22,12 @@ func main() {
 	}
 
 	runtimeMgr := runtime.NewManagerFromFiles(*configFilename, *genesisFilename)
+	bus, err := runtime.CreateBus(runtimeMgr)
+	if err != nil {
+		log.Fatalf("Failed to create bus: %s", err)
+	}
 
-	pocketNode, err := shared.CreateNode(runtimeMgr)
+	pocketNode, err := shared.CreateNode(bus)
 	if err != nil {
 		log.Fatalf("Failed to create pocket node: %s", err)
 	}

build/config/config1.json

@@ -26,8 +26,7 @@
     "use_rain_tree": true,
     "is_empty_connection_type": false,
     "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4",
-    "max_mempool_count": 100000,
-    "is_client_only": false
+    "max_mempool_count": 100000
   },
   "telemetry": {
     "enabled": true,

build/docs/CHANGELOG.md

@@ -7,10 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [0.0.0.2] - 2023-01-09
+## [0.0.0.2] - 2023-01-10
 
 - Removed `BaseConfig` from `configs`
 - Centralized `PersistenceGenesisState` and `ConsensusGenesisState` into `GenesisState`
+- Removed `is_client_only` since it's set programmatically in the CLI
 
 ## [0.0.0.1] - 2022-12-29
 

consensus/consensus_tests/hotstuff_test.go

@@ -16,12 +16,12 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) {
 	clockMock := clock.NewMock()
 	// Test configs
 	runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock)
-
+	buses := GenerateBuses(t, runtimeMgrs)
 	go timeReminder(clockMock, 100*time.Millisecond)
 
 	// Create & start test pocket nodes
 	testChannel := make(modules.EventsChannel, 100)
-	pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel)
+	pocketNodes := CreateTestConsensusPocketNodes(t, buses, testChannel)
 	StartAllTestPocketNodes(t, pocketNodes)
 
 	// Debug message to start consensus by triggering first view change

consensus/consensus_tests/pacemaker_test.go

@@ -28,10 +28,11 @@ func TestTinyPacemakerTimeouts(t *testing.T) {
 		consCfg := runtimeConfig.GetConfig().Consensus.PacemakerConfig
 		consCfg.TimeoutMsec = paceMakerTimeoutMsec
 	}
+	buses := GenerateBuses(t, runtimeMgrs)
 
 	// Create & start test pocket nodes
 	testChannel := make(modules.EventsChannel, 100)
-	pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel)
+	pocketNodes := CreateTestConsensusPocketNodes(t, buses, testChannel)
 	StartAllTestPocketNodes(t, pocketNodes)
 
 	// Debug message to start consensus by triggering next view.
@@ -124,12 +125,13 @@ func TestTinyPacemakerTimeouts(t *testing.T) {
 func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) {
 	clockMock := clock.NewMock()
 	runtimeConfigs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock)
+	buses := GenerateBuses(t, runtimeConfigs)
 
 	timeReminder(clockMock, 100*time.Millisecond)
 
 	// Create & start test pocket nodes
 	testChannel := make(modules.EventsChannel, 100)
-	pocketNodes := CreateTestConsensusPocketNodes(t, runtimeConfigs, testChannel)
+	pocketNodes := CreateTestConsensusPocketNodes(t, buses, testChannel)
 	StartAllTestPocketNodes(t, pocketNodes)
 
 	// Starting point
@@ -204,9 +206,9 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) {
 	for nodeId, pocketNode := range pocketNodes {
 		nodeState := GetConsensusNodeState(pocketNode)
 		if nodeId == leaderId {
-			require.Equal(t, uint8(consensus.Prepare), nodeState.Step)
+			require.Equal(t, consensus.Prepare.String(), typesCons.HotstuffStep(nodeState.Step).String())
 		} else {
-			require.Equal(t, uint8(consensus.PreCommit), nodeState.Step)
+			require.Equal(t, consensus.PreCommit.String(), typesCons.HotstuffStep(nodeState.Step).String())
 		}
 		require.Equal(t, uint64(3), nodeState.Height)
 		require.Equal(t, uint8(6), nodeState.Round)