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

fix(vm): Fix used bytecodes divergence #2741

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

slowli
Copy link
Contributor

@slowli slowli commented Aug 27, 2024

What ❔

Fixes divergence in used bytecodes info between the old and new VMs.

Why ❔

The new VM behaved differently to the old VM on far call if decommitting the called contract leads to out-of-gas revert. The old VM records the called contract bytecode as decommitted in this case; the new one didn't.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@slowli slowli force-pushed the aov-pla-1023-fix-used-bytecodes-divergence branch from f7cfe5c to 0052728 Compare August 27, 2024 13:36
Copy link
Contributor

Detected VM performance changes

Benchmark name change in estimated runtime change in number of opcodes executed
decode_shl_sub -16.6% +0 (NaN%)
access_memory -11.4% +0 (NaN%)
call_far -10.9% +0 (NaN%)
finish_eventful_frames -38.7% +0 (NaN%)
write_and_decode -13.7% +0 (NaN%)
slot_hash_collision -14.4% +0 (NaN%)
deploy_simple_contract -40.7% +0 (NaN%)
event_spam -36.9% +0 (NaN%)

Changes in number of opcodes executed indicate that the gas price of the benchmark has changed, which causes it run out of gas at a different time. Or that it is behaving completely differently.

@slowli slowli marked this pull request as ready for review August 27, 2024 14:02
@slowli
Copy link
Contributor Author

slowli commented Aug 27, 2024

JFYI, reported changes in the runtime are caused by switching to segmented heap (not directly related to this fix, but it was merged in vm2 codebase recently and wasn't applied in this repo).

@joonazan joonazan added this pull request to the merge queue Aug 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 27, 2024
@joonazan joonazan added this pull request to the merge queue Aug 27, 2024
Merged via the queue into main with commit 923e33e Aug 27, 2024
57 checks passed
@joonazan joonazan deleted the aov-pla-1023-fix-used-bytecodes-divergence branch August 27, 2024 20:32
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2024
🤖 I have created a release *beep* *boop*
---


##
[16.5.0](prover-v16.4.0...prover-v16.5.0)
(2024-08-28)


### Features

* **prover_cli:** Add test for status, l1 and config commands.
([#2263](#2263))
([6a2e3b0](6a2e3b0))
* **prover_cli:** Stuck status
([#2441](#2441))
([232a817](232a817))
* **prover:** Add ProverJobMonitor
([#2666](#2666))
([e22cfb6](e22cfb6))
* **prover:** parallelized memory queues simulation in BWG
([#2652](#2652))
([b4ffcd2](b4ffcd2))
* Provide easy prover setup
([#2683](#2683))
([30edda4](30edda4))


### Bug Fixes

* **prover_cli:** Remove congif file check
([#2695](#2695))
([2f456f0](2f456f0))
* **prover_cli:** Update prover cli README
([#2700](#2700))
([5a9bbb3](5a9bbb3))
* **prover:** change bucket for RAM permutation witnesses
([#2672](#2672))
([8b4cbf4](8b4cbf4))
* **prover:** fail when fri prover job is not found
([#2711](#2711))
([8776875](8776875))
* **prover:** Revert use of spawn_blocking in LWG/NWG
([#2682](#2682))
([edfcc7d](edfcc7d))
* **prover:** speed up LWG and NWG
([#2661](#2661))
([6243399](6243399))
* **vm:** Fix used bytecodes divergence
([#2741](#2741))
([923e33e](923e33e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.23.0](core-v24.22.0...core-v24.23.0)
(2024-08-28)


### Features

* Refactor metrics/make API use binaries
([#2735](#2735))
([8ed086a](8ed086a))


### Bug Fixes

* **api:** Fix duplicate DB connection acquired in `eth_call`
([#2763](#2763))
([74b764c](74b764c))
* **vm:** Fix used bytecodes divergence
([#2741](#2741))
([923e33e](923e33e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.23.0](core-v24.22.0...core-v24.23.0)
(2024-08-28)


### Features

* Refactor metrics/make API use binaries
([#2735](#2735))
([8ed086a](8ed086a))


### Bug Fixes

* **api:** Fix duplicate DB connection acquired in `eth_call`
([#2763](#2763))
([74b764c](74b764c))
* **vm:** Fix used bytecodes divergence
([#2741](#2741))
([923e33e](923e33e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants