common: wasmToSchema: Base custom section resolution on module version

[bisgardo]

Purpose

Ensure that getEmbeddedSchema (on ConcordiumGRPCClient) is able to resolve legacy schemas (i.e. schemas without embedded versions).

Also make parsing of contract schema more precise based by using the module version information that is now exposed by the gRPC client. This in turn also documents the relationship between module and schema versions.

Changes

The module version is now used to determine what custom sections to look for. Added comments explain the expected results.

The extraction code is migrated from the dapp-library @concordium/react-components. Specifically, the contents of function getEmbeddedModuleSchema was replaced with the contents of findSchema and its helper function findCustomSections was added and adjusted to use the SDK types.

Finally, getEmbeddedModuleSchema was made non-async and wasmToSchema removed because it was nothing but a more limited version of getEmbeddedModuleSchema.

Addresses Concordium/concordium-dapp-libraries#66.

[bisgardo]

@orhoj @shjortConcordium @rasmus-kirk @abizjak Do we want this? If so I'll clean it up and add some tests.

I think it would be useful and simplify https://github.com/Concordium/concordium-dapp-libraries/pull/46/files.

[soerenbf]

@bisgardo Stumbled upon this PR with no reviewers and no replies. Is this still relevant? I guess if it's a general purpose thing that you've needed to implement in dapp-libraries, it might as well exist here instead?

I'm sorry this has been dangling for so long. If you deem it relevant still, I'll be happy to take a look if you get it up to speed with the main branch.

Otherwise please close it 😄

[bisgardo]

@bisgardo Stumbled upon this PR with no reviewers and no replies. Is this still relevant? I guess if it's a general purpose thing that you've needed to implement in dapp-libraries, it might as well exist here instead?

I'm sorry this has been dangling for so long. If you deem it relevant still, I'll be happy to take a look if you get it up to speed with the main branch.

Otherwise please close it 😄

@soerenbf Thanks for poking at this. I still think it's relevant :) The reason I didn't add reviewers was that I didn't consider it ready for review and I didn't want to finish it up before it was deemed worthy of doing.

I merged main into the PR branch. A couple of things have changed in the meantime and the feature seems more widely used and there seems to be some duplication that I'll try and address as part of it.

[soerenbf]

As these are breaking changes, I don't think these should be just merged to main. As I see it, we have 2 options: either change these to not be breaking (if possible), or follow the process outlined below.

The way we usually handle breaking changes is:

• Make a new release branch, i.e. release/web-sdk/8
• Target this branch for PR's implementing breaking changes
• Merge this when we're ready.

If you need to use these changes, I would suggest publishing them to npm under either the alpha or beta tag (we usually use alpha for internal preliminary release versions. If you intend to publish the dapp-libraries based on this, I would use beta).

[bisgardo]

As these are breaking changes, I don't think these should be just merged to main. As I see it, we have 2 options: either change these to not be breaking (if possible), or follow the process outlined below.

The way we usually handle breaking changes is:

* Make a new release branch, i.e. `release/web-sdk/8`

* Target this branch for PR's implementing breaking changes

* Merge this when we're ready.

If you need to use these changes, I would suggest publishing them to npm under either the alpha or beta tag (we usually use alpha for internal preliminary release versions. If you intend to publish the dapp-libraries based on this, I would use beta).

I'm in favor of keeping it a breaking change because having ConcordiumGRPCClient.getEmbeddedSchema only support new kinds of schemas (and actually throw an error if it's an old one) doesn't make a whole lot of sense. I would actually go a step further and remove it entirely because it's trivially expressed in terms of getModuleSource and getEmbeddedModuleSchema.

I'll let you decide what should happen. Just let me know and I'll make the relevant changes.

Btw. I'm in no rush to use this in dapp-libs. But I'm happy to prepare a PR to be merged when the time is right.

[soerenbf]

I'm in favor of keeping it a breaking change because having ConcordiumGRPCClient.getEmbeddedSchema only support new kinds of schemas (and actually throw an error if it's an old one) doesn't make a whole lot of sense. I would actually go a step further and remove it entirely because it's trivially expressed in terms of getModuleSource and getEmbeddedModuleSchema.

I'll let you decide what should happen. Just let me know and I'll make the relevant changes.

I think I would prefer to just keep the method on the GRPC client, as I think it will be easier for user to migrate.

Please create a release branch for the next major version of the SDK and set it as the target branch for this. Then we'll release the changes with the next protocol update.

[soerenbf]

Assuming this is not merged to main but to a release branch for the next major version 😉

[bisgardo]

Assuming this is not merged to main but to a release branch for the next major version 😉

Created the release branch and set it as merge target. I also did the change with undefined instead of null. Can you please give that a quick look before I merge?

[soerenbf]

Thanks! looks good 👍