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

Monorepo: Develop -> Master - Breaking Release Transition PR #1943

Merged
merged 48 commits into from
Jun 8, 2022
Merged

Conversation

holgerd77
Copy link
Member

Part of #1914, final execution on #1717

This is the PR to make the develop branch - where all work for the v6 release series has been undertaken - will be merged into the master branch. With the merge of this branch there will be a transition to the v6 series becoming the new work state.

All eventually necessary v5 work will/should from now on be done towards the new v5-maintenance branch, branched off after the final round of v5 releases done in #1927 (and some follow-up work in #1929, #1931 and #1932).

Still open feature PRs towards develop should be re-applied/re-targeted towards master (likely via cherry-picking) after this PR has been merged.

Woohoo. 🙏 🎉

Thanks everyone for the great work being done here! ❤️

We are a bit late on schedule, so PR will be merged after 1-2 reviews/approvals.

jochem-brouwer and others added 30 commits June 3, 2022 17:34
* vm: add codeHash db prefix to stateManager

* vm: tests: make tests use StateManager instead of trie at utilization
vm: tests: remove now passing test from skipTests

* vm: fix tests / fix runBlock/runBlockchain tests

* vm: tests: fix cleaning up untouched accounts in tests

* vm -> StateManager: added comment on CODEHASH_PREFIX

Co-authored-by: holgerd77 <Holger.Drewes@gmail.com>
Co-authored-by: steveluscher <github@steveluscher.com>
…orks to VM (#1649)

* common: remove onlySupported and onlyActive hf options

* vm: move supportedHardforks option from common

* common: remove supportedHardforks option

* common: update README for supportedHardforks removal

* vm: update README for supportedHardforks addition

* common: cleanup in src/index.ts

* vm: refactor supportedHardforks per PR feedback

* common: remove _chooseHardfork method per PR feedback

* vm: updated README per PR feedback

* vm: supportedHardforks local variable, switch to enum
* common: delete deprecated hardforkBlock method

* common: adapt tests to use hardforkBlockBN

* common: delete deprecated nextHardforkBlock method

* common/tests: rename nextHardforkBlock() calls
to nextHardforkBlockBN()

* common: adapt tests to use BN.eqn(number)

* common: delete deprecated networkId() method

* common: delete tests for networkId()

* common: delete deprecated chainId() method

* common: delete tests for .chainId()

* delete deprecated forCustomChain meethod

* common: delete forCustomChain() test

* blockchain: switch forCustomChain to Common.custom

* client: switch fromCustomChain to Common.custom

* tx: switch forCustomChain to Common.custom

* VM: switch forCustomChain to Common.custom

* common: internalize _getInitializedChains() method

* common: delete deprecaed chains/index.ts file

* client: switch to Common._getInitializedChains

* common: remove genesisStates/index.ts

* common: remove test for genesisStates
* common: rename nextHardforkBlockBN()
to nextHardforkBlock()

* common/tests: rename nextHardforkBlockBN()
to nextHardforkBlock()

* client: rename common.nextHardforkBlockBN()
to common.nextHardforkBlock()

* devp2p: rename common.nextHardforkBlockBN()
to common.nextHardforkBlock()

* common: rename hardforkBlockBN()
to hardforkBlock()

* common/tests: rename common.hardforkBlockBN()
to common.hardforkBlock()

* block: rename common.hardforkBlockBN()
to common.hardforkBlock()

* client: rename common.hardforkBlockBN()
to common.hardforkBlock()

* blockchain: rename common.hardforkBlockBN()
to common.hardforkBlock()

* devp2p: rename common.hardforkBlockBN()
to common.hardforkBlock()

* vm: rename common.hardforkBlockBN()
to common.hardforkBlock()

* common: rename chainIdBN()
to chainId()

* common/tests: rename common.chainIdBN()
to common.chainId()

* client: rename common.chainIdBN()
to common.chainId()

* blockchain: rename common.chainIdBN()
to common.chainId()

* devp2p: rename common.chainIdBN()
to common.chainId()

* tx: rename common.chainIdBN()
to common.chainId()

* vm: rename common.chainIdBN()
to common.chainId()

* common: rename networkIdBN() to networkId()

* common/test: rename common.networkIdBN()
to common.networkId()

* client: rename common.networkIdBN()
to common.networkId()

* block: rename common.networkIdBN()
to common.networkId()
* Replace BN.js with bigints in VM

* Fix byteLength computation in EXP handler

* Fix toTwos helper

* Compute TWO_POWE256 using math instead of hex

* Update packages/util/src/constants.ts

Co-authored-by: Ryan Ghods <ryan@ryanio.com>

* Remove unused variable

* Fix exponent byte length calc

* Fix exp overflow

* Fix precompile bigint conversions

* Fix more bigint conversions

* Fix EXP opcode handler

* Fix logic bug in signextend

* vm/gas: fix EXTCODECOPY gas

* vm/gas: fix sha256 gas

* vm/gas: sha256 const -> let

* vm: lint

* vm/logic: fix sdiv/smod and fromTwos/toTwos

* vm/logic: fix SIGNEXTEND

* vm/logic: fix CALLDATALOAD padding

* vm/logic: remove weird comment

* Fix SAR opcode handler

* Use bufferToBigInt in Push opcode handler

* use bufferToBigInt everywhere

* Fix missing bufferToBigInt import

* Check for edge case in expmod

* Ignore prettier

* Update browser tsconfig to es2020 lib

* Remove dup ES2020 targets

* attempt to dedupe "target" and "lib" tsconfig values

* Update karma config to target es2020 in parser

* Various test fixes

* Lint and BN fixes

* Add bigint helpers to util

* lint fixes

* various bigint helper additions

* Lint fixes

* Fix bnToBigInt

* Lint/test fixes

* Switch Xn to BigInt(X)

* lint

* lint

* More Xn to BigInt(X) moves

Co-authored-by: Ryan Ghods <ryan@ryanio.com>
Co-authored-by: Jochem Brouwer <jochem.brouwer@nslogin.nl>
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
* Tx: Remove baseTransaction.transactionType

* tx: remove get senderR()

* tx: remove test for senderR()

* tx: remove senderS()

* tx: remove test for senderS()

* tx: remove yParity / replace with v

* tx: remove test for yParity()

* Tx: remove fromRlpSerializedTx()

* Tx: remove test for fromRlpSerializedTx()

* Tx: remove _unsignedTxImplementsEIP155

* Tx: Remove _signedTxImplementsEIP155()

* Tx: Remove TransactionFactory.getTransactionClass

* Tx: Remove test for .getTransactionClass()
* common: set default hardfork to london

* vm: set default hardfork to london

* vm: update tests after default hardfork change

* vm: update default hardfork in tester config

* block: update default hardfork and fix tests

* blockchain: fix tests for london as default hardfork

* tx: set default hardfork to london

* block: fix for uncles at hardfork transition test

* blockchain: fix for hardfork in reorg test

* ethash: fix ethash tests for default hardfork change

* tx: remove README section about hardforks

* client: fix client tests for default hardfork change

* block: rename testdata files for pre-london

* blockchain: rename testdata files for pre-london

* vm: fix examples for London hardfork change

Co-authored-by: Emerson Macro <emersonmacro@gmail.com>
* common: rename hardforkIsActiveOnChain to isIncludedHardfork

* common: remove activeHardfork and activeHardforks

* common: adapt _getHardfork, remove isIncludedHardfork

* common: type fixes in forkHash
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
…er (#1765)

* client fix parse: now with common default being london, explicitly setHardforkByBlockNumber(0) when creating genesis header
* vm: move new modifyAccountFields to baseStateManager
* fix lint
* util: use bigints. Expose new secp constants

* common: use bigints

* tx: use bigints

* lint: fix util, tx, common

* tx: fix isSigned

* update comment block style to be picked up by vscode

* remove doc typo (extra `)

* tx: add isSigned() tests to base.spec.ts to iterate across txTypes

* block: bn to bigint changes wip

block: finish header changes

block: test updates

block: test fixes

Partial difficulty fixes

* block: fix homestead difficulty

* block: fix >= Byzantium difficulty

* BigInt conversion fixes

* block: update st.ok to st.equals

* Update readme to use bigints

* ethash: bn to bigint

* blockchain: wip bn to bigint changes

* devp2p: bn to bigint

* vm: wip remaining bn -> bigint

* vm: more test fixes

* fix runTx negative value args test, ensure balance does not go negative when using skipBalance

* vm: lint:fix

* re-add newline

* Blockchain: small rebase fix

* vm: Fix API tests

* client: bn to bigint updates in source

* client: various fixes

* client: last fixes

* client: integration test fixes

* replace st.ok usage with st.equal

* normalize st.equals to st.equal

* update toType cases

* nits, lint

* fix vm benchmarks

* client: fix fullEthereumService tests

* touch ups, fix miner integration test

* use prefix increment

* client: fix full sync test

* test fixes

* Fix reorg logic bug

* bnTo... function renaming and cleanup

* Goodbye bn.js

* reverse changelog changes

* nits

* remove more BN, more nits

* turn off restrict-plus-operands and remove disable overrides (does not work properly)

* more nits, update package-lock

* fix build errors, re-add @types/bn.js for rlp v2 export to fix build (will be removed in subsequent PR for rlp)

* replace miller-rabin (bn.js) with bigint-crypto-utils isProbablyPrime

* more nits / fixes

* Fix misplaced paren

* Fix miner test

* more nits

* fix buffer to big int

* set expectedTests back to >= comparison

* last fixes

* Removed re-introduced Common methods

Co-authored-by: Paul Miller <paul@paulmillr.com>
Co-authored-by: Ryan Ghods <ryan@ryanio.com>
Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
Co-authored-by: ScottyPoi <scott.simpson@ethereum.org>
* blockchain: remove storing unsettled promise

* vm: remove storing unsettled promises. make copy() async

* client: update to new async changes

* block: doc nit

* nits

* fix vm benchmarks, add _init to mockchain

* Make blockchain and vm constructors protected

* lint

* vm: update new tests to await

Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
* vm: v6 breaking changes

* update benchmark util to use gasUsed as bigint

* VM: fixed gasUsed rebase related bug

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
* add eip3074

* remove extra space

* use vm.create instead of new vm

Co-authored-by: Ryan Ghods <ryan@ryanio.com>
* util: move misplaced functions

* add unpadBuffer back

* Fix tttttttttttttypo

* make unpadHexString return Hex String!
* blockchain/tests: remove tests for getHead()

* vm: change blockchain.getHead()
to blockchain.getIteratorHead()

* vm: remove tests for blockchain.getHead()

* client: change blockchain.getHead()
to blockchain.getIteratorHead()

* blockchain: remove setHead()
for setIteratorHead()

* blockchain: edit text of
setIteratorHead() test

* blockchain: rename getLatestHeader()
to getCanonicalHeadHeader()

* blockchain/tests: rename getLatestHeader()
to getCanonicalHeadHeader()

* client: rename blockchain.getLatestHeader()
to blockchain.getCanonicalHeadHeader()

* client/tests: rename blockchain.getLatestHeader()
to blockchain.getCanonicalHeadHeader()

* blockchain/tests: rename getLatestBlock()
to getCanonicalHeadBlock()

* client: rename blockchain.getLatestBlock
to blockchain.getCanonicalBlockHeader()

* client/tests: rename blockchain.getLatestBlock
to blockchain.getCanonicalBlockHeader()

* blockchain: remove get meta()

* VM: Remove blockchain.meta from tests

* Blockchain: Remove blockchain.meta from tests

* blockchain: fix linting errors

* BN to BigInt misses from rebase

* BN to BigInt fixes + lint

* Fix missing name replacement

* lint

* Add back blockchain testrunner last header check

* Add test for call with no gas

* Add back correct renamed functions

* lint fixes

Co-authored-by: ScottyPoi <scott.simpson@ethereum.org>
* move ethash depBrowserWorkaround fix for bigint-crypto-utils to blockchain karma.conf.js
  (karma has trouble with the default browser esm import without es6 transform, so it is easier to tell it to use umd)

* new DefaultStateManager will init a trie if not supplied, so we don't need to init one here

* remove backwards compatibility note (we will keep this property)

* trie: remove old backwards compat methods

* client: do undefined check first

* vm: undo doc change

* client: improve receipt size calculation

* vm: resolve more todos

* vm: more hardfork enum usage

* dedupe receipt rlp encoding

* lint

* runCall, runCode: add more param typedocs, simplify

* also add karma workaround to vm
* Abstract stateManager out and add vmState to wrap its context in the vm

* Move the baseStateManager to the new package

* package cleanup

* further cleanup

* more cleanup and relocate back transient storage and log type

* update package lock

* remove statemanager as dependency from client

* add the package dependency back without adding the build dependency

* remove unused deps

* avoid deep linking to docs, as typedoc changes permalinks frequently

* simplify tsconfig include

* explicitly import `inherits` to fix karma-typescript issue

* no need to specify type when default is used (can be inferred)

* add statemanager to root readme

* add statemanager deps in mermaid graph

* fix missing script

* update the short utility fn and add some tests

Co-authored-by: Ryan Ghods <ryan@ryanio.com>
 * fix legacyTransaction rebase

 * state manager build fx

 * rebase fixes

 * pick the packages/blockchain/src/index.js from develop

 * some more bn related fixes

 * update the fix corresponding to #1931

 * client fixes

 * Remove bn ops

 * fix vm test util

 * fix memory write extend

 * fix lint

 * update blockchain runner from develop

 * Add v check back for legacy Txs

 * Fix miner tests

 * resolve skeleton.getBlock errors because of mismatching config w.r.t. the one with which blocks were created

 * proper undefined comparision for bigint

 * provide genesis hash to skelteons block 1

 * switch consensus algo on hardfork consensus change

 * base fee proper fix

 * fix txpool specs

 * fix eth protocol spec

 * pending block spec fix

 * fix miner/miner spec

 * fix send raw transaction spec

 * reset the miner spec changes
@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #1943 (97554f2) into master (3e4e7be) will increase coverage by 0.20%.
The diff coverage is 82.25%.

Impacted file tree graph

Flag Coverage Δ
block 86.15% <89.92%> (+0.57%) ⬆️
blockchain 81.72% <78.85%> (-1.88%) ⬇️
client 78.40% <70.83%> (+1.45%) ⬆️
common 95.01% <95.00%> (+0.82%) ⬆️
devp2p 82.64% <77.21%> (-0.05%) ⬇️
ethash 90.81% <100.00%> (+0.04%) ⬆️
rlp 90.55% <ø> (ø)
statemanager 84.55% <73.52%> (?)
trie 81.30% <ø> (+1.27%) ⬆️
tx 92.18% <ø> (+3.90%) ⬆️
util 87.32% <ø> (-5.32%) ⬇️
vm 80.45% <ø> (-0.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@holgerd77
Copy link
Member Author

holgerd77 commented Jun 8, 2022

Haha, all failing. 😂

No, honestly, not a big thing I guess. I had this behavior locally already. See e.g. from the test-block test run:

Run npm i
npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR! 
npm ERR! While resolving: eslint-config-typestrict@1.0.5
npm ERR! Found: @typescript-eslint/eslint-plugin@4.33.0
npm ERR! node_modules/@typescript-eslint/eslint-plugin
npm ERR!   dev @typescript-eslint/eslint-plugin@"^4.27.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer @typescript-eslint/eslint-plugin@"^5" from eslint-config-typestrict@1.0.5
npm ERR! node_modules/eslint-config-typestrict
npm ERR!   dev eslint-config-typestrict@"^1.0.0" from the root project
npm ERR! 
npm ERR! Conflicting peer dependency: @typescript-eslint/eslint-plugin@5.27.1
npm ERR! node_modules/@typescript-eslint/eslint-plugin
npm ERR!   peer @typescript-eslint/eslint-plugin@"^5" from eslint-config-typestrict@1.0.5
npm ERR!   node_modules/eslint-config-typestrict
npm ERR!     dev eslint-config-typestrict@"^1.0.0" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 

So some dependency thing/conflict.

…d version conflicts due to eslint-plugin update to v5, rebuild package-lock.json
@holgerd77
Copy link
Member Author

Ok, this was an easy fix 🙂, so eslint-plugin-typestrict (from Kris, some former EthereumJS contributor 😋) jumped from eslint-plugin v4 to v5 as a peer dependency on the 1.0.3 -> 1.04 version bump, and this caused the resolution problems on our side.

We definitely not want to align and update ESLint in this stage of the process, so I just fixed our eslint-plugin-typescript version to 1.0.3 and this solved the problem.

So this is now open for review. 🙂

I will also deactivate the e2e-hardhat testrun for now for master for the first weeks or so of the transition, this will likely take some time to get fixed (on the Hardhat side).

@holgerd77
Copy link
Member Author

Ah, and another question: there is now a bigint-crypto-utils dependency added as a main dependency to the root package.json file relatively recently.

Is this intended or is this some kind of forgotten left-over from the BigInt transition days? 🤔

No blocker for this PR though in any way, this can in doubt be fixed in a follow-up PR!

@acolytec3
Copy link
Contributor

Ah, and another question: there is now a bigint-crypto-utils dependency added as a main dependency to the root package.json file relatively recently.

Is this intended or is this some kind of forgotten left-over from the BigInt transition days? thinking

No blocker for this PR though in any way, this can in doubt be fixed in a follow-up PR!

I believe @ryanio added it for ethash when we migrated to bigints. It's used here for the 'Miller-Rabin' stuff.

@holgerd77
Copy link
Member Author

I believe @ryanio added it for ethash when we migrated to bigints. It's used here for the 'Miller-Rabin' stuff.

And was it an intentional decision to add to the root package.json file? 🤔 ("Then we have it in doubt for the other libraries")?? Or did it just sneak in (for now I would assume the latter)?

Anyhow: again, up for a follow up PR in doubt.

(@acolytec3 if this looks fine for you can you give an approval?)

@acolytec3
Copy link
Contributor

I believe @ryanio added it for ethash when we migrated to bigints. It's used here for the 'Miller-Rabin' stuff.

And was it an intentional decision to add to the root package.json file? thinking ("Then we have it in doubt for the other libraries")?? Or did it just sneak in (for now I would assume the latter)?

Anyhow: again, up for a follow up PR in doubt.

(@acolytec3 if this looks fine for you can you give an approval?)

I think you can safely remove it. It's also in the ethash package.json so must have just sneaked in some errant PR over time.

@holgerd77
Copy link
Member Author

I think you can safely remove it. It's also in the ethash package.json so must have just sneaked in some errant PR over time.

Ok, will do in one of the follow-up PRs.

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM! Two things for a follow-up PR

  1. Remove bigint-crypto-util from root package.json
  2. Fix common memory leak in eip-3074 test script

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

🚀

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.