Skip to content

Commit

Permalink
Fix lp-gateway routers' weight and fee calculations (#1537)
Browse files Browse the repository at this point in the history 
* Fix lp-gateway routers' weight and fee calculations

And adjust the integration tests to use reasonable amounts and verify
the fees being charged.

* lp-gateway-routers: Refactor XcmDomain

* Fix tests

* dev: Bump spec_version to 1022

* Drop commented old code
  • Loading branch information
@NunoAlexandre
NunoAlexandre authored Sep 11, 2023
1 parent 59a039a commit 90837d9
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 177 deletions.
50 changes: 15 additions & 35 deletions pallets/liquidity-pools-gateway/routers/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use frame_system::pallet_prelude::OriginFor;
use pallet_xcm_transactor::{Currency, CurrencyPayment, TransactWeights};
use scale_info::TypeInfo;
use sp_core::{bounded::BoundedVec, ConstU32, H160, H256, U256};
use sp_runtime::traits::{BlakeTwo256, EnsureMul, Hash};
use sp_runtime::traits::{BlakeTwo256, Hash};
use sp_std::{boxed::Box, marker::PhantomData, vec::Vec};
use xcm::{
latest::{MultiLocation, OriginKind},
Expand Down Expand Up @@ -237,30 +237,6 @@ where
let ethereum_xcm_call = get_encoded_ethereum_xcm_call::<T>(self.xcm_domain.clone(), msg)
.map_err(|_| DispatchError::Other("encoded ethereum xcm call retrieval"))?;

// Note: We are using moonbeams calculation for the ref time here and their
// estimate for the PoV.
//
// - Transact weight: gasLimit * 25000 as moonbeam is doing (Proof size
// limited fixed)
let transact_required_weight_at_most = Weight::from_ref_time(
self.xcm_domain
.max_gas_limit
.ensure_mul(GAS_TO_WEIGHT_MULTIPLIER)?,
)
.set_proof_size(DEFAULT_PROOF_SIZE.saturating_div(2));

// NOTE: We are choosing an overall weight here to have full control over
// the actual weight usage.
//
// - Transact weight: gasLimit * 25000 as moonbeam is doing (Proof size
// limited fixed)
// - Weight for XCM instructions: Fixed weight * 3 (as we have 3
// instructions)
let overall_weight = Weight::from_ref_time(
transact_required_weight_at_most.ref_time() + XCM_INSTRUCTION_WEIGHT * 3,
)
.set_proof_size(DEFAULT_PROOF_SIZE);

pallet_xcm_transactor::Pallet::<T>::transact_through_sovereign(
<T as frame_system::Config>::RuntimeOrigin::root(),
// The destination to which the message should be sent.
Expand All @@ -270,16 +246,14 @@ where
// The currency in which we want to pay fees.
CurrencyPayment {
currency: Currency::AsCurrencyId(self.xcm_domain.fee_currency.clone()),
fee_amount: Some(
self.xcm_domain.fee_per_second * Into::<u128>::into(overall_weight.ref_time()),
),
fee_amount: Some(self.xcm_domain.fee_amount),
},
// The call to be executed in the destination chain.
ethereum_xcm_call,
OriginKind::SovereignAccount,
TransactWeights {
transact_required_weight_at_most,
overall_weight: Some(overall_weight),
transact_required_weight_at_most: self.xcm_domain.transact_required_weight_at_most,
overall_weight: Some(self.xcm_domain.overall_weight),
},
)?;

Expand Down Expand Up @@ -336,16 +310,22 @@ pub struct XcmDomain<CurrencyId> {
/// The target contract address on a given domain.
pub contract_address: H160,

/// The max gas_limit we want to propose for a remote evm execution
/// The max gas limit for the execution of the EVM call
pub max_gas_limit: u64,

/// The max weight we want to pay for the execution of the Transact call in
/// the destination chain
pub transact_required_weight_at_most: Weight,

/// The overall max weight we want to pay for the whole XCM message (i.e,
/// all the instructions)
pub overall_weight: Weight,

/// The currency in which execution fees will be paid on
pub fee_currency: CurrencyId,

/// The fee per second that will be multiplied with
/// the overall weight of the call to define the fees on the
/// chain that will execute the call.
pub fee_per_second: u128,
/// The fee we use to buy execution for the execution of the Transact call
pub fee_amount: u128,
}

#[derive(Debug, Encode, Decode, Clone, PartialEq, Eq, TypeInfo, MaxEncodedLen)]
Expand Down
158 changes: 24 additions & 134 deletions pallets/liquidity-pools-gateway/routers/src/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ::xcm::{
lts::WeightLimit,
v2::OriginKind,
v3::{Instruction::*, MultiAsset, Weight},
v3::{Instruction::*, MultiAsset},
};
use cfg_mocks::MessageMock;
use cfg_primitives::CFG;
Expand Down Expand Up @@ -202,8 +202,10 @@ mod xcm_router {
ethereum_xcm_transact_call_index: bounded_vec![0],
contract_address: H160::from_slice([0; 20].as_slice()),
max_gas_limit: 10,
transact_required_weight_at_most: Default::default(),
overall_weight: Default::default(),
fee_currency: currency_id.clone(),
fee_per_second: 1u128,
fee_amount: 200000000000000000,
};

let sender: AccountId32 = [0; 32].into();
Expand Down Expand Up @@ -256,34 +258,21 @@ mod xcm_router {
let sent_messages = sent_xcm();
assert_eq!(sent_messages.len(), 1);

let transact_weight = Weight::from_parts(
test_data.xcm_domain.max_gas_limit * GAS_TO_WEIGHT_MULTIPLIER,
DEFAULT_PROOF_SIZE.saturating_div(2),
);

let overall_weight = Weight::from_parts(
transact_weight.ref_time() + XCM_INSTRUCTION_WEIGHT * 3,
DEFAULT_PROOF_SIZE,
);

let fees = Into::<u128>::into(overall_weight.ref_time())
* test_data.xcm_domain.fee_per_second;

let (_, xcm) = sent_messages.first().unwrap();
assert!(xcm.0.contains(&WithdrawAsset(
(MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
})
.into()
)));

assert!(xcm.0.contains(&BuyExecution {
fees: MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
},
weight_limit: WeightLimit::Limited(overall_weight),
weight_limit: WeightLimit::Limited(test_data.xcm_domain.overall_weight),
}));

let expected_call = get_encoded_ethereum_xcm_call::<Runtime>(
Expand All @@ -294,7 +283,7 @@ mod xcm_router {

assert!(xcm.0.contains(&Transact {
origin_kind: OriginKind::SovereignAccount,
require_weight_at_most: transact_weight,
require_weight_at_most: test_data.xcm_domain.transact_required_weight_at_most,
call: expected_call.into(),
}));
});
Expand All @@ -315,29 +304,6 @@ mod xcm_router {
assert_ok!(router.do_send(test_data.sender, test_data.msg));
});
}

#[test]
fn transactor_info_not_set() {
new_test_ext().execute_with(|| {
let test_data = get_test_data();

let router = XCMRouter::<Runtime> {
xcm_domain: test_data.xcm_domain.clone(),
_marker: Default::default(),
};

// Manually insert the fee per second in the `DestinationAssetFeePerSecond`
// storage.

pallet_xcm_transactor::DestinationAssetFeePerSecond::<Runtime>::insert(
test_data.dest,
test_data.xcm_domain.fee_per_second.clone(),
);

// We ensure we can send although no `TransactInfo is set`
assert_ok!(router.do_send(test_data.sender, test_data.msg),);
});
}
}
}

Expand Down Expand Up @@ -549,8 +515,10 @@ mod axelar_xcm {
ethereum_xcm_transact_call_index: bounded_vec![0],
contract_address: H160::from_slice([0; 20].as_slice()),
max_gas_limit: 10,
transact_required_weight_at_most: Default::default(),
overall_weight: Default::default(),
fee_currency: currency_id.clone(),
fee_per_second: 1u128,
fee_amount: 200000000000000000,
};
let axelar_target_chain = TEST_EVM_CHAIN.clone();
let axelar_target_contract = H160::from_low_u64_be(1);
Expand Down Expand Up @@ -621,34 +589,24 @@ mod axelar_xcm {
let sent_messages = sent_xcm();
assert_eq!(sent_messages.len(), 1);

let transact_weight = Weight::from_parts(
test_data.xcm_domain.max_gas_limit * GAS_TO_WEIGHT_MULTIPLIER,
DEFAULT_PROOF_SIZE.saturating_div(2),
);

let overall_weight = Weight::from_parts(
transact_weight.ref_time() + XCM_INSTRUCTION_WEIGHT * 3,
DEFAULT_PROOF_SIZE,
);

let fees = Into::<u128>::into(overall_weight.ref_time())
* test_data.xcm_domain.fee_per_second;
let sent_messages = sent_xcm();
assert_eq!(sent_messages.len(), 1);

let (_, xcm) = sent_messages.first().unwrap();
assert!(xcm.0.contains(&WithdrawAsset(
(MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
})
.into()
)));

assert!(xcm.0.contains(&BuyExecution {
fees: MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
},
weight_limit: WeightLimit::Limited(overall_weight),
weight_limit: WeightLimit::Limited(test_data.xcm_domain.overall_weight),
}));

let contract_call = get_axelar_encoded_msg(
Expand All @@ -657,7 +615,6 @@ mod axelar_xcm {
test_data.axelar_target_contract,
)
.unwrap();

let expected_call = get_encoded_ethereum_xcm_call::<Runtime>(
test_data.xcm_domain.clone(),
contract_call,
Expand All @@ -666,40 +623,11 @@ mod axelar_xcm {

assert!(xcm.0.contains(&Transact {
origin_kind: OriginKind::SovereignAccount,
require_weight_at_most: transact_weight,
require_weight_at_most: test_data.xcm_domain.transact_required_weight_at_most,
call: expected_call.into(),
}));
});
}

#[test]
fn transactor_info_not_set() {
new_test_ext().execute_with(|| {
let test_data = get_test_data();

let domain_router =
DomainRouter::<Runtime>::AxelarXCM(AxelarXCMRouter::<Runtime> {
router: XCMRouter {
xcm_domain: test_data.xcm_domain.clone(),
_marker: Default::default(),
},
axelar_target_chain: test_data.axelar_target_chain,
axelar_target_contract: test_data.axelar_target_contract,
_marker: Default::default(),
});

// Manually insert the fee per second in the `DestinationAssetFeePerSecond`
// storage.

pallet_xcm_transactor::DestinationAssetFeePerSecond::<Runtime>::insert(
test_data.dest,
test_data.xcm_domain.fee_per_second.clone(),
);

// We ensure we can send although no `TransactInfo is set`
assert_ok!(domain_router.send(test_data.sender, test_data.msg),);
});
}
}
}

Expand Down Expand Up @@ -730,8 +658,10 @@ mod ethereum_xcm {
ethereum_xcm_transact_call_index: bounded_vec![0],
contract_address: H160::from_slice([0; 20].as_slice()),
max_gas_limit: 10,
transact_required_weight_at_most: Default::default(),
overall_weight: Default::default(),
fee_currency: currency_id.clone(),
fee_per_second: 1u128,
fee_amount: 200000000000000000,
};
let axelar_target_chain = TEST_EVM_CHAIN.clone();
let axelar_target_contract = H160::from_low_u64_be(1);
Expand Down Expand Up @@ -798,34 +728,21 @@ mod ethereum_xcm {
let sent_messages = sent_xcm();
assert_eq!(sent_messages.len(), 1);

let transact_weight = Weight::from_parts(
test_data.xcm_domain.max_gas_limit * GAS_TO_WEIGHT_MULTIPLIER,
DEFAULT_PROOF_SIZE.saturating_div(2),
);

let overall_weight = Weight::from_parts(
transact_weight.ref_time() + XCM_INSTRUCTION_WEIGHT * 3,
DEFAULT_PROOF_SIZE,
);

let fees = Into::<u128>::into(overall_weight.ref_time())
* test_data.xcm_domain.fee_per_second;

let (_, xcm) = sent_messages.first().unwrap();
assert!(xcm.0.contains(&WithdrawAsset(
(MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
})
.into()
)));

assert!(xcm.0.contains(&BuyExecution {
fees: MultiAsset {
id: ::xcm::v3::AssetId::Concrete(MultiLocation::here()),
fun: ::xcm::v3::Fungibility::Fungible(fees),
fun: ::xcm::v3::Fungibility::Fungible(test_data.xcm_domain.fee_amount),
},
weight_limit: WeightLimit::Limited(overall_weight),
weight_limit: WeightLimit::Limited(test_data.xcm_domain.overall_weight),
}));

let contract_call = get_encoded_contract_call(test_data.msg.serialize()).unwrap();
Expand All @@ -837,37 +754,10 @@ mod ethereum_xcm {

assert!(xcm.0.contains(&Transact {
origin_kind: OriginKind::SovereignAccount,
require_weight_at_most: transact_weight,
require_weight_at_most: test_data.xcm_domain.transact_required_weight_at_most,
call: expected_call.into(),
}));
});
}

#[test]
fn transactor_info_not_set() {
new_test_ext().execute_with(|| {
let test_data = get_test_data();

let domain_router =
DomainRouter::<Runtime>::EthereumXCM(EthereumXCMRouter::<Runtime> {
router: XCMRouter {
xcm_domain: test_data.xcm_domain.clone(),
_marker: Default::default(),
},
_marker: Default::default(),
});

// Manually insert the fee per second in the `DestinationAssetFeePerSecond`
// storage.

pallet_xcm_transactor::DestinationAssetFeePerSecond::<Runtime>::insert(
test_data.dest,
test_data.xcm_domain.fee_per_second.clone(),
);

// We ensure we can send although no `TransactInfo is set`
assert_ok!(domain_router.send(test_data.sender, test_data.msg),);
});
}
}
}
2 changes: 1 addition & 1 deletion runtime/development/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("centrifuge-devel"),
impl_name: create_runtime_str!("centrifuge-devel"),
authoring_version: 1,
spec_version: 1021,
spec_version: 1022,
impl_version: 1,
#[cfg(not(feature = "disable-runtime-api"))]
apis: RUNTIME_API_VERSIONS,
Expand Down
4 changes: 3 additions & 1 deletion runtime/integration-tests/src/liquidity_pools/gateway.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ async fn set_router() {
ethereum_xcm_transact_call_index: bounded_vec![0],
contract_address: H160::from_low_u64_be(3),
max_gas_limit: 10,
transact_required_weight_at_most: Default::default(),
overall_weight: Default::default(),
fee_currency: currency_id,
fee_per_second: 1u128,
fee_amount: 0,
};

let ethereum_xcm_router = EthereumXCMRouter::<Runtime> {
Expand Down
Loading

0 comments on commit 90837d9

Please sign in to comment.