Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Generic state backend #4632

Merged
merged 4 commits into from
Feb 25, 2017
Merged

Generic state backend #4632

merged 4 commits into from
Feb 25, 2017

Conversation

rphmeier
Copy link
Contributor

Closes #4616.

This is intended to allow us to enact state changes on general databases/caches, rather than just the single StateDB.

Also opens the door to executing EVMs over different account storage models, by moving the trie lookups fully into the backend, which may be done in the future.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Feb 22, 2017
@@ -186,8 +194,8 @@ impl AccountEntry {
/// checkpoint can be discateded with `discard_checkpoint`. All of the orignal
/// backed-up values are moved into a parent checkpoint (if any).
///
pub struct State {
db: StateDB,
pub struct State<B> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not B: Backend ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More general > less general. This is a common pattern among e.g. structures in std: constrain the generic parameters only when introducing functionality that depends on those constraints.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd agree if there was some generic functionality, but State is pretty much useless without a backend. Attempting to use it with unsupported type will leave you with a bare struct which does not really represent the State abstraction. I think it would be better to explicitly disallow that.

@arkpar arkpar added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Feb 25, 2017
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 25, 2017
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 25, 2017
@rphmeier rphmeier merged commit eb9ee35 into master Feb 25, 2017
@rphmeier rphmeier deleted the state-backend branch February 25, 2017 15:22
@rphmeier rphmeier moved this from In Review to Done in Light Client Feb 26, 2017
@ordian ordian mentioned this pull request Feb 23, 2020
@dvdplm dvdplm mentioned this pull request Feb 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
Light Client
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants