-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore: add the wrapper layer of versa #2636
base: versa_base
Are you sure you want to change the base?
Conversation
f896f80
to
41ab497
Compare
|
||
type cachingVersaDB struct { | ||
triedb *triedb.Database | ||
versionDB versa.Database |
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.
Why to have a "versionDB" here? Is there any difference with "triedb"?
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.
There is no essential difference; here, versiondb is for convenience.
codeSizeCache *lru.Cache[common.Hash, int] | ||
codeCache *lru.SizeConstrainedCache[common.Hash, []byte] | ||
|
||
accTree *VersaTree |
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.
Where is VersaTree impl? And why to have a "accTree"?
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.
Facilitate OpenStorageTree to avoid reopen.
core/state/caching_versa_db.go
Outdated
} | ||
|
||
// TODO:: if root tree, versa db shouldb ignore check version | ||
state, err := cv.versionDB.OpenState(0, root, cv.mode) |
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.
Suggest to use the block num as the version here, not 0.
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 fine, but currently, this version is of no use to the upper layer and has brought about a burden. The plan is to make the upper layer aware of the version only if needed in the future. The conclusion is that it can be transparent to the upper layer for the time being.
return val | ||
} | ||
|
||
func (vt *VersaTree) GetAccount(address common.Address) (*types.StateAccount, error) { |
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.
I think the version of the account should be returned to the caller, and maintained by the account itself, so that it can be provided to versaDB when opening the storage tree.
But if do this, a lot of changes will be required. Therefore, we need to do some trade off whether the version should be known to the caller.
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.
Similarly, currently the upper layer does not have a strong dependency on the version, so it can be transparent to the upper layer for the time being. If needed in the future, it can be exposed then. The focus at this stage should not be on this.
261e67f
to
e1a2b8e
Compare
a8cf691
to
78da6b6
Compare
78da6b6
to
bb167a9
Compare
0802a8c
to
60df2fa
Compare
11f9fa8
to
6188aae
Compare
b48c5d1
to
5153e69
Compare
5d0eec2
to
d162a63
Compare
8834138
to
1cbdf0a
Compare
1a70548
to
36044b0
Compare
ccd9ce0
to
83a9b13
Compare
Description
add a description of your changes here...
Rationale
tell us why we need these changes...
Example
add an example CLI or API response...
Changes
Notable changes: