Support multiple chain extensions #1958
Conversation
xgreenx
changed the title
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑These are the results when building the
Link to the run | Last update: Wed Nov 8 10:06:45 CET 2023 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1958 +/- ##
==========================================
+ Coverage 53.30% 53.40% +0.09%
==========================================
Files 220 220
Lines 6845 6879 +34
==========================================
+ Hits 3649 3674 +25
- Misses 3196 3205 +9 ☔ View full report in Codecov by Sentry. |
| pub struct CombinedInstance { | ||
| /// The instance of the [`Psp22Extension`] chain extension. | ||
| /// | ||
| /// It provides you access to `PSP22` functionality. | ||
| pub psp22: <Psp22Extension as ChainExtensionInstance>::Instance, | ||
| /// The instance of the [`FetchRandom`] chain extension. | ||
| /// | ||
| /// It provides you access to randomness functionality. | ||
| pub rand: <FetchRandom as ChainExtensionInstance>::Instance, | ||
| } | ||
|
|
||
| impl ChainExtensionInstance for CombinedChainExtension { | ||
| type Instance = CombinedInstance; | ||
|
|
||
| fn instantiate() -> Self::Instance { | ||
| CombinedInstance { | ||
| psp22: <Psp22Extension as ChainExtensionInstance>::instantiate(), | ||
| rand: <FetchRandom as ChainExtensionInstance>::instantiate(), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The instantiate() method can be auto generated by macro since there is no custom logic to be implemented on the developer side. You already provide each extension type as <Psp22Extension as ChainExtensionInstance>::Instance. You can just automatically generate implementation through Derive macro.
| pub struct CombinedInstance { | |
| /// The instance of the [`Psp22Extension`] chain extension. | |
| /// | |
| /// It provides you access to `PSP22` functionality. | |
| pub psp22: <Psp22Extension as ChainExtensionInstance>::Instance, | |
| /// The instance of the [`FetchRandom`] chain extension. | |
| /// | |
| /// It provides you access to randomness functionality. | |
| pub rand: <FetchRandom as ChainExtensionInstance>::Instance, | |
| } | |
| impl ChainExtensionInstance for CombinedChainExtension { | |
| type Instance = CombinedInstance; | |
| fn instantiate() -> Self::Instance { | |
| CombinedInstance { | |
| psp22: <Psp22Extension as ChainExtensionInstance>::instantiate(), | |
| rand: <FetchRandom as ChainExtensionInstance>::instantiate(), | |
| } | |
| } | |
| } | |
| #[derive(CombinedChainExtension)] | |
| pub struct CombinedInstance { | |
| /// The instance of the [`Psp22Extension`] chain extension. | |
| /// | |
| /// It provides you access to `PSP22` functionality. | |
| pub psp22: <Psp22Extension as ChainExtensionInstance>::Instance, | |
| /// The instance of the [`FetchRandom`] chain extension. | |
| /// | |
| /// It provides you access to randomness functionality. | |
| pub rand: <FetchRandom as ChainExtensionInstance>::Instance, | |
| } |
There was a problem hiding this comment.
The derive macro can't modify the field's type. It leads to a strange API where the user still should use <Psp22Extension as ChainExtensionInstance>::Instance, but implementation will be generated automatically. So, I decided to go with the macro_rule approach. You can define the structure as usual structure, where each field is a chain extension, and it will produce the implementation for you.
ink::combine_extensions! {
/// This extension combines the [`FetchRandom`] and [`Psp22Extension`] extensions.
/// It is possible to combine any number of extensions in this way.
///
/// This structure is an instance that is returned by the `self.env().extension()` call.
pub struct CombinedChainExtension {
/// The instance of the [`Psp22Extension`] chain extension.
///
/// It provides you access to `PSP22` functionality.
pub psp22: Psp22Extension,
/// The instance of the [`FetchRandom`] chain extension.
///
/// It provides you access to randomness functionality.
pub rand: FetchRandom,
}
}| /// the corresponding sub-extension implementation. | ||
| /// It is possible to combine any number of extensions in this way. | ||
| #[ink::scale_derive(TypeInfo)] | ||
| pub struct CombinedChainExtension; |
There was a problem hiding this comment.
For similar reason, I am not a fun of having two structs associated with chain extensions. Can't you use CombinedInstance instead?
There was a problem hiding this comment.
The factory type should implement TypeInfo without any information regarding private types generated by the #[ink::chain_extension] macro. For that, we must either manually implement the TypeInfo trait, or have a separate instance type. Another reason is that the type that implements the ink::ChainExtensionInstance trait is a factory. If the factory produces self instance, it leads to incorrect usage of the pattern.
| /// | ||
| /// It is a combination of the [`ExtensionId`] and [`FunctionId`]. | ||
| #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| pub struct GlobalMethodId { |
There was a problem hiding this comment.
In your example you access the method by specifying a chain extension instance first: self.env().extension().rand.fetch_random([0; 32]
Why do you need that if you can identify each method uniquely? Why can't you collect all chain extension functions into a single "list" to make them accessible together?
e.g.
self.env().extensions().fetch_random([0; 32]
and then dispatch the call to the associated chain extension since you can derive this info from the function id
There was a problem hiding this comment.
If you have the method with the same name in each extension, it is impossible to identify which instance we need to call.
There was a problem hiding this comment.
Well, you just define them with different names? CE function names can be anything since they are defined by the SC developer.
There was a problem hiding this comment.
It doesn't work if you want to use extensions defined in other crates.
…ion of the combined chain extension
# Conflicts: # CHANGELOG.md
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑These are the results when building the
Link to the run | Last update: Tue Nov 28 14:11:48 CET 2023 |
| } | ||
| } | ||
|
|
||
| /// The unique ID of a chain extension method across all chain extensions. |
There was a problem hiding this comment.
The uniqueness of the id doesn't seem to be guaranteed anywhere, so we're just relying on the contract author to check for colliding ids?
Tricky to do statically, though could possibly check for duplicates during the metadata pass.
Anyway it may not be important, since if by chance (unlikely?) there are colliding ids that will be caught very quickly. when testing.
Summary
Closes #1747
cargo-contractorpallet-contracts? - NoThis change closes #1747 by adding an example of how to use multiple chain extensions. It contains:
#[ink::chain_extension]macro and the definition of the chain extension.Description
This pull request adds a new
combined_extensionexample that shows how to userand_extension::FetchRandomandpsp22_extension::Psp22Extensiontogether inside of one contract. This kind of functionality is already available in the current version of the ink. You only need to define a new chain extension that uses other chain extensions - combined chain extension. It can be done by implementing theink::ChainExtensionInstancetrait for any type.But the current chains extension works only with
u32- a combination of the extension idu16and function idu16.I've split them explicitly. It brings a new breaking change API that requires defining the extension id during the definition of the chain extension -
ink::chain_extension(extension = u16). Plus, the attribute#[ink(extension = u32)]was replaced with#[ink(function = u16)].Checklist before requesting a review
CHANGELOG.mdFollow-up
After merging this pull request, we need to update documentation here.