Metro-Byzantium Working Branch #161

jwasinger · 2017-07-28T20:02:03Z

This is the working branch for the implementation of Metropolis EIPs.
If you'd like to help, check out ethereumjs/ethereumjs-lib on Gitter.
Please be careful when pushing changes to this branch, especially if it reverts the work of others. If you aren't sure what to do next, ask in Gitter!

holgerd77 · 2017-08-09T09:34:48Z

axic · 2017-08-09T21:16:00Z

lib/index.js

-    for (var i = 1; i <= 4; i++) {
-      this.trie.put(new Buffer('000000000000000000000000000000000000000' + i, 'hex'), new Account().serialize())
+    for (var i = 1; i <= 7; i++) {
+      // temporarily skip 5 till EIP 198 added


I think there's no harm setting it. This code only ensures those accounts are touched and thus the first call to them won't incur an account creation fee.

axic · 2017-08-09T21:19:15Z

package.json

    "ethereumjs-tx": "1.3.3",
    "level": "^1.4.0",
    "leveldown": "^1.4.6",
    "levelup": "^1.3.2",
    "memdown": "^1.1.0",
    "minimist": "^1.1.1",
    "standard": "^5.2.2",
-    "tape": "^4.2.0"
+    "tape": "4.6.3"


Why a fixed version?

its mentioned in the commit: to downgrade tape to hide stack trace in test runner. tape versions higher 4.6.3 print a full js stack trace on failures, but its useless for our purposes (we'd want an EVM trace, not a test runner trace). Hiding it lets the test results fit in the travisCI log.

gasolin · 2017-08-10T12:16:38Z

lib/opFns.js

+    length = utils.bufferToInt(length)
+
+    if (returnDataOffset + length > runState.lastReturned.length) {
+      trap(ERROR.OUT_OF_GAS)


I think RETURNDATACOPY Out of range is not the same as Out of Gas

@gasolin I just took this from the EIP specification, there out-of-gas is mentioned: https://github.com/ethereum/EIPs/pull/211/files

"if `start + length` overflows or results in a value larger than `RETURNDATASIZE`, the current call stops in an out-of-gas condition" it looks right. I referred pyethereum implementation and they seems showing the different exception https://github.com/ethereum/pyethereum/blob/develop/ethereum/fastvm.py#L415

all EVM exceptions have the same behavior (state changes are reverted and remaining gas is consumed). The error message only determines what gets printed in the EVM trace log, though it would be good to standardize that.

@cdetrio Makes a lot of sense

axic · 2017-08-10T23:26:03Z

@jwasinger can you please rebase?

axic · 2017-08-14T22:49:29Z

lib/index.js


  if (this.opts.activatePrecompiles) {
-    for (var i = 1; i <= 4; i++) {
-      this.trie.put(Buffer.from(new BN(i).toArray('be', 20)), new Account().serialize())
+    for (var i = 1; i <= 7; i++) {


Note, also isPrecompiled() must be updated in ethereumjs-util.

note that we're still using ethereumjs-util v4.5, not the latest v5.0

axic · 2017-08-18T13:14:26Z

Should be using https://github.com/ethereumjs/rustbn.js (once ready) for the new ecc precompiles.

axic · 2017-08-18T18:15:50Z

lib/runCall.js

-        var totalGas = results.gasUsed.addn(returnFee)
+        var totalGas = results.gasUsed
+        if (!results.runState.vmError) {
+          var returnFee = results.return.length * fees.createDataGas.v


Should use a BN instance just in case.

axic · 2017-08-31T12:59:59Z

package.json

@@ -8,7 +8,7 @@
    "async-eventemitter": "^0.2.2",
    "ethereum-common": "0.1.0",
    "ethereumjs-account": "^2.0.3",
-    "ethereumjs-block": "^1.2.2",
+    "ethereumjs-block": "git+https://github.com/holgerd77/ethereumjs-block.git#byzantium-changes",


Isn't this merged?

No, that's work in progress, the PR you merged was just a prerequisite for this to be implemented. Finished EIP 100, will tomorrow have a look at EIP 649.

@axic Also, before merging the Metro-Byzantium changes into ethereumjs-block, I would like to have this PR #189 to be merged, otherwise new installations target the current network will break (at lease when there is a npm ethereumjs-block release with the changes).

jwasinger · 2017-09-18T02:43:52Z

lib/opFns.js

+    length = utils.bufferToInt(length)
+
+    runState.stopped = true
+    runState.returnValue = runState.lastReturned = memLoad(runState, offset, length)


It is not necessary to set runState.lastReturned here. The return data buffer is already set at https://github.com/jwasinger/ethereumjs-vm/blob/wip/metropolis/lib/opFns.js#L1022

axic · 2017-09-19T11:11:57Z

lib/precompiled/05-modexp.js

@@ -30,7 +30,7 @@ function getAdjustedExponentLength (E, eLen) {

 // Taken from https://stackoverflow.com/a/1503019
 function expmod (B, E, M) {


This function should use the reduction context in bn.js for proper speed.

I'm not a genius on this, but what I extracted from the comments around the stack overflow solution, I think this actually is a reduction context (correct me if I'm wrong, I have no idea what a reduction context actually really is. :-))

I initially wanted to use the bn.js reduction context after reading your comments on the initial PR, but I didn't figure out how to apply this to this special problem. I actually implemented the stack overflow solution after running into long running tests ending with out-of-memory errors, with the stackoverflow solution this went back to be pretty snappy then.

You can rewrite this, but please make sure before, that you're not replacing one valid solution with another, this actually took me quite some time to implement.

… naming scheme

… ethereumjs/common

…ERT (fixes RevertOpcodeInCreateReturns)

…cute

holgerd77

This needs a last update to package.json with the updated master branch dependencies, otherwise this can be merged.

cdetrio · 2017-09-25T14:25:26Z

@holgerd77 the commit ab90781 says there was no change in test failures. runState.lastReturned is set here

New Config Class

axic reviewed Aug 9, 2017

View reviewed changes

gasolin reviewed Aug 10, 2017

View reviewed changes

jwasinger force-pushed the wip/metropolis branch 2 times, most recently from 73e234f to 2d0521f Compare August 12, 2017 08:26

axic reviewed Aug 14, 2017

View reviewed changes

holgerd77 force-pushed the wip/metropolis branch 2 times, most recently from 90ff061 to 86611c9 Compare August 16, 2017 11:09

axic reviewed Aug 18, 2017

View reviewed changes

holgerd77 force-pushed the wip/metropolis branch 2 times, most recently from b97664c to 48ae163 Compare August 23, 2017 08:47

axic reviewed Aug 31, 2017

View reviewed changes

holgerd77 mentioned this pull request Sep 8, 2017

BlockchainTests status for wip/metropolis branch #198

Closed

holgerd77 force-pushed the wip/metropolis branch from d670471 to d0096e5 Compare September 11, 2017 15:48

holgerd77 mentioned this pull request Sep 11, 2017

[MASTER-RELEASE] Byzantium changes ethereumjs/ethereumjs-block#32

Merged

axic mentioned this pull request Sep 11, 2017

[WIP] Introduce modexp precompile (EIP101) #101

Closed

holgerd77 force-pushed the wip/metropolis branch 3 times, most recently from 3c09f3c to ea592fd Compare September 15, 2017 10:56

jwasinger commented Sep 18, 2017

View reviewed changes

holgerd77 changed the title ~~Metropolis Working Branch~~ Metro-Byzantium Working Branch Sep 18, 2017

holgerd77 mentioned this pull request Sep 18, 2017

[META] Metro-Byzantium Versioned Releases #205

Closed

axic reviewed Sep 19, 2017

View reviewed changes

holgerd77 force-pushed the wip/metropolis branch 3 times, most recently from 5da7b23 to be986bc Compare September 21, 2017 12:32

jwasinger mentioned this pull request Sep 21, 2017

[META] Byzantium Implementation Progress #209

Closed

20 tasks

holgerd77 and others added 13 commits September 22, 2017 11:25

Update gas costs for EC mult precompile

f35ef3e

Fixed double import

b2ce0a7

Renamed curve operation precompile files to a simpler/more consistent…

b68374c

… naming scheme

Replaced hard-coded gas costs for new precompiles with constants from…

1e77ce9

… ethereumjs/common

fix modexp when modulus is one

8a88d7f

fix remaining modexp test case failures

497288f

lint fixes

e24908e

Add error message to returndata buffer for CREATE call in case of REV…

63f6cdb

…ERT (fixes RevertOpcodeInCreateReturns)

don't run precompile if gasLimit=0

3b288b5

add better check to make sure that precompiles have enough gas to exe…

5e9d115

…cute

Added Byzantium ethereumjs-blockchain GitHub version (temporary)

b4b8a2f

correctly calculate uncle and nephew reward

a7179eb

Readded ethUtil in GeneralStateTestsRunner

5d7377f

holgerd77 force-pushed the wip/metropolis branch from 74ede8a to 5d7377f Compare September 22, 2017 09:28

holgerd77 added 6 commits September 22, 2017 11:33

Fix linting

9b4e205

Fix run-blockchain example

0b2774a

Added a note on Byzantium support to README

a8f86f6

Updated CHANGELOG entry for Unreleased version

bc0ad30

Fix linting

73c0e8c

Temporarily removed randomStatetest642 state test

5ed376f

holgerd77 previously approved these changes Sep 25, 2017

View reviewed changes

simplify revert return data

4a3c0f6

cdetrio dismissed holgerd77’s stale review via 4a3c0f6 September 25, 2017 13:28

update deps for byzantium release

fba7652

cdetrio approved these changes Sep 25, 2017

View reviewed changes

jwasinger merged commit 45d73f0 into ethereumjs:master Sep 25, 2017

evertonfraga added the package: vm label Feb 27, 2020

holgerd77 mentioned this pull request Jun 25, 2020

SpuriousDragon HF Support #791

Merged

15 tasks

holgerd77 added a commit that referenced this pull request Dec 1, 2020

Merge pull request #161 from ethereumjs/integrate-singleton-config-class

e6581d2

New Config Class

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metro-Byzantium Working Branch #161

Metro-Byzantium Working Branch #161

jwasinger commented Jul 28, 2017

holgerd77 commented Aug 9, 2017 •

edited

Loading

axic Aug 9, 2017

axic Aug 9, 2017

cdetrio Aug 10, 2017

gasolin Aug 10, 2017

holgerd77 Aug 10, 2017

gasolin Aug 10, 2017 •

edited

Loading

cdetrio Aug 11, 2017

holgerd77 Aug 11, 2017

axic commented Aug 10, 2017

axic Aug 14, 2017

cdetrio Aug 15, 2017

axic commented Aug 18, 2017

axic Aug 18, 2017

axic Aug 31, 2017

holgerd77 Aug 31, 2017

holgerd77 Sep 1, 2017

jwasinger Sep 18, 2017

axic Sep 19, 2017

holgerd77 Sep 19, 2017

holgerd77 left a comment

cdetrio commented Sep 25, 2017

		@@ -30,7 +30,7 @@ function getAdjustedExponentLength (E, eLen) {

		// Taken from https://stackoverflow.com/a/1503019
		function expmod (B, E, M) {

Metro-Byzantium Working Branch #161

Metro-Byzantium Working Branch #161

Conversation

jwasinger commented Jul 28, 2017

holgerd77 commented Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gasolin Aug 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axic commented Aug 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axic commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerd77 left a comment

Choose a reason for hiding this comment

cdetrio commented Sep 25, 2017

holgerd77 commented Aug 9, 2017 •

edited

Loading

gasolin Aug 10, 2017 •

edited

Loading