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

Add comments to syscalls/deprecated_business_logic_syscall_handler module #884

Merged

Conversation

fguthmann
Copy link
Contributor

TITLE

Description

Added comments to syscalls/deprecated_business_logic_syscall_handler.rs

Checklist

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

@fguthmann fguthmann linked an issue Aug 3, 2023 that may be closed by this pull request
@fguthmann fguthmann marked this pull request as ready for review August 3, 2023 12:25
Copy link
Contributor

@matias-gonz matias-gonz left a comment

Choose a reason for hiding this comment

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

Nice work!
Could you please add a bit more documentation for each syscall?
You can find the descriptions here:
https://docs.starknet.io/documentation/architecture_and_concepts/Contracts/system-calls/

And you can add the link as 'further documentation' too

src/syscalls/deprecated_business_logic_syscall_handler.rs Outdated Show resolved Hide resolved
src/syscalls/deprecated_business_logic_syscall_handler.rs Outdated Show resolved Hide resolved
src/syscalls/deprecated_business_logic_syscall_handler.rs Outdated Show resolved Hide resolved
src/syscalls/deprecated_business_logic_syscall_handler.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@matias-gonz matias-gonz left a comment

Choose a reason for hiding this comment

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

The comments for each syscall at new_for_testing are very good, could you use those same comments for documenting the syscall functions?
For example at line 146:
Returns the address of the calling contract, or 0 if the call was not initiated by another contract.
But at line 500:
Get the caller's address from a virtual machine, using the syscall pointer.

I think the description at new_for_testing are more clear

@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2023

Codecov Report

Merging #884 (f15fee7) into main (4422734) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #884   +/-   ##
=======================================
  Coverage   89.54%   89.54%           
=======================================
  Files          50       50           
  Lines       14351    14360    +9     
=======================================
+ Hits        12850    12859    +9     
  Misses       1501     1501           
Files Coverage Δ
...calls/deprecated_business_logic_syscall_handler.rs 92.14% <100.00%> (+0.07%) ⬆️

juanbono and others added 13 commits October 2, 2023 16:58
* 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>
…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.

* 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.md

* add link to tg group
* 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 increment_syscall_counter

* Add test + fix test

* Fix test

* Fix tests

* fmt
…)` (#960)

* Add utils fns

* Implemented fix

* Fix some tests

* Fix clippy

* Fix tests

* Fix test

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>
…nvokeFunction (#999)

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

* document

* fix
…ne, 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>
fmoletta and others added 25 commits October 2, 2023 17:00
* Push clean changes

* fmt

* Fix test

* Fix test

* Fix test

* Fix test

* Fix tests

* Fix tests

* Fix tests
Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: Mario Rugiero <mrugiero@gmail.com>
Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com>
* 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>
* 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
#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>
* 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
* update version

* fix get_compiled_hash

* cargo clippy
Co-authored-by: Juan Bono <juanbono94@gmail.com>
* remove unneeded added set_compiled_class_hash

* fix missing events when using deprecated business syscall handler

---------

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

* fix wrong from_address in deprecated execute_constructor_entry_point

* fix test

* fix another test
Co-authored-by: Juan Bono <juanbono94@gmail.com>
* Remove `serde_json_pythonic`.

* Fix JSON formatter on `deprecated_contract_class.rs`.

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

* Add unwrap reasoning comment.
* 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>
* update version

* fix skip validation for invoke txs

* run fmt

* fix clippy suggestion

* simplify a bit the execute_tx function variants
@Oppen Oppen added this pull request to the merge queue Nov 24, 2023
Merged via the queue into main with commit 971776b Nov 24, 2023
7 checks passed
@Oppen Oppen deleted the document-syscalls/deprecated_business_logic_syscall_handler-module branch November 24, 2023 22:15
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document syscalls/deprecated_business_logic_syscall_handler module