Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

update safe call filter #7080

Merged

Conversation

xlc
Copy link
Contributor

@xlc xlc commented Apr 13, 2023

For Acala liquid crowdloan DOT redemption, we need to XCM transact with proxy.proxy to control the crowdloan vault account, which is a pure proxy.

The complexity of all the proxy call is O(1) + n whereas n is the complexity of the inner call. The filter is applied to inner call as well and therefore it is safe to whitelist proxy calls.

In additional to that, I synchronized the filters between Kusama and Polkadot just so they are as similar as possible. We had enough issues caused by mismatch behaivour in those runtimes and I want to reduce the surprising factor.

Changes:

  • Add proxy to both Kusama and Polkadot.
  • Add whitelist_call to Kusama. This is already included in Polkadot.
  • Remove Gov1 related call from Polkadot. They are abstained in Kusama.
  • Add OpenGov related call to Polkadot. They are included in Kusama.

@paritytech-ci paritytech-ci requested review from a team April 13, 2023 23:08
@KiChjang KiChjang added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Apr 14, 2023
}) => true,
}) |
RuntimeCall::Whitelist(pallet_whitelist::Call::whitelist_call { .. }) |
RuntimeCall::Proxy(..) => true,
Copy link
Member

Choose a reason for hiding this comment

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

The docs of this filter state:

/// 2. Cannot lead to another call being made;

I guess in this case it should be fine since the proxy pallet routes all calls through this filter again?

/// 3. Have a defined proof size weight, e.g. no unbounded vecs in call parameters.

Proxy::proxy does accept call: Box<<T as Config>::RuntimeCall>, so this is not given.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but we actually still allow Utility::as_derivative which is an extrinsic that itself contains a Call, in order to allow use cases on both Moonbeam and Acala to still work as intended, so allowing Proxy::proxy is also based on the same logic.

In principle however, the 2nd rule may not be necessary, as it's impossible to create an infinitely recursive extrinsic call -- the chain would have thrown an error during decoding as there is a max decoding depth of 10 in the SCALE codec.

@paritytech-ci paritytech-ci requested a review from a team April 20, 2023 20:05
@KiChjang
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit d2a41b3 into paritytech:master Apr 21, 2023
@xlc xlc deleted the update-safe-call-filter branch April 21, 2023 09:36
ordian added a commit that referenced this pull request Apr 26, 2023
* master: (30 commits)
  update rocksdb to 0.20.1 (#7113)
  Reduce base proof size weight component to zero (#7081)
  PVF: Move PVF workers into separate crate (#7101)
  Companion for #13923 (#7111)
  update safe call filter (#7080)
  PVF: Don't dispute on missing artifact (#7011)
  XCM: Properly set the pricing for the DMP router (#6843)
  pvf: Update docs for PVF artifacts (#6551)
  Bump syn from 2.0.14 to 2.0.15 (#7093)
  Companion for substrate#13771 (#6983)
  Added Dwellir Nigeria bootnodes. (#7097)
  Companion for Substrate #13889 (#7063)
  Switch to DNS name based bootnodes for Rococo (#7040)
  companion for substrate#13883 (#7059)
  [xcm] Added `UnpaidExecution` instruction to `UnpaidRemoteExporter` (#7091)
  Bump serde_json from 1.0.85 to 1.0.96 (#7072)
  Bump hex-literal from 0.3.4 to 0.4.1 (#7071)
  Small simplification (#7089)
  [XCM - UnpaidRemoteExporter] Remove unreachable code (#7088)
  sync versions with current release (#7083)
  ...
ordian added a commit that referenced this pull request Apr 26, 2023
* master: (39 commits)
  malus: dont panic on missing validation data (#6952)
  Offences Migration v1: Removes `ReportsByKindIndex` (#7114)
  Fix stalling dispute coordinator. (#7125)
  Fix rolling session window (#7126)
  [ci] Update buildah command and version (#7128)
  Bump assigned_slots params (#6991)
  XCM: Remote account converter (#6662)
  Rework `dispute-coordinator` to use `RuntimeInfo` for obtaining session information instead of `RollingSessionWindow` (#6968)
  Revert default proof size back to 64 KB (#7115)
  update rocksdb to 0.20.1 (#7113)
  Reduce base proof size weight component to zero (#7081)
  PVF: Move PVF workers into separate crate (#7101)
  Companion for #13923 (#7111)
  update safe call filter (#7080)
  PVF: Don't dispute on missing artifact (#7011)
  XCM: Properly set the pricing for the DMP router (#6843)
  pvf: Update docs for PVF artifacts (#6551)
  Bump syn from 2.0.14 to 2.0.15 (#7093)
  Companion for substrate#13771 (#6983)
  Added Dwellir Nigeria bootnodes. (#7097)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants