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.

Sign up for GitHub

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

LP: Unitary testing for add_currency #1912

Merged
merged 7 commits into from
Jul 17, 2024
Merged

LP: Unitary testing for add_currency #1912

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d3ad7b3
port add-currency tests into uts
lemunozm Jul 15, 2024
efa3873
remove LiquidityPoolsWrappedToken struct
lemunozm Jul 15, 2024
cc538c4
remove unused code
lemunozm Jul 15, 2024
e4b78f4
fix compilation issues
lemunozm Jul 15, 2024
a0917b2
fix clippy
lemunozm Jul 15, 2024
88f407c
fix clippy
lemunozm Jul 15, 2024
07ff058
Merge remote-tracking branch 'origin/main' into lp/uts-add-currency
lemunozm Jul 15, 2024
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
38 changes: 0 additions & 38 deletions libs/types/src/tokens.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,44 +395,6 @@ impl CrossChainTransferability {
}
}

/// Liquidity Pools-wrapped tokens
///
/// Currently, LiquidityPools are only deployed on EVM-based chains and
/// therefore we only support EVM tokens. In the far future, we might support
/// wrapped tokens from other chains such as Cosmos based ones.
#[derive(
Clone,
Copy,
PartialOrd,
Ord,
PartialEq,
Eq,
Debug,
Encode,
Decode,
TypeInfo,
MaxEncodedLen,
Serialize,
Deserialize,
)]
pub enum LiquidityPoolsWrappedToken {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it can be removed.

Its only current purpose is to return the inner values in the private method of pallet-liquidity-pools: try_get_wrapped_token(). From my PoV, it adds a new layer of understanding for the reading; doing that as a tuple makes things easier.

I can revert if you have some point against it.

/// An EVM-native token
EVM {
/// The EVM chain id where the token is deployed
chain_id: EVMChainId,
/// The token contract address
address: [u8; 20],
},
}

impl From<LiquidityPoolsWrappedToken> for DomainAddress {
fn from(token: LiquidityPoolsWrappedToken) -> Self {
match token {
LiquidityPoolsWrappedToken::EVM { chain_id, address } => Self::EVM(chain_id, address),
}
}
}

#[derive(
Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug, Encode, Decode, TypeInfo, MaxEncodedLen,
)]
Expand Down
25 changes: 9 additions & 16 deletions pallets/liquidity-pools/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub mod pallet {
};
use cfg_types::{
permissions::{PermissionScope, PoolRole, Role},
tokens::{CustomMetadata, LiquidityPoolsWrappedToken},
tokens::CustomMetadata,
EVMChainId,
};
use frame_support::{
Expand Down Expand Up @@ -306,7 +306,7 @@ pub mod pallet {
/// The metadata of the given asset does not declare it as transferable
/// via LiquidityPools'.
AssetNotLiquidityPoolsTransferable,
/// The asset is not a [LiquidityPoolsWrappedToken] and thus cannot be
/// The asset is not a a wrapped token and thus cannot be
/// transferred via liquidity pools.
AssetNotLiquidityPoolsWrappedToken,
/// A pool could not be found.
Expand Down Expand Up @@ -454,7 +454,7 @@ pub mod pallet {

// Check that the registered asset location matches the destination
match Self::try_get_wrapped_token(&currency_id)? {
LiquidityPoolsWrappedToken::EVM { chain_id, .. } => {
(chain_id, ..) => {
ensure!(
Domain::EVM(chain_id) == destination,
Error::<T>::InvalidDomain
Expand Down Expand Up @@ -615,7 +615,7 @@ pub mod pallet {

// Check that the registered asset location matches the destination
match Self::try_get_wrapped_token(&currency_id)? {
LiquidityPoolsWrappedToken::EVM { chain_id, .. } => {
(chain_id, ..) => {
ensure!(
Domain::EVM(chain_id) == receiver.domain(),
Error::<T>::InvalidDomain
Expand Down Expand Up @@ -674,10 +674,7 @@ pub mod pallet {

let currency = Self::try_get_general_index(currency_id)?;

let LiquidityPoolsWrappedToken::EVM {
chain_id,
address: evm_address,
} = Self::try_get_wrapped_token(&currency_id)?;
let (chain_id, evm_address) = Self::try_get_wrapped_token(&currency_id)?;

T::OutboundQueue::submit(
who,
Expand Down Expand Up @@ -872,13 +869,12 @@ pub mod pallet {
}

/// Checks whether the given currency is transferable via LiquidityPools
/// and whether its metadata contains a location which can be
/// converted to [LiquidityPoolsWrappedToken].
/// and whether its metadata contains an evm location.
///
/// Requires the currency to be registered in the `AssetRegistry`.
pub fn try_get_wrapped_token(
currency_id: &T::CurrencyId,
) -> Result<LiquidityPoolsWrappedToken, DispatchError> {
) -> Result<(EVMChainId, [u8; 20]), DispatchError> {
let meta = T::AssetRegistry::metadata(currency_id).ok_or(Error::<T>::AssetNotFound)?;
ensure!(
meta.additional.transferability.includes_liquidity_pools(),
Expand All @@ -904,9 +900,7 @@ pub mod pallet {
network: None,
key: address,
}],
) if Some(pallet_instance.into()) == pallet_index => {
Ok(LiquidityPoolsWrappedToken::EVM { chain_id, address })
}
) if Some(pallet_instance.into()) == pallet_index => Ok((chain_id, address)),
_ => Err(Error::<T>::AssetNotLiquidityPoolsWrappedToken.into()),
}
}
Expand Down Expand Up @@ -944,8 +938,7 @@ pub mod pallet {

let currency = Self::try_get_general_index(currency_id)?;

let LiquidityPoolsWrappedToken::EVM { chain_id, .. } =
Self::try_get_wrapped_token(&currency_id)?;
let (chain_id, ..) = Self::try_get_wrapped_token(&currency_id)?;

Ok((currency, chain_id))
}
Expand Down
92 changes: 86 additions & 6 deletions pallets/liquidity-pools/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ mod util {
}
}

pub fn wrapped_transferable_metadata() -> AssetMetadata {
pub fn locatable_transferable_metadata() -> AssetMetadata {
let pallet_index = PalletInfo::index::<LiquidityPools>();
AssetMetadata {
location: Some(VersionedLocation::V4(Location::new(
Expand Down Expand Up @@ -93,7 +93,7 @@ mod transfer {
#[test]
fn success() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::wrapped_transferable_metadata()));
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));
TransferFilter::mock_check(|_| Ok(()));
Tokens::mint_into(CURRENCY_ID, &ALICE, AMOUNT).unwrap();
Gateway::mock_submit(|sender, destination, msg| {
Expand Down Expand Up @@ -226,7 +226,7 @@ mod transfer {
#[test]
fn with_wrong_domain() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::wrapped_transferable_metadata()));
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));

assert_noop!(
LiquidityPools::transfer(
Expand All @@ -243,7 +243,7 @@ mod transfer {
#[test]
fn without_satisfy_filter() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::wrapped_transferable_metadata()));
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));
TransferFilter::mock_check(|_| Err(DispatchError::Other("Err")));

assert_noop!(
Expand All @@ -261,7 +261,7 @@ mod transfer {
#[test]
fn without_sufficient_balance() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::wrapped_transferable_metadata()));
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));
TransferFilter::mock_check(|_| Ok(()));

assert_noop!(
Expand Down Expand Up @@ -636,7 +636,7 @@ mod update_token_price {
assert_eq!(origin, POOL_CURRENCY_ID);
Ok(MARKET_RATIO)
});
AssetRegistry::mock_metadata(|_| Some(util::wrapped_transferable_metadata()));
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));
Gateway::mock_submit(|sender, destination, msg| {
assert_eq!(sender, ALICE);
assert_eq!(destination, EVM_DOMAIN_ADDRESS.domain());
Expand Down Expand Up @@ -878,6 +878,86 @@ mod update_member {
}
}

mod add_currency {
use super::*;

#[test]
fn success() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::locatable_transferable_metadata()));
Gateway::mock_submit(|sender, destination, msg| {
assert_eq!(sender, ALICE);
assert_eq!(destination, EVM_DOMAIN_ADDRESS.domain());
assert_eq!(
msg,
Message::AddCurrency {
currency: util::currency_index(CURRENCY_ID),
evm_address: CONTRACT_ACCOUNT,
}
);
Ok(())
});

assert_ok!(LiquidityPools::add_currency(
RuntimeOrigin::signed(ALICE),
CURRENCY_ID
));
})
}

mod erroring_out {
use super::*;

#[test]
fn with_no_metadata() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| None);

assert_noop!(
LiquidityPools::add_currency(RuntimeOrigin::signed(ALICE), CURRENCY_ID),
Error::<Runtime>::AssetNotFound,
);
})
}

#[test]
fn with_unsupported_token() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::default_metadata()));

assert_noop!(
LiquidityPools::add_currency(RuntimeOrigin::signed(ALICE), CurrencyId::Native),
TokenError::Unsupported,
);
})
}

#[test]
fn with_no_transferible_asset() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::default_metadata()));

assert_noop!(
LiquidityPools::add_currency(RuntimeOrigin::signed(ALICE), CURRENCY_ID),
Error::<Runtime>::AssetNotLiquidityPoolsTransferable,
);
})
}

#[test]
fn with_wrong_location() {
System::externalities().execute_with(|| {
AssetRegistry::mock_metadata(|_| Some(util::transferable_metadata()));

assert_noop!(
LiquidityPools::add_currency(RuntimeOrigin::signed(ALICE), CURRENCY_ID),
Error::<Runtime>::AssetNotLiquidityPoolsWrappedToken
);
})
}
}
}

#[test]
fn receiving_invalid_message() {
System::externalities().execute_with(|| {
Expand Down
Loading
Loading