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 tracker #479

Merged
merged 30 commits into from
May 27, 2019
Merged

V4 tracker #479

merged 30 commits into from
May 27, 2019

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Mar 22, 2019

This PR tracks the changes planned for v4 (see #455). It also acts as a base branch for the pull requests, to prevent mixing experimental development in master. This'd allow us to maintain v3, fix bugs and do releases independently of the changes made here, and also have parallel release candidates for v4.

@coveralls
Copy link

coveralls commented Mar 22, 2019

Coverage Status

Coverage decreased (-0.3%) to 94.95% when pulling 00f99c2 on v4 into 068db32 on master.

* Bump version to 4

* Upgrade babel to 7, use transform-runtime plugin

* evm: add Interpreter class (alternative to runCall)

* evm: add Loop, a promisified version of runCode

* evm: execute opFn calls through interpreter

* evm/loop: mv pc range check out of hot loop

* evm: use Interpreter and Loop in runCall and runCode

* evm: use promisified state in interpreter and loop

* evm/loop: break run into initRunState, runStep, handleOp, getOpHandler

* evm/loop: minor fix

* test: drop test cases assuming runCall

* evm: use util.promisify package

* evm: Mv some logic from executeMessage to private methods

* Fix linting errors
@s1na s1na mentioned this pull request Apr 1, 2019
* evm: add EEI class, use in opFns to get env values

* evm: add selfdestruct, finish, revert and useGas to eei

* evm: use storageStore, log eei methods in opFns

* evm: add call method to eei, fix getAddress

* evm: store selfdestructs in txRunState

* evm: add callCode method to eei

* evm: add callStatic, callDelegate methods to eei

* eei: get isStatic from env

* eei: refactor getting code in callCode, callDelegate

* evm: refactor gas and output in call opFns

* opFns: replace subGas with eei.useGas

* eei: add create and create2

* evm: pass txRunState to loop in runCode

* Fix linting error

* Revert moving selfdestruct to txRunState
lib/evm/interpreter.js Outdated Show resolved Hide resolved
lib/evm/interpreter.js Outdated Show resolved Hide resolved
const PStateManager = require('../state/promisified')

module.exports = class Interpreter {
constructor (vm, txContext, block, storageReader) {
Copy link
Member

Choose a reason for hiding this comment

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

General note: I think every access through _vm here is an indicator that we have some misplacement of code or suboptimal structure.

@s1na s1na mentioned this pull request Apr 8, 2019
* Detach list of precompiles from VM

* evm: add getPrecompile method to interpreter
@s1na s1na mentioned this pull request Apr 9, 2019
* Promisify runTx internals

* Fix linting error
* Drop babel deps, add initial typescript config

* Add prettier as dep, add format scripts

* Mv bloom/index.js to bloom/index.ts

* Migrate bloom to ts, limit its api to buffers

* Mv lib/evm/stack.js to lib/evm/stack.ts

* Migrate stack to ts, limit it to BN

* Mv lib/evm/memory.js to lib/evm/memory.ts

* Migrate evm memory to ts

* Build:dist before coverage and api browser tests

* Use tester dist flag for coverage tests
@s1na s1na mentioned this pull request Apr 15, 2019
* Mv lib/state/promisified.js to lib/state/promisified.ts

* Change state file types to ts

* Migrate state module to ts
@no-1ne
Copy link

no-1ne commented May 14, 2019

Looks like v4 release is on the horizon. Looking forward to it.

Visiting the repo after long time & you have made awesome strides with ethereum VM in JS, making ethereum development very smooth and hopefully helping blockchain reach its true potential

@holgerd77
Copy link
Member

@startupgurukul thanks! 😀

s1na added 3 commits May 15, 2019 10:17
* Change runCode filetype to ts

* Migrate runCode to ts
* Change runCall filetype to ts

* Migrate runCall to ts
* Change runTx filetype to ts

* Migrate runTx to ts
s1na added 4 commits May 21, 2019 16:35
* Change runBlock filetype to ts

* Migrate runBlock to ts
* Change runBlockchain filetype to ts

* Migrate runBlockchain to ts
* Use typescript configs

* Fix tslint errors

* Add prettier config

* Fix formatting errors
@holgerd77 holgerd77 mentioned this pull request May 25, 2019
18 tasks
@holgerd77
Copy link
Member

@s1na if you feel comfortable with that I would suggest we try to do the master merge here on Monday and do subsequent cleanup work towards the master branch. Then we have two days where the dust can settle on the master branch and we see through a bit again on Wednesday. 😄 😋

Can you do some master rebase here (this would be the procedure, right?)?

Let me know if you have any reservations, in doubt we are not completely bound to this date in any way and should rather make sure that the process is robust and we are confident with the integration work.

@s1na
Copy link
Contributor Author

s1na commented May 27, 2019

That sounds good to me. Only would it be okay if I merge master into this branch, and later on we merge v4 into master (instead of rebase)? Due to the large number of commits, and because there are conflicts with master, rebasing can get difficult. I also took care to squash and merge the PRs, so we don't have tons of minor commits polluting the commit history.

@holgerd77
Copy link
Member

Ok, that sounds good!

@lgtm-com
Copy link

lgtm-com bot commented May 27, 2019

This pull request introduces 1 alert and fixes 1 when merging 074c0b1 into 068db32 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 1 for Useless conditional

Comment posted by LGTM.com

@lgtm-com
Copy link

lgtm-com bot commented May 27, 2019

This pull request introduces 1 alert and fixes 1 when merging 00f99c2 into 068db32 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 1 for Useless conditional

Comment posted by LGTM.com

@s1na
Copy link
Contributor Author

s1na commented May 27, 2019

I have merged master into v4. Should be ready for evaluation and merging 😄

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.

Hooray!!! 🎉 🎉 🎉

This might still not be the optimal structure, but it will provide a much much more solid ground for all future work and comes with the most significant improvements this library has seen since I am involved in the project, thanks a lot Sina!!!

I've had a final look at all files, looks good.

Feel free to merge. 😄

@s1na s1na merged commit 7680b82 into master May 27, 2019
@no-1ne
Copy link

no-1ne commented May 27, 2019

Yay! can you also push the latest to npm pls

@holgerd77
Copy link
Member

@startupgurukul probably two days if nothing unexpected trigger a delay until a beta release on npm

@holgerd77
Copy link
Member

@s1na npm run testStatePetersburg is actually failing for me on master for all tests, even with a clean npm install + previously delete node_modules folder. Some example output:

$ npm run testStatePetersburg

> ethereumjs-vm@3.0.0 testStatePetersburg
> npm run build:dist && node ./tests/tester -s --fork='Petersburg' --dist

> ethereumjs-config-build

+ exec tsc -p ./tsconfig.prod.json
TAP version 13
# GeneralStateTests
# file: addNonConst test: addNonConst
not ok 1 the state roots should match
  ---
    operator: equal
    expected: |-
      '2d6da043f5cfd9fe3b1feab48970d855bf5570f5316492869642b228d8bed3ca'
    actual: |-
      '3c3f05727bc49ff870f5e780c5d00426d9c153de7e23443e70ec2da43d7f731c'
    at: replenish (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/async/dist/async.js:998:17)
  ...
not ok 2 the state roots should match
  ---
    operator: equal
    expected: |-
      'e396abd1e58496cc86a1a5bc31af70c98926cceef3738e1189af4c226b61da15'
    actual: |-
      '3c3f05727bc49ff870f5e780c5d00426d9c153de7e23443e70ec2da43d7f731c'
    at: replenish (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/async/dist/async.js:998:17)
  ...

Any idea?

@s1na
Copy link
Contributor Author

s1na commented May 28, 2019

@holgerd77 Hmm 🤔 I tried on a fresh clone and the tests seem to pass. I wonder what could be the cause. Could you maybe add an console.log(err) here:

https://github.com/ethereumjs/ethereumjs-vm/blob/9f96231cf7b4b185368092416aa7af2c30d2f90b/lib/runTx.ts#L95-L96

to see if any error is occuring?

@holgerd77
Copy link
Member

@s1na Ah yes, great hint:

+ exec tsc -p ./tsconfig.prod.json
TAP version 13
# GeneralStateTests
# file: addNonConst test: addNonConst
TypeError: bloom_1.default is not a constructor
    at txLogsBloom (/ethereumjs-vm/dist/runTx.js:250:17)
    at VM.<anonymous> (/ethereumjs-vm/dist/runTx.js:157:37)
    at step (/ethereumjs-vm/dist/runTx.js:32:23)
    at Object.next (/ethereumjs-vm/dist/runTx.js:13:53)
    at fulfilled (/ethereumjs-vm/dist/runTx.js:4:58)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)
not ok 1 the state roots should match
  ---
    operator: equal
    expected: |-
      '2d6da043f5cfd9fe3b1feab48970d855bf5570f5316492869642b228d8bed3ca'
    actual: |-
      '3c3f05727bc49ff870f5e780c5d00426d9c153de7e23443e70ec2da43d7f731c'
    at: replenish (/node_modules/async/dist/async.js:998:17)

@s1na
Copy link
Contributor Author

s1na commented May 28, 2019

Maybe the typescript compiler hasn't overwritten previously built files. Could you try removing dist and then running the tests?

@holgerd77
Copy link
Member

Ah, yes, works: 😄

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.

8 participants