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

feat!: V27 update #3580

Draft
wants to merge 62 commits into
base: main
Choose a base branch
from
Draft

feat!: V27 update #3580

wants to merge 62 commits into from

Conversation

0xVolosnikov
Copy link
Contributor

What ❔

Why ❔

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 zkstack dev fmt and zkstack dev lint.

0xVolosnikov and others added 5 commits February 21, 2025 11:49
## What ❔

Updates VM versions throughout the codebase.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Code has been formatted via `zkstack dev fmt` and `zkstack dev
lint`.
Copy link
Contributor

@perekopskiy perekopskiy left a comment

Choose a reason for hiding this comment

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

There are 2 places in multivm with use_evm_emulator=base_system_smart_contracts.evm_emulator.is_some() and I want to double-check they are fine:

  • EvmDeployTracer is instantiated based on this value, I think it's fine even if emulation is not actually enabled tracer will just not find anything
  • in TransactionData:
let should_deploy_contract = if execute_tx.execute.contract_address.is_none() {
                    // Transactions with no `contract_address` should be filtered out by the API server,
                    // so this is more of a sanity check.
                    assert!(
                        use_evm_emulator,
                        "`execute.contract_address` not set for transaction {:?} with EVM emulation disabled",
                        common_data.hash()
                    );
                    U256([1, 0, 0, 0])
                } else {
                    U256::zero()
                };

So I understand better, if emulation is actually disabled will VM be able to gracefully process such transactions and if so what will happen: validation fail, VM be halted or tx revert?

Copy link
Contributor

Choose a reason for hiding this comment

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

Double-checking: are those bytecodes up-to-date i.e. compiled from era-contracts commit used in the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated bytecodes, just in case. Used era-contracts commit isn't fully finalized yet

Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep this comment open until contracts are finalized

Copy link
Contributor

Detected VM performance changes

Benchmark name Est. cycles Change in est. cycles
access_memory/legacy 13744469693 +8.9%
call_far/fast 3836328607 +445.0%
call_far/fast_no_sigs 3833223461 +447.0%
call_far/legacy 2547144796 +4.2%
decode_shl_sub/legacy 12885211936 +9.6%
deploy_simple_contract/fast 49042528 +2.6%
deploy_simple_contract/fast_no_sigs 45950773 +2.7%
deploy_simple_contract/legacy 56692088 +9.5%
event_spam/legacy 7672473369 +8.5%
finish_eventful_frames/legacy 3845951473 +4.7%
heap_read_write/legacy 11580794955 +9.6%
init/fast 14241106 +74.3%
init/fast_no_sigs 14275681 +74.0%
init/legacy 1923856 +41.4%
slot_hash_collision/legacy 13337378016 +9.3%
write_and_decode/legacy 13519369446 +9.0%

⚠ Detected differing instruction counts

Benchmark Old count New count
access_memory 4748374 4747741
call_far 319525 319501
decode_shl_sub 4748479 4747846
deploy_simple_contract 15422 15436
event_spam 2558512 2558180
finish_eventful_frames 786039 785949
heap_read_write 4252844 4252268
slot_hash_collision 4749388 4748744
write_and_decode 4749388 4748744

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

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.

5 participants