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

feat(store/v2): add SetInitialVersion in SC #18665

Merged
merged 1 commit into from
Dec 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions store/commitment/iavl/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ func (t *IavlTree) GetLatestVersion() uint64 {
return uint64(t.tree.Version())
}

// SetInitialVersion sets the initial version of the database.
func (t *IavlTree) SetInitialVersion(version uint64) error {
t.tree.SetInitialVersion(version)
return nil
Comment on lines +79 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

The SetInitialVersion method does not handle the error that might be returned by t.tree.SetInitialVersion(version). It is important to handle this error to ensure that any issues with setting the initial version are properly reported and can be acted upon.

func (t *IavlTree) SetInitialVersion(version uint64) error {
-   t.tree.SetInitialVersion(version)
-   return nil
+   return t.tree.SetInitialVersion(int64(version))
}

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func (t *IavlTree) SetInitialVersion(version uint64) error {
t.tree.SetInitialVersion(version)
return nil
func (t *IavlTree) SetInitialVersion(version uint64) error {
return t.tree.SetInitialVersion(int64(version))
}

}

// Prune prunes all versions up to and including the provided version.
func (t *IavlTree) Prune(version uint64) error {
return t.tree.DeleteVersionsTo(int64(version))
Expand Down
10 changes: 10 additions & 0 deletions store/commitment/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ func (c *CommitStore) Commit() ([]store.StoreInfo, error) {
return storeInfos, nil
}

func (c *CommitStore) SetInitialVersion(version uint64) error {
for _, tree := range c.multiTrees {
if err := tree.SetInitialVersion(version); err != nil {
return err
}
}

return nil
}

func (c *CommitStore) GetProof(storeKey string, version uint64, key []byte) (*ics23.CommitmentProof, error) {
tree, ok := c.multiTrees[storeKey]
if !ok {
Expand Down
1 change: 1 addition & 0 deletions store/commitment/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Tree interface {
WorkingHash() []byte
LoadVersion(version uint64) error
Commit() ([]byte, error)
SetInitialVersion(version uint64) error
GetProof(version uint64, key []byte) (*ics23.CommitmentProof, error)
Prune(version uint64) error
Export(version uint64) (Exporter, error)
Expand Down
1 change: 1 addition & 0 deletions store/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type Committer interface {
GetLatestVersion() (uint64, error)
LoadVersion(targetVersion uint64) error
Commit() ([]StoreInfo, error)
SetInitialVersion(version uint64) error
GetProof(storeKey string, version uint64, key []byte) (*ics23.CommitmentProof, error)

// Prune attempts to prune all versions up to and including the provided
Expand Down
6 changes: 1 addition & 5 deletions store/root/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,7 @@ func (s *Store) SetMetrics(m metrics.Metrics) {
func (s *Store) SetInitialVersion(v uint64) error {
s.initialVersion = v

// TODO(bez): Call SetInitialVersion on s.stateCommitment.
//
// Ref: https://github.com/cosmos/cosmos-sdk/issues/18597

return nil
return s.stateCommitment.SetInitialVersion(v)
Comment on lines 116 to +119
Copy link
Contributor

Choose a reason for hiding this comment

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

The SetInitialVersion method should not directly set the initialVersion field according to the PR objectives and the summary. The direct assignment should be removed to align with the intended changes.

func (s *Store) SetInitialVersion(v uint64) error {
-   s.initialVersion = v
    return s.stateCommitment.SetInitialVersion(v)
}

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func (s *Store) SetInitialVersion(v uint64) error {
s.initialVersion = v
// TODO(bez): Call SetInitialVersion on s.stateCommitment.
//
// Ref: https://github.com/cosmos/cosmos-sdk/issues/18597
return nil
return s.stateCommitment.SetInitialVersion(v)
func (s *Store) SetInitialVersion(v uint64) error {
return s.stateCommitment.SetInitialVersion(v)
}

}

// GetSCStore returns the store's state commitment (SC) backend.
Expand Down
Loading