Add Paras authorize_code_hash + apply_authorized_code feature
#7592
Conversation
|
/cmd fmt |
…-set-current-code
…-set-current-code
…code' into bko-paras-authorize-set-current-code
bkontur
added
T14-system_parachains
|
/cmd prdoc --audience runtime_dev --bump patch |
|
/cmd bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo |
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has started 🚀 See logs here |
…parachains::paras --runtime westend rococo'
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
| /// triggering the same functionality as `force_set_current_code`. | ||
| #[pallet::call_index(10)] | ||
| #[pallet::weight(<T as Config>::WeightInfo::apply_authorized_force_set_current_code(new_code.0.len() as u32))] | ||
| pub fn apply_authorized_force_set_current_code( |
There was a problem hiding this comment.
Could use the feeless_if attribute.
There was a problem hiding this comment.
Could use the
feeless_ifattribute.
I don't see much example of feeless_if, but iiuc it can do calculation just based on the inputs?
Or should I do something like:
#[pallet::feeless_if(|_: &OriginFor<T>, para: ParaId, validation_code: ValidationCode| -> bool {
// check that the code was applied
Self::current_code(para) == Some(validation_code.hash())
})]
But this way, we allow spamming the chain for free, because I cannot check Self::current_code(para) before and after.
There was a problem hiding this comment.
because I cannot check Self::current_code(para) before and after.
What you mean by this?
There was a problem hiding this comment.
If I understand correctly, the purpose of using feeless_if here was to allow free execution when the code is actually applied, essentially the same effect as the PostDispatchInfo hack at the end (which should be removed when using feeless_if):
let post = PostDispatchInfo {
// Consume the rest of the block to prevent further transactions
actual_weight: Some(T::BlockWeights::get().max_block),
// No fee for a valid upgrade
pays_fee: Pays::No,
};
Ok(post)However, in this case, the PostDispatchInfo hack is applied after validations, when do_force_set_current_code_update is actually executed.
Is this really enough? Is it ok to access storage and calculate hash again inside feeless_if?
#[pallet::feeless_if(|_: &OriginFor<T>, para: ParaId, validation_code: ValidationCode| -> bool {
// check that the code was applied
Self::current_code(para) == Some(validation_code.hash())
})]
because I cannot check Self::current_code(para) before and after.
Forget about it, I'm probably overthinking. My idea was to check the result of the extrinsic inside #[pallet::feeless_if], something like (but that's not possible):
let code_before = Self::current_code(para);
.. wait extrinsic to do the job
let code_after = Self::current_code(para);
if code_before ! code_after {
PaysFee::No
} else {
PaysFee::Yes
}
In other words, I’m not sure how to properly use feeless_if here. 😅
There was a problem hiding this comment.
Is this really enough? Is it ok to access storage and calculate hash again inside
feeless_if?
Yes that would be enough.
There was a problem hiding this comment.
IIUC, for feeless_if, we need to add the SkipCheckIfFeeless extension wrapper and configure the pallet accordingly:
https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs#L16-L35
impl pallet_skip_feeless_payment::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
}However, we don’t currently use SkipCheckIfFeeless anywhere in polkadot-sdk (except in kitchensink), polkadot-fellows, or other runtimes.
IIUC, all charge payment extensions below use type Val = Val<T>; and type Pre = Pre<T>;, but SkipCheckIfFeeless instead uses type Val = Intermediate<S::Val, ...> and type Pre = Intermediate<S::Pre, ...>
pallet_skip_feeless_payment::SkipCheckIfFeeless<Runtime,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
>
pallet_skip_feeless_payment::SkipCheckIfFeeless<Runtime,
pallet_asset_conversion_tx_payment::ChargeAssetTxPayment<Runtime>,
>
pallet_skip_feeless_payment::SkipCheckIfFeeless<Runtime,
pallet_asset_tx_payment::ChargeAssetTxPayment<Runtime>,
>IIUC, based on the kitchensink extras, which include SkipCheckIfFeeless, and the usage of get_eth_extension, it should be safe to add without breaking anything.
-
Could you help confirm whether it is safe to add the
SkipCheckIfFeelesswrapper to existing (relay and/or parachain) runtimes? Specifically, would this change affect backwards compatibility for existingTransactionExtensions, or cause issues for clients submitting extrinsics (e.g., requiring them to change the format or anything else)? -
If it is safe, I can manage/handle adding
SkipCheckIfFeelessto all runtimes. However, should/want we apply it to all runtimes, only relay chains, or just system parachains?
There was a problem hiding this comment.
1. Could you help confirm whether it is safe to add the `SkipCheckIfFeeless` wrapper to existing (relay and/or parachain) runtimes? Specifically, would this change affect backwards compatibility for existing `TransactionExtensions`, or cause issues for clients submitting extrinsics (e.g., requiring them to change the format or anything else)?
The encoding/decoding of transaction is not changed when adding SkipCheckIfFeeless.
In the metadata SkipCheckIfFeeless will not appear and instead only the inner transaction identifier, type and implicit will show up.
2. If it is safe, I can manage/handle adding `SkipCheckIfFeeless` to all runtimes. However, should/want we apply it to all runtimes, only relay chains, or just system parachains?
I don't know audit status but otherwise I don't see anything that would break from this.
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <git@kchr.de>
|
/cmd fmt |
|
/cmd fmt |
|
/cmd prdoc --audience runtime_dev --bump patch --force |
|
/cmd fmt |
|
/cmd bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo |
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has started 🚀 See logs here |
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has failed ❌! See logs here |
|
/cmd bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo |
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has started 🚀 See logs here |
…parachains::paras --runtime westend rococo'
|
Command "bench --pallet polkadot_runtime_parachains::paras --runtime westend rococo" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
Closes: #7574
Relates to: #7591
This feature is useful when triggering a
Paraspallet call from a different chain than the one where theParaspallet is deployed. For example, we may want to sendParas::force_set_current_code(para, code)from the Collectives and/or AssetHub to the relay chain (because the relaychain governance will be migrated to the AssetHub as a part of AHM).The primary reason for this approach is to avoid transferring the entire
new_codeWasm blob between chains. Instead, we authorize thecode_hashusingrootviafn authorize_force_set_current_code_hash(new_authorization, expire_at). This authorization can later be applied by anyone usingParas::apply_authorized_force_set_current_code(para, new_code). Ifexpire_atis reached without the authorization being used, it is automatically removed.TODO
cover alsono see other commentadd_trusted_validation_codeorforce_schedule_code_upgrade- see comment bellow: AddParasauthorize_code_hash+apply_authorized_codefeature #7592 (comment)Open questions
Do we need something likepoke_authorized_code_hash? E.g. in case that we authorize code hash, but nobody would apply it and the parachain starts working with old/other_new code? Is this possible?frame_systempallet andset_code/set_code_without_checks?pallet-whitelist?codeattribute?validate_unsignedforapply_authorized_code?