Skip to content
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

Rework the chain extension usage #1722

Open
xgreenx opened this issue Mar 19, 2023 · 0 comments
Open

Rework the chain extension usage #1722

xgreenx opened this issue Mar 19, 2023 · 0 comments

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Mar 19, 2023

Overview

The current usage of the chain extension has several limitations:

  • Only one chain extension is supported at the same time.
  • The chain extension is part of the Environment and tied to the contract. It makes usage of it outside of the contract extraordinary and sometimes impossible.
  • Mocking the chain extension is not intuitive, and writing unit tests with it is hard enough. It is impossible to use some tools for unit testing like mockall.
  • The chain extension is defined as a trait but should be used as a struct. It causes misunderstanding sometimes on how to use it.
  • IDEs don't provide hinting or highlighting during interaction with the chain extension via self.env().extension()... syntax.
  • The default behavior for the handle_status is true, and it secretly changes the output type of the method from O to Result<O, Self::ErrorCode>.
  • The definition of the ErrorCode for status code is required, and it has pre-defined naming.

The solution below tries to solve all those problems and support chain extension in a more "Rust" way. Also untying the chain extension from the contract make us closer to the #259. Also solves #1719.

Potential Solution

The solution proposes to make several changes in the API. Each section contains a description of the proposal.

Default behavior of the handle_status

The processing of the status code from the chain extension call is more about performance optimization than development. Because, from the development perspective, it is easy to return Result<Output, substrate::ChainExtensionError> with an error from the substrate level than the status code - u32. Because sometimes, errors can have some fields with data useful for debugging or business logic.

Because of that, the proposal is to change the default behavior for the handle_status from true to false and make the ErrorCode optional. It also requires the introduction of a new attribute like #[ink(status_error)](we can decide about the naming here or in the pull request). But the user can define the name for the associated type by himself.

Simplifying the chain extension definition will make it more user-friendly for beginners. But processing the status code for more advanced users is still possible.

Remove the chain extension from the Environment trait

The chain extension doesn't use any features from the environment and may live as a standalone type.

  1. It is a part of the environment to simplify the usage for the end user. The current implementation instantiates the empty structure and returns it to the user by self.env().extension().

  2. Another reason is that the #[ink::chain_extension] creates a struct instead of the trait. If the user works with the type directly, he will have the wrong IDE highlighting with weird errors(because he works with trait on IDE level), but it will compile(because during compilation, it would be a struct).

  3. Because the user gets the chain extension from the self.env().extension(), we must replace the default chain extension with mocked in the unit tests. For that, we have the register_chain_extension function. All that makes mocking the chain extension as some "dark magic" fully unclear for the user.2.

The proposal is to remove the ChainExtension from the Environment and move responsibility to instantiate the chain extension to the user. It will add an additional step on the user's side but will be more Rust way and remove a lot of limitations.

It solves many problems:

  • The user will have highlighting and hinting in the IDE for chain extension methods(except for weird errors from point 2, but the last proposal will solve it).
  • It is possible to use an unlimited number of chain extensions in any code(inside of the contract or outside).
  • The mocking of the chain extension is the user's responsibility and depends on how the user instantiates the chain extension. It allows using all available tools for unit test writing.
  • With this issue and Rework the Environment trait #1303 we can totally remove the Environment trait and make the development of ink! and smart contracts much simpler.

It is easy to instantiate the chain extension. The user can always add the fn extension() function that returns the chain extension and use it everywhere in the code. The usage inside of the contract looks like Self::random_extension().

#[cfg(not(test))]
fn random_extension() -> RandomChainExtension {
    RandomChainExtension {}
}

#[cfg(test)]
fn random_extension() -> MockedRandomChainExtension {
    MockedRandomChainExtension {}
}

Use the trait instead of the struct

Currently, ink! removes the trait and replaces it with the structure that has the same name as a trait and the same list of methods with an implementation like:

#( #attrs )*
#[inline]
pub fn #ident(self, #inputs) -> #return_type
where
    #where_output_impls_from_error_code
{
    ::ink::env::chain_extension::ChainExtensionMethod::build(#func_id)
    .input::<#compound_input_type>()
    .output::<#output_type, {::ink::is_result_type!(#output_type)}>()
    #error_code_handling
    .call(&#compound_input_bindings)
}

The proposal is to use the trait with a default implementation for methods instead of structure. It means the user should do one more step before using the chain extension. He needs to implement the trait for some structure via:

// It is how the user defines the trait for chain extension.
// #[ink::chain_extension]
// pub trait RandomExtension {
//     // #[ink(extension = 1101)]
//     fn fetch_random(&self) -> [u8; 32];
// }

// It is how the macro will be expanded.
pub trait RandomExtension {
    fn fetch_random(&self) -> [u8; 32] {
        ::ink::env::chain_extension::ChainExtensionMethod::build(1101)
        .input::<#compound_input_type>()
        .output::<#output_type, {::ink::is_result_type!(#output_type)}>()
        #error_code_handling
        .call(&#compound_input_bindings)
    }
}

struct ChainExtentionImplementer;

impl RandomExtension for ChainExtentionImplementer {}

It allows the implementation of several chain extensions for the same type; easy mock methods for unit tests because we can override the default implementation; import chain extensions from different crates; write generic code/tests; hinting and highlighting will work in the IDE.

Example of the usage

You can find an example of how the usage will look in the playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant