-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(blockifier): remove only_qury from IvokeTxArgs #2437
chore(blockifier): remove only_qury from IvokeTxArgs #2437
Conversation
b79941b
to
62af6a7
Compare
6980513
to
7606e03
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @avivg-starkware)
62af6a7
to
126a0c7
Compare
a3d0730
to
6dce1ef
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2437 +/- ##
===========================================
+ Coverage 40.10% 72.05% +31.94%
===========================================
Files 26 101 +75
Lines 1895 13528 +11633
Branches 1895 13528 +11633
===========================================
+ Hits 760 9747 +8987
- Misses 1100 3366 +2266
- Partials 35 415 +380 ☔ View full report in Codecov by Sentry. |
126a0c7
to
6de5468
Compare
6dce1ef
to
5c8ba11
Compare
6de5468
to
e76d5a4
Compare
5c8ba11
to
1399eee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)
crates/blockifier/src/transaction/account_transactions_test.rs
line 310 at r2 (raw file):
#[case] tx_version: TransactionVersion, #[case] resource_bounds: ValidResourceBounds, #[values(true, false)] only_query: bool,
when removing 'only_query' from InvokeTxArgs this bool is no longer used. the test anyway pass, it doesn't seem like 'only_query' is used to effect the run. @Yoni-Starkware Do you think removing it is wanted?
crates/blockifier/src/transaction/execution_flavors_test.rs
line 188 at r2 (raw file):
cairo_version: CairoVersion, version: TransactionVersion, only_query: bool,
same as above.
it doesn't seem like 'only_query' is used to effect the run.
@Yoni-Starkware Do you think removing it is wanted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 4 files at r3.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @avivg-starkware)
crates/blockifier/src/transaction/account_transactions_test.rs
line 310 at r2 (raw file):
Previously, avivg-starkware wrote…
when removing 'only_query' from InvokeTxArgs this bool is no longer used. the test anyway pass, it doesn't seem like 'only_query' is used to effect the run. @Yoni-Starkware Do you think removing it is wanted?
It's okay to remove it from this one.
execution_falvors_test
tests this bool extensively
crates/blockifier/src/transaction/execution_flavors_test.rs
line 188 at r2 (raw file):
Previously, avivg-starkware wrote…
same as above.
it doesn't seem like 'only_query' is used to effect the run.
@Yoni-Starkware Do you think removing it is wanted?
Yes - if the tests pass this bool directly to the AccountTransaction now, you can remove it.
crates/blockifier/src/transaction/test_utils.rs
line 307 at r3 (raw file):
pub fn account_invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { let only_query = invoke_args.only_query; let execution_flags = ExecutionFlags { only_query, ..ExecutionFlags::default() };
Use new_with_default_flags
Code quote:
let execution_flags = ExecutionFlags::default();
AccountTransaction { tx: invoke_tx(invoke_args), execution_flags }
1399eee
to
b56c3cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)
crates/blockifier/src/transaction/account_transactions_test.rs
line 310 at r2 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
It's okay to remove it from this one.
execution_falvors_test
tests this bool extensively
cool perfect
crates/blockifier/src/transaction/execution_flavors_test.rs
line 188 at r2 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Yes - if the tests pass this bool directly to the AccountTransaction now, you can remove it.
thanks!
e76d5a4
to
3204c4d
Compare
b56c3cc
to
034af8e
Compare
3204c4d
to
df889dd
Compare
034af8e
to
1849cee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)
crates/blockifier/src/transaction/test_utils.rs
line 307 at r3 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Use new_with_default_flags
Done in following PR #2479
df889dd
to
1cdcccb
Compare
1849cee
to
7a4c3d1
Compare
1cdcccb
to
186a1e0
Compare
7a4c3d1
to
b01d6c7
Compare
ea92627
to
4bb5f5b
Compare
b01d6c7
to
93372de
Compare
93372de
to
7499db4
Compare
4bb5f5b
to
6810267
Compare
7499db4
to
cb264d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)
cb264d9
to
12b3f24
Compare
* chore(starknet_batcher): delete obsolete todo (starkware-libs#2389) * chore(blockifier): add global max_sierra_gas to versioned constants (starkware-libs#2330) * feat(starknet_api): checked mul for gas vector (starkware-libs#2300) * feat(consensus): proposer rotates across validators (starkware-libs#2405) * feat(sequencing): validate streamed proposals (starkware-libs#2305) * feat(ci): deny rust warnings in all workflows (starkware-libs#2388) Signed-off-by: Dori Medini <dori@starkware.co> * feat(blockifier): compute allocation cost (starkware-libs#2301) * chore(starknet_sequencer_infra): add dynamic logging util fn commit-id:9ffe9fbe * chore(starknet_sequencer_infra): add tracing test commit-id:76d16e9a * chore(starknet_sequencer_infra): add run_until utility fn commit-id:194a4b6c * chore(infra_utils): change run_until to support async functions commit-id:92e1f8a3 * chore(starknet_integration_tests): use run_until to await for block creation commit-id:667e001c * chore(infra_utils): rename logger struct commit-id:6520ae54 * chore(blockifier): explicit creation of AccountTransaction (starkware-libs#2331) * test(starknet_integration_tests): test commit blocks by running multiple heights * chore(starknet_batcher): set temp gas prices in propose block input (starkware-libs#2341) * chore(starknet_batcher): set use_kzg_da flag in build block input (starkware-libs#2345) * feat(ci): inherit the rust toolchain toml in moonrepo action (starkware-libs#2423) Signed-off-by: Dori Medini <dori@starkware.co> * chore(blockifier): enforce_fee() impl by api::executable_transaction::AccountTransaction (starkware-libs#2377) * chore(starknet_integration_tests): inherit sequencer node's stdout (starkware-libs#2427) * chore(blockifier): invoke() declare() deploy_account() change ret val to api_tx (starkware-libs#2412) * chore(starknet_consensus_manager): set proposer address in propose block input (starkware-libs#2346) * feat(consensus): add observer mode * feat(consensus): sequencer context broadcasts votes (starkware-libs#2422) * chore(deployment): support unified deployment config (starkware-libs#2378) * feat(starknet_api): add sierra version to class info (starkware-libs#2313) * refactor(starknet_api): change default sierra contract class to valid one (starkware-libs#2439) * feat(starknet_l1_provider): add uninitiailized state and make it the default (starkware-libs#2434) This is to comply with upcoming integration with infra, which separates instantiation with initialization. In particular, `Pending` state should be already post-syncing with L1, whereas `Uninitialized` is unsynced and unusable. Co-Authored-By: Gilad Chase <gilad@starkware.com> * feat(papyrus_storage)!: bump storage version for version 13.4 (starkware-libs#2333) * feat(native_blockifier): allow deserialization of python l1_data_gas (starkware-libs#2447) * refactor(blockifier): split FC to groups base on their tags (starkware-libs#2236) * test(consensus): remove warning on into mock propsal part (starkware-libs#2448) * chore(blockifier): use test_utils::invoke_tx() instead of trans::test_utils::account_invoke_tx() (starkware-libs#2428) * chore(blockifier): save sierra to Feature contracts (starkware-libs#2370) * feat(blockifier): don't count Sierra gas in CairoSteps mode (starkware-libs#2440) * chore(blockifier): convert Sierra gas to L1 gas if in L1 gas mode (starkware-libs#2451) * feat(blockifier): add comprehensive state diff versioned constant (starkware-libs#2407) * chore(starknet_consensus_manager): add chain_id to config * refactor(papyrus_p2p_sync): add random_header utility function (starkware-libs#2381) * chore(starknet_batcher): pass block info from consensus to batcher (starkware-libs#2238) * test(starknet_mempool): tx added to mempool are forwarded to propagator client (starkware-libs#2288) * fix: fix CR comments * test(starknet_mempool): tx added to mempool are forwarded to propagator client * feat(sequencing): cache proposals from bigger heights(starkware-libs#2325) * fix: change to latest ubuntu version in feature combo CI (starkware-libs#2414) * chore(blockifier): replace entry_point_gas_cost with initial_budget (starkware-libs#2247) * test(starknet_gateway): handle todo in test_get_block_info (starkware-libs#2267) * chore(starknet_api): revert use get_packaget_dir instead of env var This reverts commit c45f5cc. commit-id:a48736e7 * chore(starknet_api): rely on env::current_dir() instead of CARGO_MANIFEST_DIR commit-id:301ed4eb * chore(blockifier): move env var from run time to compile time commit-id:80a7265d * chore(starknet_sierra_compile): move env var from run time to compile time commit-id:6e7f2a75 * chore: remove the use of zero as a validator id (starkware-libs#2411) * refactor(papyrus_p2p_sync): add_test receives query size instead of constant (starkware-libs#2379) * fix(blockifier): merge state diff with squash (starkware-libs#2310) * feat(blockifier): get revert receipt only in case of revert (starkware-libs#2471) * chore(starknet_integration_tests): create chain info once (starkware-libs#2482) * chore(starknet_sierra_compile): split build utils commit-id:0f504fd7 * chore(starknet_sierra_compile): set runtime-accessible out_dir env var commit-id:539f16db * chore(starknet_sierra_compile): avoid using OUT_DIR in run time commit-id:cd6fba29 * refactor(starknet_api): use const in sierra version (starkware-libs#2477) * chore: cleanups of OUT_DIR env var (starkware-libs#2484) commit-id:18d61b1d * chore(starknet_api): shorten executable_transaction usage path * fix(sequencing): remove old proposal pipes from consensus (starkware-libs#2452) * test(starknet_integration_tests): match sequencer address with default validator id (starkware-libs#2486) * fix(ci): move specific versioned deps to root toml (starkware-libs#2487) * fix(ci): move specific versioned deps to root toml Signed-off-by: Dori Medini <dori@starkware.co> * fix(starknet_sierra_compile): fix build.rs Signed-off-by: Dori Medini <dori@starkware.co> * chore(starknet_batcher): in block builder use the consensus suplied sequncer address (starkware-libs#2409) * chore: cleanups of CARGO_MANIFEST_DIR env var commit-id:c96f2d88 * fix(starknet_sierra_compile): missing import in feature (starkware-libs#2495) commit-id:abd0a286 * chore(blockifier): add keccak_builtin_gas_cost (starkware-libs#2327) * chore(blockifier): add struct ExecutionFlags to AccTx & use instead of transaction::ExecutionFlags (starkware-libs#2429) * chore(blockifier): add new constructor to AccountTransaction (starkware-libs#2431) * chore(blockifier): remove only_qury from IvokeTxArgs (starkware-libs#2437) * chore(blockifier): remove struct ExecutionFlags and replace w concurrency_mode bool (starkware-libs#2470) * chore(blockifier): remove declare.rs deploy_account.rs invoke.rs from blockifier (starkware-libs#2478) --------- Signed-off-by: Dori Medini <dori@starkware.co> Co-authored-by: YaelD <70628564+Yael-Starkware@users.noreply.github.com> Co-authored-by: aner-starkware <147302140+aner-starkware@users.noreply.github.com> Co-authored-by: yoavGrs <97383386+yoavGrs@users.noreply.github.com> Co-authored-by: matan-starkware <97523054+matan-starkware@users.noreply.github.com> Co-authored-by: guy-starkware <guy.n@starkware.co> Co-authored-by: dorimedini-starkware <dori@starkware.co> Co-authored-by: Itay Tsabary <itayt@starkware.co> Co-authored-by: avivg-starkware <aviv.g@starkware.co> Co-authored-by: Yair Bakalchuk <yair@starkware.co> Co-authored-by: Arnon Hod <arnon@starkware.co> Co-authored-by: Alon Haramati <91828241+alonh5@users.noreply.github.com> Co-authored-by: Asmaa Magdoub <asmaa@starkware.co> Co-authored-by: alon-dotan-starkware <alon.dotan@starkware.co> Co-authored-by: AvivYossef-starkware <141143145+AvivYossef-starkware@users.noreply.github.com> Co-authored-by: giladchase <gilad@starkware.co> Co-authored-by: Gilad Chase <gilad@starkware.com> Co-authored-by: DvirYo-starkware <115620476+DvirYo-starkware@users.noreply.github.com> Co-authored-by: nimrod-starkware <143319383+nimrod-starkware@users.noreply.github.com> Co-authored-by: Meshi Peled <141231558+meship-starkware@users.noreply.github.com> Co-authored-by: Yoni <78365039+Yoni-Starkware@users.noreply.github.com> Co-authored-by: Yael Doweck <yael@starkware.co> Co-authored-by: ShahakShama <70578257+ShahakShama@users.noreply.github.com> Co-authored-by: Alon-Lukatch-Starkware <alon.l@starkware.co> Co-authored-by: Yonatan-Starkware <yonatan.k@starkware.co> Co-authored-by: Yair <92672946+yair-starkware@users.noreply.github.com> Co-authored-by: Itay-Tsabary-Starkware <106665835+Itay-Tsabary-Starkware@users.noreply.github.com>
No description provided.