Skip to content

Commit

Permalink
Allow specifying payable constructors (#1065)
Browse files Browse the repository at this point in the history
* Add `payable` constructor support to IR

* Add payable associated type to `DispatchableConstructorInfo`

* Allow constructors to be payable in dispatcher codegen

* Add UI tests

* Remove test related to constructors being implicitly payable

* Add some failing UI tests

* Deny payments on `deploy` if there are no payable constructors

* Appease Clippy

* Deny payments when executing constructors

* `s/message/constructor/g`

* RustFmt

* Address `unnecessary_to_owned` lint

Reference: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned

* Address `needless_borrow` lint

Reference: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

* Make a note about this PR in the RELEASES doc

* Allow `clippy::nonminimal_bool` for `deploy()`

* Remove `any_payable_*` checks from individual Config creation

It looks like this was meant to be part of an optimization, however it
doesn't look like this is necessary anymore. If no constructor or
message is payable we already do early deny payment checks in `deploy()`
and `call()` respectively.

* Add UI test for multiple non-payable constructors
  • Loading branch information
HCastano authored Jan 18, 2022
1 parent 67ad398 commit e646ddd
Show file tree
Hide file tree
Showing 14 changed files with 261 additions and 43 deletions.
1 change: 1 addition & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This is the 8th release candidate for ink! 3.0.

## Change
- Renamed the `ink_env` function `transferred_balance()` to `transferred_value()`[#1063](/~https://github.com/paritytech/ink/pull/1063).
- Allow specifying payable constructors - [#1065](/~https://github.com/paritytech/ink/pull/1065)

# Version 3.0-rc7

Expand Down
68 changes: 61 additions & 7 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 @@ -267,6 +267,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 @@ -280,6 +281,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 @@ -290,7 +292,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 @@ -402,15 +404,27 @@ 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]
#[allow(clippy::nonminimal_bool)]
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 @@ -431,6 +445,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 @@ -532,6 +547,7 @@ impl Dispatch<'_> {
}
}
};

let constructor_execute = (0..count_constructors).map(|index| {
let constructor_span = constructor_spans[index];
let constructor_ident = constructor_variant_ident(index);
Expand All @@ -542,14 +558,24 @@ impl Dispatch<'_> {
}>>::IDS[#index]
}>>::CALLABLE
);
let accepts_payment = quote_spanned!(constructor_span=>
false ||
<#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 @@ -590,6 +616,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 @@ -685,8 +712,7 @@ impl Dispatch<'_> {
}
}
};
let any_message_accept_payment =
self.any_message_accepts_payment_expr(message_spans);

let message_execute = (0..count_messages).map(|index| {
let message_span = message_spans[index];
let message_ident = message_variant_ident(index);
Expand All @@ -706,7 +732,6 @@ impl Dispatch<'_> {
);
let accepts_payment = quote_spanned!(message_span=>
false ||
!#any_message_accept_payment ||
<#storage_ident as ::ink_lang::reflect::DispatchableMessageInfo<{
<#storage_ident as ::ink_lang::reflect::ContractDispatchableMessages<{
<#storage_ident as ::ink_lang::reflect::ContractAmountDispatchables>::MESSAGES
Expand All @@ -724,6 +749,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 @@ -752,7 +778,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 @@ -826,4 +851,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: 6 additions & 7 deletions crates/lang/src/codegen/dispatch/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ where
/// Configuration for execution of ink! constructor.
#[derive(Debug, Copy, Clone)]
pub struct ExecuteConstructorConfig {
/// Yields `true` if the ink! constructor accepts payment.
pub payable: bool,
/// Yields `true` if the dynamic storage allocator has been enabled.
///
/// # Note
Expand All @@ -93,11 +95,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 Expand Up @@ -280,12 +285,6 @@ impl<C, E> InitializerReturnType<C> for Result<(), E> {
#[derive(Debug, Copy, Clone)]
pub struct ExecuteMessageConfig {
/// Yields `true` if the ink! message accepts payment.
///
/// # Note
///
/// If no ink! message within the same ink! smart contract
/// is payable then this flag will be `true` since the check
/// then is moved before the message dispatch as an optimization.
pub payable: bool,
/// Yields `true` if the ink! message might mutate contract storage.
///
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)]
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() {}
Loading

0 comments on commit e646ddd

Please sign in to comment.