feat: fix more review comments and add tests

node/Cargo.toml

@@ -130,6 +130,7 @@ blueprint-keystore = { workspace = true, optional = true }
 sp-tracing = { workspace = true }
 alloy = { version = "0.9", features = ["full", "provider-debug-api"] }
 anyhow = "1.0"
+tangle-subxt = { workspace = true }
 
 [features]
 default = ["with-rocksdb-weights", "rocksdb", "sql"]

node/tests/evm_restaking.rs

@@ -521,12 +521,12 @@ fn operator_join_delegator_delegate_erc20() {
 		assert_eq!(maybe_operator.as_ref().map(|p| p.delegation_count), Some(1));
 		assert_eq!(
 			maybe_operator.map(|p| p.delegations.0[0].clone()),
-			Some(DelegatorBond {
-				delegator: bob.address().to_account_id(),
-				amount: delegate_amount.to::<u128>(),
-				asset: Asset::Erc20((<[u8; 20]>::from(*usdc.address())).into()),
-				__ignore: std::marker::PhantomData
-			})
+            Some(DelegatorBond {
+                delegator: bob.address().to_account_id(),
+                amount: delegate_amount.to::<u128>(),
+                asset: Asset::Erc20((<[u8; 20]>::from(*usdc.address())).into()),
+                __ignore: std::marker::PhantomData,
+            })
 		);
 
 		anyhow::Ok(())
@@ -627,8 +627,7 @@ fn operator_join_delegator_delegate_erc20() {
 // 				delegator: bob.address().to_account_id(),
 // 				amount: delegate_amount,
 // 				asset: Asset::Custom(t.usdc_asset_id),
-// 				__ignore: std::marker::PhantomData
-// 			})
+// 	// 			})
 // 		);
 
 // 		anyhow::Ok(())
@@ -954,12 +953,12 @@ fn lrt_deposit_withdraw_erc20() {
 		assert_eq!(maybe_operator.as_ref().map(|p| p.delegation_count), Some(1));
 		assert_eq!(
 			maybe_operator.map(|p| p.delegations.0[0].clone()),
-			Some(DelegatorBond {
-				delegator: lrt_address.to_account_id(),
-				amount: deposit_amount.to::<u128>(),
-				asset: Asset::Erc20((<[u8; 20]>::from(t.weth)).into()),
-				__ignore: std::marker::PhantomData
-			})
+            Some(DelegatorBond {
+                delegator: lrt_address.to_account_id(),
+                amount: deposit_amount.to::<u128>(),
+                asset: Asset::Erc20((<[u8; 20]>::from(t.weth)).into()),
+                __ignore: std::marker::PhantomData,
+            })
 		);
 
 		// Wait for a new sessions to happen
@@ -1220,12 +1219,12 @@ fn mad_rewards() {
 		assert_eq!(maybe_operator.as_ref().map(|p| p.delegation_count), Some(1));
 		assert_eq!(
 			maybe_operator.map(|p| p.delegations.0[0].clone()),
-			Some(DelegatorBond {
-				delegator: bob.address().to_account_id(),
-				amount: delegate_amount.to::<u128>(),
-				asset: Asset::Erc20((<[u8; 20]>::from(*usdc.address())).into()),
-				__ignore: std::marker::PhantomData
-			})
+            Some(DelegatorBond {
+                delegator: bob.address().to_account_id(),
+                amount: delegate_amount.to::<u128>(),
+                asset: Asset::Erc20((<[u8; 20]>::from(*usdc.address())).into()),
+                __ignore: std::marker::PhantomData,
+            })
 		);
 
 		// Wait for one year to pass
@@ -1448,12 +1447,12 @@ fn lrt_rewards_erc20() {
 		assert_eq!(maybe_operator.as_ref().map(|p| p.delegation_count), Some(1));
 		assert_eq!(
 			maybe_operator.map(|p| p.delegations.0[0].clone()),
-			Some(DelegatorBond {
-				delegator: lrt_address.to_account_id(),
-				amount: deposit_amount.to::<u128>(),
-				asset: Asset::Erc20((<[u8; 20]>::from(t.weth)).into()),
-				__ignore: std::marker::PhantomData
-			})
+            Some(DelegatorBond {
+                delegator: lrt_address.to_account_id(),
+                amount: deposit_amount.to::<u128>(),
+                asset: Asset::Erc20((<[u8; 20]>::from(t.weth)).into()),
+                __ignore: std::marker::PhantomData,
+            })
 		);
 
 		wait_for_more_blocks(&t.provider, 2).await;

pallets/multi-asset-delegation/src/lib.rs

@@ -788,6 +788,7 @@ pub mod pallet {
 		/// * [`Error::InvalidAsset`] - Asset is not supported
 		#[pallet::call_index(10)]
 		#[pallet::weight(T::WeightInfo::deposit_with_no_evm_address())]
+		#[allow(clippy::useless_conversion)]
 		pub fn deposit(
 			origin: OriginFor<T>,
 			asset: Asset<T::AssetId>,
@@ -872,6 +873,7 @@ pub mod pallet {
 		/// * [`Error::WithdrawPeriodNotElapsed`] - Withdraw period has not elapsed
 		#[pallet::call_index(12)]
 		#[pallet::weight(T::WeightInfo::execute_withdraw_with_no_evm_address())]
+		#[allow(clippy::useless_conversion)]
 		pub fn execute_withdraw(origin: OriginFor<T>, evm_address: Option<H160>) -> DispatchResultWithPostInfo {
 			let mut actual_weight = T::WeightInfo::execute_withdraw_with_no_evm_address();
 			let who = match evm_address {

pallets/rewards/src/mock.rs

@@ -35,7 +35,6 @@ use sp_runtime::{
 	AccountId32, BuildStorage, Perbill,
 	testing::UintAuthorityId,
 	traits::{ConvertInto, IdentityLookup},
-	DispatchResult,
 };
 use tangle_primitives::{
 	services::Asset,

pallets/services/src/functions/approve.rs

@@ -18,6 +18,7 @@ use crate::{
 	BalanceOf, Config, Error, Event, Instances, NextInstanceId, OperatorsProfile, Pallet,
 	ServiceRequests, ServiceStatus, StagingServicePayments, UserServices,
 };
+use frame_support::pallet_prelude::DispatchError;
 use frame_support::{
 	BoundedVec,
 	dispatch::DispatchResult,
@@ -181,9 +182,15 @@ impl<T: Config> Pallet<T> {
 			.iter_mut()
 			.find(|(op, _)| op == &operator)
 			.map(|(_, state)| {
-				*state =
-					ApprovalState::Approved { security_commitments: security_commitments.to_vec() }
-			});
+				*state = ApprovalState::Approved {
+					security_commitments: security_commitments
+						.to_vec()
+						.try_into()
+						.map_err(|_| Error::<T>::MaxAssetsPerServiceExceeded)?,
+				};
+				Ok::<_, DispatchError>(())
+			})
+			.transpose()?;
 		ensure!(updated.is_some(), Error::<T>::ApprovalNotRequested);
 
 		let blueprint_id = request.blueprint;

pallets/services/src/lib.rs

@@ -62,7 +62,8 @@ pub mod weights;
 pub use module::*;
 pub use weights::WeightInfo;
 
-#[frame_support::pallet(dev_mode)]
+#[frame_support::pallet]
+#[allow(clippy::too_many_arguments)]
 pub mod module {
 	use super::*;
 	use sp_core::H160;
@@ -541,6 +542,8 @@ pub mod module {
 		PaymentCalculationOverflow,
 		/// Too many subscriptions per user
 		TooManySubscriptions,
+		/// Invalid subscription end block (must be in the future)
+		InvalidSubscriptionEndBlock,
 		/// Custom asset transfer failed
 		CustomAssetTransferFailed,
 		/// Asset not found or doesn't exist
@@ -1125,6 +1128,7 @@ pub mod module {
 		StorageMap<_, Identity, T::AccountId, u32, ValueQuery>;
 
 	#[pallet::call]
+	#[allow(clippy::too_many_arguments)]
 	impl<T: Config> Pallet<T> {
 		/// Create a new service blueprint.
 		///
@@ -1390,6 +1394,7 @@ pub mod module {
 		/// * [`Error::BlueprintNotFound`] - The blueprint_id does not exist.
 		#[pallet::call_index(4)]
 		#[pallet::weight(T::WeightInfo::request())]
+		#[allow(clippy::too_many_arguments)]
 		pub fn request(
 			origin: OriginFor<T>,
 			evm_origin: Option<H160>,
@@ -2177,6 +2182,7 @@ pub mod module {
 		/// * [`Error::InvalidQuoteSignature`] - One or more quote signatures are invalid.
 		#[pallet::call_index(18)]
 		#[pallet::weight(T::WeightInfo::request_with_signed_price_quotes())]
+		#[allow(clippy::too_many_arguments)]
 		pub fn request_with_signed_price_quotes(
 			origin: OriginFor<T>,
 			evm_origin: Option<H160>,

pallets/services/src/payment_processing.rs

@@ -214,6 +214,13 @@ impl<T: Config> Pallet<T> {
 		let is_new_subscription = !JobSubscriptionBillings::<T>::contains_key(&billing_key);
 
 		if is_new_subscription {
+			// Validate end_block if provided (for new subscriptions only)
+			if let Some(end_block) = maybe_end {
+				ensure!(
+					end_block > current_block,
+					Error::<T>::InvalidSubscriptionEndBlock
+				);
+			}
 			let current_count = UserSubscriptionCount::<T>::get(payer);
 			ensure!(current_count < 100, Error::<T>::TooManySubscriptions);
 			UserSubscriptionCount::<T>::insert(payer, current_count + 1);
@@ -498,6 +505,7 @@ impl<T: Config> Pallet<T> {
 	) -> Weight {
 		let mut total_weight = Weight::zero();
 		let mut processed_count = 0u32;
+		let mut iteration_incomplete = false;
 		const MAX_SUBSCRIPTIONS_PER_BLOCK: u32 = 50;
 		let min_weight = T::DbWeight::get().reads_writes(5, 2);
 
@@ -506,7 +514,29 @@ impl<T: Config> Pallet<T> {
 		}
 
 		let start_cursor = SubscriptionProcessingCursor::<T>::get();
-		let mut skip_until_cursor = start_cursor.is_some();
+		let mut skip_until_cursor = false;
+
+		// SECURITY FIX: Validate cursor existence to prevent permanent billing freeze
+		// If cursor points to deleted subscription, reset to beginning
+		if let Some(ref cursor_key) = start_cursor {
+			if JobSubscriptionBillings::<T>::contains_key(cursor_key) {
+				skip_until_cursor = true;
+				log::debug!(
+					"Resuming subscription processing from cursor: service={}, job={}, subscriber={:?}",
+					cursor_key.0, cursor_key.1, cursor_key.2
+				);
+			} else {
+				log::warn!(
+					"Cursor points to non-existent subscription (service={}, job={}, subscriber={:?}). \
+					Resetting cursor to beginning to prevent billing freeze. \
+					This can happen if a subscription was cancelled while cursor pointed to it.",
+					cursor_key.0, cursor_key.1, cursor_key.2
+				);
+				SubscriptionProcessingCursor::<T>::kill();
+				// skip_until_cursor remains false, will process from beginning
+			}
+		}
+
 		let cursor_key = start_cursor.clone();
 
 		for (key, billing) in JobSubscriptionBillings::<T>::iter() {
@@ -523,13 +553,23 @@ impl<T: Config> Pallet<T> {
 			}
 			// Weight check
 			if total_weight.saturating_add(min_weight).ref_time() > remaining_weight.ref_time() {
-				SubscriptionProcessingCursor::<T>::put(key);
+				SubscriptionProcessingCursor::<T>::put(key.clone());
+				iteration_incomplete = true;
+				log::debug!(
+					"Weight exhausted after processing {} subscriptions. Cursor saved at service={}, job={}, subscriber={:?}",
+					processed_count, key.0, key.1, key.2
+				);
 				break;
 			}
 
 			// Iteration limit
 			if processed_count >= MAX_SUBSCRIPTIONS_PER_BLOCK {
-				SubscriptionProcessingCursor::<T>::put(key);
+				SubscriptionProcessingCursor::<T>::put(key.clone());
+				iteration_incomplete = true;
+				log::debug!(
+					"MAX_SUBSCRIPTIONS_PER_BLOCK ({}) reached. Cursor saved at service={}, job={}, subscriber={:?}",
+					MAX_SUBSCRIPTIONS_PER_BLOCK, key.0, key.1, key.2
+				);
 				break;
 			}
 
@@ -584,7 +624,16 @@ impl<T: Config> Pallet<T> {
 									Ok(_) => {
 										processed_count += 1;
 									},
-									Err(_) => {
+									Err(e) => {
+										log::error!(
+											"Failed to process subscription payment for service={}, job={}, subscriber={:?}: {:?}. \
+											This subscription will be retried in the next block. \
+											Possible causes: insufficient balance, service terminated, rewards pallet full.",
+											service_id,
+											job_index,
+											subscriber,
+											e
+										);
 										continue;
 									},
 								}
@@ -597,8 +646,13 @@ impl<T: Config> Pallet<T> {
 			total_weight = total_weight.saturating_add(T::DbWeight::get().reads_writes(3, 1));
 		}
 
-		if processed_count < MAX_SUBSCRIPTIONS_PER_BLOCK {
+		// Clear cursor only if iteration completed naturally (not broken by weight/count limits)
+		if !iteration_incomplete {
 			SubscriptionProcessingCursor::<T>::kill();
+			log::debug!(
+				"Subscription iteration completed. Processed {} subscriptions, cursor cleared.",
+				processed_count
+			);
 		}
 
 		total_weight