This repository was archived by the owner on Jul 22, 2024. It is now read-only.
Added Starknet API / Blockifier RPC State Reader #927
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xqft
changed the title
azteca1998
reviewed
Aug 29, 2023
| )) | ||
| .set("Content-Type", "application/json") | ||
| .set("accept", "application/json") | ||
| .send_json(params) |
There was a problem hiding this comment.
I'm not sure if it's intended here, but I remember changing the JSON serialization to raw format!() strings.
| pub internal_calls: Vec<RpcCallInfo>, | ||
| } | ||
|
|
||
| #[allow(unused)] |
There was a problem hiding this comment.
I don't think this #[allow(unused)] is necessary anymore since the struct is externally public.
There was a problem hiding this comment.
The fields aren't, but there's no reason to privatize them. Originally I made everything public so we could use these types in the other RPC StateReader if necessary but now we decided to deprecate that one. I think for this PR the visibility can be left unchanged but in a next PR they may change.
rpc_state_reader_sn_api/src/lib.rs
Outdated
Comment on lines
210
to
211
| let hex: &str = Deserialize::deserialize(deserializer)?; | ||
| Ok(u128::from_str_radix(&hex[2..], 16).unwrap()) |
There was a problem hiding this comment.
Is not checking the prefix intentional? (the 0x prefix)
There was a problem hiding this comment.
I didn't deem it necessary to add a check because every StarkFelt is deserialized assuming the prefix as by implementation in starknet_api (see PrefixedBytesAsHex), and in this case because the actual fee won't ever be greater than the field's prime, it's safe to truncate, which is equivalent to directly interpreting as a u128.
Comment on lines
+237
to
+249
| // Parse retdata | ||
| let retdata_value = value["result"].clone(); | ||
| let retdata = serde_json::from_value(retdata_value).unwrap(); | ||
|
|
||
| // Parse calldata | ||
| let calldata_value = value["calldata"].clone(); | ||
| let calldata = serde_json::from_value(calldata_value).unwrap(); | ||
|
|
||
| // Parse internal calls | ||
| let internal_calls_value = value["internal_calls"].clone(); | ||
| let mut internal_calls = vec![]; | ||
|
|
||
| for call in internal_calls_value.as_array().unwrap() { |
There was a problem hiding this comment.
Errors were handled correctly up to this point. Are those .unwrap() intended?
There was a problem hiding this comment.
For now yes, the motivation of this state reader is to use it as a tool for debugging fee-related problems, which are the priority now. There are lots of .unwrap() which can all be solved in a future PR
| } | ||
| } | ||
|
|
||
| mod blockifier_transaction_tests { |
There was a problem hiding this comment.
Usually, the test module is the last thing in a module so I assume that there's nothing below. Is this module's placement after the test module intentional?
There was a problem hiding this comment.
Seeing as it has its own test module, maybe this module should be its own file?
There was a problem hiding this comment.
Yep, we should do a big refactor because there are too many types crammed in a single file, but as I said the priority now is not to make it pretty (if we keep extending the funcionality we may deem it necessary). The last module will be fidgeted with a lot when debugging.
pefontana
reviewed
Aug 29, 2023
| fn rpc_call<T: for<'a> Deserialize<'a>>( | ||
| &self, | ||
| params: &serde_json::Value, | ||
| ) -> Result<T, RpcError> { |
There was a problem hiding this comment.
Just to know,
won't be better to return a Result<RpcResponse,..>?
There was a problem hiding this comment.
There are cases in which it's useful to deserialize into a serde_json::Value, e.g. get_transaction().
| )) | ||
| .query("transactionHash", &hash.0.to_string()) | ||
| .call() | ||
| .unwrap(); |
There was a problem hiding this comment.
Will paste a previous answer:
the motivation of this state reader is to use it as a tool for debugging fee-related problems, which are the priority now. There are lots of .unwrap() which can all be solved in a future PR
rpc_state_reader_sn_api/src/lib.rs
Outdated
| "id": 1 | ||
| }); | ||
| self.rpc_call::<RpcResult<RpcTransactionReceipt>>(¶ms) | ||
| .unwrap() |
There was a problem hiding this comment.
Will paste a previous answer:
the motivation of this state reader is to use it as a tool for debugging fee-related problems, which are the priority now. There are lots of .unwrap() which can all be solved in a future PR
mmsc2
approved these changes
Aug 29, 2023
pefontana
approved these changes
Aug 29, 2023
fguthmann
pushed a commit
that referenced
this pull request
Sep 22, 2023
* 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 <[email protected]>
fguthmann
pushed a commit
that referenced
this pull request
Oct 2, 2023
* 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 <[email protected]>
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 <[email protected]> * 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 <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * change to try_from * Move functions to its respective files * Test * Delete test * Fix format * Fix test --------- Co-authored-by: Juan Bono <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Juan Bono <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: mmsc2 <[email protected]> Co-authored-by: Edgar Luque <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Juan Bono <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: mmsc2 <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Juan Bono <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: mmsc2 <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> * 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 <[email protected]> Co-authored-by: Mario Rugiero <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Mario Rugiero <[email protected]> * 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 <[email protected]> Co-authored-by: Edgar Luque <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Juan Bono <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> Co-authored-by: Edgar <[email protected]> Co-authored-by: MrAzteca <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: mmsc2 <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: Milton <[email protected]> Co-authored-by: Mario Rugiero <[email protected]> Co-authored-by: marioiordanov <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added Starknet API / Blockifier RPC State Reader
Description
This PR reimplements the RPC State Reader with types of
starknet_apiandblockifier, as it'll be used for re-executing transactions using blockifier and compare the results with starknet_in_rust.