Rename metadata.json to {contract_name}.json

[SkymanOne]

Closes #950

[cmichi]

Changelog entry plz.

[kvinwang]

Oh, this is a breaking change. We have tools reading the contract name from the metadata.json.

[ascjones]

@kvinwang makes a good point. We should think carefully before introducing breaking changes.

#950 (comment)

[ascjones]

On balance, I think this is reasonable to introduce as a breaking change into the 2.0.0 release. #950 (comment)

The main thing is for consistency with other artefact file names.

Downstream tools which depend on the static name will instead have to handle any *.json file as a contract metadata file. A breaking change, yes, but relatively easy to fix.

[kvinwang]

Downstream tools which depend on the static name will instead have to handle any *.json file as a contract metadata file. A breaking change, yes, but relatively easy to fix.

Breaking is always acceptable to me.

However, as I said in #950, the real problem here is not the breaking itself. We can read the metadata from the target/ink/metadata.json in the past in our build script. That's a well-defined robust path. Now, If we change to handle *.json instead, the code becomes fragile, there should be no guarantee that there is only one *.json file in the target/ink directory, and it is a metadata file. In the future, any other trivial changes that put extra json in the ink dir would accidentally break the tools.

[ascjones]

here should be no guarantee that there is only one *.json file in the target/ink directory, and it is a metadata file.

The guarantee now is that the ink/mycontract.wasm artifact always exists, and unless it was built with --build-artifacts code-only, there will exist mycontract.json (metadata only) and mycontract.contract (metadata + code).

[kvinwang]

there will exist mycontract.json

We can guarantee there will exist a mycontract.json, but can not guarantee there isn't a foo.json alongside. The build script can not easily know which one is the metadata file unless we explicitly pass the contract name to it.
In order to get the contract name, we read it from the metadata.json nowadays, but in the future, we might need to read it from the Cargo.toml in which the contract name isn't well-defined either and it isn't script friendly and isn't always available (when the ink/ was moved to some dist/ dir for example).

[ascjones]

The way to determine the contract name is to take the first part of the .wasm code artefact. So erc20.wasm -> erc20.json. No matter if there is a foo.json alongside or not.

The edge case with this is that if you rename your contract, then there will be multiple .wasm files in the ink directory. In this case you could just choose the most recent one or look at Cargo.toml to determine the package name.

Cargo.toml in which the contract name isn't well-defined either and it isn't script friendly and isn't always available

We have recently changed it so the contract name is always the package name #929. But anyway it should not be required in this case because of the above heuristic (all contract artifacts have the same root name, different extensions)

[kvinwang]

The way to determine the contract name is to take the first part of the .wasm code artefact. So erc20.wasm -> erc20.json. No matter if there is a foo.json alongside or not.

I don't think this is a good design. This is like asking the tools to guess what's my name. A metadata.json definitely solves the problem in the simplest and most robust way.

[ascjones]

The tool doesn't need to guess what the name is, the name is now the name of the json file itself. The target/ink directory generated by cargo-contract only contains json files which are contract metadata, so that is a good enough guarantee.

[kvinwang]

The tool doesn't need to guess what the name is, the name is now the name of the json file itself. The target/ink directory generated by cargo-contract only contains json files which are contract metadata, so that is a good enough guarantee.

Given there is ink/ dir with guaranteed a mycontract.contract, a mycontract.wasm and a mycontract.json inside.
But without guarantee, there is not any foo.json, bar.wasm inside.
How can I know which file is the contract metadata file or contains the contract name?
The first .json file found?

[ascjones]

But without guarantee, there is not any foo.json, bar.wasm inside.

I'm not exactly sure what you mean here, but if you are asking how to identify the file when it is not inside target/ink, then there are no guarantees anymore because cargo-contract is only in control of artifacts in that directory. That was also true when it was called metadata.json, because as well as being copied it could also be renamed. So you are in full control of the name, so you can also cp target/ink/mycontract.json metadata.json if you wish when copying it to your assets directory.

[SkymanOne]

