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

contracts: Allow contracts to dispatch calls into the runtime #9276

Merged
13 commits merged into from
Jul 12, 2021
Merged

Conversation

athei
Copy link
Member

@athei athei commented Jul 5, 2021

Originally, contracts had the ability to dispatch calls into the runtime. However, without any mechanism to control which calls are allowed to be dispatched it was viewed as an attack vector and was removed.

With the introduction of origin filtering we can now re-introduce this functionality as seal_call_runtime. The users of this pallet are required to set pallet_contracts::Config::CallFilter in order to determine which runtime calls are allowed to be called from contracts. A sane default is not to allow any calls here as doing so requires committing to some stability guarantees (see inline docs).

In order to support post dispatch weight correction we need to pre-charge the static weight of a runtime call and adjust post dispatch. However, we recently abolished pre-charging because it introduced a bug where the returned gas_consumed can be an unsuitable value for gas_limit of an subsequent call. To solve this issue this PR also introduces the concept of a required_weight which is different from the consumed_weight and is returned in addition to the existing consumed_weight when using the RPC so that the UI can display a proper gas estimation.

@jacogr @Robbepop UIs should switch from consumed_weight to required_weight for gas estimation.

Please note that this does not obsolete chain extensions. You can do more with chain extensions. Please see the inline docs for more information.

As always new APIs are marked as __unstable__ until they are implemented in ink! and we are certain that we do not need further iterations.

Removing the Filter implementation on ()

Most of the code churn stems from a change that removes the Filter implementation on (). It is replaced by explicit AllowAllFilter and DenyAllFilter types. This is also why we need a companion.

polkadot companion: paritytech/polkadot#3420

@athei athei added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Jul 5, 2021
@athei athei added this to In Progress in Smart Contracts via automation Jul 5, 2021
Copy link
Contributor

@thiolliere thiolliere left a comment

Choose a reason for hiding this comment

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

apart from this abosrb_nested, which seems inacurate, everything else is good to me

frame/contracts/src/gas.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@thiolliere thiolliere left a comment

Choose a reason for hiding this comment

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

looks good to me

@kianenigma kianenigma removed their request for review July 8, 2021 09:48
Copy link
Contributor

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

LGTM, I'll create a follow-up for ink!.

@athei
Copy link
Member Author

athei commented Jul 12, 2021

bot merge

@ghost
Copy link

ghost commented Jul 12, 2021

Waiting for commit status.

@ghost ghost merged commit 2543f6f into master Jul 12, 2021
@ghost ghost deleted the at-call-runtime branch July 12, 2021 20:40
Smart Contracts automation moved this from In Progress to Done Jul 12, 2021
tash-2s added a commit to tash-2s/open-emoji-battler that referenced this pull request Sep 23, 2021
This pull request was closed.
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. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants