Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add CI job to verify extrinsic ordering #1950

Merged
merged 12 commits into from
Nov 16, 2020
Merged

Conversation

s3krit
Copy link
Contributor

@s3krit s3krit commented Nov 13, 2020

What?: This PR adds a job that uses @jacogr 's polkadot-js-metadata-cmp tool to verify that if the ordering/indexing of the extrinsics of a runtime have changed since the last release, transaction_version for that runtime has been bumped.

How?: We grab the polkadot binary produced in the test-linux-stable job, and for each runtime, we run the client in the background with --chain=$RUNTIME-local. Then we use polkadot-js-metadata-cmp to compare the metadata with the live chain.

This essentially automates this check that is completed as part of our release process - but on a per-PR basis. In design and implementation it's somewhat similar to our check_runtime.sh CI job.

After merging, we should add this check as a must-pass to our branch protection rules for master

An example of a passing job can be found here
An example of a failing job can be found here (to cause it to fail, I had it compare the metadata between kusama and polkadot)

@s3krit s3krit added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Nov 13, 2020
@s3krit s3krit self-assigned this Nov 13, 2020
.gitlab-ci.yml Outdated Show resolved Hide resolved
.gitlab-ci.yml Outdated Show resolved Hide resolved
scripts/gitlab/check_extrinsics_ordering.sh Outdated Show resolved Hide resolved
)

if [ -n "$changed_extrinsics" ]; then
echo "[!] Extrinsics indexing/ordering has changed in the ${RUNTIME} runtime! If this change is intentional, please bump transaction_version in lib.rs. Changed extrinsics:"
Copy link
Contributor

Choose a reason for hiding this comment

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

ain't it too long?

Copy link
Contributor

@gabreal gabreal left a comment

Choose a reason for hiding this comment

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

lgtm

@s3krit s3krit merged commit a5b7527 into master Nov 16, 2020
@s3krit s3krit deleted the mp-check-extrinsics-job branch November 16, 2020 13:41
ordian added a commit that referenced this pull request Nov 16, 2020
* master:
  Breakdown the Router module on Dmp, Ump, Hrmp modules (#1939)
  Add CI job to verify extrinsic ordering (#1950)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants