Add remote proxy support to AH Kusama #535
Conversation
I will also port it there. I wanted to include it in the next release if possible. People are having more and more these problems of proxies not being on AH and similar. This pallet could solve the issue without requiring a root referendum (we already had one and another is prepared AFAIK). |
bkchr
changed the title
|
I'd be more comfortable if we also did a security audit for this before adding it to live Polkadot AssetHub |
The scope of the changes is quite narrow. Doing some proper review should be fine. The only difference to the real |
bkchr
changed the title
<!-- Remember that you can run `/merge` to enable auto-merge in the PR --> <!-- Remember to modify the changelog. If you don't need to modify it, you can check the following box. Instead, if you have already modified it, simply delete the following line. --> - [ ] Does not require a CHANGELOG entry
|
/cmd bench --pallet pallet_remote_proxy |
|
Command "bench --pallet pallet_remote_proxy" has started 🚀 See logs here |
|
/cmd bench --pallet pallet_remote_prox |
|
/cmd bench --pallet pallet_remote_proxy |
|
Command "bench --pallet pallet_remote_proxy" has started 🚀 See logs here |
plus feature propagation and compilation is fixed here: #573 |
|
Command "bench --pallet pallet_remote_proxy" has finished ✅ See logs here |
<!-- Remember that you can run `/merge` to enable auto-merge in the PR --> <!-- Remember to modify the changelog. If you don't need to modify it, you can check the following box. Instead, if you have already modified it, simply delete the following line. --> - [ ] Does not require a CHANGELOG entry
| authors.workspace = true | ||
| edition.workspace = true | ||
| repository.workspace = true | ||
| license.workspace = true |
There was a problem hiding this comment.
| license.workspace = true | |
| license = "Apache-2.0" |
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
| /// The remote block number. | ||
| type RemoteBlockNumber: Parameter + Saturating + MaxEncodedLen + Default; | ||
| /// The hash type used by the remote chain. | ||
| type Hash: Parameter + MaxEncodedLen; |
There was a problem hiding this comment.
| type Hash: Parameter + MaxEncodedLen; | |
| type RemoteHash: Parameter + MaxEncodedLen; |
Nit, but maybe not worth renaming.
| /// The hash type used by the remote chain. | ||
| type Hash: Parameter + MaxEncodedLen; | ||
| /// The hasher used by the remote chain. | ||
| type Hasher: Hasher<Out = Self::Hash>; |
There was a problem hiding this comment.
| type Hasher: Hasher<Out = Self::Hash>; | |
| type RemoreHasher: Hasher<Out = Self::RemoteHash>; |
Ditto
pallets/remote-proxy/src/lib.rs
Outdated
|
|
||
| // Update the block to root mappings. | ||
| BlockToRoot::<T>::insert(&block, hash); | ||
| BlockToRoot::<T>::remove(block.saturating_sub(T::MaxStorageRootsToKeep::get())); |
There was a problem hiding this comment.
Hm how does this work?
Would this not only get every second block from the relay? Then how does it ensure to clean up the storage? Or do you just assume that it is always every second and never misses a block?
| } | ||
|
|
||
| impl<T: Config, I: 'static> OnSystemEvent for Pallet<T, I> { | ||
| fn on_validation_data(validation_data: &PersistedValidationData) { |
There was a problem hiding this comment.
Should we force-consume weight here? This function does not seem to expect any weight return or similar.
Or we include its weight in on_initialize.
| let key = T::RemoteProxy::proxy_definition_storage_key(&real_remote); | ||
|
|
||
| let db = | ||
| sp_trie::StorageProof::new(proof).into_memory_db::<RemoteHasherOf<T, I>>(); |
There was a problem hiding this comment.
Is this a heavy operation or something that consumes lots of memory? Then maybe we could enforce a sane upper bound for the proof size and abort if someone puts multiple megabyte.
| }, | ||
| }; | ||
|
|
||
| ensure!(def.delay.is_zero(), Error::<T, I>::Unannounced); |
There was a problem hiding this comment.
Important check, and also good to mention in the docs that this only works for delay=0 proxies.
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
This adds remote proxy support to AssetHub on Polkadot and Kusama. Currently it is configured only to support using proxies registered on the relay chain to work on AssetHub. In the future this can be expanded.