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

fix get_sorted_events issue #1024

Merged
merged 7 commits into from
Sep 13, 2023
Merged

fix get_sorted_events issue #1024

merged 7 commits into from
Sep 13, 2023

Conversation

juanbono
Copy link
Collaborator

@juanbono juanbono commented Sep 12, 2023

Fix get_sorted_events issue

Description

This PR fixes a bug where some events were not being saved.
Fixes #1022

Checklist

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

@juanbono juanbono changed the title add failing test that reproduce the issue fix get_sorted_events issue Sep 12, 2023
@juanbono juanbono modified the milestone: 0.3.1 Sep 12, 2023
@juanbono juanbono marked this pull request as ready for review September 12, 2023 22:57
src/execution/mod.rs Outdated Show resolved Hide resolved
src/execution/mod.rs Outdated Show resolved Hide resolved
src/execution/mod.rs Outdated Show resolved Hide resolved
src/execution/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@edg-l edg-l left a comment

Choose a reason for hiding this comment

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

minor comment

src/execution/mod.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2023

Codecov Report

Merging #1024 (2164434) into main (1eb0411) will increase coverage by 0.00%.
The diff coverage is 97.14%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1024   +/-   ##
=======================================
  Coverage   90.49%   90.49%           
=======================================
  Files          54       54           
  Lines       13992    14005   +13     
=======================================
+ Hits        12662    12674   +12     
- Misses       1330     1331    +1     
Files Changed Coverage Δ
src/execution/mod.rs 92.83% <97.14%> (-0.01%) ⬇️

@juanbono juanbono added this pull request to the merge queue Sep 13, 2023
Merged via the queue into main with commit 3ae2fa6 Sep 13, 2023
13 checks passed
fguthmann pushed a commit that referenced this pull request Sep 22, 2023
* 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
fguthmann pushed a commit that referenced this pull request Oct 2, 2023
* 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
@juanbono juanbono deleted the get_sorted_events_bug branch October 24, 2023 18:50
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>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Fix issue "Unexpected holes" when getting sorted events of a TransactionExecutionInfo
5 participants