wip: review comments

Still need to add tests for setting a pending client release and for the migration

crates/vault-registry/src/lib.rs

@@ -45,7 +45,9 @@ use frame_support::{
     dispatch::{DispatchError, DispatchResult},
     ensure,
     traits::Get,
-    transactional, PalletId,
+    transactional,
+    weights::Weight,
+    PalletId,
 };
 use frame_system::{
     ensure_signed,
@@ -418,18 +420,34 @@ pub mod pallet {
             Ok(())
         }
 
-        /// Changes the collateral liquidation threshold for a currency (only executable by the Root account)
+        /// Sets the current client release version, in case of a bug fix or patch.
         ///
         /// # Arguments
         /// * `version` - the semver version, where the zero-index element is the major version
         /// * `checksum` - the SHA256 checksum of the client binary
         #[pallet::weight(<T as Config>::WeightInfo::set_liquidation_collateral_threshold())]
         #[transactional]
-        pub fn set_client_release(origin: OriginFor<T>, version: [u32; 3], checksum: [u8; 32]) -> DispatchResult {
+        pub fn set_current_client_release(origin: OriginFor<T>, uri: Vec<u8>, code_hash: T::Hash) -> DispatchResult {
             ensure_root(origin)?;
-            // TODO: Should the new semver version have a check that ensures it is striclty ascending?
-            LatestClientRelease::<T>::put(ClientRelease { version, checksum });
-            Self::deposit_event(Event::<T>::ClientRelease { version, checksum });
+            let release = ClientRelease { uri, code_hash };
+            CurrentClientRelease::<T>::put(release.clone());
+            Self::deposit_event(Event::<T>::ApplyClientRelease { release });
+            Ok(())
+        }
+
+        /// Sets the pending client release version. To be batched alongside the
+        /// `parachainSystem.enactAuthorizedUpgrade` relay chain xcm call.
+        ///
+        /// # Arguments
+        /// * `version` - the semver version, where the zero-index element is the major version
+        /// * `checksum` - the SHA256 checksum of the client binary
+        #[pallet::weight(<T as Config>::WeightInfo::set_liquidation_collateral_threshold())]
+        #[transactional]
+        pub fn set_pending_client_release(origin: OriginFor<T>, uri: Vec<u8>, code_hash: T::Hash) -> DispatchResult {
+            ensure_root(origin)?;
+            let release = ClientRelease { uri, code_hash };
+            PendingClientRelease::<T>::put(Some(release.clone()));
+            Self::deposit_event(Event::<T>::NotifyClientRelease { release });
             Ok(())
         }
     }
@@ -543,9 +561,11 @@ pub mod pallet {
             vault_id: DefaultVaultId<T>,
             banned_until: T::BlockNumber,
         },
-        ClientRelease {
-            version: [u32; 3],
-            checksum: [u8; 32],
+        NotifyClientRelease {
+            release: ClientRelease<T::Hash>,
+        },
+        ApplyClientRelease {
+            release: ClientRelease<T::Hash>,
         },
     }
 
@@ -673,8 +693,14 @@ pub mod pallet {
     /// Tuple of (semver_version, sha256_checksum) indicating the latest vault client release.
     /// `semver_version`: The element at index zero is the major semver version.
     #[pallet::storage]
-    #[pallet::getter(fn latest_client_release)]
-    pub(super) type LatestClientRelease<T: Config> = StorageValue<_, ClientRelease, ValueQuery>;
+    #[pallet::getter(fn current_client_release)]
+    pub(super) type CurrentClientRelease<T: Config> = StorageValue<_, ClientRelease<T::Hash>, ValueQuery>;
+
+    /// Tuple of (semver_version, sha256_checksum) indicating the latest vault client release.
+    /// `semver_version`: The element at index zero is the major semver version.
+    #[pallet::storage]
+    #[pallet::getter(fn pending_client_release)]
+    pub(super) type PendingClientRelease<T: Config> = StorageValue<_, Option<ClientRelease<T::Hash>>, ValueQuery>;
 
     #[pallet::type_value]
     pub(super) fn DefaultForStorageVersion() -> Version {
@@ -1609,6 +1635,19 @@ impl<T: Config> Pallet<T> {
         Ok(Amount::new(amount, currency))
     }
 
+    /// If a pending client release exists, set the current release to that.
+    /// The pending release becomes `None`.
+    pub fn try_upgrade_current_vault_release() -> Weight {
+        let mut writes: Weight = 0;
+        if let Some(pending_release) = Self::pending_client_release() {
+            CurrentClientRelease::<T>::put(pending_release);
+            let no_release: Option<ClientRelease<T::Hash>> = None;
+            PendingClientRelease::<T>::put(no_release);
+            writes = 2;
+        }
+        T::DbWeight::get().writes(writes)
+    }
+
     /// RPC
 
     /// get all vaults the are registered using the given account id. Note that one account id might be

crates/vault-registry/src/types.rs

@@ -11,7 +11,7 @@ pub use primitives::{VaultCurrencyPair, VaultId};
 use scale_info::TypeInfo;
 use sp_core::H256;
 use sp_runtime::traits::{CheckedAdd, CheckedSub, Zero};
-use sp_std::collections::btree_set::BTreeSet;
+use sp_std::{collections::btree_set::BTreeSet, vec::Vec};
 
 #[cfg(test)]
 use mocktopus::macros::mockable;
@@ -33,12 +33,12 @@ pub enum Version {
     V4,
 }
 
-#[derive(Encode, Decode, Default, TypeInfo, Eq, PartialEq, Debug)]
-pub struct ClientRelease {
+#[derive(Encode, Decode, Eq, PartialEq, Clone, Default, TypeInfo, Debug)]
+pub struct ClientRelease<Hash> {
     /// The semver version, where the zero-index element is the major version
-    pub version: [u32; 3],
+    pub uri: Vec<u8>,
     /// SHA256 checksum of the client binary
-    pub checksum: [u8; 32],
+    pub code_hash: Hash,
 }
 
 #[derive(Debug, PartialEq)]

parachain/runtime/interlay/src/lib.rs

@@ -454,14 +454,13 @@ impl pallet_preimage::Config for Runtime {
 pub struct SchedulerMigrationV3;
 impl frame_support::traits::OnRuntimeUpgrade for SchedulerMigrationV3 {
     fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        Scheduler::migrate_v2_to_v3()
+        Scheduler::migrate_v2_to_v3() + VaultRegistry::try_upgrade_current_vault_release()
     }
 
     #[cfg(feature = "try-runtime")]
     fn pre_upgrade() -> Result<(), &'static str> {
         Scheduler::pre_migrate_to_v3()
     }
-
     #[cfg(feature = "try-runtime")]
     fn post_upgrade() -> Result<(), &'static str> {
         Scheduler::post_migrate_to_v3()

parachain/runtime/kintsugi/src/lib.rs

@@ -381,11 +381,12 @@ mod migrate_aura {
 pub struct AuraMigration;
 impl frame_support::traits::OnRuntimeUpgrade for AuraMigration {
     fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        if migrate_aura::should_run_upgrade() {
+        let aura_migration = if migrate_aura::should_run_upgrade() {
             migrate_aura::on_runtime_upgrade()
         } else {
             Default::default()
-        }
+        };
+        aura_migration + VaultRegistry::try_upgrade_current_vault_release()
     }
 
     #[cfg(feature = "try-runtime")]

parachain/runtime/testnet-interlay/src/lib.rs

@@ -425,7 +425,7 @@ impl pallet_preimage::Config for Runtime {
 pub struct SchedulerMigrationV3;
 impl frame_support::traits::OnRuntimeUpgrade for SchedulerMigrationV3 {
     fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        Scheduler::migrate_v2_to_v3()
+        Scheduler::migrate_v2_to_v3() + VaultRegistry::try_upgrade_current_vault_release()
     }
 
     #[cfg(feature = "try-runtime")]

parachain/runtime/testnet-kintsugi/src/lib.rs

@@ -425,7 +425,7 @@ impl pallet_preimage::Config for Runtime {
 pub struct SchedulerMigrationV3;
 impl frame_support::traits::OnRuntimeUpgrade for SchedulerMigrationV3 {
     fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        Scheduler::migrate_v2_to_v3()
+        Scheduler::migrate_v2_to_v3() + VaultRegistry::try_upgrade_current_vault_release()
     }
 
     #[cfg(feature = "try-runtime")]

standalone/runtime/src/lib.rs

@@ -401,7 +401,7 @@ impl pallet_preimage::Config for Runtime {
 pub struct SchedulerMigrationV3;
 impl frame_support::traits::OnRuntimeUpgrade for SchedulerMigrationV3 {
     fn on_runtime_upgrade() -> frame_support::weights::Weight {
-        Scheduler::migrate_v2_to_v3()
+        Scheduler::migrate_v2_to_v3() + VaultRegistry::try_upgrade_current_vault_release()
     }
 
     #[cfg(feature = "try-runtime")]

standalone/runtime/tests/test_vault_registry.rs

@@ -234,15 +234,16 @@ mod withdraw_collateral_test {
     fn integration_test_vault_registry_set_client_release_works() {
         test_with(|_vault_id| {
             let new_release = ClientRelease {
-                version: [1, 0, 2],
-                checksum: [1u8; 32],
+                uri: b"https://github.com/interlay/interbtc-clients/releases/download/1.14.0/vault-standalone-metadata"
+                    .to_vec(),
+                code_hash: H256::default(),
             };
-            assert_ok!(Call::VaultRegistry(VaultRegistryCall::set_client_release {
-                version: new_release.version,
-                checksum: new_release.checksum
+            assert_ok!(Call::VaultRegistry(VaultRegistryCall::set_current_client_release {
+                uri: new_release.uri.clone(),
+                code_hash: new_release.code_hash.clone()
             })
             .dispatch(root()));
-            assert_eq!(VaultRegistryPallet::latest_client_release(), new_release);
+            assert_eq!(VaultRegistryPallet::current_client_release(), new_release);
         });
     }
 }