@kvinwang I reckon the obvious benefit of matching the metadata file with the contract's name is that sometimes you only have metadata file and meaningful name helps to understand which contract it belongs to.

We already encountered an instance of such problem: we have a workshop https://github.com/paritytech/ink-workshop where game.contract is deployed by someone else and the player had to upload metadata in polkadot.js ui to be able to read the game's contracts.

[kvinwang]

I'm not exactly sure what you mean here, but if you are asking how to identify the file when it is not inside target/ink, then there are no guarantees anymore because cargo-contract is only in control of artifacts in that directory.

I mean there is no guarantee that cargo-contract 2.1 or any future version never generates a foo.json in the ink/ dir there. If cargo-contract generates an additional foo.json, it would not be considered a breaking change. But it would totally break downstream build tools.

Let me show a piece of pseudocode code here.

Suppose we have a downstream build script like this:

cargo contract build --release
CONTRACT_NAME=`jq -r .contract.name target/ink/metadata.json`

Now, we might need to be changed to:

cargo contract build --release
CONTRACT_NAME=`cat target/ink/*.json | jq -r .contract.name`

Do you think the command cat target/ink/*.json would never be broken by any future cargo-contract updates? No, obviously, cargo-contract can not give such a weird guarantee. And anyone looking at cat target/ink/*.json | jq -r .contract.name would be confused and consider it a bad code.

I think a better solution might be adding an optional argument to assign the metadata file name by the caller. So the script would be like:

cargo contract build --release --metadata target/ink/metadata.json
CONTRACT_NAME=`jq -r .contract.name target/ink/metadata.json`

[kvinwang]

@kvinwang I reckon the obvious benefit of matching the metadata file with the contract's name is that sometimes you only have metadata file and meaningful name helps to understand which contract it belongs to.

We already encountered an instance of such problem: we have a workshop https://github.com/paritytech/ink-workshop where game.contract is deployed by someone else and the player had to upload metadata in polkadot.js ui to be able to read the game's contracts.

Downstream tools can easily rename the metadata.json to mycontract.json, but not vice versa.

I think a better solution might be adding an option to assign the metadata file name by the caller. Like:
cargo contract build --release --metadata target/ink/metadata.json

[SkymanOne]

@kvinwang I reckon the obvious benefit of matching the metadata file with the contract's name is that sometimes you only have metadata file and meaningful name helps to understand which contract it belongs to.
We already encountered an instance of such problem: we have a workshop https://github.com/paritytech/ink-workshop where game.contract is deployed by someone else and the player had to upload metadata in polkadot.js ui to be able to read the game's contracts.

Downstream tools can easily rename the metadata.json to mycontract.json, but not vice versa.

I think a better solution might be adding an option to assign the metadata file name by the caller. Like: cargo contract build --release --metadata target/ink/metadata.json

Why? :)

[kvinwang]

Why? :)

I explained it in this comment above.

[ascjones]

cargo contract build --release
CONTRACT_NAME=`jq -r .contract.name target/ink/metadata.json`

To make such a script more robust and agnostic of the file path of the metadata, you can read the path of the metadata from the build result e.g.:

cargo contract build --output-json | jq -r .metadata_result.dest_metadata

[kvinwang]

cargo contract build --output-json | jq -r .metadata_result.dest_metadata

Cool! Didn't know there is so much useful info in the JSON output.

[HCastano]

A few follow-ups here:

There's going to be a bunch of tutorials and examples that need to be updated across ink-docs, substrate-docs, and our inline code docs (ink!, cargo-contract, pallet-contracts, node, etc.). @SkymanOne can you take care of flagging these down and fixing them?

We should also update our docs on cargo-contract to show some common scripting patterns, like the one shown by Andrew. Since it looks like you're doing some scripting already, maybe you can start this @kvinwang. If not maybe Andrew or I can do it

[kvinwang]

Since it looks like you're doing some scripting already, maybe you can start this @kvinwang.

In fact, the only used subcommand in our script is cargo contract build .... So, sorry, I am afraid I am unable to write much there.