Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Persistence] Adds atomic Update for TreeStore #861

Merged
merged 34 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d8d8ddf
[chore] submodule pattern updates to TreeStore
dylanlott Jul 11, 2023
1a5777c
updates TreeStore to use Submodule type
dylanlott Jul 12, 2023
f87518a
[fixup] embeds TreeStoreFactory on to TreeStoreModule
dylanlott Jul 12, 2023
671598d
[fixup] TreeStoreOption and TreeStoreModule updates
dylanlott Jul 12, 2023
c988cbc
[fixup] get height provider from bus in tree tests
dylanlott Jul 17, 2023
4ae3912
[Temp] s/TreeStore/treeStore (#920)
Olshansk Jul 19, 2023
a601002
[fixup] formatting long mock definitions
dylanlott Jul 20, 2023
795ce87
[chore] submodule pattern updates to TreeStore
dylanlott Jul 11, 2023
4df81fe
[feature] adds atomic rollbacks to TreeStore
dylanlott Jul 11, 2023
f77f764
linter updates
dylanlott Jul 11, 2023
674d539
[fixup] pass testing.T to the newTestApp function
dylanlott Jul 12, 2023
5ee3f42
treestore tests use testing.T instead of err returns
dylanlott Jul 17, 2023
713017b
[fixup] privatizes treeStoreDir field
dylanlott Jul 18, 2023
7f947f9
[fixup] fix path for treestore mock generations
dylanlott Jul 19, 2023
d70a067
[fixup] use method to fetch context instead
dylanlott Jul 19, 2023
d107ed0
[fixup] adds comments and updates tests
dylanlott Jul 19, 2023
626ea5d
[fixup] adds optimize comment to save function
dylanlott Jul 19, 2023
b3ab6b0
[fixup] cleanup
dylanlott Jul 20, 2023
fc04ab0
[docs] updates package level comment
dylanlott Jul 20, 2023
2af89dd
[fixup] adds comments and updates logger use
dylanlott Jul 21, 2023
8ed31bf
dylanlott Jul 21, 2023
09097ef
[rename] s/NewSavePoint/SetSavePoint/g
dylanlott Jul 21, 2023
07b64da
[fixup] update test names and use constants in tests
dylanlott Jul 21, 2023
1ecfde6
[fixup] adds better comment to treeStore#Rollback
dylanlott Jul 21, 2023
a687768
[tests] adds a test for failing rollback
dylanlott Jul 24, 2023
800b2cb
[fixup] check rollback error value to satisfy linter
dylanlott Jul 24, 2023
0b22519
[test] adds a test for block proposal revert
dylanlott Jul 25, 2023
ba85105
[fixup] fix linter error
dylanlott Jul 25, 2023
7a7d95c
[fixup] updates rollback test to use Errors#Is
dylanlott Jul 26, 2023
0f87f0b
[fixup] prevent import cycle error errors in treestore
dylanlott Jul 27, 2023
cb4497d
[fixup] reduce export footprint of worldState
dylanlott Jul 27, 2023
6ce32dc
[docs] update rollback comment
dylanlott Jul 27, 2023
7bad92e
[fixup] s/trees_hash1/treesHash1/
dylanlott Jul 29, 2023
4d218dc
[fixup] adds a happy path test for CreateProposalBlock
dylanlott Jul 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions persistence/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/pokt-network/pocket/persistence/indexer"
coreTypes "github.com/pokt-network/pocket/shared/core/types"
"github.com/pokt-network/pocket/shared/modules"
"go.uber.org/multierr"
)

var _ modules.PersistenceRWContext = &PostgresContext{}
Expand All @@ -36,29 +37,35 @@ type PostgresContext struct {
networkId string
}

func (p *PostgresContext) NewSavePoint(bytes []byte) error {
p.logger.Info().Bool("TODO", true).Msg("NewSavePoint not implemented")
// SetSavePoint generates a new Savepoint for this context.
func (p *PostgresContext) SetSavePoint() error {
if err := p.stateTrees.Savepoint(); err != nil {
return err
}
return nil
}

// TECHDEBT(#327): Guarantee atomicity betweens `prepareBlock`, `insertBlock` and `storeBlock` for save points & rollbacks.
func (p *PostgresContext) RollbackToSavePoint(bytes []byte) error {
p.logger.Info().Bool("TODO", true).Msg("RollbackToSavePoint not fully implemented")
return p.tx.Rollback(context.TODO())
// RollbackToSavepoint triggers a rollback for the current pgx transaction and the underylying submodule stores.
func (p *PostgresContext) RollbackToSavePoint() error {
ctx, _ := p.getCtxAndTx()
pgErr := p.tx.Rollback(ctx)
treesErr := p.stateTrees.Rollback()
return multierr.Combine(pgErr, treesErr)
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
}

// IMPROVE(#361): Guarantee the integrity of the state
// Full details in the thread from the PR review: https://github.com/pokt-network/pocket/pull/285#discussion_r1018471719
func (p *PostgresContext) ComputeStateHash() (string, error) {
stateHash, err := p.stateTrees.Update(p.tx, uint64(p.Height))
if err != nil {
return "", err
}
if err := p.stateTrees.Commit(); err != nil {
return "", err
}
p.stateHash = stateHash
return p.stateHash, nil
}

// TECHDEBT(#327): Make sure these operations are atomic
func (p *PostgresContext) Commit(proposerAddr, quorumCert []byte) error {
p.logger.Info().Int64("height", p.Height).Msg("About to commit block & context")

Expand Down
1 change: 1 addition & 0 deletions persistence/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var protocolActorSchemas = []types.ProtocolActorSchema{
types.ValidatorActor,
}

// TECHDEBT(#595): Properly handle context threading and passing for the entire persistence module
func (pg *PostgresContext) getCtxAndTx() (context.Context, pgx.Tx) {
return context.TODO(), pg.tx
}
Expand Down
4 changes: 4 additions & 0 deletions persistence/docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.0.0.60] - 2023-07-11

- Adds savepoints and rollbacks implementation to TreeStore

## [0.0.0.60] - 2023-06-26

- Add place-holder for local context and servicer token usage support methods
Expand Down
92 changes: 92 additions & 0 deletions persistence/trees/atomic_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package trees

import (
"encoding/hex"
"testing"

"github.com/golang/mock/gomock"
"github.com/pokt-network/pocket/logger"
mock_types "github.com/pokt-network/pocket/persistence/types/mocks"
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
"github.com/pokt-network/pocket/shared/modules"
mockModules "github.com/pokt-network/pocket/shared/modules/mocks"

"github.com/stretchr/testify/require"
)

const (
// the root hash of a tree store where each tree is empty but present and initialized
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
h0 = "302f2956c084cc3e0e760cf1b8c2da5de79c45fa542f68a660a5fc494b486972"
// the root hash of a tree store where each tree has has key foo value bar added to it
h1 = "7d5712ea1507915c40e295845fa58773baa405b24b87e9d99761125d826ff915"
)

func TestTreeStore_AtomicUpdatesWithSuccessfulRollback(t *testing.T) {
ctrl := gomock.NewController(t)

mockTxIndexer := mock_types.NewMockTxIndexer(ctrl)
mockBus := mockModules.NewMockBus(ctrl)
mockPersistenceMod := mockModules.NewMockPersistenceModule(ctrl)

mockBus.EXPECT().GetPersistenceModule().AnyTimes().Return(mockPersistenceMod)
mockPersistenceMod.EXPECT().GetTxIndexer().AnyTimes().Return(mockTxIndexer)

ts := &treeStore{
logger: logger.Global.CreateLoggerForModule(modules.TreeStoreSubmoduleName),
treeStoreDir: ":memory:",
}
require.NoError(t, ts.setupTrees())
require.NotEmpty(t, ts.merkleTrees[TransactionsTreeName])

hash0 := ts.getStateHash()
require.NotEmpty(t, hash0)
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, hash0, h0)

require.NoError(t, ts.Savepoint())

// insert test data into every tree
for _, treeName := range stateTreeNames {
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
err := ts.merkleTrees[treeName].tree.Update([]byte("foo"), []byte("bar"))
require.NoError(t, err)
}

// commit the above changes
require.NoError(t, ts.Commit())

// assert state hash is changed
hash1 := ts.getStateHash()
require.NotEmpty(t, hash1)
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.NotEqual(t, hash0, hash1)
require.Equal(t, hash1, h1)

// set a new savepoint
require.NoError(t, ts.Savepoint())
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.NotEmpty(t, ts.prevState.merkleTrees)
require.NotEmpty(t, ts.prevState.rootTree)
// assert that savepoint creation doesn't mutate state hash
require.Equal(t, hash1, hex.EncodeToString(ts.prevState.rootTree.tree.Root()))

// verify that creating a savepoint does not change state hash
hash2 := ts.getStateHash()
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, hash2, hash1)
require.Equal(t, hash2, h1)

// validate that state tree was updated and a previous savepoint is created
for _, treeName := range stateTreeNames {
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.NotEmpty(t, ts.merkleTrees[treeName])
require.NotEmpty(t, ts.prevState.merkleTrees[treeName])
}

// insert additional test data into all of the trees
for _, treeName := range stateTreeNames {
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, ts.merkleTrees[treeName].tree.Update([]byte("fiz"), []byte("buz")))
}

