[TECHDEBT] Consolidate & encapsulate ValidatorMap logic - (Issue #203) (

#402)

## Description

## Issue

Fixes #203

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Refactored so that references to `modules.ValidatorMap` are
removed/internalized within `Consensus`
- [WIP] Make sure that the debug client can discover the validators in
`LocalNet`

## Testing

- [x] `make develop_test`
- [ ]
[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
- [x] 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](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]>

Makefile

@@ -130,7 +130,7 @@ rebuild_client_start: docker_check ## Rebuild and run a client daemon which is o
 
 .PHONY: client_connect
 client_connect: docker_check ## Connect to the running client debugging daemon
-	docker exec -it client /bin/bash -c "go run -tags=debug app/client/*.go debug"
+	docker exec -it client /bin/bash -c "POCKET_P2P_IS_CLIENT_ONLY=true go run -tags=debug app/client/*.go debug"
 
 .PHONY: build_and_watch
 build_and_watch: ## Continous build Pocket's main entrypoint as files change
@@ -224,11 +224,11 @@ mockgen: clean_mocks ## Use `mockgen` to generate mocks used for testing purpose
 	go generate ./${modules_dir}
 	echo "Mocks generated in ${modules_dir}/mocks"
 
-	$(eval p2p_types_dir = "p2p/types")
+	$(eval p2p_dir = "p2p")
 	$(eval p2p_type_mocks_dir = "p2p/types/mocks")
 	find ${p2p_type_mocks_dir} -type f ! -name "mocks.go" -exec rm {} \;
-	go generate ./${p2p_types_dir}
-	echo "P2P mocks generated in ${p2p_types_dir}/mocks"
+	go generate ./${p2p_dir}/...
+	echo "P2P mocks generated in ${p2p_type_mocks_dir}"
 
 # TODO(team): Tested locally with `protoc` version `libprotoc 3.19.4`. In the near future, only the Dockerfiles will be used to compile protos.
 

app/client/cli/debug.go

@@ -1,24 +1,18 @@
-//go:build debug
-// +build debug
-
 package cli
 
 import (
 	"log"
 	"os"
-	"sync"
 
 	"github.com/manifoldco/promptui"
-	"github.com/pokt-network/pocket/consensus"
-	"github.com/pokt-network/pocket/logger"
 	"github.com/pokt-network/pocket/p2p"
-	"github.com/pokt-network/pocket/rpc"
+	debugABP "github.com/pokt-network/pocket/p2p/providers/addrbook_provider/debug"
+	debugCHP "github.com/pokt-network/pocket/p2p/providers/current_height_provider/debug"
 	"github.com/pokt-network/pocket/runtime"
-	"github.com/pokt-network/pocket/shared"
+	coreTypes "github.com/pokt-network/pocket/shared/core/types"
 	pocketCrypto "github.com/pokt-network/pocket/shared/crypto"
 	"github.com/pokt-network/pocket/shared/messaging"
 	"github.com/pokt-network/pocket/shared/modules"
-	"github.com/pokt-network/pocket/telemetry"
 	"github.com/spf13/cobra"
 	"google.golang.org/protobuf/types/known/anypb"
 )
@@ -39,17 +33,18 @@ var (
 	// A P2P module is initialized in order to broadcast a message to the local network
 	p2pMod modules.P2PModule
 
-	// A consensus module is initialized in order to get a list of the validator network
-	consensusMod modules.ConsensusModule
-	modInitOnce  sync.Once
-
 	items = []string{
 		PromptResetToGenesis,
 		PromptPrintNodeState,
 		PromptTriggerNextView,
 		PromptTogglePacemakerMode,
 		PromptShowLatestBlockInStore,
 	}
+
+	// validators holds the list of the validators at genesis time so that we can use it to create a debug address book provider.
+	// Its purpose is to allow the CLI to "discover" the nodes in the network. Since currently we don't have churn and we run nodes only in LocalNet, we can rely on the genesis state.
+	// HACK(#416): This is a temporary solution that guarantees backward compatibility while we implement peer discovery
+	validators []*coreTypes.Actor
 )
 
 func init() {
@@ -62,7 +57,30 @@ func NewDebugCommand() *cobra.Command {
 		Short: "Debug utility for rapid development",
 		Args:  cobra.ExactArgs(0),
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
-			initDebug(remoteCLIURL)
+			var err error
+			runtimeMgr := runtime.NewManagerFromFiles(defaultConfigPath, defaultGenesisPath, runtime.WithRandomPK())
+
+			// HACK(#416): this is a temporary solution that guarantees backward compatibility while we implement peer discovery.
+			validators = runtimeMgr.GetGenesis().Validators
+
+			debugAddressBookProvider := debugABP.NewDebugAddrBookProvider(
+				runtimeMgr.GetConfig().P2P,
+				debugABP.WithActorsByHeight(
+					map[int64][]*coreTypes.Actor{
+						debugABP.ANY_HEIGHT: validators,
+					},
+				),
+			)
+
+			debugCurrentHeightProvider := debugCHP.NewDebugCurrentHeightProvider(0)
+
+			p2pM, err := p2p.CreateWithProviders(runtimeMgr, debugAddressBookProvider, debugCurrentHeightProvider)
+			if err != nil {
+				log.Fatalf("[ERROR] Failed to create p2p module: %v", err.Error())
+			}
+			p2pMod = p2pM.(modules.P2PModule)
+
+			p2pMod.Start()
 		},
 		RunE: runDebug,
 	}
@@ -148,8 +166,8 @@ func broadcastDebugMessage(debugMsg *messaging.DebugMessage) {
 	// address book of the actual validator nodes, so `node1.consensus` never receives the message.
 	// p2pMod.Broadcast(anyProto, messaging.PocketTopic_DEBUG_TOPIC)
 
-	for _, val := range consensusMod.ValidatorMap() {
-		addr, err := pocketCrypto.NewAddress(val.GetAddress())
+	for _, valAddr := range validators {
+		addr, err := pocketCrypto.NewAddress(valAddr.GetAddress())
 		if err != nil {
 			log.Fatalf("[ERROR] Failed to convert validator address into pocketCrypto.Address: %v", err)
 		}
@@ -165,58 +183,15 @@ func sendDebugMessage(debugMsg *messaging.DebugMessage) {
 	}
 
 	var validatorAddress []byte
-	for _, val := range consensusMod.ValidatorMap() {
-		validatorAddress, err = pocketCrypto.NewAddress(val.GetAddress())
-		if err != nil {
-			log.Fatalf("[ERROR] Failed to convert validator address into pocketCrypto.Address: %v", err)
-		}
-		break
+	if len(validators) == 0 {
+		log.Fatalf("[ERROR] No validators found")
 	}
 
-	p2pMod.Send(validatorAddress, anyProto)
-}
-
-func initDebug(remoteCLIURL string) {
-	modInitOnce.Do(func() {
-		var err error
-		runtimeMgr := runtime.NewManagerFromFiles(defaultConfigPath, defaultGenesisPath, runtime.WithRandomPK())
-
-		consM, err := consensus.Create(runtimeMgr)
-		if err != nil {
-			logger.Global.Fatal().Err(err).Msg("Failed to create consensus module")
-		}
-		consensusMod = consM.(modules.ConsensusModule)
-
-		p2pM, err := p2p.Create(runtimeMgr)
-		if err != nil {
-			log.Fatalf("[ERROR] Failed to create p2p module: %v", err.Error())
-		}
-		p2pMod = p2pM.(modules.P2PModule)
-
-		// This telemetry module instance is a NOOP because the 'enable_telemetry' flag in the `cfg` above is set to false.
-		// Since this client mimics partial - networking only - functionality of a full node, some of the telemetry-related
-		// code paths are executed. To avoid those messages interfering with the telemetry data collected, a non-nil telemetry
-		// module that NOOPs (per the configs above) is injected.
-		telemetryM, err := telemetry.Create(runtimeMgr)
-		if err != nil {
-			logger.Global.Fatal().Err(err).Msg("Failed to create telemetry module")
-		}
-		telemetryMod := telemetryM.(modules.TelemetryModule)
-
-		loggerM, err := logger.Create(runtimeMgr)
-		if err != nil {
-			logger.Global.Fatal().Err(err).Msg("Failed to create logger module")
-		}
-		loggerMod := loggerM.(modules.LoggerModule)
-
-		rpcM, err := rpc.Create(runtimeMgr)
-		if err != nil {
-			logger.Global.Fatal().Err(err).Msg("Failed to create rpc module")
-		}
-		rpcMod := rpcM.(modules.RPCModule)
-
-		_ = shared.CreateBusWithOptionalModules(runtimeMgr, nil, p2pMod, nil, consensusMod, telemetryMod, loggerMod, rpcMod) // REFACTOR: use the `WithXXXModule()` pattern accepting a slice of IntegratableModule
+	// if the message needs to be broadcast, it'll be handled by the business logic of the message handler
+	validatorAddress, err = pocketCrypto.NewAddress(validators[0].GetAddress())
+	if err != nil {
+		log.Fatalf("[ERROR] Failed to convert validator address into pocketCrypto.Address: %v", err)
+	}
 
-		p2pMod.Start()
-	})
+	p2pMod.Send(validatorAddress, anyProto)
 }

app/client/cli/doc/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.4] - 2023-01-09
+
+- 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
+- Simplified debug CLI initialization
+
 ## [0.0.0.3] - 2023-01-03
 
 - Updated to use `coreTypes` instead of utility types for `Actor` and `ActorType`

build/config/config1.json

@@ -25,7 +25,9 @@
     "consensus_port": 8080,
     "use_rain_tree": true,
     "is_empty_connection_type": false,
-    "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4"
+    "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4",
+    "max_mempool_count": 100000,
+    "is_client_only": false
   },
   "telemetry": {
     "enabled": true,

build/config/config2.json

@@ -25,7 +25,8 @@
     "consensus_port": 8080,
     "use_rain_tree": true,
     "is_empty_connection_type": false,
-    "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb"
+    "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb",
+    "max_mempool_count": 100000
   },
   "telemetry": {
     "enabled": true,

build/config/config3.json

@@ -25,7 +25,8 @@
     "consensus_port": 8080,
     "use_rain_tree": true,
     "is_empty_connection_type": false,
-    "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2"
+    "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2",
+    "max_mempool_count": 100000
   },
   "telemetry": {
     "enabled": true,

build/config/config4.json

@@ -25,7 +25,8 @@
     "consensus_port": 8080,
     "use_rain_tree": true,
     "is_empty_connection_type": false,
-    "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a"
+    "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a",
+    "max_mempool_count": 100000
   },
   "telemetry": {
     "enabled": true,

build/docs/CHANGELOG.md

@@ -7,11 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [0.0.0.1] - 2023-01-03
+## [0.0.0.2] - 2023-01-09
 
 - Removed `BaseConfig` from `configs`
 - Centralized `PersistenceGenesisState` and `ConsensusGenesisState` into `GenesisState`
 
+## [0.0.0.1] - 2022-12-29
+
+- Updated all `config*.json` files with the missing `max_mempool_count` value
+- Added `is_client_only` to `config1.json` so Viper knows it can be overridden. The config override is done in the Makefile's `client_connect` target. Setting this can be avoided if we merge the changes in https://github.com/pokt-network/pocket/compare/main...issue/cli-viper-environment-vars-fix
+
 ## [0.0.0.0] - 2022-12-22
 
 - Introduced this `CHANGELOG.md`

consensus/doc/CHANGELOG.md

@@ -7,11 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [0.0.0.16] - 2023-01-03
+## [0.0.0.16] - 2023-01-09
 
 - Added protobuf message definitions for requests related to sharing state sync metadata and blocks
 - Defined the interface for `StateSyncServerModule`, `StateSyncModule` (moving the old interface to `StateSyncModuleLEGACY` as a reference only)
 - Overhaul (updates, improvements, clarifications & additions) of the State Sync README
+- Removed `ValidatorMap() ValidatorMap`
 
 ## [0.0.0.15] - 2023-01-03
 

consensus/module.go

@@ -295,10 +295,6 @@ func (m *consensusModule) CurrentStep() uint64 {
 	return uint64(m.step)
 }
 
-func (m *consensusModule) ValidatorMap() modules.ValidatorMap { // TODO: This needs to be dynamically updated during various operations and network changes.
-	return typesCons.ValidatorMapToModulesValidatorMap(m.validatorMap)
-}
-
 // TODO: Populate the entire state from the persistence module: validator set, quorum cert, last block hash, etc...
 func (m *consensusModule) loadPersistedState() error {
 	persistenceContext, err := m.GetBus().GetPersistenceModule().NewReadContext(-1) // Unknown height

consensus/types/types.go

@@ -5,7 +5,6 @@ import (
 	"sort"
 
 	coreTypes "github.com/pokt-network/pocket/shared/core/types"
-	"github.com/pokt-network/pocket/shared/modules"
 )
 
 type NodeId uint64
@@ -42,14 +41,6 @@ func GetValAddrToIdMap(validatorMap ValidatorMap) (ValAddrToIdMap, IdToValAddrMa
 	return valToIdMap, idToValMap
 }
 
-func ValidatorMapToModulesValidatorMap(validatorMap ValidatorMap) (vm modules.ValidatorMap) {
-	vm = make(modules.ValidatorMap)
-	for _, v := range validatorMap {
-		vm[v.GetAddress()] = *v
-	}
-	return
-}
-
 func ActorListToValidatorMap(actors []*coreTypes.Actor) (m ValidatorMap) {
 	m = make(ValidatorMap, len(actors))
 	for _, a := range actors {

p2p/CHANGELOG.md

@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.16] - 2023-01-09
+
+- Added missing `Close()` call to `persistenceReadContext`
+
+## [0.0.0.15] - 2023-01-03
+
+- Refactored `AddrBookProvider` to support multiple implementations
+- Added `CurrentHeightProvider`
+- Dependency injection of the aforementioned provider into the module creation (used by the debug-client)
+- Updated implementation to use the providers
+- Updated tests and mocks
+
 ## [0.0.0.14] - 2023-01-03
 
 - `ActorsToAddrBook` now skips actors that are not validators since they don't have a serviceUrl generic parameter