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

feat: bouncer command for submitting runtime upgrades #4122

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

kylezs
Copy link
Contributor

@kylezs kylezs commented Oct 16, 2023

Pull Request

Part of: PRO-646

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

This can then be composed with some future work, so we can have (more) automated upgrade testing 🙌

@linear
Copy link

linear bot commented Oct 16, 2023

PRO-646 CFE upgrade testing

Once the upgrading functionality is in, we should add a bouncer test for it.

There are two types of upgrades to handle:

  • Compatible CFE
  • Incompatible CFE

The former is easy, and can be tested with a bump to the runtime version on the SC (without any other changes). Which could be done in an ordinary bouncer command.

The latter is the more difficult.

The upgrade tests could be in a separate module for bouncer, since it may require a specific setup step to create the second binary, rather than requiring that we always need the second binary even when we may not want to test upgrades.

@kylezs kylezs force-pushed the chore/bouncer-submit-runtime-upgrade branch from ada93ab to 658adaf Compare October 16, 2023 09:54
const chainflip = await getChainflipApi();

let versionPercentRestriction;
if (semverRestriction && percentNodesUpgraded) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to set only the semver restriction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #4122 (e2314ef) into main (5c70808) will increase coverage by 0%.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main   #4122   +/-   ##
=====================================
  Coverage     72%     72%           
=====================================
  Files        378     378           
  Lines      60637   60637           
  Branches   60637   60637           
=====================================
+ Hits       43492   43494    +2     
+ Misses     14881   14880    -1     
+ Partials    2264    2263    -1     

see 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kylezs kylezs enabled auto-merge (squash) October 16, 2023 12:43
@kylezs kylezs merged commit b1fe871 into main Oct 16, 2023
44 checks passed
@kylezs kylezs deleted the chore/bouncer-submit-runtime-upgrade branch October 16, 2023 13:11
// 2. Optional: A JSON string representing the semver restriction for the upgrade. If not provided, the upgrade will not be restricted by semver.
// 3. Optional: A number representing the percentage of nodes that must be upgraded before the upgrade will be allowed to proceed. If not provided, the upgrade will not be restricted by the number of nodes that have upgraded.
//
// For example: ./commands/submit_runtime_upgrade.ts /path/to/state_chain_runtime.compact.compressed.wasm '{"major": 1, "minor": 2, "patch": 3}' 50
Copy link
Contributor

@msgmaxim msgmaxim Oct 17, 2023

Choose a reason for hiding this comment

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

Is it worth making the command easier for the caller by letting them specify the version using dot notation, e.g.:
./commands/submit_runtime_upgrade.ts /path/to/state_chain_runtime.compact.compressed.wasm 1.2.3 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah thought about this, it'll mostly be used by other scripts - so it doesn't matter as much, but I may end up changing it to something like you've described as I go, because it is a bit nicer

syan095 added a commit that referenced this pull request Oct 17, 2023
* origin/main:
  chore: get gas parameters from statechain event (#4125)
  Feat: ingress-egress tracking for DOT (#4121)
  add support for hex encoded amounts on limit order and range order methods in LP API (#4120)
  feat: bouncer command for submitting runtime upgrades (#4122)
  Feat: ensure correct process termination in ingress/egress tracker (#4101)
  feat(custom-rpc): add flip balance to account info (#4119)
  chore: storage migration delete NextCompatibilityVersion (#4115)
  chore: delete unneeded function (#4116)

# Conflicts:
#	state-chain/runtime/src/lib.rs
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