block: low-level validation method refactor #1925
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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
* Common: Return BigInt for param() functionality Fixes #1846 * Change param getters to throw on non-existent value * Fixes * switch return to bigint or undefined * Switch param getters to return undefined and fix common tests * tx: changes to accomodate param getters * More changes * More adjustments * Fix NaN bug in opcode fee builder * Hack to fix EIP-2537 edge case * client: fix test syntax * lint * Return 0 when param doesn't exist * Spelling, dedup param usage * Update `paramByEip` to allow undefined * Hardcode EIP-2537 gas discount array * uncomment test
* block: update to rlp v3 * blockchain: update to rlp v3 * client: update to rlp v3 * devp2p: update to rlp v3 * ethash: fix test to pass buffer * trie: update to rlp v3 * tx: update to rlp v3 * util: update to rlp v3 * vm: update to rlp v3 * lint, etc. fixes and update package-lock * address review comments
* blockchain: remove 'void' from iterator() return * vm: remove void from vm.runBlockchain() return types * client: adapt VMExecution to stricter blockchain.iterator() call * vm: reintroduce deleted tests for blockchain.getIteratorHead() * vm: correct method name in test
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM * VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter * VM -> EVM/VM Refactor: standalone EVM stateManager option and property * VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM * VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM * VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM * VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency * Client: fix cliqueActiveSigners method assignments * VM: refactor TxContext to an interface * VM: improve Message class types and defaults handling * VM: refactor runCall and executeMessage into unified runCall * VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary * VM: reimplement transient storage clear method * VM: use VM async create method instead of constructor * VM: unify interpreter and evm DEBUG property Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
* Add typing to hardforkforForkhash * Type initializedChains * Union type for consensus algorithm * Add check for invalid chain ID * Add cliqueOpts type * block: cast to cliqueOpts * client: cast to cliqueOpts in miner * Fix tests * Add back chainsType import * Add back missing types * Address comments * lint * Address feedback
* common: create new common.ts * common: finish reorganize/rename in common * Address comments
* tx: edit legacyTransaction to throw when hash() called on unsigned tx * tx: adapt legacy.spec test to throw on unsigned tx.hash() * tx: edit tests which call hash() on unsigned tx
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@holgerd77 @ryanio This task seemed fairly straightforward at the start, but I started to struggle with it as things started moving around as part of the refactor. I had branched off of Also I know I had some failing tests in |
acolytec3
reviewed
Jun 1, 2022
| @@ -232,9 +184,16 @@ tape('[Block]: block functions', function (t) { | |||
| }) | |||
|
|
|||
| t.test('should test transaction validation with legacy tx in london', async function (st) { | |||
| // TODO: fix this test | |||
| // failing to initialize because mixHash is not filled with zeroes. is this a problem with | |||
| // the testdata, or a problem with the code? | |||
There was a problem hiding this comment.
I think this is a problem with the data. The block used in this test isn't a valid block under clique consensus rules given the mixHash has a non-zero value.
|
@emersonmacro hi there, thanks for the latest efforts 🙂 🙏, I think it is very unlucky to have all this work in just a single commit. With this it's gets extremely difficult to apply the changes in a controlled fashion. So it would have been a lot healthier to apply - let's say - 15-20 commits one-by-one by cherry pick on top of the latest Hmm, not sure how to proceed: if you still have an up-to-date version of your work in a more-commits-separated version it might be a way to reapply. Otherwise we might still be able to see in the diffs from the current one-commit-submission where eventual flaws/overrides could lie. (just as some very high-level first comment on this first round) |
|
(so haven't looked too much into the code yet) |
|
@holgerd77 Sorry, I screwed this up. I guess I was thinking about it wrong, that one big commit would make it easier to cherry-pick, but didn't factor in conflicting changes or moving large blocks of code between packages. I think I'm going to start over and reapply the changes in smaller chunks based on what's currently in develop, with a close eye on making sure Ryan's recent changes are retained. |
No problem, that's just part of the learning process to occasionally screw things up temporarily. 🙂 Nothing which can't be fixed. Yes, these one-commits-with-large-chainset really only work if one already has a consistent and working change set (e.g. when we squash together commits after approval), otherwise things can be a lot better controlled (and also: reviewed) if commits are modular and small. I think the best way is really to start over and reapply, then it will also be best to really push very often, so after 2-3 atomic change commits e.g., and make sure on every run that the CI keeps passing in between. Then it should really be possible to get things under control. Please wait with this until we (I) have pushed the latest rebased Thanks for your patience and endurance! |
|
This branch is fairly out of sync with current |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1879 and #1889