Return LangErrors from constructors

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

@@ -427,7 +427,7 @@ impl Dispatch<'_> {
                         decoded_dispatchable
                     }
                     ::core::result::Result::Err(_decoding_error) => {
-                        let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput);
+                        let error = ::ink::ConstructorResult::Err(::ink::LangError::CouldNotReadInput);
 
                         // At this point we're unable to set the `Ok` variant to be the any "real"
                         // constructor output since we were unable to figure out what the caller wanted
@@ -465,7 +465,7 @@ impl Dispatch<'_> {
                         decoded_dispatchable
                     }
                     ::core::result::Result::Err(_decoding_error) => {
-                        let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput);
+                        let error = ::ink::MessageResult::Err(::ink::LangError::CouldNotReadInput);
 
                         // At this point we're unable to set the `Ok` variant to be the any "real"
                         // message output since we were unable to figure out what the caller wanted
@@ -623,34 +623,25 @@ impl Dispatch<'_> {
 
                     let result: #constructor_output = #constructor_callable(input);
                     let output_value = ::ink::reflect::ConstructorOutputValue::new(result);
+                    let output_result = #constructor_value::as_result(&output_value);
 
-                    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::<::ink::ConstructorResult<::core::result::Result<(), ()>>>(
-                                    ::ink::env::ReturnFlags::new_with_reverted(false),
-                                    &::core::result::Result::Ok(::core::result::Result::Ok(())),
-                                )
-                            } else {
-                                ::ink::env::return_value::<::ink::ConstructorResult<()>>(
-                                    ::ink::env::ReturnFlags::new_with_reverted(false),
-                                    &::core::result::Result::Ok(()),
-                                )
-                            }
-                        }
-                        ::core::result::Result::Err(err) => ::ink::env::return_value::<
-                            ::ink::ConstructorResult<::core::result::Result<(), &#constructor_value::Error>>,
-                        >(
-                            ::ink::env::ReturnFlags::new_with_reverted(false),
-                            &::core::result::Result::Ok(::core::result::Result::Err(err)),
-                        ),
+                    if let ::core::result::Result::Ok(contract) = output_result.as_ref() {
+                        ::ink::env::set_contract_storage::<::ink::primitives::Key, #storage_ident>(
+                            &<#storage_ident as ::ink::storage::traits::StorageKey>::KEY,
+                            contract,
+                        );
                     }
+
+                    ::ink::env::return_value::<
+                        ::ink::ConstructorResult<
+                            ::core::result::Result<(), &#constructor_value::Error>
+                        >,
+                    >(
+                        ::ink::env::ReturnFlags::new_with_reverted(output_result.is_err()),
+                        // Currently no `LangError`s are raised at this level of the
+                        // dispatch logic so `Ok` is always returned to the caller.
+                        &::ink::ConstructorResult::Ok(output_result.map(|_| ())),
+                    );
                 }
             )
         });
@@ -835,27 +826,19 @@ impl Dispatch<'_> {
                     }
 
                     let result: #message_output = #message_callable(&mut contract, input);
-                    let failure = ::ink::is_result_type!(#message_output)
+                    let is_reverted = ::ink::is_result_type!(#message_output)
                         && ::ink::is_result_err!(result);
 
-                    // Currently no `LangError`s are raised at this level of the dispatch logic
-                    // so `Ok` is always returned to the caller.
-                    let return_value = ::core::result::Result::Ok(result);
-
-                    if failure {
-                        // We return early here since there is no need to push back the
-                        // intermediate results of the contract - the transaction is going to be
-                        // reverted anyways.
-                        ::ink::env::return_value::<::ink::MessageResult::<#message_output>>(
-                            ::ink::env::ReturnFlags::new_with_reverted(true),
-                            &return_value
-                        )
+                    // no need to push back results: transaction gets reverted anyways
+                    if !is_reverted {
+                        push_contract(contract, #mutates_storage);
                     }
 
-                    push_contract(contract, #mutates_storage);
-
                     ::ink::env::return_value::<::ink::MessageResult::<#message_output>>(
-                        ::ink::env::ReturnFlags::new_with_reverted(false), &return_value
+                        ::ink::env::ReturnFlags::new_with_reverted(is_reverted),
+                        // Currently no `LangError`s are raised at this level of the
+                        // dispatch logic so `Ok` is always returned to the caller.
+                        &::ink::MessageResult::Ok(result),
                     )
                 }
             )

examples/lang-err-integration-tests/call-builder/Cargo.toml

@@ -11,6 +11,9 @@ ink = { path = "../../../crates/ink", default-features = false }
 scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] }
 scale-info = { version = "2.3", default-features = false, features = ["derive"], optional = true }
 
+constructors_return_value = { path = "../constructors-return-value", default-features = false, features = ["ink-as-dependency"] }
+integration_flipper = { path = "../integration-flipper", default-features = false, features = ["ink-as-dependency"] }
+
 [dev-dependencies]
 ink_e2e = { path = "../../../crates/e2e" }
 
@@ -28,6 +31,9 @@ std = [
     "ink/std",
     "scale/std",
     "scale-info/std",
+
+    "constructors_return_value/std",
+    "integration_flipper/std",
 ]
 ink-as-dependency = []
 e2e-tests = []

examples/lang-err-integration-tests/call-builder/lib.rs

