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 historical queries support for IAVL v1 to v2 migration #23685

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
31 changes: 31 additions & 0 deletions store/v2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,34 @@ for more details.
## Test Coverage

The test coverage of the following logical components should be over 60%:

# Historical Queries After Migration

When migrating from IAVL v1 to v2, historical data access can be preserved by enabling historical queries. This feature allows querying data from the old tree for heights before the migration point.

## Configuration

To enable historical queries, set `EnableHistoricalQueries` to `true` in the IAVL configuration:

```go
cfg := iavl.DefaultConfig()
cfg.EnableHistoricalQueries = true
```

## How It Works

1. During migration, the migration height is stored
2. For queries:
- If version < migration height: uses old tree (if historical queries enabled)
- If version >= migration height: uses new tree
- If historical queries disabled: always uses new tree

## Node Operator Instructions

1. Before migration:
- Ensure old tree data is preserved
- Configure `EnableHistoricalQueries` if historical access is needed

2. After migration:
- Historical queries will automatically use appropriate tree based on height
- Can disable historical queries later by setting `EnableHistoricalQueries = false`
23 changes: 18 additions & 5 deletions store/v2/commitment/iavl/config.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
// Package iavl implements the IAVL tree commitment store.
package iavl

// Config is the configuration for the IAVL tree.
// Config contains the configuration for the IAVL tree.
// It provides options to customize the behavior of the tree.
type Config struct {
CacheSize int `mapstructure:"cache-size" toml:"cache-size" comment:"CacheSize set the size of the iavl tree cache."`
SkipFastStorageUpgrade bool `mapstructure:"skip-fast-storage-upgrade" toml:"skip-fast-storage-upgrade" comment:"If true, the tree will work like no fast storage and always not upgrade fast storage."`
// CacheSize is the size of the cache in memory.
// A larger cache size can improve performance at the cost of memory usage.
CacheSize int `mapstructure:"cache-size"`

// SkipFastStorageUpgrade determines whether to skip the fast storage upgrade.
// If true, the tree will work like no fast storage and always not upgrade fast storage.
SkipFastStorageUpgrade bool `mapstructure:"skip-fast-storage-upgrade"`

// EnableHistoricalQueries enables querying historical data from the old tree.
// When enabled, queries for versions before the migration height will use the old tree.
EnableHistoricalQueries bool `mapstructure:"enable-historical-queries"`
}

// DefaultConfig returns the default configuration for the IAVL tree.
// DefaultConfig returns the default configuration.
// It provides sensible defaults for all configuration options.
func DefaultConfig() *Config {
return &Config{
CacheSize: 1000,
CacheSize: 1000000,
SkipFastStorageUpgrade: false,
EnableHistoricalQueries: false,
}
}
81 changes: 81 additions & 0 deletions store/v2/commitment/iavl/historical_queries.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Package iavl implements the IAVL tree commitment store.
package iavl

import (
"fmt"

"cosmossdk.io/core/store"
"cosmossdk.io/store/v2/migration"
)

// HistoricalQuerier wraps an IAVL tree to support historical queries.
// It provides functionality to query data from either the old or new tree
// based on the migration height and configuration.
type HistoricalQuerier struct {
tree *IavlTree
oldReader store.Reader
db store.KVStoreWithBatch
config *Config
}

// NewHistoricalQuerier creates a new HistoricalQuerier instance.
// It requires a tree, an optional old reader for historical queries,
// a database connection, and configuration.
// Returns error if required parameters are nil.
func NewHistoricalQuerier(tree *IavlTree, oldReader store.Reader, db store.KVStoreWithBatch, config *Config) (*HistoricalQuerier, error) {
if tree == nil {
return nil, fmt.Errorf("tree cannot be nil")
}
if db == nil {
return nil, fmt.Errorf("db cannot be nil")
}
if config == nil {
config = DefaultConfig()
}
return &HistoricalQuerier{
tree: tree,
oldReader: oldReader,
db: db,
config: config,
}, nil
}

// Get retrieves a value at the specified version.
// If historical queries are enabled and the version is before migration height,
// it will use the old reader. Otherwise, it uses the current tree.
func (h *HistoricalQuerier) Get(version uint64, key []byte) ([]byte, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Get(version, key)
}

migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}

