Update `ethereumjs-util` to v7 #748

ryanio · 2020-05-16T21:27:33Z

This PR updates ethereumjs-util to v7 for various improvements and benefits (see changelog) including keccak prebuilds.

This PR also:

Switches some last ethereumjs-util wildcard imports for explicit imports
Upgrades typescript dep

codecov · 2020-05-16T23:12:00Z

Codecov Report

Merging #748 into vm/release-v5 will decrease coverage by 0.01%.
The diff coverage is 90.32%.

@@                Coverage Diff                @@
##           vm/release-v5     #748      +/-   ##
=================================================
- Coverage          90.31%   90.29%   -0.02%     
=================================================
  Files                 46       44       -2     
  Lines               3004     2999       -5     
  Branches             470      470              
=================================================
- Hits                2713     2708       -5     
  Misses               194      194              
  Partials              97       97

Flag	Coverage Δ
#account	`94.11% <ø> (ø)`
#block	`82.47% <83.33%> (-0.03%)`	⬇️
#blockchain	`88.81% <ø> (ø)`
#common	`94.11% <ø> (-0.26%)`	⬇️
#tx	`94.16% <ø> (+0.13%)`	⬆️
#vm	`92.53% <100.00%> (ø)`

Impacted Files	Coverage Δ
packages/tx/src/transaction.ts	`94.73% <ø> (ø)`
packages/block/src/header-from-rpc.ts	`87.50% <66.66%> (ø)`
packages/block/src/header.ts	`66.94% <66.66%> (-0.28%)`	⬇️
packages/block/src/block.ts	`71.15% <100.00%> (ø)`
packages/block/src/from-rpc.ts	`86.66% <100.00%> (ø)`
packages/vm/lib/evm/opFns.ts	`89.60% <100.00%> (ø)`
packages/common/src/chains/index.ts
packages/common/src/hardforks/index.ts
packages/common/src/genesisStates/index.ts
packages/tx/src/index.ts	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b9a096...9b4c55e. Read the comment docs.

evertonfraga · 2020-05-18T10:24:39Z

packages/block/src/header.ts

 import { Blockchain, BlockHeaderData, BufferLike, ChainOptions, PrefixedHexString } from './types'
 import { Buffer } from 'buffer'
 import { Block } from './block'

+const { BN } = require('ethereumjs-util')
+import IBN = require('bn.js')


What's the back story on BN vs. IBN?

I'm not too sure what was going on, I was just trying to resolve some typescript build errors. The error was "Type originates at this import" regarding bn.js in ethereumjs-util externals.d.ts - might be some confusion because the bn.js types are defined in a @types package. I did check and they are in deps and not devDeps for ethereumjs-util though so I would have hoped for them to be included in a clean way, but it did not let me simply do import { BN } from 'ethereumjs-util'.

evertonfraga · 2020-05-18T10:33:20Z

The changes look good! I just wanted to understand the context of BN vs. IBN there.

I will also learn more about the 6.x to 7.0.0 changes and discussion around 7.0.1, so my review can be more solid.

:)

ryanio · 2020-05-18T16:22:50Z

Thanks!

Check out the ethereumjs-util/CHANGELOG.md

bn.js issues regarding 7.0.1 still being explored here: ethereumjs/ethereumjs-wallet#121 (comment) (edit: concluded with patch fix releases in bn.js v4 and v5)

holgerd77 · 2020-05-25T10:48:39Z

Please take ethereumjs/ethereumjs-util#253 into account respectively upgrade on a new release before merging

holgerd77 · 2020-05-26T19:40:16Z

Let me know if you need a review here.

ryanio · 2020-05-27T02:23:48Z

sounds good thanks, I think the PR is ready for review other than the weird BN type interface issue I outlined above, I will give it another look to see if it is something I can fix here or perhaps some additional configuration we need in ethereumjs-util in passing the BN types down.

holgerd77 · 2020-05-27T08:13:44Z

@ryanio @evertonfraga Ah, sorry, didn't re-read the thread so closely, yeah, you're right, the BN issue should likely figured out here.

Not sure if this helps, just posting here for the marginal chance that it could: 😋

On a clean ethereumjs-util v7.0.2 install this works without problems with ts-node:

$ ts-node
> import { BN } from 'ethereumjs-util'
{}
> BN
[Function: BN] {
  BN: [Circular],
  wordSize: 26,
  isBN: [Function: isBN],
  max: [Function: max],
  min: [Function: min],
  red: [Function: red],
  _prime: [Function: prime],
  mont: [Function: mont]
}
>

ryanio · 2020-05-27T18:20:05Z

@holgerd77 ah thanks for that.

OK I found some more information here: https://stackoverflow.com/a/49256285

Basically, block has esModuleInterop: true which is causing the compiler to do an extra check that throws a bunch of errors like:

TSError: ⨯ Unable to compile TypeScript:
src/header.ts:144:25 - error TS2351: This expression is not constructable.
  Type 'typeof BN' has no construct signatures.

144     const blockTs = new BN(this.timestamp)
                            ~~

  node_modules/ethereumjs-util/dist/externals.d.ts:7:1
    7 import * as BN from 'bn.js';
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Type originates at this import. A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.
src/header.ts:145:26 - error TS2351: This expression is not constructable.
  Type 'typeof BN' has no construct signatures.

145     const parentTs = new BN(parentBlock.header.timestamp)
                             ~~

  node_modules/ethereumjs-util/dist/externals.d.ts:7:1
    7 import * as BN from 'bn.js';
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Type originates at this import. A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.
src/header.ts:146:27 - error TS2351: This expression is not constructable.
  Type 'typeof BN' has no construct signatures.

