-
Notifications
You must be signed in to change notification settings - Fork 379
Companion #690 to bring to master
safer template defaults
#696
Conversation
* use safer defaults in template for sovereign paraobjects * Nothing trait, fix XcmTeleportFilter * rm unused traits, update rococo and template runtimes * https://writingexplained.org/recognise-or-recognize-difference * Override for AdvertisedXcmVersion default -> to match all runtimes * cargo +nightly fmt
main
safer template defaultsmaster
safer template defaults
@@ -516,13 +512,14 @@ impl pallet_xcm::Config for Runtime { | |||
type XcmExecuteFilter = Everything; |
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.
type XcmExecuteFilter = Everything; | |
type XcmExecuteFilter = Nothing; |
This will disable the execute dispatchable on the XCM pallet. Unsure if that's ok for templates, because this means local testing will require flipping this back to Everything.
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.
Bummer... My goal is to make the template as safe as possible for production. This is unsafe to leave like this (without consideration) if used, correct?
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.
We can expose that you need to change this to enable for local testing in the cumulus tutorial and how-to guides, I think it would be best that way - so it forces you to consider the ramifications.
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.
type XcmExecuteFilter = Everything; | |
type XcmExecuteFilter = Nothing; | |
// ^ Disable dispatchable execute on the XCM pallet. | |
// Needs to be `Everything` for local testing. |
also included here: https://github.com/substrate-developer-hub/substrate-parachain-template/pull/80
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.
Yeah looks good, it's basically the same comment that you had on the PR you linked.
#690