Skip to content

Commit

Permalink
xcm: SovereignPaidRemoteExporter: remove unused RefundSurplus instruc…
Browse files Browse the repository at this point in the history 
…tion (#2312)

Refunding surplus happens anyway on xcm_executor::post_process(),
automatically refunding surplus to original_origin at the end of
execution. Since SovereignPaidRemoteExporter doesn't ClearOrigin, it can
simply rely on the automatic mechanism.

Furthermore, RefundSurplus instruction refunds _surplus_. Surplus exists
only as a result of Transact, SetErrorHandler or SetAppendix
instructions, none of which being part of the
SovereignPaidRemoteExporter XCM program. So surplus is always zero here
anyway.
  • Loading branch information
@acatangiu
acatangiu authored Nov 15, 2023
1 parent 0226b55 commit 824b782
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 18 deletions.
1 change: 0 additions & 1 deletion cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,6 @@ where
]),
]),
},
RefundSurplus,
DepositAsset {
assets: Wild(All),
beneficiary: MultiLocation { parents: 1, interior: X1(Parachain(1000)) },
Expand Down
6 changes: 3 additions & 3 deletions polkadot/xcm/xcm-builder/src/tests/bridging/local_para_para.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn sending_to_bridged_chain_works() {
maybe_with_topic(|| {
let msg = Xcm(vec![Trap(1)]);
let dest = (Parent, Parent, Remote::get(), Parachain(1)).into();
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, (Here, 100).into());
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, Price::get());
assert_eq!(TheBridge::service(), 1);
assert_eq!(
take_received_remote_messages(),
Expand Down Expand Up @@ -78,7 +78,7 @@ fn sending_to_parachain_of_bridged_chain_works() {
maybe_with_topic(|| {
let msg = Xcm(vec![Trap(1)]);
let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into();
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, (Here, 100).into());
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, Price::get());
assert_eq!(TheBridge::service(), 1);
let expected = vec![(
(Parent, Parachain(1000)).into(),
Expand Down Expand Up @@ -110,7 +110,7 @@ fn sending_to_relay_chain_of_bridged_chain_works() {
maybe_with_topic(|| {
let msg = Xcm(vec![Trap(1)]);
let dest = (Parent, Parent, Remote::get()).into();
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, (Here, 100).into());
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, Price::get());
assert_eq!(TheBridge::service(), 1);
let expected = vec![(
Parent.into(),
Expand Down
4 changes: 2 additions & 2 deletions polkadot/xcm/xcm-builder/src/tests/bridging/local_relay_relay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn sending_to_bridged_chain_works() {
let msg = Xcm(vec![Trap(1)]);
assert_eq!(
send_xcm::<Router>((Parent, Remote::get()).into(), msg).unwrap().1,
(Here, 100).into()
Price::get()
);
assert_eq!(TheBridge::service(), 1);
let expected = vec![(
Expand All @@ -68,7 +68,7 @@ fn sending_to_parachain_of_bridged_chain_works() {
maybe_with_topic(|| {
let msg = Xcm(vec![Trap(1)]);
let dest = (Parent, Remote::get(), Parachain(1000)).into();
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, (Here, 100).into());
assert_eq!(send_xcm::<Router>(dest, msg).unwrap().1, Price::get());
assert_eq!(TheBridge::service(), 1);
let expected = vec![(
Parachain(1000).into(),
Expand Down
21 changes: 10 additions & 11 deletions polkadot/xcm/xcm-builder/src/tests/bridging/paid_remote_relay_relay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
use super::*;

parameter_types! {
// 100 to use the bridge (export) and 80 for the remote execution weight (4 instructions x (10 +
// 10) weight each).
pub SendOverBridgePrice: u128 = 180u128 + if UsingTopic::get() { 20 } else { 0 };
pub UniversalLocation: Junctions = X2(GlobalConsensus(Local::get()), Parachain(100));
pub RelayUniversalLocation: Junctions = X1(GlobalConsensus(Local::get()));
pub RemoteUniversalLocation: Junctions = X1(GlobalConsensus(Remote::get()));
Expand All @@ -32,11 +35,9 @@ parameter_types! {
Remote::get(),
None,
MultiLocation::parent(),
Some((Parent, 200u128 + if UsingTopic::get() { 20 } else { 0 }).into())
Some((Parent, SendOverBridgePrice::get()).into())
)
];
// ^^^ 100 to use the bridge (export) and 100 for the remote execution weight (5 instructions
// x (10 + 10) weight each).
}
type TheBridge =
TestBridge<BridgeBlobDispatcher<TestRemoteIncomingRouter, RemoteUniversalLocation, ()>>;
Expand Down Expand Up @@ -68,7 +69,7 @@ fn sending_to_bridged_chain_works() {
clear_assets(Parachain(100));
add_asset(Parachain(100), (Here, 1000u128));

let price = 200u128 + if UsingTopic::get() { 20 } else { 0 };
let price = SendOverBridgePrice::get();

let msg = Xcm(vec![Trap(1)]);
assert_eq!(send_xcm::<LocalRouter>(dest, msg).unwrap().1, (Parent, price).into());
Expand All @@ -86,7 +87,7 @@ fn sending_to_bridged_chain_works() {
)];
assert_eq!(take_received_remote_messages(), expected);

// The export cost 50 ref time and 50 proof size weight units (and thus 100 units of
// The export cost 40 ref time and 40 proof size weight units (and thus 80 units of
// balance).
assert_eq!(asset_list(Parachain(100)), vec![(Here, 1000u128 - price).into()]);

Expand All @@ -104,11 +105,10 @@ fn sending_to_bridged_chain_works() {
destination: Here,
xcm: xcm_with_topic([0; 32], vec![Trap(1)]),
},
RefundSurplus,
DepositAsset { assets: Wild(All), beneficiary: Parachain(100).into() },
],
),
outcome: Outcome::Complete(test_weight(5)),
outcome: Outcome::Complete(test_weight(4)),
paid: true,
};
assert_eq!(RoutingLog::take(), vec![entry]);
Expand Down Expand Up @@ -143,7 +143,7 @@ fn sending_to_parachain_of_bridged_chain_works() {
clear_assets(Parachain(100));
add_asset(Parachain(100), (Here, 1000u128));

let price = 200u128 + if UsingTopic::get() { 20 } else { 0 };
let price = SendOverBridgePrice::get();

let msg = Xcm(vec![Trap(1)]);
assert_eq!(send_xcm::<LocalRouter>(dest, msg).unwrap().1, (Parent, price).into());
Expand All @@ -161,7 +161,7 @@ fn sending_to_parachain_of_bridged_chain_works() {
)];
assert_eq!(take_received_remote_messages(), expected);

// The export cost 50 ref time and 50 proof size weight units (and thus 100 units of
// The export cost 40 ref time and 40 proof size weight units (and thus 80 units of
// balance).
assert_eq!(asset_list(Parachain(100)), vec![(Here, 1000u128 - price).into()]);

Expand All @@ -179,11 +179,10 @@ fn sending_to_parachain_of_bridged_chain_works() {
destination: Parachain(100).into(),
xcm: xcm_with_topic([0; 32], vec![Trap(1)]),
},
RefundSurplus,
DepositAsset { assets: Wild(All), beneficiary: Parachain(100).into() },
],
),
outcome: Outcome::Complete(test_weight(5)),
outcome: Outcome::Complete(test_weight(4)),
paid: true,
};
assert_eq!(RoutingLog::take(), vec![entry]);
Expand Down
1 change: 0 additions & 1 deletion polkadot/xcm/xcm-builder/src/universal_exports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ impl<Bridges: ExporterFor, Router: SendXcm, UniversalLocation: Get<InteriorMulti
WithdrawAsset(fees.clone().into()),
BuyExecution { fees, weight_limit: Unlimited },
export_instruction,
RefundSurplus,
DepositAsset { assets: All.into(), beneficiary: local_from_bridge },
]
} else {
Expand Down

0 comments on commit 824b782

Please sign in to comment.