Return LangErrors from constructors

.config/cargo_spellcheck.dic

@@ -46,6 +46,7 @@ evaluable
 fuzzer
 getter
 growable
+ident
 interoperate
 invariants
 kB

crates/e2e/src/client.rs

@@ -341,9 +341,9 @@ where
             .instantiate_with_code_dry_run(
                 value,
                 storage_deposit_limit,
-                code.clone(),
-                data.clone(),
-                salt.clone(),
+                code,
+                data,
+                salt,
                 signer,
             )
             .await

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

@@ -270,8 +270,8 @@ impl Dispatch<'_> {
                         type Input = #input_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;
+                        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 ),* )
@@ -420,16 +420,33 @@ impl Dispatch<'_> {
                         .unwrap_or_else(|error| ::core::panic!("{}", error))
                 }
 
-                ::ink::env::decode_input::<
-                        <#storage_ident as ::ink::reflect::ContractConstructorDecoder>::Type>()
-                    .map_err(|_| ::ink::reflect::DispatchError::CouldNotReadInput)
-                    .and_then(|decoder| {
-                        <<#storage_ident as ::ink::reflect::ContractConstructorDecoder>::Type
-                            as ::ink::reflect::ExecuteDispatchable>::execute_dispatchable(decoder)
-                    })
-                    .unwrap_or_else(|error| {
-                        ::core::panic!("dispatching ink! constructor failed: {}", error)
-                    })
+                let dispatchable = match ::ink::env::decode_input::<
+                    <#storage_ident as ::ink::reflect::ContractConstructorDecoder>::Type,
+                >() {
+                    ::core::result::Result::Ok(decoded_dispatchable) => {
+                        decoded_dispatchable
+                    }
+                    ::core::result::Result::Err(_decoding_error) => {
+                        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"
+                        // constructor output since we were unable to figure out what the caller wanted
+                        // to dispatch in the first place, so we set it to `()`.
+                        //
+                        // This is okay since we're going to only be encoding the `Err` variant
+                        // into the output buffer anyways.
+                        ::ink::env::return_value::<::ink::ConstructorResult<()>>(
+                            ::ink::env::ReturnFlags::new_with_reverted(true),
+                            &error,
+                        );
+                    }
+                };
+
+                <<#storage_ident as ::ink::reflect::ContractConstructorDecoder>::Type
+                    as ::ink::reflect::ExecuteDispatchable>::execute_dispatchable(dispatchable)
+                .unwrap_or_else(|error| {
+                    ::core::panic!("dispatching ink! message failed: {}", error)
+                })
             }
 
             #[cfg(not(test))]
@@ -607,28 +624,32 @@ impl Dispatch<'_> {
                     let result: #constructor_output = #constructor_callable(input);
                     let output_value = ::ink::reflect::ConstructorOutputValue::new(result);
 
-                    match #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::<::core::result::Result<&(), ()>>(
+                            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(::core::result::Result::Ok(())),
                                 )
                             } else {
-                                ::core::result::Result::Ok(())
+                                ::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::<::core::result::Result<(), & #constructor_value :: Error>>(
-                                ::ink::env::ReturnFlags::new_with_reverted(true),
-                                &::core::result::Result::Err(err)
-                            )
                         }
+                        ::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)),
+                        ),
                     }
                 }
             )

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

@@ -334,7 +334,7 @@ impl Metadata<'_> {
         }
     }
 
