Update check for contract instantiation

[statictype]

Replaced the isWasm check on contract instantiation with isU8a in order to allow experimentation with other targets.
For more info see use-ink/contracts-ui#507

[athei]

Can we get this merged? I can't use contracts UI with RISC-V contracts otherwise.

[statictype]

this PR seems pretty innocent, can we get it merged @jacogr? 🙏🏻

[jacogr]

I'm not convinced this is "innocent enough".

Basically at this point we can throw any data into the mix as expect it to work, i.e. some JSON, a random piece of text, etc. Basically we end up with garbage that cannot be executed in any interpreter.

If there are additional formats that should be supported, the better approach is to check the magic flag for that specific format in addition to the isWasm check.

EDIT: The is<NewFormat>() check function can just be added locally to the specific file - it would make sense to also add it to common (and when that is available in a release, the local version inside the file can be removed)

[athei]

Basically at this point we can throw any data into the mix as expect it to work, i.e. some JSON, a random piece of text, etc. Basically we end up with garbage that cannot be executed in any interpreter.

So what? pallet_contracts has to make absolutely sure that it can handle any input securely anyways. It will fail gracefully and report an error back in case of the data not being processable.

The verification also does only check the first bytes for the "wasm magic". There are still infinite many ways to send "garbage that cannot be executed in any interpreter" after those bytes.

I cannot see the value that this very shallow check provides. Users have to do error handling anyways since pallet_contracts can reject a module for all sorts of reasons. On the other hand having this check makes our lives more difficult for the stated reasons.

[jacogr]

It is what it is :)

[jacogr]

As explained in #5699 (review):

1. Add a is<FormatName>() check function (e.g. assuming the new format is XXX we add isXXX - used in the sample for the next points) to check the magic bytes for the specific additional type
2. Adjust the isWasm checks to do isWasm(...) || isXXX(...)
3. (Optional, but appreciated) Also add the isXXX to the common repo (will eventually replace the local version when that stuff is released)

That should more-or-less cover the same functionality. (Not 100% sure on the source.wasm usage, implementers would know better on that)

[statictype]

Added a similar check for RISC-V @jacogr
If it's ok i can add it to the common repo and import it from there in a follow-up PR

[jacogr]

Thank you, looks like it fits the bill.

Two small comment added just to be slightly more pedantic/explicit on return types/guards. (It always helps when we come back years from now)

[jacogr]

Slightly weird that a different format may come through in source.wasm, but either way, all ok.

[statictype]

the field in the contract metadata will most likely be renamed once the support for RISC-V is released. not something we should address now imo.

Stale.

[polkadot-js-bot]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.