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

fix: Read before writing when executing the deprecated version of storate_write syscall #1006

Merged
merged 11 commits into from
Sep 5, 2023

Conversation

fmoletta
Copy link
Contributor

@fmoletta fmoletta commented Sep 4, 2023

The storage_write syscall was not reading the storage before writing into it. This read updates the cache's storage_initial_values with values from the state reader, forgoing this step can lead to incorrect results when counting the storage changes

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.

@fmoletta fmoletta added the bug Something isn't working label Sep 4, 2023
@fmoletta fmoletta marked this pull request as ready for review September 4, 2023 21:45
Copy link
Collaborator

@juanbono juanbono left a comment

Choose a reason for hiding this comment

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

LGTM

@juanbono juanbono added this pull request to the merge queue Sep 5, 2023
Merged via the queue into main with commit 3d55d62 Sep 5, 2023
13 checks passed
fguthmann pushed a commit that referenced this pull request Sep 22, 2023
…orate_write` syscall (#1006)

* Read before writing

* Add test

* Fix some tests

* Fix test

* Fix test

* Fix test

* Fix test

* Fix tests

* Fix tests

* Fix tests
fguthmann pushed a commit that referenced this pull request Oct 2, 2023
…orate_write` syscall (#1006)

* Read before writing

* Add test

* Fix some tests

* Fix test

* Fix test

* Fix test

* Fix test

* Fix tests

* Fix tests

* Fix tests
@fmoletta fmoletta deleted the fix-deprec-storage-write branch October 24, 2023 17:46
github-merge-queue bot pushed a commit that referenced this pull request Nov 24, 2023
…dule (#884)

* added comments to syscalls/deprecated_business_logic_syscall_handler.rs

* corrected comments

* added informations on system calls

* change comments

* Added Starknet API / Blockifier RPC State Reader (#927)

* initial commit

* add get_class_hash_at

* Added get_nonce_at

* Added get_storage_at

* Remove comments

* Added get block info

* Fixed get_contract_class()

* WIP fixing desearlization

* WIP Fix

* Finished fixing get_contract_class()

* Uncommented tests, new get_contract_class

* Remove file

* WIP Fixing tests

* Finish fixing simple tests

* Fixed transaction trace and block info

* Fix import

* Refactor, fixes, added test

* Fixed warnings, removed tests

* Fixed get_transaction_receipt

* Fixed actual_fee from get_transaction_receipt

* Format Cargo.toml

* Redid BlockValue

* Removed middle response types

* Changed unreachable with unimplemented

* Move import inside fn

* Fix tests

---------

Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>

* Deserialize transactions too on the block info for the Rpc Reader SN (#961)

* initial commit

* add get_class_hash_at

* Added get_nonce_at

* Added get_storage_at

* Remove comments

* Added get block info

* Fixed get_contract_class()

* WIP fixing desearlization

* WIP Fix

* Finished fixing get_contract_class()

* Uncommented tests, new get_contract_class

* Remove file

* WIP Fixing tests

* Finish fixing simple tests

* Fixed transaction trace and block info

* Fix import

* Refactor, fixes, added test

* Fixed warnings, removed tests

* deserialize all transactions in the block info too

* docs

* refactor into standalone fn

* get gas from block via RPC SN (#963)

* get gas automatically from block

* cleanup

* fix wrong gas

* unintended change

---------

Co-authored-by: juanbono <juanbono94@gmail.com>
Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>

* Unify deprecated and casm contract caches. (#937)

* Unify deprecated and casm contract caches.

* Fix formatting and clippy.

* Remove unused code.

* Unify contract classes in the state traits too.

* Restore type alias.

---------

Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com>

* Update README with Telegram group link and badge (#843)

* Update README.md

* add link to tg group

* From/TryFrom starknet api types (#962)

* From/TryFrom starknet api types

* Add deploy account

* Modify gitignore

* Deploy account and invoke function

* Change into_iter to iter

* Update .gitignore

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>

* change to try_from

* Move functions to its respective files

* Test

* Delete test

* Fix format

* Fix test

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>

* Fix gas/fee price type consistency (to `u128`). (#987)

* Fix `ExecutionResources::increment_syscall_counter` (#971)

* Fix increment_syscall_counter

* Add test + fix test

* Fix test

* Fix tests

* fmt

* minor code cleanup (#968)

* Added fee transfer storage update into `count_actual_storage_changes()` (#960)

* Add utils fns

* Implemented fix

* Fix some tests

* Fix clippy

* Fix tests

* Fix test

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>

* Remove missing unwrap from codebase. (#1000)

* refactor/ fix TryFrom InvokeTransaction into a standalone method on InvokeFunction (#999)

* refactor and fix TryFrom InvokeTransaction into a standalone method on InvokeFunction

* document

* fix

* Deprecate old RPC StateReader in favor of `starknet_api` compatible one, support SiR execution (#967)

* From/TryFrom starknet api types

* Add deploy account

* Modify gitignore

* Deploy account and invoke function

* Change into_iter to iter

* Update .gitignore

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>

* change to try_from

* Move functions to its respective files

* WIP Added SIR support to SNRPC

* Implemented state reader for sir

* WIP Transaction

* WIP SiR execution

* Fixed rpc sir execute_tx

* Fix clippy

* Import last version of sn_api

* Formatting

* Test

* Test try from

* Delete test

* Fix format

* Fix test

* Fix clippy

* Replaced try_from with from_invoke_transaction

* infer version

* Fix version

* Changed test_try_from_invoke

* Ignore test_recent_tx

* Refactor tx deser, (un)ignore tests

* Added support for reverted

---------

Co-authored-by: Milton <milton.scuderi@lambdaclass.com>
Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Co-authored-by: mmsc2 <88055861+mmsc2@users.noreply.github.com>
Co-authored-by: Edgar Luque <git@edgarluque.com>

* perf: remove clone from compute_sierra_class_hash (#1008)

* fix: Read before writing when executing the deprecated version of `storate_write` syscall (#1006)

* Read before writing

* Add test

* Fix some tests

* Fix test

* Fix test

* Fix test

* Fix test

* Fix tests

* Fix tests

* Fix tests

* BREAKING: `StateReader::get_storage_at` return zero by default (#1011)

* update InMemoryStateReader & CachedState

* Add comments

* Apply changes to State implementation too

* Fix behaviour

* Add test

* BREAKING: `StateReader::get_class_hash_at` return zero by default (#1012)

* Use unwrap_or_default

* return zero by default state reader get_class_hash_at

* Add changes

* clippy

* RPC use test_case with local cache and add more tests (#970)

* From/TryFrom starknet api types

* Add deploy account

* Modify gitignore

* Deploy account and invoke function

* Change into_iter to iter

* Update .gitignore

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>

* change to try_from

* Move functions to its respective files

* WIP Added SIR support to SNRPC

* Implemented state reader for sir

* WIP Transaction

* WIP SiR execution

* Fixed rpc sir execute_tx

* Fix clippy

* Import last version of sn_api

* Formatting

* Test

* Test try from

* Delete test

* Fix format

* Fix test

* local test cases

* specify block manually

* add test_case for blockifier

* use more recent txs

* dont keep cached responses

* add failing tx on blockifier

* more tests

* Fix clippy

* fix bug

* tests

* Replaced try_from with from_invoke_transaction

* infer version

* Fix version

* Changed test_try_from_invoke

* Ignore test_recent_tx

* ignore failing tests

* Refactor tx deser, (un)ignore tests

* sorted assert and fee threshold

* fixes

---------

Co-authored-by: Milton <milton.scuderi@lambdaclass.com>
Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Co-authored-by: mmsc2 <88055861+mmsc2@users.noreply.github.com>
Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>

* Update cache initial values with write-only accesses in `CachedState::count_actual_storage_changes` (#1009)

* Add update_initial_values_of_write_only_access

* Change name

* Update variable names

* Fix + expand test

* Add band-aid soluction + restore test

* update InMemoryStateReader & CachedState

* Add comments

* Apply changes to State implementation too

* Fix behaviour

* Add test

* Remove band-aid solution

* Use unwrap_or_default

* return zero by default state reader get_class_hash_at

* Add changes

* clippy

* Add initial values to test

* remove duplictaed test

* Refactor new RPC into several files (#1007)

* From/TryFrom starknet api types

* Add deploy account

* Modify gitignore

* Deploy account and invoke function

* Change into_iter to iter

* Update .gitignore

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>

* change to try_from

* Move functions to its respective files

* WIP Added SIR support to SNRPC

* Implemented state reader for sir

* WIP Transaction

* WIP SiR execution

* Fixed rpc sir execute_tx

* Fix clippy

* Import last version of sn_api

* Formatting

* Test

* Test try from

* Delete test

* Fix format

* Fix test

* local test cases

* specify block manually

* add test_case for blockifier

* use more recent txs

* dont keep cached responses

* add failing tx on blockifier

* more tests

* Fix clippy

* fix bug

* tests

* Replaced try_from with from_invoke_transaction

* infer version

* Fix version

* Changed test_try_from_invoke

* Ignore test_recent_tx

* ignore failing tests

* Refactor tx deser, (un)ignore tests

* sorted assert and fee threshold

* refactor rpc state reader

* fixes

---------

Co-authored-by: Milton <milton.scuderi@lambdaclass.com>
Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Co-authored-by: mmsc2 <88055861+mmsc2@users.noreply.github.com>
Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>

* Fix storage changes count for transactions with fee transfers (#1015)

* Push clean changes

* fmt

* Fix test

* Fix test

* Fix test

* Fix test

* Fix tests

* Fix tests

* Fix tests

* fix estimate fee missing casmclasscache (#916)

Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: Mario Rugiero <mrugiero@gmail.com>

* fix: declare v0 requires max_fee=0, consider for simulation (#1017)

* Remove redundant `tx_type` field from transactions. (#1019)

Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com>

* remove files and rename the new one (#1020)

* Add contract class cache stats (#958)

* added cache_hit and cahce_misses to count the number of error in our cache, abd a test for it

* wip

* Apply suggestions from code review

* fix: qnd borrow checker

* clippy

---------

Co-authored-by: fannyguthmann <fanny.guthmann@post.idc.ac.il>
Co-authored-by: Mario Rugiero <mrugiero@gmail.com>

* add tests and remove ignore on fixed ones (#1021)

* perf: refactor substract_mappings and friends to avoid clones (#1023)

* refactor substract_mappings and friends to avoid clones of the whole hashmap

* another opt

* dont use deref

* fix deref again

* no need for contains_key

* oops

* Fix transactions bypassing the max_fee by introducing new revert logic (#901)

* Make tx fail when actual_fee exceeds max_fee

* Changed test

* Formatting

* Fix logic

* Leave fail only without charging

* Change test

* Fix test broken by better fee calc

* Fixed test fee

* Update fee on test_deploy_account

* Remove comment

* Added fee transfer

* Test with invoke

* Added revert logic for invoke

* Modify tests, add fixes

* Add revert error

* Fix test_invoke_tx_account

* Fixed test_invoke_tx_exceeded_max_fee

* Fix test_get_nonce_at

* Rely on another contract

* Introduced transactional state (#917)

* Introduced transactional state

* WIP

* Fixed the rest of tests

* Replaced old revert logic from entrypoint exec

* depl acc revert test

* Remove update writes fix

* WIP Fixed many tests

* fix test

* fix more tests

* more fixes

* fix another test

* fix latest test

* name

* remove comment

* merge

* unignore

* format

* vis

* need to be pub for  tests

* fix test

* format

* use the count_actual_storage_changes impl from cached state

* fix bug

* fix tests

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: Edgar Luque <git@edgarluque.com>

* fix get_sorted_events issue (#1024)

* add failing test that reproduce the issue

* fix the bug

* fix test since now 2 events with the same order are ok

* handle multiple events

* fix comments

* cargo fmt

* pin version (#1026)

* update version (#1028)

* Fix coverage control mechanism. (#1035)

* Fix SIR RPC get compiled class hash (#1032)

* update version

* fix get_compiled_hash

* cargo clippy

* remove unneeded added set_compiled_class_hash (#1031)

Co-authored-by: Juan Bono <juanbono94@gmail.com>

* Fix missing events (#1034)

* remove unneeded added set_compiled_class_hash

* fix missing events when using deprecated business syscall handler

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>

* fix wrong from_address in deprecated execute_constructor_entry_point (#1041)

* fix wrong from_address in deprecated execute_constructor_entry_point

* fix test

* fix another test

* remove testing, move erc20 test, update fibonacci bin (#1038)

Co-authored-by: Juan Bono <juanbono94@gmail.com>

* Remove `serde_json_pythonic`. (#1047)

* Remove `serde_json_pythonic`.

* Fix JSON formatter on `deprecated_contract_class.rs`.

* Fix hash JSON formatter (non-ascii support).

* Add unwrap reasoning comment.

* Add debug logging. (#1018)

* Add `tracing` and update dependencies.

* Configure the example to use tracing logging (and make it work again).

* Add tracing logging.

* Add error logging.

* Fix error logging.

* Reduce the amount of spam logged.

* Update `README.md`.

* Fix `Makefile` dependencies.

* Remove `Debug` trait dependency.

* Update `Cargo.lock` after merge.

* Fix warnings.

* Fix formatting.

---------

Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com>

* Fix skip validate (#1053)

* update version

* fix skip validation for invoke txs

* run fmt

* fix clippy suggestion

* simplify a bit the execute_tx function variants

---------

Co-authored-by: fannyguthmann <fanny.guthmann@post.idc.ac.il>
Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: Estéfano Bargas <estefano.bargas@fing.edu.uy>
Co-authored-by: Edgar <git@edgarluque.com>
Co-authored-by: MrAzteca <azteca1998@users.noreply.github.com>
Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com>
Co-authored-by: mmsc2 <88055861+mmsc2@users.noreply.github.com>
Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Co-authored-by: Milton <milton.scuderi@lambdaclass.com>
Co-authored-by: Mario Rugiero <mrugiero@gmail.com>
Co-authored-by: marioiordanov <mio@shardlabs.io>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants