diff --git a/persistence/context.go b/persistence/context.go index c91560086..04ad2812f 100644 --- a/persistence/context.go +++ b/persistence/context.go @@ -46,10 +46,9 @@ func (p *PostgresContext) SetSavePoint() error { // 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 errors.Join(pgErr, treesErr) + err := p.stateTrees.Rollback() + p.Release() + return err } // Full details in the thread from the PR review: https://github.com/pokt-network/pocket/pull/285#discussion_r1018471719 diff --git a/persistence/trees/trees.go b/persistence/trees/trees.go index 8fdc43676..1d0c6db01 100644 --- a/persistence/trees/trees.go +++ b/persistence/trees/trees.go @@ -15,6 +15,7 @@ package trees import ( "crypto/sha256" "encoding/hex" + "errors" "fmt" "hash" "log" @@ -322,7 +323,14 @@ func (t *treeStore) Rollback() error { return nil } t.logger.Err(ErrFailedRollback) - return ErrFailedRollback + // emergency shut down to prevent improper commit to trees + errs := []error{ErrFailedRollback} + for _, st := range t.merkleTrees { + if err := st.nodeStore.Stop(); err != nil { + errs = append(errs, err) + } + } + return errors.Join(errs...) } // save commits any pending changes to the trees and creates a copy of the current worldState, diff --git a/persistence/trees/trees_test.go b/persistence/trees/trees_test.go index aa8c41ab4..9259e0cc6 100644 --- a/persistence/trees/trees_test.go +++ b/persistence/trees/trees_test.go @@ -46,8 +46,9 @@ func TestTreeStore_Update(t *testing.T) { require.NoError(t, pool.Purge(resource)) }) - t.Run("should update actor trees, commit, and modify the state hash", func(t *testing.T) { - pmod := newTestPersistenceModule(t, dbUrl) + t.Run("should update application trees, commit, and modify the state hash", func(t *testing.T) { + cfg := newTestDefaultConfig(t, dbUrl) + pmod := newTestPersistenceModule(t, cfg) context := newTestPostgresContext(t, 0, pmod) require.NoError(t, context.SetSavePoint()) @@ -69,21 +70,43 @@ func TestTreeStore_Update(t *testing.T) { }) t.Run("should fail to rollback when no treestore savepoint is set", func(t *testing.T) { - pmod := newTestPersistenceModule(t, dbUrl) + // NB: test needs a file to persist against for test purposes + tmpdir := t.TempDir() + cfg := newTestDefaultConfig(t, dbUrl) + cfg.Persistence.TreesStoreDir = tmpdir + pmod := newTestPersistenceModule(t, cfg) context := newTestPostgresContext(t, 0, pmod) - err := context.RollbackToSavePoint() + // calculate first hash + hash1, err := context.ComputeStateHash() + require.NoError(t, err) + require.NotEmpty(t, hash1) + + // insert default app + _, err = createAndInsertDefaultTestApp(t, context) + require.NoError(t, err) + + // trigger a rollback without creating a savepoint + err = context.RollbackToSavePoint() require.Error(t, err) require.ErrorIs(t, err, trees.ErrFailedRollback) + + // initialize a new context with the same persistence module + context2 := newTestPostgresContext(t, 0, pmod) + hash3, err := context2.ComputeStateHash() + require.NoError(t, err) + require.NotEmpty(t, hash3) + + // unsuccessful rollback should not violate treeStore integrity + require.Equal(t, hash3, hash1) }) } -func newTestPersistenceModule(t *testing.T, databaseURL string) modules.PersistenceModule { +func newTestPersistenceModule(t *testing.T, cfg *configs.Config) modules.PersistenceModule { t.Helper() teardownDeterministicKeygen := keygen.GetInstance().SetSeed(42) defer teardownDeterministicKeygen() - cfg := newTestDefaultConfig(t, databaseURL) genesisState, _ := test_artifacts.NewGenesisState( genesisStateNumValidators, genesisStateNumServicers,