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

chore: add the wrapper layer of versa #2636

Open
wants to merge 93 commits into
base: versa_base
Choose a base branch
from
Open

Conversation

joeylichang
Copy link
Contributor

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:

  • add each change in a bullet point here
  • ...

@joeylichang joeylichang added the wip work in process label Aug 7, 2024

type cachingVersaDB struct {
triedb *triedb.Database
versionDB versa.Database
Copy link

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"?

Copy link
Contributor Author

@joeylichang joeylichang Aug 12, 2024

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
Copy link

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"?

Copy link
Contributor Author

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.

}

// TODO:: if root tree, versa db shouldb ignore check version
state, err := cv.versionDB.OpenState(0, root, cv.mode)
Copy link
Contributor

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.

Copy link
Contributor Author

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) {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip work in process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants