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

Add a filter for filtering unsupported MultiLocation #714

Merged
merged 8 commits into from
Mar 20, 2022

Conversation

Dengjianping
Copy link
Contributor

@Dengjianping Dengjianping commented Mar 14, 2022

Closes #714

Add an associated type MultiLocationsFilter to filter unsupported MultiLocation.

Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2022

Codecov Report

Merging #714 (8ff5aa1) into master (9e041dc) will increase coverage by 0.08%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##           master     #714      +/-   ##
==========================================
+ Coverage   75.37%   75.46%   +0.08%     
==========================================
  Files          83       83              
  Lines        7441     7476      +35     
==========================================
+ Hits         5609     5642      +33     
- Misses       1832     1834       +2     
Impacted Files Coverage Δ
xtokens/src/mock/para.rs 75.75% <ø> (ø)
xtokens/src/lib.rs 64.68% <83.33%> (+0.72%) ⬆️
xtokens/src/tests.rs 99.37% <100.00%> (+0.02%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

xtokens/src/lib.rs Outdated Show resolved Hide resolved

ParaB::execute_with(|| {
assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 1_000));
assert_noop!(
Copy link
Member

Choose a reason for hiding this comment

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

more test with transfer_multiassets and transfer_multicurrencies etc

@xlc xlc requested review from zqhxuyuan and shaunxw March 15, 2022 10:35
}

pub struct WhiteListingMultiLocations;
impl Contains<MultiLocation> for WhiteListingMultiLocations {
Copy link
Contributor

@zqhxuyuan zqhxuyuan Mar 15, 2022

Choose a reason for hiding this comment

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

we could offer some default implementation(in trait/location.rs), and use Tuple combine with fall-through way. i.e. AllowParentAccount, AllowParachainAccount, AllowLocalAccount. so parachain can use like type MultiLocationsFilter = (AllowParentAccount, AllowParachainAccount) or type MultiLocationsFilter = (AllowParentAccount, AllowParachainAccount, AllowLocalAccount) if they allow local asset.

Copy link
Contributor

@zqhxuyuan zqhxuyuan Mar 15, 2022

Choose a reason for hiding this comment

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

or a simple way like:

match_type! {
	pub type ParentOrParachain: impl Contains<MultiLocation> = {
		MultiLocation { parents: 1, interior: X1(AccountId32(_)) } |
		MultiLocation { parents: 1, interior: X2(Parachain(_), AccountId32(_)) }
	};
}

if allow local asset:

match_type! {
	pub type ParentOrParachainOrLocal: impl Contains<MultiLocation> = {
                 MultiLocation { parents: 0, interior: X1(AccountId32(_)) } |
		MultiLocation { parents: 1, interior: X1(AccountId32(_)) } |
		MultiLocation { parents: 1, interior: X2(Parachain(_), AccountId32(_)) }
	};
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot use Parachain(_), becasue it will match all para id.

Signed-off-by: Dengjianping <djptux@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants