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

Store Refactor #2986

Closed
mossid opened this issue Dec 3, 2018 · 2 comments
Closed

Store Refactor #2986

mossid opened this issue Dec 3, 2018 · 2 comments
Assignees
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Milestone

Comments

@mossid
Copy link
Contributor

mossid commented Dec 3, 2018

The store/ is quite large and the parts are deeply interconnected with each other, so refactoring will be split into three parts:

1. Move store types to separate folders #2309

Each store type, such as rootmultistore.go, iavlstore.go, will be moved into separated folders, for example, rootmulti/store.go, iavl/store.go. Actual types will be named Store, making them exposed and easily accessible from external code - the caller can use iavl.Store for example.

We cannot use KVStore.Gas() and KVStore.Prefix() now, since it causes import cycle between store/prefix and store/gas. For now we remove those methods and use gas.NewStore and prefix.NewStore directly.

The dependency of store on types will be inverted, meaning that types/store.go, types/gas.go, types/queryable.go and part of types/errors.go will be moved to store/types/, and types/store.go will reexport them. But in the first PR, only types/store.go and types/gas.go will be moved.

WIP on #2985

2. Invert dependency

Because some of the error types are used by both store/* and the outside, we have to duplicate the error types from types/ to store/types/. This PR will move types/errors.go and types/queryable.go to store/types/.

3. Simplify store interfaces

Currently the interfaces in types/store.go have multiple layers of embedding which is hard to analyze. The second PR will simplify it to two-layer structure.

            KVStore         CommitStore          MultiStore              ^
           /        \         /        \           /       \             |
         /            \     /            \       /          \            |  embeds
CacheKVStore    CommitKVStore    CommitMultiStore    CacheMultiStore     *

It will also contain #2824. (Should it also be applied on gas?)

Also, instead of using StoreType, which is an enum type rootmulti.Store switches internally, we associate the StoreKey and KVStore/MultiStore implementation so the multistore can also mount third-party store type (needs discussion).

StoreKey will be split into KVStoreKey and MultiStoreKey, making it typesafe when accessing on substores and to prepare on future recursive multistore implementation.

The primitive version can be found in #2344

@mossid mossid added core Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. labels Dec 3, 2018
This was referenced Dec 3, 2018
@mossid mossid mentioned this issue Jan 25, 2019
5 tasks
@alessio alessio added this to the v0.31.0 (Launch RC) milestone Jan 29, 2019
@cwgoes cwgoes modified the milestones: v0.31.0 (Launch RC), Backlog Feb 5, 2019
@cwgoes
Copy link
Contributor

cwgoes commented Feb 5, 2019

Both of the first two PRs have been merged & we decided to leave the rest for post-launch.

@jackzampolin
Copy link
Member

@mossid going to close this issue as complete based on the completed store refactors. If I'm incorrect please reopen this issue with the remaining work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

No branches or pull requests

4 participants