FIX: cleanup FeeLock queue entries on unlock (#469)

- reproduce bug in dedicated UT
- automatically cleanup UnlockQueue storage entries

pallets/fee-lock/src/lib.rs

@@ -525,7 +525,9 @@ impl<T: Config> FeeLockTriggerTrait<T::AccountId> for Pallet<T> {
 			);
 		}
 
-		FeeLockMetadataQeueuePosition::<T>::take(&who);
+		if let Some(pos) = FeeLockMetadataQeueuePosition::<T>::take(&who) {
+			UnlockQueue::<T>::take(pos);
+		}
 		AccountFeeLockData::<T>::remove(&who);
 
 		Self::deposit_event(Event::FeeLockUnlocked(

pallets/fee-lock/src/tests.rs

@@ -741,12 +741,6 @@ const ACCOUNT_WITH_LOCKED_TOKENS: orml_tokens::AccountData<u128> = AccountData {
 	frozen: 0u128,
 };
 
-const ACCOUNT_WITH_LOCKED_TOKENS_TWICE: orml_tokens::AccountData<u128> = AccountData {
-	free: INITIAL_AMOUNT - 2 * FEE_LOCK_AMOUNT,
-	reserved: 2 * FEE_LOCK_AMOUNT,
-	frozen: 0u128,
-};
-
 fn calculate_estimated_weight(unlock_fee_calls: u64, reads: u64, writes: u64) -> Weight {
 	<Test as frame_system::Config>::DbWeight::get().reads(reads) +
 		<Test as frame_system::Config>::DbWeight::get().writes(writes) +
@@ -1123,3 +1117,45 @@ fn test_unlock_happens_in_order() {
 			);
 		});
 }
+
+#[test]
+fn test_queue_storage_is_cleaned_up() {
+	ExtBuilder::new()
+		.create_token(NativeCurrencyId::get())
+		.mint(ALICE, NativeCurrencyId::get(), INITIAL_AMOUNT)
+		.mint(BOB, NativeCurrencyId::get(), INITIAL_AMOUNT)
+		.initialize_fee_locks(PERIOD_LENGTH, FEE_LOCK_AMOUNT, SWAP_VALUE_THRESHOLD)
+		.build()
+		.execute_with(|| {
+			assert_eq!(UnlockQueue::<Test>::get(0), None);
+			assert_eq!(UnlockQueue::<Test>::get(1), None);
+
+			<FeeLock as FeeLockTriggerTrait<_>>::process_fee_lock(&ALICE).unwrap();
+			assert_eq!(UnlockQueue::<Test>::get(0), Some(ALICE));
+			assert_eq!(UnlockQueue::<Test>::get(1), None);
+			assert_eq!(
+				Tokens::accounts(ALICE, NativeCurrencyId::get()),
+				ACCOUNT_WITH_LOCKED_TOKENS
+			);
+
+			<FeeLock as FeeLockTriggerTrait<_>>::process_fee_lock(&BOB).unwrap();
+			assert_eq!(UnlockQueue::<Test>::get(0), Some(ALICE));
+			assert_eq!(UnlockQueue::<Test>::get(1), Some(BOB));
+			assert_eq!(Tokens::accounts(BOB, NativeCurrencyId::get()), ACCOUNT_WITH_LOCKED_TOKENS);
+
+			fast_forward_blocks(PERIOD_LENGTH);
+			FeeLock::on_idle(System::block_number(), Weight::from_ref_time(u64::MAX));
+
+			assert_eq!(
+				Tokens::accounts(ALICE, NativeCurrencyId::get()),
+				ACCOUNT_WITHOUT_LOCKED_TOKENS
+			);
+			assert_eq!(
+				Tokens::accounts(BOB, NativeCurrencyId::get()),
+				ACCOUNT_WITHOUT_LOCKED_TOKENS
+			);
+
+			assert_eq!(UnlockQueue::<Test>::get(0), None);
+			assert_eq!(UnlockQueue::<Test>::get(1), None);
+		});
+}

runtime/mangata-kusama/src/weights/pallet_fee_lock.rs

@@ -19,16 +19,16 @@
 //! Autogenerated weights for pallet_fee_lock
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2023-02-13, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
-//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
+//! DATE: 2023-04-06, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("kusama"), DB CACHE: 1024
 
 // Executed Command:
 // target/release/mangata-node
 // benchmark
 // pallet
 // -l=info,xyk=error,collective-mangata=warn,bootstrap=warn
 // --chain
-// dev
+// kusama
 // --execution
 // wasm
 // --wasm-execution
@@ -65,34 +65,38 @@ pub struct ModuleWeight<T>(PhantomData<T>);
 impl<T: frame_system::Config> pallet_fee_lock::WeightInfo for ModuleWeight<T> {
 	// Storage: FeeLock FeeLockMetadata (r:1 w:1)
 	fn update_fee_lock_metadata() -> Weight {
-		(Weight::from_ref_time(33_569_000))
+		(Weight::from_ref_time(37_491_000))
 			.saturating_add(T::DbWeight::get().reads(1 as u64))
 			.saturating_add(T::DbWeight::get().writes(1 as u64))
 	}
 	// Storage: FeeLock AccountFeeLockData (r:1 w:1)
 	// Storage: FeeLock FeeLockMetadata (r:1 w:0)
 	// Storage: Tokens Accounts (r:1 w:1)
+	// Storage: FeeLock FeeLockMetadataQeueuePosition (r:1 w:1)
+	// Storage: FeeLock UnlockQueue (r:1 w:1)
 	fn unlock_fee() -> Weight {
-		(Weight::from_ref_time(48_631_000))
-			.saturating_add(T::DbWeight::get().reads(3 as u64))
-			.saturating_add(T::DbWeight::get().writes(2 as u64))
+		(Weight::from_ref_time(58_080_000))
+			.saturating_add(T::DbWeight::get().reads(5 as u64))
+			.saturating_add(T::DbWeight::get().writes(4 as u64))
 	}
 }
 
 // For backwards compatibility and tests
 impl WeightInfo for () {
 	// Storage: FeeLock FeeLockMetadata (r:1 w:1)
 	fn update_fee_lock_metadata() -> Weight {
-		(Weight::from_ref_time(33_569_000))
+		(Weight::from_ref_time(37_491_000))
 			.saturating_add(RocksDbWeight::get().reads(1 as u64))
 			.saturating_add(RocksDbWeight::get().writes(1 as u64))
 	}
 	// Storage: FeeLock AccountFeeLockData (r:1 w:1)
 	// Storage: FeeLock FeeLockMetadata (r:1 w:0)
 	// Storage: Tokens Accounts (r:1 w:1)
+	// Storage: FeeLock FeeLockMetadataQeueuePosition (r:1 w:1)
+	// Storage: FeeLock UnlockQueue (r:1 w:1)
 	fn unlock_fee() -> Weight {
-		(Weight::from_ref_time(48_631_000))
-			.saturating_add(RocksDbWeight::get().reads(3 as u64))
-			.saturating_add(RocksDbWeight::get().writes(2 as u64))
+		(Weight::from_ref_time(58_080_000))
+			.saturating_add(RocksDbWeight::get().reads(5 as u64))
+			.saturating_add(RocksDbWeight::get().writes(4 as u64))
 	}
 }

runtime/mangata-rococo/src/lib.rs

@@ -164,10 +164,10 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("mangata-parachain"),
 	impl_name: create_runtime_str!("mangata-parachain"),
 	authoring_version: 14,
-	spec_version: 2802,
+	spec_version: 002900,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
-	transaction_version: 2802,
+	transaction_version: 002900,
 	state_version: 0,
 };
 
@@ -178,10 +178,10 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	impl_name: create_runtime_str!("mangata-parachain"),
 
 	authoring_version: 14,
-	spec_version: 002900,
+	spec_version: 002901,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
-	transaction_version: 002900,
+	transaction_version: 002901,
 	state_version: 0,
 };
 

runtime/mangata-rococo/src/weights/pallet_fee_lock.rs

@@ -19,16 +19,16 @@
 //! Autogenerated weights for pallet_fee_lock
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2023-02-13, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
-//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
+//! DATE: 2023-04-06, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("kusama"), DB CACHE: 1024
 
 // Executed Command:
 // target/release/mangata-node
 // benchmark
 // pallet
 // -l=info,xyk=error,collective-mangata=warn,bootstrap=warn
 // --chain
-// dev
+// kusama
 // --execution
 // wasm
 // --wasm-execution
@@ -65,34 +65,38 @@ pub struct ModuleWeight<T>(PhantomData<T>);
 impl<T: frame_system::Config> pallet_fee_lock::WeightInfo for ModuleWeight<T> {
 	// Storage: FeeLock FeeLockMetadata (r:1 w:1)
 	fn update_fee_lock_metadata() -> Weight {
-		(Weight::from_ref_time(33_569_000))
+		(Weight::from_ref_time(37_491_000))
 			.saturating_add(T::DbWeight::get().reads(1 as u64))
 			.saturating_add(T::DbWeight::get().writes(1 as u64))
 	}
 	// Storage: FeeLock AccountFeeLockData (r:1 w:1)
 	// Storage: FeeLock FeeLockMetadata (r:1 w:0)
 	// Storage: Tokens Accounts (r:1 w:1)
+	// Storage: FeeLock FeeLockMetadataQeueuePosition (r:1 w:1)
+	// Storage: FeeLock UnlockQueue (r:1 w:1)
 	fn unlock_fee() -> Weight {
-		(Weight::from_ref_time(48_631_000))
-			.saturating_add(T::DbWeight::get().reads(3 as u64))
-			.saturating_add(T::DbWeight::get().writes(2 as u64))
+		(Weight::from_ref_time(58_080_000))
+			.saturating_add(T::DbWeight::get().reads(5 as u64))
+			.saturating_add(T::DbWeight::get().writes(4 as u64))
 	}
 }
 
 // For backwards compatibility and tests
 impl WeightInfo for () {
 	// Storage: FeeLock FeeLockMetadata (r:1 w:1)
 	fn update_fee_lock_metadata() -> Weight {
-		(Weight::from_ref_time(33_569_000))
+		(Weight::from_ref_time(37_491_000))
 			.saturating_add(RocksDbWeight::get().reads(1 as u64))
 			.saturating_add(RocksDbWeight::get().writes(1 as u64))
 	}
 	// Storage: FeeLock AccountFeeLockData (r:1 w:1)
 	// Storage: FeeLock FeeLockMetadata (r:1 w:0)
 	// Storage: Tokens Accounts (r:1 w:1)
+	// Storage: FeeLock FeeLockMetadataQeueuePosition (r:1 w:1)
+	// Storage: FeeLock UnlockQueue (r:1 w:1)
 	fn unlock_fee() -> Weight {
-		(Weight::from_ref_time(48_631_000))
-			.saturating_add(RocksDbWeight::get().reads(3 as u64))
-			.saturating_add(RocksDbWeight::get().writes(2 as u64))
+		(Weight::from_ref_time(58_080_000))
+			.saturating_add(RocksDbWeight::get().reads(5 as u64))
+			.saturating_add(RocksDbWeight::get().writes(4 as u64))
 	}
 }