-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix(blockifier,committer_cli,papyrus_network,papyrus_storage,starknet… #2355
Conversation
Artifacts upload workflows: |
…_client): clippy Signed-off-by: Dori Medini <dori@starkware.co>
a2f879d
to
6c2bab5
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 29 files reviewed, all discussions resolved (waiting on @dorimedini-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.
Reviewed 9 of 29 files at r1.
Reviewable status: 9 of 29 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)
crates/blockifier/src/execution/syscalls/hint_processor.rs
line 190 at r1 (raw file):
/// Error codes returned by Cairo 1.0 code. // "Out of gas";
Does this work?
Suggestion:
/// Error codes returned by Cairo 1.0 code.
// "Out of gas";
crates/blockifier/src/execution/entry_point.rs
line 346 at r1 (raw file):
// Logically, we update remaining steps to `max(0, remaining_steps - steps_to_subtract)`. let remaining_steps = self.n_remaining_steps(); let new_remaining_steps = remaining_steps.saturating_sub(steps_to_subtract);
Uch... saturating_sub
is indeed exactly that - but I would argue it is less readable.
Code quote:
let new_remaining_steps = remaining_steps.saturating_sub(steps_to_subtract);
crates/blockifier/src/fee/gas_usage.rs
line 68 at r1 (raw file):
discount += eth_gas_constants::GAS_PER_MEMORY_WORD - fee_balance_value_cost; let gas = naive_cost.saturating_sub(discount);
Suggestion:
// Cost must be non-negative after discount.
let gas = naive_cost.saturating_sub(discount);
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2355 +/- ##
===========================================
+ Coverage 40.10% 65.38% +25.28%
===========================================
Files 26 226 +200
Lines 1895 24499 +22604
Branches 1895 24499 +22604
===========================================
+ Hits 760 16019 +15259
- Misses 1100 7341 +6241
- Partials 35 1139 +1104 ☔ View full report in Codecov by Sentry. |
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 16 of 29 files at r1, all commit messages.
Reviewable status: 25 of 29 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)
crates/papyrus_test_utils/src/lib.rs
line 365 at r1 (raw file):
////////////////////////////////////////////////////////////////////////// // /// EXTERNAL FUNCTIONS - REMOVE DUPLICATIONS //////////////////////////////////////////////////////////////////////////
hu?
Code quote:
//////////////////////////////////////////////////////////////////////////
// /// EXTERNAL FUNCTIONS - REMOVE DUPLICATIONS
//////////////////////////////////////////////////////////////////////////
Creating a discussion to get a link. Code quote: /// The address of a contract, used for example in [StateDiff](`crate::state::StateDiff`),
/// [DeclareTransaction](`crate::transaction::DeclareTransaction`), and
/// [BlockHeader](`crate::block::BlockHeader`).
// The block hash table is stored in address 0x1, |
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: 25 of 29 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware)
a discussion (no related file):
There is one use in the repo of:
is_none_or
It can be replaced with:
match self.current_height {
Some(h) => h < height,
None => true,
}
Use this to fix if it is blocking.
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 29 files at r1.
Reviewable status: 27 of 29 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware)
crates/papyrus_test_utils/src/lib.rs
line 365 at r1 (raw file):
////////////////////////////////////////////////////////////////////////// // /// EXTERNAL FUNCTIONS - REMOVE DUPLICATIONS //////////////////////////////////////////////////////////////////////////
This is how other headlines in this file are set.
Suggestion:
//////////////////////////////////////////////////////////////////////////
// EXTERNAL FUNCTIONS - REMOVE DUPLICATIONS
//////////////////////////////////////////////////////////////////////////
If we plan to revive this PR, it needs a restack. |
Replaced with #2365 |
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.
Please delete this PR. #2371 was merged.
Reviewable status: 27 of 29 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware)
…_client): clippy