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.

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

Generate metadata for constructor return type #1460

Merged
merged 14 commits into from
Nov 7, 2022
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Allows to use `Result<Self, Error>` as a return type in constructors - [#1446](https://github.com/paritytech/ink/pull/1446)
- Allows to use `Result<Self, Error>` as a return type in constructors - [#1446](https://github.com/paritytech/ink/pull/1446) and indicate it in metadata - [#1460](https://github.com/paritytech/ink/pull/1460)

- Remove random function from ink!.

Expand Down
80 changes: 80 additions & 0 deletions crates/ink/codegen/src/generator/metadata.rs
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 for the given type.
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,48 @@ impl Metadata<'_> {
}
}

/// Generates ink! metadata for the given return type of a constructor.
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

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.


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
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

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
38 changes: 38 additions & 0 deletions crates/ink/src/result_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,44 @@ macro_rules! is_result_err {
}};
}

// trying to evaluate error type here

// pub struct GetResultErr<'lt, T>(pub &'lt T);

// impl<T, E> GetResultErr<'_, ::core::result::Result<T, E>>
// where E: ToString{
// #[inline]
// // We need to allow for dead code at this point because
// // the Rust compiler thinks this function is unused even
// // though it acts as the specialized case for detection.
// #[allow(dead_code)]
// pub fn value(&self) -> String {
// }
// }

// pub trait GetResultErrFallback {
// #[inline]
// fn value(&self) -> bool {
// false
// }
// }
// impl<T> GetResultErrFallback for GetResultErr<'_, T> {}

// /// Evaluates to `true` if the given expression is a `Result::Err(_)`.
// ///
// /// # Note
// ///
// /// This given expression is not required to be of type `Result`.
// #[macro_export]
// #[doc(hidden)]
// macro_rules! get_result_err {
// ( $e:expr $(,)? ) => {{
// #[allow(unused_imports)]
// use $crate::result_info::GetResultErrFallback as _;
// $crate::result_info::GetResultErr(&$e).value()
// }};
// }

#[cfg(test)]
mod tests {
#[test]
Expand Down
1 change: 1 addition & 0 deletions crates/metadata/src/lib.rs
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
85 changes: 73 additions & 12 deletions crates/metadata/src/specs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ use alloc::{
vec,
vec::Vec,
};
use core::marker::PhantomData;
use core::{
any::TypeId,
marker::PhantomData,
};
use scale_info::{
form::{
Form,
Expand Down Expand Up @@ -221,8 +224,9 @@ impl ContractSpec {
#[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 @@ -234,6 +238,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 @@ -251,6 +257,7 @@ impl IntoPortable for ConstructorSpec {
.into_iter()
.map(|arg| arg.into_portable(registry))
.collect::<Vec<_>>(),
return_type: self.return_type.into_portable(registry),
docs: registry.map_into_portable(self.docs),
}
}
Expand All @@ -267,7 +274,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 @@ -282,6 +289,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 @@ -295,36 +307,42 @@ 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<Selector, IsPayable> {
pub struct ConstructorSpecBuilder<Selector, IsPayable, Returns> {
spec: ConstructorSpec,
marker: PhantomData<fn() -> (Selector, IsPayable)>,
marker: PhantomData<fn() -> (Selector, IsPayable, Returns)>,
}

impl ConstructorSpec {
/// Creates a new constructor spec builder.
pub fn from_label(
label: &'static str,
) -> ConstructorSpecBuilder<Missing<state::Selector>, Missing<state::IsPayable>> {
) -> ConstructorSpecBuilder<
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<P> ConstructorSpecBuilder<Missing<state::Selector>, P> {
impl<P, R> ConstructorSpecBuilder<Missing<state::Selector>, P, R> {
/// Sets the function selector of the message.
pub fn selector(
self,
selector: [u8; 4],
) -> ConstructorSpecBuilder<state::Selector, P> {
) -> ConstructorSpecBuilder<state::Selector, P, R> {
ConstructorSpecBuilder {
spec: ConstructorSpec {
selector: selector.into(),
Expand All @@ -335,12 +353,12 @@ impl<P> ConstructorSpecBuilder<Missing<state::Selector>, P> {
}
}

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

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

impl<S, P, R> ConstructorSpecBuilder<S, P, R> {
/// Sets the input arguments of the message specification.
pub fn args<A>(self, args: A) -> Self
where
Expand All @@ -378,7 +412,7 @@ impl<S, P> ConstructorSpecBuilder<S, P> {
}
}

impl ConstructorSpecBuilder<state::Selector, state::IsPayable> {
impl ConstructorSpecBuilder<state::Selector, state::IsPayable, state::Returns> {
/// Finishes construction of the constructor.
pub fn done(self) -> ConstructorSpec {
self.spec
Expand Down Expand Up @@ -890,6 +924,33 @@ where
}
}

/// This is wrapper for [TypeSpec] which acts as a factory.
/// The whole purpose of the factory is to take 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,
{
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::new::<core::result::Result<(), E>>()
}
}
}

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