[Utility] Session Interface - GetSession Implementation (1st iteration)

Makefile

@@ -436,8 +436,10 @@ benchmark_p2p_peerstore: ## Run P2P peerstore benchmarks
 
 ### Inspired by @goldinguy_ in this post: https://goldin.io/blog/stop-using-todo ###
 # TODO          - General Purpose catch-all.
+# DECIDE        - A TODO indicating we need to make a decision and document it using an ADR in the future; https://github.com/pokt-network/pocket-network-protocol/tree/main/ADRs
 # TECHDEBT      - Not a great implementation, but we need to fix it later.
 # IMPROVE       - A nice to have, but not a priority. It's okay if we never get to this.
+# OPTIMIZE      - An opportunity for performance improvement if/when it's necessary
 # DISCUSS       - Probably requires a lengthy offline discussion to understand next steps.
 # INCOMPLETE    - A change which was out of scope of a specific PR but needed to be documented.
 # INVESTIGATE   - TBD what was going on, but needed to continue moving and not get distracted.
@@ -453,7 +455,7 @@ benchmark_p2p_peerstore: ## Run P2P peerstore benchmarks
 # BUG           - There is a known existing bug in this code
 # DISCUSS_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way for the reviewer of a PR to start / reply to a discussion.
 # TODO_IN_THIS_COMMIT    - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to start the review process while non-critical changes are still in progress
-TODO_KEYWORDS = -e "TODO" -e "TECHDEBT" -e "IMPROVE" -e "DISCUSS" -e "INCOMPLETE" -e "INVESTIGATE" -e "CLEANUP" -e "HACK" -e "REFACTOR" -e "CONSIDERATION" -e "TODO_IN_THIS_COMMIT" -e "DISCUSS_IN_THIS_COMMIT" -e "CONSOLIDATE" -e "DEPRECATE" -e "ADDTEST" -e "RESEARCH" -e "BUG"
+TODO_KEYWORDS = -e "TODO" -e "DECIDE" -e "TECHDEBT" -e "IMPROVE" -e "OPTIMIZE" -e "DISCUSS" -e "INCOMPLETE" -e "INVESTIGATE" -e "CLEANUP" -e "HACK" -e "REFACTOR" -e "CONSIDERATION" -e "TODO_IN_THIS_COMMIT" -e "DISCUSS_IN_THIS_COMMIT" -e "CONSOLIDATE" -e "DEPRECATE" -e "ADDTEST" -e "RESEARCH" -e "BUG"
 
 # How do I use TODOs?
 # 1. <KEYWORD>: <Description of follow up work>;
@@ -470,6 +472,13 @@ TODO_KEYWORDS = -e "TODO" -e "TECHDEBT" -e "IMPROVE" -e "DISCUSS" -e "INCOMPLETE
 todo_list: ## List all the TODOs in the project (excludes vendor and prototype directories)
 	grep --exclude-dir={.git,vendor,prototype} -r ${TODO_KEYWORDS}  .
 
+
+TODO_SEARCH ?= $(shell pwd)
+
+.PHONY: todo_search
+todo_search: ## List all the TODOs in a specific directory specific by `TODO_SEARCH`
+	grep --exclude-dir={.git,vendor,prototype} -r ${TODO_KEYWORDS} ${TODO_SEARCH}
+
 .PHONY: todo_count
 todo_count: ## Print a count of all the TODOs in the project
 	grep --exclude-dir={.git,vendor,prototype} -r ${TODO_KEYWORDS} . | wc -l

build/config/genesis.json

@@ -4120,7 +4120,7 @@
     }
   ],
   "params": {
-    "blocks_per_session": 4,
+    "blocks_per_session": 1,
     "app_minimum_stake": "15000000000",
     "app_max_chains": 15,
     "app_session_tokens_multiplier": 100,
@@ -4138,6 +4138,7 @@
     "fisherman_unstaking_blocks": 2016,
     "fisherman_minimum_pause_blocks": 4,
     "fisherman_max_pause_blocks": 672,
+    "fisherman_per_session": 1,
     "validator_minimum_stake": "15000000000",
     "validator_unstaking_blocks": 2016,
     "validator_minimum_pause_blocks": 4,
@@ -4192,6 +4193,7 @@
     "fisherman_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "fisherman_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "fisherman_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
+    "fisherman_per_session_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",

build/config/genesis_localhost.json

@@ -159,6 +159,7 @@
     "fisherman_unstaking_blocks": 2016,
     "fisherman_minimum_pause_blocks": 4,
     "fisherman_max_pause_blocks": 672,
+    "fisherman_per_session": 1,
     "validator_minimum_stake": "15000000000",
     "validator_unstaking_blocks": 2016,
     "validator_minimum_pause_blocks": 4,
@@ -214,6 +215,7 @@
     "fisherman_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "fisherman_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "fisherman_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
+    "fisherman_per_session_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
     "validator_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",

build/docs/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.39] - 2023-04-28
+
+- Added a `fisherman_per_session` governance parameter
+- Updated the default `blocks_per_session` from `4` to `1`
+
 ## [0.0.0.38] - 2023-04-28
 
 - Removed unused `tmpDir` in `debug_keybase` package

build/localnet/manifests/configs.yaml

@@ -4124,7 +4124,7 @@ data:
         }
       ],
       "params": {
-        "blocks_per_session": 4,
+        "blocks_per_session": 1,
         "app_minimum_stake": "15000000000",
         "app_max_chains": 15,
         "app_session_tokens_multiplier": 100,
@@ -4142,6 +4142,7 @@ data:
         "fisherman_unstaking_blocks": 2016,
         "fisherman_minimum_pause_blocks": 4,
         "fisherman_max_pause_blocks": 672,
+        "fisherman_per_session": 1,
         "validator_minimum_stake": "15000000000",
         "validator_unstaking_blocks": 2016,
         "validator_minimum_pause_blocks": 4,
@@ -4196,6 +4197,7 @@ data:
         "fisherman_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
         "fisherman_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
         "fisherman_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
+        "fisherman_per_session_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
         "validator_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
         "validator_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",
         "validator_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45",

logger/docs/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.10] - 2023-04-28
+
+- Extracted a couple of shared helpers (e.g. `stringLogArrayMarshaler`, `MarshalZerologArray`)
+- Updated documentation on how to build a context specific logger
+
 ## [0.0.0.9] - 2023-02-28
 
 - Removed the unused `bus` from the `logger` struct

logger/docs/README.md

