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

VM: Jump dest analysis enhancements #1629

Merged
merged 15 commits into from
Jan 13, 2022
Merged

Conversation

acolytec3
Copy link
Contributor

@acolytec3 acolytec3 commented Jan 11, 2022

  • Switch valid Jumps storage from multiple number[] to a single Uint8Array of predefined length
  • Only do jump destination analysis if JUMP/JUMPI/JUMPSUB opcode encountered

Should finish off #1575

@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #1629 (b4ad685) into master (81c2bf0) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

Flag Coverage Δ
block 85.01% <ø> (ø)
blockchain 83.28% <ø> (ø)
client 71.27% <ø> (ø)
common 93.89% <ø> (ø)
devp2p 82.70% <ø> (-0.27%) ⬇️
ethash 90.76% <ø> (ø)
rlp ?
trie 86.18% <ø> (ø)
tx 89.94% <ø> (ø)
util 92.69% <ø> (ø)
vm 81.27% <100.00%> (+<0.01%) ⬆️

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

@acolytec3 acolytec3 added the type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR. label Jan 11, 2022
@ryanio
Copy link
Contributor

ryanio commented Jan 12, 2022

just had one comment but otherwise this lgtm! thanks for taking it on so quickly

let's wait for one more approval from @jochem-brouwer or @holgerd77 since this is critical code (I think we should expect for vm internals)

@ryanio ryanio changed the title Jump dest analysis enhancements VM: Jump dest analysis enhancements Jan 12, 2022
@ryanio ryanio linked an issue Jan 12, 2022 that may be closed by this pull request
@acolytec3
Copy link
Contributor Author

just had one comment but otherwise this lgtm! thanks for taking it on so quickly

What's the comment? :-)

packages/vm/src/evm/interpreter.ts Outdated Show resolved Hide resolved
jochem-brouwer
jochem-brouwer previously approved these changes Jan 13, 2022
Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

LGTM!

@ryanio ryanio merged commit 93c4c4a into master Jan 13, 2022
@ryanio ryanio deleted the jump-dest-analysis-enhancements branch January 13, 2022 21:34
@jochem-brouwer
Copy link
Member

🎉

@holgerd77
Copy link
Member

Cool!! 😄 👍

