-
Notifications
You must be signed in to change notification settings - Fork 25
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): use test_utils::invoke_tx() instead of trans::test_utils::account_invoke_tx() #2428
chore(blockifier): use test_utils::invoke_tx() instead of trans::test_utils::account_invoke_tx() #2428
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
c5e3b6f
to
4eff1f1
Compare
c2255ad
to
15d3b81
Compare
4eff1f1
to
cce03af
Compare
15d3b81
to
0e07882
Compare
cce03af
to
48dec19
Compare
0e07882
to
b076c16
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2428 +/- ##
===========================================
+ Coverage 40.10% 71.41% +31.31%
===========================================
Files 26 102 +76
Lines 1895 13623 +11728
Branches 1895 13623 +11728
===========================================
+ Hits 760 9729 +8969
- Misses 1100 3480 +2380
- Partials 35 414 +379 ☔ View full report in Codecov by Sentry. |
51280fe
to
c448144
Compare
a9f4ae2
to
ab0a55d
Compare
ab0a55d
to
8b56643
Compare
c448144
to
dee0a42
Compare
8b56643
to
bac51a0
Compare
bac51a0
to
0d554b2
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 5 files at r1, all commit messages.
Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware)
crates/blockifier/src/transaction/execution_flavors_test.rs
line 593 at r1 (raw file):
calldata: recurse_calldata(test_contract_address, true, 3), nonce: nonce_manager.next(account_address), only_query,
Do we still need this on InvokeTxArgs?
Code quote:
only_query,
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 1 of 5 files at r1.
Reviewable status: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware)
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: 3 of 5 files reviewed, all discussions resolved
crates/blockifier/src/transaction/execution_flavors_test.rs
line 593 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Do we still need this on InvokeTxArgs?
Non-blocking, let it be for now
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: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware)
crates/blockifier/src/transaction/test_utils.rs
line 297 at r1 (raw file):
let only_query = invoke_args.only_query; AccountTransaction { tx: invoke_tx(invoke_args), only_query } }
So, can you delete this now?
Code quote:
pub fn account_invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction {
let only_query = invoke_args.only_query;
AccountTransaction { tx: invoke_tx(invoke_args), only_query }
}
…_utils::account_invoke_tx()
bac51a0
to
b523e04
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: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware)
crates/blockifier/src/transaction/execution_flavors_test.rs
line 593 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Non-blocking, let it be for now
It seems like it is still used.
though strangely, I created a PR removing it and it works.
I need to look into this.
my source of confusion is that test_account_flow_test doesn't fail although info is lost after the removal of only_query from invoke_tx_args
link to code:
#[values(true, false)] only_query: bool, |
PR:
#2437
crates/blockifier/src/transaction/test_utils.rs
line 297 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
So, can you delete this now?
this is my intention. It is still used quite a lot in the code. added a TODO.
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: 3 of 5 files reviewed, all discussions resolved
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 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @avivg-starkware)
* 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * fix(starknet_sierra_compile): fix build.rs Signed-off-by: Dori Medini <[email protected]> * 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 <[email protected]> Co-authored-by: YaelD <[email protected]> Co-authored-by: aner-starkware <[email protected]> Co-authored-by: yoavGrs <[email protected]> Co-authored-by: matan-starkware <[email protected]> Co-authored-by: guy-starkware <[email protected]> Co-authored-by: dorimedini-starkware <[email protected]> Co-authored-by: Itay Tsabary <[email protected]> Co-authored-by: avivg-starkware <[email protected]> Co-authored-by: Yair Bakalchuk <[email protected]> Co-authored-by: Arnon Hod <[email protected]> Co-authored-by: Alon Haramati <[email protected]> Co-authored-by: Asmaa Magdoub <[email protected]> Co-authored-by: alon-dotan-starkware <[email protected]> Co-authored-by: AvivYossef-starkware <[email protected]> Co-authored-by: giladchase <[email protected]> Co-authored-by: Gilad Chase <[email protected]> Co-authored-by: DvirYo-starkware <[email protected]> Co-authored-by: nimrod-starkware <[email protected]> Co-authored-by: Meshi Peled <[email protected]> Co-authored-by: Yoni <[email protected]> Co-authored-by: Yael Doweck <[email protected]> Co-authored-by: ShahakShama <[email protected]> Co-authored-by: Alon-Lukatch-Starkware <[email protected]> Co-authored-by: Yonatan-Starkware <[email protected]> Co-authored-by: Yair <[email protected]> Co-authored-by: Itay-Tsabary-Starkware <[email protected]>
No description provided.