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

CW-53: Added a runtime metadata check #1762

Merged
merged 23 commits into from
Jun 19, 2024
Merged

Conversation

Marcin-Radecki
Copy link
Contributor

@Marcin-Radecki Marcin-Radecki commented Jun 13, 2024

Description

This PR adds a runtime metadata check, which consists of two phases:

  1. Build subxt using large self-hosted runner (1 minute)
  2. Run subxt codegen on runtime built from main (used in e2e tests - short session) (1 minute)
    This check fails if someone forgets to regenerate the aleph-runtime static metadata file.

There's no if condition when this check is supposed to run as if practically it does not make sense to check, e.g., if runtime code changed or aleph pallet code changed in git, as then we'd need to check every aleph-runtime dependency, etc. This check is lightweight (2-3 minutes, and runs in parallel with the longest check excluded packages).

Also, this PR consists of changes to the Aleph::NextAuthorities hash. This hash is what subtext thinks the hash of NextAuthorities metadata is. Normally, this hash does not include any of what is stored as a storage value, but only data such as pallet name, storage function name, storage value type (Option<T> or T), etc. Also, maybe a bit surprisingly, it includes default value of given type into that subxt hash. Now, NextAuthorities storage value type is Vec<AccountId>, and normally default is vec![]. However, we do have a default initializer DefaultNextAuthorities which, turns out, is used by subxt codegen when generating hash for NextAuthorities. When DefaultNextAuthorities, static runtime metadata generated by subxt is always the same in CI. DefaultNextAuthorities is not required in pallet aleph, as initialization of NextAuthorities is done in on_genesis_session.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Testing

@Marcin-Radecki Marcin-Radecki marked this pull request as ready for review June 19, 2024 07:27
Copy link
Member

@pmikolajczyk41 pmikolajczyk41 left a comment

Choose a reason for hiding this comment

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

well done! lgtm

.github/scripts/run_consensus.sh Show resolved Hide resolved
@Marcin-Radecki Marcin-Radecki added this pull request to the merge queue Jun 19, 2024
Merged via the queue into main with commit 2b195a0 Jun 19, 2024
18 checks passed
@Marcin-Radecki Marcin-Radecki deleted the update-aleph-client-metadata branch June 19, 2024 08:33
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