-
Notifications
You must be signed in to change notification settings - Fork 69
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: transaction pause bounded storage #641
Conversation
Crate versions that have not been updated:
Crate versions that have been updated:
Runtime version has not been increased. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #641 +/- ##
==========================================
+ Coverage 64.77% 64.81% +0.04%
==========================================
Files 134 135 +1
Lines 9749 9800 +51
==========================================
+ Hits 6315 6352 +37
- Misses 3434 3448 +14
☔ 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.
hope I'm not being too nitpicky
let mut weight = Weight::zero(); | ||
|
||
let status = v0::PausedTransactions::<T>::drain().collect::<Vec<_>>(); | ||
weight.saturating_accrue(T::DbWeight::get().reads(status.len() as u64)); |
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.
should be one read, one write, no?
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.
Couldn't find these numbers in the docs. I'm just curious, how do you know it's 1w/1r? Thanks.
|
||
StorageVersion::new(1).put::<Pallet<T>>(); | ||
|
||
T::DbWeight::get().reads_writes(1, 1) |
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.
you accumulate weight, but then don't return it
continue; | ||
} | ||
|
||
PausedTransactions::<T>::insert((pallet_name_b.unwrap(), function_name_b.unwrap()), ()); |
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.
nitpick: write with expect
or rewrite the block to not use unwrap
e.g. with match statement like:
match (pallet_name_b, function_name_b) {
(Some(pallet), Some(function)) => insert(...),
_ => log::info!(...)
}
log::info!( | ||
target: TARGET, | ||
"Value not migrated because it's too long: {:?}", | ||
(pallet_name_b, function_name_b) |
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.
probably want to print the unbounded names?
This PR removes
without_storage_info
from the Transaction pause pallet and adds a storage migration.