ryanio added a commit that referenced this pull request Jan 25, 2022
* VM: Adopted Backwards-compatible Dynamic Gas Cost Refactoring (#1553)

* VM: report dynamic gas values in `fee` field of `step` event (#1364)
* vm: codes: add dynamicGas property
vm: codes: make codes more typesafe
* vm: create no-op dynamic gas handlers
* vm: first batch of dynamic gas up to 0x3f
* vm: add other opcodes to gas map
vm: change step event fee type from number to BN
vm: deduct dynamic gas
vm: fix stack peek
vm: do not add gasLimit to gas cost for CREATE
* vm: move errors to gas map
* vm: fix memory dynamic  gas bug
* vm: fix gas bugs caused by not considering base fee
* vm: fix message call gas related bugs, clone current gas left
* add typedoc for peek
use underscore for unused peek params (and fix eslint config)
format some comment newlines for readability
switch from require to import for exceptions
* simplify the 2929 state manager castings in runTx
* add changelog entry
* vm: add EIP1283 tests
* vm: split non-eip2929 and eip2929 gas costs
* vm: fix gas costs
* vm: add early-hardfork coverage
* vm: clarify pre-Constantinople SSTORE gas
vm: clarify EIP-150 comment
* run coverage for all state and blockchain tests, remove redundant istanbul run
* vm: fix CALLCODE gas
vm: explicitly clone gasLimit for maxCallGas
* vm: remove TODO in interpreter
* update defaultCost to BN, cast 2929 statemanager to simplify use syntax
* use underscore for unused variables, simplify types since they can be inferred
* vm: fix browser tests + fix rebase
* VM: moved dynamic fee to dedicated dynamicFee field in step event and retained fee behavior and type for backwards compatibility
* VM: aligned InterpreterStep and step event object property documentation, completed missing step event properties
* VM: test fix
* vm: fix hardhat e2e tests
* vm: fix MSTORE opcodes
* vm: added dynamicGas property to push0 (EIP-3855) opcode
* hardhat e2e: add temporary workaround for skipping tests with inconsistent memory field
* nit style: use underscore instead of comment out unused variable
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
Co-authored-by: Ryan Ghods <ryan@ryanio.com>

* ci: bump karma-typescript version (#1631)

* bump karma-typescript to new release

* bump libp2p-bootstrap

* update package-lock

* VM: Jump dest analysis enhancements (#1629)

* Change valid jumps to single array
* Switch array to Uint8Array
* use correct opt code
* Check for jumpsub
* Address feedback
* Address fixes
* More efficiency
* Adjust if/else logic for efficiency
* Move comments
Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
Co-authored-by: Ryan Ghods <ryan@ryanio.com>

* rlp: add to root readme (#1642)

* add rlp to root readme
* add carryforward for rlp flag since it is not always tested on every build unless its paths are modified

* Client: Add tests to verify shutdown (#1641)

* Add tests to verify client shutdown

* Add libp2p test

* Address feedback

* most libp2p into separate file

* VM: Add comparison testing between branchs for State Test Runner (#1634)

* Add diff tester and performance

* update script

* Simplify script

* Readme updates

* move start

* Update stashing logic in script

* tx: input value testing (#1620)

* tx: helper methods to generate tx values

* tx: helper methods to generate tx values

* chore: scaffold test and add extra to values

* chore: add missing spread operator

* chore: single value for each tx key

* tx: add generateCombinations method and legacy + eip 1559 test cases

* chore: remove console log

* chore: use london hardfork for eip1559

* tx: refactor generateCombinations interface and properly compare hash

* tx: deterministically randomly sample 1000 elements from array for testing

* chore: remove v

* chore: remove todo

* chore: clearer variable declarations and types

* tx: fix and simplify random sampling

* tx: display tx hash in testing output

* tx: convert buffer to hex for output log

* ci: fix node-versions run for node <16 (#1653)

* re-add updating to npm v7 for node versions <16
* only upgrade npm for node v <16
* fix bin/rlp js: node 12 doesn't support ES11 which added support for nullish coalescing operator (??) so we'll use ternary here
alternatively we could write this file in TS and compile to e.g. dist/bin/rlp (like we do in the client bin), but maybe if the file gets more complicated, in its current state i don't think it's so neccessary
* use same errorMsg format for JSON.parse, remove unneeded extra Uint8Array.from (already is uint8array)

* rlp: v3 updates from integration (#1648)

* rlp updates
* util: add arrToBufArr and bufArrToArr and tests
* util updates, add deprecation notice for re-exports
* rlp tsconfig: include rootDir for composite option
* remove util exports deprecation notices
* rlp: add readme note for buffer compatibility
* undo capitalization

* Devp2p, Client: Fix duplicated debug messages (#1643)

* devp2p: update debug package

* blockchain: update debug package

* client: update debug package

* vm: update debug package

* devp2p/dpt fix for #1485

* devp2p/eth: Fix for 485

* devp2p: Fix for #1485:  add base debug

* devp2p/dpt  #1485:  change to debug.extend()

* devp2p:dpt:server change to degub.extend()

* devp2p:eth change to debug.extend()

* devp2p/les:  change to debug.extend()

* devp2p:rlpx:  change to debug.extend()

* rlps: change to debug.extend()

* Update debug to use IP as first tag

* vm, client: removed ProofStateManager interface, added optional getProof, verifyProof methods to StateManager interface (#1660)

* Monorepo (& Tx, VM): Examples scripts in CI (#1658)

* chore(examples): examples added to ci

* chore(examples-ci): remove script from VM (for now) & rename examples workflow file

* chore(ci): new script formwatted with prettier & example workflow changes to run with non-test branches

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

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
Co-authored-by: Gabriel Rocheleau <contact@rockwaterweb.com>
Co-authored-by: ScottyPoi <66335769+ScottyPoi@users.noreply.github.com>
Co-authored-by: Cesar Brazon <cesarbrazon10@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: vm PR state: needs review type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VM: Optimize (Replace) Jump Analysis Algorithm
4 participants