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/v2): add version exists #22235

Merged
merged 8 commits into from
Oct 16, 2024
Merged

feat(store/v2): add version exists #22235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
59b785b
add version exists
tac0turtle Oct 11, 2024
a3168cc
cleanup
tac0turtle Oct 11, 2024
dffb45d
add test cases
tac0turtle Oct 13, 2024
64ae24d
changes
tac0turtle Oct 15, 2024
cbd465c
fix tests
cool-develope Oct 15, 2024
72feb12
rocksdb fix
cool-develope Oct 15, 2024
fd3909c
bench test fix
cool-develope Oct 15, 2024
74de046
Merge branch 'main' into marko/store_stateat
tac0turtle Oct 16, 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 runtime/v2/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type Store interface {
Query(storeKey []byte, version uint64, key []byte, prove bool) (storev2.QueryResult, error)

// GetStateStorage returns the SS backend.
GetStateStorage() storev2.VersionedDatabase
GetStateStorage() storev2.VersionedWriter
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

Issues Found: Update Remaining VersionedDatabase References

The VersionedDatabase type is still referenced in multiple files:

  • store/v2/storage/store.go
  • store/v2/mock/types.go
  • server/v2/cometbft/internal/mock/mock_store.go

Please update these references to VersionedWriter to ensure consistency across the codebase.

🔗 Analysis chain

LGTM. Verify impact and update documentation if needed.

The change from storev2.VersionedDatabase to storev2.VersionedWriter for the GetStateStorage method return type looks good. This modification aligns with the PR objectives of enhancing the state machine functionality.

To ensure this change doesn't introduce any inconsistencies, please run the following script to check for any other occurrences of VersionedDatabase that might need updating:

Consider updating any relevant documentation or comments related to the GetStateStorage method to reflect this change in return type, if necessary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of VersionedDatabase that might need updating

# Search for VersionedDatabase usage
echo "Searching for VersionedDatabase usage:"
rg --type go "VersionedDatabase"

# Search for GetStateStorage method calls
echo "Searching for GetStateStorage method calls:"
ast-grep --lang go --pattern 'GetStateStorage()'

Length of output: 897


// GetStateCommitment returns the SC backend.
GetStateCommitment() storev2.Committer
Expand Down
1 change: 1 addition & 0 deletions store/v2/commitment/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ func (c *CommitStore) GetProof(storeKey []byte, version uint64, key []byte) ([]p
return []proof.CommitmentOp{commitOp, *storeCommitmentOp}, nil
}

// Get implements store.VersionedReader.
func (c *CommitStore) Get(storeKey []byte, version uint64, key []byte) ([]byte, error) {
tree, ok := c.multiTrees[conv.UnsafeBytesToStr(storeKey)]
if !ok {
Expand Down
28 changes: 17 additions & 11 deletions store/v2/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,29 @@ import (
"cosmossdk.io/store/v2/proof"
)

// VersionedDatabase defines an API for a versioned database that allows reads,
// VersionedWriter defines an API for a versioned database that allows reads,
// writes, iteration and commitment over a series of versions.
type VersionedDatabase interface {
type VersionedWriter interface {
VersionedReader

SetLatestVersion(version uint64) error
ApplyChangeset(version uint64, cs *corestore.Changeset) error
Comment on lines +15 to +16
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

Add missing documentation for exported methods SetLatestVersion and ApplyChangeset

The methods SetLatestVersion and ApplyChangeset are exported but lack doc comments. According to the Uber Go Style Guide, all exported functions and methods should have comments that begin with the method name and clearly describe their purpose. Please add doc comments to improve code readability and maintainability.


// Close releases associated resources. It should NOT be idempotent. It must
// only be called once and any call after may panic.
io.Closer
}

type VersionedReader interface {
Has(storeKey []byte, version uint64, key []byte) (bool, error)
Get(storeKey []byte, version uint64, key []byte) ([]byte, error)

GetLatestVersion() (uint64, error)
SetLatestVersion(version uint64) error
VersionExists(v uint64) (bool, error)
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

Use descriptive parameter name and add documentation for VersionExists method

The parameter v in VersionExists is not descriptive. For clarity and consistency, consider renaming it to version. Additionally, the method lacks a doc comment. As per the Uber Go Style Guide, please add a comment starting with the method name to explain its functionality.


Iterator(storeKey []byte, version uint64, start, end []byte) (corestore.Iterator, error)
ReverseIterator(storeKey []byte, version uint64, start, end []byte) (corestore.Iterator, error)

ApplyChangeset(version uint64, cs *corestore.Changeset) error

// Close releases associated resources. It should NOT be idempotent. It must
// only be called once and any call after may panic.
io.Closer
Expand Down Expand Up @@ -53,18 +63,14 @@ type Committer interface {
// GetProof returns the proof of existence or non-existence for the given key.
GetProof(storeKey []byte, version uint64, key []byte) ([]proof.CommitmentOp, error)

// Get returns the value for the given key at the given version.
//
// NOTE: This method only exists to support migration from IAVL v0/v1 to v2.
// Once migration is complete, this method should be removed and/or not used.
Get(storeKey []byte, version uint64, key []byte) ([]byte, error)

// SetInitialVersion sets the initial version of the committer.
SetInitialVersion(version uint64) error

// GetCommitInfo returns the CommitInfo for the given version.
GetCommitInfo(version uint64) (*proof.CommitInfo, error)

Get(storeKey []byte, version uint64, key []byte) ([]byte, error)
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

Add missing documentation for exported method Get

The method Get in the Committer interface is exported but doesn't have a doc comment. To comply with the Uber Go Style Guide, please provide a comment starting with the method name to describe its behavior and purpose.


// Close releases associated resources. It should NOT be idempotent. It must
// only be called once and any call after may panic.
io.Closer
Expand Down
15 changes: 15 additions & 0 deletions store/v2/mock/db_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion store/v2/mock/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type StateCommitter interface {

// StateStorage is a mock of store.VersionedDatabase
type StateStorage interface {
store.VersionedDatabase
store.VersionedWriter
store.UpgradableDatabase
store.Pruner
store.PausablePruner
Expand Down
20 changes: 11 additions & 9 deletions store/v2/root/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Store struct {
dbCloser io.Closer

// stateStorage reflects the state storage backend
stateStorage store.VersionedDatabase
stateStorage store.VersionedWriter

// stateCommitment reflects the state commitment (SC) backend
stateCommitment store.Committer
Expand Down Expand Up @@ -74,7 +74,7 @@ type Store struct {
func New(
dbCloser io.Closer,
logger corelog.Logger,
ss store.VersionedDatabase,
ss store.VersionedWriter,
sc store.Committer,
pm *pruning.Manager,
mm *migration.Manager,
Expand Down Expand Up @@ -127,19 +127,21 @@ func (s *Store) StateLatest() (uint64, corestore.ReaderMap, error) {
return v, NewReaderMap(v, s), nil
}

// StateAt checks if the requested version is present in ss.
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider renaming 'StateAt' method for clarity

The method name StateAt may not clearly convey its functionality. Consider renaming it to GetStateAtVersion or GetReaderMapAtVersion to better reflect that it retrieves a ReaderMap for a specific version.

func (s *Store) StateAt(v uint64) (corestore.ReaderMap, error) {
// TODO(bez): We may want to avoid relying on the SC metadata here. Instead,
// we should add a VersionExists() method to the VersionedDatabase interface.
//
// Ref: https://github.com/cosmos/cosmos-sdk/issues/19091
if cInfo, err := s.stateCommitment.GetCommitInfo(v); err != nil || cInfo == nil {
return nil, fmt.Errorf("failed to get commit info for version %d: %w", v, err)
// check if version is present in state storage
isExist, err := s.stateStorage.VersionExists(v)
if err != nil {
return nil, err
}
if !isExist {
return nil, fmt.Errorf("version %d does not exist", v)
}

return NewReaderMap(v, s), nil
}

func (s *Store) GetStateStorage() store.VersionedDatabase {
func (s *Store) GetStateStorage() store.VersionedWriter {
return s.stateStorage
}

Expand Down
2 changes: 1 addition & 1 deletion store/v2/root/store_mock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"cosmossdk.io/store/v2/pruning"
)

func newTestRootStore(ss store.VersionedDatabase, sc store.Committer) *Store {
func newTestRootStore(ss store.VersionedWriter, sc store.Committer) *Store {
noopLog := coretesting.NewNopLogger()
pm := pruning.NewManager(sc.(store.Pruner), ss.(store.Pruner), nil, nil)
return &Store{
Expand Down
2 changes: 1 addition & 1 deletion store/v2/root/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (s *RootStoreTestSuite) newStoreWithPruneConfig(config *store.PruningOption
s.rootStore = rs
}

func (s *RootStoreTestSuite) newStoreWithBackendMount(ss store.VersionedDatabase, sc store.Committer, pm *pruning.Manager) {
func (s *RootStoreTestSuite) newStoreWithBackendMount(ss store.VersionedWriter, sc store.Committer, pm *pruning.Manager) {
noopLog := coretesting.NewNopLogger()

rs, err := New(dbm.NewMemDB(), noopLog, ss, sc, pm, nil, nil)
Expand Down
1 change: 1 addition & 0 deletions store/v2/storage/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Database interface {
Get(storeKey []byte, version uint64, key []byte) ([]byte, error)
GetLatestVersion() (uint64, error)
SetLatestVersion(version uint64) error
VersionExists(version uint64) (bool, error)

Iterator(storeKey []byte, version uint64, start, end []byte) (corestore.Iterator, error)
ReverseIterator(storeKey []byte, version uint64, start, end []byte) (corestore.Iterator, error)
Expand Down
9 changes: 9 additions & 0 deletions store/v2/storage/pebbledb/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ func (db *Database) GetLatestVersion() (uint64, error) {
return binary.LittleEndian.Uint64(bz), closer.Close()
}

func (db *Database) VersionExists(version uint64) (bool, error) {
latestVersion, err := db.GetLatestVersion()
if err != nil {
return false, err
}

return latestVersion >= version && version >= db.earliestVersion, nil
}

func (db *Database) setPruneHeight(pruneVersion uint64) error {
db.earliestVersion = pruneVersion + 1

Expand Down
9 changes: 9 additions & 0 deletions store/v2/storage/rocksdb/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ func (db *Database) GetLatestVersion() (uint64, error) {
return binary.LittleEndian.Uint64(bz), nil
}

func (db *Database) VersionExists(version uint64) (bool, error) {
latestVersion, err := db.GetLatestVersion()
if err != nil {
return false, err
}

return latestVersion >= version && version >= db.tsLow, nil
}

func (db *Database) Has(storeKey []byte, version uint64, key []byte) (bool, error) {
slice, err := db.getSlice(storeKey, version, key)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions store/v2/storage/rocksdb/db_noflag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func (db *Database) GetLatestVersion() (uint64, error) {
panic("rocksdb requires a build flag")
}

func (db *Database) VersionExists(version uint64) (bool, error) {
panic("rocksdb requires a build flag")
}

func (db *Database) Has(storeKey []byte, version uint64, key []byte) (bool, error) {
panic("rocksdb requires a build flag")
}
Expand Down
15 changes: 14 additions & 1 deletion store/v2/storage/sqlite/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ func (db *Database) NewBatch(version uint64) (store.Batch, error) {
}

func (db *Database) GetLatestVersion() (uint64, error) {
stmt, err := db.storage.Prepare("SELECT value FROM state_storage WHERE store_key = ? AND key = ?")
stmt, err := db.storage.Prepare(`
SELECT value
FROM state_storage
WHERE store_key = ? AND key = ?
`)
if err != nil {
return 0, fmt.Errorf("failed to prepare SQL statement: %w", err)
}
Expand All @@ -123,6 +127,15 @@ func (db *Database) GetLatestVersion() (uint64, error) {
return latestHeight, nil
}

func (db *Database) VersionExists(v uint64) (bool, error) {
latestVersion, err := db.GetLatestVersion()
if err != nil {
return false, err
}

return latestVersion >= v && v >= db.earliestVersion, nil
}

func (db *Database) SetLatestVersion(version uint64) error {
_, err := db.storage.Exec(reservedUpsertStmt, reservedStoreKey, keyLatestHeight, version, 0, version)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions store/v2/storage/storage_bench_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ import (
var storeKey1 = []byte("store1")

var (
backends = map[string]func(dataDir string) (store.VersionedDatabase, error){
"rocksdb_versiondb_opts": func(dataDir string) (store.VersionedDatabase, error) {
backends = map[string]func(dataDir string) (store.VersionedWriter, error){
"rocksdb_versiondb_opts": func(dataDir string) (store.VersionedWriter, error) {
db, err := rocksdb.New(dataDir)
return storage.NewStorageStore(db, coretesting.NewNopLogger()), err
},
"pebbledb_default_opts": func(dataDir string) (store.VersionedDatabase, error) {
"pebbledb_default_opts": func(dataDir string) (store.VersionedWriter, error) {
db, err := pebbledb.New(dataDir)
if err == nil && db != nil {
db.SetSync(false)
}

return storage.NewStorageStore(db, coretesting.NewNopLogger()), err
},
"btree_sqlite": func(dataDir string) (store.VersionedDatabase, error) {
"btree_sqlite": func(dataDir string) (store.VersionedWriter, error) {
db, err := sqlite.New(dataDir)
return storage.NewStorageStore(db, coretesting.NewNopLogger()), err
},
Expand Down
Loading
Loading