Rework fallible constructors (#1491)

* Remove unused trait `InitializerReturnType`

* WIP experimenting with static constructor return types

* WIP

* WIP generating metadata at compile time

* WIP using constructor result type

* Use generated method for metadata

* Generate constructor info using static types

* WIP restoring dispatch

* WIP adding metadata test to ui test

* Fix match as_result

* Attempt to infer result type

* Constructor dispatch compiles, remove metadata syntax stuff

* Fmt

* Remove unnecessary braces

* Fix up execution for results

* Fix up tests

* Remove printlns

* Explicitly set reverted flag

* ReturnTypeSpec accurately reflects ABI

* Polished refactor for constructor return type (#1500)

* revert instantiating when error

* display name + correct return typ when Self

* fix dereferencing issue

* UI test error variant + cleanup

* cargo fmt

* cargo fmt

* fix another reference issue

* fix spec contract test

Co-authored-by: Andrew Jones <ascjones@gmail.com>

* explicit prelude + fixing test error messages (#1503)

* Add constructor to ReturnFlags to not require Default trait impl

* Only invoke return_value for fallible constructors

* Add e2e fallible constructor value tests

* Fmt

* Add some reflection tests for constructors returning an error

* Check reflection for return types in UI tests

* Check reflection in example

* Fmt

* Add test for fallible constructor succeeding

* Add test for fallible constructor failing

* Remove unused message

* Fully qualify Ok(())

Co-authored-by: German <german@parity.io>

Author: ascjones

crates/e2e/src/client.rs

@@ -321,6 +321,34 @@ where
         Ok(ret)
     }
 
+    /// Dry run contract instantiation using the given constructor.
+    pub async fn instantiate_dry_run<CO: InkConstructor>(
+        &mut self,
+        signer: &Signer<C>,
+        constructor: &CO,
+        value: E::Balance,
+        storage_deposit_limit: Option<E::Balance>,
+    ) -> ContractInstantiateResult<C::AccountId, E::Balance>
+    where
+        CO: InkConstructor,
+    {
+        let mut data = CO::SELECTOR.to_vec();
+        <CO as scale::Encode>::encode_to(constructor, &mut data);
+
+        let code = crate::utils::extract_wasm(CO::CONTRACT_PATH);
+        let salt = Self::salt();
+        self.api
+            .instantiate_with_code_dry_run(
+                value,
+                storage_deposit_limit,
+                code.clone(),
+                data.clone(),
+                salt.clone(),
+                signer,
+            )
+            .await
+    }
+
     /// Executes an `instantiate_with_code` call and captures the resulting events.
     async fn exec_instantiate<CO: InkConstructor>(
         &mut self,
@@ -337,13 +365,7 @@ where
         ));
         <CO as scale::Encode>::encode_to(constructor, &mut data);
 
-        let salt = std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .unwrap_or_else(|err| panic!("unable to get unix time: {}", err))
-            .as_millis()
-            .as_u128()
-            .to_le_bytes()
-            .to_vec();
+        let salt = Self::salt();
 
         // dry run the instantiate to calculate the gas limit
         let dry_run = self
@@ -429,6 +451,17 @@ where
         })
     }
 
+    /// Generate a unique salt based on the system time.
+    fn salt() -> Vec<u8> {
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap_or_else(|err| panic!("unable to get unix time: {}", err))
+            .as_millis()
+            .as_u128()
+            .to_le_bytes()
+            .to_vec()
+    }
+
     /// This function extracts the metadata of the contract at the file path
     /// `target/ink/$contract_name.contract`.
     ///

crates/env/src/backend.rs

@@ -36,6 +36,11 @@ pub struct ReturnFlags {
 }
 
 impl ReturnFlags {
+    /// Initialize [`ReturnFlags`] with the reverted flag.
+    pub fn new_with_reverted(has_reverted: bool) -> Self {
+        Self::default().set_reverted(has_reverted)
+    }
+
     /// Sets the bit to indicate that the execution is going to be reverted.
     #[must_use]
     pub fn set_reverted(mut self, has_reverted: bool) -> Self {

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

@@ -256,16 +256,22 @@ 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)
+                let output_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());
+                let constructor_return_type = quote_spanned!(constructor_span=>
+                    <::ink::reflect::ConstructorOutputValue<#output_type>
+                        as ::ink::reflect::ConstructorOutput<#storage_ident>>
+                );
                 quote_spanned!(constructor_span=>
                     impl ::ink::reflect::DispatchableConstructorInfo<#selector_id> for #storage_ident {
                         type Input = #input_tuple_type;
-                        type Output = #output_tuple_type;
+                        type Output = #output_type;
                         type Storage = #storage_ident;
+                        type Error = #constructor_return_type :: Error;
+                        const IS_RESULT: ::core::primitive::bool = #constructor_return_type :: IS_RESULT;
 
                         const CALLABLE: fn(Self::Input) -> Self::Output = |#input_tuple_bindings| {
                             #storage_ident::#constructor_ident(#( #input_bindings ),* )
@@ -442,7 +448,6 @@ impl Dispatch<'_> {
                         decoded_dispatchable
                     }
                     ::core::result::Result::Err(_decoding_error) => {
-                        use ::core::default::Default;
                         let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput);
 
                         // At this point we're unable to set the `Ok` variant to be the any "real"
