-
Notifications
You must be signed in to change notification settings - Fork 49
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
Enable maintenance pallets for the testnet #1336
Conversation
Check 712cb62
|
Check 712cb62
|
Check 712cb62
|
Check 712cb62
|
{ | ||
fn contains(full_name: &pallet_tx_pause::RuntimeCallNameOf<Runtime>) -> bool { | ||
match (full_name.0.as_slice(), full_name.1.as_slice()) { | ||
(b"System", b"remark_with_event") => true, |
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.
Any specific reason?
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.
It's used to test that the whitelist is working well. Check out the tx_pause_pause_calls_except_on_whitelist
testcase. This remark doesn't concern about the chain security, so I think this is the perfect test example.
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.
Yep, I know those tests. They are written in common runtime, which means that all runtimes need to be configured like this.
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.
Indeed. Do you think this is only needs to be enabled in the testnet? I suggest leaving it as it is now. Later we maybe add some meaningful dipatch call, we can change the testcase later to adapt to that situation.
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.
That could potentially become a problem in the future. Honestly, I'm not sure if we should whitelist that call for the mainnet. Can we assume some calls are "unconcern" about security?
This PR introduces
pallet-tx-pause
to the Pangolin network, one of the two maintenance pallets in the polkadot-sdk upstream repo. It allows to pause some dispatch calls in the case of emergency. It's worth noting the reviewers is the two origins and make sure they are secure enough before going into the production. As for why don't addpallet-safe-mode
? Two reasons:safe-mod
, theenter
call is permissionless, that means anyone who has enough fund over the Limitation can put the system into safe-mode state. I don't think this is a secure enough solution for our system now. If the max deposit value is not set to an appropriate value, your system is at risk.tx-pause
is enough IMO. The most common senerios is that some dispatch calls are vulnerable, pausing these calls is enough.