From d13f24e40648e0a16a017c048c2e6af3f9bbf544 Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Fri, 17 Nov 2023 15:39:19 +0000 Subject: [PATCH 1/6] Messages return TypeSpec directly --- crates/ink/codegen/src/generator/metadata.rs | 22 +++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 298f4397ff1..65051b7c0b8 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -248,7 +248,8 @@ impl Metadata<'_> { let ident = message.ident(); let args = message.inputs().map(Self::generate_dispatch_argument); let cfg_attrs = message.get_cfg_attrs(span); - let ret_ty = Self::generate_return_type(Some(&message.wrapped_output())); + let ret_ty = + Self::generate_message_return_type(&message.wrapped_output()); quote_spanned!(span => #( #cfg_attrs )* ::ink::metadata::MessageSpec::from_label(::core::stringify!(#ident)) @@ -311,7 +312,7 @@ impl Metadata<'_> { as #trait_path>::__ink_TraitInfo as ::ink::reflect::TraitMessageInfo<#local_id>>::SELECTOR }}; - let ret_ty = Self::generate_return_type(Some(&message.wrapped_output())); + let ret_ty = Self::generate_message_return_type(&message.wrapped_output()); let label = [trait_ident.to_string(), message_ident.to_string()].join("::"); quote_spanned!(message_span=> #( #cfg_attrs )* @@ -333,19 +334,10 @@ impl Metadata<'_> { } /// Generates ink! metadata for the given return type. - fn generate_return_type(ret_ty: Option<&syn::Type>) -> TokenStream2 { - match ret_ty { - None => { - quote! { - ::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None) - } - } - Some(ty) => { - let type_spec = Self::generate_type_spec(ty); - quote! { - ::ink::metadata::ReturnTypeSpec::new(#type_spec) - } - } + fn generate_message_return_type(ret_ty: &syn::Type) -> TokenStream2 { + let type_spec = Self::generate_type_spec(ret_ty); + quote! { + ::ink::metadata::ReturnTypeSpec::new(#type_spec) } } From 84ae71280b909cd7279fcd42ad325a19a920275e Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Fri, 17 Nov 2023 15:43:05 +0000 Subject: [PATCH 2/6] arrange changelog --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e8c8a2fd0b..17f08c0858d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,16 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [E2E] Allow testing with live-chain state - [#1949](/~https://github.com/paritytech/ink/pull/1949) - [E2E] Call builders and extra gas margin option - [#1917](/~https://github.com/paritytech/ink/pull/1917) -- Make `set_code_hash` generic - [#1906](/~https://github.com/paritytech/ink/pull/1906) +- Linter: `storage_never_freed` lint - [#1932](/~https://github.com/paritytech/ink/pull/1932) +- Linter: `strict_balance_equality` lint - [#1914](/~https://github.com/paritytech/ink/pull/1914) - Clean E2E configuration parsing - [#1922](/~https://github.com/paritytech/ink/pull/1922) +- Make `set_code_hash` generic - [#1906](/~https://github.com/paritytech/ink/pull/1906) ### Changed +- Messages return `TypeSpec` directly - #[1999](/~https://github.com/paritytech/ink/pull/1999) - Fail when decoding from storage and not all bytes consumed - [#1897](/~https://github.com/paritytech/ink/pull/1897) - [E2E] resolve DispatchError error details for dry-runs - [#1944](/~https://github.com/paritytech/ink/pull/1994) -### Added -- Linter: `storage_never_freed` lint - [#1932](/~https://github.com/paritytech/ink/pull/1932) -- Linter: `strict_balance_equality` lint - [#1914](/~https://github.com/paritytech/ink/pull/1914) ## Version 5.0.0-alpha From 843abd4ee37eb933223a5c8a754183f2b24ec477 Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Mon, 20 Nov 2023 17:01:39 +0000 Subject: [PATCH 3/6] remove `Option` in `ReturnTypeSpec` + adjust tests --- crates/ink/codegen/src/generator/metadata.rs | 8 +- crates/metadata/src/specs.rs | 28 +-- crates/metadata/src/tests.rs | 182 +++++++++++++++---- crates/primitives/src/lib.rs | 4 + 4 files changed, 169 insertions(+), 53 deletions(-) diff --git a/crates/ink/codegen/src/generator/metadata.rs b/crates/ink/codegen/src/generator/metadata.rs index 65051b7c0b8..d5559d34d84 100644 --- a/crates/ink/codegen/src/generator/metadata.rs +++ b/crates/ink/codegen/src/generator/metadata.rs @@ -353,13 +353,13 @@ 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::metadata::TypeSpec::with_name_str::< ::ink::ConstructorResult<::core::result::Result<(), #constructor_info::Error>>, - >("ink_primitives::ConstructorResult")) + >("ink_primitives::ConstructorResult") } else { - ::core::option::Option::Some(::ink::metadata::TypeSpec::with_name_str::< + ::ink::metadata::TypeSpec::with_name_str::< ::ink::ConstructorResult<()>, - >("ink_primitives::ConstructorResult")) + >("ink_primitives::ConstructorResult") }) ) } diff --git a/crates/metadata/src/specs.rs b/crates/metadata/src/specs.rs index c43c8333a35..926e2f90514 100644 --- a/crates/metadata/src/specs.rs +++ b/crates/metadata/src/specs.rs @@ -468,6 +468,7 @@ pub struct ConstructorSpecBuilder { impl ConstructorSpec where F: Form, + TypeSpec: Default, { /// Creates a new constructor spec builder. pub fn from_label( @@ -484,7 +485,7 @@ where selector: Selector::default(), payable: Default::default(), args: Vec::new(), - return_type: ReturnTypeSpec::new(None), + return_type: ReturnTypeSpec::new(TypeSpec::default()), docs: Vec::new(), default: false, }, @@ -668,6 +669,7 @@ mod state { impl MessageSpec where F: Form, + TypeSpec: Default, { /// Creates a new message spec builder. pub fn from_label( @@ -686,7 +688,7 @@ where mutates: false, payable: false, args: Vec::new(), - return_type: ReturnTypeSpec::new(None), + return_type: ReturnTypeSpec::new(TypeSpec::default()), docs: Vec::new(), default: false, }, @@ -1282,6 +1284,11 @@ where pub fn new(ty: ::Type, display_name: DisplayName) -> Self { Self { ty, display_name } } + + // /// Creates a new type specification for a given type and display name. + // pub fn of_type_with_display_name(display_name: + // DisplayName) -> Self { Self { ty: meta_type::(), display_name } + // } } /// Describes a pair of parameter label and type. @@ -1417,7 +1424,7 @@ where #[must_use] pub struct ReturnTypeSpec { #[serde(rename = "type")] - opt_type: Option>, + ret_type: TypeSpec, } impl IntoPortable for ReturnTypeSpec { @@ -1425,9 +1432,7 @@ impl IntoPortable for ReturnTypeSpec { fn into_portable(self, registry: &mut Registry) -> Self::Output { ReturnTypeSpec { - opt_type: self - .opt_type - .map(|opt_type| opt_type.into_portable(registry)), + ret_type: self.ret_type.into_portable(registry), } } } @@ -1435,6 +1440,7 @@ impl IntoPortable for ReturnTypeSpec { impl ReturnTypeSpec where F: Form, + TypeSpec: Default, { /// Creates a new return type specification from the given type or `None`. /// @@ -1446,16 +1452,16 @@ where /// ``` pub fn new(ty: T) -> Self where - T: Into>>, + T: Into>, { Self { - opt_type: ty.into(), + ret_type: ty.into(), } } - /// Returns the optional return type - pub fn opt_type(&self) -> Option<&TypeSpec> { - self.opt_type.as_ref() + /// Returns the return type + pub fn ret_type(&self) -> &TypeSpec { + &self.ret_type } } diff --git a/crates/metadata/src/tests.rs b/crates/metadata/src/tests.rs index b9b4ae162ef..3cf82fe38a5 100644 --- a/crates/metadata/src/tests.rs +++ b/crates/metadata/src/tests.rs @@ -24,11 +24,16 @@ use serde_json::json; #[test] fn spec_constructor_selector_must_serialize_to_hex() { // given + let selector_id = 123_456_789u32; let label = "foo"; let cs = ConstructorSpec::from_label(label) - .selector(123_456_789u32.to_be_bytes()) + .selector(selector_id.to_be_bytes()) .payable(true) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .done(); let mut registry = Registry::new(); let portable_spec = cs.into_portable(&mut registry); @@ -45,7 +50,13 @@ fn spec_constructor_selector_must_serialize_to_hex() { "label": "foo", "payable": true, "selector": "0x075bcd15", - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 0 + }, "args": [], "docs": [], "default": false, @@ -66,7 +77,11 @@ fn spec_contract_only_one_default_message_allowed() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .done()]) .messages(vec![ @@ -79,7 +94,11 @@ fn spec_contract_only_one_default_message_allowed() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::MessageResult<()>, + >( + "ink_primitives::MessageResult" + ))) .default(true) .done(), MessageSpec::from_label("get") @@ -116,7 +135,11 @@ fn spec_contract_only_one_default_constructor_allowed() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .default(true) .done(), @@ -124,7 +147,11 @@ fn spec_contract_only_one_default_constructor_allowed() { .selector([2u8, 34u8, 255u8, 24u8]) .payable(Default::default()) .args(Vec::new()) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .default(true) .done(), @@ -138,7 +165,11 @@ fn spec_contract_only_one_default_constructor_allowed() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::MessageResult<()>, + >( + "ink_primitives::MessageResult" + ))) .default(true) .done()]) .events(Vec::new()) @@ -168,7 +199,11 @@ fn spec_contract_event_definition_exceeds_environment_topics_limit() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .default(true) .done()]) @@ -181,7 +216,11 @@ fn spec_contract_event_definition_exceeds_environment_topics_limit() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::MessageResult<()>, + >( + "ink_primitives::MessageResult" + ))) .default(true) .done()]) .events(vec![ @@ -262,7 +301,11 @@ fn spec_contract_event_definition_signature_topic_collision() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .default(true) .done()]) @@ -275,7 +318,11 @@ fn spec_contract_event_definition_signature_topic_collision() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::MessageResult<()>, + >( + "ink_primitives::MessageResult" + ))) .default(true) .done()]) .events(vec![ @@ -337,14 +384,22 @@ fn spec_contract_json() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .done(), ConstructorSpec::from_label("default") .selector([2u8, 34u8, 255u8, 24u8]) .payable(Default::default()) .args(Vec::new()) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .default(true) .done(), @@ -352,11 +407,11 @@ fn spec_contract_json() { .selector([6u8, 3u8, 55u8, 123u8]) .payable(Default::default()) .args(Vec::new()) - .returns(ReturnTypeSpec::new(Some(TypeSpec::with_name_str::< - Result<(), ()>, + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult>, >( - "core::result::Result" - )))) + "ink_primitives::ConstructorResult" + ))) .docs(Vec::new()) .done(), ]) @@ -370,7 +425,11 @@ fn spec_contract_json() { vec!["i32"].into_iter().map(AsRef::as_ref), )) .done()]) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::MessageResult<()>, + >( + "ink_primitives::MessageResult" + ))) .default(true) .done(), MessageSpec::from_label("get") @@ -460,7 +519,13 @@ fn spec_contract_json() { "default": false, "label": "new", "payable": true, - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 1 + }, "selector": "0x5ebd88d6" }, { @@ -469,7 +534,13 @@ fn spec_contract_json() { "default": true, "label": "default", "payable": false, - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 1 + }, "selector": "0x0222ff18" }, { @@ -480,11 +551,10 @@ fn spec_contract_json() { "payable": false, "returnType": { "displayName": [ - "core", - "result", - "Result" + "ink_primitives", + "ConstructorResult" ], - "type": 1 + "type": 4 }, "selector": "0x0603377b" } @@ -495,39 +565,39 @@ fn spec_contract_json() { "displayName": [ "AccountId", ], - "type": 4, + "type": 6, }, "balance": { "displayName": [ "Balance", ], - "type": 7, + "type": 9, }, "blockNumber": { "displayName": [ "BlockNumber", ], - "type": 9, + "type": 11, }, "staticBufferSize": 16384, "chainExtension": { "displayName": [ "ChainExtension", ], - "type": 10, + "type": 12, }, "hash": { "displayName": [ "Hash", ], - "type": 8, + "type": 10, }, "maxEventTopics": 4, "timestamp": { "displayName": [ "Timestamp", ], - "type": 7, + "type": 9, }, }, "events": [], @@ -556,7 +626,13 @@ fn spec_contract_json() { "mutates": true, "payable": true, "label": "inc", - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "MessageResult" + ], + "type": 1 + }, "selector": "0xe7d0590f" }, { @@ -588,7 +664,11 @@ fn trim_docs() { .selector(123_456_789u32.to_be_bytes()) .docs(vec![" foobar "]) .payable(Default::default()) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .done(); let mut registry = Registry::new(); let compact_spec = cs.into_portable(&mut registry); @@ -604,7 +684,13 @@ fn trim_docs() { json!({ "label": "foo", "payable": false, - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 0 + }, "selector": "0x075bcd15", "args": [], "docs": ["foobar"], @@ -630,7 +716,11 @@ fn trim_docs_with_code() { " ```", ]) .payable(Default::default()) - .returns(ReturnTypeSpec::new(None)) + .returns(ReturnTypeSpec::new(TypeSpec::with_name_str::< + ink_primitives::ConstructorResult<()>, + >( + "ink_primitives::ConstructorResult" + ))) .done(); let mut registry = Registry::new(); let compact_spec = cs.into_portable(&mut registry); @@ -646,7 +736,13 @@ fn trim_docs_with_code() { json!({ "label": "foo", "payable": false, - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 0 + }, "selector": "0x075bcd15", "args": [], "docs": [ @@ -728,8 +824,12 @@ fn environment_spec() -> EnvironmentSpec { /// Helper for creating a constructor spec at runtime fn runtime_constructor_spec() -> ConstructorSpec { let path: Path = Path::from_segments_unchecked(["FooType".to_string()]); + let lang_path: Path = Path::from_segments_unchecked([ + "ink_primitives".to_string(), + "ConstructorResult".to_string(), + ]); let spec = TypeSpec::new(123.into(), path); - let ret_spec = ReturnTypeSpec::new(None); + let ret_spec = ReturnTypeSpec::new(TypeSpec::new(456.into(), lang_path)); let args = [MessageParamSpec::new("foo_arg".to_string()) .of_type(spec) .done()]; @@ -794,7 +894,13 @@ fn construct_runtime_contract_spec() { "label": "foo", "selector": "0x00000000", "payable": true, - "returnType": null, + "returnType": { + "displayName": [ + "ink_primitives", + "ConstructorResult" + ], + "type": 456 + }, "args": [ { "label": "foo_arg", diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index b43c31162bc..733c1df4c70 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -62,3 +62,7 @@ pub type MessageResult = ::core::result::Result; /// The `Result` type for ink! constructors. pub type ConstructorResult = ::core::result::Result; + +// impl Into { + +// } From 09d976e9b394cb3096e50dcdb9ad429122667d27 Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Mon, 20 Nov 2023 17:02:40 +0000 Subject: [PATCH 4/6] remove rubbish --- crates/primitives/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index 733c1df4c70..b43c31162bc 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -62,7 +62,3 @@ pub type MessageResult = ::core::result::Result; /// The `Result` type for ink! constructors. pub type ConstructorResult = ::core::result::Result; - -// impl Into { - -// } From 099602e6d42f549df0641c1d533ccc9299969d88 Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Mon, 20 Nov 2023 17:11:13 +0000 Subject: [PATCH 5/6] fix tests --- crates/ink/tests/return_type_metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ink/tests/return_type_metadata.rs b/crates/ink/tests/return_type_metadata.rs index 8fccefcd1b3..ebb037672ba 100644 --- a/crates/ink/tests/return_type_metadata.rs +++ b/crates/ink/tests/return_type_metadata.rs @@ -106,7 +106,7 @@ mod tests { let message = metadata.spec().messages().iter().next().unwrap(); assert_eq!("TraitDefinition::get_value", message.label()); - let type_spec = message.return_type().opt_type().unwrap(); + let type_spec = message.return_type().ret_type(); let ty = resolve_type(&metadata, type_spec.ty().id); let (ok_ty, _) = extract_result(&metadata, ty); @@ -119,7 +119,7 @@ mod tests { let constructor = metadata.spec().constructors().iter().next().unwrap(); assert_eq!("try_new", constructor.label()); - let type_spec = constructor.return_type().opt_type().unwrap(); + let type_spec = constructor.return_type().ret_type(); assert_eq!( "ink_primitives::ConstructorResult", format!("{}", type_spec.display_name()) From 178d4ffe3d6bf7d270c850f6d4c33d394a0f2082 Mon Sep 17 00:00:00 2001 From: German Nikolishin Date: Mon, 20 Nov 2023 18:50:53 +0000 Subject: [PATCH 6/6] fix docs --- crates/metadata/src/specs.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/metadata/src/specs.rs b/crates/metadata/src/specs.rs index 926e2f90514..1d9599f668b 100644 --- a/crates/metadata/src/specs.rs +++ b/crates/metadata/src/specs.rs @@ -1284,11 +1284,6 @@ where pub fn new(ty: ::Type, display_name: DisplayName) -> Self { Self { ty, display_name } } - - // /// Creates a new type specification for a given type and display name. - // pub fn of_type_with_display_name(display_name: - // DisplayName) -> Self { Self { ty: meta_type::(), display_name } - // } } /// Describes a pair of parameter label and type. @@ -1448,7 +1443,7 @@ where /// /// ```no_run /// # use ink_metadata::{TypeSpec, ReturnTypeSpec}; - /// >::new(None); // no return type; + /// >::new(TypeSpec::default()); // no return type; /// ``` pub fn new(ty: T) -> Self where