// rollback the changes made to the trees above BEFORE anything was committed
err := ts.Rollback()
require.NoError(t, err)

// validate that the state hash is unchanged after new data was inserted but rolled back before commitment
hash3 := ts.getStateHash()
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, hash3, hash2)
require.Equal(t, hash3, h1)
}
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 12 additions & 0 deletions persistence/trees/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//go:build test

package trees
Comment on lines +1 to +3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In persistence/trees/trees_test.go, we use package trees_test and not have the //go:build test tag.

Here, we have the package trees and do have the //go:build test tag.

Feels inconsistent, but not sure if this is general best practice for main_test.go` vs non-main test files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following the prescription of TestUtils for testing unexported members like previously discussed.


import (
"crypto/sha256"
"hash"
)

type TreeStore = treeStore

var SMTTreeHasher hash.Hash = sha256.New()
dylanlott marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion persistence/trees/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ func TestTreeStore_Create(t *testing.T) {

treemod, err := trees.Create(mockBus, trees.WithTreeStoreDirectory(":memory:"))
assert.NoError(t, err)

got := treemod.GetBus()
assert.Equal(t, got, mockBus)

Expand Down
90 changes: 90 additions & 0 deletions persistence/trees/prove_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package trees

import (
"fmt"
"testing"

"github.com/pokt-network/pocket/persistence/kvstore"
"github.com/pokt-network/smt"
"github.com/stretchr/testify/require"
)

func TestTreeStore_Prove(t *testing.T) {
nodeStore := kvstore.NewMemKVStore()
tree := smt.NewSparseMerkleTree(nodeStore, smtTreeHasher)
testTree := &stateTree{
name: "test",
tree: tree,
nodeStore: nodeStore,
}

require.NoError(t, testTree.tree.Update([]byte("key"), []byte("value")))
require.NoError(t, testTree.tree.Commit())

treeStore := &treeStore{
merkleTrees: make(map[string]*stateTree, 1),
}
treeStore.merkleTrees["test"] = testTree

testCases := []struct {
name string
treeName string
key []byte
value []byte
valid bool
expectedErr error
}{
{
name: "valid inclusion proof: key and value in tree",
treeName: "test",
key: []byte("key"),
value: []byte("value"),
valid: true,
expectedErr: nil,
},
{
name: "valid exclusion proof: key not in tree",
treeName: "test",
key: []byte("key2"),
value: nil,
valid: true,
expectedErr: nil,
},
{
name: "invalid proof: tree not in store",
treeName: "unstored tree",
key: []byte("key"),
value: []byte("value"),
valid: false,
expectedErr: fmt.Errorf("tree not found: %s", "unstored tree"),
},
{
name: "invalid inclusion proof: key in tree, wrong value",
treeName: "test",
key: []byte("key"),
value: []byte("wrong value"),
valid: false,
expectedErr: nil,
},
{
name: "invalid exclusion proof: key in tree",
treeName: "test",
key: []byte("key"),
value: nil,
valid: false,
expectedErr: nil,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
valid, err := treeStore.Prove(tc.treeName, tc.key, tc.value)
require.Equal(t, valid, tc.valid)
if tc.expectedErr == nil {
require.NoError(t, err)
return
}
require.ErrorAs(t, err, &tc.expectedErr)
})
}
}
Loading
Loading