-    /// Generates ink! metadata for the storage with given selector and indentation.
+    /// Generates ink! metadata for the storage with given selector and ident.
     fn generate_constructor_return_type(
         storage_ident: &Ident,
         selector_id: u32,
@@ -343,20 +343,28 @@ impl Metadata<'_> {
         let constructor_info = quote_spanned!(span =>
             < #storage_ident as ::ink::reflect::DispatchableConstructorInfo<#selector_id>>
         );
+
         quote_spanned!(span=>
             ::ink::metadata::ReturnTypeSpec::new(
-                if #constructor_info ::IS_RESULT {
+                if #constructor_info::IS_RESULT {
                     ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::<
-                        ::core::result::Result<
+                        ::ink::ConstructorResult<::core::result::Result<
                             (),
-                            #constructor_info ::Error
-                        >
+                            #constructor_info::Error
+                        >>
                     >(
-                        "core::result::Result"
+                        "ink_primitives::ConstructorResult"
                     )
                 )
                 } else {
-                    ::core::option::Option::None
+                    ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::<
+                        ::ink::ConstructorResult<
+                            (),
+                        >
+                    >(
+                        "ink_primitives::ConstructorResult"
+                    )
+                )
                 }
             )
         )

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

@@ -227,6 +227,21 @@ impl Constructor {
             syn::ReturnType::Type(_, return_type) => Some(return_type),
         }
     }
+
+    /// Returns the return type of the constructor, but wrapped within a `Result`.
+    ///
+    /// This is used to to allow callers to handle certain types of errors which are not exposed
+    /// by constructors.
+    pub fn wrapped_output(&self) -> syn::Type {
+        let return_type = self
+            .output()
+            .map(quote::ToTokens::to_token_stream)
+            .unwrap_or_else(|| quote::quote! { () });
+
+        syn::parse_quote! {
+            ::ink::ConstructorResult<#return_type>
+        }
+    }
 }
 
 #[cfg(test)]

crates/ink/src/lib.rs

@@ -72,6 +72,7 @@ pub use ink_macro::{
     trait_definition,
 };
 pub use ink_primitives::{
+    ConstructorResult,
     LangError,
     MessageResult,
 };

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

@@ -1,8 +1,8 @@
-error[E0277]: the trait bound `Result<(), &contract::Error>: Encode` is not satisfied
+error[E0277]: the trait bound `Result<Result<(), &contract::Error>, LangError>: Encode` 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 `Encode` is not implemented for `Result<(), &contract::Error>`
+   |         ^^^ the trait `Encode` is not implemented for `Result<Result<(), &contract::Error>, LangError>`
    |
    = help: the trait `Encode` is implemented for `Result<T, E>`
 note: required by a bound in `return_value`
@@ -28,6 +28,8 @@ error[E0277]: the trait bound `contract::Error: TypeInfo` is not satisfied
             (A, B, C, D, E, F)
           and $N others
   = note: required for `Result<(), contract::Error>` to implement `TypeInfo`
+  = note: 1 redundant requirement hidden
+  = note: required for `Result<Result<(), contract::Error>, LangError>` to implement `TypeInfo`
 note: required by a bound in `TypeSpec::with_name_str`
  --> $WORKSPACE/crates/metadata/src/specs.rs
   |

crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs

@@ -58,7 +58,7 @@ fn main() {
     assert_eq!("constructor", constructor.label());
     let type_spec = constructor.return_type().opt_type().unwrap();
     assert_eq!(
-        "core::result::Result",
+        "ink_primitives::ConstructorResult",
         format!("{}", type_spec.display_name())
     );
     let ty = metadata.registry().resolve(type_spec.ty().id()).unwrap();
@@ -68,16 +68,39 @@ fn main() {
         scale_info::TypeDef::Variant(variant) => {
             assert_eq!(2, variant.variants().len());
 
-            let ok_variant = &variant.variants()[0];
-            let ok_field = &ok_variant.fields()[0];
-            let ok_ty = metadata.registry().resolve(ok_field.ty().id()).unwrap();
+            // Outer Result
+            let outer_ok_variant = &variant.variants()[0];
+            let outer_ok_field = &outer_ok_variant.fields()[0];
+            let outer_ok_ty = metadata
+                .registry()
+                .resolve(outer_ok_field.ty().id())
+                .unwrap();
+            assert_eq!("Ok", outer_ok_variant.name());
+
+            // Inner Result
+            let inner_ok_ty = match outer_ok_ty.type_def() {
+                scale_info::TypeDef::Variant(variant) => {
+                    assert_eq!(2, variant.variants().len());
+
+                    let inner_ok_variant = &variant.variants()[0];
+                    assert_eq!("Ok", inner_ok_variant.name());
+
+                    let inner_ok_field = &inner_ok_variant.fields()[0];
+                    metadata
+                        .registry()
+                        .resolve(inner_ok_field.ty().id())
+                        .unwrap()
+                }
+                td => panic!("Expected a Variant type def enum, got {:?}", td),
+            };
+
             let unit_ty = scale_info::TypeDef::Tuple(
                 scale_info::TypeDefTuple::new_portable(vec![]),
             );
-            assert_eq!("Ok", ok_variant.name());
+
             assert_eq!(
                 &unit_ty,
-                ok_ty.type_def(),
+                inner_ok_ty.type_def(),
                 "Ok variant should be a unit `()` type"
             );
 

crates/primitives/src/lib.rs

@@ -59,3 +59,7 @@ pub enum LangError {
 /// The `Result` type for ink! messages.
 #[doc(hidden)]
 pub type MessageResult<T> = ::core::result::Result<T, LangError>;
+
+/// The `Result` type for ink! constructors.
+#[doc(hidden)]
+pub type ConstructorResult<T> = ::core::result::Result<T, LangError>;

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

@@ -13,7 +13,7 @@ pub mod constructors_return_value {
         value: bool,
     }
 
-    #[derive(scale::Encode, scale::Decode)]
+    #[derive(scale::Encode, scale::Decode, Debug)]
     #[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
     pub struct ConstructorError;
 
@@ -107,9 +107,12 @@ pub mod constructors_return_value {
                 .result
                 .expect("Instantiate dry run should succeed");
 
+            let data = infallible_constructor_result.result.data;
+            let decoded_result = Result::<(), ::ink::LangError>::decode(&mut &data[..])
+                .expect("Failed to decode constructor Result");
             assert!(
-                infallible_constructor_result.result.data.is_empty(),
-                "Infallible constructor should return no data"
+                decoded_result.is_ok(),
+                "Constructor dispatch should have succeeded"
             );
 
             let success = client
@@ -134,13 +137,19 @@ pub mod constructors_return_value {
                 .result
                 .expect("Instantiate dry run should succeed");
 
-            let decoded_result = Result::<(), super::ConstructorError>::decode(
-                &mut &result.result.data[..],
-            )
+            let decoded_result = Result::<
+                Result<(), super::ConstructorError>,
+                ink::LangError,
+            >::decode(&mut &result.result.data[..])
             .expect("Failed to decode fallible constructor Result");
 
             assert!(
                 decoded_result.is_ok(),
+                "Constructor dispatch should have succeeded"
+            );
+
+            assert!(
+                decoded_result.unwrap().is_ok(),
                 "Fallible constructor should have succeeded"
             );
 
@@ -183,21 +192,21 @@ pub mod constructors_return_value {
                 .result
                 .expect("Instantiate dry run should succeed");
 
-            let decoded_result = Result::<(), super::ConstructorError>::decode(
-                &mut &result.result.data[..],
-            )
+            let decoded_result = Result::<
+                Result<(), super::ConstructorError>,
+                ink::LangError,
+            >::decode(&mut &result.result.data[..])
             .expect("Failed to decode fallible constructor Result");
 
             assert!(
-                decoded_result.is_err(),
-                "Fallible constructor should have failed"
+                decoded_result.is_ok(),
+                "Constructor dispatch should have succeeded"
             );
 
-            let result = client
-                .instantiate(&mut ink_e2e::charlie(), constructor, 0, None)
-                .await;
-
-            assert!(result.is_err(), "Constructor should fail");
+            assert!(
+                decoded_result.unwrap().is_err(),
+                "Fallible constructor should have failed"
+            );
 
             Ok(())
         }