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

Implement wildcard selector, add proxy example #1020

Merged
merged 42 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
64901a9
Implement wildcard selector, add `proxy` example
cmichi Nov 9, 2021
2eaf655
Minor code and text improvements
cmichi Nov 17, 2021
bd7eea1
Remove code
cmichi Nov 17, 2021
7b7c43b
Fix function name
cmichi Nov 17, 2021
4677a59
Add unit tests for lang parsing
cmichi Nov 17, 2021
fbe82d2
Clarify where state resides
cmichi Nov 17, 2021
2e633e3
Apply suggestions from code review
Nov 17, 2021
23e2baf
Implement comments
cmichi Nov 17, 2021
75886af
Apply suggestions from code review
Nov 22, 2021
0299f4d
Pre-process `_` to `"_"`
cmichi Nov 18, 2021
1d102b0
Update String error message
cmichi Nov 22, 2021
7f1a7ea
Merge branch 'master' into cmichi-implement-wildcard-selector
cmichi Nov 22, 2021
7dba798
Rename `is_wildcard_selector` to `has_wildcard_selector`
cmichi Nov 22, 2021
66a3455
Move `_` to `"_"` transformation to lower layer
cmichi Nov 22, 2021
f643a30
Update test fixtures
cmichi Nov 22, 2021
1b0c396
Update error message
cmichi Nov 22, 2021
5cb1f3c
Update test fixtures
cmichi Nov 22, 2021
ee3d140
Migrate to enum `SelectorOrWildcard`
cmichi Nov 23, 2021
3ed386c
Allow wildcard selectors for constructors
cmichi Nov 23, 2021
724ff9f
Fix tests
cmichi Nov 23, 2021
f4a2765
Include clippy suggestion
cmichi Nov 23, 2021
dc2a9fd
Fix error output matching
cmichi Nov 23, 2021
17067e7
Apply suggestions from code review
Nov 23, 2021
a7e624a
Fix suggestions
cmichi Nov 23, 2021
f1429cf
Implement reviewer suggestions
cmichi Nov 23, 2021
0b7cf6e
Merge branch 'master' into cmichi-implement-wildcard-selector
cmichi Nov 23, 2021
cc078ea
Improve matching
cmichi Nov 23, 2021
c2a2697
Include clippy suggestion
cmichi Nov 23, 2021
99a4ff8
Update test fixture
cmichi Nov 23, 2021
a2ba30a
Detect multiple wildcard selectors in constructors
cmichi Nov 23, 2021
9f58738
Apply suggestions from code review
Nov 23, 2021
1a7b33c
Rename enum variant `Selector` to `UserProvided`
cmichi Nov 23, 2021
00f360d
Return `Result:Err` instead of asserting
cmichi Nov 23, 2021
1bc3bb6
Implement suggestions
cmichi Nov 23, 2021
98da359
Revert "Return `Result:Err` instead of asserting"
cmichi Nov 23, 2021
b498308
Apply `cargo fmt`
cmichi Nov 23, 2021
36a374c
Disable overflow checks for `proxy` example
cmichi Nov 23, 2021
d55c84b
Return `Result:Err` instead of asserting
cmichi Nov 24, 2021
fbc6097
Fix import path
cmichi Nov 24, 2021
7698ad5
Fix import path
cmichi Nov 24, 2021
c297f50
Fix import path
cmichi Nov 24, 2021
21bb558
Revert "Return `Result:Err` instead of asserting"
cmichi Nov 24, 2021
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ In a module annotated with `#[ink::contract]` these attributes are available:
| `#[ink(constructor)]` | Applicable to method. | Flags a method for the ink! storage struct as constructor making it available to the API for instantiating the contract. |
| `#[ink(payable)]` | Applicable to ink! messages. | Allows receiving value as part of the call of the ink! message. ink! constructors are implicitly payable. |
| `#[ink(selector = S:u32)]` | Applicable to ink! messages and ink! constructors. | Specifies a concrete dispatch selector for the flagged entity. This allows a contract author to precisely control the selectors of their APIs making it possible to rename their API without breakage. |
| `#[ink(selector = _)]` | Applicable to ink! messages. | Specifies a fallback message that is invoked if no other ink! message matches a selector. |
| `#[ink(namespace = N:string)]` | Applicable to ink! trait implementation blocks. | Changes the resulting selectors of all the ink! messages and ink! constructors within the trait implementation. Allows to disambiguate between trait implementations with overlapping message or constructor names. Use only with great care and consideration! |
| `#[ink(impl)]` | Applicable to ink! implementation blocks. | Tells the ink! codegen that some implementation block shall be granted access to ink! internals even without it containing any ink! messages or ink! constructors. |

Expand Down
4 changes: 4 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ This is the 7th release candidate for ink! 3.0.
- Some of those traits and their carried information can be used for static reflection of ink!
smart contracts. Those types and traits reside in the new `ink_lang::reflect` module and is
publicly usable by ink! smart contract authors.
- Added basic support for wildcard selectors ‒ [#1020](https://github.com/paritytech/ink/pull/1020).
- This enables writing upgradable smart contracts using the proxy pattern.
We added a new example illustrating this ‒ the [proxy](https://github.com/paritytech/ink/tree/master/examples/proxy) example.
- Annotating a wildcard selector in traits is not supported.

## Changed
- Upgraded to the unstable `seal_call` API ‒ [#960](https://github.com/paritytech/ink/pull/960).
Expand Down
71 changes: 65 additions & 6 deletions crates/lang/codegen/src/generator/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,26 @@ impl Dispatch<'_> {
.count()
}

/// Returns the index of the ink! message which has a wildcard selector, if existent.
fn query_wildcard_message(&self) -> Option<usize> {
cmichi marked this conversation as resolved.
Show resolved Hide resolved
self.contract
.module()
.impls()
.map(|item_impl| item_impl.iter_messages())
.flatten()
.position(|item| item.has_wildcard_selector())
}

/// Returns the index of the ink! constructor which has a wildcard selector, if existent.
fn query_wildcard_constructor(&self) -> Option<usize> {
self.contract
.module()
.impls()
.map(|item_impl| item_impl.iter_constructors())
.flatten()
.position(|item| item.has_wildcard_selector())
}

/// Generates code for the [`ink_lang::ContractDispatchables`] trait implementation.
///
/// This trait implementation stores information of how many dispatchable
Expand Down Expand Up @@ -495,6 +515,30 @@ impl Dispatch<'_> {
}
)
});
let possibly_wildcard_selector_constructor = match self
.query_wildcard_constructor()
{
Some(wildcard_index) => {
let constructor_span = constructor_spans[wildcard_index];
let constructor_ident = constructor_variant_ident(wildcard_index);
let constructor_input = expand_constructor_input(
constructor_span,
storage_ident,
wildcard_index,
);
quote! {
::core::result::Result::Ok(Self::#constructor_ident(
<#constructor_input as ::scale::Decode>::decode(input)
.map_err(|_| ::ink_lang::reflect::DispatchError::InvalidParameters)?
))
}
}
None => {
quote! {
::core::result::Result::Err(::ink_lang::reflect::DispatchError::UnknownSelector)
}
}
};
let constructor_execute = (0..count_constructors).map(|index| {
let constructor_span = constructor_spans[index];
let constructor_ident = constructor_variant_ident(index);
Expand Down Expand Up @@ -537,9 +581,7 @@ impl Dispatch<'_> {
.map_err(|_| ::ink_lang::reflect::DispatchError::InvalidSelector)?
{
#( #constructor_match , )*
_invalid => ::core::result::Result::Err(
::ink_lang::reflect::DispatchError::UnknownSelector
)
_invalid => #possibly_wildcard_selector_constructor
}
}
}
Expand Down Expand Up @@ -631,6 +673,25 @@ impl Dispatch<'_> {
}
)
});
let possibly_wildcard_selector_message = match self.query_wildcard_message() {
Some(wildcard_index) => {
let message_span = message_spans[wildcard_index];
let message_ident = message_variant_ident(wildcard_index);
let message_input =
expand_message_input(message_span, storage_ident, wildcard_index);
quote! {
::core::result::Result::Ok(Self::#message_ident(
<#message_input as ::scale::Decode>::decode(input)
.map_err(|_| ::ink_lang::reflect::DispatchError::InvalidParameters)?
))
}
}
None => {
quote! {
::core::result::Result::Err(::ink_lang::reflect::DispatchError::UnknownSelector)
}
}
};
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 @@ -713,9 +774,7 @@ impl Dispatch<'_> {
.map_err(|_| ::ink_lang::reflect::DispatchError::InvalidSelector)?
{
#( #message_match , )*
_invalid => ::core::result::Result::Err(
::ink_lang::reflect::DispatchError::UnknownSelector
)
_invalid => #possibly_wildcard_selector_message
}
}
}
Expand Down
145 changes: 125 additions & 20 deletions crates/lang/ir/src/ir/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ use core::{
result::Result,
};
use proc_macro2::{
Group as Group2,
Ident,
Span,
TokenStream as TokenStream2,
TokenTree as TokenTree2,
};
use std::collections::HashMap;
use syn::spanned::Spanned;
Expand Down Expand Up @@ -265,7 +268,7 @@ impl InkAttribute {
}

/// Returns the selector of the ink! attribute if any.
cmichi marked this conversation as resolved.
Show resolved Hide resolved
pub fn selector(&self) -> Option<ir::Selector> {
pub fn selector(&self) -> Option<SelectorOrWildcard> {
self.args().find_map(|arg| {
if let ir::AttributeArg::Selector(selector) = arg.kind() {
return Some(*selector)
Expand All @@ -280,6 +283,16 @@ impl InkAttribute {
.any(|arg| matches!(arg.kind(), AttributeArg::Payable))
}

/// Returns `true` if the ink! attribute contains the wildcard selector.
pub fn has_wildcard_selector(&self) -> bool {
self.args().any(|arg| {
matches!(
arg.kind(),
AttributeArg::Selector(SelectorOrWildcard::Wildcard)
)
})
}

/// Returns `true` if the ink! attribute contains the `anonymous` argument.
pub fn is_anonymous(&self) -> bool {
self.args()
Expand Down Expand Up @@ -342,6 +355,7 @@ pub enum AttributeArgKind {
Constructor,
/// `#[ink(payable)]`
Payable,
/// `#[ink(selector = _)]`
/// `#[ink(selector = 0xDEADBEEF)]`
Selector,
/// `#[ink(extension = N: u32)]`
Expand Down Expand Up @@ -395,11 +409,15 @@ pub enum AttributeArg {
/// Applied on ink! constructors or messages in order to specify that they
/// can receive funds from callers.
Payable,
/// `#[ink(selector = 0xDEADBEEF)]`
/// Can be either one of:
///
/// Applied on ink! constructors or messages to manually control their
/// selectors.
Selector(Selector),
/// - `#[ink(selector = 0xDEADBEEF)]`
/// Applied on ink! constructors or messages to manually control their
/// selectors.
/// - `#[ink(selector = _)]`
/// Applied on ink! messages to define a fallback messages that is invoked
/// if no other ink! message matches a given selector.
Selector(SelectorOrWildcard),
/// `#[ink(namespace = "my_namespace")]`
///
/// Applied on ink! trait implementation blocks to disambiguate other trait
Expand Down Expand Up @@ -449,7 +467,7 @@ impl core::fmt::Display for AttributeArgKind {
Self::Constructor => write!(f, "constructor"),
Self::Payable => write!(f, "payable"),
Self::Selector => {
write!(f, "selector = S:[u8; 4]")
write!(f, "selector = S:[u8; 4] || _")
}
Self::Extension => {
write!(f, "extension = N:u32)")
Expand Down Expand Up @@ -495,9 +513,7 @@ impl core::fmt::Display for AttributeArg {
Self::Message => write!(f, "message"),
Self::Constructor => write!(f, "constructor"),
Self::Payable => write!(f, "payable"),
Self::Selector(selector) => {
write!(f, "selector = {:?}", selector.to_bytes())
}
Self::Selector(selector) => core::fmt::Display::fmt(&selector, f),
Self::Extension(extension) => {
write!(f, "extension = {:?}", extension.into_u32())
}
Expand All @@ -511,6 +527,32 @@ impl core::fmt::Display for AttributeArg {
}
}

/// Either a wildcard selector or a specified selector.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum SelectorOrWildcard {
/// A wildcard selector. If no other selector matches, the message/constructor
/// annotated with the wildcard selector will be invoked.
Wildcard,
/// A user provided selector.
UserProvided(ir::Selector),
}

impl SelectorOrWildcard {
/// Create a new `SelectorOrWildcard::Selector` from the supplied bytes.
fn selector(bytes: [u8; 4]) -> SelectorOrWildcard {
SelectorOrWildcard::UserProvided(Selector::from(bytes))
}
}

impl core::fmt::Display for SelectorOrWildcard {
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
match self {
Self::UserProvided(selector) => core::fmt::Debug::fmt(&selector, f),
Self::Wildcard => write!(f, "_"),
}
}
}

/// An ink! namespace applicable to a trait implementation block.
#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Namespace {
Expand Down Expand Up @@ -723,13 +765,66 @@ impl From<InkAttribute> for Attribute {
}
}

/// This function replaces occurrences of a `TokenTree::Ident` of the sequence
/// `selector = _` with the sequence `selector = "_"`.
///
/// This is done because `syn::Attribute::parse_meta` does not support parsing a
/// verbatim like `_`. For this we would need to switch to `syn::Attribute::parse_args`,
/// which requires a more in-depth rewrite of our IR parsing.
fn transform_wildcard_selector_to_string(group: Group2) -> TokenTree2 {
let mut found_selector = false;
let mut found_equal = false;

let new_group: TokenStream2 = group
.stream()
.into_iter()
.map(|tt| {
match tt {
TokenTree2::Group(grp) => transform_wildcard_selector_to_string(grp),
TokenTree2::Ident(ident)
if found_selector && found_equal && ident == "_" =>
{
let mut lit = proc_macro2::Literal::string("_");
lit.set_span(ident.span());
found_selector = false;
found_equal = false;
TokenTree2::Literal(lit)
}
TokenTree2::Ident(ident) if ident == "selector" => {
found_selector = true;
TokenTree2::Ident(ident)
}
TokenTree2::Punct(punct) if punct.as_char() == '=' => {
found_equal = true;
TokenTree2::Punct(punct)
}
_ => tt,
}
})
.collect();
TokenTree2::Group(Group2::new(group.delimiter(), new_group))
}

impl TryFrom<syn::Attribute> for InkAttribute {
type Error = syn::Error;

fn try_from(attr: syn::Attribute) -> Result<Self, Self::Error> {
fn try_from(mut attr: syn::Attribute) -> Result<Self, Self::Error> {
if !attr.path.is_ident("ink") {
return Err(format_err_spanned!(attr, "unexpected non-ink! attribute"))
}

let ts: TokenStream2 = attr
.tokens
.into_iter()
.map(|tt| {
match tt {
TokenTree2::Group(grp) => transform_wildcard_selector_to_string(grp),
_ => tt,
}
})
.collect();
Comment on lines +816 to +825
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very fragile because it changes syntax without looking at the semantics.
A point where this can fail is any input that includes a _ token, such as:

#[compose(Bar, _)]
#[precondition x == Some(_)]

Since with this implementation we accept _ as well as "_" we should just remove this transformation and instead use a custom parsing local to this code point that parses the given attr.tokens instead of blindfoldedly applying these generic transformations to its input. And if the parsing finds selector = _ then it can replace it with selector = "_". This way we have a stable implementation without requiring too much efforts into re-implementing the custom parsing for attributes everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's fragile, the case you mentioned is not possible though, since the impl of transform_wildcard_selector_to_string already has two preconditions that match on selector = before replacing.

attr.tokens = ts;

match attr.parse_meta().map_err(|_| {
format_err_spanned!(attr, "unexpected ink! attribute structure")
})? {
Expand Down Expand Up @@ -809,6 +904,23 @@ impl TryFrom<syn::NestedMeta> for AttributeFrag {
match &meta {
syn::Meta::NameValue(name_value) => {
if name_value.path.is_ident("selector") {
if let syn::Lit::Str(lit_str) = &name_value.lit {
let argument = lit_str.value();
// We've pre-processed the verbatim `_` to `"_"`. This was done
// because `syn::Attribute::parse_meta` does not support verbatim.
if argument != "_" {
return Err(format_err!(
name_value,
"#[ink(selector = ..)] attributes with string inputs are deprecated. \
use an integer instead, e.g. #[ink(selector = 1)] or #[ink(selector = 0xC0DECAFE)]."
))
}
return Ok(AttributeFrag {
ast: meta,
arg: AttributeArg::Selector(SelectorOrWildcard::Wildcard),
})
}
Comment on lines +907 to +922
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd appreciate if we create a follow-up issue that fixes this work around with a proper implementation that customizes the argument parsing.


if let syn::Lit::Int(lit_int) = &name_value.lit {
let selector_u32 = lit_int.base10_parse::<u32>()
.map_err(|error| {
Expand All @@ -821,16 +933,9 @@ impl TryFrom<syn::NestedMeta> for AttributeFrag {
let selector = Selector::from(selector_u32.to_be_bytes());
return Ok(AttributeFrag {
ast: meta,
arg: AttributeArg::Selector(selector),
arg: AttributeArg::Selector(SelectorOrWildcard::UserProvided(selector)),
})
}
if let syn::Lit::Str(_) = &name_value.lit {
return Err(format_err!(
name_value,
"#[ink(selector = ..)] attributes with string inputs are deprecated. \
use an integer instead, e.g. #[ink(selector = 1)] or #[ink(selector = 0xC0DECAFE)]."
))
}
return Err(format_err!(name_value, "expecteded 4-digit hexcode for `selector` argument, e.g. #[ink(selector = 0xC0FEBABE]"))
}
if name_value.path.is_ident("namespace") {
Expand Down Expand Up @@ -1116,15 +1221,15 @@ mod tests {
#[ink(selector = 42)]
},
Ok(test::Attribute::Ink(vec![AttributeArg::Selector(
Selector::from([0, 0, 0, 42]),
SelectorOrWildcard::UserProvided(Selector::from([0, 0, 0, 42])),
)])),
);
assert_attribute_try_from(
syn::parse_quote! {
#[ink(selector = 0xDEADBEEF)]
},
Ok(test::Attribute::Ink(vec![AttributeArg::Selector(
Selector::from([0xDE, 0xAD, 0xBE, 0xEF]),
SelectorOrWildcard::selector([0xDE, 0xAD, 0xBE, 0xEF]),
)])),
);
}
Expand Down
7 changes: 7 additions & 0 deletions crates/lang/ir/src/ir/item_impl/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ where
<C as Callable>::is_payable(self.callable)
}

fn has_wildcard_selector(&self) -> bool {
<C as Callable>::has_wildcard_selector(self.callable)
}

fn visibility(&self) -> Visibility {
<C as Callable>::visibility(self.callable)
}
Expand Down Expand Up @@ -162,6 +166,9 @@ pub trait Callable {
/// Flagging as payable is done using the `#[ink(payable)]` attribute.
fn is_payable(&self) -> bool;

/// Returns `true` if the ink! callable is flagged as a wildcard selector.
fn has_wildcard_selector(&self) -> bool;

/// Returns the visibility of the ink! callable.
fn visibility(&self) -> Visibility;

Expand Down
Loading