Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Elin/merge main v0.13.1 into main #1534

Merged
merged 7 commits into from
Feb 18, 2024
Merged

Conversation

elintul
Copy link
Collaborator

@elintul elintul commented Feb 18, 2024

This change is Reviewable

Signed-off-by: Dori Medini <dori@starkware.co>
* chore: move validate consts into os constants

Had to bump serde_json due to some apparent bug with the recent stable
rust.

* feat: make 13_1 consts backwards compatible

Assign the following defaults for keys missing in versioned_constants
files (for example, they are missing in versioned_constants_13_0.json).

- EventSizeLimit => max values
- L2ResourceGasCosts => 0 values
- validate rounding numbers => 1 (to preserve past no-rounding
  behavior). This required bundling them up in a specialized struct in
  order to define a custom default as 1 (rather than 0).
- Add test for default values: required extracting validation logic of
  `OsResources` so it won't trigger automatically in tests.

Co-Authored-By: Gilad Chase <gilad@starkware.com>
If os resources' inner tx is missing `constant` and `calldata_factor`
keys, then:
- assume it is a flat `ExecutionResources` instance, and put its
  contents inside a `constant` key.
- initialize calldata_factor as default (all zeros).
…ata-factor

feat: backwards compatibility for calldata_factor
Changes between this and the current versioned_constants:
- no `event_size_limit`
- invoke_tx_max_n_steps is 3_000_000 instead of 4_000_000
- no `l2_resource_gas_costs`
- multiple value changes in os_resources
- `vm_resource_fee_cost` is X2 for all values
- no constants for validate's block number and timestamp rounding

No other changes (in particular, os constants is indeed _unchanged_)

Co-Authored-By: Gilad Chase <gilad@starkware.com>
Copy link
Collaborator

@giladchase giladchase left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, 3 of 6 files at r2, 1 of 1 files at r4, 1 of 2 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @elintul)

@codecov-commenter
Copy link

Codecov Report

Attention: 58 lines in your changes are missing coverage. Please review.

Comparison is base (d4e3b93) 69.88% compared to head (85bef82) 69.50%.

Files Patch % Lines
crates/blockifier/src/versioned_constants.rs 32.55% 50 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1534      +/-   ##
==========================================
- Coverage   69.88%   69.50%   -0.39%     
==========================================
  Files          59       59              
  Lines        8180     8220      +40     
  Branches     8180     8220      +40     
==========================================
- Hits         5717     5713       -4     
- Misses       2005     2044      +39     
- Partials      458      463       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@elintul elintul merged commit 8838261 into main Feb 18, 2024
15 checks passed
gswirski pushed a commit to reilabs/blockifier that referenced this pull request Jun 26, 2024
…-libs#1534)

Co-authored-by: Jonatan Chaverri <jonatan.chaverri@kyndryl.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants