-
Notifications
You must be signed in to change notification settings - Fork 22
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
test(blockifier): test enforce_fee flag with new resource bounds #1311
test(blockifier): test enforce_fee flag with new resource bounds #1311
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1311 +/- ##
===========================================
+ Coverage 40.10% 69.12% +29.02%
===========================================
Files 26 100 +74
Lines 1895 13446 +11551
Branches 1895 13446 +11551
===========================================
+ Hits 760 9295 +8535
- Misses 1100 3750 +2650
- Partials 35 401 +366 ☔ View full report in Codecov by Sentry. |
baf905b
to
0b69dfd
Compare
63f505b
to
b1eb61d
Compare
0b69dfd
to
bc9fb5a
Compare
b1eb61d
to
ad71ad0
Compare
bc9fb5a
to
5a44e4a
Compare
ad71ad0
to
94b0ea2
Compare
5a44e4a
to
c941c05
Compare
826231f
to
24b7c7a
Compare
c941c05
to
700647a
Compare
24b7c7a
to
fd1a2e9
Compare
700647a
to
4661ae0
Compare
fd1a2e9
to
701e17e
Compare
4661ae0
to
34f2cfe
Compare
701e17e
to
5b48978
Compare
34f2cfe
to
6c50f41
Compare
5b48978
to
3a92389
Compare
6c50f41
to
356d3c9
Compare
7338867
to
310f1b1
Compare
356d3c9
to
e293497
Compare
310f1b1
to
6d46d6a
Compare
e293497
to
d310e7f
Compare
6d46d6a
to
dd72a30
Compare
d310e7f
to
947b091
Compare
dd72a30
to
ed36be2
Compare
947b091
to
a8bda94
Compare
ed36be2
to
20e99f2
Compare
a8bda94
to
409e971
Compare
20e99f2
to
e5f3853
Compare
625b6a3
to
a3a6d78
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 1 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware)
crates/blockifier/src/transaction/account_transactions_test.rs
line 187 at r1 (raw file):
deploy_account_tx_args! { class_hash: account.get_class_hash(), max_fee: Fee(u128::from(bound_multiplier) * MAX_FEE.0),
I prefer to write it explicitly.
(If you accept it, remove thebound_multiplier
)
Suggestion:
max_fee: Fee(if zero_bounds { 0 } else { MAX_FEE.0 }),
crates/blockifier/src/transaction/account_transactions_test.rs
line 208 at r1 (raw file):
let account_tx = AccountTransaction::DeployAccount(deploy_account_tx); let enforce_fee = account_tx.create_tx_info().enforce_fee();
Please verify that
assert_ne!(zero_bounds, enforce_fee);
crates/blockifier/src/transaction/account_transactions_test.rs
line 210 at r1 (raw file):
let enforce_fee = account_tx.create_tx_info().enforce_fee(); let result = account_tx.execute(state, &block_context, enforce_fee, true); assert_eq!(result.is_err(), enforce_fee);
Explain in a comment, why the tx fails where there are gas bounds.
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 1 files reviewed, 2 unresolved discussions (waiting on @yoavGrs)
crates/blockifier/src/transaction/account_transactions_test.rs
line 208 at r1 (raw file):
Previously, yoavGrs wrote…
Please verify that
assert_ne!(zero_bounds, enforce_fee);
Done.
crates/blockifier/src/transaction/account_transactions_test.rs
line 210 at r1 (raw file):
Previously, yoavGrs wrote…
Explain in a comment, why the tx fails where there are gas bounds.
Done.
a3a6d78
to
f76c2ef
Compare
Artifacts upload triggered. View details here |
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 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)
This change is