Skip to content

Commit

Permalink
[cumulus] Improved check for sane bridge fees calculations (#3175)
Browse files Browse the repository at this point in the history
## 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)    
```
  • Loading branch information
bkontur authored Feb 2, 2024
1 parent 5ba8921 commit 74b597f
Show file tree
Hide file tree
Showing 10 changed files with 235 additions and 169 deletions.
4 changes: 2 additions & 2 deletions bridges/primitives/chain-bridge-hub-rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ 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%`)
pub const BridgeHubRococoBaseDeliveryFeeInRocs: u128 = 5_651_581_649;

/// 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;
}
2 changes: 1 addition & 1 deletion bridges/primitives/chain-bridge-hub-westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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%`)
Expand Down
12 changes: 12 additions & 0 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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::<
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
);
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,
>()
},
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::<
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
);
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)
},
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::<
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
);
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)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message confirmation` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
}

Expand Down Expand Up @@ -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 =
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
);
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)
},
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 =
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
);
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)
},
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()
),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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::<
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
);
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)
},
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::<
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
);
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)
},
Perbill::from_percent(33),
Some(-33),
&format!(
"Estimate fee for `single message confirmation` for runtime: {:?}",
<Runtime as frame_system::Config>::Version::get()
),
)
}
Loading

0 comments on commit 74b597f

Please sign in to comment.