fix: tipping migration (#144)

* fix: generate server id

* fix: server migration

* fix: pallet tipping benchmark

* fix: tipping migration

* chore: bump version 2.1.8

* fix: checking migration

node/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'myriad'
-version = '2.1.7'
+version = '2.1.8'
 edition = '2021'
 license = 'AGPL-3.0'
 authors = ['Myriad Dev Team <[email protected]>']

pallets/server/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'pallet-server'
-version = '2.1.7'
+version = '2.1.8'
 edition = '2021'
 license = 'AGPL-3.0'
 authors = ['Myriad Dev Team <[email protected]>']

pallets/server/src/benchmarking.rs

@@ -5,6 +5,7 @@ use super::*;
 #[allow(unused)]
 use crate::{Pallet as Server, ServerInterface};
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
+use frame_support::sp_runtime::SaturatedConversion;
 use frame_system::RawOrigin;
 use sp_std::vec;
 

pallets/server/src/functions.rs

@@ -1,22 +1,6 @@
 use crate::*;
 
 impl<T: Config> Pallet<T> {
-	pub fn can_update_server(
-		server_id: u64,
-		account_id: &T::AccountId,
-	) -> Result<ServerOf<T>, Error<T>> {
-		let server =
-			<Self as ServerInterface<T>>::get_by_id(server_id).ok_or(Error::<T>::NotExists)?;
-
-		let current_owner = server.get_owner();
-
-		if current_owner != account_id {
-			return Err(Error::<T>::Unauthorized)
-		}
-
-		Ok(server)
-	}
-
 	pub fn do_mutate_server(
 		server_id: u64,
 		owner: &T::AccountId,
@@ -28,48 +12,20 @@ impl<T: Config> Pallet<T> {
 					return Err(Error::<T>::Unauthorized)
 				}
 
-				*server = match data {
+				let updated_server = match data {
 					ServerDataKind::Owner(new_owner) => server.clone().set_owner(new_owner),
 					ServerDataKind::ApiUrl(new_url) => server.clone().set_api_url(new_url),
 				};
 
+				ServerByOwner::<T>::insert(owner, server_id, &updated_server);
+
+				*server = updated_server;
+
 				Ok(())
 			},
 			None => Err(Error::<T>::NotExists),
 		})?;
 
 		Ok(())
 	}
-
-	pub fn do_set_server(
-		register: bool,
-		operator: &OperatorKind,
-		server: &ServerOf<T>,
-	) -> Result<(), Error<T>> {
-		ServerCount::<T>::try_mutate(|value| {
-			let result = match operator {
-				OperatorKind::Add => value.checked_add(1),
-				OperatorKind::Sub => value.checked_sub(1),
-			};
-
-			let total_value = result.ok_or(Error::<T>::Overflow)?;
-
-			*value = total_value;
-
-			Ok(())
-		})?;
-
-		let server_id = server.get_id();
-		let owner = server.get_owner();
-
-		if register {
-			ServerById::<T>::insert(server_id, server);
-			ServerByOwner::<T>::insert(owner, server_id, server);
-		} else {
-			ServerById::<T>::remove(server_id);
-			ServerByOwner::<T>::remove(owner, server_id);
-		}
-
-		Ok(())
-	}
 }

pallets/server/src/impl_server.rs

