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
Girazoki supported fee payment assets storage #1118
Girazoki supported fee payment assets storage #1118
Conversation
…t-maintenance' into girazoki-xcm-max-ump-weight-migration
…runtime-pallet-maintenance
…t-maintenance' into girazoki-xcm-max-ump-weight-migration
…d-fee-payment-assets-storage
…d-fee-payment-assets-storage
pub struct PopulateSupportedFeePaymentAssets<T>(PhantomData<T>); | ||
impl<T: Config> OnRuntimeUpgrade for PopulateSupportedFeePaymentAssets<T> { | ||
fn on_runtime_upgrade() -> Weight { | ||
log::info!(target: "PopulateSupportedFeePaymentAssets", "actually running it"); |
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.
These log statements are potentially helpful (I think this is copy-pasted from other migrations) but we could probably rephrase them to have more helpful context. Maybe not info
, too...?
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.
Looks fine so far, although I haven't reviewed it as thoroughly as I'd like.
no worries this is not as high priority as the others and can be included in 1400 |
…d-fee-payment-assets-storage
…d-fee-payment-assets-storage
…d-fee-payment-assets-storage
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.
AssetType
impl Ord
, so use binary_search
instead of push
, see: https://substrate.recipes/vec-set.html
It will allow to find more quickly if an assettype is present or not, in O(log n)
instead of O(n)
…d-fee-payment-assets-storage
What does it do?
Adds a storage with a vec of the supported fee payment assets.
This storage is read when receiving an incoming xcm message, and if the asset is not in this vec, the xcm message is not processed.
This is an improvement with respect to what we had because this storage item will be cached during the first read per block, therefore limiting a potential free DB DOS attack to the number of assets supported
What important points reviewers should know?
Target: runtime 1300
Is there something left for follow-up PRs?
What alternative implementations were considered?
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?