Skip to content

Commit

Permalink
Fix foreign assets migration (#3190)
Browse files Browse the repository at this point in the history
* fix: foreign assets migration

* fix: warnings

* remove used method

* update hard-coded gas limits to a reasonable value

* fix test

* improve code
  • Loading branch information
RomarQ committed Feb 17, 2025
1 parent 1dc577e commit a196261
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 56 deletions.
132 changes: 101 additions & 31 deletions pallets/moonbeam-foreign-assets/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,30 @@ use precompile_utils::solidity::codec::{Address, BoundedString};
use precompile_utils::solidity::Codec;
use precompile_utils_macro::keccak256;
use sp_runtime::traits::ConstU32;
use sp_runtime::DispatchError;
use sp_runtime::{format, DispatchError, SaturatedConversion};
use sp_std::vec::Vec;
use xcm::latest::Error as XcmError;

const ERC20_CALL_MAX_CALLDATA_SIZE: usize = 4 + 32 + 32; // selector + address + uint256
const ERC20_CREATE_MAX_CALLDATA_SIZE: usize = 16 * 1024; // 16Ko

// Hardcoded gas limits (from manual binary search)
const ERC20_CREATE_GAS_LIMIT: u64 = 3_410_000; // highest failure: 3_406_000
pub(crate) const ERC20_BURN_FROM_GAS_LIMIT: u64 = 155_000; // highest failure: 154_000
pub(crate) const ERC20_MINT_INTO_GAS_LIMIT: u64 = 155_000; // highest failure: 154_000
const ERC20_PAUSE_GAS_LIMIT: u64 = 150_500; // highest failure: 150_500
pub(crate) const ERC20_TRANSFER_GAS_LIMIT: u64 = 155_000; // highest failure: 154_000
pub(crate) const ERC20_APPROVE_GAS_LIMIT: u64 = 154_000; // highest failure: 153_000
const ERC20_UNPAUSE_GAS_LIMIT: u64 = 151_000; // highest failure: 149_500

const ERC20_CREATE_GAS_LIMIT: u64 = 3_600_000; // highest failure: 3_600_000
pub(crate) const ERC20_BURN_FROM_GAS_LIMIT: u64 = 160_000; // highest failure: 154_000
pub(crate) const ERC20_MINT_INTO_GAS_LIMIT: u64 = 160_000; // highest failure: 154_000
const ERC20_PAUSE_GAS_LIMIT: u64 = 160_000; // highest failure: 150_500
pub(crate) const ERC20_TRANSFER_GAS_LIMIT: u64 = 160_000; // highest failure: 154_000
pub(crate) const ERC20_APPROVE_GAS_LIMIT: u64 = 160_000; // highest failure: 153_000
const ERC20_UNPAUSE_GAS_LIMIT: u64 = 160_000; // highest failure: 149_500

#[derive(Debug)]
pub enum EvmError {
BurnFromFail,
BurnFromFail(String),
ContractReturnInvalidValue,
DispatchError(DispatchError),
EvmCallFail,
MintIntoFail,
TransferFail,
EvmCallFail(String),
MintIntoFail(String),
TransferFail(String),
}

impl From<DispatchError> for EvmError {
Expand All @@ -59,7 +60,8 @@ impl From<DispatchError> for EvmError {
impl From<EvmError> for XcmError {
fn from(error: EvmError) -> XcmError {
match error {
EvmError::BurnFromFail => {
EvmError::BurnFromFail(err) => {
log::debug!("BurnFromFail error: {:?}", err);
XcmError::FailedToTransactAsset("Erc20 contract call burnFrom fail")
}
EvmError::ContractReturnInvalidValue => {
Expand All @@ -69,11 +71,16 @@ impl From<EvmError> for XcmError {
log::debug!("dispatch error: {:?}", err);
Self::FailedToTransactAsset("storage layer error")
}
EvmError::EvmCallFail => XcmError::FailedToTransactAsset("Fail to call erc20 contract"),
EvmError::MintIntoFail => {
XcmError::FailedToTransactAsset("Erc20 contract call mintInto fail")
EvmError::EvmCallFail(err) => {
log::debug!("EvmCallFail error: {:?}", err);
XcmError::FailedToTransactAsset("Fail to call erc20 contract")
}
EvmError::MintIntoFail(err) => {
log::debug!("MintIntoFail error: {:?}", err);
XcmError::FailedToTransactAsset("Erc20 contract call mintInto fail+")
}
EvmError::TransferFail => {
EvmError::TransferFail(err) => {
log::debug!("TransferFail error: {:?}", err);
XcmError::FailedToTransactAsset("Erc20 contract call transfer fail")
}
}
Expand Down Expand Up @@ -134,7 +141,10 @@ impl<T: crate::Config> EvmCaller<T> {
&<T as pallet_evm::Config>::config(),
contract_adress,
)
.map_err(|_| Error::Erc20ContractCreationFail)?;
.map_err(|err| {
log::debug!("erc20_create (error): {:?}", err.error.into());
Error::<T>::Erc20ContractCreationFail
})?;

ensure!(
matches!(
Expand Down Expand Up @@ -179,14 +189,17 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| EvmError::EvmCallFail)?;
.map_err(|err| EvmError::MintIntoFail(format!("{:?}", err.error.into())))?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
EvmError::MintIntoFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
EvmError::MintIntoFail(err)
}
);

Ok(())
Expand Down Expand Up @@ -225,14 +238,17 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| EvmError::EvmCallFail)?;
.map_err(|err| EvmError::TransferFail(format!("{:?}", err.error.into())))?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
EvmError::TransferFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
EvmError::TransferFail(err)
}
);

// return value is true.
Expand Down Expand Up @@ -280,14 +296,17 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| EvmError::EvmCallFail)?;
.map_err(|err| EvmError::EvmCallFail(format!("{:?}", err.error.into())))?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
EvmError::EvmCallFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
EvmError::EvmCallFail(err)
}
);

Ok(())
Expand Down Expand Up @@ -325,14 +344,17 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| EvmError::EvmCallFail)?;
.map_err(|err| EvmError::EvmCallFail(format!("{:?}", err.error.into())))?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
EvmError::BurnFromFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
EvmError::BurnFromFail(err)
}
);

Ok(())
Expand Down Expand Up @@ -362,14 +384,21 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| Error::<T>::EvmInternalError)?;
.map_err(|err| {
log::debug!("erc20_pause (error): {:?}", err.error.into());
Error::<T>::EvmInternalError
})?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
Error::<T>::EvmCallPauseFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
log::debug!("erc20_pause (error): {:?}", err);
Error::<T>::EvmCallPauseFail
}
);

Ok(())
Expand Down Expand Up @@ -400,16 +429,57 @@ impl<T: crate::Config> EvmCaller<T> {
Some(0),
&<T as pallet_evm::Config>::config(),
)
.map_err(|_| Error::<T>::EvmInternalError)?;
.map_err(|err| {
log::debug!("erc20_unpause (error): {:?}", err.error.into());
Error::<T>::EvmInternalError
})?;

ensure!(
matches!(
exec_info.exit_reason,
ExitReason::Succeed(ExitSucceed::Returned | ExitSucceed::Stopped)
),
Error::<T>::EvmCallUnpauseFail
{
let err = error_on_execution_failure(&exec_info.exit_reason, &exec_info.value);
log::debug!("erc20_unpause (error): {:?}", err);
Error::<T>::EvmCallUnpauseFail
}
);

Ok(())
}
}

fn error_on_execution_failure(reason: &ExitReason, data: &[u8]) -> String {
match reason {
ExitReason::Succeed(_) => String::new(),
ExitReason::Error(err) => format!("evm error: {err:?}"),
ExitReason::Fatal(err) => format!("evm fatal: {err:?}"),
ExitReason::Revert(_) => extract_revert_message(data),
}
}

/// The data should contain a UTF-8 encoded revert reason with a minimum size consisting of:
/// error function selector (4 bytes) + offset (32 bytes) + reason string length (32 bytes)
fn extract_revert_message(data: &[u8]) -> String {
const LEN_START: usize = 36;
const MESSAGE_START: usize = 68;
const BASE_MESSAGE: &str = "VM Exception while processing transaction: revert";
// Return base message if data is too short
if data.len() <= MESSAGE_START {
return BASE_MESSAGE.into();
}
// Extract message length and calculate end position
let message_len = U256::from(&data[LEN_START..MESSAGE_START]).saturated_into::<usize>();
let message_end = MESSAGE_START.saturating_add(message_len);
// Return base message if data is shorter than expected message end
if data.len() < message_end {
return BASE_MESSAGE.into();
}
// Extract and decode the message
let body = &data[MESSAGE_START..message_end];
match core::str::from_utf8(body) {
Ok(reason) => format!("{BASE_MESSAGE} {reason}"),
Err(_) => BASE_MESSAGE.into(),
}
}
1 change: 0 additions & 1 deletion pallets/moonbeam-foreign-assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ pub mod pallet {
AssetIdFiltered,
AssetNotFrozen,
CorruptedStorageOrphanLocation,
//Erc20ContractCallFail,
Erc20ContractCreationFail,
EvmCallPauseFail,
EvmCallUnpauseFail,
Expand Down
2 changes: 2 additions & 0 deletions pallets/moonbeam-lazy-migrations/src/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use frame_support::traits::Currency;
use frame_support::BoundedVec;
use frame_system::RawOrigin;
use pallet_asset_manager::AssetRegistrar;
use sp_core::H160;
use sp_core::{Get, U256};
use sp_runtime::traits::StaticLookup;
use sp_runtime::Saturating;
Expand Down Expand Up @@ -112,6 +113,7 @@ benchmarks! {
where
<T as pallet_assets::Config>::Balance: Into<U256>,
T::ForeignAssetType: Into<Option<Location>>,
<T as frame_system::Config>::AccountId: Into<H160> + From<H160>,
}
approve_assets_to_migrate {
let n in 1 .. 100u32;
Expand Down
46 changes: 34 additions & 12 deletions pallets/moonbeam-lazy-migrations/src/foreign_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use super::*;
use frame_support::sp_runtime::Saturating;
use frame_support::traits::{fungibles::metadata::Inspect, ReservableCurrency};
use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
use sp_core::U256;
use sp_core::{H160, U256};

#[derive(Debug, Encode, Decode, scale_info::TypeInfo, PartialEq, MaxEncodedLen)]
pub enum ForeignAssetMigrationStatus {
Expand All @@ -47,6 +47,7 @@ impl<T: Config> Pallet<T>
where
<T as pallet_assets::Config>::Balance: Into<U256>,
<T as pallet_asset_manager::Config>::ForeignAssetType: Into<Option<Location>>,
<T as frame_system::Config>::AccountId: Into<H160> + From<H160>,
{
/// Start a foreign asset migration by freezing the asset and creating the SC with the moonbeam
/// foreign assets pallet.
Expand Down Expand Up @@ -139,14 +140,20 @@ where
_ => {}
};

MIGRATING_FOREIGN_ASSETS::using_once(&mut true, || {
pallet_moonbeam_foreign_assets::Pallet::<T>::mint_into(
info.asset_id,
who.clone(),
asset.balance.into(),
)
})
.map_err(|_| Error::<T>::MintFailed)?;
let zero_address = T::AccountId::from(H160::zero());
if who.clone() != zero_address {
MIGRATING_FOREIGN_ASSETS::using_once(&mut true, || {
pallet_moonbeam_foreign_assets::Pallet::<T>::mint_into(
info.asset_id,
who.clone(),
asset.balance.into(),
)
})
.map_err(|err| {
log::debug!("Error: {err:?}");
Error::<T>::MintFailed
})?;
}

info.remaining_balances = info.remaining_balances.saturating_sub(1);
Ok::<(), Error<T>>(())
Expand All @@ -171,14 +178,29 @@ where
<T as pallet_assets::Config>::Currency::unreserve(&owner, approval.deposit);

MIGRATING_FOREIGN_ASSETS::using_once(&mut true, || {
pallet_moonbeam_foreign_assets::Pallet::<T>::approve(
let address: H160 = owner.clone().into();

// Temporarily remove metadata
let meta = pallet_evm::AccountCodesMetadata::<T>::take(address.clone());

let result = pallet_moonbeam_foreign_assets::Pallet::<T>::approve(
info.asset_id,
owner.clone(),
beneficiary,
approval.amount.into(),
)
);

// Re-add metadata
if let Some(metadata) = meta {
pallet_evm::AccountCodesMetadata::<T>::insert(address, metadata);
}

result
})
.map_err(|_| Error::<T>::ApprovalFailed)?;
.map_err(|err| {
log::debug!("Error: {err:?}");
Error::<T>::ApprovalFailed
})?;

info.remaining_approvals = info.remaining_approvals.saturating_sub(1);
Ok::<(), Error<T>>(())
Expand Down
1 change: 1 addition & 0 deletions pallets/moonbeam-lazy-migrations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ pub mod pallet {
where
<T as pallet_assets::Config>::Balance: Into<U256>,
<T as pallet_asset_manager::Config>::ForeignAssetType: Into<Option<Location>>,
<T as frame_system::Config>::AccountId: Into<H160> + From<H160>,
{
#[pallet::call_index(2)]
#[pallet::weight(Pallet::<T>::create_contract_metadata_weight(MAX_CONTRACT_CODE_SIZE))]
Expand Down
Loading

0 comments on commit a196261

Please sign in to comment.