Skip to content

Commit 87a6060

Browse files
deblasisOlshansk
deblasis
and
Olshansk
authored
[Shared] Config and genesis handling refactoring idea (#235)
## Description This PR is a way to convey a design idea for the refactoring of the way we handle `config` and `genesis` as highlighted in a couple of Protocol Hours and also summarized, along with more context that should help us making the right decisions, into the discussion #234 ## Issue There's no issue per-se, this is basically TECHDEBT handling I guess. Let me know if I should create one. ## Type of change Please mark the options that are relevant. - [x] New feature, functionality or library - [x] Code health or cleanup ## List of changes - Abstracted config and genesis handling - Mockable runtime - Refactored all modules to use RuntimeMgr - Updated RuntimeMgr to manage clock as well - Modules now accept `interfaces` instead of paths. - Unmarshalling is done in a new `runtime` package (runtime because what we do in there affects the runtime of the application) - We are now able to accept configuration via environment variables (thanks to @okdas for inspiration and [sp13 for Viper]("github.com/spf13/viper")) ## Testing - [x] `make test_all` - [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md) w/ all of the steps outlined in the `README` ## 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] If applicable, I have made corresponding changes to related local or global README - [x] If applicable, I have added new diagrams using [mermaid.js](https://mermaid-js.github.io) - [x] If applicable, I have added tests that prove my fix is effective or that my feature works * refactor(Config): runtime module * style(Client): consistency * fix(Config): make configPath and genesisPath available in Base * style(Persistence): Persistance -> Persistence * fix(Config): Base init * feat(Config): viper config parsing + environment variables * fix(Config): filename handling * refactor(Config): runtime module and InitializableModule interface * feat(Config): useRandomPK in runtime * refactor(Config): renamed Builder / updated signatures * style(Config): builder -> runtime * feat(Config): implement InitlializableModule in all modules * fix(Consensus): remove pacemakerConfig from interface * feat(Config): params/flags injection (stab @Private key for CLI) * style(Client): cleanup * feat(Shared): ConfigurableModule and GenesisDependentModule 👀 * chore(go.mod): tidy * feat(Shared): KeyholderModule 👀 * fix(Shared): Enforcing for all modules * refactor(Runtime): renamed b to rc * style(Shared): renamed moduleCfg to moduleNameCfg * fix(SHARED): build error for importing unnecessary package * Update shared/modules/runtime_module.go Co-authored-by: Daniel Olshansky <[email protected]> * refactor(runtime): renamed RuntimeConfig -> runtimeConfig * fix(runtime): improved WithRandomPK() * docs(runtime): // RuntimeConfig option helpers comment * refactor(Shared): nodeModule, P2PAddressableModule * refactor(Consensus): interface embedding * refactor(Consensus): consistency in naming * refactor(Persistence): consistence in naming * feat(P2P): P2P module now implements ConfigurableModule * fix(Persistence): improved DEBUG_CLEAR_STATE * feat(Shared): modules implement ConfigurableModule * refactor(Shared): consistency: adding config *structType in modules * Update consensus/module.go Co-authored-by: Daniel Olshansky <[email protected]> * fix(Runtime): fix WithPK for when called with a nil P2PConfig * fix(Shared): bus creation via runtime * refactor(Shared): NewWithAddress -> NewNodeWithAddress * refactor(Shared): improved Create, making all modules unexported * refactor(Consensus): consistency in naming paceMaker * refactor(Shared): runtime -> runtimeCfg * refactor(Shared): code review feedback * fix(Shared): bugfixes * fix(Consensus): merge fix * test(Consensus): fixed tests (StoreBlock and InsertBlock mocks cfg) * fix(Shared): removed config and genesis from bus * style(Runtime): cleanup * style(Runtime): import alias * fix(Shared): module creation failures returns err * refactor(Persistence): reverted converters to shared * feat(Tooling): check_cross_module_imports makefile target * style(Runtime): import names * refactor(Shared): refactored clock to be sourced from runtimeMgr * fix(Makefile): excluding makefile! * feat(Tooling): improved check_cross_module_imports * docs(Shared): Updated README and CHANGELOGs * docs(Runtime): missing items * test(Persistence): Tests: added missing mock configuration Closes 262 * fix(Tooling): execute tests sequentially fix (wrong syntax) * style(Makefile): check_cross_module_imports * Update app/client/main.go Co-authored-by: Daniel Olshansky <[email protected]> * refactor(Consensus): keys -> validatorKeys * Update consensus/consensus_tests/utils_test.go Co-authored-by: Daniel Olshansky <[email protected]> * Update consensus/consensus_tests/utils_test.go Co-authored-by: Daniel Olshansky <[email protected]> * refactor(Consensu): NewNodeWithAddress -> NewNodeWithP2PAddress * fix(Makefile): revert silent * style(Shared): init var before return * Update shared/test_artifacts/generator.go Co-authored-by: Daniel Olshansky <[email protected]> * Update utility/test/module_test.go Co-authored-by: Daniel Olshansky <[email protected]> * Update consensus/module.go Co-authored-by: Daniel Olshansky <[email protected]> * Update consensus/types/converters.go Co-authored-by: Daniel Olshansky <[email protected]> * refactor(Consensus): s/ActorListToMap/ActorListToValidatorMap * Update consensus/types/converters.go Co-authored-by: Daniel Olshansky <[email protected]> * fix(Consensus): converters fix * refactor(Shared): preallocations * docs(Shared): nits * style(Persistence): cleanup * Update shared/modules/module.go Co-authored-by: Daniel Olshansky <[email protected]> * fix(Telemetry): error management * Update shared/modules/module.go Co-authored-by: Daniel Olshansky <[email protected]> * style(P2P): renamed p2pCfg * fix(Tooling): reverted a line change introduced in #282 It was causing test failures because of inconsistent Postgresql state * refactor(Shared): modules use interfaces for config and genesis * refactor(Shared): using interfaces vs structs * style(Shared): formatting Goland is the offender... weird format on save rules... investigate * style(Consensus): cleanup * docs(Runtime): NewManagerFromReaders comment * refactor(Shared): shared.test_artifacts -> runtime.test_artifacts * fix(Persistence): leftover casts * refactor(Shared): removed NodeModule Signed-off-by: Alessandro De Blasis <[email protected]> Co-authored-by: Daniel Olshansky <[email protected]>
1 parent 2c390c3 commit 87a6060

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+1401
-1736
lines changed

‎Makefile

+22
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,25 @@ gen_genesis_and_config:
394394
## Clear the genesis and config files for LocalNet
395395
clear_genesis_and_config:
396396
rm build/config/gen.*.json
397+
398+
.PHONY: check_cross_module_imports
399+
## Lists cross-module imports
400+
check_cross_module_imports:
401+
$(eval exclude_common=--exclude=Makefile --exclude-dir=shared --exclude-dir=app --exclude-dir=runtime)
402+
echo "persistence:\n"
403+
grep ${exclude_common} --exclude-dir=persistence -r "github.com/pokt-network/pocket/persistence" || echo "✅ OK!"
404+
echo "-----------------------"
405+
echo "utility:\n"
406+
grep ${exclude_common} --exclude-dir=utility -r "github.com/pokt-network/pocket/utility" || echo "✅ OK!"
407+
echo "-----------------------"
408+
echo "consensus:\n"
409+
grep ${exclude_common} --exclude-dir=consensus -r "github.com/pokt-network/pocket/consensus" || echo "✅ OK!"
410+
echo "-----------------------"
411+
echo "telemetry:\n"
412+
grep ${exclude_common} --exclude-dir=telemetry -r "github.com/pokt-network/pocket/telemetry" || echo "✅ OK!"
413+
echo "-----------------------"
414+
echo "p2p:\n"
415+
grep ${exclude_common} --exclude-dir=p2p -r "github.com/pokt-network/pocket/p2p" || echo "✅ OK!"
416+
echo "-----------------------"
417+
echo "runtime:\n"
418+
grep ${exclude_common} --exclude-dir=runtime -r "github.com/pokt-network/pocket/runtime" || echo "✅ OK!"

‎app/client/main.go

+13-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"log"
77
"os"
88

9+
"github.com/pokt-network/pocket/runtime"
910
"github.com/pokt-network/pocket/shared/debug"
1011
"github.com/pokt-network/pocket/telemetry"
1112

@@ -47,24 +48,32 @@ var consensusMod modules.ConsensusModule
4748

4849
func main() {
4950
var err error
50-
consensusMod, err = consensus.Create(defaultConfigPath, defaultGenesisPath, true) // TECHDEBT: extra param required for injecting private key hack for debug client
51+
52+
runtimeMgr := runtime.NewManagerFromFiles(defaultConfigPath, defaultGenesisPath, runtime.WithRandomPK())
53+
54+
consM, err := consensus.Create(runtimeMgr)
5155
if err != nil {
5256
log.Fatalf("[ERROR] Failed to create consensus module: %v", err.Error())
5357
}
54-
p2pMod, err = p2p.Create(defaultConfigPath, defaultGenesisPath, true) // TECHDEBT: extra param required for injecting private key hack for debug client
58+
consensusMod = consM.(modules.ConsensusModule)
59+
60+
p2pM, err := p2p.Create(runtimeMgr)
5561
if err != nil {
5662
log.Fatalf("[ERROR] Failed to create p2p module: %v", err.Error())
5763
}
64+
p2pMod = p2pM.(modules.P2PModule)
65+
5866
// This telemetry module instance is a NOOP because the 'enable_telemetry' flag in the `cfg` above is set to false.
5967
// Since this client mimics partial - networking only - functionality of a full node, some of the telemetry-related
6068
// code paths are executed. To avoid those messages interfering with the telemetry data collected, a non-nil telemetry
6169
// module that NOOPs (per the configs above) is injected.
62-
telemetryMod, err := telemetry.Create(defaultConfigPath, defaultGenesisPath)
70+
telemetryM, err := telemetry.Create(runtimeMgr)
6371
if err != nil {
6472
log.Fatalf("[ERROR] Failed to create NOOP telemetry module: " + err.Error())
6573
}
74+
telemetryMod := telemetryM.(modules.TelemetryModule)
6675

67-
_ = shared.CreateBusWithOptionalModules(nil, p2pMod, nil, consensusMod, telemetryMod)
76+
_ = shared.CreateBusWithOptionalModules(runtimeMgr, nil, p2pMod, nil, consensusMod, telemetryMod)
6877

6978
p2pMod.Start()
7079

‎build/config/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"fmt"
77
"io/ioutil"
88

9-
"github.com/pokt-network/pocket/shared/test_artifacts"
9+
"github.com/pokt-network/pocket/runtime/test_artifacts"
1010
)
1111

1212
// Utility to generate config and genesis files

‎consensus/CHANGELOG.md

+9
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [0.0.0.6] - 2022-10-12
11+
12+
### [#235](https://github.com/pokt-network/pocket/pull/235) Config and genesis handling
13+
14+
- Updated to use `RuntimeMgr`
15+
- Made `ConsensusModule` struct unexported
16+
- Updated tests and mocks
17+
- Removed some cross-module dependencies
18+
1019
## [0.0.0.5] - 2022-10-06
1120

1221
- Don't ignore the exit code of `m.Run()` in the unit tests

‎consensus/block.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"github.com/pokt-network/pocket/shared/codec"
99
)
1010

11-
func (m *ConsensusModule) commitBlock(block *typesCons.Block) error {
11+
func (m *consensusModule) commitBlock(block *typesCons.Block) error {
1212
m.nodeLog(typesCons.CommittingBlock(m.Height, len(block.Transactions)))
1313

1414
// Store the block in the KV store
@@ -41,7 +41,7 @@ func (m *ConsensusModule) commitBlock(block *typesCons.Block) error {
4141
return nil
4242
}
4343

44-
func (m *ConsensusModule) storeBlock(block *typesCons.Block, blockProtoBytes []byte) error {
44+
func (m *consensusModule) storeBlock(block *typesCons.Block, blockProtoBytes []byte) error {
4545
store := m.utilityContext.GetPersistenceContext()
4646
// Store in KV Store
4747
if err := store.StoreBlock(blockProtoBytes); err != nil {
@@ -57,7 +57,7 @@ func (m *ConsensusModule) storeBlock(block *typesCons.Block, blockProtoBytes []b
5757
}
5858

5959
// TODO: Add unit tests specific to block validation
60-
func (m *ConsensusModule) validateBlockBasic(block *typesCons.Block) error {
60+
func (m *consensusModule) validateBlockBasic(block *typesCons.Block) error {
6161
if block == nil && m.Step != NewRound {
6262
return typesCons.ErrNilBlock
6363
}
@@ -66,8 +66,8 @@ func (m *ConsensusModule) validateBlockBasic(block *typesCons.Block) error {
6666
return typesCons.ErrBlockExists
6767
}
6868

69-
if block != nil && unsafe.Sizeof(*block) > uintptr(m.consGenesis.MaxBlockBytes) {
70-
return typesCons.ErrInvalidBlockSize(uint64(unsafe.Sizeof(*block)), m.consGenesis.MaxBlockBytes)
69+
if block != nil && unsafe.Sizeof(*block) > uintptr(m.consGenesis.GetMaxBlockBytes()) {
70+
return typesCons.ErrInvalidBlockSize(uint64(unsafe.Sizeof(*block)), m.consGenesis.GetMaxBlockBytes())
7171
}
7272

7373
// If the current block being processed (i.e. voted on) by consensus is non nil, we need to make
@@ -84,7 +84,7 @@ func (m *ConsensusModule) validateBlockBasic(block *typesCons.Block) error {
8484
}
8585

8686
// Creates a new Utility context and clears/nullifies any previous contexts if they exist
87-
func (m *ConsensusModule) refreshUtilityContext() error {
87+
func (m *consensusModule) refreshUtilityContext() error {
8888
// Catch-all structure to release the previous utility context if it wasn't properly cleaned up.
8989
// Ideally, this should not be called.
9090
if m.utilityContext != nil {

‎consensus/consensus_tests/hotstuff_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ import (
1313
)
1414

1515
func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) {
16+
clockMock := clock.NewMock()
1617
// Test configs
1718
numNodes := 4
18-
configs, genesisStates := GenerateNodeConfigs(t, numNodes)
19+
runtimeMgrs := GenerateNodeRuntimeMgrs(t, numNodes, clockMock)
1920

20-
clockMock := clock.NewMock()
2121
go timeReminder(clockMock, 100*time.Millisecond)
2222

2323
// Create & start test pocket nodes
2424
testChannel := make(modules.EventsChannel, 100)
25-
pocketNodes := CreateTestConsensusPocketNodes(t, configs, genesisStates, clockMock, testChannel)
25+
pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel)
2626
StartAllTestPocketNodes(t, pocketNodes)
2727

2828
// Debug message to start consensus by triggering first view change

‎consensus/consensus_tests/pacemaker_test.go

+14-9
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,17 @@ func TestTinyPacemakerTimeouts(t *testing.T) {
2424
// Test configs
2525
numNodes := 4
2626
paceMakerTimeoutMsec := uint64(50) // Set a very small pacemaker timeout
27-
paceMakerTimeout := 50 * timePkg.Millisecond
28-
configs, genesisStates := GenerateNodeConfigs(t, numNodes)
29-
for _, config := range configs {
30-
config.Consensus.GetPaceMakerConfig().SetTimeoutMsec(paceMakerTimeoutMsec)
27+
paceMakerTimeout := 50 * time.Millisecond
28+
runtimeMgrs := GenerateNodeRuntimeMgrs(t, numNodes, clockMock)
29+
for _, runtimeConfig := range runtimeMgrs {
30+
if consCfg, ok := runtimeConfig.GetConfig().GetConsensusConfig().(consensus.HasPacemakerConfig); ok {
31+
consCfg.GetPacemakerConfig().SetTimeoutMsec(paceMakerTimeoutMsec)
32+
}
3133
}
3234

3335
// Create & start test pocket nodes
3436
testChannel := make(modules.EventsChannel, 100)
35-
pocketNodes := CreateTestConsensusPocketNodes(t, configs, genesisStates, clockMock, testChannel)
37+
pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel)
3638
StartAllTestPocketNodes(t, pocketNodes)
3739

3840
// Debug message to start consensus by triggering next view.
@@ -123,15 +125,15 @@ func TestTinyPacemakerTimeouts(t *testing.T) {
123125
}
124126

125127
func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) {
128+
clockMock := clock.NewMock()
126129
numNodes := 4
127-
configs, genesisStates := GenerateNodeConfigs(t, numNodes)
130+
runtimeConfigs := GenerateNodeRuntimeMgrs(t, numNodes, clockMock)
128131

129-
clockMock := clock.NewMock()
130132
timeReminder(clockMock, 100*time.Millisecond)
131133

132134
// Create & start test pocket nodes
133135
testChannel := make(modules.EventsChannel, 100)
134-
pocketNodes := CreateTestConsensusPocketNodes(t, configs, genesisStates, clockMock, testChannel)
136+
pocketNodes := CreateTestConsensusPocketNodes(t, runtimeConfigs, testChannel)
135137
StartAllTestPocketNodes(t, pocketNodes)
136138

137139
// Starting point
@@ -143,13 +145,16 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) {
143145
leader := pocketNodes[leaderId]
144146
leaderRound := uint64(6)
145147

148+
consensusPK, err := leader.GetBus().GetConsensusModule().GetPrivateKey()
149+
require.NoError(t, err)
150+
146151
// Placeholder block
147152
blockHeader := &typesCons.BlockHeader{
148153
Height: int64(testHeight),
149154
Hash: hex.EncodeToString(appHash),
150155
NumTxs: 0,
151156
LastBlockHash: "",
152-
ProposerAddress: leader.Address.Bytes(),
157+
ProposerAddress: consensusPK.Address(),
153158
QuorumCertificate: nil,
154159
}
155160
block := &typesCons.Block{

0 commit comments

Comments
 (0)