@@ -452,7 +457,7 @@ impl Dispatch<'_> {
                         // This is okay since we're going to only be encoding the `Err` variant
                         // into the output buffer anyways.
                         ::ink::env::return_value::<::ink::MessageResult<()>>(
-                            ::ink::env::ReturnFlags::default().set_reverted(true),
+                            ::ink::env::ReturnFlags::new_with_reverted(true),
                             &error,
                         );
                     }
@@ -573,23 +578,58 @@ impl Dispatch<'_> {
                     }>>::IDS[#index]
                 }>>::CALLABLE
             );
+            let constructor_output = quote_spanned!(constructor_span=>
+                <#storage_ident as ::ink::reflect::DispatchableConstructorInfo<{
+                    <#storage_ident as ::ink::reflect::ContractDispatchableConstructors<{
+                        <#storage_ident as ::ink::reflect::ContractAmountDispatchables>::CONSTRUCTORS
+                    }>>::IDS[#index]
+                }>>::Output
+            );
             let deny_payment = quote_spanned!(constructor_span=>
                 !<#storage_ident as ::ink::reflect::DispatchableConstructorInfo<{
                     <#storage_ident as ::ink::reflect::ContractDispatchableConstructors<{
                         <#storage_ident as ::ink::reflect::ContractAmountDispatchables>::CONSTRUCTORS
                     }>>::IDS[#index]
                 }>>::PAYABLE
             );
+            let constructor_value = quote_spanned!(constructor_span=>
+                <::ink::reflect::ConstructorOutputValue<#constructor_output>
+                    as ::ink::reflect::ConstructorOutput::<#storage_ident>>
+            );
+
             quote_spanned!(constructor_span=>
                 Self::#constructor_ident(input) => {
                     if #any_constructor_accept_payment && #deny_payment {
                         ::ink::codegen::deny_payment::<
                             <#storage_ident as ::ink::reflect::ContractEnv>::Env>()?;
                     }
 
-                    ::ink::codegen::execute_constructor::<#storage_ident, _, _>(
-                        move || { #constructor_callable(input) }
-                    )
+                    let result: #constructor_output = #constructor_callable(input);
+                    let output_value = ::ink::reflect::ConstructorOutputValue::new(result);
+
+                    match #constructor_value :: as_result(&output_value) {
+                        ::core::result::Result::Ok(contract) => {
+                            ::ink::env::set_contract_storage::<::ink::primitives::Key, #storage_ident>(
+                                &<#storage_ident as ::ink::storage::traits::StorageKey>::KEY,
+                                contract,
+                            );
+                            // only fallible constructors return success `Ok` back to the caller.
+                            if #constructor_value :: IS_RESULT {
+                                ::ink::env::return_value::<::core::result::Result<&(), ()>>(
+                                    ::ink::env::ReturnFlags::new_with_reverted(false),
+                                    &::core::result::Result::Ok(&())
+                                )
+                            } else {
+                                ::core::result::Result::Ok(())
+                            }
+                        },
+                        ::core::result::Result::Err(err) => {
+                           ::ink::env::return_value::<::core::result::Result<(), & #constructor_value :: Error>>(
+                                ::ink::env::ReturnFlags::new_with_reverted(true),
+                                &::core::result::Result::Err(err)
+                            )
+                        }
+                    }
                 }
             )
         });
@@ -768,8 +808,6 @@ impl Dispatch<'_> {
 
             quote_spanned!(message_span=>
                 Self::#message_ident(input) => {
-                    use ::core::default::Default;
-
                     if #any_message_accept_payment && #deny_payment {
                         ::ink::codegen::deny_payment::<
                             <#storage_ident as ::ink::reflect::ContractEnv>::Env>()?;
@@ -788,14 +826,15 @@ impl Dispatch<'_> {
                         // intermediate results of the contract - the transaction is going to be
                         // reverted anyways.
                         ::ink::env::return_value::<::ink::MessageResult::<#message_output>>(
-                            ::ink::env::ReturnFlags::default().set_reverted(true), &return_value
+                            ::ink::env::ReturnFlags::new_with_reverted(true),
+                            &return_value
                         )
                     }
 
                     push_contract(contract, #mutates_storage);
 
                     ::ink::env::return_value::<::ink::MessageResult::<#message_output>>(
-                        ::ink::env::ReturnFlags::default(), &return_value
+                        ::ink::env::ReturnFlags::new_with_reverted(false), &return_value
                     )
                 }
             )