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

contracts: Allow ChainExtension::call() to access &mut self #11874

Merged
merged 7 commits into from
Jul 25, 2022

Conversation

athei
Copy link
Member

@athei athei commented Jul 20, 2022

Previously, ChainExtension:call() didn't had access to self. Mainly because it wasn't clear what the lifetime (when is it created and droped) of such a value would be.

Turned out that a useful lifetime for this value is per-call (not per call stack). This allows chain extensions to hold some temporary values within a single call. The need for this came up during implementation of an XCM chain extension. This PR therefore introduces two changes:

Porting Guide

This PR changes the interface of ChainExtension and hence it will break the build of existing extensions. Extensions authors need to be aware of the following changes:

ChainExtension:call() now takes &mut self instead of no self at all

Just add mut &self to your existing chain extensions' call functions. This is all that needs to be done.

ChainExtension need to implement Default because they need to be initialized during contract execution

Pre-existing chain extensions are probably unit structs and hence a simple #[derive(Default)] on this struct will be enough to resolve the situation.

The func_id parameter was removed from call

It is replaced by an associated function on the Environment. The id can be fetched by calling env.func_id(). This is a drive-by followup fix for #11816.

@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 20, 2022
@athei athei requested review from cmichi, agryaznov and HCastano July 20, 2022 17:40
frame/contracts/src/tests.rs Show resolved Hide resolved
frame/contracts/src/tests.rs Show resolved Hide resolved
frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
frame/contracts/src/wasm/runtime.rs Show resolved Hide resolved
Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Good stuff! Also, I ❤️ the porting guide

frame/contracts/src/chain_extension.rs Show resolved Hide resolved
@athei
Copy link
Member Author

athei commented Jul 25, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 7200934 into master Jul 25, 2022
@paritytech-processbot paritytech-processbot bot deleted the at/chainextv2 branch July 25, 2022 15:48
/// It returns the two least significant bytes of the `id` passed by a contract as the other
/// two bytes represent the chain extension itself (the code which is calling this function).
pub fn func_id(&self) -> u16 {
(self.inner.id & 0x00FF) as u16
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be 0xFFFF?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. You are right. This just masks the last byte. Do you are to open a PR? Would be good to modify one of the tests to actually catch that (change one of the function ids to use the second byte, too).

DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
…tytech#11874)

* Give chain extensions the ability to store some temporary values

* Update frame/contracts/src/wasm/runtime.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Rename func_id -> id

* Replace `id` param by two functions on `env`

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…tytech#11874)

* Give chain extensions the ability to store some temporary values

* Update frame/contracts/src/wasm/runtime.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Rename func_id -> id

* Replace `id` param by two functions on `env`

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
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: No status
Development

Successfully merging this pull request may close these issues.

4 participants