refactor(store/v2): get rid of WorkingHash (cosmos#20379)

Author: cool-develope

store/v2/root/migrate_test.go

@@ -108,8 +108,6 @@ func (s *MigrateStoreTestSuite) TestMigrateState() {
 				cs.Add([]byte(storeKey), []byte(fmt.Sprintf("key-%d-%d", latestVersion, i)), []byte(fmt.Sprintf("value-%d-%d", latestVersion, i)), false)
 			}
 		}
-		_, err := s.rootStore.WorkingHash(cs)
-		s.Require().NoError(err)
 		_, err = s.rootStore.Commit(cs)
 		s.Require().NoError(err)
 
@@ -156,8 +154,6 @@ func (s *MigrateStoreTestSuite) TestMigrateState() {
 				cs.Add([]byte(storeKey), []byte(fmt.Sprintf("key-%d-%d", version, i)), []byte(fmt.Sprintf("value-%d-%d", version, i)), false)
 			}
 		}
-		_, err := s.rootStore.WorkingHash(cs)
-		s.Require().NoError(err)
 		_, err = s.rootStore.Commit(cs)
 		s.Require().NoError(err)
 	}

store/v2/root/store.go

@@ -3,7 +3,6 @@ package root
 import (
 	"bytes"
 	"fmt"
-	"slices"
 	"sync"
 	"time"
 
@@ -41,9 +40,6 @@ type Store struct {
 	// lastCommitInfo reflects the last version/hash that has been committed
 	lastCommitInfo *proof.CommitInfo
 
-	// workingHash defines the current (yet to be committed) hash
-	workingHash []byte
-
 	// telemetry reflects a telemetry agent responsible for emitting metrics (if any)
 	telemetry metrics.StoreMetrics
 
@@ -238,7 +234,6 @@ func (s *Store) loadVersion(v uint64) error {
 		return fmt.Errorf("failed to load SC version %d: %w", v, err)
 	}
 
-	s.workingHash = nil
 	s.commitHeader = nil
 
 	// set lastCommitInfo explicitly s.t. Commit commits the correct version, i.e. v+1
@@ -256,43 +251,19 @@ func (s *Store) SetCommitHeader(h *coreheader.Info) {
 	s.commitHeader = h
 }
 
-// WorkingHash returns the working hash of the root store. Note, WorkingHash()
-// should only be called once per block once all writes are complete and prior
-// to Commit() being called.
-//
-// If working hash is nil, then we need to compute and set it on the root store
-// by constructing a CommitInfo object, which in turn creates and writes a batch
-// of the current changeset to the SC tree.
-func (s *Store) WorkingHash(cs *corestore.Changeset) ([]byte, error) {
-	if s.telemetry != nil {
-		now := time.Now()
-		defer s.telemetry.MeasureSince(now, "root_store", "working_hash")
-	}
-
-	if s.workingHash == nil {
-		if err := s.writeSC(cs); err != nil {
-			return nil, err
-		}
-
-		s.workingHash = s.lastCommitInfo.Hash()
-	}
-
-	return slices.Clone(s.workingHash), nil
-}
-
-// Commit commits all state changes to the underlying SS and SC backends. Note,
-// at the time of Commit(), we expect WorkingHash() to have already been called
-// with the same Changeset, which internally sets the working hash, retrieved by
-// writing a batch of the changeset to the SC tree, and CommitInfo on the root
-// store.
+// Commit commits all state changes to the underlying SS and SC backends. It
+// writes a batch of the changeset to the SC tree, and retrieves the CommitInfo
+// from the SC tree. Finally, it commits the SC tree and returns the hash of the
+// CommitInfo.
 func (s *Store) Commit(cs *corestore.Changeset) ([]byte, error) {
 	if s.telemetry != nil {
 		now := time.Now()
 		defer s.telemetry.MeasureSince(now, "root_store", "commit")
 	}
 
-	if s.workingHash == nil {
-		return nil, fmt.Errorf("working hash is nil; must call WorkingHash() before Commit()")
+	// write the changeset to the SC tree and update lastCommitInfo
+	if err := s.writeSC(cs); err != nil {
+		return nil, err
 	}
 
 	version := s.lastCommitInfo.Version
@@ -318,7 +289,7 @@ func (s *Store) Commit(cs *corestore.Changeset) ([]byte, error) {
 
 	// commit SC async
 	eg.Go(func() error {
-		if err := s.commitSC(cs); err != nil {
+		if err := s.commitSC(); err != nil {
 			return fmt.Errorf("failed to commit SC: %w", err)
 		}
 
@@ -333,8 +304,6 @@ func (s *Store) Commit(cs *corestore.Changeset) ([]byte, error) {
 		s.lastCommitInfo.Timestamp = s.commitHeader.Time
 	}
 
-	s.workingHash = nil
-
 	return s.lastCommitInfo.Hash(), nil
 }
 
@@ -441,24 +410,17 @@ func (s *Store) writeSC(cs *corestore.Changeset) error {
 }
 
 // commitSC commits the SC store. At this point, a batch of the current changeset
-// should have already been written to the SC via WorkingHash(). This method
-// solely commits that batch. An error is returned if commit fails or if the
-// resulting commit hash is not equivalent to the working hash.
-func (s *Store) commitSC(cs *corestore.Changeset) error {
+// should have already been written to the SC via writeSC(). This method solely
+// commits that batch. An error is returned if commit fails or the hash of the
+// committed state does not match the hash of the working state.
+func (s *Store) commitSC() error {
 	cInfo, err := s.stateCommitment.Commit(s.lastCommitInfo.Version)
 	if err != nil {
 		return fmt.Errorf("failed to commit SC store: %w", err)
 	}
 
-	commitHash := cInfo.Hash()
-
-	workingHash, err := s.WorkingHash(cs)
-	if err != nil {
-		return fmt.Errorf("failed to get working hash: %w", err)
-	}
-
-	if !bytes.Equal(commitHash, workingHash) {
-		return fmt.Errorf("unexpected commit hash; got: %X, expected: %X", commitHash, workingHash)
+	if !bytes.Equal(cInfo.Hash(), s.lastCommitInfo.Hash()) {
+		return fmt.Errorf("unexpected commit hash; got: %X, expected: %X", cInfo.Hash(), s.lastCommitInfo.Hash())
 	}
 
 	return nil

store/v2/root/store_test.go

@@ -94,14 +94,9 @@ func (s *RootStoreTestSuite) TestQuery() {
 	cs := corestore.NewChangeset()
 	cs.Add(testStoreKeyBytes, []byte("foo"), []byte("bar"), false)
 
-	workingHash, err := s.rootStore.WorkingHash(cs)
-	s.Require().NoError(err)
-	s.Require().NotNil(workingHash)
-
 	commitHash, err := s.rootStore.Commit(cs)
 	s.Require().NoError(err)
 	s.Require().NotNil(commitHash)
-	s.Require().Equal(workingHash, commitHash)
 
 	// ensure the proof is non-nil for the corresponding version
 	result, err := s.rootStore.Query([]byte(testStoreKey), 1, []byte("foo"), true)
@@ -146,9 +141,7 @@ func (s *RootStoreTestSuite) TestQueryProof() {
 	cs.Add(testStoreKey3Bytes, []byte("key4"), []byte("value4"), false)
 
 	// commit
-	_, err := s.rootStore.WorkingHash(cs)
-	s.Require().NoError(err)
-	_, err = s.rootStore.Commit(cs)
+	_, err := s.rootStore.Commit(cs)
 	s.Require().NoError(err)
 
 	// query proof for testStoreKey
@@ -174,14 +167,9 @@ func (s *RootStoreTestSuite) TestLoadVersion() {
 		cs := corestore.NewChangeset()
 		cs.Add(testStoreKeyBytes, []byte("key"), []byte(val), false)
 
-		workingHash, err := s.rootStore.WorkingHash(cs)
-		s.Require().NoError(err)
-		s.Require().NotNil(workingHash)
-
 		commitHash, err := s.rootStore.Commit(cs)
 		s.Require().NoError(err)
 		s.Require().NotNil(commitHash)
-		s.Require().Equal(workingHash, commitHash)
 	}
 
 	// ensure the latest version is correct
@@ -219,14 +207,9 @@ func (s *RootStoreTestSuite) TestLoadVersion() {
 		cs := corestore.NewChangeset()
 		cs.Add(testStoreKeyBytes, []byte("key"), []byte(val), false)
 
-		workingHash, err := s.rootStore.WorkingHash(cs)
-		s.Require().NoError(err)
-		s.Require().NotNil(workingHash)
-
 		commitHash, err := s.rootStore.Commit(cs)
 		s.Require().NoError(err)
 		s.Require().NotNil(commitHash)
-		s.Require().Equal(workingHash, commitHash)
 	}
 
 	// ensure the latest version is correct
@@ -259,17 +242,9 @@ func (s *RootStoreTestSuite) TestCommit() {
 		cs.Add(testStoreKeyBytes, []byte(key), []byte(val), false)
 	}
 
-	// committing w/o calling WorkingHash should error
-	_, err = s.rootStore.Commit(cs)
-	s.Require().Error(err)
-
-	// execute WorkingHash and Commit
-	wHash, err := s.rootStore.WorkingHash(cs)
-	s.Require().NoError(err)
-
 	cHash, err := s.rootStore.Commit(cs)
 	s.Require().NoError(err)
-	s.Require().Equal(wHash, cHash)
+	s.Require().NotNil(cHash)
 
 	// ensure latest version is updated
 	lv, err = s.rootStore.GetLatestVersion()
@@ -305,13 +280,10 @@ func (s *RootStoreTestSuite) TestStateAt() {
 			cs.Add(testStoreKeyBytes, []byte(key), []byte(val), false)
 		}
 
-		// execute WorkingHash and Commit
-		wHash, err := s.rootStore.WorkingHash(cs)
-		s.Require().NoError(err)
-
+		// execute Commit
 		cHash, err := s.rootStore.Commit(cs)
 		s.Require().NoError(err)
-		s.Require().Equal(wHash, cHash)
+		s.Require().NotNil(cHash)
 	}
 
 	lv, err := s.rootStore.GetLatestVersion()

store/v2/store.go

@@ -49,20 +49,11 @@ type RootStore interface {
 	// queries based on block time need to be supported.
 	SetCommitHeader(h *coreheader.Info)
 
-	// WorkingHash returns the current WIP commitment hash by applying the Changeset
-	// to the SC backend. Typically, WorkingHash() is called prior to Commit() and
-	// must be applied with the exact same Changeset. This is because WorkingHash()
-	// is responsible for writing the Changeset to the SC backend and returning the
-	// resulting root hash. Then, Commit() would return this hash and flush writes
-	// to disk.
-	WorkingHash(cs *corestore.Changeset) ([]byte, error)
-
 	// Commit should be responsible for taking the provided changeset and flushing
 	// it to disk. Note, depending on the implementation, the changeset, at this
 	// point, may already be written to the SC backends. Commit() should ensure
 	// the changeset is committed to all SC and SC backends and flushed to disk.
-	// It must return a hash of the merkle-ized committed state. This hash should
-	// be the same as the hash returned by WorkingHash() prior to calling Commit().
+	// It must return a hash of the merkle-ized committed state.
 	Commit(cs *corestore.Changeset) ([]byte, error)
 
 	// LastCommitID returns a CommitID pertaining to the last commitment.