if version < migrationHeight && h.oldReader != nil {
return h.oldReader.Get(version, key)
}

return h.tree.Get(version, key)
}
Comment on lines +46 to +61
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

Fix interface mismatch in oldReader calls.
Static analysis indicates that h.oldReader.Get(...) may not match the signature of store.Reader, which typically expects only Get(key []byte) ([]byte, error) without a version parameter. This code will fail to compile or will lead to a runtime panic if oldReader does not support version-based APIs.

Below is a possible fix if the old reader does not support version-based queries. You could remove the version parameter in the call (and logic), or employ a specialized versioned interface:

-if version < migrationHeight && h.oldReader != nil {
-    return h.oldReader.Get(version, key)
+if version < migrationHeight && h.oldReader != nil {
+    // Consider using a specialized interface or remove version argument:
+    return h.oldReader.Get(key)
}
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (h *HistoricalQuerier) Get(version uint64, key []byte) ([]byte, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Get(version, key)
}
migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}
if version < migrationHeight && h.oldReader != nil {
return h.oldReader.Get(version, key)
}
return h.tree.Get(version, key)
}
func (h *HistoricalQuerier) Get(version uint64, key []byte) ([]byte, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Get(version, key)
}
migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}
if version < migrationHeight && h.oldReader != nil {
// Consider using a specialized interface or remove version argument:
return h.oldReader.Get(key)
}
return h.tree.Get(version, key)
}
🧰 Tools
🪛 golangci-lint (1.62.2)

57-57: too many arguments in call to h.oldReader.Get
have (uint64, []byte)
want ([]byte)

(typecheck)


// Iterator returns an iterator over a domain of keys at the specified version.
// If historical queries are enabled and the version is before migration height,
// it will use the old reader. Otherwise, it uses the current tree.
func (h *HistoricalQuerier) Iterator(version uint64, start, end []byte, ascending bool) (store.Iterator, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Iterator(version, start, end, ascending)
}

migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}

if version < migrationHeight && h.oldReader != nil {
return h.oldReader.Iterator(version, start, end, ascending)
}

return h.tree.Iterator(version, start, end, ascending)
}
Comment on lines +63 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.

⚠️ Potential issue

Same interface mismatch for oldReader in the iterator call.
As with Get, static analysis shows a similar discrepancy in oldReader.Iterator(...). The standard store.Reader interface typically has a method signature like Iterator(start, end []byte), lacking a version argument.

Below is a possible fix if your old reader is not version aware:

-if version < migrationHeight && h.oldReader != nil {
-    return h.oldReader.Iterator(version, start, end, ascending)
+if version < migrationHeight && h.oldReader != nil {
+    // Use an interface that supports version-based iteration or remove version argument:
+    return h.oldReader.Iterator(start, end)
}
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Iterator returns an iterator over a domain of keys at the specified version.
// If historical queries are enabled and the version is before migration height,
// it will use the old reader. Otherwise, it uses the current tree.
func (h *HistoricalQuerier) Iterator(version uint64, start, end []byte, ascending bool) (store.Iterator, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Iterator(version, start, end, ascending)
}
migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}
if version < migrationHeight && h.oldReader != nil {
return h.oldReader.Iterator(version, start, end, ascending)
}
return h.tree.Iterator(version, start, end, ascending)
}
// Iterator returns an iterator over a domain of keys at the specified version.
// If historical queries are enabled and the version is before migration height,
// it will use the old reader. Otherwise, it uses the current tree.
func (h *HistoricalQuerier) Iterator(version uint64, start, end []byte, ascending bool) (store.Iterator, error) {
if !h.config.EnableHistoricalQueries {
return h.tree.Iterator(version, start, end, ascending)
}
migrationHeight, err := migration.GetMigrationHeight(h.db)
if err != nil {
return nil, fmt.Errorf("failed to get migration height: %w", err)
}
if version < migrationHeight && h.oldReader != nil {
// Use an interface that supports version-based iteration or remove version argument:
return h.oldReader.Iterator(start, end)
}
return h.tree.Iterator(version, start, end, ascending)
}
🧰 Tools
🪛 golangci-lint (1.62.2)

77-77: too many arguments in call to h.oldReader.Iterator
have (uint64, []byte, []byte, bool)
want ([]byte, []byte)

(typecheck)

Loading