Conversation
tomusdrw
requested review from
Demi-Marie,
andresilva and
kianenigma
as code owners
tomusdrw
requested review from
marcio-diaz and
shawntabrizi
and removed request for
Demi-Marie and
andresilva
kianenigma
left a comment
kianenigma
left a comment
There was a problem hiding this comment.
Looks overall very good to me.
…fences-benchmarks' into shawntabrizi-fix-offences-benchmarks
frame/balances/src/lib.rs
Outdated
| /// # </weight> | ||
| #[weight = T::DbWeight::get().reads_writes(1, 1) + 100_000_000] | ||
| fn set_balance( | ||
| pub fn set_balance( |
There was a problem hiding this comment.
Just started reviewing, but you shouldn't need to make this pub. You can instead call make_free_balance_be which is part of the currency trait.
shawntabrizi
left a comment
shawntabrizi
left a comment
There was a problem hiding this comment.
Need to fix the balance/event stuff
|
|
||
| #![cfg_attr(not(feature = "std"), no_std)] | ||
| #![forbid(unused_must_use, unsafe_code, unused_variables, unused_must_use)] | ||
| #![warn(unused_must_use, unsafe_code, unused_variables, unused_must_use)] |
There was a problem hiding this comment.
is this intentional (or a result of debugging)?
There was a problem hiding this comment.
Intentional. Using forbid may cause issues with:
- Quick development cycle
- Build randomly breaking on rustc upgrades
We did try that in the past, but it was doing more harm than good.
|
bot merge |
tomusdrw
added
A8-mergeoncegreen
and removed
A0-please_review
| let raw_amount = bond_amount::<T>(); | ||
| // add twice as much balance to prevent the account from being killed. | ||
| let free_amount = raw_amount.saturating_mul(2.into()); | ||
| T::Currency::make_free_balance_be(&stash, free_amount); |
There was a problem hiding this comment.
just a note for whoever might read this in the future and want to work on benchmarks: we use this function quite a lot, and it does not change T::Currency::issuance. And we use total issuance in staking's phragmen, and it can cause some very hard to track bugs... so keep that in mind.
kianenigma
left a comment
kianenigma
left a comment
There was a problem hiding this comment.
Had another brief look and looks okay to me!
5 participants
An attempt to benchmark
offenceswith the same kind of reports that are most likely going to be used in the final runtime (likeim-online,grandpaandbabe).Even though we don't report
babe(see #5752) norgrandpa(see #3868) yet, the final report is not likely to change that much (or at all).