From 536b50019f1e61be2e2825cb3ee25f764683865b Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 16 Nov 2022 16:43:30 -0500 Subject: [PATCH 01/30] Return `ConstructorResult` in `deploy()` --- crates/ink/codegen/src/generator/dispatch.rs | 38 ++++++++++++++------ crates/ink/src/codegen/dispatch/execution.rs | 15 ++++++-- crates/ink/src/lib.rs | 1 + crates/primitives/src/lib.rs | 4 +++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 8875fd8d51..2c8bcc65a1 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -414,16 +414,34 @@ 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) => { + use ::core::default::Default; + let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput); + + // At this point we're unable to set the `Ok` variant to be the any "real" + // construcotr 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::default().set_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))] diff --git a/crates/ink/src/codegen/dispatch/execution.rs b/crates/ink/src/codegen/dispatch/execution.rs index 6118efd360..b568a72f3c 100644 --- a/crates/ink/src/codegen/dispatch/execution.rs +++ b/crates/ink/src/codegen/dispatch/execution.rs @@ -61,6 +61,8 @@ where let result = ManuallyDrop::new(private::Seal(f())); match result.as_result() { Ok(contract) => { + let return_value = ::core::result::Result::Ok(()); + // Constructor is infallible or is fallible but succeeded. // // This requires us to sync back the changes of the contract storage. @@ -68,15 +70,22 @@ where &Contract::KEY, contract, ); - ink_env::return_value(ReturnFlags::default().set_reverted(false), &()); + ink_env::return_value::<::ink_primitives::ConstructorResult<()>>( + ReturnFlags::default().set_reverted(false), + &return_value, + ); } Err(error) => { + let return_value = ::core::result::Result::Ok(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), error) + ::ink_primitives::ConstructorResult< + & as ConstructorReturnType>::Error, + >, + >(ReturnFlags::default().set_reverted(true), &return_value) } } } diff --git a/crates/ink/src/lib.rs b/crates/ink/src/lib.rs index a426811b11..1422416741 100644 --- a/crates/ink/src/lib.rs +++ b/crates/ink/src/lib.rs @@ -72,6 +72,7 @@ pub use ink_macro::{ trait_definition, }; pub use ink_primitives::{ + ConstructorResult, LangError, MessageResult, }; diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index e4d5608e2b..1e18f825e4 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -59,3 +59,7 @@ pub enum LangError { /// The `Result` type for ink! messages. #[doc(hidden)] pub type MessageResult = ::core::result::Result; + +/// The `Result` type for ink! constructors. +#[doc(hidden)] +pub type ConstructorResult = ::core::result::Result; From 1ab55d9598897d2eb23bb82635a3cfba3bd09b82 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 16 Nov 2022 16:51:00 -0500 Subject: [PATCH 02/30] Wrap return type with `Result` in metadata --- crates/ink/codegen/src/generator/metadata.rs | 3 ++- crates/ink/ir/src/ir/item_impl/constructor.rs | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 3f7cdc2aa6..1fcc452522 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -142,7 +142,8 @@ impl Metadata<'_> { let constructor = constructor.callable(); let ident = constructor.ident(); let args = constructor.inputs().map(Self::generate_dispatch_argument); - let ret_ty = Self::generate_constructor_return_type(constructor.output()); + let ret_ty = + Self::generate_constructor_return_type(Some(&constructor.wrapped_output())); quote_spanned!(span=> ::ink::metadata::ConstructorSpec::from_label(::core::stringify!(#ident)) .selector([ diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 276c26ce06..72bb1d3acb 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/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)] From 79dff487e497dc058bed93cae2a9e7bf87d121bc Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 16 Nov 2022 17:32:20 -0500 Subject: [PATCH 03/30] Check that constructor's return Results in tests --- .../integration-flipper/lib.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/examples/lang-err-integration-tests/integration-flipper/lib.rs b/examples/lang-err-integration-tests/integration-flipper/lib.rs index a33d95cce8..f7c613a77b 100644 --- a/examples/lang-err-integration-tests/integration-flipper/lib.rs +++ b/examples/lang-err-integration-tests/integration-flipper/lib.rs @@ -52,6 +52,30 @@ pub mod integration_flipper { mod e2e_tests { type E2EResult = std::result::Result>; + #[ink_e2e::test] + async fn e2e_constructor_returns_result( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + let constructor = integration_flipper::constructors::default(); + let instantiate = client + .instantiate(&mut ink_e2e::alice(), constructor, 0, None) + .await + .expect("Instantiate `integration_flipper` failed"); + + let data = instantiate + .dry_run + .result + .expect("Dispatch to Contracts pallet failed.") + .result + .data; + let constructor_output: Result<(), ()> = scale::Decode::decode(&mut &data[..]) + .expect("We expect a `Result` from constructors now, so this should be decodable."); + + assert!(matches!(constructor_output, Ok(()))); + + Ok(()) + } + #[ink_e2e::test] async fn e2e_can_flip_correctly( mut client: ink_e2e::Client, From 228604c93fb312ba39c2d86c7503f95e291d74d9 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 16 Nov 2022 18:25:44 -0500 Subject: [PATCH 04/30] Add test showing that `Result`s are decoded correctly --- .../integration-flipper/lib.rs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/examples/lang-err-integration-tests/integration-flipper/lib.rs b/examples/lang-err-integration-tests/integration-flipper/lib.rs index f7c613a77b..a85357a5cd 100644 --- a/examples/lang-err-integration-tests/integration-flipper/lib.rs +++ b/examples/lang-err-integration-tests/integration-flipper/lib.rs @@ -19,6 +19,13 @@ pub mod integration_flipper { Self { value: init_value } } + /// Creates a new integration_flipper smart contract initialized with the given value. + #[ink(constructor)] + #[allow(clippy::result_unit_err)] + pub fn try_new(init_value: bool) -> Result { + Ok(Self { value: init_value }) + } + /// Creates a new integration_flipper smart contract initialized to `false`. #[ink(constructor)] pub fn default() -> Self { @@ -58,10 +65,12 @@ pub mod integration_flipper { ) -> E2EResult<()> { let constructor = integration_flipper::constructors::default(); let instantiate = client - .instantiate(&mut ink_e2e::alice(), constructor, 0, None) + .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) .await .expect("Instantiate `integration_flipper` failed"); + // dbg!(&instantiate.dry_run); + let data = instantiate .dry_run .result @@ -73,6 +82,55 @@ pub mod integration_flipper { assert!(matches!(constructor_output, Ok(()))); + let constructor = integration_flipper::constructors::try_new(true); + let instantiate = client + .instantiate(&mut ink_e2e::alice(), constructor, 0, None) + .await + .expect("Instantiate `integration_flipper` failed"); + dbg!(&instantiate.dry_run); + + Ok(()) + } + + #[ink_e2e::test] + async fn e2e_constructor_returns_result_of_result( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + let constructor = integration_flipper::constructors::try_new(true); + let instantiate = client + .instantiate(&mut ink_e2e::dave(), constructor, 0, None) + .await + .expect("Instantiate `integration_flipper` failed"); + + // This comes back as: + // + // + // InstantiateReturnValue { + // result: ExecReturnValue { + // flags: (empty), + // data: [ + // 0, + // ], + // }, + // account_id: 4fa8528f2bed2d8fc186075067b6940f0d3229c7b977f3a4d0378c1781b3c1e8 (5Ds9eCso...), + // } + // + // Pretty sure we should have another thing for the wrapped `Result` beyond just `0`. + // + // The metadata between `new` and `try_new` look the same, which is suspicious... + dbg!(&instantiate.dry_run); + + let data = instantiate + .dry_run + .result + .expect("Dispatch to Contracts pallet failed.") + .result + .data; + let constructor_output: Result, ()> = scale::Decode::decode(&mut &data[..]) + .expect("We expect a `Result` from constructors now, so this should be decodable."); + + assert!(matches!(constructor_output, Ok(Ok(())))); + Ok(()) } From bb4c1a2db737075745e02e2f56b698913ac29472 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 12:48:50 -0500 Subject: [PATCH 05/30] Correctly generate metadata for constructors --- crates/ink/codegen/src/generator/metadata.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 1fcc452522..6fe1d172d0 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -376,15 +376,16 @@ impl Metadata<'_> { Some(syn::Type::Path(syn::TypePath { qself: None, path })) if path.is_ident("Self") => { - quote! { ::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None)} + quote! { + ::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None) + } } Some(ty) => { - let type_token = Self::replace_self_with_unit(ty); - let segments = Self::generate_constructor_type_segments(ty); + let ty = Self::replace_self_with_unit(ty); + let ty: syn::Type = syn::parse2(ty).unwrap(); + let type_spec = Self::generate_type_spec(&ty); quote! { - ::ink::metadata::ReturnTypeSpec::new( - <#type_token as ::ink::metadata::ConstructorReturnSpec>::generate(#segments) - ) + ::ink::metadata::ReturnTypeSpec::new(#type_spec) } } } From 27b6b97342a0f35f10d218c78a5db79748a67315 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 16:09:27 -0500 Subject: [PATCH 06/30] Fix some nitpicks from Andrew's PR --- crates/e2e/src/client.rs | 6 +++--- crates/ink/codegen/src/generator/dispatch.rs | 11 ++++++----- crates/ink/codegen/src/generator/metadata.rs | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/e2e/src/client.rs b/crates/e2e/src/client.rs index 65a06aa3b2..1843cc87c9 100644 --- a/crates/e2e/src/client.rs +++ b/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 diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 8c04d85535..7dd7ad0380 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/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 ),* ) @@ -625,14 +625,15 @@ 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 { + if #constructor_value::IS_RESULT { ::ink::env::return_value::<::core::result::Result<&(), ()>>( ::ink::env::ReturnFlags::new_with_reverted(false), &::core::result::Result::Ok(&()) @@ -642,7 +643,7 @@ impl Dispatch<'_> { } }, ::core::result::Result::Err(err) => { - ::ink::env::return_value::<::core::result::Result<(), & #constructor_value :: Error>>( + ::ink::env::return_value::<::core::result::Result<(), & #constructor_value::Error>>( ::ink::env::ReturnFlags::new_with_reverted(true), &::core::result::Result::Err(err) ) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 979e53b16c..84aac9e0ee 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/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, From eb10da69d6435500d3120d23904df35d3c29a691 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 17:01:34 -0500 Subject: [PATCH 07/30] Wrap dispatch return types with `Ok` These can't return dispatch errors atm. I may want to clean up how this is done later. --- crates/ink/codegen/src/generator/dispatch.rs | 26 +++++++++++--------- crates/ink/codegen/src/generator/metadata.rs | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 7dd7ad0380..0a114fa244 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -427,17 +427,16 @@ impl Dispatch<'_> { decoded_dispatchable } ::core::result::Result::Err(_decoding_error) => { - use ::core::default::Default; let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput); // At this point we're unable to set the `Ok` variant to be the any "real" - // construcotr output since we were unable to figure out what the caller wanted + // 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::default().set_reverted(true), + ::ink::env::ReturnFlags::new_with_reverted(true), &error, ); } @@ -634,20 +633,23 @@ impl Dispatch<'_> { // only fallible constructors return success `Ok` back to the caller. if #constructor_value::IS_RESULT { - ::ink::env::return_value::<::core::result::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)), + ), } } ) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 84aac9e0ee..ebd8c0ab83 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -345,7 +345,7 @@ impl Metadata<'_> { ); 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< (), From 8b1aa667f89ef982d353d354dcbc90229137f8e6 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 17:26:18 -0500 Subject: [PATCH 08/30] Manually wrap metadata return with `ConstructorResult` --- crates/ink/codegen/src/generator/metadata.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index ebd8c0ab83..e6f8599037 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -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 { ::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" + ) + ) } ) ) From 36556e70c1664f5ae1550e4b120364e10d202c84 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 18:00:17 -0500 Subject: [PATCH 09/30] Fix existing constructor integration tests --- .../constructors-return-value/lib.rs | 41 +++++++++++-------- .../integration-flipper/lib.rs | 16 -------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/examples/lang-err-integration-tests/constructors-return-value/lib.rs b/examples/lang-err-integration-tests/constructors-return-value/lib.rs index 5763288a0a..b004fcc54a 100644 --- a/examples/lang-err-integration-tests/constructors-return-value/lib.rs +++ b/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(()) } diff --git a/examples/lang-err-integration-tests/integration-flipper/lib.rs b/examples/lang-err-integration-tests/integration-flipper/lib.rs index a85357a5cd..e954888243 100644 --- a/examples/lang-err-integration-tests/integration-flipper/lib.rs +++ b/examples/lang-err-integration-tests/integration-flipper/lib.rs @@ -102,22 +102,6 @@ pub mod integration_flipper { .await .expect("Instantiate `integration_flipper` failed"); - // This comes back as: - // - // - // InstantiateReturnValue { - // result: ExecReturnValue { - // flags: (empty), - // data: [ - // 0, - // ], - // }, - // account_id: 4fa8528f2bed2d8fc186075067b6940f0d3229c7b977f3a4d0378c1781b3c1e8 (5Ds9eCso...), - // } - // - // Pretty sure we should have another thing for the wrapped `Result` beyond just `0`. - // - // The metadata between `new` and `try_new` look the same, which is suspicious... dbg!(&instantiate.dry_run); let data = instantiate From e61576ebdb047cb8619176eb64bdd06f0da91e46 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 18:28:31 -0500 Subject: [PATCH 10/30] Remove constructor related test from `integration-flipper` --- .../integration-flipper/lib.rs | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/examples/lang-err-integration-tests/integration-flipper/lib.rs b/examples/lang-err-integration-tests/integration-flipper/lib.rs index e954888243..a33d95cce8 100644 --- a/examples/lang-err-integration-tests/integration-flipper/lib.rs +++ b/examples/lang-err-integration-tests/integration-flipper/lib.rs @@ -19,13 +19,6 @@ pub mod integration_flipper { Self { value: init_value } } - /// Creates a new integration_flipper smart contract initialized with the given value. - #[ink(constructor)] - #[allow(clippy::result_unit_err)] - pub fn try_new(init_value: bool) -> Result { - Ok(Self { value: init_value }) - } - /// Creates a new integration_flipper smart contract initialized to `false`. #[ink(constructor)] pub fn default() -> Self { @@ -59,65 +52,6 @@ pub mod integration_flipper { mod e2e_tests { type E2EResult = std::result::Result>; - #[ink_e2e::test] - async fn e2e_constructor_returns_result( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - let constructor = integration_flipper::constructors::default(); - let instantiate = client - .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) - .await - .expect("Instantiate `integration_flipper` failed"); - - // dbg!(&instantiate.dry_run); - - let data = instantiate - .dry_run - .result - .expect("Dispatch to Contracts pallet failed.") - .result - .data; - let constructor_output: Result<(), ()> = scale::Decode::decode(&mut &data[..]) - .expect("We expect a `Result` from constructors now, so this should be decodable."); - - assert!(matches!(constructor_output, Ok(()))); - - let constructor = integration_flipper::constructors::try_new(true); - let instantiate = client - .instantiate(&mut ink_e2e::alice(), constructor, 0, None) - .await - .expect("Instantiate `integration_flipper` failed"); - dbg!(&instantiate.dry_run); - - Ok(()) - } - - #[ink_e2e::test] - async fn e2e_constructor_returns_result_of_result( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - let constructor = integration_flipper::constructors::try_new(true); - let instantiate = client - .instantiate(&mut ink_e2e::dave(), constructor, 0, None) - .await - .expect("Instantiate `integration_flipper` failed"); - - dbg!(&instantiate.dry_run); - - let data = instantiate - .dry_run - .result - .expect("Dispatch to Contracts pallet failed.") - .result - .data; - let constructor_output: Result, ()> = scale::Decode::decode(&mut &data[..]) - .expect("We expect a `Result` from constructors now, so this should be decodable."); - - assert!(matches!(constructor_output, Ok(Ok(())))); - - Ok(()) - } - #[ink_e2e::test] async fn e2e_can_flip_correctly( mut client: ink_e2e::Client, From a3e0b2470045c49da4026d87c7ac95ac48c3dac8 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 18:29:09 -0500 Subject: [PATCH 11/30] Fix compile tests --- ...uctor-return-result-non-codec-error.stderr | 6 ++-- .../pass/constructor-return-result-alias.rs | 35 +++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) 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 f6256d78c5..19d2d86df6 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,8 +1,8 @@ -error[E0277]: the trait bound `Result<(), &contract::Error>: Encode` is not satisfied +error[E0277]: the trait bound `Result, LangError>: Encode` is not satisfied --> tests/ui/contract/fail/constructor-return-result-non-codec-error.rs:13:9 | 13 | pub fn constructor() -> Result { - | ^^^ the trait `Encode` is not implemented for `Result<(), &contract::Error>` + | ^^^ the trait `Encode` is not implemented for `Result, LangError>` | = help: the trait `Encode` is implemented for `Result` 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, LangError>` to implement `TypeInfo` note: required by a bound in `TypeSpec::with_name_str` --> $WORKSPACE/crates/metadata/src/specs.rs | 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 index 08f48c4a71..4fe4f90f79 100644 --- a/crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs +++ b/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" ); From fb8d293ab45528d3001fb3cf53b78e8d197a8530 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 17 Nov 2022 18:32:44 -0500 Subject: [PATCH 12/30] Add `ident` to dictionary --- .config/cargo_spellcheck.dic | 1 + 1 file changed, 1 insertion(+) diff --git a/.config/cargo_spellcheck.dic b/.config/cargo_spellcheck.dic index aaf325fb8a..463c3ac312 100644 --- a/.config/cargo_spellcheck.dic +++ b/.config/cargo_spellcheck.dic @@ -46,6 +46,7 @@ evaluable fuzzer getter growable +ident interoperate invariants kB From 394ffca60a7b1b71b79c7404fbd27b392841ae77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 18 Nov 2022 13:58:50 +0100 Subject: [PATCH 13/30] Simplify code --- crates/ink/codegen/src/generator/dispatch.rs | 41 +++++++------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 0a114fa244..05326379b4 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -623,34 +623,23 @@ impl Dispatch<'_> { let result: #constructor_output = #constructor_callable(input); let output_value = ::ink::reflect::ConstructorOutputValue::new(result); + let output_result = #constructor_value::as_result(&output_value); - match #constructor_value::as_result(&output_value) { - ::core::result::Result::Ok(contract) => { - ::ink::env::set_contract_storage::<::ink::primitives::Key, #storage_ident>( - &<#storage_ident as ::ink::storage::traits::StorageKey>::KEY, - contract, - ); - - // only fallible constructors return success `Ok` back to the caller. - if #constructor_value::IS_RESULT { - ::ink::env::return_value::<::ink::ConstructorResult<::core::result::Result<(), ()>>>( - ::ink::env::ReturnFlags::new_with_reverted(false), - &::core::result::Result::Ok(::core::result::Result::Ok(())), - ) - } else { - ::ink::env::return_value::<::ink::ConstructorResult<()>>( - ::ink::env::ReturnFlags::new_with_reverted(false), - &::core::result::Result::Ok(()), - ) - } - } - ::core::result::Result::Err(err) => ::ink::env::return_value::< - ::ink::ConstructorResult<::core::result::Result<(), &#constructor_value::Error>>, - >( - ::ink::env::ReturnFlags::new_with_reverted(false), - &::core::result::Result::Ok(::core::result::Result::Err(err)), - ), + if let Ok(contract) = output_result.as_ref() { + ::ink::env::set_contract_storage::<::ink::primitives::Key, #storage_ident>( + &<#storage_ident as ::ink::storage::traits::StorageKey>::KEY, + contract, + ); } + + ::ink::env::return_value::< + ::ink::ConstructorResult< + ::core::result::Result<(), &#constructor_value::Error> + >, + >( + ::ink::env::ReturnFlags::new_with_reverted(output_result.is_err()), + &::ink::ConstructorResult::Ok(output_result.map(|_| ())), + ); } ) }); From 2f026a23f1255dbf7e6a48d61ac0046acff207a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 18 Nov 2022 14:44:34 +0100 Subject: [PATCH 14/30] Driveby: Also simplify call dispatch --- crates/ink/codegen/src/generator/dispatch.rs | 24 +++++++------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index 05326379b4..dc81ef61e1 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -824,27 +824,19 @@ impl Dispatch<'_> { } let result: #message_output = #message_callable(&mut contract, input); - let failure = ::ink::is_result_type!(#message_output) + let is_reverted = ::ink::is_result_type!(#message_output) && ::ink::is_result_err!(result); - // Currently no `LangError`s are raised at this level of the dispatch logic - // so `Ok` is always returned to the caller. - let return_value = ::core::result::Result::Ok(result); - - if failure { - // We return early here since there is no need to push back the - // intermediate results of the contract - the transaction is going to be - // reverted anyways. - ::ink::env::return_value::<::ink::MessageResult::<#message_output>>( - ::ink::env::ReturnFlags::new_with_reverted(true), - &return_value - ) + // no need to push back results: transaction gets reverted anyways + if !is_reverted { + push_contract(contract, #mutates_storage); } - push_contract(contract, #mutates_storage); - ::ink::env::return_value::<::ink::MessageResult::<#message_output>>( - ::ink::env::ReturnFlags::new_with_reverted(false), &return_value + ::ink::env::ReturnFlags::new_with_reverted(is_reverted), + // Currently no `LangError`s are raised at this level of the + // dispatch logic so `Ok` is always returned to the caller. + &::ink::MessageResult::Ok(result), ) } ) From c501d849c699653caa9aeb5820b1f2bab081e0c5 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 18 Nov 2022 12:28:19 -0500 Subject: [PATCH 15/30] Small fixes to type paths --- crates/ink/codegen/src/generator/dispatch.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ink/codegen/src/generator/dispatch.rs b/crates/ink/codegen/src/generator/dispatch.rs index dc81ef61e1..4246d98210 100644 --- a/crates/ink/codegen/src/generator/dispatch.rs +++ b/crates/ink/codegen/src/generator/dispatch.rs @@ -427,7 +427,7 @@ impl Dispatch<'_> { decoded_dispatchable } ::core::result::Result::Err(_decoding_error) => { - let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput); + let error = ::ink::ConstructorResult::Err(::ink::LangError::CouldNotReadInput); // At this point we're unable to set the `Ok` variant to be the any "real" // constructor output since we were unable to figure out what the caller wanted @@ -465,7 +465,7 @@ impl Dispatch<'_> { decoded_dispatchable } ::core::result::Result::Err(_decoding_error) => { - let error = ::core::result::Result::Err(::ink::LangError::CouldNotReadInput); + let error = ::ink::MessageResult::Err(::ink::LangError::CouldNotReadInput); // At this point we're unable to set the `Ok` variant to be the any "real" // message output since we were unable to figure out what the caller wanted @@ -625,7 +625,7 @@ impl Dispatch<'_> { let output_value = ::ink::reflect::ConstructorOutputValue::new(result); let output_result = #constructor_value::as_result(&output_value); - if let Ok(contract) = output_result.as_ref() { + if let ::core::result::Result::Ok(contract) = output_result.as_ref() { ::ink::env::set_contract_storage::<::ink::primitives::Key, #storage_ident>( &<#storage_ident as ::ink::storage::traits::StorageKey>::KEY, contract, @@ -638,6 +638,8 @@ impl Dispatch<'_> { >, >( ::ink::env::ReturnFlags::new_with_reverted(output_result.is_err()), + // Currently no `LangError`s are raised at this level of the + // dispatch logic so `Ok` is always returned to the caller. &::ink::ConstructorResult::Ok(output_result.map(|_| ())), ); } From c307516e8fff58a4dcf912df222cfc007e3f9268 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 18 Nov 2022 12:52:11 -0500 Subject: [PATCH 16/30] Check that actual instantiate call fails --- .../constructors-return-value/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/examples/lang-err-integration-tests/constructors-return-value/lib.rs b/examples/lang-err-integration-tests/constructors-return-value/lib.rs index b004fcc54a..94dbf2f1e7 100644 --- a/examples/lang-err-integration-tests/constructors-return-value/lib.rs +++ b/examples/lang-err-integration-tests/constructors-return-value/lib.rs @@ -208,6 +208,15 @@ pub mod constructors_return_value { "Fallible constructor should have failed" ); + let result = client + .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) + .await; + + assert!( + matches!(result, Err(ink_e2e::Error::InstantiateExtrinsic(_))), + "Constructor should fail" + ); + Ok(()) } } From aaebac4f1345b2d9e4af0d2fa70cf7b9c53f25ea Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 18 Nov 2022 16:12:46 -0500 Subject: [PATCH 17/30] Add some tests using the `CreateBuilder` --- .../call-builder/Cargo.toml | 6 + .../call-builder/lib.rs | 154 +++++++++++++++++- 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/examples/lang-err-integration-tests/call-builder/Cargo.toml b/examples/lang-err-integration-tests/call-builder/Cargo.toml index 25aaf12a4e..01bdb28cc3 100755 --- a/examples/lang-err-integration-tests/call-builder/Cargo.toml +++ b/examples/lang-err-integration-tests/call-builder/Cargo.toml @@ -11,6 +11,9 @@ ink = { path = "../../../crates/ink", default-features = false } scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } scale-info = { version = "2.3", default-features = false, features = ["derive"], optional = true } +constructors_return_value = { path = "../constructors-return-value", default-features = false, features = ["ink-as-dependency"] } +integration_flipper = { path = "../integration-flipper", default-features = false, features = ["ink-as-dependency"] } + [dev-dependencies] ink_e2e = { path = "../../../crates/e2e" } @@ -28,6 +31,9 @@ std = [ "ink/std", "scale/std", "scale-info/std", + + "constructors_return_value/std", + "integration_flipper/std", ] ink-as-dependency = [] e2e-tests = [] diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index a56fc3b5f3..d672aafb81 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -55,13 +55,66 @@ mod call_builder { } } } + + #[ink(message)] + pub fn call_instantiate( + &mut self, + code_hash: Hash, + selector: [u8; 4], + init_value: bool, + ) -> Option<::ink::LangError> { + use ink::env::{ + call::{ + build_create, + ExecutionInput, + Selector, + }, + DefaultEnvironment, + }; + + // let _my_contract: constructors_return_value::ConstructorsReturnValueRef = + let result = build_create::< + DefaultEnvironment, + constructors_return_value::ConstructorsReturnValueRef, + >() + .code_hash(code_hash) + .gas_limit(0) + .endowment(0) + .exec_input(ExecutionInput::new(Selector::new(selector)).push_arg(init_value)) + .salt_bytes(&[0xDE, 0xAD, 0xBE, 0xEF]) + .params() + .instantiate(); + ::ink::env::debug_println!("Result from `instantiate` {:?}", &result); + let contract: constructors_return_value::ConstructorsReturnValueRef = + result.expect("Error from the Contracts pallet."); + + None + + // let result = build_call::() + // .call_type(Call::new().callee(address)) + // .exec_input(ExecutionInput::new(Selector::new(selector))) + // .returns::>() + // .fire() + // .expect("Error from the Contracts pallet."); + + // match result { + // Ok(_) => None, + // Err(e @ ink::LangError::CouldNotReadInput) => Some(e), + // Err(_) => { + // unimplemented!("No other `LangError` variants exist at the moment.") + // } + // } + } } #[cfg(all(test, feature = "e2e-tests"))] mod e2e_tests { type E2EResult = std::result::Result>; - #[ink_e2e::test(additional_contracts = "../integration-flipper/Cargo.toml")] + // TODO: Add to open issue that only the first `additional_contract` arg works + #[ink_e2e::test( + additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" + )] async fn e2e_invalid_selector_can_be_handled( mut client: ink_e2e::Client, ) -> E2EResult<()> { @@ -137,5 +190,104 @@ mod call_builder { Ok(()) } + + #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] + async fn e2e_valid_constructor_create_builder( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + use call_builder::contract_types::ink_primitives::{ + types::AccountId as E2EAccountId, + LangError as E2ELangError, + }; + + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::dave(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let code_hash = client + .upload( + &mut ink_e2e::dave(), + constructors_return_value::CONTRACT_PATH, + None, + ) + .await + .expect("upload `constructors_return_value` failed") + .code_hash; + + let new_selector = [0x9B, 0xAE, 0x9D, 0x5E]; + let _invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::dave(), + contract_acc_id.clone(), + call_builder::messages::call_instantiate( + ink_e2e::utils::runtime_hash_to_ink_hash::< + ink::env::DefaultEnvironment, + >(&code_hash), + new_selector, + true, + ), + 0, + None, + ) + .await + .expect("Calling `call_builder::call_instantiate` failed"); + dbg!(&call_result.value); + + Ok(()) + } + + #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] + async fn e2e_invalid_constructor_create_builder( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + use call_builder::contract_types::ink_primitives::{ + types::AccountId as E2EAccountId, + LangError as E2ELangError, + }; + + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::eve(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let code_hash = client + .upload( + &mut ink_e2e::eve(), + constructors_return_value::CONTRACT_PATH, + None, + ) + .await + .expect("upload `constructors_return_value` failed") + .code_hash; + + // let new_selector = [0x9B, 0xAE, 0x9D, 0x5E]; + let invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::eve(), + contract_acc_id.clone(), + call_builder::messages::call_instantiate( + ink_e2e::utils::runtime_hash_to_ink_hash::< + ink::env::DefaultEnvironment, + >(&code_hash), + invalid_selector, + true, + ), + 0, + None, + ) + .await + .expect("Calling `call_builder::call_instantiate` failed"); + dbg!(&call_result.dry_run); + dbg!(&call_result.value); + + Ok(()) + } } } From b63ef910b6c02a563cd70b081adbe90ba24ffb12 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 18 Nov 2022 19:40:29 -0500 Subject: [PATCH 18/30] Clean up the `create_builder` tests --- .../call-builder/lib.rs | 65 +++++++------------ 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index d672aafb81..fccfff0c43 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -62,7 +62,7 @@ mod call_builder { code_hash: Hash, selector: [u8; 4], init_value: bool, - ) -> Option<::ink::LangError> { + ) -> Option { use ink::env::{ call::{ build_create, @@ -72,7 +72,6 @@ mod call_builder { DefaultEnvironment, }; - // let _my_contract: constructors_return_value::ConstructorsReturnValueRef = let result = build_create::< DefaultEnvironment, constructors_return_value::ConstructorsReturnValueRef, @@ -84,26 +83,10 @@ mod call_builder { .salt_bytes(&[0xDE, 0xAD, 0xBE, 0xEF]) .params() .instantiate(); - ::ink::env::debug_println!("Result from `instantiate` {:?}", &result); - let contract: constructors_return_value::ConstructorsReturnValueRef = - result.expect("Error from the Contracts pallet."); - - None - - // let result = build_call::() - // .call_type(Call::new().callee(address)) - // .exec_input(ExecutionInput::new(Selector::new(selector))) - // .returns::>() - // .fire() - // .expect("Error from the Contracts pallet."); - // match result { - // Ok(_) => None, - // Err(e @ ink::LangError::CouldNotReadInput) => Some(e), - // Err(_) => { - // unimplemented!("No other `LangError` variants exist at the moment.") - // } - // } + // NOTE: Right now we can't handle any `LangError` from `instantiate`, we can only tell + // that our contract reverted (i.e we see error from the Contracts pallet). + result.ok().map(|id| ink::ToAccountId::to_account_id(&id)) } } @@ -111,11 +94,10 @@ mod call_builder { mod e2e_tests { type E2EResult = std::result::Result>; - // TODO: Add to open issue that only the first `additional_contract` arg works #[ink_e2e::test( additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" )] - async fn e2e_invalid_selector_can_be_handled( + async fn e2e_invalid_message_selector_can_be_handled( mut client: ink_e2e::Client, ) -> E2EResult<()> { use call_builder::contract_types::ink_primitives::{ @@ -192,14 +174,9 @@ mod call_builder { } #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] - async fn e2e_valid_constructor_create_builder( + async fn e2e_create_builder_works_with_valid_selector( mut client: ink_e2e::Client, ) -> E2EResult<()> { - use call_builder::contract_types::ink_primitives::{ - types::AccountId as E2EAccountId, - LangError as E2ELangError, - }; - let constructor = call_builder::constructors::new(); let contract_acc_id = client .instantiate(&mut ink_e2e::dave(), constructor, 0, None) @@ -218,7 +195,6 @@ mod call_builder { .code_hash; let new_selector = [0x9B, 0xAE, 0x9D, 0x5E]; - let _invalid_selector = [0x00, 0x00, 0x00, 0x00]; let call_result = client .call( &mut ink_e2e::dave(), @@ -234,21 +210,22 @@ mod call_builder { None, ) .await - .expect("Calling `call_builder::call_instantiate` failed"); - dbg!(&call_result.value); + .expect("Client failed to call `call_builder::call_instantiate`.") + .value + .expect("Dispatching `call_builder::call_instantiate` failed."); + + assert!( + call_result.is_some(), + "Call using valid selector failed, when it should've succeeded." + ); Ok(()) } #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] - async fn e2e_invalid_constructor_create_builder( + async fn e2e_create_builder_fails_with_invalid_selector( mut client: ink_e2e::Client, ) -> E2EResult<()> { - use call_builder::contract_types::ink_primitives::{ - types::AccountId as E2EAccountId, - LangError as E2ELangError, - }; - let constructor = call_builder::constructors::new(); let contract_acc_id = client .instantiate(&mut ink_e2e::eve(), constructor, 0, None) @@ -266,7 +243,6 @@ mod call_builder { .expect("upload `constructors_return_value` failed") .code_hash; - // let new_selector = [0x9B, 0xAE, 0x9D, 0x5E]; let invalid_selector = [0x00, 0x00, 0x00, 0x00]; let call_result = client .call( @@ -283,9 +259,14 @@ mod call_builder { None, ) .await - .expect("Calling `call_builder::call_instantiate` failed"); - dbg!(&call_result.dry_run); - dbg!(&call_result.value); + .expect("Client failed to call `call_builder::call_instantiate`.") + .value + .expect("Dispatching `call_builder::call_instantiate` failed."); + + assert!( + call_result.is_none(), + "Call using invalid selector succeeded, when it should've failed." + ); Ok(()) } From ec6253d2b51918e25fa81ffc6213a3f9a01d48a6 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 09:11:50 -0500 Subject: [PATCH 19/30] Remove unused method --- crates/ink/ir/src/ir/item_impl/constructor.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/crates/ink/ir/src/ir/item_impl/constructor.rs b/crates/ink/ir/src/ir/item_impl/constructor.rs index 72bb1d3acb..276c26ce06 100644 --- a/crates/ink/ir/src/ir/item_impl/constructor.rs +++ b/crates/ink/ir/src/ir/item_impl/constructor.rs @@ -227,21 +227,6 @@ 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)] From d56b728db0045c16fdb3ad799ecf0302cf51a4ad Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 13:22:11 -0500 Subject: [PATCH 20/30] Format code for generating constructor return type --- crates/ink/codegen/src/generator/metadata.rs | 31 ++++++-------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index e6f8599037..d685c99785 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -345,28 +345,15 @@ impl Metadata<'_> { ); quote_spanned!(span=> - ::ink::metadata::ReturnTypeSpec::new( - if #constructor_info::IS_RESULT { - ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::< - ::ink::ConstructorResult<::core::result::Result< - (), - #constructor_info::Error - >> - >( - "ink_primitives::ConstructorResult" - ) - ) - } else { - ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::< - ::ink::ConstructorResult< - (), - > - >( - "ink_primitives::ConstructorResult" - ) - ) - } - ) + ::ink::metadata::ReturnTypeSpec::new(if #constructor_info::IS_RESULT { + ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::< + ::ink::ConstructorResult<::core::result::Result<(), #constructor_info::Error>>, + >("ink_primitives::ConstructorResult")) + } else { + ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::< + ::ink::ConstructorResult<()>, + >("ink_primitives::ConstructorResult")) + }) ) } From 053f57069f33d16c97a25a39b2c86ad6dc198f2e Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 14:16:53 -0500 Subject: [PATCH 21/30] Add `constructors-return-value` to CI --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3bf7ff351d..a77ca82499 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -34,7 +34,7 @@ variables: ALL_CRATES: "${PURELY_STD_CRATES} ${ALSO_WASM_CRATES}" DELEGATOR_SUBCONTRACTS: "accumulator adder subber" UPGRADEABLE_CONTRACTS: "forward-calls set-code-hash" - LANG_ERR_INTEGRATION_CONTRACTS: "integration-flipper call-builder contract-ref" + LANG_ERR_INTEGRATION_CONTRACTS: "integration-flipper call-builder contract-ref constructors-return-value" # TODO `cargo clippy --verbose --all-targets --all-features` for this crate # currently fails on `stable`, but succeeds on `nightly`. This is due to # this fix not yet in stable: https://github.com/rust-lang/rust-clippy/issues/8895. From 73058875dcc901ff7ffa7423b2c5a054e4565e14 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 14:17:42 -0500 Subject: [PATCH 22/30] Move shared imports out of messages --- .../call-builder/lib.rs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index fccfff0c43..db5462df5d 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -9,6 +9,14 @@ #[ink::contract] mod call_builder { + use ink::env::{ + call::{ + Call, + ExecutionInput, + Selector, + }, + DefaultEnvironment, + }; #[ink(storage)] #[derive(Default)] @@ -30,15 +38,7 @@ mod call_builder { address: AccountId, selector: [u8; 4], ) -> Option<::ink::LangError> { - use ink::env::{ - call::{ - build_call, - Call, - ExecutionInput, - Selector, - }, - DefaultEnvironment, - }; + use ink::env::call::build_call; let result = build_call::() .call_type(Call::new().callee(address)) @@ -63,14 +63,7 @@ mod call_builder { selector: [u8; 4], init_value: bool, ) -> Option { - use ink::env::{ - call::{ - build_create, - ExecutionInput, - Selector, - }, - DefaultEnvironment, - }; + use ink::env::call::build_create; let result = build_create::< DefaultEnvironment, From 0ef75f9bf2739308cc86b00756546e12d8b1bed2 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 14:27:24 -0500 Subject: [PATCH 23/30] Appease Clippy --- .../constructors-return-value/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/examples/lang-err-integration-tests/constructors-return-value/lib.rs b/examples/lang-err-integration-tests/constructors-return-value/lib.rs index 94dbf2f1e7..788f255054 100644 --- a/examples/lang-err-integration-tests/constructors-return-value/lib.rs +++ b/examples/lang-err-integration-tests/constructors-return-value/lib.rs @@ -55,9 +55,8 @@ pub mod constructors_return_value { }, >>::IDS[0]; - assert_eq!( - >::IS_RESULT, - false + assert!( + !>::IS_RESULT, ); assert_eq!( TypeId::of::< @@ -76,9 +75,8 @@ pub mod constructors_return_value { }, >>::IDS[1]; - assert_eq!( + assert!( >::IS_RESULT, - true ); assert_eq!( TypeId::of::< From 4607a67df46fb53a9b946dc7a808fe74eafe5859 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 14:35:26 -0500 Subject: [PATCH 24/30] Appease Clippy --- .../lang-err-integration-tests/constructors-return-value/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/lang-err-integration-tests/constructors-return-value/lib.rs b/examples/lang-err-integration-tests/constructors-return-value/lib.rs index 788f255054..76fbae6e61 100644 --- a/examples/lang-err-integration-tests/constructors-return-value/lib.rs +++ b/examples/lang-err-integration-tests/constructors-return-value/lib.rs @@ -47,6 +47,7 @@ pub mod constructors_return_value { use std::any::TypeId; #[test] + #[allow(clippy::assertions_on_constants)] fn infallible_constructor_reflection() { const ID: u32 = Date: Mon, 21 Nov 2022 15:15:06 -0500 Subject: [PATCH 25/30] Try flipping order of tests --- .../call-builder/lib.rs | 100 +++++++++--------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index db5462df5d..8b5d3154c7 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -87,6 +87,57 @@ mod call_builder { mod e2e_tests { type E2EResult = std::result::Result>; + #[ink_e2e::test( + additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" + )] + async fn e2e_create_builder_fails_with_invalid_selector( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::eve(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let code_hash = client + .upload( + &mut ink_e2e::eve(), + constructors_return_value::CONTRACT_PATH, + None, + ) + .await + .expect("upload `constructors_return_value` failed") + .code_hash; + + let invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::eve(), + contract_acc_id.clone(), + call_builder::messages::call_instantiate( + ink_e2e::utils::runtime_hash_to_ink_hash::< + ink::env::DefaultEnvironment, + >(&code_hash), + invalid_selector, + true, + ), + 0, + None, + ) + .await + .expect("Client failed to call `call_builder::call_instantiate`.") + .value + .expect("Dispatching `call_builder::call_instantiate` failed."); + + assert!( + call_result.is_none(), + "Call using invalid selector succeeded, when it should've failed." + ); + + Ok(()) + } + #[ink_e2e::test( additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" )] @@ -214,54 +265,5 @@ mod call_builder { Ok(()) } - - #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] - async fn e2e_create_builder_fails_with_invalid_selector( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - let constructor = call_builder::constructors::new(); - let contract_acc_id = client - .instantiate(&mut ink_e2e::eve(), constructor, 0, None) - .await - .expect("instantiate failed") - .account_id; - - let code_hash = client - .upload( - &mut ink_e2e::eve(), - constructors_return_value::CONTRACT_PATH, - None, - ) - .await - .expect("upload `constructors_return_value` failed") - .code_hash; - - let invalid_selector = [0x00, 0x00, 0x00, 0x00]; - let call_result = client - .call( - &mut ink_e2e::eve(), - contract_acc_id.clone(), - call_builder::messages::call_instantiate( - ink_e2e::utils::runtime_hash_to_ink_hash::< - ink::env::DefaultEnvironment, - >(&code_hash), - invalid_selector, - true, - ), - 0, - None, - ) - .await - .expect("Client failed to call `call_builder::call_instantiate`.") - .value - .expect("Dispatching `call_builder::call_instantiate` failed."); - - assert!( - call_result.is_none(), - "Call using invalid selector succeeded, when it should've failed." - ); - - Ok(()) - } } } From 5d0f164b1338a3a928fe35a8cffa6a60b2f1da77 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 15:17:27 -0500 Subject: [PATCH 26/30] Change message name Maybe it's a naming clash? --- examples/lang-err-integration-tests/call-builder/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index 8b5d3154c7..c264b8597f 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -33,7 +33,7 @@ mod call_builder { /// Since we can't use the `CallBuilder` in a test environment directly we need this /// wrapper to test things like crafting calls with invalid selectors. #[ink(message)] - pub fn call( + pub fn call_build( &mut self, address: AccountId, selector: [u8; 4], @@ -183,7 +183,10 @@ mod call_builder { .call( &mut ink_e2e::charlie(), contract_acc_id.clone(), - call_builder::messages::call(flipper_ink_acc_id, invalid_selector), + call_builder::messages::call_build( + flipper_ink_acc_id, + invalid_selector, + ), 0, None, ) From e8b4e75e437d9322d7d4967c5c2a6f3dbf8b8d47 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 15:24:24 -0500 Subject: [PATCH 27/30] Revert last two changes --- .../call-builder/lib.rs | 107 +++++++++--------- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index c264b8597f..db5462df5d 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -33,7 +33,7 @@ mod call_builder { /// Since we can't use the `CallBuilder` in a test environment directly we need this /// wrapper to test things like crafting calls with invalid selectors. #[ink(message)] - pub fn call_build( + pub fn call( &mut self, address: AccountId, selector: [u8; 4], @@ -87,57 +87,6 @@ mod call_builder { mod e2e_tests { type E2EResult = std::result::Result>; - #[ink_e2e::test( - additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" - )] - async fn e2e_create_builder_fails_with_invalid_selector( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - let constructor = call_builder::constructors::new(); - let contract_acc_id = client - .instantiate(&mut ink_e2e::eve(), constructor, 0, None) - .await - .expect("instantiate failed") - .account_id; - - let code_hash = client - .upload( - &mut ink_e2e::eve(), - constructors_return_value::CONTRACT_PATH, - None, - ) - .await - .expect("upload `constructors_return_value` failed") - .code_hash; - - let invalid_selector = [0x00, 0x00, 0x00, 0x00]; - let call_result = client - .call( - &mut ink_e2e::eve(), - contract_acc_id.clone(), - call_builder::messages::call_instantiate( - ink_e2e::utils::runtime_hash_to_ink_hash::< - ink::env::DefaultEnvironment, - >(&code_hash), - invalid_selector, - true, - ), - 0, - None, - ) - .await - .expect("Client failed to call `call_builder::call_instantiate`.") - .value - .expect("Dispatching `call_builder::call_instantiate` failed."); - - assert!( - call_result.is_none(), - "Call using invalid selector succeeded, when it should've failed." - ); - - Ok(()) - } - #[ink_e2e::test( additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" )] @@ -183,10 +132,7 @@ mod call_builder { .call( &mut ink_e2e::charlie(), contract_acc_id.clone(), - call_builder::messages::call_build( - flipper_ink_acc_id, - invalid_selector, - ), + call_builder::messages::call(flipper_ink_acc_id, invalid_selector), 0, None, ) @@ -268,5 +214,54 @@ mod call_builder { Ok(()) } + + #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] + async fn e2e_create_builder_fails_with_invalid_selector( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::eve(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let code_hash = client + .upload( + &mut ink_e2e::eve(), + constructors_return_value::CONTRACT_PATH, + None, + ) + .await + .expect("upload `constructors_return_value` failed") + .code_hash; + + let invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::eve(), + contract_acc_id.clone(), + call_builder::messages::call_instantiate( + ink_e2e::utils::runtime_hash_to_ink_hash::< + ink::env::DefaultEnvironment, + >(&code_hash), + invalid_selector, + true, + ), + 0, + None, + ) + .await + .expect("Client failed to call `call_builder::call_instantiate`.") + .value + .expect("Dispatching `call_builder::call_instantiate` failed."); + + assert!( + call_result.is_none(), + "Call using invalid selector succeeded, when it should've failed." + ); + + Ok(()) + } } } From cab6c980bd731a25de43dd94156165d9f236631b Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 15:34:01 -0500 Subject: [PATCH 28/30] Try moving message related test to different module --- .../call-builder/lib.rs | 161 +++++++++--------- 1 file changed, 83 insertions(+), 78 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index db5462df5d..9ecbc3d4ce 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -88,85 +88,8 @@ mod call_builder { type E2EResult = std::result::Result>; #[ink_e2e::test( - additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" + additional_contracts = "../constructors-return-value/Cargo.toml ../integration-flipper/Cargo.toml" )] - async fn e2e_invalid_message_selector_can_be_handled( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - use call_builder::contract_types::ink_primitives::{ - types::AccountId as E2EAccountId, - LangError as E2ELangError, - }; - - let constructor = call_builder::constructors::new(); - let contract_acc_id = client - .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) - .await - .expect("instantiate failed") - .account_id; - - let flipper_constructor = integration_flipper::constructors::default(); - let flipper_acc_id = client - .instantiate(&mut ink_e2e::charlie(), flipper_constructor, 0, None) - .await - .expect("instantiate `flipper` failed") - .account_id; - - let get_call_result = client - .call( - &mut ink_e2e::charlie(), - flipper_acc_id.clone(), - integration_flipper::messages::get(), - 0, - None, - ) - .await - .expect("Calling `flipper::get` failed"); - let initial_value = get_call_result - .value - .expect("Input is valid, call must not fail."); - - let flipper_ink_acc_id = E2EAccountId(flipper_acc_id.clone().into()); - let invalid_selector = [0x00, 0x00, 0x00, 0x00]; - let call_result = client - .call( - &mut ink_e2e::charlie(), - contract_acc_id.clone(), - call_builder::messages::call(flipper_ink_acc_id, invalid_selector), - 0, - None, - ) - .await - .expect("Calling `call_builder::call` failed"); - - let flipper_result = call_result - .value - .expect("Call to `call_builder::call` failed"); - - assert!(matches!( - flipper_result, - Some(E2ELangError::CouldNotReadInput) - )); - - let get_call_result = client - .call( - &mut ink_e2e::charlie(), - flipper_acc_id.clone(), - integration_flipper::messages::get(), - 0, - None, - ) - .await - .expect("Calling `flipper::get` failed"); - let flipped_value = get_call_result - .value - .expect("Input is valid, call must not fail."); - assert!(flipped_value == initial_value); - - Ok(()) - } - - #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] async fn e2e_create_builder_works_with_valid_selector( mut client: ink_e2e::Client, ) -> E2EResult<()> { @@ -264,4 +187,86 @@ mod call_builder { Ok(()) } } + + #[cfg(all(test, feature = "e2e-tests"))] + mod e2e_message_tests { + type E2EResult = std::result::Result>; + + #[ink_e2e::test(additional_contracts = "../integration-flipper/Cargo.toml")] + async fn e2e_invalid_message_selector_can_be_handled( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + use call_builder::contract_types::ink_primitives::{ + types::AccountId as E2EAccountId, + LangError as E2ELangError, + }; + + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let flipper_constructor = integration_flipper::constructors::default(); + let flipper_acc_id = client + .instantiate(&mut ink_e2e::charlie(), flipper_constructor, 0, None) + .await + .expect("instantiate `flipper` failed") + .account_id; + + let get_call_result = client + .call( + &mut ink_e2e::charlie(), + flipper_acc_id.clone(), + integration_flipper::messages::get(), + 0, + None, + ) + .await + .expect("Calling `flipper::get` failed"); + let initial_value = get_call_result + .value + .expect("Input is valid, call must not fail."); + + let flipper_ink_acc_id = E2EAccountId(flipper_acc_id.clone().into()); + let invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::charlie(), + contract_acc_id.clone(), + call_builder::messages::call(flipper_ink_acc_id, invalid_selector), + 0, + None, + ) + .await + .expect("Calling `call_builder::call` failed"); + + let flipper_result = call_result + .value + .expect("Call to `call_builder::call` failed"); + + assert!(matches!( + flipper_result, + Some(E2ELangError::CouldNotReadInput) + )); + + let get_call_result = client + .call( + &mut ink_e2e::charlie(), + flipper_acc_id.clone(), + integration_flipper::messages::get(), + 0, + None, + ) + .await + .expect("Calling `flipper::get` failed"); + let flipped_value = get_call_result + .value + .expect("Input is valid, call must not fail."); + assert!(flipped_value == initial_value); + + Ok(()) + } + } } From 4dc9973720f45906752321822b02f1ccdb9e0cec Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 15:42:28 -0500 Subject: [PATCH 29/30] Revert "Try moving message related test to different module" This reverts commit cab6c980bd731a25de43dd94156165d9f236631b. --- .../call-builder/lib.rs | 161 +++++++++--------- 1 file changed, 78 insertions(+), 83 deletions(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index 9ecbc3d4ce..db5462df5d 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -88,8 +88,85 @@ mod call_builder { type E2EResult = std::result::Result>; #[ink_e2e::test( - additional_contracts = "../constructors-return-value/Cargo.toml ../integration-flipper/Cargo.toml" + additional_contracts = "../integration-flipper/Cargo.toml ../constructors-return-value/Cargo.toml" )] + async fn e2e_invalid_message_selector_can_be_handled( + mut client: ink_e2e::Client, + ) -> E2EResult<()> { + use call_builder::contract_types::ink_primitives::{ + types::AccountId as E2EAccountId, + LangError as E2ELangError, + }; + + let constructor = call_builder::constructors::new(); + let contract_acc_id = client + .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) + .await + .expect("instantiate failed") + .account_id; + + let flipper_constructor = integration_flipper::constructors::default(); + let flipper_acc_id = client + .instantiate(&mut ink_e2e::charlie(), flipper_constructor, 0, None) + .await + .expect("instantiate `flipper` failed") + .account_id; + + let get_call_result = client + .call( + &mut ink_e2e::charlie(), + flipper_acc_id.clone(), + integration_flipper::messages::get(), + 0, + None, + ) + .await + .expect("Calling `flipper::get` failed"); + let initial_value = get_call_result + .value + .expect("Input is valid, call must not fail."); + + let flipper_ink_acc_id = E2EAccountId(flipper_acc_id.clone().into()); + let invalid_selector = [0x00, 0x00, 0x00, 0x00]; + let call_result = client + .call( + &mut ink_e2e::charlie(), + contract_acc_id.clone(), + call_builder::messages::call(flipper_ink_acc_id, invalid_selector), + 0, + None, + ) + .await + .expect("Calling `call_builder::call` failed"); + + let flipper_result = call_result + .value + .expect("Call to `call_builder::call` failed"); + + assert!(matches!( + flipper_result, + Some(E2ELangError::CouldNotReadInput) + )); + + let get_call_result = client + .call( + &mut ink_e2e::charlie(), + flipper_acc_id.clone(), + integration_flipper::messages::get(), + 0, + None, + ) + .await + .expect("Calling `flipper::get` failed"); + let flipped_value = get_call_result + .value + .expect("Input is valid, call must not fail."); + assert!(flipped_value == initial_value); + + Ok(()) + } + + #[ink_e2e::test(additional_contracts = "../constructors-return-value/Cargo.toml")] async fn e2e_create_builder_works_with_valid_selector( mut client: ink_e2e::Client, ) -> E2EResult<()> { @@ -187,86 +264,4 @@ mod call_builder { Ok(()) } } - - #[cfg(all(test, feature = "e2e-tests"))] - mod e2e_message_tests { - type E2EResult = std::result::Result>; - - #[ink_e2e::test(additional_contracts = "../integration-flipper/Cargo.toml")] - async fn e2e_invalid_message_selector_can_be_handled( - mut client: ink_e2e::Client, - ) -> E2EResult<()> { - use call_builder::contract_types::ink_primitives::{ - types::AccountId as E2EAccountId, - LangError as E2ELangError, - }; - - let constructor = call_builder::constructors::new(); - let contract_acc_id = client - .instantiate(&mut ink_e2e::charlie(), constructor, 0, None) - .await - .expect("instantiate failed") - .account_id; - - let flipper_constructor = integration_flipper::constructors::default(); - let flipper_acc_id = client - .instantiate(&mut ink_e2e::charlie(), flipper_constructor, 0, None) - .await - .expect("instantiate `flipper` failed") - .account_id; - - let get_call_result = client - .call( - &mut ink_e2e::charlie(), - flipper_acc_id.clone(), - integration_flipper::messages::get(), - 0, - None, - ) - .await - .expect("Calling `flipper::get` failed"); - let initial_value = get_call_result - .value - .expect("Input is valid, call must not fail."); - - let flipper_ink_acc_id = E2EAccountId(flipper_acc_id.clone().into()); - let invalid_selector = [0x00, 0x00, 0x00, 0x00]; - let call_result = client - .call( - &mut ink_e2e::charlie(), - contract_acc_id.clone(), - call_builder::messages::call(flipper_ink_acc_id, invalid_selector), - 0, - None, - ) - .await - .expect("Calling `call_builder::call` failed"); - - let flipper_result = call_result - .value - .expect("Call to `call_builder::call` failed"); - - assert!(matches!( - flipper_result, - Some(E2ELangError::CouldNotReadInput) - )); - - let get_call_result = client - .call( - &mut ink_e2e::charlie(), - flipper_acc_id.clone(), - integration_flipper::messages::get(), - 0, - None, - ) - .await - .expect("Calling `flipper::get` failed"); - let flipped_value = get_call_result - .value - .expect("Input is valid, call must not fail."); - assert!(flipped_value == initial_value); - - Ok(()) - } - } } From ad1cab7b75d6aab93a92302b090726c1d0dc43f6 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 21 Nov 2022 16:25:15 -0500 Subject: [PATCH 30/30] Try different type for account ID --- examples/lang-err-integration-tests/call-builder/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/examples/lang-err-integration-tests/call-builder/lib.rs b/examples/lang-err-integration-tests/call-builder/lib.rs index db5462df5d..f0f132b735 100755 --- a/examples/lang-err-integration-tests/call-builder/lib.rs +++ b/examples/lang-err-integration-tests/call-builder/lib.rs @@ -126,7 +126,9 @@ mod call_builder { .value .expect("Input is valid, call must not fail."); - let flipper_ink_acc_id = E2EAccountId(flipper_acc_id.clone().into()); + let flipper_ink_acc_id = + ink::primitives::AccountId::try_from(flipper_acc_id.clone().as_ref()) + .unwrap(); let invalid_selector = [0x00, 0x00, 0x00, 0x00]; let call_result = client .call(