Skip to content

Conversation

@zqhxuyuan
Copy link
Contributor

issue: #1557

@zqhxuyuan zqhxuyuan requested a review from xlc October 29, 2021 09:49
Copy link
Contributor

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

I would recommend to use allow list on pallet-xcm, instead of block list. Polkadot xcm pallet has been adding dispatchable calls in it's new releases, and it's easy to miss new added calls with block list.

@xlc
Copy link
Member

xlc commented Oct 30, 2021

best to do a exhaustive match listing all the calls so in future if new calls are introduced, it will be a compiling error

@zqhxuyuan
Copy link
Contributor Author

zqhxuyuan commented Nov 1, 2021

As the Call enum has __Ignore case, so here the default case we return unimplemented!()

Modified: use the __Ignore instead of _, this can ensure that if there're method changes in pallet-xcm, then we should change this BaseFilter, or-else it'll have compiling error.

@zqhxuyuan
Copy link
Contributor Author

@zjb0807 would you look at the failed e2e-tests?

@zjb0807
Copy link
Contributor

zjb0807 commented Nov 1, 2021

@zjb0807 would you look at the failed e2e-tests?

Retry it works.

@xlc xlc merged commit 1317df5 into master Nov 1, 2021
@xlc xlc deleted the disable_xcm_method branch November 1, 2021 07:45
syan095 pushed a commit that referenced this pull request Nov 3, 2021
…eriodic-update

* origin/master:
  Feature/add update available staking (#1514)
  Clean Dependencies (#1577)
  Disable xcm method (#1558)

# Conflicts:
#	modules/homa-lite/src/benchmarking.rs
#	modules/homa-lite/src/lib.rs
#	modules/homa-lite/src/tests.rs
#	modules/homa-lite/src/weights.rs
#	runtime/acala/src/weights/module_homa_lite.rs
#	runtime/karura/src/weights/module_homa_lite.rs
#	runtime/mandala/src/weights/module_homa_lite.rs
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