Skip to content
This repository was archived by the owner on Feb 28, 2021. It is now read-only.

Drop CheckVersion signed extension#458

Merged
NunoAlexandre merged 5 commits intomasterfrom
drop-check-version
May 25, 2020
Merged

Drop CheckVersion signed extension#458
NunoAlexandre merged 5 commits intomasterfrom
drop-check-version

Conversation

@NunoAlexandre
Copy link
Copy Markdown
Contributor

Closes #457

CheckVersion requires a client and a node interacting to use the same
spec_version of the runtime, which is not something we want. Rather,
we want those to be as compatible as technically possible, something we
will tackle in the near future.

@NunoAlexandre NunoAlexandre added this to the Registry Updates milestone May 18, 2020
@NunoAlexandre NunoAlexandre self-assigned this May 18, 2020
@NunoAlexandre
Copy link
Copy Markdown
Contributor Author

@geigerzaehler so the e2e tests fail because the wasm runtime expects the check version signed extension while the latest native runtime doesn't provide it 🙃 any idea of how we can work around this and with such changes to signed extensions?

@geigerzaehler
Copy link
Copy Markdown

@geigerzaehler so the e2e tests fail because the wasm runtime expects the check version signed extension while the latest native runtime doesn't provide it upside_down_face any idea of how we can work around this and with such changes to signed extensions?

As mentioned on the issue, this change is a semantic change to the runtime and requires us to go through the upgrade process.

@NunoAlexandre NunoAlexandre force-pushed the drop-check-version branch 2 times, most recently from e29484f to 41642e6 Compare May 21, 2020 14:27
@NunoAlexandre
Copy link
Copy Markdown
Contributor Author

NunoAlexandre commented May 21, 2020

@geigerzaehler this is ready for review. In terms of how to deploy this change, see #464 (comment).

Edit: Bumping to spec_version 6 since v5 is entering master soon...

impl_name: create_runtime_str!("radicle-registry"),
spec_version: 4,
spec_version: 5,
impl_version: 4,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please reset impl_version to 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I noticed that. I also have to bump the spec_version to 6 after #468 but I can't re-generate the wasm file: #469

@geigerzaehler
Copy link
Copy Markdown

Could you please create a follow-up issue to add this check again once we’ve figured out the client compatibility story?

It requires a client and a node interacting to use the same
`spec_version` of the runtime, which is not something we want. Rather,
we want those to be as compatible as technically possible, something we
will tackle in the near future.
@NunoAlexandre NunoAlexandre merged commit f13c1d7 into master May 25, 2020
@NunoAlexandre NunoAlexandre deleted the drop-check-version branch May 25, 2020 10:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider dropping CheckVersion

2 participants