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

[v4] Add promisified wrapper for stateManager #480

Merged
merged 1 commit into from
Mar 22, 2019
Merged

[v4] Add promisified wrapper for stateManager #480

merged 1 commit into from
Mar 22, 2019

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Mar 22, 2019

One of the main sources of asynchrony in VM is the state manager. This wrapper allows us to slowly shift towards promises without having to modify the whole codebase at once. E.g. a new part that's being modernized could wrap its stateManager like this:

const state = new StateManager()

const pstate = new PStateManager(state)
await pstate.getAccount(addr)

This is a temporary class and will have to be deleted when the whole codebase has transitioned to promises.

(Will be merged into #479)

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

That's a great approach and also looks good! 👍

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.126% when pulling 60d3cc1 on promisified-state into 256fe59 on v4.

@holgerd77
Copy link
Member

Maybe we just merge this into master instead of v4 and release this with v3, then others can also use this to prepare for a transition? Since this is not breaking/touching anything at all and is just the additional preparatory class/base layer?

@s1na
Copy link
Contributor Author

s1na commented Mar 22, 2019

To be honest I haven't tested this as thoroughly. What do you think about this approach:

Maybe we could start with non-breaking changes in #479, and every once in a while port the changes back to master for v3.x releases, e.g. when the initial refactoring round stabilizes a bit, we can merge back and release v3.1.

But of course, as you mentioned, since this is not touching any code, and it's opt-in (they have to explicitly import the file) there shouldn't be any problem with merging right now.

Whatever you think best.

@holgerd77
Copy link
Member

Ok, then let's go on with the 3.1 approach, think this is generally a solid ground to continue.

@s1na s1na merged commit 3ab73a6 into v4 Mar 22, 2019
@s1na s1na deleted the promisified-state branch March 22, 2019 10:21
@s1na s1na mentioned this pull request Mar 22, 2019
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants