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

Migrate state module to TS #496

Merged
merged 14 commits into from
Apr 17, 2019
Merged

Migrate state module to TS #496

merged 14 commits into from
Apr 17, 2019

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Apr 15, 2019

This PR migrates the files in the state module to typescript. It also updates ethereumjs-account to make use of its type definitions.

Side question: What do you think about deprecating ethereumjs-account and adding it as a module to the VM? (it'd be possible to publish it separately to npm if desired when we migrate to a monorepo).

(To 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.

Relatively straight-forward, looks good, thanks Sina! 😄

@@ -1,10 +1,13 @@
const Buffer = require('safe-buffer').Buffer
const asyncLib = require('async')
Copy link
Member

Choose a reason for hiding this comment

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

That's actually a better naming.

if (it.node) {
this._cache = it.update({
val: val,
modified: modified,
deleted: deleted
})
} else {
this._cache = this._cache.insert(key, {
this._cache = this._cache.insert(keyHex, {
val: val,
modified: modified,
deleted: deleted
Copy link
Member

Choose a reason for hiding this comment

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

Looks good (cache.js).

const touched = this._touchedStack.pop()
if (!touched) {
throw new Error('Reverting to invalid state checkpoint failed')
}
Copy link
Member

Choose a reason for hiding this comment

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

Note: new error on revert introduced here.

const address = Buffer.from(addressHex, 'hex')
this.accountIsEmpty(address, (err, empty) => {
this.accountIsEmpty(address, (err: Error, empty: boolean) => {
if (err) {
next(err)
return
Copy link
Member

Choose a reason for hiding this comment

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

Ok (stateManager.ts).

const addressHex = address.toString('hex')
const keyHex = key.toString('hex')

this._stateManager.getContractStorage(address, key, (err, current) => {
this._stateManager.getContractStorage(address, key, (err: Error, current: any) => {
if (err) return cb(err)

let map = null
Copy link
Member

Choose a reason for hiding this comment

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

Ok (storageReader.ts).

return promisify(this._wrapped.accountIsEmpty.bind(this._wrapped))(address)
}

cleanupTouchedAccounts () {
cleanupTouchedAccounts (): Promise<void> {
return promisify(this._wrapped.cleanupTouchedAccounts.bind(this._wrapped))()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Ok (promisified.ts).

@s1na s1na merged commit b75d1da into v4 Apr 17, 2019
@s1na s1na mentioned this pull request Apr 17, 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.

3 participants