@@ -55,14 +55,49 @@ mod call_builder {
                 }
             }
         }
+
+        #[ink(message)]
+        pub fn call_instantiate(
+            &mut self,
+            code_hash: Hash,
+            selector: [u8; 4],
+            init_value: bool,
+        ) -> Option<AccountId> {
+            use ink::env::{
+                call::{
+                    build_create,
+                    ExecutionInput,
+                    Selector,
+                },
+                DefaultEnvironment,
+            };
+
+            let result = build_create::<
+                DefaultEnvironment,
+                constructors_return_value::ConstructorsReturnValueRef,
+            >()
+            .code_hash(code_hash)
+            .gas_limit(0)
+            .endowment(0)
+            .exec_input(ExecutionInput::new(Selector::new(selector)).push_arg(init_value))
+            .salt_bytes(&[0xDE, 0xAD, 0xBE, 0xEF])
+            .params()
+            .instantiate();
+
+            // NOTE: Right now we can't handle any `LangError` from `instantiate`, we can only tell
+            // that our contract reverted (i.e we see error from the Contracts pallet).
+            result.ok().map(|id| ink::ToAccountId::to_account_id(&id))
+        }
     }
 
     #[cfg(all(test, feature = "e2e-tests"))]
     mod e2e_tests {
         type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
 
-        #[ink_e2e::test(additional_contracts = "../integration-flipper/Cargo.toml")]
-        async fn e2e_invalid_selector_can_be_handled(
+        #[ink_e2e::test(
+            additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml"
+        )]
+        async fn e2e_invalid_message_selector_can_be_handled(
             mut client: ink_e2e::Client<C, E>,
         ) -> E2EResult<()> {
             use call_builder::contract_types::ink_primitives::{
@@ -137,5 +172,103 @@ mod call_builder {
 
             Ok(())
         }
+
+        #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")]
+        async fn e2e_create_builder_works_with_valid_selector(
+            mut client: ink_e2e::Client<C, E>,
+        ) -> E2EResult<()> {
+            let constructor = call_builder::constructors::new();
+            let contract_acc_id = client
+                .instantiate(&mut ink_e2e::dave(), constructor, 0, None)
+                .await
+                .expect("instantiate failed")
+                .account_id;
+
+            let code_hash = client
+                .upload(
+                    &mut ink_e2e::dave(),
+                    constructors_return_value::CONTRACT_PATH,
+                    None,
+                )
+                .await
+                .expect("upload `constructors_return_value` failed")
+                .code_hash;
+
+            let new_selector = [0x9B, 0xAE, 0x9D, 0x5E];
+            let call_result = client
+                .call(
+                    &mut ink_e2e::dave(),
+                    contract_acc_id.clone(),
+                    call_builder::messages::call_instantiate(
+                        ink_e2e::utils::runtime_hash_to_ink_hash::<
+                            ink::env::DefaultEnvironment,
+                        >(&code_hash),
+                        new_selector,
+                        true,
+                    ),
+                    0,
+                    None,
+                )
+                .await
+                .expect("Client failed to call `call_builder::call_instantiate`.")
+                .value
+                .expect("Dispatching `call_builder::call_instantiate` failed.");
+
+            assert!(
+                call_result.is_some(),
+                "Call using valid selector failed, when it should've succeeded."
+            );
+
+            Ok(())
+        }
+
+        #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")]
+        async fn e2e_create_builder_fails_with_invalid_selector(
+            mut client: ink_e2e::Client<C, E>,
+        ) -> E2EResult<()> {
+            let constructor = call_builder::constructors::new();
+            let contract_acc_id = client
+                .instantiate(&mut ink_e2e::eve(), constructor, 0, None)
+                .await
+                .expect("instantiate failed")
+                .account_id;
+
+            let code_hash = client
+                .upload(
+                    &mut ink_e2e::eve(),
+                    constructors_return_value::CONTRACT_PATH,
+                    None,
+                )
+                .await
+                .expect("upload `constructors_return_value` failed")
+                .code_hash;
+
+            let invalid_selector = [0x00, 0x00, 0x00, 0x00];
+            let call_result = client
+                .call(
+                    &mut ink_e2e::eve(),
+                    contract_acc_id.clone(),
+                    call_builder::messages::call_instantiate(
+                        ink_e2e::utils::runtime_hash_to_ink_hash::<
+                            ink::env::DefaultEnvironment,
+                        >(&code_hash),
+                        invalid_selector,
+                        true,
+                    ),
+                    0,
+                    None,
+                )
+                .await
+                .expect("Client failed to call `call_builder::call_instantiate`.")
+                .value
+                .expect("Dispatching `call_builder::call_instantiate` failed.");
+
+            assert!(
+                call_result.is_none(),
+                "Call using invalid selector succeeded, when it should've failed."
+            );
+
+            Ok(())
+        }
     }
 }

examples/lang-err-integration-tests/constructors-return-value/lib.rs

@@ -208,6 +208,15 @@ pub mod constructors_return_value {
                 "Fallible constructor should have failed"
             );
 
+            let result = client
+                .instantiate(&mut ink_e2e::charlie(), constructor, 0, None)
+                .await;
+
+            assert!(
+                matches!(result, Err(ink_e2e::Error::InstantiateExtrinsic(_))),
+                "Constructor should fail"
+            );
+
             Ok(())
         }
     }