@@ -10,6 +10,7 @@
 - [Global Logging](#global-logging)
 - [Module Logging](#module-logging)
   - [Logger Initialization](#logger-initialization)
+- [Submodule / Subcontext Logging](#submodule--subcontext-logging)
 - [Accessing Logs](#accessing-logs)
   - [Grafana](#grafana)
   - [Example Queries](#example-queries)
@@ -128,6 +129,24 @@ func (m *sweetModule) Start() error {
 }
 ```
 
+## Submodule / Subcontext Logging
+
+A common helpful practice is to create a logger that can be easily filtered for within a specific context, such as a specific submodule, a function or a code path.
+
+```golang
+m.logger.With().Str("source", "contextName").Logger(),
+```
+
+For example:
+
+```golang
+
+func (m *Module) fooFunc() {
+  fooLogger := m.logger.With().Str("source", "fooFunc").Logger(),
+  // use fooLogger here
+}
+```
+
 ## Accessing Logs
 
 Logs are written to stdout. In LocalNet, Loki is used to capture log output. Logs can then be queried using [LogQL](https://grafana.com/docs/loki/latest/logql/) syntax. Grafana can be used to visualize the logs.

logger/utils.go

@@ -0,0 +1,17 @@
+package logger
+
+import "github.com/rs/zerolog"
+
+// stringLogArrayMarshaler implements the `zerolog.LogArrayMarshaler` interface
+// to marshal an array of strings for use with zerolog.
+type StringLogArrayMarshaler struct {
+	Strings []string
+}
+
+// MarshalZerologArray implements the respective `zerolog.LogArrayMarshaler`
+// interface member.
+func (marshaler StringLogArrayMarshaler) MarshalZerologArray(arr *zerolog.Array) {
+	for _, str := range marshaler.Strings {
+		arr.Str(str)
+	}
+}

p2p/CHANGELOG.md

@@ -7,7 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [0.0.0.47] - 2023-04-27
+## [0.0.0.49] - 2023-04-28
+
+- Extracted a couple of shared helpers (e.g. `stringLogArrayMarshaler`, `MarshalZerologArray`)
+
+## [0.0.0.48] - 2023-04-27
 
 - Renamed `Network` interface to `Router`
 - Shortened `Router#NetworkBroadcast` to `#Broadcast`
@@ -26,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Simplified p2p module/router config handoff
 - Updated debug logging
 
-## [0.0.0.46] - 2023-04-27
+## [0.0.0.47] - 2023-04-27
 
 - Removed unneeded `stdnetwork` package
 - Removed unneeded `use_rain_tree` P2P config field

p2p/utils/url_conversion.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/multiformats/go-multiaddr"
 	"github.com/pokt-network/pocket/logger"
-	"github.com/rs/zerolog"
 )
 
 const (
@@ -183,23 +182,8 @@ func getPeerIP(hostname string) (net.IP, error) {
 	logger.Global.Warn().Msg("resolved multiple addresses but only using one. See ticket #557 for more details")
 	logger.Global.Warn().
 		Str("hostname", hostname).
-		Array("resolved", stringLogArrayMarshaler{strs: addrs}).
+		Array("resolved", logger.StringLogArrayMarshaler{Strings: addrs}).
 		IPAddr("using", peerIP)
 
 	return peerIP, nil
 }
-
-// stringLogArrayMarshaler implements the `zerolog.LogArrayMarshaler` interface
-// to marshal an array of strings for use with zerolog.
-// TECHDEBT(#609): move & de-duplicate
-type stringLogArrayMarshaler struct {
-	strs []string
-}
-
-// MarshalZerologArray implements the respective `zerolog.LogArrayMarshaler`
-// interface member.
-func (marshaler stringLogArrayMarshaler) MarshalZerologArray(arr *zerolog.Array) {
-	for _, str := range marshaler.strs {
-		arr.Str(str)
-	}
-}

persistence/actor.go

@@ -1,10 +1,29 @@
 package persistence
 
 import (
+	"fmt"
+
 	"github.com/pokt-network/pocket/persistence/types"
 	coreTypes "github.com/pokt-network/pocket/shared/core/types"
 )
 
+func (p *PostgresContext) GetActor(actorType coreTypes.ActorType, address []byte, height int64) (*coreTypes.Actor, error) {
+	var schema types.ProtocolActorSchema
+	switch actorType {
+	case types.ApplicationActor.GetActorType():
+		schema = types.ApplicationActor
+	case types.ServicerActor.GetActorType():
+		schema = types.ServicerActor
+	case types.FishermanActor.GetActorType():
+		schema = types.FishermanActor
+	case types.ValidatorActor.GetActorType():
+		schema = types.FishermanActor
+	default:
+		return nil, fmt.Errorf("invalid actor type: %s", actorType)
+	}
+	return p.getActor(schema, address, height)
+}
+
 // TODO (#399): All of the functions below following a structure similar to `GetAll<Actor>`
 // can easily be refactored and condensed into a single function using a generic type or a common
 // interface.
@@ -113,7 +132,7 @@ func (p *PostgresContext) GetAllFishermen(height int64) (f []*coreTypes.Actor, e
 	return
 }
 
-// IMPROVE: This is a proof of concept. Ideally we should have a single query that returns all actors.
+// OPTIMIZE: There is an opportunity to have one SQL query returning all the actorsp
 func (p *PostgresContext) GetAllStakedActors(height int64) (allActors []*coreTypes.Actor, err error) {
 	type actorGetter func(height int64) ([]*coreTypes.Actor, error)
 	actorGetters := []actorGetter{p.GetAllValidators, p.GetAllServicers, p.GetAllFishermen, p.GetAllApps}

persistence/docs/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.50] - 2023-04-28
+
+- Implemented a new `GetActor` persistence modular functoin
+- Added `fisherman_per_session` parameter
+- Minor code and comment cleanup
+
 ## [0.0.0.49] - 2023-04-14
 
 - Index transactions in the `TxIndexer` by sender and recipient using the height and block index of the transactions

persistence/test/module_test.go

@@ -9,7 +9,8 @@ import (
 )
 
 func TestPersistenceContextParallelReadWrite(t *testing.T) {
-	prepareAndCleanContext(t)
+	clearAllState()
+	t.Cleanup(clearAllState)
 
 	// variables for testing
 	_, _, poolAddr := keygen.GetInstance().Next()
@@ -57,7 +58,8 @@ func TestPersistenceContextParallelReadWrite(t *testing.T) {
 }
 
 func TestPersistenceContextTwoWritesErrors(t *testing.T) {
-	prepareAndCleanContext(t)
+	clearAllState()
+	t.Cleanup(clearAllState)
 
 	// Opening up first write context succeeds
 	rwCtx1, err := testPersistenceMod.NewRWContext(0)
@@ -74,7 +76,8 @@ func TestPersistenceContextTwoWritesErrors(t *testing.T) {
 }
 
 func TestPersistenceContextSequentialWrites(t *testing.T) {
-	prepareAndCleanContext(t)
+	clearAllState()
+	t.Cleanup(clearAllState)
 
 	// Opening up first write context succeeds
 	writeContext1, err := testPersistenceMod.NewRWContext(0)
@@ -93,7 +96,8 @@ func TestPersistenceContextSequentialWrites(t *testing.T) {
 }
 
 func TestPersistenceContextMultipleParallelReads(t *testing.T) {
-	prepareAndCleanContext(t)
+	clearAllState()
+	t.Cleanup(clearAllState)
 
 	// Opening up first read context succeeds
 	readContext1, err := testPersistenceMod.NewReadContext(0)
@@ -111,10 +115,3 @@ func TestPersistenceContextMultipleParallelReads(t *testing.T) {
 	readContext2.Release()
 	readContext3.Release()
 }
-
-func prepareAndCleanContext(t *testing.T) {
-	// Cleanup context after the test
-	t.Cleanup(clearAllState)
-
-	clearAllState()
-}

persistence/test/setup_test.go

@@ -41,8 +41,8 @@ var (
 	StakeToUpdate        = utils.BigIntToString((&big.Int{}).Add(DefaultStakeBig, DefaultDeltaBig))
 
 	DefaultStakeStatus     = int32(coreTypes.StakeStatus_Staked)
-	DefaultPauseHeight     = int64(-1) // pauseHeight=-1 means not paused
-	DefaultUnstakingHeight = int64(-1) // pauseHeight=-1 means not unstaking
+	DefaultPauseHeight     = int64(-1) // pauseHeight=-1 implies not paused
+	DefaultUnstakingHeight = int64(-1) // unstakingHeight=-1 implies not unstaking
 
 	OlshanskyURL    = "https://olshansky.info"
 	OlshanskyChains = []string{"OLSH"}
@@ -53,8 +53,10 @@ var (
 	genesisStateNumServicers    = 1
 	genesisStateNumApplications = 1
 	genesisStateNumFishermen    = 1
+
+	// Initialized in TestMain
+	testPersistenceMod modules.PersistenceModule
 )
-var testPersistenceMod modules.PersistenceModule // initialized in TestMain
 
 // See https://github.com/ory/dockertest as reference for the template of this code
 // Postgres example can be found here: https://github.com/ory/dockertest/blob/v3/examples/PostgreSQL.md
@@ -360,6 +362,7 @@ func resetStateToGenesis() {
 	}
 }
 
+// TECHDEBT: Make sure all tests run `t.Cleanup(clearAllState)`
 // This is necessary for unit tests that are dependant on a completely clear state when starting
 func clearAllState() {
 	if err := testPersistenceMod.ReleaseWriteContext(); err != nil {

persistence/test/state_test.go

@@ -44,9 +44,9 @@ func TestStateHash_DeterministicStateWhenUpdatingAppStake(t *testing.T) {
 	// logic changes, these hashes will need to be updated based on the test output.
 	// TODO: Add an explicit updateSnapshots flag to the test to make this more clear.
 	stateHashes := []string{
-		"3a9a33c4d6b106f656c859296cd5ac16b608980d1d921f6de77051a707f48cb5",
-		"ec3d62106f1ca61dfe9c1d80a16c4ee3923550db5175389777bd1ecd9d50136e",
-		"e440914fba03bbb5ff4fbb73f760e4e75c3635263bbf7c15ca649870ee865222",
+		"4b5da068a4792c30a73c36d00f01af12262e9d753992f2d56d4ca64f3f2ac894",
+		"0565768b758981f511a71131a63710b9e43fa51095de658a1454f3b8b7895a15",
+		"f0796a7072c402e22abd0d2ef5c5a199aa5b6cb208824af24b58c4c777fc3284",
 	}
 
 	stakeAmount := initialStakeAmount