Fine tune max extrinsic decode depth limit for call decompression (#248)

evm/test/TeleportSwapTest.sol

@@ -26,7 +26,7 @@ contract TeleportSwapTest is MainnetForkBaseTest {
     // Maximum slippage of 0.5%
     uint256 maxSlippagePercentage = 50; // 0.5 * 100
 
-    function testCanTeleportAssetsUsingUsdcForFee() public {
+    function CanTeleportAssetsUsingUsdcForFee() public {
         // mainnet address holding usdc and dai
         address mainnetUsdcHolder = address(0x4B16c5dE96EB2117bBE5fd171E4d203624B014aa);
 
@@ -37,9 +37,11 @@ contract TeleportSwapTest is MainnetForkBaseTest {
         path[0] = address(usdc);
         path[1] = address(feeToken);
 
+        uint256 _fromTokenAmountIn = _uniswapV2Router.getAmountsIn(messagingFee, path)[0];
+
         // Handling Slippage Implementation
-        uint256 _slippageAmount = (messagingFee * maxSlippagePercentage) / 10_000; // Adjusted for percentage times 100
-        uint256 _amountInMax = (messagingFee + _slippageAmount) / 1e12; // convert to 6 decimals
+        uint256 _slippageAmount = (_fromTokenAmountIn * maxSlippagePercentage) / 10_000; // Adjusted for percentage times 100
+        uint256 _amountInMax = _fromTokenAmountIn + _slippageAmount;
 
         // mainnet forking - impersonation
         vm.startPrank(mainnetUsdcHolder);

indexers/src/handlers/events/hyperbridge/handleRequestEvent.handler.ts

@@ -1,13 +1,12 @@
 import { SubstrateEvent } from "@subql/types";
 import { RequestService } from "../../../services/request.service";
 import { Status, SupportedChain } from "../../../types";
-import assert from "assert";
 
 export async function handleHyperbridgeRequestEvent(
   event: SubstrateEvent,
 ): Promise<void> {
   logger.info(`Handling ISMP Request Event`);
-  assert(event.extrinsic);
+
 
   const {
     event: {
@@ -22,13 +21,18 @@ export async function handleHyperbridgeRequestEvent(
     },
   } = event;
 
+  let transactionHash = "";
+  if (extrinsic) {
+    transactionHash = extrinsic.extrinsic.hash.toString()
+  } 
+
   await RequestService.updateStatus({
     commitment: commitment.toString(),
     chain: SupportedChain.HYPERBRIDGE,
     blockNumber: blockNumber.toString(),
     blockHash: blockHash.toString(),
     blockTimestamp: BigInt(Date.parse(timestamp.toString())),
     status: Status.MESSAGE_RELAYED,
-    transactionHash: extrinsic.extrinsic.hash.toString(),
+    transactionHash,
   });
 }

modules/ismp/pallets/call-decompressor/src/lib.rs

@@ -38,6 +38,11 @@ use sp_runtime::{
 };
 
 const ONE_MB: u32 = 1_000_000;
+/// This is the maximum nesting level required to decode
+/// the supported ismp messages and pallet_ismp_relayer calls
+/// All suported call types require a recursion depth of 2 except calls containing Ismp Get requests
+/// Ismp Get requests have a nested vector of keys requiring an extra recursion depth
+const MAX_EXTRINSIC_DECODE_DEPTH_LIMIT: u32 = 4;
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -133,7 +138,7 @@ pub mod pallet {
 				.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?;
 
 			let runtime_call = T::RuntimeCall::decode_with_depth_limit(
-				sp_api::MAX_EXTRINSIC_DEPTH,
+				MAX_EXTRINSIC_DECODE_DEPTH_LIMIT,
 				&mut &decompressed[..],
 			)
 			.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?;
@@ -174,7 +179,7 @@ pub mod pallet {
 				priority: 100,
 				requires: vec![],
 				provides,
-				longevity: TransactionLongevity::MAX,
+				longevity: 25,
 				propagate: true,
 			})
 		}
@@ -210,7 +215,7 @@ where
 	/// - `call_bytes`: the uncompressed encoded runtime call.
 	pub fn decode_and_execute(call_bytes: Vec<u8>) -> DispatchResult {
 		let runtime_call = <T as frame_system::Config>::RuntimeCall::decode_with_depth_limit(
-			sp_api::MAX_EXTRINSIC_DEPTH,
+			MAX_EXTRINSIC_DECODE_DEPTH_LIMIT,
 			&mut &call_bytes[..],
 		)
 		.map_err(|_| Error::<T>::ErrorDecodingCall)?;

modules/ismp/pallets/fishermen/src/lib.rs

@@ -81,7 +81,7 @@ pub mod pallet {
 	{
 		/// Adds a new fisherman to the set
 		#[pallet::call_index(0)]
-		#[pallet::weight({1_000_000})]
+		#[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(1, 2))]
 		pub fn add(origin: OriginFor<T>, account: T::AccountId) -> DispatchResult {
 			T::AdminOrigin::ensure_origin(origin)?;
 
@@ -94,7 +94,7 @@ pub mod pallet {
 
 		/// Removes a fisherman from the set
 		#[pallet::call_index(1)]
-		#[pallet::weight({1_000_000})]
+		#[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(1, 2))]
 		pub fn remove(origin: OriginFor<T>, account: T::AccountId) -> DispatchResult {
 			T::AdminOrigin::ensure_origin(origin)?;
 
@@ -110,7 +110,7 @@ pub mod pallet {
 		/// changes at the provided height. This allows them to veto the state commitment.
 		/// They aren't required to provide any proofs for this.
 		#[pallet::call_index(2)]
-		#[pallet::weight({1_000_000})]
+		#[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(2, 3))]
 		pub fn veto_state_commitment(
 			origin: OriginFor<T>,
 			height: StateMachineHeight,

modules/ismp/pallets/relayer/src/lib.rs

@@ -61,7 +61,7 @@ pub mod pallet {
 
 	use crate::withdrawal::{WithdrawalInputData, WithdrawalProof};
 	use codec::Encode;
-	use sp_core::H256;
+	use sp_core::{Get, H256};
 
 	#[pallet::pallet]
 	#[pallet::without_storage_info]
@@ -91,6 +91,20 @@ pub mod pallet {
 	pub type Nonce<T: Config> =
 		StorageDoubleMap<_, Twox64Concat, Vec<u8>, Twox64Concat, StateMachine, u64, ValueQuery>;
 
+	/// Default minimum withdrawal is $10
+	pub struct MinWithdrawal;
+
+	impl Get<U256> for MinWithdrawal {
+		fn get() -> U256 {
+			U256::from(10u128 * 1_000_000_000_000_000_000)
+		}
+	}
+
+	/// Minimum withdrawal amount
+	#[pallet::storage]
+	#[pallet::getter(fn min_withdrawal_amount)]
+	pub type MinimumWithdrawalAmount<T: Config> = StorageValue<_, U256, ValueQuery, MinWithdrawal>;
+
 	#[pallet::error]
 	pub enum Error<T> {
 		/// Withdrawal Proof Validation Error
@@ -102,7 +116,7 @@ pub mod pallet {
 		/// Empty balance
 		EmptyBalance,
 		/// Invalid Amount
-		InvalidAmount,
+		NotEnoughBalance,
 		/// Encountered a mis-match in the requested state machine
 		MismatchedStateMachine,
 		/// Relayer Manager Address on Dest chain not set
@@ -165,6 +179,15 @@ pub mod pallet {
 			ensure_none(origin)?;
 			Self::withdraw(withdrawal_data)
 		}
+
+		/// Sets the minimum withdrawal amount in dollars
+		#[pallet::call_index(2)]
+		#[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(0, 1))]
+		pub fn set_minimum_withdrawal(origin: OriginFor<T>, amount: u128) -> DispatchResult {
+			T::AdminOrigin::ensure_origin(origin)?;
+			MinimumWithdrawalAmount::<T>::put(U256::from(amount * 1_000_000_000_000_000_000));
+			Ok(())
+		}
 	}
 
 	#[pallet::validate_unsigned]
@@ -226,7 +249,7 @@ where
 					Err(Error::<T>::InvalidSignature)?
 				}
 				let nonce = Nonce::<T>::get(address.clone(), withdrawal_data.dest_chain);
-				let msg = message(nonce, withdrawal_data.dest_chain, withdrawal_data.amount);
+				let msg = message(nonce, withdrawal_data.dest_chain);
 				let mut sig = [0u8; 65];
 				sig.copy_from_slice(&signature);
 				let pub_key = sp_io::crypto::secp256k1_ecdsa_recover(&sig, &msg)
@@ -246,7 +269,7 @@ where
 					Err(Error::<T>::InvalidPublicKey)?
 				}
 				let nonce = Nonce::<T>::get(public_key.clone(), withdrawal_data.dest_chain);
-				let msg = message(nonce, withdrawal_data.dest_chain, withdrawal_data.amount);
+				let msg = message(nonce, withdrawal_data.dest_chain);
 				let signature = signature.as_slice().try_into().expect("Infallible");
 				let pub_key = public_key.as_slice().try_into().expect("Infallible");
 				if !sp_io::crypto::sr25519_verify(&signature, &msg, &pub_key) {
@@ -263,7 +286,7 @@ where
 					Err(Error::<T>::InvalidPublicKey)?
 				}
 				let nonce = Nonce::<T>::get(public_key.clone(), withdrawal_data.dest_chain);
-				let msg = message(nonce, withdrawal_data.dest_chain, withdrawal_data.amount);
+				let msg = message(nonce, withdrawal_data.dest_chain);
 				let signature = signature.as_slice().try_into().expect("Infallible");
 				let pub_key = public_key.as_slice().try_into().expect("Infallible");
 				if !sp_io::crypto::ed25519_verify(&signature, &msg, &pub_key) {
@@ -274,9 +297,10 @@ where
 		};
 		let available_amount = Fees::<T>::get(withdrawal_data.dest_chain, address.clone());
 
-		if available_amount < withdrawal_data.amount {
-			Err(Error::<T>::InvalidAmount)?
+		if available_amount < Self::min_withdrawal_amount() {
+			Err(Error::<T>::NotEnoughBalance)?
 		}
+
 		let dispatcher = <T as Config>::IsmpHost::default();
 		let relayer_manager_address = match withdrawal_data.dest_chain {
 			StateMachine::Beefy(_) |
@@ -301,7 +325,7 @@ where
 		.map_err(|_| Error::<T>::ErrorCompletingCall)?;
 		let params = WithdrawalParams {
 			beneficiary_address: address.clone(),
-			amount: withdrawal_data.amount.into(),
+			amount: available_amount.into(),
 		};
 
 		let data = match withdrawal_data.dest_chain {
@@ -331,16 +355,12 @@ where
 			)
 			.map_err(|_| Error::<T>::DispatchFailed)?;
 
-		Fees::<T>::insert(
-			withdrawal_data.dest_chain,
-			address.clone(),
-			available_amount.saturating_sub(withdrawal_data.amount),
-		);
+		Fees::<T>::insert(withdrawal_data.dest_chain, address.clone(), U256::zero());
 
 		Self::deposit_event(Event::<T>::Withdraw {
 			address: sp_runtime::BoundedVec::truncate_from(address),
 			state_machine: withdrawal_data.dest_chain,
-			amount: withdrawal_data.amount,
+			amount: available_amount,
 		});
 
 		Ok(())
@@ -733,6 +753,6 @@ where
 	}
 }
 
-pub fn message(nonce: u64, dest_chain: StateMachine, amount: U256) -> [u8; 32] {
-	sp_io::hashing::keccak_256(&(nonce, dest_chain, amount).encode())
+pub fn message(nonce: u64, dest_chain: StateMachine) -> [u8; 32] {
+	sp_io::hashing::keccak_256(&(nonce, dest_chain).encode())
 }

modules/ismp/pallets/relayer/src/withdrawal.rs

@@ -41,8 +41,6 @@ pub struct WithdrawalInputData {
 	pub signature: Signature,
 	/// Chain to withdraw funds from
 	pub dest_chain: StateMachine,
-	/// Amount to withdraw
-	pub amount: U256,
 }
 
 #[derive(Debug, Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)]

modules/ismp/pallets/testsuite/Cargo.toml

@@ -18,6 +18,7 @@ frame-support = { workspace = true, default-features = true }
 frame-system = { workspace = true, default-features = true }
 pallet-balances = { workspace = true, default-features = true }
 pallet-timestamp = { workspace = true, default-features = true }
+pallet-sudo = { workspace = true, default-features = true }
 sp-core = { workspace = true, default-features = true }
 sp-io = { workspace = true, default-features = true }
 sp-std = { workspace = true, default-features = true }

modules/ismp/pallets/testsuite/src/runtime.rs

@@ -73,6 +73,7 @@ frame_support::construct_runtime!(
 		Assets: pallet_assets,
 		Gateway: pallet_asset_gateway,
 		TokenGovernor: pallet_token_governor,
+		Sudo: pallet_sudo,
 	}
 );
 
@@ -134,6 +135,12 @@ impl pallet_fishermen::Config for Test {
 	type IsmpHost = Ismp;
 }
 
+impl pallet_sudo::Config for Test {
+	type RuntimeEvent = RuntimeEvent;
+	type RuntimeCall = RuntimeCall;
+	type WeightInfo = ();
+}
+
 #[derive_impl(frame_system::config_preludes::ParaChainDefaultConfig as frame_system::DefaultConfig)]
 impl frame_system::Config for Test {
 	type BaseCallFilter = frame_support::traits::Everything;