Commit 48fb16f0 authored by Michael Müller's avatar Michael Müller Committed by Hero Bird

[abi] Improve JSON encoding of selectors + layout key (#207)

* Export selector as hex string

To have an unambiguous exported representation.
The previously used `u32` can be interpreted
differently depending on the endianness of the
target system.

* Improve JSON encoding of Key `layout` in Metadata

* Add tests for selector serialization

* Encode selector as [u8; 4]

* Remove unused import

* Satisfy rustfmt check

* Improve messages! macro

* Convert tabs to spaces.
* Replace `DELIMITER` with `@delimiter`.
* Improve comments.

* Improve comment

* Ensure old ABI format stays the same

* Reduce code duplication by introducing selector_to_expr()

* Satisfy rustfmt
parent b0429dea
Pipeline #54736 failed with stages
in 1 minute and 33 seconds
......@@ -19,6 +19,9 @@ derive_more = { version = "0.15", default-features = false, features = ["no_std"
ink_abi_derive = { version = "0.1.0", path = "derive", default-features = false, optional = true }
type-metadata = { git = "https://github.com/type-metadata/type-metadata.git", default-features = false, features = ["derive"] }
[dev-dependencies]
serde_json = "1.0"
[features]
default = [
"std",
......
......@@ -15,7 +15,11 @@
// along with ink!. If not, see <http://www.gnu.org/licenses/>.
use derive_more::From;
use serde::Serialize;
use serde::{
Serialize,
Serializer,
};
use std::fmt::Write;
use type_metadata::{
form::{
CompactForm,
......@@ -158,7 +162,7 @@ impl IntoCompact for LayoutField {
#[serde(bound = "F::TypeId: Serialize")]
pub struct LayoutRange<F: Form = MetaForm> {
/// The single key for cells or the starting key address for chunks.
#[serde(rename = "range.offset")]
#[serde(rename = "range.offset", serialize_with = "serialize_key")]
offset: LayoutKey,
/// The amount of associated key addresses starting from the offset key.
#[serde(rename = "range.len")]
......@@ -205,3 +209,52 @@ impl LayoutRange {
}
}
}
fn serialize_key<S>(key: &LayoutKey, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let bytes = key.0;
let mut hex = String::with_capacity(bytes.len() * 2 + 2);
write!(hex, "0x").expect("failed writing to string");
for byte in &bytes {
write!(hex, "{:02x}", byte).expect("failed writing to string");
}
serializer.serialize_str(&hex)
}
#[cfg(test)]
mod tests {
use super::*;
use type_metadata::{
form::{
Form,
MetaForm,
},
IntoCompact,
Registry,
};
#[test]
fn key_must_serialize_to_hex() {
// given
let type_id = <MetaForm as Form>::TypeId::new::<u32>();
let offset = LayoutKey([1; 32]);
let cs: LayoutRange<MetaForm> = LayoutRange {
offset,
len: 1337,
elem_ty: type_id,
};
let mut registry = Registry::new();
// when
let json = serde_json::to_string(&cs.into_compact(&mut registry)).unwrap();
// then
assert_eq!(
json,
"{\"range.offset\":\"0x0101010101010101010101010101010101010101010101010101010101010101\",\"range.len\":1337,\"range.elem_type\":1}"
);
}
}
......@@ -15,7 +15,10 @@
// along with ink!. If not, see <http://www.gnu.org/licenses/>.
use core::marker::PhantomData;
use serde::Serialize;
use serde::{
Serialize,
Serializer,
};
use type_metadata::{
form::{
CompactForm,
......@@ -190,7 +193,8 @@ pub struct ConstructorSpec<F: Form = MetaForm> {
/// The name of the message.
name: F::String,
/// The selector hash of the message.
selector: u32,
#[serde(serialize_with = "serialize_selector")]
selector: [u8; 4],
/// The parameters of the deploy handler.
args: Vec<MessageParamSpec<F>>,
/// The deploy handler documentation.
......@@ -234,7 +238,7 @@ impl ConstructorSpec {
ConstructorSpecBuilder {
spec: Self {
name,
selector: 0,
selector: [0u8; 4],
args: Vec::new(),
docs: Vec::new(),
},
......@@ -245,7 +249,7 @@ impl ConstructorSpec {
impl ConstructorSpecBuilder<Missing<state::Selector>> {
/// Sets the function selector of the message.
pub fn selector(self, selector: u32) -> ConstructorSpecBuilder<state::Selector> {
pub fn selector(self, selector: [u8; 4]) -> ConstructorSpecBuilder<state::Selector> {
ConstructorSpecBuilder {
spec: ConstructorSpec {
selector,
......@@ -294,7 +298,8 @@ pub struct MessageSpec<F: Form = MetaForm> {
/// The name of the message.
name: F::String,
/// The selector hash of the message.
selector: u32,
#[serde(serialize_with = "serialize_selector")]
selector: [u8; 4],
/// If the message is allowed to mutate the contract state.
mutates: bool,
/// The parameters of the message.
......@@ -333,7 +338,7 @@ impl MessageSpec {
MessageSpecBuilder {
spec: Self {
name,
selector: 0,
selector: [0u8; 4],
mutates: false,
args: Vec::new(),
return_type: ReturnTypeSpec::new(None),
......@@ -358,7 +363,10 @@ pub struct MessageSpecBuilder<Selector, Mutates, Returns> {
impl<M, R> MessageSpecBuilder<Missing<state::Selector>, M, R> {
/// Sets the function selector of the message.
pub fn selector(self, selector: u32) -> MessageSpecBuilder<state::Selector, M, R> {
pub fn selector(
self,
selector: [u8; 4],
) -> MessageSpecBuilder<state::Selector, M, R> {
MessageSpecBuilder {
spec: MessageSpec {
selector,
......@@ -795,3 +803,38 @@ impl MessageParamSpecBuilder {
self.spec
}
}
fn serialize_selector<S>(s: &[u8; 4], serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let hex = format!("0x{:02X}{:02X}{:02X}{:02X}", s[0], s[1], s[2], s[3]);
serializer.serialize_str(&hex)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn construct_selector_must_serialize_to_hex() {
// given
let name = <MetaForm as Form>::String::from("foo");
let cs: ConstructorSpec<MetaForm> = ConstructorSpec {
name,
selector: 123456789u32.to_be_bytes(),
args: Vec::new(),
docs: Vec::new(),
};
let mut registry = Registry::new();
// when
let json = serde_json::to_string(&cs.into_compact(&mut registry)).unwrap();
// then
assert_eq!(
json,
"{\"name\":1,\"selector\":\"0x075BCD15\",\"args\":[],\"docs\":[]}"
);
}
}
......@@ -40,10 +40,9 @@ struct CallAbi {
impl CallAbi {
/// Creates new call ABI data for the given selector.
pub fn new(selector: u32) -> Self {
let bytes = selector.to_le_bytes();
pub fn new(selector: [u8; 4]) -> Self {
Self {
raw: vec![bytes[0], bytes[1], bytes[2], bytes[3]],
raw: vec![selector[0], selector[1], selector[2], selector[3]],
}
}
......@@ -177,7 +176,7 @@ where
E::Balance: Default,
{
/// Instantiates an evaluatable (returns data) remote smart contract call.
pub fn eval(account_id: E::AccountId, selector: u32) -> Self {
pub fn eval(account_id: E::AccountId, selector: [u8; 4]) -> Self {
Self {
account_id,
gas_limit: 0,
......@@ -194,7 +193,7 @@ where
E::Balance: Default,
{
/// Instantiates a non-evaluatable (returns no data) remote smart contract call.
pub fn invoke(account_id: E::AccountId, selector: u32) -> Self {
pub fn invoke(account_id: E::AccountId, selector: [u8; 4]) -> Self {
Self {
account_id,
gas_limit: 0,
......
......@@ -21,6 +21,7 @@
use crate::{
ast,
gen::selector_to_expr,
hir,
};
use proc_macro2::TokenStream as TokenStream2;
......@@ -121,7 +122,7 @@ fn generate_abi_constructor(contract: &hir::Contract) -> TokenStream2 {
quote! {
ink_abi::ConstructorSpec::new("on_deploy")
.selector(0)
.selector([0u8; 4])
.args(vec![
#(#args ,)*
])
......@@ -136,7 +137,7 @@ fn generate_abi_messages<'a>(
contract: &'a hir::Contract,
) -> impl Iterator<Item = TokenStream2> + 'a {
contract.messages.iter().map(|message| {
let selector = message.selector();
let selector = selector_to_expr(message.selector());
let is_mut = message.is_mut();
let docs = message.docs().map(trim_doc_string);
let name = message.sig.ident.to_string();
......
......@@ -24,6 +24,7 @@
use crate::{
ast,
gen::selector_to_expr,
hir,
};
use proc_macro2::TokenStream as TokenStream2;
......@@ -236,7 +237,7 @@ fn generate_call_enhancer_messages<'a>(
.map(|ident| quote! { #ident });
let output = &message.sig.decl.output;
let (_impl_generics, type_generics, where_clause) = message.sig.decl.generics.split_for_impl();
let selector = message.selector();
let selector = selector_to_expr(message.selector());
match output {
syn::ReturnType::Default => quote_spanned! { ident.span() =>
#(#attrs)*
......
......@@ -21,11 +21,11 @@
use crate::{
ast,
gen::selector_to_expr,
hir,
};
use proc_macro2::{
Ident,
Span,
TokenStream as TokenStream2,
};
use quote::{
......@@ -473,12 +473,7 @@ fn codegen_for_messages(tokens: &mut TokenStream2, contract: &hir::Contract) {
for attr in &message.attrs {
attr.to_tokens(&mut content)
}
let msg_selector = message.selector();
let msg_id = syn::LitInt::new(
msg_selector.into(),
syn::IntSuffix::None,
Span::call_site(),
);
let msg_id = selector_to_expr(message.selector());
msg_id.to_tokens(&mut content);
<Token![=>]>::default().to_tokens(&mut content);
use heck::CamelCase as _;
......
......@@ -23,6 +23,10 @@ mod test;
use crate::hir;
use proc_macro2::TokenStream as TokenStream2;
use quote::quote;
use syn::{
parse_str,
Expr,
};
/// Generates code for the given contract.
///
......@@ -39,3 +43,13 @@ pub fn generate_code(contract: &hir::Contract) -> TokenStream2 {
as_dependency::generate_code(&mut tokens, contract);
tokens
}
/// The function is required because syn doesn't provide a
/// `ToTokens` implementation for `[u8; 4]`.
fn selector_to_expr(selector: [u8; 4]) -> Expr {
let selector = format!(
"[{}, {}, {}, {}]",
selector[0], selector[1], selector[2], selector[3]
);
parse_str::<syn::Expr>(&selector).expect("failed to parse selector")
}
......@@ -507,14 +507,14 @@ impl Message {
}
/// Returns the message selector for this message.
pub fn selector(&self) -> u32 {
pub fn selector(&self) -> [u8; 4] {
raw_message_selector(self.sig.ident.to_string().as_str())
}
}
fn raw_message_selector(name: &str) -> u32 {
fn raw_message_selector(name: &str) -> [u8; 4] {
let keccak = ink_utils::hash::keccak256(name.as_bytes());
u32::from_le_bytes([keccak[0], keccak[1], keccak[2], keccak[3]])
[keccak[3], keccak[2], keccak[1], keccak[0]]
}
impl From<&ast::ItemImplMethod> for Message {
......@@ -570,8 +570,8 @@ mod tests {
#[test]
fn message_selectors() {
assert_eq!(raw_message_selector("inc"), 257544423);
assert_eq!(raw_message_selector("get"), 4266279973);
assert_eq!(raw_message_selector("compare"), 363906316);
assert_eq!(raw_message_selector("inc"), [15, 89, 208, 231]);
assert_eq!(raw_message_selector("get"), [254, 74, 68, 37]);
assert_eq!(raw_message_selector("compare"), [21, 176, 197, 12]);
}
}
......@@ -478,9 +478,11 @@ impl TryFrom<&hir::Message> for MessageDescription {
type Error = syn::Error;
fn try_from(message: &hir::Message) -> Result<Self> {
let s = message.selector();
let selector = u32::from_le_bytes([s[3], s[2], s[1], s[0]]).into();
Ok(Self {
name: message.sig.ident.to_string(),
selector: message.selector().into(),
selector,
mutates: message.is_mut(),
args: {
message
......
......@@ -115,13 +115,13 @@ fn contract_compiles() {
/// # Note
///
/// Also emits an event.
257544423 => Inc();
[15, 89, 208, 231] => Inc();
/// Decrements the internal counter.
///
/// # Note
///
/// Also emits an event.
1772705147 => Dec();
[105, 169, 85, 123] => Dec();
}
}
......@@ -310,14 +310,14 @@ fn contract_compiles() {
ink_abi::ContractSpec::new("CallCounter")
.constructors(vec![
ink_abi::ConstructorSpec::new("on_deploy")
.selector(0)
.selector([0u8; 4])
.args(vec![])
.docs(vec![])
.done()
])
.messages(vec![
ink_abi::MessageSpec::new("inc")
.selector(257544423u32)
.selector([15, 89, 208, 231])
.mutates(true)
.args(vec![])
.docs(vec![
......@@ -332,7 +332,7 @@ fn contract_compiles() {
)
.done(),
ink_abi::MessageSpec::new("dec")
.selector(1772705147u32)
.selector([105, 169, 85, 123])
.mutates(true)
.args(vec![])
.docs(vec![
......@@ -496,7 +496,7 @@ fn contract_compiles() {
/// Also emits an event.
pub fn inc(self,) -> ink_core::env::CallBuilder<Env, ()> {
ink_core::env::CallBuilder::<Env, ()>::invoke(
self.contract.account_id.clone(), 257544423u32)
self.contract.account_id.clone(), [15, 89, 208, 231])
}
/// Decrements the internal counter.
......@@ -506,7 +506,7 @@ fn contract_compiles() {
/// Also emits an event.
pub fn dec(self,) -> ink_core::env::CallBuilder<Env, ()> {
ink_core::env::CallBuilder::<Env, ()>::invoke(
self.contract.account_id.clone(), 1772705147u32)
self.contract.account_id.clone(), [105, 169, 85, 123])
}
}
}
......
......@@ -90,9 +90,9 @@ fn contract_compiles() {
ink_model::messages! {
/// Flips the internal boolean.
970692492 => Flip();
[57, 219, 151, 140] => Flip();
/// Returns the internal boolean.
4266279973 => Get() -> bool;
[254, 74, 68, 37] => Get() -> bool;
}
}
......@@ -208,7 +208,7 @@ fn contract_compiles() {
ink_abi::ContractSpec::new("Flipper")
.constructors(vec![
ink_abi::ConstructorSpec::new("on_deploy")
.selector(0)
.selector([0u8; 4])
.args(vec![])
.docs(vec![
"The internal boolean is initialized with `true`.",
......@@ -217,14 +217,14 @@ fn contract_compiles() {
])
.messages(vec![
ink_abi::MessageSpec::new("flip")
.selector(970692492u32)
.selector([57, 219, 151, 140])
.mutates(true)
.args(vec![])
.docs(vec!["Flips the internal boolean.",])
.returns(ink_abi::ReturnTypeSpec::new(None))
.done(),
ink_abi::MessageSpec::new("get")
.selector(4266279973u32)
.selector([254, 74, 68, 37])
.mutates(false)
.args(vec![])
.docs(vec!["Returns the internal boolean.",])
......@@ -345,7 +345,7 @@ fn contract_compiles() {
/// Returns the internal boolean.
pub fn get(self,) -> ink_core::env::CallBuilder<Env, ink_core::env::ReturnType<bool>> {
ink_core::env::CallBuilder::eval(
self.contract.account_id.clone(), 4266279973u32
self.contract.account_id.clone(), [254, 74, 68, 37]
)
}
}
......@@ -354,7 +354,7 @@ fn contract_compiles() {
/// Flips the internal boolean.
pub fn flip(self,) -> ink_core::env::CallBuilder<Env, ()> {
ink_core::env::CallBuilder::<Env, ()>::invoke(
self.contract.account_id.clone(), 970692492u32)
self.contract.account_id.clone(), [57, 219, 151, 140])
}
}
}
......
......@@ -97,11 +97,11 @@ fn contract_compiles() {
ink_model::messages! {
/// Increments the internal counter.
257544423 => Inc(by: u32);
[15, 89, 208, 231] => Inc(by: u32);
/// Returns the internal counter.
4266279973 => Get() -> u32;
[254, 74, 68, 37] => Get() -> u32;
/// Returns `true` if `x` is greater than the internal value.
363906316 => Compare(x: u32) -> bool;
[21, 176, 197, 12] => Compare(x: u32) -> bool;
}
}
......@@ -231,7 +231,7 @@ fn contract_compiles() {
ink_abi::ContractSpec::new("Incrementer")
.constructors(vec![
ink_abi::ConstructorSpec::new("on_deploy")
.selector(0)
.selector([0u8; 4])
.args(vec![
ink_abi::MessageParamSpec::new("init_value")
.of_type(
......@@ -248,7 +248,7 @@ fn contract_compiles() {
])
.messages(vec![
ink_abi::MessageSpec::new("inc")
.selector(257544423u32)
.selector([15, 89, 208, 231])
.mutates(true)
.args(vec![
ink_abi::MessageParamSpec::new("by")
......@@ -267,7 +267,7 @@ fn contract_compiles() {
)
.done(),
ink_abi::MessageSpec::new("get")
.selector(4266279973u32)
.selector([254, 74, 68, 37])
.mutates(false)
.args(vec![])
.docs(vec![
......@@ -282,7 +282,7 @@ fn contract_compiles() {
)
.done(),
ink_abi::MessageSpec::new("compare")
.selector(363906316u32)
.selector([21, 176, 197, 12])
.mutates(false)
.args(vec![
ink_abi::MessageParamSpec::new("x")
......@@ -432,7 +432,7 @@ fn contract_compiles() {
/// Returns the internal counter.
pub fn get(self,) -> ink_core::env::CallBuilder<Env, ink_core::env::ReturnType<u32>> {
ink_core::env::CallBuilder::eval(
self.contract.account_id.clone(), 4266279973u32
self.contract.account_id.clone(), [254, 74, 68, 37]
)
}
......@@ -443,7 +443,7 @@ fn contract_compiles() {
.contract
.account_id
.clone(),
363906316u32
[21, 176, 197, 12]
).push_arg(&x)
}
}
......@@ -452,7 +452,7 @@ fn contract_compiles() {
/// Increments the internal counter.
pub fn inc(self, by: u32,) -> ink_core::env::CallBuilder<Env, ()> {
ink_core::env::CallBuilder::<Env, ()>::invoke(
self.contract.account_id.clone(), 257544423u32
self.contract.account_id.clone(), [15, 89, 208, 231]
).push_arg(&by)
}
}
......
......@@ -164,7 +164,7 @@ fn contract_compiles() {
ink_abi::ContractSpec::new("Noop")
.constructors(vec![
ink_abi::ConstructorSpec::new("on_deploy")
.selector(0)
.selector([0u8; 4])
.args(vec![])
.docs(vec![
"Does nothing to initialize itself.",
......
......@@ -40,44 +40,122 @@ pub trait Message {
/// Defines messages for contracts with less boilerplate code.
#[macro_export]
macro_rules! messages {
(
$( #[$msg_meta:meta] )*
$msg_id:literal => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) -> $ret_ty:ty ;
$($rest:tt)*
) => {
$( #[$msg_meta] )*
#[derive(Copy, Clone)]
pub(crate) struct $msg_name;
impl $crate::Message for $msg_name {
type Input = ($($param_ty),*);
type Output = $ret_ty;
const ID: $crate::MessageHandlerSelector = $crate::MessageHandlerSelector::new($msg_id);
const NAME: &'static str = stringify!($msg_name);
}
messages!($($rest)*);
};
(
$( #[$msg_meta:meta] )*
$msg_id:literal => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) ;
$($rest:tt)*
) => {
messages!(
$( #[$msg_meta] )*
$msg_id => $msg_name (
$( $param_name : $param_ty ),*
) -> ();
$($rest)*
);
};
() => {};
// When parsing the macro with a doc comment Rust cannot decide
// unambiguously if it should match the doc comment to `meta` or
// `expr`. Hence we introduce a `@delimiter` between the two of them.
// The first two rules match for macro invocations containing this
// delimiter, the subsequent ones add the delimiter if it's missing.
// With delimiter and a metavariable (typically a doc comment).
(
$( #[$msg_meta:meta] )+
@delimiter $msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) -> $ret_ty:ty ;
$($rest:tt)*
) => {
$( #[$msg_meta] )+
#[derive(Copy, Clone)]
pub(crate) struct $msg_name;
impl $crate::Message for $msg_name {
type Input = ($($param_ty),*);
type Output = $ret_ty;
const ID: $crate::MessageHandlerSelector = $crate::MessageHandlerSelector::new($msg_id);
const NAME: &'static str = stringify!($msg_name);
}
messages!($($rest)*);
};
// With delimiter and no metavariable.
(
@delimiter $msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) -> $ret_ty:ty ;
$($rest:tt)*
) => {
#[derive(Copy, Clone)]
pub(crate) struct $msg_name;
impl $crate::Message for $msg_name {
type Input = ($($param_ty),*);
type Output = $ret_ty;
const ID: $crate::MessageHandlerSelector = $crate::MessageHandlerSelector::new($msg_id);
const NAME: &'static str = stringify!($msg_name);
}
messages!($($rest)*);
};
// Without delimiter, with a metavariable and a return type.
(
$( #[$msg_meta:meta] )+
$msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) -> $ret_ty:ty ;
$($rest:tt)*
) => {
messages!(
$( #[$msg_meta] )+
@delimiter $msg_id => $msg_name (
$( $param_name : $param_ty ),*
) -> $ret_ty;
$($rest)*
);
};
// Without delimiter, with a metavariable but no return type.
(
$( #[$msg_meta:meta] )+
$msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) ;
$($rest:tt)*
) => {
messages!(
$( #[$msg_meta] )+
@delimiter $msg_id => $msg_name (
$( $param_name : $param_ty ),*
) -> ();
$($rest)*
);
};
// Without delimiter, no metavariable but a return type.
(
$msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) -> $ret_ty:ty ;
$($rest:tt)*
) => {
messages!(
@delimiter $msg_id => $msg_name (
$( $param_name : $param_ty ),*
) -> $ret_ty;
$($rest)*
);
};
// Without delimiter, no metavariable and no return type.
(
$msg_id:expr => $msg_name:ident (
$( $param_name:ident : $param_ty:ty ),*
) ;
$($rest:tt)*
) => {
messages!(
@delimiter $msg_id => $msg_name (
$( $param_name : $param_ty ),*
) -> ();
$($rest)*
);
};
() => {};
}
......@@ -103,11 +103,11 @@ impl CallData {
/// A hash to identify a called function.