Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Generate metadata for constructor return type #1460

Merged
merged 14 commits into from
Nov 7, 2022
Merged

Generate metadata for constructor return type #1460

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4be003b
generates correct metadata for Self type
Nov 1, 2022
68df576
generate metadata for Result type
Nov 1, 2022
ced904f
generate metadata for Result<Self, ...>
Nov 1, 2022
1686a8a
docs and comments
Nov 2, 2022
42fac54
Merge branch 'master' into gn/constructor-metadata
Nov 3, 2022
7e4aed2
cargo fmt
Nov 3, 2022
2912089
fix the metadata tests
Nov 4, 2022
bdd717f
Merge branch 'master' into gn/constructor-metadata
Nov 4, 2022
32e99fa
makes compatible with new tests
Nov 4, 2022
f5ec420
rewrite return type transformation to support aliases
Nov 4, 2022
4959e4c
test changes (#1473)
xermicus Nov 5, 2022
e59821f
remove unecessary helper fn (#1475)
xermicus Nov 7, 2022
e335081
Update crates/ink/codegen/src/generator/metadata.rs
Nov 7, 2022
17795d0
Add test for metadata generator (#1477)
xermicus Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions crates/ink/codegen/src/generator/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use proc_macro2::TokenStream as TokenStream2;
use quote::{
quote,
quote_spanned,
ToTokens,
};
use syn::spanned::Spanned as _;

Expand Down Expand Up @@ -131,6 +132,7 @@ 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());
quote_spanned!(span=>
::ink::metadata::ConstructorSpec::from_label(::core::stringify!(#ident))
.selector([
Expand All @@ -140,6 +142,7 @@ impl Metadata<'_> {
#( #args ),*
])
.payable(#is_payable)
.returns(#ret_ty)
.docs([
#( #docs ),*
])
Expand Down Expand Up @@ -190,6 +193,41 @@ impl Metadata<'_> {
}
}

/// Generates the ink! metadata specs for the given type of a constructor.
fn generate_constructor_type_spec(ty: &syn::Type) -> TokenStream2 {
fn without_display_name(ty: TokenStream2) -> TokenStream2 {
quote! {
::ink::metadata::TransformResult::<#ty>::new_type_spec(None)
}
}
if let syn::Type::Path(type_path) = ty {
if type_path.qself.is_some() {
let ty = Self::replace_self_with_unit(ty);
return without_display_name(ty)
}
let path = &type_path.path;
if path.segments.is_empty() {
let ty = Self::replace_self_with_unit(ty);
return without_display_name(ty)
}
let segs = path
.segments
.iter()
.map(|seg| &seg.ident)
.collect::<Vec<_>>();
let ty = Self::replace_self_with_unit(ty);
quote! {
::ink::metadata::TransformResult::<#ty>::new_type_spec(
Some(::core::iter::IntoIterator::into_iter([ #( ::core::stringify!(#segs) ),* ])
.map(::core::convert::AsRef::as_ref)
))
}
} else {
let ty = Self::replace_self_with_unit(ty);
without_display_name(ty)
}
}

/// Generates the ink! metadata for all ink! smart contract messages.
fn generate_messages(&self) -> Vec<TokenStream2> {
let mut messages = Vec::new();
Expand Down Expand Up @@ -316,6 +354,53 @@ impl Metadata<'_> {
}
}

/// Generates ink! metadata for the given return type of a constructor.
/// If constructor result type is not `Result`,
xermicus marked this conversation as resolved.
Show resolved Hide resolved
/// the metadata will not display any metadata for return type.
SkymanOne marked this conversation as resolved.
Show resolved Hide resolved
/// Otherwise, the return type is `Result<(), E>`.
fn generate_constructor_return_type(ret_ty: Option<&syn::Type>) -> TokenStream2 {
/// Returns `true` if the given type is `Self`.
fn type_is_self_val(ty: &syn::Type) -> bool {
xermicus marked this conversation as resolved.
Show resolved Hide resolved
matches!(ty, syn::Type::Path(syn::TypePath {
qself: None,
path
}) if path.is_ident("Self"))
}

match ret_ty {
None => {
quote! {
::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None)
}
}
Some(ty) => {
if type_is_self_val(ty) {
return quote! { ::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None)}
}
let type_spec = Self::generate_constructor_type_spec(ty);
let type_token = Self::replace_self_with_unit(ty);
quote! {
if !::ink::is_result_type!(#type_token) {
xermicus marked this conversation as resolved.
Show resolved Hide resolved
::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None)
} else {
::ink::metadata::ReturnTypeSpec::new(#type_spec)
}
}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This again does syntactical checks instead of using the Rust type system properly and will fail for various syntactic contexts. To do this properly you'd need to take a similar approach as in generate_trait_messages with Rust type system reflection such as

                let is_payable = quote! {{
                    <<::ink::reflect::TraitDefinitionRegistry<<#storage_ident as ::ink::reflect::ContractEnv>::Env>
                        as #trait_path>::__ink_TraitInfo
                        as ::ink::reflect::TraitMessageInfo<#local_id>>::PAYABLE
                }};

We do have the information for ink! messages and constructors as well. They are identified using their selectors which are always known at proc. macro compile time. Therefore you can access them in this scope just like in the linked example.

Copy link
Contributor Author

@SkymanOne SkymanOne Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The metadata codegen takes place after the WASM blob compilation, hence at this stage all types have already been evaluated by compiler and we can work with syntax.

In fact, I generally do not do syntactical checks. I only check if the token is Self here because metadata generation takes place outside the storage definition, hence Self can not be evaluated there. Same logic applies for Result<Self, ...>.

However, if the return type is the explicit type of a contract or Result with explicit contract type for Ok variant, then I do the type evaluation, hence why we have TransformResult factory.
I can be sure that the Self is valid in this context is because the WASM build is already done by now.

I am not sure how message selectors are relevant here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may be able to refactor this to make it use the Rust type system, I'm looking into this now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done an experiment with this in #1491, take a look and see what you think.


/// Helper function which replace all occurrences of `Self`
/// with `()`.
fn replace_self_with_unit(ty: &syn::Type) -> TokenStream2 {
if ty.to_token_stream().to_string().contains("< Self") {
let s = ty.to_token_stream().to_string().replace("< Self", "< ()");
s.parse().unwrap()
} else {
ty.to_token_stream()
}
}

/// Generates ink! metadata for all user provided ink! event definitions.
fn generate_events(&self) -> impl Iterator<Item = TokenStream2> + '_ {
self.contract.module().events().map(|event| {
Expand Down
2 changes: 1 addition & 1 deletion crates/ink/src/codegen/dispatch/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ where
&Contract::KEY,
contract,
);
Ok(())
ink_env::return_value(ReturnFlags::default().set_reverted(false), &());
Copy link
Collaborator

@ascjones ascjones Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what if the return type is Result<Self, _>, the value will never be returned. The client decoding will fail because it will expect 0x00 for Ok but will get empty bytes instead.

I suppose you have not been able to test this since there are no clients which are attempting to decode the return value?

One way to test it would be via the in-contract instantiation with CreateBuilder. E.g used in delegator.

}
Err(error) => {
// Constructor is fallible and failed.
Expand Down
1 change: 1 addition & 0 deletions crates/metadata/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub use self::specs::{
MessageSpecBuilder,
ReturnTypeSpec,
Selector,
TransformResult,
TypeSpec,
};

Expand Down
86 changes: 74 additions & 12 deletions crates/metadata/src/specs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,9 @@ where
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(bound(
serialize = "F::Type: Serialize, F::String: Serialize",
deserialize = "F::Type: DeserializeOwned, F::String: DeserializeOwned"
deserialize = "F::Type: DeserializeOwned, F::String: DeserializeOwned",
))]
#[serde(rename_all = "camelCase")]
pub struct ConstructorSpec<F: Form = MetaForm> {
/// The label of the constructor.
///
Expand All @@ -248,6 +249,8 @@ pub struct ConstructorSpec<F: Form = MetaForm> {
pub payable: bool,
/// The parameters of the deployment handler.
pub args: Vec<MessageParamSpec<F>>,
/// The return type of the constructor..
pub return_type: ReturnTypeSpec<F>,
/// The deployment handler documentation.
pub docs: Vec<F::String>,
}
Expand All @@ -265,6 +268,7 @@ impl IntoPortable for ConstructorSpec {
.into_iter()
.map(|arg| arg.into_portable(registry))
.collect::<Vec<_>>(),
return_type: self.return_type.into_portable(registry),
docs: self.docs.into_iter().map(|s| s.into()).collect(),
}
}
Expand All @@ -281,7 +285,7 @@ where
&self.label
}

/// Returns the selector hash of the message.
/// Returns the selector hash of the constructor.
pub fn selector(&self) -> &Selector {
&self.selector
}
Expand All @@ -296,6 +300,11 @@ where
&self.args
}

/// Returns the return type of the constructor.
pub fn return_type(&self) -> &ReturnTypeSpec<F> {
&self.return_type
}

/// Returns the deployment handler documentation.
pub fn docs(&self) -> &[F::String] {
&self.docs
Expand All @@ -309,10 +318,11 @@ where
/// Some fields are guarded by a type-state pattern to fail at
/// compile-time instead of at run-time. This is useful to better
/// debug code-gen macros.
#[allow(clippy::type_complexity)]
#[must_use]
pub struct ConstructorSpecBuilder<F: Form, Selector, IsPayable> {
pub struct ConstructorSpecBuilder<F: Form, Selector, IsPayable, Returns> {
spec: ConstructorSpec<F>,
marker: PhantomData<fn() -> (Selector, IsPayable)>,
marker: PhantomData<fn() -> (Selector, IsPayable, Returns)>,
}

impl<F> ConstructorSpec<F>
Expand All @@ -322,30 +332,35 @@ where
/// Creates a new constructor spec builder.
pub fn from_label(
label: <F as Form>::String,
) -> ConstructorSpecBuilder<F, Missing<state::Selector>, Missing<state::IsPayable>>
{
) -> ConstructorSpecBuilder<
F,
Missing<state::Selector>,
Missing<state::IsPayable>,
Missing<state::Returns>,
> {
ConstructorSpecBuilder {
spec: Self {
label,
selector: Selector::default(),
payable: Default::default(),
args: Vec::new(),
return_type: ReturnTypeSpec::new(None),
docs: Vec::new(),
},
marker: PhantomData,
}
}
}

impl<F, P> ConstructorSpecBuilder<F, Missing<state::Selector>, P>
impl<F, P, R> ConstructorSpecBuilder<F, Missing<state::Selector>, P, R>
where
F: Form,
{
/// Sets the function selector of the message.
pub fn selector(
self,
selector: [u8; 4],
) -> ConstructorSpecBuilder<F, state::Selector, P> {
) -> ConstructorSpecBuilder<F, state::Selector, P, R> {
ConstructorSpecBuilder {
spec: ConstructorSpec {
selector: selector.into(),
Expand All @@ -356,15 +371,15 @@ where
}
}

impl<F, S> ConstructorSpecBuilder<F, S, Missing<state::IsPayable>>
impl<F, S, R> ConstructorSpecBuilder<F, S, Missing<state::IsPayable>, R>
where
F: Form,
{
/// Sets if the constructor is payable, thus accepting value for the caller.
pub fn payable(
self,
is_payable: bool,
) -> ConstructorSpecBuilder<F, S, state::IsPayable> {
) -> ConstructorSpecBuilder<F, S, state::IsPayable, R> {
ConstructorSpecBuilder {
spec: ConstructorSpec {
payable: is_payable,
Expand All @@ -375,7 +390,26 @@ where
}
}

impl<F, S, P> ConstructorSpecBuilder<F, S, P>
impl<F, S, P> ConstructorSpecBuilder<F, S, P, Missing<state::Returns>>
where
F: Form,
{
/// Sets the return type of the message.
pub fn returns(
self,
return_type: ReturnTypeSpec<F>,
) -> ConstructorSpecBuilder<F, S, P, state::Returns> {
ConstructorSpecBuilder {
spec: ConstructorSpec {
return_type,
..self.spec
},
marker: PhantomData,
}
}
}

impl<F, S, P, R> ConstructorSpecBuilder<F, S, P, R>
where
F: Form,
{
Expand Down Expand Up @@ -406,7 +440,7 @@ where
}
}

impl<F> ConstructorSpecBuilder<F, state::Selector, state::IsPayable>
impl<F> ConstructorSpecBuilder<F, state::Selector, state::IsPayable, state::Returns>
where
F: Form,
{
Expand Down Expand Up @@ -994,6 +1028,34 @@ where
}
}

/// This is wrapper for [`TypeSpec`] which acts as a factory.
xermicus marked this conversation as resolved.
Show resolved Hide resolved
/// The whole purpose of the factory is to replace the type of
/// `Ok` variant of `Result` with `()` because constructors do not return
/// any data upon successful instantiation.
///
/// # Important Note
/// Only use this factory with constructors!
pub struct TransformResult<T> {
marker: core::marker::PhantomData<fn() -> T>,
}

impl<O, E> TransformResult<core::result::Result<O, E>>
where
E: TypeInfo + 'static,
{
/// Produces [`TypeSpec`] for a `Result<(), E>`.
pub fn new_type_spec<S>(segments_opt: Option<S>) -> TypeSpec
where
S: IntoIterator<Item = &'static str>,
xermicus marked this conversation as resolved.
Show resolved Hide resolved
{
if let Some(segments) = segments_opt {
TypeSpec::with_name_segs::<core::result::Result<(), E>, S>(segments)
} else {
TypeSpec::of_type::<core::result::Result<(), E>>()
}
}
}

/// Describes a pair of parameter label and type.
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(bound(
Expand Down
Loading