From 7a4193bac14e24c23624a2a5d6778ad971b2e2c1 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Thu, 22 Sep 2022 13:08:36 +0100 Subject: [PATCH 01/28] constructors return Err abort transaction + started example --- crates/ink/codegen/src/generator/dispatch.rs | 21 +++++- crates/ink/ir/src/ir/item_impl/constructor.rs | 11 ++- examples/return_err/.gitignore | 9 +++ examples/return_err/Cargo.toml | 35 +++++++++ examples/return_err/lib.rs | 74 +++++++++++++++++++ 5 files changed, 145 insertions(+), 5 deletions(-) create mode 100755 examples/return_err/.gitignore create mode 100755 examples/return_err/Cargo.toml create mode 100755 examples/return_err/lib.rs diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 3566739fa6..8c2368c9c0 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -545,6 +545,13 @@ impl Dispatch<'_> { }>>::IDS[#index] }>>::CALLABLE ); + let constructor_output = quote_spanned!(constructor_span=> + <#storage_ident as ::ink::reflect::DispatchableConstructorInfo<{ + <#storage_ident as ::ink::reflect::ContractDispatchableMessages<{ + <#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<{ @@ -552,7 +559,6 @@ impl Dispatch<'_> { }>>::IDS[#index] }>>::PAYABLE ); - quote_spanned!(constructor_span=> Self::#constructor_ident(input) => { if #any_constructor_accept_payment && #deny_payment { @@ -560,6 +566,19 @@ impl Dispatch<'_> { <#storage_ident as ::ink::reflect::ContractEnv>::Env>()?; } + let result: #constructor_output = #constructor_callable(input); + let failure = ::ink::is_result_type!(#constructor_output) + && ::ink::is_result_err!(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::<#constructor_output>( + ::ink::env::ReturnFlags::default().set_reverted(true), &result + ) + } + ::ink::codegen::execute_constructor::<#storage_ident, _, _>( move || { #constructor_callable(input) } ) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index aa753ca832..a43a787f6f 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -84,15 +84,18 @@ impl quote::ToTokens for Constructor { } impl Constructor { - /// Returns `true` if the given type is `Self`. + /// Returns `true` if the given type is `Self` or `Result` fn type_is_self_val(ty: &syn::Type) -> bool { - matches!(ty, syn::Type::Path(syn::TypePath { + // TODO: clean and parse Result + let res = matches!(ty, syn::Type::Path(syn::TypePath { qself: None, path - }) if path.is_ident("Self")) + }) if path.is_ident("Self") ); + println!("{:?}", ty); + res } - /// Ensures that the return type of the ink! constructor is `Self`. + /// Ensures that the return type of the ink! constructor is `Self` or `Result`. /// /// Returns an appropriate error otherwise. /// diff --git a/examples/return_err/.gitignore b/examples/return_err/.gitignore new file mode 100755 index 0000000000..8de8f877e4 --- /dev/null +++ b/examples/return_err/.gitignore @@ -0,0 +1,9 @@ +# Ignore build artifacts from the local tests sub-crate. +/target/ + +# Ignore backup files creates by cargo fmt. +**/*.rs.bk + +# Remove Cargo.lock when creating an executable, leave it for libraries +# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock +Cargo.lock diff --git a/examples/return_err/Cargo.toml b/examples/return_err/Cargo.toml new file mode 100755 index 0000000000..426d0f586b --- /dev/null +++ b/examples/return_err/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "return_err" +version = "0.1.0" +authors = ["[your_name] <[your_email]>"] +edition = "2021" + +[dependencies] +ink_primitives = { version = "4.0.0-alpha.1", default-features = false } +ink_metadata = { version = "4.0.0-alpha.1", default-features = false, features = ["derive"], optional = true } +ink_env = { version = "4.0.0-alpha.1", default-features = false } +ink_storage = { version = "4.0.0-alpha.1", default-features = false } +ink_lang = { version = "4.0.0-alpha.1", default-features = false } + +scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } +scale-info = { version = "2", default-features = false, features = ["derive"], optional = true } + +[lib] +name = "return_err" +path = "lib.rs" +crate-type = [ + # Used for normal contract Wasm blobs. + "cdylib", +] + +[features] +default = ["std"] +std = [ + "ink_metadata/std", + "ink_env/std", + "ink_storage/std", + "ink_primitives/std", + "scale/std", + "scale-info/std", +] +ink-as-dependency = [] diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs new file mode 100755 index 0000000000..8c10564592 --- /dev/null +++ b/examples/return_err/lib.rs @@ -0,0 +1,74 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +use ink_lang as ink; + +#[ink::contract] +mod return_err { + + /// Defines the storage of your contract. + /// Add new fields to the below struct in order + /// to add new static storage fields to your contract. + #[ink(storage)] + pub struct ReturnErr { + /// Stores a single `bool` value on the storage. + value: bool, + } + + impl ReturnErr { + /// Constructor that initializes the `bool` value to the given `init_value`. + #[ink(constructor)] + pub fn new(bool: false) -> Result { + Result::Err(Error::Foo) + } + + /// Constructor that initializes the `bool` value to `false`. + /// + /// Constructors can delegate to other constructors. + #[ink(constructor)] + pub fn default() -> Self { + Self::new(Default::default()) + } + + /// A message that can be called on instantiated contracts. + /// This one flips the value of the stored `bool` from `true` + /// to `false` and vice versa. + #[ink(message)] + pub fn flip(&mut self) { + self.value = !self.value; + } + + /// Simply returns the current value of our `bool`. + #[ink(message)] + pub fn get(&self) -> bool { + self.value + } + } + + /// Unit tests in Rust are normally defined within such a `#[cfg(test)]` + /// module and test functions are marked with a `#[test]` attribute. + /// The below code is technically just normal Rust code. + #[cfg(test)] + mod tests { + /// Imports all the definitions from the outer scope so we can use them here. + use super::*; + + /// Imports `ink_lang` so we can use `#[ink::test]`. + use ink_lang as ink; + + /// We test if the default constructor does its job. + #[ink::test] + fn default_works() { + let return_err = ReturnErr::default(); + assert_eq!(return_err.get(), false); + } + + /// We test a simple use case of our contract. + #[ink::test] + fn it_works() { + let mut return_err = ReturnErr::new(false); + assert_eq!(return_err.get(), false); + return_err.flip(); + assert_eq!(return_err.get(), true); + } + } +} From e9af561a3a0843e22a5e429332950b19726ea72e Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 25 Sep 2022 20:31:43 +0100 Subject: [PATCH 02/28] return type parser --- crates/ink/ir/src/ir/item_impl/constructor.rs | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index a43a787f6f..077e71a8ac 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -12,21 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{ - ensure_callable_invariants, - Callable, - CallableKind, - InputsIter, - Visibility, -}; -use crate::{ - ir, - ir::attrs::SelectorOrWildcard, -}; -use proc_macro2::{ - Ident, - Span, -}; +use super::{ensure_callable_invariants, Callable, CallableKind, InputsIter, Visibility}; +use crate::{ir, ir::attrs::SelectorOrWildcard}; +use proc_macro2::{Ident, Span}; use syn::spanned::Spanned as _; /// An ink! constructor definition. @@ -84,17 +72,35 @@ impl quote::ToTokens for Constructor { } impl Constructor { - /// Returns `true` if the given type is `Self` or `Result` + /// Returns `true` if the given type is `Self`. fn type_is_self_val(ty: &syn::Type) -> bool { - // TODO: clean and parse Result + matches!(ty, syn::Type::Path(syn::TypePath { + qself: None, + path + }) if path.is_ident("Self")) + } + + /// Returns `true` if the given type is `Result`. + fn type_is_result_self_val(ty: &syn::Type) -> bool { + println!("\n{:#?}\n", ty); let res = matches!(ty, syn::Type::Path(syn::TypePath { qself: None, path - }) if path.is_ident("Self") ); - println!("{:?}", ty); + }) if Self::is_result_path(path)); res } + fn is_result_path(path: &syn::Path) -> bool { + if path.leading_colon.is_none() && path.segments.len() == 1 && path.segments[0].ident == "Result" { + if let syn::PathArguments::AngleBracketed(angle_args) = &path.segments[0].arguments { + if let Some(syn::GenericArgument::Type(ty)) = angle_args.args.first() { + return Self::type_is_self_val(ty) + } + } + } + false + } + /// Ensures that the return type of the ink! constructor is `Self` or `Result`. /// /// Returns an appropriate error otherwise. @@ -114,11 +120,11 @@ impl Constructor { )) } syn::ReturnType::Type(_, return_type) => { - if !Self::type_is_self_val(return_type.as_ref()) { + if !Self::type_is_self_val(return_type.as_ref()) && !Self::type_is_result_self_val(return_type.as_ref()) { return Err(format_err_spanned!( return_type, - "ink! constructors must return Self", - )) + "ink! constructors must return Self or Result", + )); } } } @@ -158,13 +164,11 @@ impl Constructor { method_item.span(), method_item.attrs.clone(), &ir::AttributeArgKind::Constructor, - |arg| { - match arg.kind() { - ir::AttributeArg::Constructor - | ir::AttributeArg::Payable - | ir::AttributeArg::Selector(_) => Ok(()), - _ => Err(None), - } + |arg| match arg.kind() { + ir::AttributeArg::Constructor + | ir::AttributeArg::Payable + | ir::AttributeArg::Selector(_) => Ok(()), + _ => Err(None), }, ) } @@ -202,14 +206,14 @@ impl Callable for Constructor { fn user_provided_selector(&self) -> Option<&ir::Selector> { if let Some(SelectorOrWildcard::UserProvided(selector)) = self.selector.as_ref() { - return Some(selector) + return Some(selector); } None } fn has_wildcard_selector(&self) -> bool { if let Some(SelectorOrWildcard::Wildcard) = self.selector { - return true + return true; } false } @@ -445,7 +449,7 @@ mod tests { }, ]; for item_method in item_methods { - assert_try_from_fails(item_method, "ink! constructors must return Self") + assert_try_from_fails(item_method, "ink! constructors must return Self or Result") } } From 2cd9dd933b30fdc136061058523d8865fe00b476 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 25 Sep 2022 22:22:44 +0100 Subject: [PATCH 03/28] add output type to dispatchable constructor --- crates/ink/codegen/src/generator/dispatch.rs | 20 ++++++----------- crates/ink/ir/src/ir/item_impl/constructor.rs | 8 +++++++ crates/ink/src/reflect/dispatch.rs | 3 +++ examples/return_err/Cargo.toml | 16 +++----------- examples/return_err/lib.rs | 22 ++++++++++++------- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 8c2368c9c0..5a6cefba20 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -14,20 +14,11 @@ use core::iter; -use crate::{ - generator, - GenerateCode, -}; +use crate::{generator, GenerateCode}; use derive_more::From; -use ir::{ - Callable, - HexLiteral as _, -}; +use ir::{Callable, HexLiteral as _}; use proc_macro2::TokenStream as TokenStream2; -use quote::{ - quote, - quote_spanned, -}; +use quote::{quote, quote_spanned}; use syn::spanned::Spanned as _; /// Generates code for the message and constructor dispatcher. @@ -255,12 +246,15 @@ 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| { @@ -547,7 +541,7 @@ impl Dispatch<'_> { ); let constructor_output = quote_spanned!(constructor_span=> <#storage_ident as ::ink::reflect::DispatchableConstructorInfo<{ - <#storage_ident as ::ink::reflect::ContractDispatchableMessages<{ + <#storage_ident as ::ink::reflect::ContractDispatchableConstructors<{ <#storage_ident as ::ink::reflect::ContractAmountDispatchables>::CONSTRUCTORS }>>::IDS[#index] }>>::Output diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 077e71a8ac..8f51d7a49d 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -248,6 +248,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)] diff --git a/crates/ink/src/reflect/dispatch.rs b/crates/ink/src/reflect/dispatch.rs index e5c64e67f6..340d1817bb 100644 --- a/crates/ink/src/reflect/dispatch.rs +++ b/crates/ink/src/reflect/dispatch.rs @@ -331,6 +331,9 @@ pub trait DispatchableConstructorInfo { /// 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; diff --git a/examples/return_err/Cargo.toml b/examples/return_err/Cargo.toml index 426d0f586b..57c4ff06ad 100755 --- a/examples/return_err/Cargo.toml +++ b/examples/return_err/Cargo.toml @@ -5,11 +5,7 @@ authors = ["[your_name] <[your_email]>"] edition = "2021" [dependencies] -ink_primitives = { version = "4.0.0-alpha.1", default-features = false } -ink_metadata = { version = "4.0.0-alpha.1", default-features = false, features = ["derive"], optional = true } -ink_env = { version = "4.0.0-alpha.1", default-features = false } -ink_storage = { version = "4.0.0-alpha.1", default-features = false } -ink_lang = { version = "4.0.0-alpha.1", default-features = false } +ink = { path = "../../crates/ink", default-features = false } scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } scale-info = { version = "2", default-features = false, features = ["derive"], optional = true } @@ -17,18 +13,12 @@ scale-info = { version = "2", default-features = false, features = ["derive"], o [lib] name = "return_err" path = "lib.rs" -crate-type = [ - # Used for normal contract Wasm blobs. - "cdylib", -] +crate-type = ["cdylib"] [features] default = ["std"] std = [ - "ink_metadata/std", - "ink_env/std", - "ink_storage/std", - "ink_primitives/std", + "ink/std", "scale/std", "scale-info/std", ] diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs index 8c10564592..5375fc6323 100755 --- a/examples/return_err/lib.rs +++ b/examples/return_err/lib.rs @@ -1,7 +1,5 @@ #![cfg_attr(not(feature = "std"), no_std)] -use ink_lang as ink; - #[ink::contract] mod return_err { @@ -14,20 +12,28 @@ mod return_err { value: bool, } + #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] + #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] + pub enum Error { + Foo, + } + + pub type Result = core::result::Result; + impl ReturnErr { /// Constructor that initializes the `bool` value to the given `init_value`. #[ink(constructor)] - pub fn new(bool: false) -> Result { - Result::Err(Error::Foo) + pub fn new(init_value: bool) -> Result { + Err(Error::Foo) } /// Constructor that initializes the `bool` value to `false`. /// /// Constructors can delegate to other constructors. - #[ink(constructor)] - pub fn default() -> Self { - Self::new(Default::default()) - } + // #[ink(constructor)] + // pub fn default() -> Self { + // Self::new(Default::default()) + // } /// A message that can be called on instantiated contracts. /// This one flips the value of the stored `bool` from `true` From fdad4dcb756632c1e45b6290fed649069f0a610b Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 26 Sep 2022 18:38:38 +0100 Subject: [PATCH 04/28] codegen adds Result to return type --- crates/ink/codegen/src/generator/dispatch.rs | 6 +++--- crates/ink/codegen/src/generator/item_impls.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 5a6cefba20..2adf3f5a0f 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -257,8 +257,8 @@ impl Dispatch<'_> { 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 ),* ]; @@ -573,7 +573,7 @@ impl Dispatch<'_> { ) } - ::ink::codegen::execute_constructor::<#storage_ident, _, _>( + ::ink::codegen::execute_constructor::<#constructor_output, _, _>( move || { #constructor_callable(input) } ) } diff --git a/crates/ink/codegen/src/generator/item_impls.rs b/crates/ink/codegen/src/generator/item_impls.rs index 4122d83251..673c6fe977 100644 --- a/crates/ink/codegen/src/generator/item_impls.rs +++ b/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 )* } ) From 47fba58237ba7b103a224837d3798df13d1d5e6b Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 26 Sep 2022 18:55:37 +0100 Subject: [PATCH 05/28] add type cheking --- crates/ink/codegen/src/generator/item_impls.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/ink/codegen/src/generator/item_impls.rs b/crates/ink/codegen/src/generator/item_impls.rs index 673c6fe977..e5077a20af 100644 --- a/crates/ink/codegen/src/generator/item_impls.rs +++ b/crates/ink/codegen/src/generator/item_impls.rs @@ -138,8 +138,17 @@ impl ItemImpls<'_> { >(); ) }); + let constructor_output = constructor.output().map(|output_type| { + let span = output_type.span(); + quote_spanned!(span=> + ::ink::codegen::utils::consume_type::< + ::ink::codegen::DispatchOutput<#output_type> + >(); + ) + }); quote_spanned!(constructor_span=> #( #constructor_inputs )* + #constructor_output ) }); let message_inout_guards = self From 109536d43abab7d6eed5000c4b208b74c49aa106 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 26 Sep 2022 21:36:53 +0100 Subject: [PATCH 06/28] fix CALLABLE type --- crates/ink/codegen/src/generator/dispatch.rs | 2 +- crates/ink/codegen/src/generator/item_impls.rs | 9 --------- crates/ink/ir/src/ir/item_impl/constructor.rs | 1 + crates/ink/src/reflect/dispatch.rs | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 2adf3f5a0f..36ef6cc773 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -573,7 +573,7 @@ impl Dispatch<'_> { ) } - ::ink::codegen::execute_constructor::<#constructor_output, _, _>( + ::ink::codegen::execute_constructor::<#storage_ident, _, _>( move || { #constructor_callable(input) } ) } diff --git a/crates/ink/codegen/src/generator/item_impls.rs b/crates/ink/codegen/src/generator/item_impls.rs index e5077a20af..673c6fe977 100644 --- a/crates/ink/codegen/src/generator/item_impls.rs +++ b/crates/ink/codegen/src/generator/item_impls.rs @@ -138,17 +138,8 @@ impl ItemImpls<'_> { >(); ) }); - let constructor_output = constructor.output().map(|output_type| { - let span = output_type.span(); - quote_spanned!(span=> - ::ink::codegen::utils::consume_type::< - ::ink::codegen::DispatchOutput<#output_type> - >(); - ) - }); quote_spanned!(constructor_span=> #( #constructor_inputs )* - #constructor_output ) }); let message_inout_guards = self diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 8f51d7a49d..77e91e511e 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -250,6 +250,7 @@ impl Constructor { } /// Returns the return type of the ink! constructor if any. + // TODO: rewrite as enum pub fn output(&self) -> Option<&syn::Type> { match &self.item.sig.output { syn::ReturnType::Default => None, diff --git a/crates/ink/src/reflect/dispatch.rs b/crates/ink/src/reflect/dispatch.rs index 340d1817bb..c6c9548724 100644 --- a/crates/ink/src/reflect/dispatch.rs +++ b/crates/ink/src/reflect/dispatch.rs @@ -335,7 +335,7 @@ pub trait DispatchableConstructorInfo { 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; From ab3897f61a633fc0452b5ce167fa12dbaf6e0a7c Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Wed, 12 Oct 2022 16:01:15 +0100 Subject: [PATCH 07/28] add comments for refactoring --- crates/ink/codegen/src/generator/dispatch.rs | 4 ++++ crates/ink/ir/src/ir/item_impl/constructor.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 36ef6cc773..518acbeea7 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -539,6 +539,8 @@ impl Dispatch<'_> { }>>::IDS[#index] }>>::CALLABLE ); + + //TODO: needs to return an enum and see whether the return type is Self or Result or similar let constructor_output = quote_spanned!(constructor_span=> <#storage_ident as ::ink::reflect::DispatchableConstructorInfo<{ <#storage_ident as ::ink::reflect::ContractDispatchableConstructors<{ @@ -560,6 +562,7 @@ impl Dispatch<'_> { <#storage_ident as ::ink::reflect::ContractEnv>::Env>()?; } + //TODO: conditioally extract result type check for error let result: #constructor_output = #constructor_callable(input); let failure = ::ink::is_result_type!(#constructor_output) && ::ink::is_result_err!(result); @@ -573,6 +576,7 @@ impl Dispatch<'_> { ) } + //TODO: this has to be reworked too ::ink::codegen::execute_constructor::<#storage_ident, _, _>( move || { #constructor_callable(input) } ) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 77e91e511e..a371d7b59d 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -72,6 +72,7 @@ impl quote::ToTokens for Constructor { } impl Constructor { + //TODO: need to rework this support type aliases /// Returns `true` if the given type is `Self`. fn type_is_self_val(ty: &syn::Type) -> bool { matches!(ty, syn::Type::Path(syn::TypePath { @@ -80,6 +81,7 @@ impl Constructor { }) if path.is_ident("Self")) } + //TODO: Hence why this method also needs to be reworked /// Returns `true` if the given type is `Result`. fn type_is_result_self_val(ty: &syn::Type) -> bool { println!("\n{:#?}\n", ty); From 4d6766f9a053db4472f18dc05bd7f60accf5ea24 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Wed, 12 Oct 2022 17:27:07 +0100 Subject: [PATCH 08/28] ugly solution for failable contracts --- crates/ink/codegen/src/generator/storage.rs | 1 + examples/contract-transfer/lib.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/crates/ink/codegen/src/generator/storage.rs b/crates/ink/codegen/src/generator/storage.rs index dcc38a3b0f..70f4a6b502 100644 --- a/crates/ink/codegen/src/generator/storage.rs +++ b/crates/ink/codegen/src/generator/storage.rs @@ -101,6 +101,7 @@ impl Storage<'_> { #[::ink::storage_item] #[cfg_attr(test, derive(::core::fmt::Debug))] #[cfg(not(feature = "__ink_dylint_Storage"))] + #[derive(scale::Encode)] pub struct #ident #generics { #( #fields ),* } diff --git a/examples/contract-transfer/lib.rs b/examples/contract-transfer/lib.rs index 0e159e9b15..89d17c4e71 100644 --- a/examples/contract-transfer/lib.rs +++ b/examples/contract-transfer/lib.rs @@ -10,6 +10,14 @@ pub mod give_me { #[ink(storage)] pub struct GiveMe {} + #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] + #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] + pub enum Error { + Foo, + } + + pub type Result = core::result::Result; + impl GiveMe { /// Creates a new instance of this contract. #[ink(constructor, payable)] @@ -17,6 +25,11 @@ pub mod give_me { Self {} } + #[ink(constructor, payable)] + pub fn new2() -> Result { + Ok(Self {}) + } + /// Transfers `value` amount of tokens to the caller. /// /// # Errors From 10126f2e29ca7dd261816380dc394e8d90579756 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 17 Oct 2022 21:24:44 +0100 Subject: [PATCH 09/28] avoid unnecessary error check for constructors --- crates/ink/codegen/src/generator/dispatch.rs | 24 ------------------- crates/ink/ir/src/ir/item_impl/constructor.rs | 1 - 2 files changed, 25 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 4314dcfac6..4df05a9ed6 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -555,15 +555,6 @@ impl Dispatch<'_> { }>>::IDS[#index] }>>::CALLABLE ); - - //TODO: needs to return an enum and see whether the return type is Self or Result or similar - 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<{ @@ -578,21 +569,6 @@ impl Dispatch<'_> { <#storage_ident as ::ink::reflect::ContractEnv>::Env>()?; } - //TODO: conditioally extract result type check for error - let result: #constructor_output = #constructor_callable(input); - let failure = ::ink::is_result_type!(#constructor_output) - && ::ink::is_result_err!(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::<#constructor_output>( - ::ink::env::ReturnFlags::default().set_reverted(true), &result - ) - } - - //TODO: this has to be reworked too ::ink::codegen::execute_constructor::<#storage_ident, _, _>( move || { #constructor_callable(input) } ) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index cfb6ec218a..d33b661cff 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -96,7 +96,6 @@ impl Constructor { // TODO: Hence why this method also needs to be reworked /// Returns `true` if the given type is `Result`. fn type_is_result_self_val(ty: &syn::Type) -> bool { - println!("\n{:#?}\n", ty); let res = matches!(ty, syn::Type::Path(syn::TypePath { qself: None, path From 2fa1ac0115dda03a6da405e8242c796f19ff60b4 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 17 Oct 2022 23:36:04 +0100 Subject: [PATCH 10/28] avoids encoding storage, adds demo example --- crates/ink/codegen/src/generator/storage.rs | 1 - crates/ink/src/codegen/dispatch/execution.rs | 29 +++----- examples/contract-transfer/lib.rs | 13 ---- examples/return_err/lib.rs | 70 +++++--------------- 4 files changed, 28 insertions(+), 85 deletions(-) diff --git a/crates/ink/codegen/src/generator/storage.rs b/crates/ink/codegen/src/generator/storage.rs index 70f4a6b502..dcc38a3b0f 100644 --- a/crates/ink/codegen/src/generator/storage.rs +++ b/crates/ink/codegen/src/generator/storage.rs @@ -101,7 +101,6 @@ impl Storage<'_> { #[::ink::storage_item] #[cfg_attr(test, derive(::core::fmt::Debug))] #[cfg(not(feature = "__ink_dylint_Storage"))] - #[derive(scale::Encode)] pub struct #ident #generics { #( #fields ),* } diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index f169440b49..6000df3401 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/crates/ink/src/codegen/dispatch/execution.rs @@ -12,22 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::reflect::{ - ContractEnv, - DispatchError, -}; -use core::{ - convert::Infallible, - mem::ManuallyDrop, -}; -use ink_env::{ - Environment, - ReturnFlags, -}; -use ink_storage::traits::{ - Storable, - StorageKey, -}; +use crate::reflect::{ContractEnv, DispatchError}; +use core::{convert::Infallible, mem::ManuallyDrop}; +use ink_env::{Environment, ReturnFlags}; +use ink_storage::traits::{Storable, StorageKey}; use scale::Encode; /// Returns `Ok` if the caller did not transfer additional value to the callee. @@ -42,7 +30,7 @@ where { let transferred = ink_env::transferred_value::(); if transferred != ::Balance::from(0_u32) { - return Err(DispatchError::PaidUnpayableMessage) + return Err(DispatchError::PaidUnpayableMessage); } Ok(()) } @@ -167,7 +155,7 @@ impl ConstructorReturnType for private::Seal { impl ConstructorReturnType for private::Seal> { const IS_RESULT: bool = true; type Error = E; - type ReturnValue = Result; + type ReturnValue = Self::Error; #[inline] fn as_result(&self) -> Result<&C, &Self::Error> { @@ -176,7 +164,10 @@ impl ConstructorReturnType for private::Seal> { #[inline] fn return_value(&self) -> &Self::ReturnValue { - &self.0 + self.0 + .as_ref() + .err() + .expect("value returned only when error occurs. qed") } } diff --git a/examples/contract-transfer/lib.rs b/examples/contract-transfer/lib.rs index a6b67854e7..7e17fef194 100644 --- a/examples/contract-transfer/lib.rs +++ b/examples/contract-transfer/lib.rs @@ -10,14 +10,6 @@ pub mod give_me { #[ink(storage)] pub struct GiveMe {} - #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] - #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] - pub enum Error { - Foo, - } - - pub type Result = core::result::Result; - impl GiveMe { /// Creates a new instance of this contract. #[ink(constructor, payable)] @@ -25,11 +17,6 @@ pub mod give_me { Self {} } - #[ink(constructor, payable)] - pub fn new2() -> Result { - Err(Error::Foo) - } - /// Transfers `value` amount of tokens to the caller. /// /// # Errors diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs index 5375fc6323..be7943a7d5 100755 --- a/examples/return_err/lib.rs +++ b/examples/return_err/lib.rs @@ -3,13 +3,9 @@ #[ink::contract] mod return_err { - /// Defines the storage of your contract. - /// Add new fields to the below struct in order - /// to add new static storage fields to your contract. #[ink(storage)] pub struct ReturnErr { - /// Stores a single `bool` value on the storage. - value: bool, + instantiated: bool, } #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] @@ -18,63 +14,33 @@ mod return_err { Foo, } + impl Default for ReturnErr { + fn default() -> Self { + Self { instantiated: true } + } + } + pub type Result = core::result::Result; impl ReturnErr { /// Constructor that initializes the `bool` value to the given `init_value`. #[ink(constructor)] - pub fn new(init_value: bool) -> Result { - Err(Error::Foo) + pub fn new() -> Self { + Default::default() } - /// Constructor that initializes the `bool` value to `false`. - /// - /// Constructors can delegate to other constructors. - // #[ink(constructor)] - // pub fn default() -> Self { - // Self::new(Default::default()) - // } - - /// A message that can be called on instantiated contracts. - /// This one flips the value of the stored `bool` from `true` - /// to `false` and vice versa. - #[ink(message)] - pub fn flip(&mut self) { - self.value = !self.value; + #[ink(constructor, payable)] + pub fn new2(fail: bool) -> Result { + if fail { + Err(Error::Foo) + } else { + Ok(Default::default()) + } } - /// Simply returns the current value of our `bool`. #[ink(message)] - pub fn get(&self) -> bool { - self.value - } - } - - /// Unit tests in Rust are normally defined within such a `#[cfg(test)]` - /// module and test functions are marked with a `#[test]` attribute. - /// The below code is technically just normal Rust code. - #[cfg(test)] - mod tests { - /// Imports all the definitions from the outer scope so we can use them here. - use super::*; - - /// Imports `ink_lang` so we can use `#[ink::test]`. - use ink_lang as ink; - - /// We test if the default constructor does its job. - #[ink::test] - fn default_works() { - let return_err = ReturnErr::default(); - assert_eq!(return_err.get(), false); - } - - /// We test a simple use case of our contract. - #[ink::test] - fn it_works() { - let mut return_err = ReturnErr::new(false); - assert_eq!(return_err.get(), false); - return_err.flip(); - assert_eq!(return_err.get(), true); + pub fn is_instantiated(&self) -> bool { + self.instantiated } } } From e2649c55e955476a04dad88e0d73f96e3b89baf3 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Fri, 21 Oct 2022 23:31:43 +0100 Subject: [PATCH 11/28] allows differents syntax format of result return type --- crates/ink/ir/src/ir/item_impl/constructor.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index d33b661cff..c26bbf6fbb 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -84,7 +84,6 @@ impl quote::ToTokens for Constructor { } impl Constructor { - // TODO: need to rework this support type aliases /// Returns `true` if the given type is `Self`. fn type_is_self_val(ty: &syn::Type) -> bool { matches!(ty, syn::Type::Path(syn::TypePath { @@ -93,7 +92,6 @@ impl Constructor { }) if path.is_ident("Self")) } - // TODO: Hence why this method also needs to be reworked /// Returns `true` if the given type is `Result`. fn type_is_result_self_val(ty: &syn::Type) -> bool { let res = matches!(ty, syn::Type::Path(syn::TypePath { @@ -105,11 +103,11 @@ impl Constructor { fn is_result_path(path: &syn::Path) -> bool { if path.leading_colon.is_none() - && path.segments.len() == 1 - && path.segments[0].ident == "Result" + && path.segments.last().is_some() + && path.segments.last().unwrap().ident == "Result" { if let syn::PathArguments::AngleBracketed(angle_args) = - &path.segments[0].arguments + &path.segments.last().unwrap().arguments { if let Some(syn::GenericArgument::Type(ty)) = angle_args.args.first() { return Self::type_is_self_val(ty) From 8d79fef7089d023869a1468b04c5655c1ce72978 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Fri, 21 Oct 2022 23:32:16 +0100 Subject: [PATCH 12/28] cargo fmt --- crates/ink/src/codegen/dispatch/execution.rs | 22 +++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index 6000df3401..edeac1fb06 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/crates/ink/src/codegen/dispatch/execution.rs @@ -12,10 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::reflect::{ContractEnv, DispatchError}; -use core::{convert::Infallible, mem::ManuallyDrop}; -use ink_env::{Environment, ReturnFlags}; -use ink_storage::traits::{Storable, StorageKey}; +use crate::reflect::{ + ContractEnv, + DispatchError, +}; +use core::{ + convert::Infallible, + mem::ManuallyDrop, +}; +use ink_env::{ + Environment, + ReturnFlags, +}; +use ink_storage::traits::{ + Storable, + StorageKey, +}; use scale::Encode; /// Returns `Ok` if the caller did not transfer additional value to the callee. @@ -30,7 +42,7 @@ where { let transferred = ink_env::transferred_value::(); if transferred != ::Balance::from(0_u32) { - return Err(DispatchError::PaidUnpayableMessage); + return Err(DispatchError::PaidUnpayableMessage) } Ok(()) } From 89e178929050ac098eb428b9ba33c250a0c80423 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Fri, 21 Oct 2022 23:43:47 +0100 Subject: [PATCH 13/28] add changes to CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40065bc5d0..5acf6844c0 100644 --- a/CHANGELOG.md +++ b/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` 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. From 36de262a09978c1b8bf3806bc59276eff8a9ccc1 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 23 Oct 2022 19:02:22 +0100 Subject: [PATCH 14/28] add tests to the example --- examples/dns/lib.rs | 2 +- examples/return_err/lib.rs | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/examples/dns/lib.rs b/examples/dns/lib.rs index 6374386304..b67eeb43a1 100644 --- a/examples/dns/lib.rs +++ b/examples/dns/lib.rs @@ -73,7 +73,7 @@ mod dns { CallerIsNotOwner, } - /// Type alias for the contract's result type. + pub type Result = core::result::Result; impl DomainNameService { diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs index be7943a7d5..41a1490e5a 100755 --- a/examples/return_err/lib.rs +++ b/examples/return_err/lib.rs @@ -8,6 +8,7 @@ mod return_err { instantiated: bool, } + /// Example of Error enum #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] pub enum Error { @@ -20,17 +21,19 @@ mod return_err { } } + /// Type alias for the contract's result type. pub type Result = core::result::Result; impl ReturnErr { - /// Constructor that initializes the `bool` value to the given `init_value`. + /// Classic constructor that instantiated the contract #[ink(constructor)] pub fn new() -> Self { Default::default() } + /// Constructor that can be manually failed #[ink(constructor, payable)] - pub fn new2(fail: bool) -> Result { + pub fn another_new(fail: bool) -> Result { if fail { Err(Error::Foo) } else { @@ -38,9 +41,34 @@ mod return_err { } } + /// Gets if the contract has been instantiated #[ink(message)] pub fn is_instantiated(&self) -> bool { self.instantiated } } + #[cfg(test)] + mod tests { + use super::*; + + #[ink::test] + fn classic_constructor_works() { + let contract = ReturnErr::new(); + assert!(contract.is_instantiated()) + } + + #[ink::test] + fn constructor_safely_fails() { + let contract = ReturnErr::another_new(true); + assert!(contract.is_err()); + assert_eq!(contract.err(), Some(Error::Foo)) + } + + #[ink::test] + fn constructor_safely_works() { + let contract = ReturnErr::another_new(false); + assert!(contract.is_ok()); + assert!(contract.unwrap().is_instantiated()) + } + } } From 50cf90000655cb77ecc3f7b414682a66b69c6860 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 23 Oct 2022 19:35:07 +0100 Subject: [PATCH 15/28] add UI tests --- .../fail/constructor-return-result-invalid.rs | 23 ++++++++ .../constructor-return-result-invalid.stderr | 5 ++ ...nstructor-return-result-non-codec-error.rs | 22 ++++++++ ...uctor-return-result-non-codec-error.stderr | 22 ++++++++ .../pass/constructor-return-result-alias.rs | 25 +++++++++ .../pass/constructor-return-result-err.rs | 23 ++++++++ .../constructor-return-result-explicit.rs | 23 ++++++++ .../pass/constructor-return-result-ok.rs | 23 ++++++++ .../contract/pass/example-return-err-works.rs | 55 +++++++++++++++++++ examples/return_err/Cargo.toml | 5 +- 10 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs create mode 100644 crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr create mode 100644 crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs create mode 100644 crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr create mode 100644 crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs create mode 100644 crates/ink/tests/ui/contract/pass/constructor-return-result-err.rs create mode 100644 crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs create mode 100644 crates/ink/tests/ui/contract/pass/constructor-return-result-ok.rs create mode 100644 crates/ink/tests/ui/contract/pass/example-return-err-works.rs diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs new file mode 100644 index 0000000000..85a83c3dc3 --- /dev/null +++ b/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() -> core::result::Result{ + Ok(5_u8) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr new file mode 100644 index 0000000000..55d251ac00 --- /dev/null +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr @@ -0,0 +1,5 @@ +error: ink! constructors must return Self or Result + --> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:33 + | +14 | pub fn constructor() -> core::result::Result{ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs new file mode 100644 index 0000000000..a91fe215ea --- /dev/null +++ b/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() -> core::result::Result{ + Ok(Self{}) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr new file mode 100644 index 0000000000..a9815d876f --- /dev/null +++ b/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() -> core::result::Result{ + | ^^^ the trait `WrapperTypeEncode` is not implemented for `contract::Error` + | + = help: the following other types implement trait `WrapperTypeEncode`: + &T + &mut T + Arc + Box + Cow<'a, T> + Rc + String + Vec + 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 + | + | as ConstructorReturnType>::ReturnValue: Encode, + | ^^^^^^ required by this bound in `execute_constructor` diff --git a/crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs b/crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs new file mode 100644 index 0000000000..9d7f8ba1ee --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs @@ -0,0 +1,25 @@ +#[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, + } + + pub type Result = core::result::Result; + + impl Contract { + #[ink(constructor)] + pub fn constructor() -> Result { + Err(Error::Foo) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/pass/constructor-return-result-err.rs b/crates/ink/tests/ui/contract/pass/constructor-return-result-err.rs new file mode 100644 index 0000000000..a176a45764 --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/constructor-return-result-err.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 { + Err(Error::Foo) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs b/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs new file mode 100644 index 0000000000..0bd6452a9b --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.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() -> core::result::Result{ + Err(Error::Foo) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/pass/constructor-return-result-ok.rs b/crates/ink/tests/ui/contract/pass/constructor-return-result-ok.rs new file mode 100644 index 0000000000..949c549671 --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/constructor-return-result-ok.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 { + Ok(Self {}) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} diff --git a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs new file mode 100644 index 0000000000..0f824a24bd --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs @@ -0,0 +1,55 @@ +use return_err::ReturnErr; + +#[ink::contract] +mod return_err { + + #[ink(storage)] + pub struct ReturnErr { + instantiated: bool, + } + + #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] + #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] + pub enum Error { + Foo, + } + + impl Default for ReturnErr { + fn default() -> Self { + Self { instantiated: true } + } + } + + pub type Result = core::result::Result; + + impl ReturnErr { + #[ink(constructor)] + pub fn new() -> Self { + Default::default() + } + + #[ink(constructor, payable)] + pub fn another_new(fail: bool) -> Result { + if fail { + Err(Error::Foo) + } else { + Ok(Default::default()) + } + } + + #[ink(message)] + pub fn is_instantiated(&self) -> bool { + self.instantiated + } + } +} + +fn main() { + let contract = ReturnErr::another_new(true); + assert!(contract.is_err()); + assert_eq!(contract.err(), Some(Error::Foo)); + + let contract = ReturnErr::another_new(false); + assert!(contract.is_ok()); + assert!(contract.unwrap().is_instantiated()); +} diff --git a/examples/return_err/Cargo.toml b/examples/return_err/Cargo.toml index 57c4ff06ad..58741f811b 100755 --- a/examples/return_err/Cargo.toml +++ b/examples/return_err/Cargo.toml @@ -1,8 +1,9 @@ [package] name = "return_err" -version = "0.1.0" -authors = ["[your_name] <[your_email]>"] +version = "4.0.0-alpha.3" +authors = ["Parity Technologies "] edition = "2021" +publish = false [dependencies] ink = { path = "../../crates/ink", default-features = false } From c372352268c250e05d2843eeae3d6a4c370e20ac Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 23 Oct 2022 19:37:51 +0100 Subject: [PATCH 16/28] return comment to dns example --- examples/dns/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/dns/lib.rs b/examples/dns/lib.rs index b67eeb43a1..6374386304 100644 --- a/examples/dns/lib.rs +++ b/examples/dns/lib.rs @@ -73,7 +73,7 @@ mod dns { CallerIsNotOwner, } - + /// Type alias for the contract's result type. pub type Result = core::result::Result; impl DomainNameService { From 52da525c906eb66a4e13a7744090cab0d3b5c6cb Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Sun, 23 Oct 2022 19:44:17 +0100 Subject: [PATCH 17/28] fix formatting in UI examples --- .../ui/contract/fail/constructor-return-result-invalid.rs | 2 +- .../ui/contract/fail/constructor-return-result-invalid.stderr | 2 +- .../fail/constructor-return-result-non-codec-error.rs | 4 ++-- .../fail/constructor-return-result-non-codec-error.stderr | 2 +- .../ui/contract/pass/constructor-return-result-explicit.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs index 85a83c3dc3..1cbabf8ec1 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs @@ -11,7 +11,7 @@ mod contract { impl Contract { #[ink(constructor)] - pub fn constructor() -> core::result::Result{ + pub fn constructor() -> core::result::Result { Ok(5_u8) } diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr index 55d251ac00..470210bc2c 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr @@ -1,5 +1,5 @@ error: ink! constructors must return Self or Result --> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:33 | -14 | pub fn constructor() -> core::result::Result{ +14 | pub fn constructor() -> core::result::Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs index a91fe215ea..17330ee1ed 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs @@ -10,8 +10,8 @@ mod contract { impl Contract { #[ink(constructor)] - pub fn constructor() -> core::result::Result{ - Ok(Self{}) + pub fn constructor() -> core::result::Result { + Ok(Self {}) } #[ink(message)] diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr index a9815d876f..ef2458701a 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr @@ -1,7 +1,7 @@ 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() -> core::result::Result{ +13 | pub fn constructor() -> core::result::Result { | ^^^ the trait `WrapperTypeEncode` is not implemented for `contract::Error` | = help: the following other types implement trait `WrapperTypeEncode`: diff --git a/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs b/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs index 0bd6452a9b..8c76207564 100644 --- a/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs +++ b/crates/ink/tests/ui/contract/pass/constructor-return-result-explicit.rs @@ -11,7 +11,7 @@ mod contract { impl Contract { #[ink(constructor)] - pub fn constructor() -> core::result::Result{ + pub fn constructor() -> core::result::Result { Err(Error::Foo) } From 6cea984402cdb4aa338a845887655417d6452a1a Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 24 Oct 2022 12:09:27 +0100 Subject: [PATCH 18/28] fix tests and docs --- crates/ink/ir/src/ir/item_impl/constructor.rs | 10 ++++++++-- .../tests/ui/contract/pass/example-return-err-works.rs | 2 +- examples/return_err/lib.rs | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index c26bbf6fbb..1455d54765 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -101,6 +101,8 @@ impl Constructor { res } + /// Helper functions that evaluates that the return type is `Result` + /// and the `Ok` variant is `Self` type fn is_result_path(path: &syn::Path) -> bool { if path.leading_colon.is_none() && path.segments.last().is_some() @@ -270,7 +272,6 @@ impl Constructor { } /// Returns the return type of the ink! constructor if any. - // TODO: rewrite as enum pub fn output(&self) -> Option<&syn::Type> { match &self.item.sig.output { syn::ReturnType::Default => None, @@ -426,6 +427,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 {} + }, ]; for item_method in item_methods { assert!(>::try_from(item_method).is_ok()); @@ -474,7 +480,7 @@ mod tests { }, syn::parse_quote! { #[ink(constructor)] - pub fn my_constructor() -> Result {} + pub fn my_constructor() -> Result {} }, ]; for item_method in item_methods { diff --git a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs index 0f824a24bd..5441d169df 100644 --- a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs +++ b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs @@ -1,4 +1,4 @@ -use return_err::ReturnErr; +use return_err::{ReturnErr, Error}; #[ink::contract] mod return_err { diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs index 41a1490e5a..74c151755d 100755 --- a/examples/return_err/lib.rs +++ b/examples/return_err/lib.rs @@ -41,7 +41,7 @@ mod return_err { } } - /// Gets if the contract has been instantiated + /// Checks if the contract has been instantiated #[ink(message)] pub fn is_instantiated(&self) -> bool { self.instantiated From d7dcb5952dd7276b027c131c7a9da8751fcc0805 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 24 Oct 2022 12:11:09 +0100 Subject: [PATCH 19/28] apply formatting suggestions --- .../ink/tests/ui/contract/pass/example-return-err-works.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs index 5441d169df..b07a45a73b 100644 --- a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs +++ b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs @@ -1,4 +1,7 @@ -use return_err::{ReturnErr, Error}; +use return_err::{ + Error, + ReturnErr, +}; #[ink::contract] mod return_err { From 9d00fd27e285fa63928d4f1f21d836d9764013cb Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 24 Oct 2022 14:05:08 +0100 Subject: [PATCH 20/28] change example and UI tests for more clarity --- .../contract/pass/example-return-err-works.rs | 31 +++++++--------- examples/return_err/lib.rs | 35 ++++++++++--------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs index b07a45a73b..e90eac867e 100644 --- a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs +++ b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs @@ -1,14 +1,10 @@ -use return_err::{ - Error, - ReturnErr, -}; - #[ink::contract] mod return_err { #[ink(storage)] + #[derive(Default)] pub struct ReturnErr { - instantiated: bool, + count: i32, } #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] @@ -17,14 +13,6 @@ mod return_err { Foo, } - impl Default for ReturnErr { - fn default() -> Self { - Self { instantiated: true } - } - } - - pub type Result = core::result::Result; - impl ReturnErr { #[ink(constructor)] pub fn new() -> Self { @@ -32,7 +20,7 @@ mod return_err { } #[ink(constructor, payable)] - pub fn another_new(fail: bool) -> Result { + pub fn another_new(fail: bool) -> Result { if fail { Err(Error::Foo) } else { @@ -41,8 +29,13 @@ mod return_err { } #[ink(message)] - pub fn is_instantiated(&self) -> bool { - self.instantiated + pub fn get_count(&self) -> i32 { + self.count + } + + #[ink(message)] + pub fn incr(&mut self, n: i32) { + self.count += n; } } } @@ -54,5 +47,7 @@ fn main() { let contract = ReturnErr::another_new(false); assert!(contract.is_ok()); - assert!(contract.unwrap().is_instantiated()); + let mut contract = contract.unwrap(); + contract.incr(-5); + assert_eq!(contract.get_count(), -5); } diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs index 74c151755d..b733dc42cd 100755 --- a/examples/return_err/lib.rs +++ b/examples/return_err/lib.rs @@ -4,8 +4,9 @@ mod return_err { #[ink(storage)] + #[derive(Default)] pub struct ReturnErr { - instantiated: bool, + count: i32, } /// Example of Error enum @@ -15,15 +16,6 @@ mod return_err { Foo, } - impl Default for ReturnErr { - fn default() -> Self { - Self { instantiated: true } - } - } - - /// Type alias for the contract's result type. - pub type Result = core::result::Result; - impl ReturnErr { /// Classic constructor that instantiated the contract #[ink(constructor)] @@ -33,7 +25,7 @@ mod return_err { /// Constructor that can be manually failed #[ink(constructor, payable)] - pub fn another_new(fail: bool) -> Result { + pub fn another_new(fail: bool) -> Result { if fail { Err(Error::Foo) } else { @@ -41,10 +33,16 @@ mod return_err { } } - /// Checks if the contract has been instantiated + /// Gets the value of a counter + #[ink(message)] + pub fn get_count(&self) -> i32 { + self.count + } + + /// Changes the value of counter #[ink(message)] - pub fn is_instantiated(&self) -> bool { - self.instantiated + pub fn incr(&mut self, n: i32) { + self.count += n; } } #[cfg(test)] @@ -53,8 +51,9 @@ mod return_err { #[ink::test] fn classic_constructor_works() { - let contract = ReturnErr::new(); - assert!(contract.is_instantiated()) + let mut contract = ReturnErr::new(); + contract.incr(5); + assert_eq!(contract.get_count(), 5) } #[ink::test] @@ -68,7 +67,9 @@ mod return_err { fn constructor_safely_works() { let contract = ReturnErr::another_new(false); assert!(contract.is_ok()); - assert!(contract.unwrap().is_instantiated()) + let mut contract = contract.unwrap(); + contract.incr(-5); + assert_eq!(contract.get_count(), -5); } } } From f35ef9886f1bc81c3751467fcb60d961c8c697b8 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Mon, 24 Oct 2022 14:47:36 +0100 Subject: [PATCH 21/28] add imports for UI test --- .../ink/tests/ui/contract/pass/example-return-err-works.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs index e90eac867e..7d33e57892 100644 --- a/crates/ink/tests/ui/contract/pass/example-return-err-works.rs +++ b/crates/ink/tests/ui/contract/pass/example-return-err-works.rs @@ -1,3 +1,8 @@ +use return_err::{ + Error, + ReturnErr, +}; + #[ink::contract] mod return_err { From 05329934655447496cebbc29f4e74975a0a19a9d Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Wed, 26 Oct 2022 10:45:10 +0200 Subject: [PATCH 22/28] refactor execution.rs --- crates/ink/src/codegen/dispatch/execution.rs | 33 ++++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index edeac1fb06..55e0057f5f 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/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(f: F) -> Result<(), DispatchError> where Contract: Storable + StorageKey + ContractEnv, F: FnOnce() -> R, - as ConstructorReturnType>::ReturnValue: Encode, + as ConstructorReturnType>::Error: Encode, private::Seal: ConstructorReturnType, { let result = ManuallyDrop::new(private::Seal(f())); @@ -78,10 +75,10 @@ where // // We need to revert the state of the transaction. ink_env::return_value::< - as ConstructorReturnType>::ReturnValue, + as ConstructorReturnType>::Error, >( ReturnFlags::default().set_reverted(true), - result.return_value(), + result.error_value(), ) } } @@ -119,19 +116,9 @@ pub trait ConstructorReturnType: private::Sealed { /// /// # Note /// - /// For infallible constructors this is `core::convert::Infallible`. + /// For infallible constructors this is `()`. 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 - /// does not matter. - type ReturnValue; - /// Converts the return value into a `Result` instance. /// /// # Note @@ -146,12 +133,11 @@ pub trait ConstructorReturnType: private::Sealed { /// 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; + fn error_value(&self) -> &Self::Error; } impl ConstructorReturnType for private::Seal { - type Error = Infallible; - type ReturnValue = (); + type Error = (); #[inline] fn as_result(&self) -> Result<&C, &Self::Error> { @@ -159,7 +145,7 @@ impl ConstructorReturnType for private::Seal { } #[inline] - fn return_value(&self) -> &Self::ReturnValue { + fn error_value(&self) -> &Self::Error { &() } } @@ -167,7 +153,6 @@ impl ConstructorReturnType for private::Seal { impl ConstructorReturnType for private::Seal> { const IS_RESULT: bool = true; type Error = E; - type ReturnValue = Self::Error; #[inline] fn as_result(&self) -> Result<&C, &Self::Error> { @@ -175,7 +160,7 @@ impl ConstructorReturnType for private::Seal> { } #[inline] - fn return_value(&self) -> &Self::ReturnValue { + fn error_value(&self) -> &Self::Error { self.0 .as_ref() .err() From 7b4d56c31f803afc74010c1d86e77bbabb3f2df8 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Wed, 26 Oct 2022 17:16:06 +0200 Subject: [PATCH 23/28] remove syntactical check for constructors return type --- crates/ink/ir/src/ir/item_impl/constructor.rs | 85 ++----------------- crates/ink/src/reflect/dispatch.rs | 1 - .../fail/constructor-return-result-invalid.rs | 2 +- .../constructor-return-result-invalid.stderr | 17 +++- ...nstructor-return-result-non-codec-error.rs | 2 +- ...uctor-return-result-non-codec-error.stderr | 6 +- 6 files changed, 23 insertions(+), 90 deletions(-) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 1455d54765..342f9ee515 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -84,41 +84,6 @@ 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")) - } - - /// Returns `true` if the given type is `Result`. - fn type_is_result_self_val(ty: &syn::Type) -> bool { - let res = matches!(ty, syn::Type::Path(syn::TypePath { - qself: None, - path - }) if Self::is_result_path(path)); - res - } - - /// Helper functions that evaluates that the return type is `Result` - /// and the `Ok` variant is `Self` type - fn is_result_path(path: &syn::Path) -> bool { - if path.leading_colon.is_none() - && path.segments.last().is_some() - && path.segments.last().unwrap().ident == "Result" - { - if let syn::PathArguments::AngleBracketed(angle_args) = - &path.segments.last().unwrap().arguments - { - if let Some(syn::GenericArgument::Type(ty)) = angle_args.args.first() { - return Self::type_is_self_val(ty) - } - } - } - false - } - /// Ensures that the return type of the ink! constructor is `Self` or `Result`. /// /// Returns an appropriate error otherwise. @@ -130,23 +95,11 @@ impl Constructor { 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()) - && !Self::type_is_result_self_val(return_type.as_ref()) - { - return Err(format_err_spanned!( - return_type, - "ink! constructors must return Self or Result", - )) - } - } + if let syn::ReturnType::Default = &method_item.sig.output { + return Err(format_err_spanned!( + &method_item.sig, + "missing return for ink! constructor", + )) } Ok(()) } @@ -463,34 +416,6 @@ mod tests { } } - #[test] - fn try_from_invalid_return_fails() { - let item_methods: Vec = 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 {} - }, - ]; - for item_method in item_methods { - assert_try_from_fails( - item_method, - "ink! constructors must return Self or Result", - ) - } - } - #[test] fn try_from_invalid_self_receiver_fails() { let item_methods: Vec = vec![ diff --git a/crates/ink/src/reflect/dispatch.rs b/crates/ink/src/reflect/dispatch.rs index c6c9548724..832cefeb24 100644 --- a/crates/ink/src/reflect/dispatch.rs +++ b/crates/ink/src/reflect/dispatch.rs @@ -330,7 +330,6 @@ pub trait DispatchableConstructorInfo { type Input; /// The ink! storage struct type. type Storage; - /// Reflects the output type of the dispatchable ink! message. type Output; diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs index 1cbabf8ec1..9fac79f37b 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.rs @@ -11,7 +11,7 @@ mod contract { impl Contract { #[ink(constructor)] - pub fn constructor() -> core::result::Result { + pub fn constructor() -> Result { Ok(5_u8) } diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr index 470210bc2c..7d06785ab5 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-invalid.stderr @@ -1,5 +1,14 @@ -error: ink! constructors must return Self or Result - --> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:33 +error[E0277]: the trait bound `codegen::dispatch::execution::private::Seal>: codegen::dispatch::execution::ConstructorReturnType` is not satisfied + --> tests/ui/contract/fail/constructor-return-result-invalid.rs:14:9 | -14 | pub fn constructor() -> core::result::Result { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +14 | pub fn constructor() -> Result { + | ^^^ the trait `codegen::dispatch::execution::ConstructorReturnType` is not implemented for `codegen::dispatch::execution::private::Seal>` + | + = help: the following other types implement trait `codegen::dispatch::execution::ConstructorReturnType`: + codegen::dispatch::execution::private::Seal + codegen::dispatch::execution::private::Seal> +note: required by a bound in `execute_constructor` + --> src/codegen/dispatch/execution.rs + | + | private::Seal: ConstructorReturnType, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `execute_constructor` diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs index 17330ee1ed..270be01759 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.rs @@ -10,7 +10,7 @@ mod contract { impl Contract { #[ink(constructor)] - pub fn constructor() -> core::result::Result { + pub fn constructor() -> Result { Ok(Self {}) } diff --git a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr index ef2458701a..c0b23450c1 100644 --- a/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr +++ b/crates/ink/tests/ui/contract/fail/constructor-return-result-non-codec-error.stderr @@ -1,7 +1,7 @@ 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() -> core::result::Result { +13 | pub fn constructor() -> Result { | ^^^ the trait `WrapperTypeEncode` is not implemented for `contract::Error` | = help: the following other types implement trait `WrapperTypeEncode`: @@ -18,5 +18,5 @@ error[E0277]: the trait bound `contract::Error: WrapperTypeEncode` is not satisf note: required by a bound in `execute_constructor` --> src/codegen/dispatch/execution.rs | - | as ConstructorReturnType>::ReturnValue: Encode, - | ^^^^^^ required by this bound in `execute_constructor` + | as ConstructorReturnType>::Error: Encode, + | ^^^^^^ required by this bound in `execute_constructor` From 0f8abd1afe5436d28039c43e6031417162d8c3fa Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Wed, 26 Oct 2022 18:26:33 +0200 Subject: [PATCH 24/28] UI test for type aliases --- .../ui/contract/pass/constructor-aliases.rs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 crates/ink/tests/ui/contract/pass/constructor-aliases.rs diff --git a/crates/ink/tests/ui/contract/pass/constructor-aliases.rs b/crates/ink/tests/ui/contract/pass/constructor-aliases.rs new file mode 100644 index 0000000000..ea0e18c0bc --- /dev/null +++ b/crates/ink/tests/ui/contract/pass/constructor-aliases.rs @@ -0,0 +1,36 @@ +#[ink::contract] +mod contract { + #[ink(storage)] + pub struct Contract {} + + pub type MyTypeAlias = Contract; + pub type MyResultAlias = Result; + + #[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() -> MyTypeAlias { + Self {} + } + + #[ink(constructor)] + pub fn constructor_result() -> Result { + Ok(Self {}) + } + + #[ink(constructor)] + pub fn constructor_result_alias() -> MyResultAlias { + Ok(Self {}) + } + + #[ink(message)] + pub fn message(&self) {} + } +} + +fn main() {} From b5e99fe7147ec3d9122f77d296f90ff5caebe56f Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Thu, 27 Oct 2022 13:33:38 +0200 Subject: [PATCH 25/28] LLVM optimisation and refactoring --- crates/ink/src/codegen/dispatch/execution.rs | 33 +++----------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index 55e0057f5f..a92c42ad5c 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/crates/ink/src/codegen/dispatch/execution.rs @@ -70,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::< as ConstructorReturnType>::Error, - >( - ReturnFlags::default().set_reverted(true), - result.error_value(), - ) + >(ReturnFlags::default().set_reverted(true), error) } } } @@ -125,47 +122,25 @@ pub trait ConstructorReturnType: private::Sealed { /// /// 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 error_value(&self) -> &Self::Error; } impl ConstructorReturnType for private::Seal { type Error = (); - #[inline] + #[inline(always)] fn as_result(&self) -> Result<&C, &Self::Error> { Ok(&self.0) } - - #[inline] - fn error_value(&self) -> &Self::Error { - &() - } } impl ConstructorReturnType for private::Seal> { const IS_RESULT: bool = true; type Error = E; - #[inline] + #[inline(always)] fn as_result(&self) -> Result<&C, &Self::Error> { self.0.as_ref() } - - #[inline] - fn error_value(&self) -> &Self::Error { - self.0 - .as_ref() - .err() - .expect("value returned only when error occurs. qed") - } } /// Trait used to convert return types of contract initializer routines. From ae37fe68295e6cddc728758e8feb4bebf16332c5 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Thu, 27 Oct 2022 16:44:21 +0200 Subject: [PATCH 26/28] updated docs --- crates/ink/src/codegen/dispatch/execution.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index a92c42ad5c..a261e370ea 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/crates/ink/src/codegen/dispatch/execution.rs @@ -113,7 +113,10 @@ pub trait ConstructorReturnType: private::Sealed { /// /// # Note /// - /// For infallible constructors this is `()`. + /// For infallible constructors this is `()` whereas for fallible + /// 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 Error; /// Converts the return value into a `Result` instance. From 10bbbc6a0f6940eacdcd9a7d2f6a76e92ea6a08d Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Thu, 27 Oct 2022 17:39:41 +0200 Subject: [PATCH 27/28] refactoring and moving feature demo to mother contract --- crates/ink/ir/src/ir/item_impl/constructor.rs | 14 ++-- examples/mother/lib.rs | 23 ++++++ examples/return_err/.gitignore | 9 --- examples/return_err/Cargo.toml | 26 ------- examples/return_err/lib.rs | 75 ------------------- 5 files changed, 28 insertions(+), 119 deletions(-) delete mode 100755 examples/return_err/.gitignore delete mode 100755 examples/return_err/Cargo.toml delete mode 100755 examples/return_err/lib.rs diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 342f9ee515..276c26ce06 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -84,17 +84,13 @@ impl quote::ToTokens for Constructor { } impl Constructor { - /// Ensures that the return type of the ink! constructor is `Self` or `Result`. - /// - /// 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> { + /// 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, @@ -154,7 +150,7 @@ impl TryFrom for Constructor { fn try_from(method_item: syn::ImplItemMethod) -> Result { 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(); diff --git a/examples/mother/lib.rs b/examples/mother/lib.rs index c0dc79887c..bfb90698a2 100755 --- a/examples/mother/lib.rs +++ b/examples/mother/lib.rs @@ -150,6 +150,16 @@ mod mother { Default::default() } + /// Demonstrates the ability to fail a constructor safely. + #[ink(constructor)] + pub fn failed_new(fail: bool) -> Result { + if fail { + Err(Failure::Revert("Reverting instantiation".to_string())) + } else { + Ok(Default::default()) + } + } + /// Takes an auction data struct as input and returns it back. #[ink(message)] pub fn echo_auction(&mut self, auction: Auction) -> Auction { @@ -205,6 +215,19 @@ mod mother { .expect("Contract unexpected failure!"); } + #[ink::test] + fn constructor_works_or_fails() { + let contract = Mother::failed_new(true); + assert!(contract.is_err()); + assert_eq!( + contract.err(), + Some(Failure::Revert("Reverting instantiation".to_string())) + ); + + let contract = Mother::failed_new(false); + assert!(contract.is_ok()); + } + #[ink::test] #[should_panic] fn trap_works() { diff --git a/examples/return_err/.gitignore b/examples/return_err/.gitignore deleted file mode 100755 index 8de8f877e4..0000000000 --- a/examples/return_err/.gitignore +++ /dev/null @@ -1,9 +0,0 @@ -# Ignore build artifacts from the local tests sub-crate. -/target/ - -# Ignore backup files creates by cargo fmt. -**/*.rs.bk - -# Remove Cargo.lock when creating an executable, leave it for libraries -# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock -Cargo.lock diff --git a/examples/return_err/Cargo.toml b/examples/return_err/Cargo.toml deleted file mode 100755 index 58741f811b..0000000000 --- a/examples/return_err/Cargo.toml +++ /dev/null @@ -1,26 +0,0 @@ -[package] -name = "return_err" -version = "4.0.0-alpha.3" -authors = ["Parity Technologies "] -edition = "2021" -publish = false - -[dependencies] -ink = { path = "../../crates/ink", default-features = false } - -scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } -scale-info = { version = "2", default-features = false, features = ["derive"], optional = true } - -[lib] -name = "return_err" -path = "lib.rs" -crate-type = ["cdylib"] - -[features] -default = ["std"] -std = [ - "ink/std", - "scale/std", - "scale-info/std", -] -ink-as-dependency = [] diff --git a/examples/return_err/lib.rs b/examples/return_err/lib.rs deleted file mode 100755 index b733dc42cd..0000000000 --- a/examples/return_err/lib.rs +++ /dev/null @@ -1,75 +0,0 @@ -#![cfg_attr(not(feature = "std"), no_std)] - -#[ink::contract] -mod return_err { - - #[ink(storage)] - #[derive(Default)] - pub struct ReturnErr { - count: i32, - } - - /// Example of Error enum - #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)] - #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))] - pub enum Error { - Foo, - } - - impl ReturnErr { - /// Classic constructor that instantiated the contract - #[ink(constructor)] - pub fn new() -> Self { - Default::default() - } - - /// Constructor that can be manually failed - #[ink(constructor, payable)] - pub fn another_new(fail: bool) -> Result { - if fail { - Err(Error::Foo) - } else { - Ok(Default::default()) - } - } - - /// Gets the value of a counter - #[ink(message)] - pub fn get_count(&self) -> i32 { - self.count - } - - /// Changes the value of counter - #[ink(message)] - pub fn incr(&mut self, n: i32) { - self.count += n; - } - } - #[cfg(test)] - mod tests { - use super::*; - - #[ink::test] - fn classic_constructor_works() { - let mut contract = ReturnErr::new(); - contract.incr(5); - assert_eq!(contract.get_count(), 5) - } - - #[ink::test] - fn constructor_safely_fails() { - let contract = ReturnErr::another_new(true); - assert!(contract.is_err()); - assert_eq!(contract.err(), Some(Error::Foo)) - } - - #[ink::test] - fn constructor_safely_works() { - let contract = ReturnErr::another_new(false); - assert!(contract.is_ok()); - let mut contract = contract.unwrap(); - contract.incr(-5); - assert_eq!(contract.get_count(), -5); - } - } -} From b94d31f9679e92906cf5efb68d70a6cddd6ee805 Mon Sep 17 00:00:00 2001 From: SkymanOne Date: Thu, 27 Oct 2022 18:13:17 +0200 Subject: [PATCH 28/28] remove cargo-contract install in docker --- .gitlab-ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fd63e76ce6..268ab3f489 100644 --- a/.gitlab-ci.yml +++ b/.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;