feat(store/v2): add support for iavl/v2 #22424
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package iavlv2 | ||
|
There was a problem hiding this comment. Could you please add the tree test like iavl v1? There was a problem hiding this comment. I implemented the tests, skipping snaphots for now. It needs to be filled out but that can be done in parallel with the performance tests I'm now doing. |
||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "github.com/cosmos/iavl/v2" | ||
| ics23 "github.com/cosmos/ics23/go" | ||
|
|
||
| "cosmossdk.io/store/v2" | ||
| "cosmossdk.io/store/v2/commitment" | ||
| ) | ||
|
|
||
| var ( | ||
| _ commitment.Tree = (*Tree)(nil) | ||
| _ store.PausablePruner = (*Tree)(nil) | ||
| ) | ||
|
|
||
| type Tree struct { | ||
| tree *iavl.Tree | ||
| } | ||
|
|
||
| func NewTree(treeOptions iavl.TreeOptions, dbOptions iavl.SqliteDbOptions, pool *iavl.NodePool) (*Tree, error) { | ||
| sql, err := iavl.NewSqliteDb(pool, dbOptions) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| tree := iavl.NewTree(sql, pool, treeOptions) | ||
| return &Tree{tree: tree}, nil | ||
| } | ||
|
|
||
| func (t Tree) Set(key, value []byte) error { | ||
| _, err := t.tree.Set(key, value) | ||
| return err | ||
| } | ||
kocubinski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| func (t Tree) Remove(key []byte) error { | ||
| _, _, err := t.tree.Remove(key) | ||
| return err | ||
| } | ||
|
|
||
| func (t Tree) GetLatestVersion() (uint64, error) { | ||
| return uint64(t.tree.Version()), nil | ||
| } | ||
|
|
||
| func (t Tree) Hash() []byte { | ||
| return t.tree.Hash() | ||
| } | ||
|
|
||
| func (t Tree) Version() uint64 { | ||
| return uint64(t.tree.Version()) | ||
| } | ||
|
|
||
| func (t Tree) LoadVersion(version uint64) error { | ||
| // TODO fix this in iavl v2 | ||
kocubinski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if version == 0 { | ||
| return nil | ||
| } | ||
| return t.tree.LoadVersion(int64(version)) | ||
| } | ||
|
|
||
| func (t Tree) Commit() ([]byte, uint64, error) { | ||
| h, v, err := t.tree.SaveVersion() | ||
| return h, uint64(v), err | ||
| } | ||
|
|
||
| func (t Tree) SetInitialVersion(version uint64) error { | ||
| t.tree.SetShouldCheckpoint() | ||
| return t.tree.SetInitialVersion(int64(version)) | ||
| } | ||
|
|
||
| func (t Tree) GetProof(version uint64, key []byte) (*ics23.CommitmentProof, error) { | ||
| return t.tree.GetProof(int64(version), key) | ||
| } | ||
|
|
||
| func (t Tree) Get(version uint64, key []byte) ([]byte, error) { | ||
| if int64(version) != t.tree.Version() { | ||
| return nil, fmt.Errorf("loading past version not yet supported") | ||
| } | ||
| return t.tree.Get(key) | ||
| } | ||
kocubinski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| func (t Tree) Export(version uint64) (commitment.Exporter, error) { | ||
| return nil, errors.New("snapshot import/export not yet supported") | ||
| } | ||
|
|
||
| func (t Tree) Import(version uint64) (commitment.Importer, error) { | ||
| return nil, errors.New("snapshot import/export not yet supported") | ||
| } | ||
|
|
||
| func (t Tree) Close() error { | ||
| return t.tree.Close() | ||
| } | ||
|
|
||
| func (t Tree) Prune(version uint64) error { | ||
| return t.tree.DeleteVersionsTo(int64(version)) | ||
| } | ||
|
|
||
| // PausePruning is unnecessary in IAVL v2 due to the advanced pruning mechanism | ||
| func (t Tree) PausePruning(bool) {} | ||
kocubinski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| func (t *Tree) WorkingHash() []byte { | ||
| return t.tree.Hash() | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Consider using a more actively maintained SQLite library
The current choice of
github.com/bvinc/go-sqlite-lite v0.6.1raises several concerns:Consider evaluating alternative SQLite libraries with explicit locking mechanisms that are actively maintained, such as:
modernc.org/sqlitegithub.com/glebarez/go-sqlite🔗 Analysis chain
Verify the SQLite library migration.
The addition of
github.com/bvinc/go-sqlite-lite v0.6.1aligns with the PR objective to migrate frommattn/go-sqlite3. However, let's verify if v0.6.1 is the latest stable version and if there are any known issues.Let me gather more information about the issues and the repository activity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 355
Script:
Length of output: 2015
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a fair point, but I personally favor how
bvinc/go-sqlite-liteis a very minimal wrapper around the original libc API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kocubinski, understood. Thank you for explaining your preference for
bvinc/go-sqlite-litedue to its minimal wrapper around the original libc API. I'll keep this in mind for future reviews.✏️ Learnings added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modernc.org/sqlite could be a good fit since it's a pure go implementation (won't conflict with iavl/v2) and maybe even more performant than the C one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kocubinski, that's a great point. Modernc.org/sqlite, being a pure Go implementation, could indeed avoid conflicts with iavl/v2 and potentially offer better performance.
From the information gathered:
It seems like a promising alternative worth evaluating. Let me know if you need any assistance with this.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 2119
✏️ Learnings added