This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Introduce XCM matcher for writing barriers #6756
Merged
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
83ba763
Introduce XCM matcher for writing barriers
KiChjang 3920975
Fix compilation
KiChjang ba143e8
cargo fmt
KiChjang 0f43e74
Add more doc comments
KiChjang a3c86eb
Add mod doc comment
KiChjang 9f9967e
Merge branch 'master' into kckyeung/xcm-pattern-matcher
KiChjang 23d0edf
More doc comments
KiChjang 5dc0fda
Add tests and fix logic
KiChjang ddbdb88
Remove redundant syntax
KiChjang 2a0819b
Add more doc comments
KiChjang c9c8fe3
Add more doc comments
KiChjang 5e3f859
Add more doc comments
KiChjang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next
Next commit
Introduce XCM matcher for writing barriers
- Loading branch information
commit 83ba763ddbf281ce1f94953cbb7ee30b1b776de0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Copyright 2023 Parity Technologies (UK) Ltd. | ||
| // This file is part of Polkadot. | ||
|
|
||
| // Substrate is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Substrate is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Polkadot. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| use super::{Instruction, MultiLocation}; | ||
| use crate::{CreateMatcher, MatchXcm}; | ||
|
|
||
| impl<'a, Call> CreateMatcher for &'a mut [Instruction<Call>] { | ||
| type Matcher = Matcher<'a, Call>; | ||
|
|
||
| fn matcher(self) -> Self::Matcher { | ||
| let total_inst = self.len(); | ||
|
|
||
| Matcher { xcm: self, current_idx: 0, total_inst } | ||
| } | ||
| } | ||
|
|
||
| pub struct Matcher<'a, Call> { | ||
| pub(crate) xcm: &'a mut [Instruction<Call>], | ||
| pub(crate) current_idx: usize, | ||
| pub(crate) total_inst: usize, | ||
| } | ||
|
|
||
| impl<'a, Call> MatchXcm for Matcher<'a, Call> { | ||
| type Error = (); | ||
| type Inst = Instruction<Call>; | ||
| type Loc = MultiLocation; | ||
|
|
||
| fn assert_remaining_insts(self, n: usize) -> Result<Self, Self::Error> | ||
| where | ||
| Self: Sized, | ||
| { | ||
| if self.total_inst - self.current_idx != n { | ||
| return Err(()) | ||
| } | ||
|
|
||
| Ok(self) | ||
| } | ||
|
|
||
| fn match_next_inst<F>(mut self, mut f: F) -> Result<Self, Self::Error> | ||
| where | ||
| Self: Sized, | ||
| F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>, | ||
| { | ||
| if self.current_idx < self.total_inst { | ||
| f(&mut self.xcm[self.current_idx])?; | ||
| self.current_idx += 1; | ||
| Ok(self) | ||
| } else { | ||
| Err(()) | ||
| } | ||
| } | ||
|
|
||
| fn match_next_inst_while<C, F>(mut self, cond: C, mut f: F) -> Result<Self, Self::Error> | ||
| where | ||
| Self: Sized, | ||
| C: Fn() -> bool, | ||
| F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>, | ||
| { | ||
| if self.current_idx >= self.total_inst { | ||
| return Err(()) | ||
| } | ||
|
|
||
| while cond() && self.current_idx < self.total_inst { | ||
| f(&mut self.xcm[self.current_idx])?; | ||
| self.current_idx += 1; | ||
| } | ||
|
|
||
| Ok(self) | ||
| } | ||
|
|
||
| fn skip_inst_while<C>(mut self, cond: C) -> Result<Self, Self::Error> | ||
| where | ||
| Self: Sized, | ||
| C: Fn(&Self::Inst) -> bool, | ||
| { | ||
| if self.current_idx >= self.total_inst { | ||
| return Err(()) | ||
| } | ||
|
|
||
| while cond(&self.xcm[self.current_idx]) && self.current_idx < self.total_inst { | ||
KiChjang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| self.current_idx += 1; | ||
| } | ||
|
|
||
| Ok(self) | ||
| } | ||
| } | ||
KiChjang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,12 +22,15 @@ use frame_support::{ | |||||||||||||||
| }; | ||||||||||||||||
| use polkadot_parachain::primitives::IsSystem; | ||||||||||||||||
| use sp_std::{marker::PhantomData, result::Result}; | ||||||||||||||||
| use xcm::latest::{ | ||||||||||||||||
| Instruction::{self, *}, | ||||||||||||||||
| InteriorMultiLocation, Junction, Junctions, | ||||||||||||||||
| Junctions::X1, | ||||||||||||||||
| MultiLocation, Weight, | ||||||||||||||||
| WeightLimit::*, | ||||||||||||||||
| use xcm::{ | ||||||||||||||||
| latest::{ | ||||||||||||||||
| Instruction::{self, *}, | ||||||||||||||||
| InteriorMultiLocation, Junction, Junctions, | ||||||||||||||||
| Junctions::X1, | ||||||||||||||||
| MultiLocation, Weight, | ||||||||||||||||
| WeightLimit::*, | ||||||||||||||||
| }, | ||||||||||||||||
| CreateMatcher, MatchXcm, | ||||||||||||||||
| }; | ||||||||||||||||
| use xcm_executor::traits::{OnResponse, ShouldExecute}; | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -77,32 +80,31 @@ impl<T: Contains<MultiLocation>> ShouldExecute for AllowTopLevelPaidExecutionFro | |||||||||||||||
| // We will read up to 5 instructions. This allows up to 3 `ClearOrigin` instructions. We | ||||||||||||||||
| // allow for more than one since anything beyond the first is a no-op and it's conceivable | ||||||||||||||||
| // that composition of operations might result in more than one being appended. | ||||||||||||||||
| let mut iter = instructions.iter_mut().take(5); | ||||||||||||||||
| let i = iter.next().ok_or(())?; | ||||||||||||||||
| match i { | ||||||||||||||||
| ReceiveTeleportedAsset(..) | | ||||||||||||||||
| WithdrawAsset(..) | | ||||||||||||||||
| ReserveAssetDeposited(..) | | ||||||||||||||||
| ClaimAsset { .. } => (), | ||||||||||||||||
| _ => return Err(()), | ||||||||||||||||
| } | ||||||||||||||||
| let mut i = iter.next().ok_or(())?; | ||||||||||||||||
| while let ClearOrigin = i { | ||||||||||||||||
| i = iter.next().ok_or(())?; | ||||||||||||||||
| } | ||||||||||||||||
| match i { | ||||||||||||||||
| BuyExecution { weight_limit: Limited(ref mut weight), .. } | ||||||||||||||||
| if weight.all_gte(max_weight) => | ||||||||||||||||
| { | ||||||||||||||||
| *weight = weight.max(max_weight); | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| }, | ||||||||||||||||
| BuyExecution { ref mut weight_limit, .. } if weight_limit == &Unlimited => { | ||||||||||||||||
| *weight_limit = Limited(max_weight); | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| }, | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| } | ||||||||||||||||
| let end = instructions.len().min(5); | ||||||||||||||||
| instructions[..end] | ||||||||||||||||
| .matcher() | ||||||||||||||||
| .match_next_inst(|inst| match inst { | ||||||||||||||||
| ReceiveTeleportedAsset(..) | | ||||||||||||||||
| WithdrawAsset(..) | | ||||||||||||||||
| ReserveAssetDeposited(..) | | ||||||||||||||||
| ClaimAsset { .. } => Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| })? | ||||||||||||||||
| .skip_inst_while(|inst| matches!(inst, ClearOrigin))? | ||||||||||||||||
| .match_next_inst(|inst| match inst { | ||||||||||||||||
| BuyExecution { weight_limit: Limited(ref mut weight), .. } | ||||||||||||||||
| if weight.all_gte(max_weight) => | ||||||||||||||||
| { | ||||||||||||||||
| *weight = weight.max(max_weight); | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| }, | ||||||||||||||||
| BuyExecution { ref mut weight_limit, .. } if weight_limit == &Unlimited => { | ||||||||||||||||
| *weight_limit = Limited(max_weight); | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| }, | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| })?; | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -178,20 +180,24 @@ impl< | |||||||||||||||
| // execution. This technical could get it past the barrier condition, but the execution | ||||||||||||||||
| // would instantly fail since the first instruction would cause an error with the | ||||||||||||||||
| // invalid UniversalOrigin. | ||||||||||||||||
| while skipped < MaxPrefixes::get() as usize { | ||||||||||||||||
| match instructions.get(skipped) { | ||||||||||||||||
| Some(UniversalOrigin(new_global)) => { | ||||||||||||||||
| // Note the origin is *relative to local consensus*! So we need to escape local | ||||||||||||||||
| // consensus with the `parents` before diving in into the `universal_location`. | ||||||||||||||||
| actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); | ||||||||||||||||
| }, | ||||||||||||||||
| Some(DescendOrigin(j)) => { | ||||||||||||||||
| actual_origin.append_with(*j).map_err(|_| ())?; | ||||||||||||||||
| }, | ||||||||||||||||
| _ => break, | ||||||||||||||||
| } | ||||||||||||||||
| skipped += 1; | ||||||||||||||||
| } | ||||||||||||||||
| instructions.matcher().match_next_inst_while( | ||||||||||||||||
| || skipped < MaxPrefixes::get() as usize && skipped < instructions.len(), | ||||||||||||||||
| |inst| { | ||||||||||||||||
| let res = match inst { | ||||||||||||||||
| UniversalOrigin(new_global) => { | ||||||||||||||||
| // Note the origin is *relative to local consensus*! So we need to escape | ||||||||||||||||
| // local consensus with the `parents` before diving in into the | ||||||||||||||||
| // `universal_location`. | ||||||||||||||||
| actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| }, | ||||||||||||||||
| DescendOrigin(j) => actual_origin.append_with(*j).map_err(|_| ()), | ||||||||||||||||
| _ => Ok(()), | ||||||||||||||||
| }; | ||||||||||||||||
| skipped += 1; | ||||||||||||||||
| res | ||||||||||||||||
| }, | ||||||||||||||||
| )?; | ||||||||||||||||
| InnerBarrier::should_execute( | ||||||||||||||||
| &actual_origin, | ||||||||||||||||
| &mut instructions[skipped..], | ||||||||||||||||
|
|
@@ -241,12 +247,12 @@ impl<T: Contains<MultiLocation>> ShouldExecute for AllowExplicitUnpaidExecutionF | |||||||||||||||
| origin, instructions, max_weight, _weight_credit, | ||||||||||||||||
| ); | ||||||||||||||||
| ensure!(T::contains(origin), ()); | ||||||||||||||||
| match instructions.first() { | ||||||||||||||||
| Some(UnpaidExecution { weight_limit: Limited(m), .. }) if m.all_gte(max_weight) => | ||||||||||||||||
| Ok(()), | ||||||||||||||||
| Some(UnpaidExecution { weight_limit: Unlimited, .. }) => Ok(()), | ||||||||||||||||
| instructions.matcher().match_next_inst(|inst| match inst { | ||||||||||||||||
| UnpaidExecution { weight_limit: Limited(m), .. } if m.all_gte(max_weight) => Ok(()), | ||||||||||||||||
| UnpaidExecution { weight_limit: Unlimited, .. } => Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| } | ||||||||||||||||
| })?; | ||||||||||||||||
| Ok(()) | ||||||||||||||||
|
Comment on lines
+255
to
+256
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
But this is why I opted for the |
||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -276,13 +282,16 @@ impl<ResponseHandler: OnResponse> ShouldExecute for AllowKnownQueryResponses<Res | |||||||||||||||
| "AllowKnownQueryResponses origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", | ||||||||||||||||
| origin, instructions, _max_weight, _weight_credit, | ||||||||||||||||
| ); | ||||||||||||||||
| ensure!(instructions.len() == 1, ()); | ||||||||||||||||
| match instructions.first() { | ||||||||||||||||
| Some(QueryResponse { query_id, querier, .. }) | ||||||||||||||||
| if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) => | ||||||||||||||||
| Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| } | ||||||||||||||||
| instructions | ||||||||||||||||
| .matcher() | ||||||||||||||||
| .assert_remaining_insts(1)? | ||||||||||||||||
| .match_next_inst(|inst| match inst { | ||||||||||||||||
| QueryResponse { query_id, querier, .. } | ||||||||||||||||
| if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) => | ||||||||||||||||
| Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| })?; | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -302,9 +311,13 @@ impl<T: Contains<MultiLocation>> ShouldExecute for AllowSubscriptionsFrom<T> { | |||||||||||||||
| origin, instructions, _max_weight, _weight_credit, | ||||||||||||||||
| ); | ||||||||||||||||
| ensure!(T::contains(origin), ()); | ||||||||||||||||
| match instructions { | ||||||||||||||||
| &mut [SubscribeVersion { .. } | UnsubscribeVersion] => Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| } | ||||||||||||||||
| instructions | ||||||||||||||||
| .matcher() | ||||||||||||||||
| .assert_remaining_insts(1)? | ||||||||||||||||
| .match_next_inst(|inst| match inst { | ||||||||||||||||
| SubscribeVersion { .. } | UnsubscribeVersion => Ok(()), | ||||||||||||||||
| _ => Err(()), | ||||||||||||||||
| })?; | ||||||||||||||||
| Ok(()) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this whole API exclusively tailored to mutable Instruction slices?
That seems to limit the environments that it can be used in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you may want to mutate the instruction whilst iterating through it -- this is how the
AllowUnpaidExecutionFrombarrier works.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But can we then not have two flavours? One for mut and one normal? Or do you think its not worth it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how it is limited tbh, we mutably borrow instructions one at a time, so unless there is another (mutable) borrow of the same instruction somewhere else, it shouldn't restrict the places in which you can use this API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition, the
Matcherstruct already mutably borrows the entire slice, so the mutable borrow of the instruction here isn't really doing anything worse than that; instead, it's narrowing the scope of the mutable borrow to a single instruction.