Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add suport for latest ink! metadata #5650

Merged
merged 11 commits into from
May 29, 2023
Merged

Add suport for latest ink! metadata #5650

merged 11 commits into from
May 29, 2023

Conversation

statictype
Copy link
Member

@statictype statictype commented May 19, 2023

Adds latest ink! metadata updates to the Abi interface and type definitions.
Related to use-ink/contracts-ui#469

Changes:

  • contract environment types
  • "default" property on messages and constructors
  • return type for constructors

Contract with modified environment
custom_env.json.zip

@statictype statictype marked this pull request as draft May 19, 2023 12:40
@statictype statictype marked this pull request as ready for review May 21, 2023 16:18
@statictype statictype requested a review from jacogr May 21, 2023 16:18
jacogr
jacogr previously requested changes May 22, 2023
Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to have version specific logic which is unsatainable the more there are. While we always convert from Vx -> V as part of the logic - this allows us to always deal with only the latest, without specific existence checks.

packages/api-contract/src/Abi/index.ts Outdated Show resolved Hide resolved
packages/api-contract/src/Abi/index.ts Outdated Show resolved Hide resolved
@statictype
Copy link
Member Author

@jacogr I removed all unnecessary checks and decoded environment types.
decoding is weird because the environment can contain both types and values so some checks are needed.
For example maxEventTopics is a value and the rest are types.
https://github.com/paritytech/ink/blob/8bb4b40d76adb28cf046f385cb5842bcd90b8787/crates/env/src/types.rs#L125

Btw, is this ok for usize?

I added ink! v4.2.0 contracts for testing and the tests still fail, not sure why.

@jacogr
Copy link
Member

jacogr commented May 24, 2023

usize should never be used. It acts differently on WASM vs native - i.e. it has a different size based on the execution environment. Using it will end up with consensus failures.

@jacogr jacogr dismissed their stale review May 24, 2023 18:25

Stale review.

packages/api-contract/src/Abi/toV4.ts Outdated Show resolved Hide resolved
packages/api-contract/src/Abi/index.ts Outdated Show resolved Hide resolved
@statictype statictype requested a review from jacogr May 25, 2023 19:14
Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@jacogr jacogr merged commit 0be50b2 into master May 29, 2023
@jacogr jacogr deleted the ae-inkmeta branch May 29, 2023 17:19
@polkadot-js-bot
Copy link

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.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants