Rename name in metadata to label

[cmichi]

In our metadata we currently have a name Array field for constructors, messages and arguments.

There are some issues with it:

• Contrary to the term it is not actually any kind of identifier. This field does not have any impact on contract execution, it is used purely for display purposes in the UI's.
• We have a field selector which actually serves as the identifier. There is some concern that users will mistakenly assume that the name has some implication for the contract execution. Hence renaming it to label would clarify this.
• For traits, ink! generates a field in the form of "name": ["BaseErc20", "new"]. The BaseErc20 hereby is a trait implemented by the contract. This exposes internal implementation details of the contract without the developer having any influence over it.
• Right now it's not clear how a UI is supposed to handle the different elements in an Array. Concatenate them? How? Having a String would also simplify that.

Recently there was the PR #859 by @xgreenx where the need for influencing this value in the metadata was also stated.

My proposal is this:

• Rename name to label for constructors, messages and arguments.
• Make it a String instead of an Array.
• For traits, default to the form BaseErc20::new as String.

I'm creating this issue to start a discussion on this. It would be great if especially @seanyoung could give his opinion.

If we decide to do this these would need to be done:

ToDo

• Rename name to label for constructors, messages and arguments.
• Make it a String instead of an Array.
• Revive New metadata_name attribute #859, but with label instead of metadata_name. I would leave the possibility of modifying the arguments labels out for now.
• Create follow-up issues for Canvas UI and polkadot-js.

[seanyoung]

• Should be there be defined upgrade path for polkadot-js/Canvas UI? Something like: try label first, else fall back to name. Or will just this change from one version to the next?
• Making this a string rather than an array is a good thing, I think.

[cmichi]

Should be there be defined upgrade path for polkadot-js/Canvas UI? Something like: try label first, else fall back to name. Or will just this change from one version to the next?

Good point and good idea! We should specify this fallback in the follow-up issues for the UI's.

[xgreenx]

I agree with everything=)
The string will be more clear for developers instead of the array. If they want, they can easily split it with ::.
label is more descriptive than name.

I can rework #859 and add label, string instead of the array. Do I need to add support for changing the name of the arguments?

Something like that:

    #[ink(message)]
    #[ink(label = "my_ balance_of")]
    fn balance_of(&self, #[ink(label = "my_owner")] owner: AccountId) -> u32 {
        self.get().owned_tokens_count.get(&owner).cloned().unwrap_or(0)
    }

Or it will be implemented later?

[cmichi]

I agree with everything=)

Good to hear :-)!

I can rework #859 and add label, string instead of the array.

That would be awesome!

Do I need to add support for changing the name of the arguments?

No, let's leave it out for now.

[xgreenx]

Hi @cmichi , we had a conversation with @Robbepop regarding #923. He helped me to resolve my issue with overlapping names of methods. Example of how it can be resolved after #665. So for me #[ink(label)] attribute is not actual anymore=)

So if you are okay with it, we can remove it from the scope of this issue. I will re-work the pull request to implement other points of this issue.

[cmichi]

So if you are okay with it, we can remove it from the scope of this issue. I will re-work the pull request to implement other points of this issue.

@xgreenx I'm not sure if I understand you right that you suggest to remove #[ink(label)] altogether from this issue? How about just removing the possibility of specifying it for impl blocks?

[Robbepop]

So if you are okay with it, we can remove it from the scope of this issue. I will re-work the pull request to implement other points of this issue.

@xgreenx I'm not sure if I understand you right that you suggest to remove #[ink(label)] altogether from this issue? How about just removing the possibility of specifying it for impl blocks?

Yesterday we had a chat and concluded that the #[ink(label)] attribute is not really needed which is the best possible outcome for all of us since no code is the best code.

[cmichi]

Yesterday we had a chat and concluded that the #[ink(label)] attribute is not really needed which is the best possible outcome for all of us since no code is the best code.

The original motivation behind this issue was that right now there is no way for a developer to control how UI's display the exposed contract functions. Even worse, a developer has no control over if internal implementation details are exposed (since the trait name is prepended to the metadata field). How is this root issue solved?

[Robbepop]

Yesterday we had a chat and concluded that the #[ink(label)] attribute is not really needed which is the best possible outcome for all of us since no code is the best code.

The original motivation behind this issue was that right now there is no way for a developer to control how UI's display the exposed contract functions. Even worse, a developer has no control over if internal implementation details are exposed (since the trait name is prepended to the metadata field). How is this root issue solved?

It was kinda decided that it is actually a feature that people cannot take full control over this and instead can rely on a common naming scheme provided by ink!. There are a lot of disadvantages to having this feature that do not weight into the advantage of it. During the chat we found an elegant solution for the problem that caused @xgreenx to require this feature. So there is no more need for this feature from their side, too. We should only ever implement new features (that we have to maintain) if there are good reasons and users.

[cmichi]

There are a lot of disadvantages to having this feature that do not weight into the advantage of it.

Could you elaborate on the disadvantages that you see?

It was kinda decided that it is actually a feature that people cannot take full control over this and instead can rely on a common naming scheme provided by ink!.

My point still stands, that we have a field for human-readable purposes, but generate the content only from an algorithm without giving the developer any control over it. Right now, our generation exposes internal implementation details by default, without giving the developer any control over if he actually wants it.

Why can't uses take full control over it? In my mind, the most basic implementation would just allow #[ink(message, label = "…"] for messages and constructors, but not for traits or impl blocks. My understanding is that the existing PR already does this and we could just removed the label from any other place, but messages and constructors.

[xgreenx]

I agree with @cmichi, that developers better control labels in some cases. I had the case with traits, and it can be resolved by namespacing. But other developers can have a case with common functions in impl block.

How about just removing the possibility of specifying it for impl blocks?

I agreed with this idea in this comment. Because the selector attribute can be applied only to constructors and messages=)

I think that ink! should provide a label attribute like the selector attribute. For me, it is easy to understand the difference between the label and selector attributes.

So I vote for label. But I will agree with any of your decisions=)

[HCastano]

Closed via #923