146     const parentDif = new BN(parentBlock.header.difficulty)
                              ~~

Removing esModuleInterop (or setting to false) fixes the issue, however I'm not sure if there are any other consequences (i.e. why it was added). However our new ethereumjs-config tsconfig (as compared to old) will have esModuleInterop set to true so it may come back to haunt us again.

edit: pushed a commit (ffc943b) to try this out (cc @evertonfraga)

…e compiler option `esModuleInterop`

evertonfraga · 2020-06-01T12:19:03Z

packages/blockchain/package.json

@@ -41,7 +41,7 @@
    "ethashjs": "~0.0.7",
    "ethereumjs-block": "~3.0.0",
    "ethereumjs-common": "^1.5.0",
-    "ethereumjs-util": "~6.1.0",
+    "ethereumjs-util": "^7.0.2",


I have reverted it back to a caret range, to comply with the other monorepo packages.
Looking at old versions of this line led to this PR ethereumjs/ethereumjs-blockchain#33, proving this was a protection mechanism for a hardfork changes.

Any opinions here, @holgerd77?

This is from some really old ages 😄 , no reason for special rules here any more.

Think its very beneficial if we end up using caret ranges at least for our own internal dependencies all over the place and just follow semver without exception. This was quite different some time (2-3 years) ago but we have gotten quite some way here and I think we are now with most (all?) of the (actively worked on) libraries in a semver-compliant release process.

evertonfraga

Unless there's any specific reason to keep ethereumjs-util under a tilde version, we're good to go!

evertonfraga · 2020-06-01T12:23:23Z

I will test the esModuleInterop change now and see if there's any implication on the Block side.

holgerd77

Ok, also had another short look, I will merge this in now since this is so relatively heavy and I would like to do some rebases on top.

evertonfraga reviewed May 18, 2020

View reviewed changes

ryanio added the PR state: needs review label May 19, 2020

holgerd77 mentioned this pull request May 26, 2020

Update ethereumjs-util dependency from v6.1.0 to v7.0.2 ethereumjs/ethashjs#22

Closed

ryanio force-pushed the update-ethereumjs-util-to-v7 branch 3 times, most recently from 4142143 to 4108730 Compare May 26, 2020 17:24

update to ethereumjs-util 7.0.2

e9f6396

ryanio force-pushed the update-ethereumjs-util-to-v7 branch from 4108730 to e9f6396 Compare May 26, 2020 17:55

ethereumjs-util BN ts build compatibility: remove use of IBN, remov…

ffc943b

…e compiler option `esModuleInterop`

holgerd77 mentioned this pull request May 29, 2020

VM v5 Release Planning #681

Closed

27 tasks

Use caret for ethereumjs-util

9b4c55e

evertonfraga self-requested a review June 1, 2020 12:15

evertonfraga reviewed Jun 1, 2020

View reviewed changes

evertonfraga approved these changes Jun 1, 2020

View reviewed changes

holgerd77 approved these changes Jun 1, 2020

View reviewed changes

holgerd77 merged commit 09de6f4 into vm/release-v5 Jun 1, 2020

holgerd77 deleted the update-ethereumjs-util-to-v7 branch June 1, 2020 14:52

evertonfraga mentioned this pull request Jun 9, 2020

VM v5 changes #767

Merged

evertonfraga added this to the VM v5 milestone Jun 12, 2020

holgerd77 added package: monorepo and removed package: monorepo labels Sep 4, 2020

holgerd77 added package: account package: block package: blockchain package: common package: tx package: vm labels Sep 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `ethereumjs-util` to v7 #748

Update `ethereumjs-util` to v7 #748

ryanio commented May 16, 2020 •

edited

codecov bot commented May 16, 2020 •

edited

evertonfraga May 18, 2020

ryanio May 18, 2020

evertonfraga commented May 18, 2020

ryanio commented May 18, 2020 •

edited

holgerd77 commented May 25, 2020

holgerd77 commented May 26, 2020

ryanio commented May 27, 2020

holgerd77 commented May 27, 2020

ryanio commented May 27, 2020 •

edited

evertonfraga Jun 1, 2020

holgerd77 Jun 1, 2020

evertonfraga left a comment

evertonfraga commented Jun 1, 2020

holgerd77 left a comment

Update ethereumjs-util to v7 #748

Update ethereumjs-util to v7 #748

Conversation

ryanio commented May 16, 2020 • edited

codecov bot commented May 16, 2020 • edited

Codecov Report

evertonfraga May 18, 2020

Choose a reason for hiding this comment

ryanio May 18, 2020

Choose a reason for hiding this comment

evertonfraga commented May 18, 2020

ryanio commented May 18, 2020 • edited

holgerd77 commented May 25, 2020

holgerd77 commented May 26, 2020

ryanio commented May 27, 2020

holgerd77 commented May 27, 2020

ryanio commented May 27, 2020 • edited

evertonfraga Jun 1, 2020

Choose a reason for hiding this comment

holgerd77 Jun 1, 2020

Choose a reason for hiding this comment

evertonfraga left a comment

Choose a reason for hiding this comment

evertonfraga commented Jun 1, 2020

holgerd77 left a comment

Choose a reason for hiding this comment

Update `ethereumjs-util` to v7 #748

Update `ethereumjs-util` to v7 #748

ryanio commented May 16, 2020 •

edited

codecov bot commented May 16, 2020 •

edited

ryanio commented May 18, 2020 •

edited

ryanio commented May 27, 2020 •

edited