Skip to content

Commit

Permalink
Fine tune max extrinsic decode depth limit for call decompression (#248)
Browse files Browse the repository at this point in the history
  • Loading branch information
Wizdave97 authored Jun 27, 2024
1 parent e20c893 commit 49e2a72
Show file tree
Hide file tree
Showing 15 changed files with 395 additions and 50 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions evm/test/TeleportSwapTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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: {
Expand All @@ -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,
});
}
11 changes: 8 additions & 3 deletions modules/ismp/pallets/call-decompressor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))?;
Expand Down Expand Up @@ -174,7 +179,7 @@ pub mod pallet {
priority: 100,
requires: vec![],
provides,
longevity: TransactionLongevity::MAX,
longevity: 25,
propagate: true,
})
}
Expand Down Expand Up @@ -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)?;
Expand Down
6 changes: 3 additions & 3 deletions modules/ismp/pallets/fishermen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand All @@ -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)?;

Expand All @@ -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,
Expand Down
52 changes: 36 additions & 16 deletions modules/ismp/pallets/relayer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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(_) |
Expand All @@ -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 {
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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())
}
2 changes: 0 additions & 2 deletions modules/ismp/pallets/relayer/src/withdrawal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions modules/ismp/pallets/testsuite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
7 changes: 7 additions & 0 deletions modules/ismp/pallets/testsuite/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ frame_support::construct_runtime!(
Assets: pallet_assets,
Gateway: pallet_asset_gateway,
TokenGovernor: pallet_token_governor,
Sudo: pallet_sudo,
}
);

Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 49e2a72

Please sign in to comment.