@@ -10,9 +10,17 @@ impl<T: Config> ServerInterface<T> for Pallet<T> {
 
 	fn register(owner: &T::AccountId, api_url: &[u8]) -> Result<Self::Server, Self::Error> {
 		let count = Self::server_count();
-		let server = Server::new(count, owner, api_url);
+		let index = Self::server_index();
 
-		Self::do_set_server(true, &OperatorKind::Add, &server)?;
+		let server = Server::new(index, owner, api_url);
+
+		let updated_count = count.checked_add(1).ok_or(Error::<T>::Overflow)?;
+		let updated_index = index.checked_add(1).ok_or(Error::<T>::Overflow)?;
+
+		ServerCount::<T>::set(updated_count);
+		ServerIndex::<T>::set(updated_index);
+		ServerById::<T>::insert(index, &server);
+		ServerByOwner::<T>::insert(owner, index, &server);
 
 		Ok(server)
 	}
@@ -24,6 +32,8 @@ impl<T: Config> ServerInterface<T> for Pallet<T> {
 	) -> Result<(), Self::Error> {
 		Self::do_mutate_server(server_id, owner, &ServerDataKind::Owner(new_owner.clone()))?;
 
+		ServerByOwner::<T>::swap(owner, server_id, new_owner, server_id);
+
 		Ok(())
 	}
 
@@ -38,9 +48,20 @@ impl<T: Config> ServerInterface<T> for Pallet<T> {
 	}
 
 	fn unregister(server_id: u64, owner: &T::AccountId) -> Result<(), Self::Error> {
-		let server = Self::can_update_server(server_id, owner)?;
+		let server =
+			<Self as ServerInterface<T>>::get_by_id(server_id).ok_or(Error::<T>::NotExists)?;
+
+		let current_owner = server.get_owner();
+
+		if current_owner != owner {
+			return Err(Error::<T>::Unauthorized)
+		}
+
+		let count = Self::server_count().checked_sub(1).ok_or(Error::<T>::Overflow)?;
 
-		Self::do_set_server(false, &OperatorKind::Sub, &server)?;
+		ServerById::<T>::remove(server_id);
+		ServerByOwner::<T>::remove(owner, server_id);
+		ServerCount::<T>::set(count);
 
 		Ok(())
 	}

pallets/server/src/lib.rs

@@ -23,7 +23,7 @@ pub use weights::WeightInfo;
 use frame_support::traits::StorageVersion;
 
 /// The current storage version.
-const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
+const STORAGE_VERSION: StorageVersion = StorageVersion::new(3);
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -49,6 +49,10 @@ pub mod pallet {
 	#[pallet::getter(fn server_count)]
 	pub type ServerCount<T> = StorageValue<_, ServerId, ValueQuery>;
 
+	#[pallet::storage]
+	#[pallet::getter(fn server_index)]
+	pub type ServerIndex<T> = StorageValue<_, u64, ValueQuery>;
+
 	#[pallet::storage]
 	#[pallet::getter(fn server_by_id)]
 	pub(super) type ServerById<T: Config> = StorageMap<_, Blake2_128Concat, ServerId, ServerOf<T>>;

pallets/server/src/migrations.rs

@@ -1,6 +1,7 @@
 use crate::{
-	AccountIdOf, Config, Pallet, Server as NewServer, ServerById as NewServerById, ServerByOwner,
-	ServerCount,
+	AccountIdOf, Config, Pallet, Server as NewServer, ServerById as NewServerById,
+	ServerByOwner as NewServerByOwner, ServerCount as NewServerCount, ServerId,
+	ServerIndex as NewServerIndex, ServerOf,
 };
 use frame_support::{
 	generate_storage_alias, pallet_prelude::*, traits::Get, weights::Weight, Blake2_128Concat,
@@ -11,49 +12,141 @@ pub fn migrate<T: Config>() -> Weight {
 	use frame_support::traits::StorageVersion;
 
 	let mut weight: Weight = 0;
-	let version = StorageVersion::get::<Pallet<T>>();
+	let mut version = StorageVersion::get::<Pallet<T>>();
+
+	if version < 1 {
+		weight = weight.saturating_add(versions::v1::migrate::<T>());
+		version = StorageVersion::new(1);
+	}
 
 	if version == 1 {
-		weight = weight.saturating_add(v2::migrate::<T>());
-		StorageVersion::new(2).put::<Pallet<T>>();
+		weight = weight.saturating_add(versions::v2::migrate::<T>());
+		version = StorageVersion::new(2);
+	}
+
+	if version == 2 {
+		weight = weight.saturating_add(versions::v3::migrate::<T>());
+		version = StorageVersion::new(3);
 	}
 
+	version.put::<Pallet<T>>();
 	weight
 }
 
-mod v2 {
+mod versions {
 	use super::*;
 
-	pub fn migrate<T: Config>() -> Weight {
-		let mut weight = T::DbWeight::get().writes(1);
-
-		#[allow(dead_code)]
-		#[derive(Encode, Decode, Clone)]
-		pub struct OldServer<AccountId> {
-			id: Vec<u8>,
-			owner: AccountId,
-			name: Vec<u8>,
-			api_url: Vec<u8>,
-			web_url: Vec<u8>,
+	pub mod v1 {
+		use super::*;
+
+		pub fn migrate<T: Config>() -> Weight {
+			let mut weight = T::DbWeight::get().writes(1);
+
+			#[derive(Encode, Decode, Clone)]
+			pub struct OldServer<AccountId> {
+				pub id: Vec<u8>,
+				pub owner: AccountId,
+				pub name: Vec<u8>,
+			}
+
+			#[derive(Encode, Decode, Clone)]
+			pub struct Server<AccountId> {
+				pub id: Vec<u8>,
+				pub owner: AccountId,
+				pub name: Vec<u8>,
+				pub api_url: Vec<u8>,
+				pub web_url: Vec<u8>,
+			}
+
+			generate_storage_alias!(
+				Server,
+				ServerById<T: Config> => Map<(Blake2_128Concat, Vec<u8>), Server<AccountIdOf<T>>>
+			);
+
+			ServerById::<T>::translate(|_key, old: OldServer<AccountIdOf<T>>| {
+				weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
+				Some(Server {
+					id: old.id,
+					owner: old.owner,
+					name: old.name,
+					api_url: "https://api.example.com".as_bytes().to_vec(),
+					web_url: "https://web.example.com".as_bytes().to_vec(),
+				})
+			});
+
+			weight
+		}
+	}
+
+	pub mod v2 {
+		use super::*;
+
+		pub fn migrate<T: Config>() -> Weight {
+			let mut weight = T::DbWeight::get().writes(1);
+
+			#[allow(dead_code)]
+			#[derive(Encode, Decode, Clone)]
+			pub struct OldServer<AccountId> {
+				id: Vec<u8>,
+				owner: AccountId,
+				name: Vec<u8>,
+				api_url: Vec<u8>,
+				web_url: Vec<u8>,
+			}
+
+			generate_storage_alias!(
+				Server,
+				ServerById<T: Config> => Map<(Blake2_128Concat, Vec<u8>), OldServer<AccountIdOf<T>>>
+			);
+
+			ServerById::<T>::translate(|_key, old: OldServer<AccountIdOf<T>>| {
+				weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
+
+				let new_server = NewServer::new(0, &old.owner, &old.api_url);
+
+				NewServerById::<T>::insert(0, new_server.clone());
+				NewServerByOwner::<T>::insert(old.owner, 0, new_server);
+				NewServerCount::<T>::set(1);
+
+				None
+			});
+
+			weight
 		}
+	}
+
+	pub mod v3 {
+		use super::*;
+
+		pub fn migrate<T: Config>() -> Weight {
+			let mut weight = T::DbWeight::get().writes(1);
+
+			NewServerById::<T>::translate(|server_id: ServerId, old: ServerOf<T>| {
+				weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
 
-		generate_storage_alias!(
-			Server,
-			ServerById<T: Config> => Map<(Blake2_128Concat, Vec<u8>), OldServer<AccountIdOf<T>>>
-		);
+				if server_id == 0_u64 {
+					return Some(old)
+				}
 
-		ServerById::<T>::translate(|_key, old: OldServer<AccountIdOf<T>>| {
-			weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
+				None
+			});
 
-			let new_server = NewServer::new(0, &old.owner, &old.api_url);
+			NewServerByOwner::<T>::translate(|_owner, server_id: ServerId, old: ServerOf<T>| {
+				weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
 
-			NewServerById::<T>::insert(0, new_server.clone());
-			ServerByOwner::<T>::insert(old.owner, 0, new_server);
-			ServerCount::<T>::set(1);
+				if server_id == 0_u64 {
+					return Some(old)
+				}
 
-			None
-		});
+				None
+			});
 
-		weight
+			weight = weight.saturating_add(T::DbWeight::get().writes(2));
+
+			NewServerCount::<T>::set(1);
+			NewServerIndex::<T>::set(1);
+
+			weight
+		}
 	}
 }