New metadata_name attribute

[xgreenx]

Added support of a new attribute metadata_name for ink! messages, constructors, and impl sections.
This attribute allows specifying which name to use during the generation of metadata(ABI) for messages, and constructors.

This attribute can be used for cases if you want to use another naming of methods or traits on rust level, but you still want to implement some trait(generate the same ABI).

Example:
You have a trait of some standard(let's call it Erc20).

#[ink_lang::trait_definition]
pub trait Erc20 {
    /// Returns the total supply.
    #[ink(message)]
    fn total_supply(&self) -> Balance;

    /// Transfers `amount` from caller to `to`.
    #[ink(message)]
    fn transfer(&mut self, to: AccountId, amount: Balance);
}

impl Erc20 for Erc20Struct {
    #[ink(message)]
    fn total_supply(&self) -> Balance { 0 }

    #[ink(message)]
    fn transfer(&mut self, to: AccountId, amount: Balance) {}
}

But you already have methods total_supply and transfer for another implementation. During compilation, it will cause an error where you need to specify which implementation you want to use(for example you need to write Erc20::total_supply(self)).

In this case combination of selector and metadata_name allow using another trait and another naming of function but still generate the same ABI.

#[ink_lang::trait_definition]
pub trait Erc20External {
    /// Returns the total supply.
    #[ink(message)]
    fn total_supply_external(&self) -> Balance;

    /// Transfers `amount` from caller to `to`.
    #[ink(message)]
    fn transfer_external(&mut self, to: AccountId, amount: Balance);
}

#[ink(metadata_name = "Erc20")]
impl Erc20External for Erc20Struct {
    #[ink(message)]
    // 0x3ef71755 - is identifier for "Erc20::total_supply"
    #[ink(selector = "0x3ef71755", metadata_name = "total_supply")]
    fn total_supply_external(&self) -> Balance { 0 }

    #[ink(message)]
    // 0x46607e68 - is identifier for "Erc20::transfer"
    #[ink(selector = "0x46607e68", metadata_name = "transfer")]
    fn transfer_external(&mut self, to: AccountId, amount: Balance) {}
}

Generated ABI for first trait and for second will be the same

"messages": [
      {
        "args": [],
        "docs": [],
        "mutates": false,
        "name": [
          "Erc20",
          "total_supply"
        ],
        "payable": false,
        "returnType": {
          "displayName": [
            "Balance"
          ],
          "type": 1
        },
        "selector": "0x3ef71755"
      },
      {
        "args": [
          {
            "name": "to",
            "type": {
              "displayName": [
                "AccountId"
              ],
              "type": 2
            }
          },
          {
            "name": "amount",
            "type": {
              "displayName": [
                "Balance"
              ],
              "type": 1
            }
          }
        ],
        "docs": [],
        "mutates": true,
        "name": [
          "Erc20",
          "transfer"
        ],
        "payable": false,
        "returnType": null,
        "selector": "0x46607e68"
      }
    ]

[xgreenx]

Waterfall CI doesn't see the branch=(

[Robbepop]

Note that with the redesigned trait system it will no longer be possible to change selectors of trait implementations but instead trait definitions will allow to customize selectors of trait methods. Also selectors between trait definitions with equally named methods only conflict if the traits are named the same and have the same syntactical structure.

[xgreenx]

Note that with the redesigned trait system it will no longer be possible to change selectors of trait implementations but instead trait definitions will allow to customize selectors of trait methods.

Where I can read about the redesigned trait system?=)

Also selectors between trait definitions with equally named methods only conflict if the traits are named the same and have the same syntactical structure.

Yes. ink! doesn't allow injecting code of one contract to another. And the solution is to implement some logic on the rust level and reuse this implementation in the public trait. In this case methods in internal implementation will have the same signature in public trait.

[cmichi]

Waterfall CI doesn't see the branch=(

Fixed! I added support for external contributions to the Waterfall CI.

[cmichi]

@xgreenx Thanks for the PR, the main issue I have with it is that I think this functionality:

This attribute can be used for cases if you want to use another naming of methods or traits on rust level, but you still want to implement some trait(generate the same ABI).

is already covered by the [ink(selector = "…")] attribute.

So for flipper you could have this implementation:

impl Flipper {
    #[ink(constructor, selector = "0x00000001")]
    pub fn new(init_value: bool) -> Self {
        Self { value: init_value }
    }

    #[ink(constructor, selector = "0x00000002")]
    pub fn default() -> Self {
        Self::new(Default::default())
    }

    #[ink(message, selector = "0x00000003")]
    pub fn flip(&mut self) {
        self.value = !self.value;
    }

    #[ink(message, selector = "0x00000004")]
    pub fn get(&self) -> bool {
        self.value
    }
}

and then modify the generated ABI. For example, you could modify the target/ink/flipper.contract to change the constructors.name to new_flipper and default_flipper, as well as message.name to flip_value and get_value. The UI's would then still work properly ‒ they would display the names from the ABI, but use the selector when invoking them.

So those name's are basically "just" a UX feature, there is a dispatcher in ink! which selects the constructor/message based on the selector field, the name does play no role there.

What do you think?

[xgreenx]

@xgreenx Thanks for the PR, the main issue I have with it is that I think this functionality:

This attribute can be used for cases if you want to use another naming of methods or traits on rust level, but you still want to implement some trait(generate the same ABI).

is already covered by the [ink(selector = "…")] attribute.

So for flipper you could have this implementation:

impl Flipper {
    #[ink(constructor, selector = "0x00000001")]
    pub fn new(init_value: bool) -> Self {
        Self { value: init_value }
    }

    #[ink(constructor, selector = "0x00000002")]
    pub fn default() -> Self {
        Self::new(Default::default())
    }

    #[ink(message, selector = "0x00000003")]
    pub fn flip(&mut self) {
        self.value = !self.value;
    }

    #[ink(message, selector = "0x00000004")]
    pub fn get(&self) -> bool {
        self.value
    }
}

and then modify the generated ABI. For example, you could modify the target/ink/flipper.contract to change the constructors.name to new_flipper and default_flipper, as well as message.name to flip_value and get_value. The UI's would then still work properly ‒ they would display the names from the ABI, but use the selector when invoking them.

So those name's are basically "just" a UX feature, there is a dispatcher in ink! which selects the constructor/message based on the selector field, the name does play no role there.

What do you think?

I agree with you, that it can be changed manually and it is only UI sugar. But we can add this feature to change it automatically=)

For example, we are using this feature in our library. Our macro will use this feature to create two traits, the first on rust level, the second on ink! level.

For example, user defines this trait:

#[brush::trait_definition]
pub trait Erc20 {
    /// Returns the total supply.
    #[ink(message)]
    fn total_supply(&self) -> Balance;

    /// Transfers `amount` from caller to `to`.
    #[ink(message)]
    fn transfer(&mut self, to: AccountId, amount: Balance) {
        // Some logic here
    }
}

Macro will generate two traits:

// This trait on rust level
pub trait Erc20 {
    fn total_supply(&self) -> Balance;

    fn transfer(&mut self, to: AccountId, amount: Balance) {
        // Some logic here
    }
}

// This trait on ink! level
#[ink_lang::trait_definition]
pub trait Erc20External {
    /// Returns the total supply.
    #[ink(message)]
    fn total_supply_external(&self) -> Balance;

    /// Transfers `amount` from caller to `to`.
    #[ink(message)]
    fn transfer_external(&mut self, to: AccountId, amount: Balance);
}

After that, another brush::contract macro will create two implementations for the user:

// This implementation on rust level
impl Erc20 for Erc20Struct {
    fn total_supply(&self) -> Balance { 0 }
}


// This implementation on ink! level
#[ink(metadata_name = "Erc20")]
impl Erc20External for Erc20Struct {
    #[ink(message)]
    // 0x3ef71755 - is identifier for "Erc20::total_supply"
    #[ink(selector = "0x3ef71755", metadata_name = "total_supply")]
    fn total_supply_external(&self) -> Balance { 
        Erc20::total_supply(self)
    }

    #[ink(message)]
    // 0x46607e68 - is identifier for "Erc20::transfer"
    #[ink(selector = "0x46607e68", metadata_name = "transfer")]
    fn transfer_external(&mut self, to: AccountId, amount: Balance) { 
        Erc20:: transfer(self, to, amount)
    }
}

And the user doesn't know anything about how it works inside and it allows us to generate the same ABI, but adds additional functionality.

[cmichi]

@xgreenx What I don't understand in the example which you used is why the Erc20External trait methods need to have the …_external suffix? Why couldn't they just be called transfer() and total_supply. Same goes for the Erc20External trait, which with namespacing could e.g. be placed in a module external_traits::Erc20.

Your code looks fine, the reason why we're so cautious about merging this issue is because it ties the metadata much closer into the language. Right now, ink! doesn't have any attributes that expose control about mutating the metadata inside the language. We are currently more on the cautious side regarding this and would rather like to help you solve your issue in a different manner.

[xgreenx]

I will try to describe why we need two traits and ..._external suffix.

At the moment #[ink_lang::trait_definition] has a lot of restriction. The user can't declare methods with default implementation, internal methods, have super traits, define aliases. But the user can do it in rust's traits. But usage of rust's traits with ink!'s traits requires duplicating of logic. And also it requires the usage of another name for rust trait or for ink! trait.

// This trait on rust level
pub trait Erc20Internal/Erc20 {
    fn total_supply(&self) -> Balance { 0 }

    fn transfer(&mut self, to: AccountId, amount: Balance) {
        // Some logic here
    }
}

// This trait on ink! level
#[ink_lang::trait_definition]
pub trait Erc20/Erc20External {
    /// Returns the total supply.
    #[ink(message)]
    fn total_supply(&self) -> Balance;

    /// Transfers `amount` from caller to `to`.
    #[ink(message)]
    fn transfer(&mut self, to: AccountId, amount: Balance);
}

And later it will require us to write a boilerplate code to call methods from rust trait in ink! trait(But we need ink! trait here ONLY to generate metadata and create external dispatchers which will call implementation from internal trait).

// This implementation on rust level
impl Erc20Internal for Erc20Struct {
    // we override this method
    fn total_supply(&self) -> Balance { 1 }
}

// This implementation on ink! level
impl Erc20 for Erc20Struct {
    #[ink(message)]
    fn total_supply(&self) -> Balance {
        Erc20Internal::total_supply(self)
    }

    #[ink(message)]
    fn transfer(&mut self, to: AccountId, amount: Balance) { 
        Erc20Internal::transfer(self, to, amount)
    }
}

...

impl Erc20Struct {
    #[ink(message)]
    fn temp(&mut self) -> Balance  { 
        // And here the user must specify which method he want to use
        // But Erc20 trait only is used for metadata and dispatches so it is a helper trait.
        Erc20Internal::total_supply(self)
    }
}

The usage of the same naming for methods in both traits will require the user to explicitly specify the trait during calls of methods.

We want to simplify the usage of traits with our library. The user will be able to operate only with one trait Erc20 and he doesn't need to write a boilerplate code, our macro will do it instead of him. So the user can use trait on rust level and the macro will generate a helper trait and will generate an implementation for this helper trait.

You can check how it looks like here. The developer only must specify during trait declaration which methods must be public(or another ink! stuff like payable and etc).

The combination of metadata_name and selector allows us to hide a helper Erc20External trait and methods with ..._external suffix(it helps the user of trait to doesn't specify explicit calls) under the hood.

[cmichi]

@xgreenx Thanks for the explanation. So we decided that we currently don't want to include a new attribute which can influence the metadata directly. The reasoning is:

• We currently don't want to tie the metadata too much into the language itself, but rather have a clear separation between UI and contract logic. At the moment there is no ink! attribute which can manipulate the metadata itself and enabling this would set a precedent for similar features.
• Introducing a new attribute is expensive, it needs to be maintained for a possibly very long time and every feature/bug going forward needs to possibly take it into account.
• ink! is also a user interface and we want to reduce the possibility of footguns. There is some concern that this could be a source of confusion when people mistakenly choose this attribute instead of selectors, in the mistaken assumption that it influences the dispatching.
• It seems that the motivation for your use-case stems mostly from having a workaround for the fact that our current trait support has a number of shortcomings. We are very much aware of this and are working on a redesign.

So overall we think these implications outweigh the benefits. Sorry for the work you've put into the PR!

Until the trait support is improved: are there other solutions to your problem which you are aware of? I still think that it should be possible to achieve this naming through namespacing.

[xgreenx]

@cmichi Thanks for your response!=)

So we decided that we currently don't want to include a new attribute which can influence the metadata directly

Pretty sad=( We don't want to use/support our custom version of ink! =D

We currently don't want to tie the metadata too much into the language itself, but rather have a clear separation between UI and contract logic. At the moment there is no ink! attribute which can manipulate the metadata itself and enabling this would set a precedent for similar features.

It turns out pretty unfair that we don't have possibilities to affect the metadata. The metadata generation process affects the ink! very strong. It forces to create a lot of restrictions like:

• No default methods in traits(And other features from rust traits)
• Non-external events
• No code injection of one contract into another

I'm sure, that answering the question "How to aggregate metadata from parent and child contracts?" will resolve the restrictions above.

Introducing a new attribute is expensive, it needs to be maintained for a possibly very long time and every feature/bug going forward needs to possibly take it into account.

I agree with you, that it can take a part of the resources in the future. But the idea of this attribute is simple than other attributes, so maybe it has a chance to be alive=)

ink! is also a user interface and we want to reduce the possibility of footguns. There is some concern that this could be a source of confusion when people mistakenly choose this attribute instead of selectors, in the mistaken assumption that it influences the dispatching.

Yes, I'm afraid of that too. Because it is necessary to acquaint the user with the concept of metadata. It will be cool to move this attribute under "For experts" sections or something like this=)

It seems that the motivation for your use-case stems mostly from having a workaround for the fact that our current trait support has a number of shortcomings. We are very much aware of this and are working on a redesign.

Yes, at the moment it is used as a workaround. This attribute can be removed after redesigned trait implementation. @Robbepop mentioned above that you also want to remove the selector attribute with a new version of traits. And I agree with it too, because the selector attribute seems like a hack/workaround for me too=)

Until the trait support is improved: are there other solutions to your problem which you are aware of? I still think that it should be possible to achieve this naming through namespacing.

No, we tried to find another solution, but seems it is the best. The original trait defined by the user is on rust level, and the user can use it everywhere across the project like simple rust trait and it will fit well. Additional trait generated by macro only is used for metadata generation(and dispatchers). The selector attribute allows us to generate the right dispatchers and methods in metadata, the metadata_name attribute allows using the right naming of methods in metadata.

We can't use the same naming for the original trait and the same naming for the trait to generate metadata. To generate the right metadata we MUST use the original naming. It means that we need to change the naming in trait on rust level, but it will break an illusion that the user is working with a simple rust trait because he must use another trait and another naming of methods(or explicitly use the right trait).

BTW, Where I can read about redesigned traits? We want to help with the implementation of it, but we need to know what you want to do and your thought on how it can be implemented.

[cmichi]

I'm reluctantly closing this PR after some additional private discussion. The reasons are outlined in my comment above. tl;dr is that we intend to solve the root issue (proper trait support) which will make this PR obsolete.

In general we could think about renaming name in the metadata to label. This would yield less confusion regarding it's function, since it's not used as an identifier at all and only has display purposes.

Just as a sidenote: The args (i.e. the parameters for messages) in the metadata also have a name, which is only used for UI purposes. It's value doesn't have any influence on the contract execution.