[Persistence] First Implementation of the StateHash (#284) (#285)

## Description

The first implementation of the 

## Issue

Fixes #284 with follow up work in #361.

## Type of change

Please mark the relevant option(s):

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

## List of changes

### Persistence - Core Changes for SMT
* Introduced & defined for `block_persistence.proto`
    * A persistence specific protobuf for the Block stored in the BlockStore
* On `Commit`, prepare and store a persistence block in the KV Store, SQL Store
* Replace `IndexTransactions` (plural) to `IndexTransaction` (singular)
* Maintaining a list of StateTrees using Celestia’s SMT and badger as the KV store to compute the state hash
* Implemented `ComputeStateHash` to update the global state based on:
    * Validators
    * Applications
    * Servicers
    * Fisherman
    * Accounts
    * Pools
    * Transactions
    * Added a placeholder for `params` and `flags`
* Added a benchmarking and a determinism test suite to validate this

### Persistence - General module changes
* Implemented `GetAccountsUpdated`, `GetPoolsUpdated` and `GetActorsUpdated` functions
* Removed `GetPrevAppHash` and `indexTransactions` functions
* Removed `blockProtoBytes` and `txResults` from the local state and added `quorumCert`
* Consolidate all `resetContext` related operations into a single function
* Implemented `ReleaseWriteContext`
* Implemented ability to `ClearAllState` and `ResetToGenesis` for debugging & testing purposes
* Added unit tests for all of the supporting SQL functions implemented
* Some improvements in unit test preparation & cleanup (limited to this PR's functionality)

### Persistence - KVStore changes
* Renamed `Put` to `Set`
* Embedded `smt.MapStore` in the interface containing `Get`, `Set` and `Delete`
* Implemented `Delete`
* Modified `GetAll` to return both `keys` and `values`
* Turned off badger logging options since it’s noisy

### Persistence - Module Interface changes
* Removed `GetPrevHash` and just using `GetBlockHash` instead
* Removed `blockProtoBz` from `SetProposalBlock` interface
* Removed `GetLatestBlockTxs` and `SetLatestTxResults` in exchange for `IndexTransaction`
* Removed `SetTxResults`
* Renamed `UpdateAppHash` to `ComputeStateHash`
* Removed some getters related to the proposal block (`GetBlockTxs`, `GetBlockHash`, etc…)

### Consensus
* Propagate `highPrepareQC` if available to the block being created
* Remove `blockProtoBytes` from propagation in `SetProposalBlock`
* Guarantee that `writeContext` is released when refreshing the `utilityContext`
* Use `GetBlockHash(height)` instead of `GetPrevAppHash` to be more explicit
* Use the real `quorumCert` when preparing a new block

### Configs
* Updated the test generator to produce deterministic keys
* Added `trees_store_dir` to persistence configs
* Updated `LocalNet` configs to have an empty `tx_indexer_path` and `trees_store_dir`

### Makefile changes
* Added `db_cli_node`
* Added `db_show_schemas`
* Added `test_persistence_state_hash`
* Added `benchmark_persistence_state_hash`

### Debug
* `ResetToGenesis` - Added the ability to reset the state to genesis
* `ClearState` - Added the ability to clear the state completely (height 0 without genesis data)
## Testing

**New:**
- [x] `make benchmark_persistence_state_hash`
- [x] `make test_persistence_state_hash`
- [x] Iteration 3 demo was done using this PR: https://drive.google.com/file/d/1IOrzq-XJP04BJjyqPPpPu873aSfwrnur/view?usp=sharing

**Existing:**
- [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
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist
- [ ] 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)

--- 

Co-authored-by: Alessandro De Blasis <alex@deblasis.net>
Co-authored-by: Irving A.J. Rivas Z. <axel.rivas@gmail.com>
Co-authored-by: Andrew Nguyen <amnguyen1@mail.usf.edu>
Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>
Co-authored-by: Jason You <Jasonyou1995@users.noreply.github.com>
Co-authored-by: Dmitry Knyazev <okdas@users.noreply.github.com>

.github/workflows/main.yml

@@ -50,33 +50,50 @@ jobs:
         run: make install_cli_deps
       - name: generate protobufs, RPC server, RPC client and mocks
         run: make protogen_local && make mockgen && make generate_rpc_openapi
-      - name: run all tests
-        run: make test_all_with_json
+      - name: Create coverage report and run tests
+        # Not utilizing makefile target here to make use of pipefail bash feature.
+        run: |
+          set -euo pipefail
+          go test -p 1 -json ./... -covermode=count -coverprofile=coverage.out 2>&1 | tee test_results.json
+      - name: Output test failures
+        # Makes it easier to find failed tests so no need to scroll through the whole log.
+        if: ${{ failure() && env.TARGET_GOLANG_VERSION == matrix.go }}
+        run: cat test_results.json | jq 'select(.Action == "fail")'
+      - name: Upload test results
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-results
+          path: |
+            test_results.json
       - name: Annotate tests on GitHub
         # Only annotate if the test failed on target version to avoid duplicated annotations on GitHub.
         if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
         uses: guyarb/golang-test-annotations@v0.5.1
         with:
           test-results: test_results.json
+      - name: Prepare code coverage report
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
+        run: go tool cover -func=coverage.out -o=coverage.out
+      - name: Upload coverage to Codecov
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
+        uses: codecov/codecov-action@v3
       - name: Run golangci-lint
         # Only run if the test failed on target version to avoid duplicated annotations on GitHub.
         if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
         uses: golangci/golangci-lint-action@v3
         with:
           # only-new-issues: true
           args: --issues-exit-code=0 # TODO: Remove this once we fix all the issues.
-      - name: create coverage report
-        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
-        run: make test_all_with_coverage
-      - name: Upload coverage to Codecov
-        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
-        uses: codecov/codecov-action@v3
 
   # TODO(@okdas): reuse artifacts built by the previous job instead
   # of going through the build process in container build job again
   # - figure out how to handle musl/alpine case if we want to support it
   build-images:
     runs-on: ubuntu-latest
+    needs: test-multiple-go-versions
+    # Until we have developer environments, we don't need the images built on other that main branches.
+    if: github.ref == 'refs/heads/main'
     strategy:
       matrix:
         # Build dev & prod images

Makefile

@@ -8,6 +8,8 @@ CWD ?= CURRENT_WORKING_DIRECTIONRY_NOT_SUPPLIED
 #		                        seconds, and fail if any additional messages are received.
 EXTRA_MSG_FAIL ?= false
 
+# IMPROVE: Add `-shuffle=on` to the `go test` command to randomize the order in which tests are run.
+
 # An easy way to turn off verbose test output for some of the test targets. For example
 #  `$ make test_persistence` by default enables verbose testing
 #  `VERBOSE_TEST="" make test_persistence` is an easy way to run the same tests without verbose output
@@ -162,6 +164,14 @@ db_cli:
 	echo "View schema by running 'SELECT schema_name FROM information_schema.schemata;'"
 	docker exec -it pocket-db bash -c "psql -U postgres"
 
+psqlSchema ?= node1
+
+.PHONY: db_cli_node
+## Open a CLI to the local containerized postgres instance for a specific node
+db_cli_node:
+	echo "View all avialable tables by running \dt"
+	docker exec -it pocket-db bash -c "PGOPTIONS=--search_path=${psqlSchema} psql -U postgres"
+
 .PHONY: db_drop
 ## Drop all schemas used for LocalNet development matching `node%`
 db_drop: docker_check
@@ -177,6 +187,11 @@ db_bench_init: docker_check
 db_bench: docker_check
 	docker exec -it pocket-db bash -c "pgbench -U postgres -d postgres"
 
+.PHONY: db_show_schemas
+## Show all the node schemas in the local SQL DB
+db_show_schemas: docker_check
+	docker exec -it pocket-db bash -c "psql -U postgres -d postgres -a -f /tmp/scripts/show_all_schemas.sql"
+
 .PHONY: db_admin
 ## Helper to access to postgres admin GUI interface
 db_admin:
@@ -252,7 +267,7 @@ protogen_local: go_protoc-go-inject-tag
 	protoc --go_opt=paths=source_relative  -I=./p2p/types/proto           --go_out=./p2p/types          	./p2p/types/proto/*.proto           --experimental_allow_proto3_optional
 	protoc --go_opt=paths=source_relative  -I=./telemetry/proto           --go_out=./telemetry          	./telemetry/proto/*.proto           --experimental_allow_proto3_optional
 	protoc --go_opt=paths=source_relative  -I=./logger/proto              --go_out=./logger             	./logger/proto/*.proto              --experimental_allow_proto3_optional
-	protoc --go_opt=paths=source_relative  -I=./rpc/types/proto 		      --go_out=./rpc/types          	./rpc/types/proto/*.proto           --experimental_allow_proto3_optional
+	protoc --go_opt=paths=source_relative  -I=./rpc/types/proto 		  --go_out=./rpc/types          	./rpc/types/proto/*.proto           --experimental_allow_proto3_optional
 	echo "View generated proto files by running: make protogen_show"
 
 .PHONY: protogen_docker_m1
@@ -290,16 +305,10 @@ generate_cli_commands_docs:
 test_all: # generate_mocks
 	go test -p 1 -count=1 ./...
 
-.PHONY: test_all_with_json
-## Run all go unit tests, output results in json file
-test_all_with_json: generate_rpc_openapi # generate_mocks
-	go test -p 1 -json ./... > test_results.json
-
-.PHONY: test_all_with_coverage
-## Run all go unit tests, output results & coverage into files
-test_all_with_coverage: generate_rpc_openapi # generate_mocks
-	go test -p 1 -v ./... -covermode=count -coverprofile=coverage.out
-	go tool cover -func=coverage.out -o=coverage.out
+.PHONY: test_all_with_json_coverage
+## Run all go unit tests, output results & coverage into json & coverage files
+test_all_with_json_coverage: generate_rpc_openapi # generate_mocks
+	go test -p 1 -json ./... -covermode=count -coverprofile=coverage.out | tee test_results.json | jq
 
 .PHONY: test_race
 ## Identify all unit tests that may result in race conditions
@@ -317,9 +326,9 @@ test_shared: # generate_mocks
 	go test ${VERBOSE_TEST} -p 1 ./shared/...
 
 .PHONY: test_consensus
-## Run all go unit tests in the Consensus module
+## Run all go unit tests in the consensus module
 test_consensus: # mockgen
-	go test ${VERBOSE_TEST} ./consensus/...
+	go test ${VERBOSE_TEST} -count=1 ./consensus/...
 
 .PHONY: test_consensus_concurrent_tests
 ## Run unit tests in the consensus module that could be prone to race conditions (#192)
@@ -354,6 +363,11 @@ test_sortition:
 test_persistence:
 	go test ${VERBOSE_TEST} -p 1 -count=1 ./persistence/...
 
+.PHONY: test_persistence_state_hash
+## Run all go unit tests in the Persistence module related to the state hash
+test_persistence_state_hash:
+	go test ${VERBOSE_TEST} -run TestStateHash -count=1 ./persistence/...
+
 .PHONY: test_p2p
 ## Run all p2p
 test_p2p:
@@ -362,22 +376,29 @@ test_p2p:
 .PHONY: test_p2p_raintree
 ## Run all p2p raintree related tests
 test_p2p_raintree:
-	go test -run RainTreeNetwork -v -count=1 ./p2p/...
+	go test ${VERBOSE_TEST} -run RainTreeNetwork -count=1 ./p2p/...
 
 .PHONY: test_p2p_raintree_addrbook
 ## Run all p2p raintree addr book related tests
 test_p2p_raintree_addrbook:
-	go test -run RainTreeAddrBook -v -count=1 ./p2p/...
+	go test ${VERBOSE_TEST} -run RainTreeAddrBook -count=1 ./p2p/...
+
+# TIP: For benchmarks, consider appending `-run=^#` to avoid running unit tests in the same package
+
+.PHONY: benchmark_persistence_state_hash
+## Benchmark the state hash computation
+benchmark_persistence_state_hash:
+	go test ${VERBOSE_TEST} -cpu 1,2 -benchtime=1s -benchmem -bench=. -run BenchmarkStateHash -count=1 ./persistence/...
 
 .PHONY: benchmark_sortition
 ## Benchmark the Sortition library
 benchmark_sortition:
-	go test ${VERBOSE_TEST} ./consensus/leader_election/sortition -bench=.
+	go test ${VERBOSE_TEST} -bench=. -run ^# ./consensus/leader_election/sortition
 
 .PHONY: benchmark_p2p_addrbook
 ## Benchmark all P2P addr book related tests
 benchmark_p2p_addrbook:
-	go test -bench=. -run BenchmarkAddrBook -v -count=1 ./p2p/...
+	go test ${VERBOSE_TEST} -bench=. -run BenchmarkAddrBook -count=1 ./p2p/...
 
 ### Inspired by @goldinguy_ in this post: https://goldin.io/blog/stop-using-todo ###
 # TODO          - General Purpose catch-all.
@@ -391,10 +412,11 @@ benchmark_p2p_addrbook:
 # REFACTOR      - Similar to TECHDEBT, but will require a substantial rewrite and change across the codebase
 # CONSIDERATION - A comment that involves extra work but was thoughts / considered as part of some implementation
 # CONSOLIDATE   - We likely have similar implementations/types of the same thing, and we should consolidate them.
+# ADDTEST       - Add more tests for a specific code section
 # DEPRECATE     - Code that should be removed in the future
 # 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"
+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"
 
 # How do I use TODOs?
 # 1. <KEYWORD>: <Description of follow up work>;

app/client/cli/debug.go

@@ -23,6 +23,7 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 )
 
+// TECHDEBT: Lowercase variables / constants that do not need to be exported.
 const (
 	PromptResetToGenesis         string = "ResetToGenesis"
 	PromptPrintNodeState         string = "PrintNodeState"
@@ -207,10 +208,10 @@ func initDebug(remoteCLIURL string) {
 			logger.Global.Fatal().Err(err).Msg("Failed to create logger module")
 		}
 		loggerMod := loggerM.(modules.LoggerModule)
-    
- 		rpcM, err := rpc.Create(runtimeMgr)
+
+		rpcM, err := rpc.Create(runtimeMgr)
 		if err != nil {
-      logger.Global.Fatal().Err(err).Msg("Failed to create rpc module")
+			logger.Global.Fatal().Err(err).Msg("Failed to create rpc module")
 		}
 		rpcMod := rpcM.(modules.RPCModule)
 

build/config/config1.json

@@ -19,7 +19,9 @@
   "persistence": {
     "postgres_url": "postgres://postgres:postgres@pocket-db:5432/postgres",
     "node_schema": "node1",
-    "block_store_path": "/var/blockstore"
+    "block_store_path": "/var/blockstore",
+    "tx_indexer_path": "",
+    "trees_store_dir": "/var/trees"
   },
   "p2p": {
     "consensus_port": 8080,

build/config/config2.json

@@ -19,7 +19,9 @@
   "persistence": {
     "postgres_url": "postgres://postgres:postgres@pocket-db:5432/postgres",
     "node_schema": "node2",
-    "block_store_path": "/var/blockstore"
+    "block_store_path": "/var/blockstore",
+    "tx_indexer_path": "",
+    "trees_store_dir": "/var/trees"
   },
   "p2p": {
     "consensus_port": 8080,

build/config/config3.json

@@ -19,7 +19,9 @@
   "persistence": {
     "postgres_url": "postgres://postgres:postgres@pocket-db:5432/postgres",
     "node_schema": "node3",
-    "block_store_path": "/var/blockstore"
+    "block_store_path": "/var/blockstore",
+    "tx_indexer_path": "",
+    "trees_store_dir": "/var/trees"
   },
   "p2p": {
     "consensus_port": 8080,

build/config/config4.json

@@ -19,7 +19,9 @@
   "persistence": {
     "postgres_url": "postgres://postgres:postgres@pocket-db:5432/postgres",
     "node_schema": "node4",
-    "block_store_path": "/var/blockstore"
+    "block_store_path": "/var/blockstore",
+    "tx_indexer_path": "",
+    "trees_store_dir": "/var/trees"
   },
   "p2p": {
     "consensus_port": 8080,

build/sql/show_all_schemas.sql

@@ -0,0 +1 @@
+SELECT schema_name FROM information_schema.schemata;

consensus/block.go

@@ -63,6 +63,12 @@ func (m *consensusModule) refreshUtilityContext() error {
 		m.utilityContext = nil
 	}
 
+	// Only one write context can exist at a time, and the utility context needs to instantiate
+	// a new one to modify the state.
+	if err := m.GetBus().GetPersistenceModule().ReleaseWriteContext(); err != nil {
+		log.Printf("[WARN] Error releasing persistence write context: %v\n", err)
+	}
+
 	utilityContext, err := m.GetBus().GetUtilityModule().NewContext(int64(m.Height))
 	if err != nil {
 		return err

consensus/consensus_tests/utils_test.go

@@ -141,8 +141,8 @@ func CreateTestConsensusPocketNode(
 	utilityMock := baseUtilityMock(t, testChannel)
 	telemetryMock := baseTelemetryMock(t, testChannel)
 	loggerMock := baseLoggerMock(t, testChannel)
-  rpcMock := baseRpcMock(t, testChannel)
-  
+	rpcMock := baseRpcMock(t, testChannel)
+
 	bus, err := shared.CreateBus(runtimeMgr, persistenceMock, p2pMock, utilityMock, consensusMod.(modules.ConsensusModule), telemetryMock, loggerMock, rpcMock)
 
 	require.NoError(t, err)
@@ -157,6 +157,7 @@ func CreateTestConsensusPocketNode(
 	return pocketNode
 }
 
+// CLEANUP: Reduce package scope visibility in the consensus test module
 func StartAllTestPocketNodes(t *testing.T, pocketNodes IdToNodeMapping) {
 	for _, pocketNode := range pocketNodes {
 		go pocketNode.Start()
@@ -307,6 +308,7 @@ func basePersistenceMock(t *testing.T, _ modules.EventsChannel) *modulesMock.Moc
 	persistenceMock.EXPECT().Start().Return(nil).AnyTimes()
 	persistenceMock.EXPECT().SetBus(gomock.Any()).Return().AnyTimes()
 	persistenceMock.EXPECT().NewReadContext(int64(-1)).Return(persistenceContextMock, nil).AnyTimes()
+	persistenceMock.EXPECT().ReleaseWriteContext().Return(nil).AnyTimes()
 
 	// The persistence context should usually be accessed via the utility module within the context
 	// of the consensus module. This one is only used when loading the initial consensus module
@@ -363,7 +365,7 @@ func baseUtilityContextMock(t *testing.T) *modulesMock.MockUtilityContext {
 	utilityContextMock := modulesMock.NewMockUtilityContext(ctrl)
 	persistenceContextMock := modulesMock.NewMockPersistenceRWContext(ctrl)
 	persistenceContextMock.EXPECT().SetProposalBlock(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-	persistenceContextMock.EXPECT().GetPrevAppHash().Return("", nil).AnyTimes()
+	persistenceContextMock.EXPECT().GetBlockHash(gomock.Any()).Return([]byte(""), nil).AnyTimes()
 
 	utilityContextMock.EXPECT().
 		CreateAndApplyProposalBlock(gomock.Any(), maxTxBytes).
@@ -377,6 +379,8 @@ func baseUtilityContextMock(t *testing.T) *modulesMock.MockUtilityContext {
 	utilityContextMock.EXPECT().Release().Return(nil).AnyTimes()
 	utilityContextMock.EXPECT().GetPersistenceContext().Return(persistenceContextMock).AnyTimes()
 
+	persistenceContextMock.EXPECT().Release().Return(nil).AnyTimes()
+
 	return utilityContextMock
 }
 

consensus/debugging.go

@@ -48,7 +48,7 @@ func (m *consensusModule) resetToGenesis(_ *messaging.DebugMessage) {
 	m.clearLeader()
 	m.clearMessagesPool()
 	m.GetBus().GetPersistenceModule().HandleDebugMessage(&messaging.DebugMessage{
-		Action:  messaging.DebugMessageAction_DEBUG_CLEAR_STATE,
+		Action:  messaging.DebugMessageAction_DEBUG_PERSISTENCE_RESET_TO_GENESIS,
 		Message: nil,
 	})
 	m.GetBus().GetPersistenceModule().Start() // reload genesis state

consensus/doc/CHANGELOG.md

@@ -7,7 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.10] - 2022-11-30
+
+- Propagate `highPrepareQC` if available to the block being created
+- Remove `blockProtoBytes` from propagation in `SetProposalBlock`
+- Guarantee that write context is released when refreshing the utility context
+- Use `GetBlockHash(height)` instead of `GetPrevAppHash` to be more explicit
+- Use the real `quorumCert` when preparing a new block
+
 ## [0.0.0.9] - 2022-11-30
+
 - Added state sync interfaces and diagrams
 
 ## [0.0.0.8] - 2022-11-15

consensus/helpers.go

@@ -5,12 +5,10 @@ import (
 	"encoding/base64"
 	"log"
 
-	"github.com/pokt-network/pocket/shared/codec"
-
-	"google.golang.org/protobuf/proto"
-
 	typesCons "github.com/pokt-network/pocket/consensus/types"
+	"github.com/pokt-network/pocket/shared/codec"
 	cryptoPocket "github.com/pokt-network/pocket/shared/crypto"
+	"google.golang.org/protobuf/proto"
 )
 
 // These constants and variables are wrappers around the autogenerated protobuf types and were