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

Bump metadata version to 5 #2126

Merged
merged 5 commits into from
Feb 27, 2024
Merged

Bump metadata version to 5 #2126

merged 5 commits into from
Feb 27, 2024

Conversation

SkymanOne
Copy link
Contributor

@SkymanOne SkymanOne commented Feb 26, 2024

Summary

Bumps ink! metadata version to 5

Description

Since the introduction of Event 2.0 (#1827), and static buffer size in metadata (#1880), we need to increase the ABI metadata version to 5.

Note

This PR temporarily disables e2e on-chain test step in CI until the new version of cargo-contract including new ink_metadata crate is released.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 53.67%. Comparing base (835775c) to head (6af9bdd).

Files Patch % Lines
crates/metadata/src/lib.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2126   +/-   ##
=======================================
  Coverage   53.67%   53.67%           
=======================================
  Files         224      224           
  Lines        7048     7046    -2     
  Branches     3118     3117    -1     
=======================================
- Hits         3783     3782    -1     
+ Misses       3265     3264    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

What do you think about just making it a const uint instead of the enum?

crates/metadata/src/lib.rs Outdated Show resolved Hide resolved
layout,
spec,
registry,
}
}

/// Returns the metadata version used by the contract.
pub fn version(&self) -> &MetadataVersion {
pub fn version(&self) -> &u64 {
Copy link
Contributor

@peetzweg peetzweg Feb 27, 2024

Choose a reason for hiding this comment

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

Will this change result in the version being output as a number?

So {version: 5} instead of {version: '5'}?

Would prefer it as a number instead of string for easier version comparison up and down. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this changes the string into the integer. All good there

@SkymanOne SkymanOne requested a review from a team as a code owner February 27, 2024 14:46
Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the integration-tests/* contracts from this branch with cargo-contract and comparing them to ink! master:

Contract Upstream Size (kB) PR Size (kB) Diff (kB) Diff (%) Change
call-builder-return-value 9.237 9.237 0 0
call-runtime 2.061 2.061 0 0
combined-extension 2.137 2.12 -0.017 -0.795508 📉
conditional-compilation 1.49 1.49 0 0
contract-storage 7.568 7.568 0 0
contract-terminate 1.329 1.329 0 0
contract-transfer 1.689 1.689 0 0
cross-contract-calls 7.73 7.73 0 0
cross-contract-calls/other-contract 1.583 1.583 0 0
custom-allocator 7.775 7.775 0 0
custom-environment 2.146 2.146 0 0
dns 7.318 7.318 0 0
e2e-call-runtime 1.296 1.296 0 0
e2e-runtime-only-backend 1.881 1.881 0 0
erc1155 14.308 14.308 0 0
erc20 6.918 6.918 0 0
erc721 10.007 10.007 0 0
events 5.258 5.258 0 0
flipper 1.639 1.639 0 0
incrementer 1.504 1.504 0 0
lang-err-integration-tests/call-builder-delegate 2.638 2.638 0 0
lang-err-integration-tests/call-builder 5.567 5.567 0 0
lang-err-integration-tests/constructors-return-value 1.985 1.985 0 0
lang-err-integration-tests/contract-ref 5.058 5.058 0 0
lang-err-integration-tests/integration-flipper 1.815 1.815 0 0
lazyvec-integration-test 4.616 4.616 0 0
mapping-integration-tests 7.999 7.999 0 0
mother 12.741 12.741 0 0
multi-contract-caller 6.645 6.645 0 0
multi-contract-caller/accumulator 1.378 1.378 0 0
multi-contract-caller/adder 1.912 1.912 0 0
multi-contract-caller/subber 1.932 1.932 0 0
multisig 21.821 21.821 0 0
payment-channel 5.659 5.659 0 0
psp22-extension 7.071 7.071 0 0
rand-extension 2.965 2.965 0 0
sr25519-verification 1.142 1.142 0 0
static-buffer 2.536 2.536 0 0
trait-dyn-cross-contract-calls 2.887 2.887 0 0
trait-dyn-cross-contract-calls/contracts/incrementer 1.545 1.545 0 0
trait-erc20 7.294 7.294 0 0
trait-flipper 1.49 1.49 0 0
trait-incrementer 1.614 1.614 0 0
upgradeable-contracts/delegator 3.928 3.928 0 0
upgradeable-contracts/delegator/delegatee 1.609 1.609 0 0
upgradeable-contracts/delegator/delegatee2 1.609 1.609 0 0
upgradeable-contracts/set-code-hash-migration 1.743 1.743 0 0
upgradeable-contracts/set-code-hash-migration/migration 1.45 1.45 0 0
upgradeable-contracts/set-code-hash-migration/updated-incrementer 1.897 1.897 0 0
upgradeable-contracts/set-code-hash 1.743 1.743 0 0
upgradeable-contracts/set-code-hash/updated-incrementer 1.721 1.721 0 0
wildcard-selector 2.846 2.846 0 0

Link to the run | Last update: Tue Feb 27 16:59:26 CET 2024

@SkymanOne SkymanOne merged commit ae73d0b into master Feb 27, 2024
23 checks passed
@SkymanOne SkymanOne deleted the gn/bump-metadata branch February 27, 2024 19:21
This was referenced Feb 28, 2024
@use-ink use-ink deleted a comment from Rockettty Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants