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.

Sign up for GitHub

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): add new api LatestVersion #22305

Merged
merged 2 commits into from
Oct 21, 2024
Merged

feat(store): add new api LatestVersion #22305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a57482
add new api LatestVersion
cool-develope Oct 17, 2024
5c729a6
changelog
cool-develope Oct 17, 2024
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
2 changes: 1 addition & 1 deletion baseapp/baseapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ func (app *BaseApp) LastCommitID() storetypes.CommitID {

// LastBlockHeight returns the last committed block height.
func (app *BaseApp) LastBlockHeight() int64 {
return app.cms.LastCommitID().Version
return app.cms.LatestVersion()
}

// ChainID returns the chainID of the app.
Expand Down
4 changes: 4 additions & 0 deletions store/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Improvements

* (store) [#22305](https://github.com/cosmos/cosmos-sdk/pull/22305) Add `LatestVersion` to the `Committer` interface to get the latest version of the store.

### Bug Fixes

* (store) [#20425](https://github.com/cosmos/cosmos-sdk/pull/20425) Fix nil pointer panic when query historical state where a new store don't exist.
Expand Down
5 changes: 5 additions & 0 deletions store/iavl/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ func (st *Store) LastCommitID() types.CommitID {
}
}

// LatestVersion implements Committer.
func (st *Store) LatestVersion() int64 {
return st.tree.Version()
}

// PausePruning implements CommitKVStore interface.
func (st *Store) PausePruning(pause bool) {
if pause {
Expand Down
2 changes: 2 additions & 0 deletions store/mem/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,6 @@ func (s *Store) GetPruning() pruningtypes.PruningOptions {

func (s Store) LastCommitID() (id types.CommitID) { return }

func (s Store) LatestVersion() (version int64) { return }
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Standardize LatestVersion() Method Across Stores

The LatestVersion() method in store/mem/store.go currently returns an int64, whereas other store implementations return uint64. This inconsistency can lead to type mismatches and potential bugs.

To ensure consistency and prevent type-related issues:

  • Standardize the Return Type: Decide on a single return type (int64 or uint64) for the LatestVersion() method across all store implementations.
  • Implement the Method Correctly: Update the LatestVersion() method in store/mem/store.go to return the actual latest version instead of the zero value.
  • Review Method Naming: Ensure that all version-related methods (GetLatestVersion, LoadLatestVersion, etc.) are clearly defined and consistently named to reflect their functionality.
// LatestVersion returns the latest version of the store.
// This method is introduced to improve performance by avoiding
// repeated hash computations in GetLastBlockHeight.
func (s Store) LatestVersion() (version uint64) {
    // TODO: Implement the logic to return the latest version
    return 0
}
🔗 Analysis chain

Implement the LatestVersion() method

The LatestVersion() method has been added as per the PR objectives, but it currently has an empty implementation. This doesn't fulfill the intended purpose of returning the latest version of the store.

Please implement the method to return the actual latest version. Consider the following:

  1. How is the version information stored or tracked in the Store struct?
  2. Should this method read from a field, or calculate the version on-the-fly?

Also, add a comment explaining the purpose of this method. For example:

// LatestVersion returns the latest version of the store.
// This method is introduced to improve performance by avoiding
// repeated hash computations in GetLastBlockHeight.
func (s Store) LatestVersion() (version int64) {
    // TODO: Implement the logic to return the latest version
    return 0
}

To ensure this method is used correctly throughout the codebase, let's check its usage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of LatestVersion method on Store
rg --type go 'Store.*LatestVersion\(\)'

Length of output: 3891


func (s Store) WorkingHash() (hash []byte) { return }
4 changes: 4 additions & 0 deletions store/rootmulti/dbadapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func (cdsa commitDBStoreAdapter) LastCommitID() types.CommitID {
}
}

func (cdsa commitDBStoreAdapter) LatestVersion() int64 {
return -1
}
Comment on lines +39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider revising the LatestVersion implementation

The current implementation of LatestVersion always returns -1, which may not accurately represent the latest version in all scenarios. This could lead to unexpected behavior or errors in code that relies on this method.

Consider the following suggestions:

  1. If -1 is intended to represent an uninitialized state, it might be better to return an error along with the version to indicate this special case.
  2. If possible, implement logic to return the actual latest version of the store.
  3. Add a comment explaining the rationale behind returning -1 if this is the intended behavior.

Here's a potential improvement:

func (cdsa commitDBStoreAdapter) LatestVersion() (int64, error) {
    // TODO: Implement logic to get the actual latest version
    return -1, errors.New("latest version not available")
}

This change would require updating the interface and other implementations as well.


func (cdsa commitDBStoreAdapter) WorkingHash() []byte {
return commithash
}
Expand Down
6 changes: 5 additions & 1 deletion store/rootmulti/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,11 @@ func (rs *Store) PopStateCache() []*types.StoreKVPair {

// LatestVersion returns the latest version in the store
func (rs *Store) LatestVersion() int64 {
return rs.LastCommitID().Version
if rs.lastCommitInfo == nil {
return GetLatestVersion(rs.db)
}

return rs.lastCommitInfo.Version
}

// LastCommitID implements Committer/CommitStore.
Expand Down
5 changes: 5 additions & 0 deletions store/transient/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func (ts *Store) LastCommitID() types.CommitID {
return types.CommitID{}
}

// LatestVersion implements Committer
func (ts *Store) LatestVersion() int64 {
return 0
}

func (ts *Store) WorkingHash() []byte {
return []byte{}
}
Expand Down
1 change: 1 addition & 0 deletions store/types/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type Store interface {
type Committer interface {
Commit() CommitID
LastCommitID() CommitID
LatestVersion() int64

// WorkingHash returns the hash of the KVStore's state before commit.
WorkingHash() []byte
Expand Down
Loading