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

Allow specifying payable constructors #1065

Merged
merged 20 commits into from
Jan 18, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
67 changes: 63 additions & 4 deletions crates/lang/codegen/src/generator/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl GenerateCode for Dispatch<'_> {
let constructor_decoder_type =
self.generate_constructor_decoder_type(&constructor_spans);
let message_decoder_type = self.generate_message_decoder_type(&message_spans);
let entry_points = self.generate_entry_points(&message_spans);
let entry_points = self.generate_entry_points(&constructor_spans, &message_spans);
quote! {
#amount_dispatchables
#contract_dispatchable_messages
Expand Down Expand Up @@ -274,6 +274,7 @@ impl Dispatch<'_> {
.map(|constructor| {
let constructor_span = constructor.span();
let constructor_ident = constructor.ident();
let payable = constructor.is_payable();
let selector_id = constructor.composed_selector().into_be_u32().hex_padded_suffixed();
let selector_bytes = constructor.composed_selector().hex_lits();
let input_bindings = generator::input_bindings(constructor.inputs());
Expand All @@ -287,6 +288,7 @@ impl Dispatch<'_> {
const CALLABLE: fn(Self::Input) -> Self::Storage = |#input_tuple_bindings| {
#storage_ident::#constructor_ident( #( #input_bindings ),* )
};
const PAYABLE: ::core::primitive::bool = #payable;
const SELECTOR: [::core::primitive::u8; 4usize] = [ #( #selector_bytes ),* ];
const LABEL: &'static ::core::primitive::str = ::core::stringify!(#constructor_ident);
}
Expand All @@ -297,7 +299,7 @@ impl Dispatch<'_> {
)
}

/// Generate code for the [`ink_lang::DispatchableConstructorInfo`] trait implementations.
/// Generate code for the [`ink_lang::DispatchableMessageInfo`] trait implementations.
///
/// These trait implementations store relevant dispatch information for every
/// dispatchable ink! constructor of the ink! smart contract.
Expand Down Expand Up @@ -410,15 +412,26 @@ impl Dispatch<'_> {
///
/// This generates the `deploy` and `call` functions with which the smart
/// contract runtime mainly interacts with the ink! smart contract.
fn generate_entry_points(&self, message_spans: &[proc_macro2::Span]) -> TokenStream2 {
fn generate_entry_points(
&self,
constructor_spans: &[proc_macro2::Span],
message_spans: &[proc_macro2::Span],
) -> TokenStream2 {
let span = self.contract.module().storage().span();
let storage_ident = self.contract.module().storage().ident();
let any_constructor_accept_payment =
self.any_constructor_accepts_payment_expr(constructor_spans);
let any_message_accept_payment =
self.any_message_accepts_payment_expr(message_spans);
quote_spanned!(span=>
#[cfg(not(test))]
#[no_mangle]
fn deploy() {
if !#any_constructor_accept_payment {
::ink_lang::codegen::deny_payment::<<#storage_ident as ::ink_lang::reflect::ContractEnv>::Env>()
.unwrap_or_else(|error| ::core::panic!("{}", error))
}

::ink_env::decode_input::<
<#storage_ident as ::ink_lang::reflect::ContractConstructorDecoder>::Type>()
.map_err(|_| ::ink_lang::reflect::DispatchError::CouldNotReadInput)
Expand All @@ -438,6 +451,7 @@ impl Dispatch<'_> {
::ink_lang::codegen::deny_payment::<<#storage_ident as ::ink_lang::reflect::ContractEnv>::Env>()
.unwrap_or_else(|error| ::core::panic!("{}", error))
}

::ink_env::decode_input::<
<#storage_ident as ::ink_lang::reflect::ContractMessageDecoder>::Type>()
.map_err(|_| ::ink_lang::reflect::DispatchError::CouldNotReadInput)
Expand Down Expand Up @@ -539,6 +553,9 @@ impl Dispatch<'_> {
}
}
};

let any_constructor_accept_payment =
self.any_constructor_accepts_payment_expr(constructor_spans);
let constructor_execute = (0..count_constructors).map(|index| {
let constructor_span = constructor_spans[index];
let constructor_ident = constructor_variant_ident(index);
Expand All @@ -549,14 +566,25 @@ impl Dispatch<'_> {
}>>::IDS[#index]
}>>::CALLABLE
);
let accepts_payment = quote_spanned!(constructor_span=>
false ||
!#any_constructor_accept_payment ||
<#storage_ident as ::ink_lang::reflect::DispatchableConstructorInfo<{
<#storage_ident as ::ink_lang::reflect::ContractDispatchableConstructors<{
<#storage_ident as ::ink_lang::reflect::ContractAmountDispatchables>::CONSTRUCTORS
}>>::IDS[#index]
}>>::PAYABLE
);
let is_dynamic_storage_allocation_enabled = self
.contract
.config()
.is_dynamic_storage_allocator_enabled();

quote_spanned!(constructor_span=>
Self::#constructor_ident(input) => {
::ink_lang::codegen::execute_constructor::<#storage_ident, _, _>(
::ink_lang::codegen::ExecuteConstructorConfig {
payable: #accepts_payment,
dynamic_storage_alloc: #is_dynamic_storage_allocation_enabled
},
move || { #constructor_callable(input) }
Expand Down Expand Up @@ -597,6 +625,7 @@ impl Dispatch<'_> {
}

impl ::ink_lang::reflect::ExecuteDispatchable for __ink_ConstructorDecoder {
#[allow(clippy::nonminimal_bool)]
fn execute_dispatchable(self) -> ::core::result::Result<(), ::ink_lang::reflect::DispatchError> {
match self {
#( #constructor_execute ),*
Expand Down Expand Up @@ -692,6 +721,7 @@ impl Dispatch<'_> {
}
}
};

let any_message_accept_payment =
self.any_message_accepts_payment_expr(message_spans);
let message_execute = (0..count_messages).map(|index| {
Expand Down Expand Up @@ -731,6 +761,7 @@ impl Dispatch<'_> {
.contract
.config()
.is_dynamic_storage_allocator_enabled();

quote_spanned!(message_span=>
Self::#message_ident(input) => {
let config = ::ink_lang::codegen::ExecuteMessageConfig {
Expand Down Expand Up @@ -759,7 +790,6 @@ impl Dispatch<'_> {
quote_spanned!(span=>
const _: () = {
#[allow(non_camel_case_types)]
// #[derive(::core::fmt::Debug, ::core::cmp::PartialEq)]
pub enum __ink_MessageDecoder {
#( #message_variants ),*
}
Expand Down Expand Up @@ -833,4 +863,33 @@ impl Dispatch<'_> {
{ false #( || #message_is_payable )* }
)
}

/// Generates code to express if any dispatchable ink! constructor accepts payment.
///
/// This information can be used to speed-up dispatch since denying of payment
/// can be generalized to work before dispatch happens if none of the ink! constructors
/// accept payment anyways.
fn any_constructor_accepts_payment_expr(
&self,
constructor_spans: &[proc_macro2::Span],
) -> TokenStream2 {
assert_eq!(constructor_spans.len(), self.query_amount_constructors());

let span = self.contract.module().storage().span();
let storage_ident = self.contract.module().storage().ident();
let count_constructors = self.query_amount_constructors();
let constructor_is_payable = (0..count_constructors).map(|index| {
let constructor_span = constructor_spans[index];
quote_spanned!(constructor_span=>
<#storage_ident as ::ink_lang::reflect::DispatchableConstructorInfo<{
<#storage_ident as ::ink_lang::reflect::ContractDispatchableConstructors<{
<#storage_ident as ::ink_lang::reflect::ContractAmountDispatchables>::CONSTRUCTORS
}>>::IDS[#index]
}>>::PAYABLE
)
});
quote_spanned!(span=>
{ false #( || #constructor_is_payable )* }
)
}
}
70 changes: 54 additions & 16 deletions crates/lang/ir/src/ir/item_impl/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ use syn::spanned::Spanned as _;
pub struct Constructor {
/// The underlying Rust method item.
pub(super) item: syn::ImplItemMethod,
/// If the ink! constructor can receive funds.
is_payable: bool,
/// An optional user provided selector.
///
/// # Note
Expand Down Expand Up @@ -158,15 +160,9 @@ impl Constructor {
&ir::AttributeArgKind::Constructor,
|arg| {
match arg.kind() {
ir::AttributeArg::Constructor | ir::AttributeArg::Selector(_) => {
Ok(())
}
ir::AttributeArg::Payable => {
Err(Some(format_err!(
arg.span(),
"constructors are implicitly payable"
)))
}
ir::AttributeArg::Constructor
| ir::AttributeArg::Payable
| ir::AttributeArg::Selector(_) => Ok(()),
_ => Err(None),
}
},
Expand All @@ -182,9 +178,11 @@ impl TryFrom<syn::ImplItemMethod> for Constructor {
Self::ensure_valid_return_type(&method_item)?;
Self::ensure_no_self_receiver(&method_item)?;
let (ink_attrs, other_attrs) = Self::sanitize_attributes(&method_item)?;
let is_payable = ink_attrs.is_payable();
let selector = ink_attrs.selector();
Ok(Constructor {
selector,
is_payable,
item: syn::ImplItemMethod {
attrs: other_attrs,
..method_item
Expand Down Expand Up @@ -217,7 +215,7 @@ impl Callable for Constructor {
}

fn is_payable(&self) -> bool {
true
self.is_payable
}

fn visibility(&self) -> Visibility {
Expand Down Expand Up @@ -302,6 +300,52 @@ mod tests {
}
}

#[test]
fn is_payable_works() {
let test_inputs: Vec<(bool, syn::ImplItemMethod)> = vec![
// Not payable.
(
false,
syn::parse_quote! {
#[ink(constructor)]
fn my_constructor() -> Self {}
},
),
// Normalized ink! attribute.
(
true,
syn::parse_quote! {
#[ink(constructor, payable)]
pub fn my_constructor() -> Self {}
},
),
// Different ink! attributes.
(
true,
syn::parse_quote! {
#[ink(constructor)]
#[ink(payable)]
pub fn my_constructor() -> Self {}
},
),
// Another ink! attribute, separate and normalized attribute.
(
true,
syn::parse_quote! {
#[ink(constructor)]
#[ink(selector = 0xDEADBEEF, payable)]
pub fn my_constructor() -> Self {}
},
),
];
for (expect_payable, item_method) in test_inputs {
let is_payable = <ir::Constructor as TryFrom<_>>::try_from(item_method)
.unwrap()
.is_payable();
assert_eq!(is_payable, expect_payable);
}
}

#[test]
fn visibility_works() {
let test_inputs: Vec<(bool, syn::ImplItemMethod)> = vec![
Expand Down Expand Up @@ -577,12 +621,6 @@ mod tests {
#[ink(event)]
fn my_constructor() -> Self {}
},
// constructor + payable
syn::parse_quote! {
#[ink(constructor)]
#[ink(payable)]
fn my_constructor() -> Self {}
},
];
for item_method in item_methods {
assert_try_from_fails(
Expand Down
13 changes: 12 additions & 1 deletion crates/lang/src/codegen/dispatch/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ where
/// Configuration for execution of ink! constructor.
#[derive(Debug, Copy, Clone)]
pub struct ExecuteConstructorConfig {
/// Yields `true` if the ink! constructor accepts payment.
///
/// # Note
///
/// If no ink! constructor within the same ink! smart contract
/// is payable then this flag will be `true` since the check
/// then is moved before the constructor dispatch as an optimization.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, not happy about this. This possibility of payable being set to true despite no payable constructor seems to be destined for a bug. How about we reflect this information e.g. with an enum here as a wrapper around the bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there was already a small bug here actually. If there were indeed no payable messages for example, we'd never hit this check. Although to be fair, payment would have been denied in call() (maybe this was the optimization alluded to in the comment?).

Either way, this was confusing and I think it's unnecessary so I removed it.

pub payable: bool,
/// Yields `true` if the dynamic storage allocator has been enabled.
///
/// # Note
Expand All @@ -93,11 +101,14 @@ pub fn execute_constructor<Contract, F, R>(
f: F,
) -> Result<(), DispatchError>
where
Contract: SpreadLayout + ContractRootKey,
Contract: SpreadLayout + ContractRootKey + ContractEnv,
F: FnOnce() -> R,
<private::Seal<R> as ConstructorReturnType<Contract>>::ReturnValue: scale::Encode,
private::Seal<R>: ConstructorReturnType<Contract>,
{
if !config.payable {
deny_payment::<<Contract as ContractEnv>::Env>()?;
}
if config.dynamic_storage_alloc {
alloc::initialize(ContractPhase::Deploy);
}
Expand Down
6 changes: 5 additions & 1 deletion crates/lang/src/reflect/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use core::fmt::Display;
/// # Note
///
/// - This is automatically implemented by all ink! smart contracts.
/// - All ink! constructors and ink! messages of an ink! smart contract are dispatchables.
/// - All ink! constructors and ink! messages of an ink! smart contract are dispatchables.
/// This explicitly includes ink! messages from ink! trait implementations.
///
/// # Usage
Expand Down Expand Up @@ -339,8 +339,12 @@ pub trait DispatchableConstructorInfo<const ID: u32> {
/// The closure that can be used to dispatch into the dispatchable ink! constructor.
const CALLABLE: fn(Self::Input) -> Self::Storage;

/// Yields `true` if the dispatchable ink! constructor is payable.
const PAYABLE: bool;

/// The selectors of the dispatchable ink! constructor.
const SELECTOR: [u8; 4];

/// The label of the dispatchable ink! constructor.
const LABEL: &'static str;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod contract {
pub struct Contract {}

impl Contract {
#[ink(constructor, payable)]
#[ink(constructor, payable = true)]
HCastano marked this conversation as resolved.
Show resolved Hide resolved
pub fn constructor() -> Self {
Self {}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: unknown ink! attribute argument (name = value)
--> tests/ui/contract/fail/constructor-payable-invalid-1.rs:9:28
|
9 | #[ink(constructor, payable = true)]
| ^^^^^^^^^^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use ink_lang as ink;

#[ink::contract]
mod contract {
#[ink(storage)]
pub struct Contract {}

impl Contract {
#[ink(constructor, payable = false)]
pub fn constructor() -> Self {
Self {}
}

#[ink(message)]
pub fn message(&self) {}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: unknown ink! attribute argument (name = value)
--> tests/ui/contract/fail/constructor-payable-invalid-2.rs:9:28
|
9 | #[ink(constructor, payable = false)]
| ^^^^^^^^^^^^^^^
11 changes: 0 additions & 11 deletions crates/lang/tests/ui/contract/fail/constructor-payable.stderr

This file was deleted.

Loading