Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bail out early if switching zero amount #739

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions pallets/pallet-asset-switch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ pub mod pallet {
Xcm,
/// Internal error.
Internal,
/// Attempt to switch zero tokens.
ZeroAmount,
}

#[pallet::storage]
Expand Down Expand Up @@ -379,17 +381,24 @@ pub mod pallet {
) -> DispatchResult {
let submitter = T::SubmitterOrigin::ensure_origin(origin)?;

// 1. Retrieve switch pair info from storage, else fail.
// 1. Prevent switches of 0 tokens, which would just waste users' XCM fee
// balances.
ensure!(
local_asset_amount > LocalCurrencyBalanceOf::<T, I>::zero(),
Error::<T, I>::ZeroAmount
);

// 2. Retrieve switch pair info from storage, else fail.
let switch_pair =
SwitchPair::<T, I>::get().ok_or(DispatchError::from(Error::<T, I>::SwitchPairNotFound))?;

// 2. Check if switches are enabled.
// 3. Check if switches are enabled.
ensure!(
switch_pair.is_enabled(),
DispatchError::from(Error::<T, I>::SwitchPairNotEnabled)
);

// 3. Verify the tx submitter has enough local assets for the switch, without
// 4. Verify the tx submitter has enough local assets for the switch, without
// having their balance go to zero.
T::LocalCurrency::can_withdraw(&submitter, local_asset_amount)
.into_result(true)
Expand All @@ -398,7 +407,7 @@ pub mod pallet {
DispatchError::from(Error::<T, I>::UserSwitchBalance)
})?;

// 4. Verify the local assets can be transferred to the switch pool account.
// 5. Verify the local assets can be transferred to the switch pool account.
// This could fail if the pool's balance is `0` and the sent amount is less
// than ED.
T::LocalCurrency::can_deposit(&switch_pair.pool_account, local_asset_amount, Provenance::Extant)
Expand All @@ -408,7 +417,7 @@ pub mod pallet {
DispatchError::from(Error::<T, I>::LocalPoolBalance)
})?;

// 5. Verify we have enough balance (minus ED, already substracted from the
// 6. Verify we have enough balance (minus ED, already substracted from the
// stored balance info) on the remote location to perform the transfer.
let remote_asset_amount_as_u128 = local_asset_amount.into();
ensure!(
Expand Down Expand Up @@ -466,7 +475,7 @@ pub mod pallet {
DispatchError::from(Error::<T, I>::Xcm)
})?;

// 6. Compose and validate XCM message
// 7. Compose and validate XCM message
let appendix: Xcm<()> = vec![
RefundSurplus,
DepositAsset {
Expand Down Expand Up @@ -499,11 +508,11 @@ pub mod pallet {
DispatchError::from(Error::<T, I>::Xcm)
})?;

// 7. Call into hook pre-switch checks
// 8. Call into hook pre-switch checks
T::SwitchHooks::pre_local_to_remote_switch(&submitter, &beneficiary, local_asset_amount)
.map_err(|e| DispatchError::from(Error::<T, I>::Hook(e.into())))?;

// 8. Transfer funds from user to pool
// 9. Transfer funds from user to pool
let transferred_amount = T::LocalCurrency::transfer(
&submitter,
&switch_pair.pool_account,
Expand All @@ -520,7 +529,7 @@ pub mod pallet {
return Err(Error::<T, I>::Internal.into());
}

// 9. Take XCM fee from submitter.
// 10. Take XCM fee from submitter.
let withdrawn_fees = T::AssetTransactor::withdraw_asset(&remote_asset_fee_v4, &submitter_as_location, None)
.map_err(|e| {
log::info!(
Expand All @@ -542,13 +551,13 @@ pub mod pallet {
return Err(DispatchError::from(Error::<T, I>::Internal));
}

// 10. Send XCM out
// 11. Send XCM out
T::XcmRouter::deliver(xcm_ticket.0).map_err(|e| {
log::info!("Failed to deliver ticket with error {:?}", e);
DispatchError::from(Error::<T, I>::Xcm)
})?;

// 11. Update remote asset balance and circulating supply.
// 12. Update remote asset balance and circulating supply.
SwitchPair::<T, I>::try_mutate(|entry| {
let Some(switch_pair_info) = entry.as_mut() else {
log::error!(target: LOG_TARGET, "Failed to borrow stored switch pair info as mut.");
Expand All @@ -567,7 +576,7 @@ pub mod pallet {
Ok(())
})?;

// 12. Call into hook post-switch checks
// 13. Call into hook post-switch checks
T::SwitchHooks::post_local_to_remote_switch(&submitter, &beneficiary, local_asset_amount)
.map_err(|e| DispatchError::from(Error::<T, I>::Hook(e.into())))?;

Expand Down
27 changes: 27 additions & 0 deletions pallets/pallet-asset-switch/src/tests/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,33 @@ fn successful() {
});
}

#[test]
fn fails_on_zero_amount() {
let user = AccountId32::from([0; 32]);
let pool_account = AccountId32::from([1; 32]);
ExtBuilder::default()
.with_switch_pair_info(NewSwitchPairInfoOf::<MockRuntime> {
pool_account,
remote_asset_circulating_supply: 0,
remote_asset_ed: 0,
remote_asset_id: get_remote_erc20_asset_id().into(),
remote_asset_total_supply: 100_000,
remote_reserve_location: get_asset_hub_location().into(),
remote_xcm_fee: XCM_ASSET_FEE.into(),
status: SwitchPairStatus::Running,
})
.build_and_execute_with_sanity_tests(|| {
assert_noop!(
Pallet::<MockRuntime>::switch(
RawOrigin::Signed(user).into(),
0,
Box::new(get_asset_hub_location().into())
),
Error::<MockRuntime>::ZeroAmount
);
});
}

#[test]
fn fails_on_invalid_origin() {
ExtBuilder::default().build_and_execute_with_sanity_tests(|| {
Expand Down
Loading