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

Have a pass over metadata validation #959

Merged
merged 6 commits into from
May 16, 2023
Merged

Have a pass over metadata validation #959

merged 6 commits into from
May 16, 2023

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented May 15, 2023

  • Loosen struct and enum field ordering now that EncodeAsType and DecodeAsType render the order irrelevant
  • Tighten up some other places where we used xor instead of concat_and_hash
  • Get rid of sorting and such by relying on XOR to produce order-independent hashes
  • merge the per pallet and global metadata hashing logic into one function (We may want to extend this for only specific runtime APIs too once we allow runtime APIs to be stripped from metadata too)

@jsdw jsdw requested a review from a team as a code owner May 15, 2023 12:51
}

/// Hash the given metadata.
pub fn hash(&self, metadata: &RuntimeMetadataV15) -> [u8; HASH_LEN] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I merged the per pallet hashing and the global hashing here. The effect is that the per pallet hashing will now also care about any additional details that the global one did (eg runtime APIs).

I think that this is the correct behaviour, but worth a ponder as I might be forgetting something :)

Copy link
Contributor

@tadeohepperle tadeohepperle left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@jsdw jsdw merged commit 8149402 into master May 16, 2023
@jsdw jsdw deleted the jsdw-metadata-validation branch May 16, 2023 11:47
@jsdw jsdw mentioned this pull request Jun 1, 2023
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.

3 participants