[TECHDEBT] Tackle TODOs in shared/modules Issue #232

Makefile

@@ -222,7 +222,7 @@ protogen_local: go_protoc-go-inject-tag
 	$(eval proto_dir = ".")
 	protoc --go_opt=paths=source_relative  -I=./shared/debug/proto        --go_out=./shared/debug       ./shared/debug/proto/*.proto        --experimental_allow_proto3_optional
 	protoc --go_opt=paths=source_relative  -I=./shared/codec/proto        --go_out=./shared/codec       ./shared/codec/proto/*.proto        --experimental_allow_proto3_optional
-	protoc --go_opt=paths=source_relative  -I=./shared/indexer/proto      --go_out=./shared/indexer/    ./shared/indexer/proto/*.proto      --experimental_allow_proto3_optional
+	protoc --go_opt=paths=source_relative  -I=./utility/indexer/proto     --go_out=./utility/indexer/   ./utility/indexer/proto/*.proto     --experimental_allow_proto3_optional
 	protoc --go_opt=paths=source_relative  -I=./persistence/proto         --go_out=./persistence/types  ./persistence/proto/*.proto         --experimental_allow_proto3_optional
 	protoc-go-inject-tag -input="./persistence/types/*.pb.go"
 	protoc --go_opt=paths=source_relative  -I=./utility/types/proto       --go_out=./utility/types      ./utility/types/proto/*.proto       --experimental_allow_proto3_optional

consensus/module.go

@@ -259,7 +259,7 @@ func (m *ConsensusModule) CurrentHeight() uint64 {
 	return m.Height
 }
 
-func (m *ConsensusModule) ValidatorMap() modules.ValidatorMap {
+func (m *ConsensusModule) ValidatorMap() modules.ValidatorMap { // TODO: This needs to be dynamically updated during various operations and network changes.
 	return typesCons.ValidatorMapToModulesValidatorMap(m.validatorMap)
 }
 

persistence/db.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log"
 
 	"github.com/pokt-network/pocket/persistence/types"
 
@@ -54,6 +55,27 @@ func (pg *PostgresContext) GetCtx() (context.Context, error) {
 	return context.TODO(), nil
 }
 
+func (pg *PostgresContext) ResetContext() error {
+	if pg == nil {
+		return nil
+	}
+	tx := pg.GetTx()
+	if tx == nil {
+		return nil
+	}
+	conn := tx.Conn()
+	if conn == nil {
+		return nil
+	}
+	if !conn.IsClosed() {
+		if err := pg.Release(); err != nil {
+			log.Println("[TODO][ERROR] Error releasing write context...", err)
+		}
+	}
+	pg.tx = nil
+	return nil
+}
+
 // TECHDEBT: Implement proper connection pooling
 func connectToDatabase(postgresUrl string, schema string) (*pgx.Conn, error) {
 	ctx := context.TODO()

persistence/debug.go

@@ -49,7 +49,7 @@ func (m *PersistenceModule) clearState(_ *debug.DebugMessage) {
 		log.Printf("Error creating new context: %s \n", err)
 		return
 	}
-	if err := context.(PostgresContext).DebugClearAll(); err != nil {
+	if err := context.(*PostgresContext).DebugClearAll(); err != nil {
 		log.Printf("Error clearing state: %s \n", err)
 		return
 	}

persistence/docs/CHANGELOG.md

@@ -6,12 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-
-## [0.0.0.6] - 2022-09-22
+## [0.0.0.6] - 2022-09-30
 
 - Removed no-op `DeleteActor` code
 - Consolidated `CHANGELOG`s into one under `persistence/docs`
 - Consolidated `README`s into one under `persistence/docs`
+- Deprecated `persMod.ResetContext()` for -> `persRWContext.ResetContext()` for more appropriate encapsulation
+- Added ticks to CHANGELOG.md
+- Removed reference to Utility Mod's `BigIntToString()` and used internal `BigIntToString()`
 
 ## [0.0.0.5] - 2022-08-25
 

persistence/gov.go

@@ -7,18 +7,23 @@ import (
 	"strconv"
 
 	"github.com/pokt-network/pocket/persistence/types"
-	"github.com/pokt-network/pocket/shared/modules"
+)
+
+// TODO : Deprecate these two constants when we change the persistenceRWContext interface to pass the `paramName`
+const (
+	BlocksPerSessionParamName       = "blocks_per_session"
+	ServiceNodesPerSessionParamName = "service_nodes_per_session"
 )
 
 // TODO (Team) BUG setting parameters twice on the same height causes issues. We need to move the schema away from 'end_height' and
 // more towards the height_constraint architecture
 
 func (p PostgresContext) GetBlocksPerSession(height int64) (int, error) {
-	return p.GetIntParam(modules.BlocksPerSessionParamName, height)
+	return p.GetIntParam(BlocksPerSessionParamName, height)
 }
 
 func (p PostgresContext) GetServiceNodesPerSessionAt(height int64) (int, error) {
-	return p.GetIntParam(modules.ServiceNodesPerSessionParamName, height)
+	return p.GetIntParam(ServiceNodesPerSessionParamName, height)
 }
 
 func (p PostgresContext) InitParams() error {

persistence/module.go

@@ -20,6 +20,8 @@ var _ modules.PersistenceRWContext = &PostgresContext{}
 var _ modules.PersistenceGenesisState = &types.PersistenceGenesisState{}
 var _ modules.PersistenceConfig = &types.PersistenceConfig{}
 
+// TODO: convert address and public key to string not bytes in all account and actor functions
+// TODO: remove address parameter from all pool operations
 type PersistenceModule struct {
 	bus modules.Bus
 
@@ -168,7 +170,7 @@ func (m *PersistenceModule) NewRWContext(height int64) (modules.PersistenceRWCon
 		blockstore: m.blockStore,
 	}
 
-	return *m.writeContext, nil
+	return m.writeContext, nil
 
 }
 
@@ -194,22 +196,14 @@ func (m *PersistenceModule) NewReadContext(height int64) (modules.PersistenceRea
 	}, nil
 }
 
-func (m *PersistenceModule) ResetContext() error {
-	if m.writeContext != nil {
-		if !m.writeContext.GetTx().Conn().IsClosed() {
-			if err := m.writeContext.Release(); err != nil {
-				log.Println("[TODO][ERROR] Error releasing write context...", err)
-			}
-		}
-		m.writeContext = nil
-	}
-	return nil
-}
-
 func (m *PersistenceModule) GetBlockStore() kvstore.KVStore {
 	return m.blockStore
 }
 
+func (m *PersistenceModule) NewWriteContext() modules.PersistenceRWContext {
+	return m.writeContext
+}
+
 func initializeBlockStore(blockStorePath string) (kvstore.KVStore, error) {
 	if blockStorePath == "" {
 		return kvstore.NewMemKVStore(), nil

persistence/test/gov_test.go

@@ -2,12 +2,18 @@ package test
 
 import (
 	"encoding/hex"
-	"github.com/pokt-network/pocket/shared/modules"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
+// TODO(#230): Remove these testing_artifacts when we nail down a design for parameter name sharing / owning
+const (
+	AppMaxChainsParamName            = "app_max_chains"
+	ServiceNodeMinimumStakeParamName = "service_node_minimum_stake"
+	ServiceNodeUnstakingBlocksOwner  = "service_node_unstaking_blocks_owner"
+)
+
 func TestInitParams(t *testing.T) {
 	db := NewTestPostgresContext(t, 0)
 	err := db.InitParams()
@@ -22,13 +28,13 @@ func TestGetSetIntParam(t *testing.T) {
 
 	newMaxChains := 42
 
-	err = db.SetParam(modules.AppMaxChainsParamName, newMaxChains)
+	err = db.SetParam(AppMaxChainsParamName, newMaxChains)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	maxChains, err := db.GetIntParam(modules.AppMaxChainsParamName, height)
+	maxChains, err := db.GetIntParam(AppMaxChainsParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newMaxChains, maxChains)
@@ -42,13 +48,13 @@ func TestGetSetStringParam(t *testing.T) {
 
 	newServiceNodeMinimumStake := "99999999"
 
-	err = db.SetParam(modules.ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake)
+	err = db.SetParam(ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	serviceNodeMinimumStake, err := db.GetStringParam(modules.ServiceNodeMinimumStakeParamName, height)
+	serviceNodeMinimumStake, err := db.GetStringParam(ServiceNodeMinimumStakeParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newServiceNodeMinimumStake, serviceNodeMinimumStake)
@@ -63,13 +69,13 @@ func TestGetSetByteArrayParam(t *testing.T) {
 	newOwner, err := hex.DecodeString("63585955783252764a6e576a5631647542486168426c63774e4655345a57617468545532637a6330516e4d5978575977674553537857644e4a6b4c7734575335416a65616c6d57494a47535364555933686d565a706e57564a6d6143526c54594248626864465a72646c624f646c59704a45536a6c6c52794d32527849545733566c6557464763745a465377466a57324a316157314562554a6c564b6c325470394753696c58544846474e786331567a70554d534a6c5335566d4c356f305157684663726c6b4e4a4e305931496c624a4e58537035554d4a7058564a705561506c3259484a47614b6c585a")
 	require.NoError(t, err)
 
-	err = db.SetParam(modules.ServiceNodeUnstakingBlocksOwner, newOwner)
+	err = db.SetParam(ServiceNodeUnstakingBlocksOwner, newOwner)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	owner, err := db.GetBytesParam(modules.ServiceNodeUnstakingBlocksOwner, height)
+	owner, err := db.GetBytesParam(ServiceNodeUnstakingBlocksOwner, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newOwner, owner)
@@ -84,27 +90,27 @@ func TestGetSetToggleIntFlag(t *testing.T) {
 	newMaxChains := 42
 
 	// insert with false
-	err = db.SetFlag(modules.AppMaxChainsParamName, newMaxChains, false)
+	err = db.SetFlag(AppMaxChainsParamName, newMaxChains, false)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	maxChains, enabled, err := db.GetIntFlag(modules.AppMaxChainsParamName, height)
+	maxChains, enabled, err := db.GetIntFlag(AppMaxChainsParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newMaxChains, maxChains)
 
 	require.Equal(t, false, enabled)
 
 	// toggle to true
-	err = db.SetFlag(modules.AppMaxChainsParamName, newMaxChains, true)
+	err = db.SetFlag(AppMaxChainsParamName, newMaxChains, true)
 	require.NoError(t, err)
 
 	height, err = db.GetHeight()
 	require.NoError(t, err)
 
-	maxChains, enabled, err = db.GetIntFlag(modules.AppMaxChainsParamName, height)
+	maxChains, enabled, err = db.GetIntFlag(AppMaxChainsParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newMaxChains, maxChains)
@@ -121,26 +127,26 @@ func TestGetSetToggleStringFlag(t *testing.T) {
 	newServiceNodeMinimumStake := "99999999"
 
 	// insert with false
-	err = db.SetFlag(modules.ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake, false)
+	err = db.SetFlag(ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake, false)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	serviceNodeMinimumStake, enabled, err := db.GetStringFlag(modules.ServiceNodeMinimumStakeParamName, height)
+	serviceNodeMinimumStake, enabled, err := db.GetStringFlag(ServiceNodeMinimumStakeParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newServiceNodeMinimumStake, serviceNodeMinimumStake)
 	require.Equal(t, false, enabled)
 
 	//toggle to true
-	err = db.SetFlag(modules.ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake, true)
+	err = db.SetFlag(ServiceNodeMinimumStakeParamName, newServiceNodeMinimumStake, true)
 	require.NoError(t, err)
 
 	height, err = db.GetHeight()
 	require.NoError(t, err)
 
-	serviceNodeMinimumStake, enabled, err = db.GetStringFlag(modules.ServiceNodeMinimumStakeParamName, height)
+	serviceNodeMinimumStake, enabled, err = db.GetStringFlag(ServiceNodeMinimumStakeParamName, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newServiceNodeMinimumStake, serviceNodeMinimumStake)
@@ -158,26 +164,26 @@ func TestGetSetToggleByteArrayFlag(t *testing.T) {
 	require.NoError(t, err)
 
 	// insert with false
-	err = db.SetFlag(modules.ServiceNodeUnstakingBlocksOwner, newOwner, false)
+	err = db.SetFlag(ServiceNodeUnstakingBlocksOwner, newOwner, false)
 	require.NoError(t, err)
 
 	height, err := db.GetHeight()
 	require.NoError(t, err)
 
-	owner, enabled, err := db.GetBytesFlag(modules.ServiceNodeUnstakingBlocksOwner, height)
+	owner, enabled, err := db.GetBytesFlag(ServiceNodeUnstakingBlocksOwner, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newOwner, owner)
 	require.Equal(t, false, enabled)
 
 	//toggle to true
-	err = db.SetFlag(modules.ServiceNodeUnstakingBlocksOwner, newOwner, true)
+	err = db.SetFlag(ServiceNodeUnstakingBlocksOwner, newOwner, true)
 	require.NoError(t, err)
 
 	height, err = db.GetHeight()
 	require.NoError(t, err)
 
-	owner, enabled, err = db.GetBytesFlag(modules.ServiceNodeUnstakingBlocksOwner, height)
+	owner, enabled, err = db.GetBytesFlag(ServiceNodeUnstakingBlocksOwner, height)
 	require.NoError(t, err)
 
 	require.Equal(t, newOwner, owner)

persistence/test/module_test.go

@@ -7,12 +7,9 @@ import (
 )
 
 func TestPersistenceContextParallelReadWrite(t *testing.T) {
-	// Cleanup previous contexts
-	testPersistenceMod.ResetContext()
 	t.Cleanup(func() {
-		testPersistenceMod.ResetContext()
+		require.NoError(t, testPersistenceMod.NewWriteContext().ResetContext())
 	})
-
 	// variables for testing
 	poolName := "fake"
 	poolAddress := []byte("address")
@@ -52,12 +49,9 @@ func TestPersistenceContextParallelReadWrite(t *testing.T) {
 }
 
 func TestPersistenceContextTwoWritesErrors(t *testing.T) {
-	// Cleanup previous contexts
-	testPersistenceMod.ResetContext()
 	t.Cleanup(func() {
-		testPersistenceMod.ResetContext()
+		require.NoError(t, testPersistenceMod.NewWriteContext().ResetContext())
 	})
-
 	// Opening up first write context succeeds
 	_, err := testPersistenceMod.NewRWContext(0)
 	require.NoError(t, err)

persistence/test/setup_test.go

@@ -66,15 +66,15 @@ func NewTestPostgresContext(t *testing.T, height int64) *persistence.PostgresCon
 	ctx, err := testPersistenceMod.NewRWContext(height)
 	require.NoError(t, err)
 
-	db, ok := ctx.(persistence.PostgresContext)
+	db, ok := ctx.(*persistence.PostgresContext)
 	require.True(t, ok)
 
 	t.Cleanup(func() {
 		require.NoError(t, db.Release())
-		require.NoError(t, testPersistenceMod.ResetContext())
+		require.NoError(t, db.ResetContext())
 	})
 
-	return &db
+	return db
 }
 
 func NewFuzzTestPostgresContext(f *testing.F, height int64) *persistence.PostgresContext {
@@ -83,7 +83,7 @@ func NewFuzzTestPostgresContext(f *testing.F, height int64) *persistence.Postgre
 		log.Fatalf("Error creating new context: %v\n", err)
 	}
 
-	db, ok := ctx.(persistence.PostgresContext)
+	db, ok := ctx.(*persistence.PostgresContext)
 	if !ok {
 		log.Fatalf("Error casting RW context to Postgres context")
 	}
@@ -92,12 +92,12 @@ func NewFuzzTestPostgresContext(f *testing.F, height int64) *persistence.Postgre
 		if err := db.Release(); err != nil {
 			f.FailNow()
 		}
-		if err := testPersistenceMod.ResetContext(); err != nil {
+		if err := db.ResetContext(); err != nil {
 			f.FailNow()
 		}
 	})
 
-	return &db
+	return db
 }
 
 // TODO(andrew): Take in `t testing.T` as a parameter and error if there's an issue