Add Result<Self, Error> return type for constructors (#1446)

* constructors return Err abort transaction + started example

* return type parser

* add output type to dispatchable constructor

* codegen adds Result to return type

* add type cheking

* fix CALLABLE type

* add comments for refactoring

* ugly solution for failable contracts

* avoid unnecessary error check for constructors

* avoids encoding storage, adds demo example

* allows differents syntax format of result return type

* cargo fmt

* add changes to CHANGELOG.md

* add tests to the example

* add UI tests

* return comment to dns example

* fix formatting in UI examples

* fix tests and docs

* apply formatting suggestions

* change example and UI tests for more clarity

* add imports for UI test

* refactor execution.rs

* remove syntactical check for constructors return type

* UI test for type aliases

* LLVM optimisation and refactoring

* updated docs

* refactoring and moving feature demo to mother contract

* remove cargo-contract install in docker

.gitlab-ci.yml

@@ -379,7 +379,6 @@ examples-contract-build:
   <<:                              *test-refs
   script:
     - rustup component add rust-src --toolchain stable
-    - cargo install cargo-contract --git https://github.com/paritytech/cargo-contract --version 2.0.0-alpha.4 --force
     - cargo contract -V
     - for example in examples/*/; do
         if [ "$example" = "examples/upgradeable-contracts/" ]; then continue; fi;

CHANGELOG.md

@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Allows to use `Result<Self, Error>` as a return type in constructors - [#1446](https://github.com/paritytech/ink/pull/1446)
 
 ## Version 4.0.0-alpha.3
 
@@ -22,7 +23,7 @@ crate. All existing sub-crates are reexported and should be used via the new `in
   - Replace all usages of individual crates with reexports, e.g. `ink_env` ➜ `ink::env`.
 
 #### Storage Rework
-[#1331](https://github.com/paritytech/ink/pull/1331) changes the way `ink!` works with contract storage. Storage keys 
+[#1331](https://github.com/paritytech/ink/pull/1331) changes the way `ink!` works with contract storage. Storage keys
 are generated at compile-time, and user facing abstractions which determine how contract data is laid out in storage
 have changed.
 

crates/ink/codegen/src/generator/dispatch.rs

@@ -256,16 +256,19 @@ impl Dispatch<'_> {
                 let payable = constructor.is_payable();
                 let selector_id = constructor.composed_selector().into_be_u32().hex_padded_suffixed();
                 let selector_bytes = constructor.composed_selector().hex_lits();
+                let output_tuple_type = constructor.output().map(quote::ToTokens::to_token_stream)
+                    .unwrap_or_else(|| quote! { () });
                 let input_bindings = generator::input_bindings(constructor.inputs());
                 let input_tuple_type = generator::input_types_tuple(constructor.inputs());
                 let input_tuple_bindings = generator::input_bindings_tuple(constructor.inputs());
                 quote_spanned!(constructor_span=>
                     impl ::ink::reflect::DispatchableConstructorInfo<#selector_id> for #storage_ident {
                         type Input = #input_tuple_type;
+                        type Output = #output_tuple_type;
                         type Storage = #storage_ident;
 
-                        const CALLABLE: fn(Self::Input) -> Self::Storage = |#input_tuple_bindings| {
-                            #storage_ident::#constructor_ident( #( #input_bindings ),* )
+                        const CALLABLE: fn(Self::Input) -> Self::Output = |#input_tuple_bindings| {
+                            #storage_ident::#constructor_ident(#( #input_bindings ),* )
                         };
                         const PAYABLE: ::core::primitive::bool = #payable;
                         const SELECTOR: [::core::primitive::u8; 4usize] = [ #( #selector_bytes ),* ];
@@ -559,7 +562,6 @@ impl Dispatch<'_> {
                     }>>::IDS[#index]
                 }>>::PAYABLE
             );
-
             quote_spanned!(constructor_span=>
                 Self::#constructor_ident(input) => {
                     if #any_constructor_accept_payment && #deny_payment {

crates/ink/codegen/src/generator/item_impls.rs

@@ -239,10 +239,11 @@ impl ItemImpls<'_> {
         let ident = constructor.ident();
         let inputs = constructor.inputs();
         let statements = constructor.statements();
+        let output = constructor.output();
         quote_spanned!(span =>
             #( #attrs )*
             #[cfg(not(feature = "__ink_dylint_Constructor"))]
-            #vis fn #ident( #( #inputs ),* ) -> Self {
+            #vis fn #ident( #( #inputs ),* ) -> #output {
                 #( #statements )*
             }
         )

crates/ink/ir/src/ir/item_impl/constructor.rs

@@ -84,40 +84,18 @@ impl quote::ToTokens for Constructor {
 }
 
 impl Constructor {
-    /// Returns `true` if the given type is `Self`.
-    fn type_is_self_val(ty: &syn::Type) -> bool {
-        matches!(ty, syn::Type::Path(syn::TypePath {
-            qself: None,
-            path
-        }) if path.is_ident("Self"))
-    }
-
-    /// Ensures that the return type of the ink! constructor is `Self`.
-    ///
-    /// Returns an appropriate error otherwise.
+    /// Ensure that the constructor has a return.
+    /// Returns an error otherwise.
     ///
     /// # Errors
     ///
-    /// If the ink! constructor does not return `Self` or is missing a return
-    /// type entirely.
-    fn ensure_valid_return_type(
-        method_item: &syn::ImplItemMethod,
-    ) -> Result<(), syn::Error> {
-        match &method_item.sig.output {
-            syn::ReturnType::Default => {
-                return Err(format_err_spanned!(
-                    &method_item.sig,
-                    "missing return for ink! constructor",
-                ))
-            }
-            syn::ReturnType::Type(_, return_type) => {
-                if !Self::type_is_self_val(return_type.as_ref()) {
-                    return Err(format_err_spanned!(
-                        return_type,
-                        "ink! constructors must return Self",
-                    ))
-                }
-            }
+    /// If the ink! constructor is missing a return type.
+    fn ensure_return(method_item: &syn::ImplItemMethod) -> Result<(), syn::Error> {
+        if let syn::ReturnType::Default = &method_item.sig.output {
+            return Err(format_err_spanned!(
+                &method_item.sig,
+                "missing return for ink! constructor",
+            ))
         }
         Ok(())
     }
@@ -172,7 +150,7 @@ impl TryFrom<syn::ImplItemMethod> for Constructor {
 
     fn try_from(method_item: syn::ImplItemMethod) -> Result<Self, Self::Error> {
         ensure_callable_invariants(&method_item, CallableKind::Constructor)?;
-        Self::ensure_valid_return_type(&method_item)?;
+        Self::ensure_return(&method_item)?;
         Self::ensure_no_self_receiver(&method_item)?;
         let (ink_attrs, other_attrs) = Self::sanitize_attributes(&method_item)?;
         let is_payable = ink_attrs.is_payable();
@@ -241,6 +219,14 @@ impl Constructor {
     pub fn attrs(&self) -> &[syn::Attribute] {
         &self.item.attrs
     }
+
+    /// Returns the return type of the ink! constructor if any.
+    pub fn output(&self) -> Option<&syn::Type> {
+        match &self.item.sig.output {
+            syn::ReturnType::Default => None,
+            syn::ReturnType::Type(_, return_type) => Some(return_type),
+        }
+    }
 }
 
 #[cfg(test)]
@@ -390,6 +376,11 @@ mod tests {
                 #[ink(constructor)]
                 fn my_constructor(input1: i32, input2: i64, input3: u32, input4: u64) -> Self {}
             },
+            // Result return type
+            syn::parse_quote! {
+                #[ink(constructor)]
+                pub fn my_constructor() -> Result<Self, ()> {}
+            },
         ];
         for item_method in item_methods {
             assert!(<ir::Constructor as TryFrom<_>>::try_from(item_method).is_ok());
@@ -421,31 +412,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn try_from_invalid_return_fails() {
-        let item_methods: Vec<syn::ImplItemMethod> = vec![
-            syn::parse_quote! {
-                #[ink(constructor)]
-                fn my_constructor() -> &Self {}
-            },
-            syn::parse_quote! {
-                #[ink(constructor)]
-                pub fn my_constructor() -> &mut Self {}
-            },
-            syn::parse_quote! {
-                #[ink(constructor)]
-                pub fn my_constructor() -> i32 {}
-            },
-            syn::parse_quote! {
-                #[ink(constructor)]
-                pub fn my_constructor() -> Result<Self, ()> {}
-            },
-        ];
-        for item_method in item_methods {
-            assert_try_from_fails(item_method, "ink! constructors must return Self")
-        }
-    }
-
     #[test]
     fn try_from_invalid_self_receiver_fails() {
         let item_methods: Vec<syn::ImplItemMethod> = vec![

crates/ink/src/codegen/dispatch/execution.rs

@@ -16,10 +16,7 @@ use crate::reflect::{
     ContractEnv,
     DispatchError,
 };
-use core::{
-    convert::Infallible,
-    mem::ManuallyDrop,
-};
+use core::mem::ManuallyDrop;
 use ink_env::{
     Environment,
     ReturnFlags,
@@ -58,7 +55,7 @@ pub fn execute_constructor<Contract, F, R>(f: F) -> Result<(), DispatchError>
 where
     Contract: Storable + StorageKey + ContractEnv,
     F: FnOnce() -> R,
-    <private::Seal<R> as ConstructorReturnType<Contract>>::ReturnValue: Encode,
+    <private::Seal<R> as ConstructorReturnType<Contract>>::Error: Encode,
     private::Seal<R>: ConstructorReturnType<Contract>,
 {
     let result = ManuallyDrop::new(private::Seal(f()));
@@ -73,16 +70,13 @@ where
             );
             Ok(())
         }
-        Err(_) => {
+        Err(error) => {
             // Constructor is fallible and failed.
             //
             // We need to revert the state of the transaction.
             ink_env::return_value::<
-                <private::Seal<R> as ConstructorReturnType<Contract>>::ReturnValue,
-            >(
-                ReturnFlags::default().set_reverted(true),
-                result.return_value(),
-            )
+                <private::Seal<R> as ConstructorReturnType<Contract>>::Error,
+            >(ReturnFlags::default().set_reverted(true), error)
         }
     }
 }
@@ -119,65 +113,37 @@ pub trait ConstructorReturnType<C>: private::Sealed {
     ///
     /// # Note
     ///
-    /// For infallible constructors this is `core::convert::Infallible`.
-    type Error;
-
-    /// The type of the return value of the constructor.
-    ///
-    /// # Note
-    ///
     /// For infallible constructors this is `()` whereas for fallible
-    /// constructors this is the actual return value. Since we only ever
-    /// return a value in case of `Result::Err` the `Result::Ok` value
+    /// constructors this is the actual return error type. Since we only ever
+    /// return a value in case of `Result::Err` the `Result::Ok` value type
     /// does not matter.
-    type ReturnValue;
+    type Error;
 
     /// Converts the return value into a `Result` instance.
     ///
     /// # Note
     ///
     /// For infallible constructor returns this always yields `Ok`.
     fn as_result(&self) -> Result<&C, &Self::Error>;
-
-    /// Returns the actual return value of the constructor.
-    ///
-    /// # Note
-    ///
-    /// For infallible constructor returns this always yields `()`
-    /// and is basically ignored since this does not get called
-    /// if the constructor did not fail.
-    fn return_value(&self) -> &Self::ReturnValue;
 }
 
 impl<C> ConstructorReturnType<C> for private::Seal<C> {
-    type Error = Infallible;
-    type ReturnValue = ();
+    type Error = ();
 
-    #[inline]
+    #[inline(always)]
     fn as_result(&self) -> Result<&C, &Self::Error> {
         Ok(&self.0)
     }
-
-    #[inline]
-    fn return_value(&self) -> &Self::ReturnValue {
-        &()
-    }
 }
 
 impl<C, E> ConstructorReturnType<C> for private::Seal<Result<C, E>> {
     const IS_RESULT: bool = true;
     type Error = E;
-    type ReturnValue = Result<C, E>;
 
-    #[inline]
+    #[inline(always)]
     fn as_result(&self) -> Result<&C, &Self::Error> {
         self.0.as_ref()
     }
-
-    #[inline]
-    fn return_value(&self) -> &Self::ReturnValue {
-        &self.0
-    }
 }
 
 /// Trait used to convert return types of contract initializer routines.

crates/ink/src/reflect/dispatch.rs

@@ -330,9 +330,11 @@ pub trait DispatchableConstructorInfo<const ID: u32> {
     type Input;
     /// The ink! storage struct type.
     type Storage;
+    /// Reflects the output type of the dispatchable ink! message.
+    type Output;
 
     /// The closure that can be used to dispatch into the dispatchable ink! constructor.
-    const CALLABLE: fn(Self::Input) -> Self::Storage;
+    const CALLABLE: fn(Self::Input) -> Self::Output;
 
     /// Yields `true` if the dispatchable ink! constructor is payable.
     const PAYABLE: bool;

crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs

@@ -0,0 +1,23 @@
+#[ink::contract]
+mod contract {
+    #[ink(storage)]
+    pub struct Contract {}
+
+    #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
+    #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
+    pub enum Error {
+        Foo,
+    }
+
+    impl Contract {
+        #[ink(constructor)]
+        pub fn constructor() -> Result<u8, Error> {
+            Ok(5_u8)
+        }
+
+        #[ink(message)]
+        pub fn message(&self) {}
+    }
+}
+
+fn main() {}

crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr

@@ -0,0 +1,14 @@
+error[E0277]: the trait bound `codegen::dispatch::execution::private::Seal<Result<u8, contract::Error>>: codegen::dispatch::execution::ConstructorReturnType<Contract>` is not satisfied
+  --> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:9
+   |
+14 |         pub fn constructor() -> Result<u8, Error> {
+   |         ^^^ the trait `codegen::dispatch::execution::ConstructorReturnType<Contract>` is not implemented for `codegen::dispatch::execution::private::Seal<Result<u8, contract::Error>>`
+   |
+   = help: the following other types implement trait `codegen::dispatch::execution::ConstructorReturnType<C>`:
+             codegen::dispatch::execution::private::Seal<C>
+             codegen::dispatch::execution::private::Seal<Result<C, E>>
+note: required by a bound in `execute_constructor`
+  --> src/codegen/dispatch/execution.rs
+   |
+   |     private::Seal<R>: ConstructorReturnType<Contract>,
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `execute_constructor`

crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs

@@ -0,0 +1,22 @@
+#[ink::contract]
+mod contract {
+    #[ink(storage)]
+    pub struct Contract {}
+
+    #[derive(Debug, PartialEq, Eq)]
+    pub enum Error {
+        Foo,
+    }
+
+    impl Contract {
+        #[ink(constructor)]
+        pub fn constructor() -> Result<Self, Error> {
+            Ok(Self {})
+        }
+
+        #[ink(message)]
+        pub fn message(&self) {}
+    }
+}
+
+fn main() {}

crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr

@@ -0,0 +1,22 @@
+error[E0277]: the trait bound `contract::Error: WrapperTypeEncode` is not satisfied
+  --> tests/ui/contract/fail/constructor-return-result-non-codec-error.rs:13:9
+   |
+13 |         pub fn constructor() -> Result<Self, Error> {
+   |         ^^^ the trait `WrapperTypeEncode` is not implemented for `contract::Error`
+   |
+   = help: the following other types implement trait `WrapperTypeEncode`:
+             &T
+             &mut T
+             Arc<T>
+             Box<T>
+             Cow<'a, T>
+             Rc<T>
+             String
+             Vec<T>
+             parity_scale_codec::Ref<'a, T, U>
+   = note: required because of the requirements on the impl of `Encode` for `contract::Error`
+note: required by a bound in `execute_constructor`
+  --> src/codegen/dispatch/execution.rs
+   |
+   |     <private::Seal<R> as ConstructorReturnType<Contract>>::Error: Encode,
+   |                                                                   ^^^^^^ required by this bound in `execute_constructor`