[cumulus] Improved check for sane bridge fees calculations (#3175)

## TODO
- [x] change constants when CI fails (should fail :) )

## Result

On the AssetHubRococo: 1701175800126 -> 1700929825257 = 0.15 %
decreased.
```
# Before ( [xcm] Fix `SovereignPaidRemoteExporter` and `DepositAsset` handling (#3157))
Feb 02 12:59:05.520 ERROR bridges::estimate: `bridging::XcmBridgeHubRouterBaseFee` actual value: 1701175800126 for runtime: statemine-1006000 (statemine-0.tx14.au1)    

# After
Feb 02 13:02:40.647 ERROR bridges::estimate: `bridging::XcmBridgeHubRouterBaseFee` actual value: 1700929825257 for runtime: statemine-1006000 (statemine-0.tx14.au1)    

```

On the AssetHubWestend: 2116038876326 -> 1641718372993 = 22.4 %
decreased.
```
# Before ( [xcm] Fix `SovereignPaidRemoteExporter` and `DepositAsset` handling (#3157))
Feb 02 12:56:00.880 ERROR bridges::estimate: `bridging::XcmBridgeHubRouterBaseFee` actual value: 2116038876326 for runtime: westmint-1006000 (westmint-0.tx14.au1)    


# After
Feb 02 13:04:42.515 ERROR bridges::estimate: `bridging::XcmBridgeHubRouterBaseFee` actual value: 1641718372993 for runtime: westmint-1006000 (westmint-0.tx14.au1)    
```
This commit is contained in:
Branislav Kontur
2024-02-02 13:56:32 +01:00
committed by GitHub
parent 5ba8921787
commit 74b597fcaf
10 changed files with 235 additions and 169 deletions
@@ -99,7 +99,7 @@ frame_support::parameter_types! {
/// The XCM fee that is paid for executing XCM program (with `ExportMessage` instruction) at the Rococo
/// BridgeHub.
/// (initially was calculated by test `BridgeHubRococo::can_calculate_weight_for_paid_export_message_with_reserve_transfer` + `33%`)
pub const BridgeHubRococoBaseXcmFeeInRocs: u128 = 1_640_102_205;
pub const BridgeHubRococoBaseXcmFeeInRocs: u128 = 59_034_266;
/// Transaction fee that is paid at the Rococo BridgeHub for delivering single inbound message.
/// (initially was calculated by test `BridgeHubRococo::can_calculate_fee_for_complex_message_delivery_transaction` + `33%`)
@@ -107,5 +107,5 @@ frame_support::parameter_types! {
/// Transaction fee that is paid at the Rococo BridgeHub for delivering single outbound message confirmation.
/// (initially was calculated by test `BridgeHubRococo::can_calculate_fee_for_complex_message_confirmation_transaction` + `33%`)
pub const BridgeHubRococoBaseConfirmationFeeInRocs: u128 = 4_045_736_577;
pub const BridgeHubRococoBaseConfirmationFeeInRocs: u128 = 5_380_829_647;
}
@@ -90,7 +90,7 @@ frame_support::parameter_types! {
/// The XCM fee that is paid for executing XCM program (with `ExportMessage` instruction) at the Westend
/// BridgeHub.
/// (initially was calculated by test `BridgeHubWestend::can_calculate_weight_for_paid_export_message_with_reserve_transfer` + `33%`)
pub const BridgeHubWestendBaseXcmFeeInWnds: u128 = 492_077_333_333;
pub const BridgeHubWestendBaseXcmFeeInWnds: u128 = 17_756_830_000;
/// Transaction fee that is paid at the Westend BridgeHub for delivering single inbound message.
/// (initially was calculated by test `BridgeHubWestend::can_calculate_fee_for_complex_message_delivery_transaction` + `33%`)
@@ -1263,6 +1263,12 @@ fn change_xcm_bridge_hub_router_base_fee_by_governance_works() {
1000,
Box::new(|call| RuntimeCall::System(call).encode()),
|| {
log::error!(
target: "bridges::estimate",
"`bridging::XcmBridgeHubRouterBaseFee` actual value: {} for runtime: {}",
bridging::XcmBridgeHubRouterBaseFee::get(),
<Runtime as frame_system::Config>::Version::get(),
);
(
bridging::XcmBridgeHubRouterBaseFee::key().to_vec(),
bridging::XcmBridgeHubRouterBaseFee::get(),
@@ -1289,6 +1295,12 @@ fn change_xcm_bridge_hub_ethereum_base_fee_by_governance_works() {
1000,
Box::new(|call| RuntimeCall::System(call).encode()),
|| {
log::error!(
target: "bridges::estimate",
"`bridging::BridgeHubEthereumBaseFee` actual value: {} for runtime: {}",
bridging::to_ethereum::BridgeHubEthereumBaseFee::get(),
<Runtime as frame_system::Config>::Version::get(),
);
(
bridging::to_ethereum::BridgeHubEthereumBaseFee::key().to_vec(),
bridging::to_ethereum::BridgeHubEthereumBaseFee::get(),
@@ -1209,6 +1209,38 @@ fn change_xcm_bridge_hub_router_byte_fee_by_governance_works() {
)
}
#[test]
fn change_xcm_bridge_hub_router_base_fee_by_governance_works() {
asset_test_utils::test_cases::change_storage_constant_by_governance_works::<
Runtime,
bridging::XcmBridgeHubRouterBaseFee,
Balance,
>(
collator_session_keys(),
1000,
Box::new(|call| RuntimeCall::System(call).encode()),
|| {
log::error!(
target: "bridges::estimate",
"`bridging::XcmBridgeHubRouterBaseFee` actual value: {} for runtime: {}",
bridging::XcmBridgeHubRouterBaseFee::get(),
<Runtime as frame_system::Config>::Version::get(),
);
(
bridging::XcmBridgeHubRouterBaseFee::key().to_vec(),
bridging::XcmBridgeHubRouterBaseFee::get(),
)
},
|old_value| {
if let Some(new_value) = old_value.checked_add(1) {
new_value
} else {
old_value.checked_sub(1).unwrap()
}
},
)
}
#[test]
fn reserve_transfer_native_asset_to_non_teleport_para_works() {
asset_test_utils::test_cases::reserve_transfer_native_asset_to_non_teleport_para_works::<
@@ -34,7 +34,7 @@ use sp_core::H160;
use sp_keyring::AccountKeyring::Alice;
use sp_runtime::{
generic::{Era, SignedPayload},
AccountId32,
AccountId32, Perbill,
};
use testnet_parachains_constants::rococo::{
consensus::RELAY_CHAIN_SLOT_DURATION_MILLIS, fee::WeightToFee,
@@ -400,53 +400,61 @@ mod bridge_hub_westend_tests {
#[test]
pub fn can_calculate_weight_for_paid_export_message_with_reserve_transfer() {
let estimated = bridge_hub_test_utils::test_cases::can_calculate_weight_for_paid_export_message_with_reserve_transfer::<
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs",
bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs::get(),
|| {
bridge_hub_test_utils::test_cases::can_calculate_weight_for_paid_export_message_with_reserve_transfer::<
Runtime,
XcmConfig,
WeightToFee,
>();
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs` value",
estimated,
max_expected
);
>()
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `ExportMessage` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
#[test]
pub fn can_calculate_fee_for_complex_message_delivery_transaction() {
let estimated = from_parachain::can_calculate_fee_for_complex_message_delivery_transaction::<
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs",
bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs::get(),
|| {
from_parachain::can_calculate_fee_for_complex_message_delivery_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message delivery` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
#[test]
pub fn can_calculate_fee_for_complex_message_confirmation_transaction() {
let estimated =
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs",
bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs::get(),
|| {
from_parachain::can_calculate_fee_for_complex_message_confirmation_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message confirmation` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
}
@@ -594,55 +602,43 @@ mod bridge_hub_bulletin_tests {
);
}
#[test]
pub fn can_calculate_weight_for_paid_export_message_with_reserve_transfer() {
let estimated = bridge_hub_test_utils::test_cases::can_calculate_weight_for_paid_export_message_with_reserve_transfer::<
Runtime,
XcmConfig,
WeightToFee,
>();
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseXcmFeeInRocs` value",
estimated,
max_expected
);
}
#[test]
pub fn can_calculate_fee_for_complex_message_delivery_transaction() {
let estimated =
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs",
bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs::get(),
|| {
from_grandpa_chain::can_calculate_fee_for_complex_message_delivery_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseDeliveryFeeInRocs` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
None, /* we don't want lowering according to the Bulletin setup, because
* `from_grandpa_chain` is cheaper then `from_parachain_chain` */
&format!(
"Estimate fee for `single message delivery` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
#[test]
pub fn can_calculate_fee_for_complex_message_confirmation_transaction() {
let estimated =
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs",
bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs::get(),
|| {
from_grandpa_chain::can_calculate_fee_for_complex_message_confirmation_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_rococo::BridgeHubRococoBaseConfirmationFeeInRocs` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
None, /* we don't want lowering according to the Bulletin setup, because
* `from_grandpa_chain` is cheaper then `from_parachain_chain` */
&format!(
"Estimate fee for `single message confirmation` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
}
@@ -38,7 +38,7 @@ use sp_consensus_aura::SlotDuration;
use sp_keyring::AccountKeyring::Alice;
use sp_runtime::{
generic::{Era, SignedPayload},
AccountId32,
AccountId32, Perbill,
};
use testnet_parachains_constants::westend::{
consensus::RELAY_CHAIN_SLOT_DURATION_MILLIS, fee::WeightToFee,
@@ -295,50 +295,59 @@ pub fn complex_relay_extrinsic_works() {
#[test]
pub fn can_calculate_weight_for_paid_export_message_with_reserve_transfer() {
let estimated = bridge_hub_test_utils::test_cases::can_calculate_weight_for_paid_export_message_with_reserve_transfer::<
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_westend::BridgeHubWestendBaseXcmFeeInWnds",
bp_bridge_hub_westend::BridgeHubWestendBaseXcmFeeInWnds::get(),
|| {
bridge_hub_test_utils::test_cases::can_calculate_weight_for_paid_export_message_with_reserve_transfer::<
Runtime,
XcmConfig,
WeightToFee,
>();
// check if estimated value is sane
let max_expected = bp_bridge_hub_westend::BridgeHubWestendBaseXcmFeeInWnds::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_westend::BridgeHubWestendBaseXcmFeeInWnds` value",
estimated,
max_expected
);
>()
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `ExportMessage` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
#[test]
pub fn can_calculate_fee_for_complex_message_delivery_transaction() {
let estimated = from_parachain::can_calculate_fee_for_complex_message_delivery_transaction::<
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_westend::BridgeHubWestendBaseDeliveryFeeInWnds",
bp_bridge_hub_westend::BridgeHubWestendBaseDeliveryFeeInWnds::get(),
|| {
from_parachain::can_calculate_fee_for_complex_message_delivery_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_westend::BridgeHubWestendBaseDeliveryFeeInWnds::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_westend::BridgeHubWestendBaseDeliveryFeeInWnds` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message delivery` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
#[test]
pub fn can_calculate_fee_for_complex_message_confirmation_transaction() {
let estimated = from_parachain::can_calculate_fee_for_complex_message_confirmation_transaction::<
bridge_hub_test_utils::check_sane_fees_values(
"bp_bridge_hub_westend::BridgeHubWestendBaseConfirmationFeeInWnds",
bp_bridge_hub_westend::BridgeHubWestendBaseConfirmationFeeInWnds::get(),
|| {
from_parachain::can_calculate_fee_for_complex_message_confirmation_transaction::<
RuntimeTestsAdapter,
>(collator_session_keys(), construct_and_estimate_extrinsic_fee);
// check if estimated value is sane
let max_expected = bp_bridge_hub_westend::BridgeHubWestendBaseConfirmationFeeInWnds::get();
assert!(
estimated <= max_expected,
"calculated: {:?}, max_expected: {:?}, please adjust `bp_bridge_hub_westend::BridgeHubWestendBaseConfirmationFeeInWnds` value",
estimated,
max_expected
);
>(collator_session_keys(), construct_and_estimate_extrinsic_fee)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message confirmation` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
@@ -21,3 +21,60 @@ pub mod test_data;
pub use bp_test_utils::test_header;
pub use parachains_runtimes_test_utils::*;
use sp_runtime::Perbill;
/// A helper function for comparing the actual value of a fee constant with its estimated value. The
/// estimated value can be overestimated (`overestimate_in_percent`), and if the difference to the
/// actual value is below `margin_overestimate_diff_in_percent_for_lowering`, we should lower the
/// actual value.
pub fn check_sane_fees_values(
const_name: &str,
actual: u128,
calculate_estimated_fee: fn() -> u128,
overestimate_in_percent: Perbill,
margin_overestimate_diff_in_percent_for_lowering: Option<i16>,
label: &str,
) {
let estimated = calculate_estimated_fee();
let estimated_plus_overestimate = estimated + (overestimate_in_percent * estimated);
let diff_to_estimated = diff_as_percent(actual, estimated);
let diff_to_estimated_plus_overestimate = diff_as_percent(actual, estimated_plus_overestimate);
sp_tracing::try_init_simple();
log::error!(
target: "bridges::estimate",
"{label}:\nconstant: {const_name}\n[+] actual: {actual}\n[+] estimated: {estimated} ({diff_to_estimated:.2?})\n[+] estimated(+33%): {estimated_plus_overestimate} ({diff_to_estimated_plus_overestimate:.2?})",
);
// check if estimated value is sane
assert!(
estimated <= actual,
"estimated: {estimated}, actual: {actual}, please adjust `{const_name}` to the value: {estimated_plus_overestimate}",
);
assert!(
estimated_plus_overestimate <= actual,
"estimated_plus_overestimate: {estimated_plus_overestimate}, actual: {actual}, please adjust `{const_name}` to the value: {estimated_plus_overestimate}",
);
if let Some(margin_overestimate_diff_in_percent_for_lowering) =
margin_overestimate_diff_in_percent_for_lowering
{
assert!(
diff_to_estimated_plus_overestimate > margin_overestimate_diff_in_percent_for_lowering as f64,
"diff_to_estimated_plus_overestimate: {diff_to_estimated_plus_overestimate:.2}, overestimate_diff_in_percent_for_lowering: {margin_overestimate_diff_in_percent_for_lowering}, please adjust `{const_name}` to the value: {estimated_plus_overestimate}",
);
}
}
pub fn diff_as_percent(left: u128, right: u128) -> f64 {
let left = left as f64;
let right = right as f64;
((left - right).abs() / left) * 100f64 * (if left >= right { -1 } else { 1 }) as f64
}
#[test]
fn diff_as_percent_works() {
assert_eq!(-20_f64, diff_as_percent(100, 80));
assert_eq!(25_f64, diff_as_percent(80, 100));
assert_eq!(33_f64, diff_as_percent(13351000000, 17756830000));
}
@@ -36,7 +36,7 @@ use bridge_runtime_common::{
},
messages_xcm_extension::XcmAsPlainPayload,
};
use frame_support::traits::{Get, OnFinalize, OnInitialize};
use frame_support::traits::{OnFinalize, OnInitialize};
use frame_system::pallet_prelude::BlockNumberFor;
use parachains_runtimes_test_utils::{
AccountIdOf, BasicParachainRuntime, CollatorSessionKeys, RuntimeCallOf, SlotDurations,
@@ -358,16 +358,8 @@ where
message_proof,
helpers::relayer_id_at_bridged_chain::<RuntimeHelper::Runtime, RuntimeHelper::MPI>(),
);
let estimated_fee = compute_extrinsic_fee(batch);
log::error!(
target: "bridges::estimate",
"Estimate fee: {:?} for single message delivery for runtime: {:?}",
estimated_fee,
<RuntimeHelper::Runtime as frame_system::Config>::Version::get(),
);
estimated_fee
compute_extrinsic_fee(batch)
})
}
@@ -427,15 +419,7 @@ where
message_delivery_proof,
unrewarded_relayers,
);
let estimated_fee = compute_extrinsic_fee(batch);
log::error!(
target: "bridges::estimate",
"Estimate fee: {:?} for single message confirmation for runtime: {:?}",
estimated_fee,
<RuntimeHelper::Runtime as frame_system::Config>::Version::get(),
);
estimated_fee
compute_extrinsic_fee(batch)
})
}
@@ -37,7 +37,7 @@ use bridge_runtime_common::{
},
messages_xcm_extension::XcmAsPlainPayload,
};
use frame_support::traits::{Get, OnFinalize, OnInitialize};
use frame_support::traits::{OnFinalize, OnInitialize};
use frame_system::pallet_prelude::BlockNumberFor;
use parachains_runtimes_test_utils::{
AccountIdOf, BasicParachainRuntime, CollatorSessionKeys, RuntimeCallOf, SlotDurations,
@@ -446,16 +446,8 @@ where
message_proof,
helpers::relayer_id_at_bridged_chain::<RuntimeHelper::Runtime, RuntimeHelper::MPI>(),
);
let estimated_fee = compute_extrinsic_fee(batch);
log::error!(
target: "bridges::estimate",
"Estimate fee: {:?} for single message delivery for runtime: {:?}",
estimated_fee,
<RuntimeHelper::Runtime as frame_system::Config>::Version::get(),
);
estimated_fee
compute_extrinsic_fee(batch)
})
}
@@ -531,15 +523,7 @@ where
message_delivery_proof,
unrewarded_relayers,
);
let estimated_fee = compute_extrinsic_fee(batch);
log::error!(
target: "bridges::estimate",
"Estimate fee: {:?} for single message confirmation for runtime: {:?}",
estimated_fee,
<RuntimeHelper::Runtime as frame_system::Config>::Version::get(),
);
estimated_fee
compute_extrinsic_fee(batch)
})
}
@@ -641,13 +641,5 @@ where
let estimated_fee = WeightToFee::weight_to_fee(&weight);
assert!(estimated_fee > BalanceOf::<Runtime>::zero());
sp_tracing::try_init_simple();
log::error!(
target: "bridges::estimate",
"Estimate fee: {:?} for `ExportMessage` for runtime: {:?}",
estimated_fee,
Runtime::Version::get(),
);
estimated_fee.into()
}