From fbc61d2f5d3883bc4845f87e30ad8dbdbaabd178 Mon Sep 17 00:00:00 2001 From: cool-developer <51834436+cool-develope@users.noreply.github.com> Date: Tue, 14 May 2024 09:49:31 -0400 Subject: [PATCH] refactor(store/v2): get rid of `WorkingHash` (#20379) --- store/v2/root/migrate_test.go | 4 --- store/v2/root/store.go | 66 ++++++++--------------------------- store/v2/root/store_test.go | 36 +++---------------- store/v2/store.go | 11 +----- 4 files changed, 19 insertions(+), 98 deletions(-) diff --git a/store/v2/root/migrate_test.go b/store/v2/root/migrate_test.go index e80f41fd1bb0..eabb21f03ae8 100644 --- a/store/v2/root/migrate_test.go +++ b/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) } diff --git a/store/v2/root/store.go b/store/v2/root/store.go index b619624fa5b6..a8e717fd5a64 100644 --- a/store/v2/root/store.go +++ b/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 diff --git a/store/v2/root/store_test.go b/store/v2/root/store_test.go index d6c52367a265..8bb5d4dbb0b1 100644 --- a/store/v2/root/store_test.go +++ b/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() diff --git a/store/v2/store.go b/store/v2/store.go index e8437eee756e..e7c557dd533b 100644 --- a/store/v2/store.go +++ b/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.