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
97 changes: 97 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 All @@ -39,11 +40,14 @@ impl GenerateCode for Metadata<'_> {
fn generate_code(&self) -> TokenStream2 {
let contract = self.generate_contract();
let layout = self.generate_layout();
let storage_ident = self.contract.module().storage().ident();

quote! {
#[cfg(feature = "std")]
#[cfg(not(feature = "ink-as-dependency"))]
const _: () = {
impl ::ink::metadata::ConstructorReturnSpec for #storage_ident {}

#[no_mangle]
pub fn __ink_generate_metadata() -> ::ink::metadata::InkProject {
let layout = #layout;
Expand Down Expand Up @@ -131,6 +135,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 +145,7 @@ impl Metadata<'_> {
#( #args ),*
])
.payable(#is_payable)
.returns(#ret_ty)
.docs([
#( #docs ),*
])
Expand Down Expand Up @@ -190,6 +196,34 @@ impl Metadata<'_> {
}
}

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

/// 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 +350,44 @@ impl Metadata<'_> {
}
}

/// Generates ink! metadata for the given return type of a constructor.
/// If the constructor return type is not `Result`,
/// the metadata will not display any type spec for the return type.
/// Otherwise, the return type spec is `Result<(), E>`.
fn generate_constructor_return_type(ret_ty: Option<&syn::Type>) -> TokenStream2 {
match ret_ty {
None => {
quote! {
::ink::metadata::ReturnTypeSpec::new(::core::option::Option::None)
}
}
Some(syn::Type::Path(syn::TypePath { qself: None, path }))
if path.is_ident("Self") =>
{
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);
quote! {
::ink::metadata::ReturnTypeSpec::new(
<#type_token as ::ink::metadata::ConstructorReturnSpec>::generate(#segments)
)
}
}
}
}

/// Helper function to 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 Expand Up @@ -442,4 +514,29 @@ mod tests {
],
)
}

#[test]
fn constructor_return_type_works() {
let expected_no_ret_type_spec = ":: ink :: metadata :: ReturnTypeSpec :: new (:: core :: option :: Option :: None)";
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not a great way to test it using strings like this. It's brittle and dependent upon the codegen impl.

If we can come up with a good solution with types + reflection this will be easier to test.


let actual = Metadata::generate_constructor_return_type(None);
assert_eq!(&actual.to_string(), expected_no_ret_type_spec);

match syn::parse_quote!( -> Self ) {
syn::ReturnType::Type(_, t) => {
let actual = Metadata::generate_constructor_return_type(Some(&t));
assert_eq!(&actual.to_string(), expected_no_ret_type_spec);
}
_ => unreachable!(),
}

match syn::parse_quote!( -> Result<Self, ()> ) {
syn::ReturnType::Type(_, t) => {
let actual = Metadata::generate_constructor_return_type(Some(&t));
let expected = ":: ink :: metadata :: ReturnTypeSpec :: new (< Result < () , () > as :: ink :: metadata :: ConstructorReturnSpec > :: generate (Some (:: core :: iter :: IntoIterator :: into_iter ([:: core :: stringify ! (Result)]) . map (:: core :: convert :: AsRef :: as_ref))))";
assert_eq!(&actual.to_string(), expected);
}
_ => unreachable!(),
}
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,22 @@ note: required by a bound in `execute_constructor`
|
| <private::Seal<R> as ConstructorReturnType<Contract>>::Error: Encode,
| ^^^^^^ required by this bound in `execute_constructor`

error[E0277]: the trait bound `contract::Error: TypeInfo` is not satisfied
--> tests/ui/contract/fail/constructor-return-result-non-codec-error.rs:1:1
|
1 | #[ink::contract]
| ^^^^^^^^^^^^^^^^ the trait `TypeInfo` is not implemented for `contract::Error`
|
= help: the following other types implement trait `TypeInfo`:
&T
&mut T
()
(A, B)
(A, B, C)
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
and $N others
= note: required because of the requirements on the impl of `ConstructorReturnSpec` for `Result<(), contract::Error>`
= note: this error originates in the attribute macro `ink::contract` (in Nightly builds, run with -Z macro-backtrace for more info)
1 change: 1 addition & 0 deletions crates/metadata/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod specs;
mod utils;

pub use self::specs::{
ConstructorReturnSpec,
ConstructorSpec,
ConstructorSpecBuilder,
ContractSpec,
Expand Down
94 changes: 82 additions & 12 deletions crates/metadata/src/specs.rs
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,42 @@ where
}
}

/// Implementing this trait for some type `T` indicates what the return spec of
/// `T` will be in the metadata, given `T` is used as the result of a constructor.
pub trait ConstructorReturnSpec {
/// Generates the type spec.
///
/// Note:
/// The default implementation generates [`TypeSpec`] for `()`, hence it can
/// be used directly for constructor returning `Self`.
fn generate<S>(segments_opt: Option<S>) -> TypeSpec
where
S: IntoIterator<Item = &'static str>,
{
if let Some(segments) = segments_opt {
TypeSpec::with_name_segs::<(), S>(segments)
} else {
TypeSpec::of_type::<()>()
}
}
}

impl<O, E> ConstructorReturnSpec for Result<O, E>
where
E: TypeInfo + 'static,
{
fn generate<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