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

Bump to spec version 297 #503

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Bump to spec version 297 #503

merged 1 commit into from
Jan 9, 2024

Conversation

imstar15
Copy link
Member

@imstar15 imstar15 commented Jan 9, 2024

PRs:
Modify the scheduleAs parameter to be an optional parameter
#499

@imstar15 imstar15 requested review from chrisli30 and v9n January 9, 2024 03:44
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

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

No, why do we change the spec_version of the chain? Please read the instructions on Notion, specifically "transaction number must change when an existing dispatchable (module ID, dispatch ID) is changed, either through an alteration in its user-level semantics, a parameter added/removed/changed" and "If this number changes, then spec_version must change, also."

Full reference:

	/// Version of the implementation of the specification. Nodes are free to ignore this; it
	/// serves only as an indication that the code is different; as long as the other two versions
	/// are the same then while the actual code may be different, it is nonetheless required to
	/// do the same thing.
	/// Non-consensus-breaking optimizations are about the only changes that could be made which
	/// would result in only the `impl_version` changing.
	pub impl_version: u32,

	/// All existing dispatches are fully compatible when this number doesn't change. If this
	/// number changes, then `spec_version` must change, also.
	///
	/// This number must change when an existing dispatchable (module ID, dispatch ID) is changed,
	/// either through an alteration in its user-level semantics, a parameter
	/// added/removed/changed, a dispatchable being removed, a module being removed, or a
	/// dispatchable/module changing its index.
	///
	/// It need *not* change when a new module is added or when a dispatchable is added.
	pub transaction_version: u32,

@imstar15
Copy link
Member Author

imstar15 commented Jan 9, 2024

No, why do we change the spec_version of the chain? Please read the instructions on Notion, specifically "transaction number must change when an existing dispatchable (module ID, dispatch ID) is changed, either through an alteration in its user-level semantics, a parameter added/removed/changed" and "If this number changes, then spec_version must change, also."

Full reference:

	/// Version of the implementation of the specification. Nodes are free to ignore this; it
	/// serves only as an indication that the code is different; as long as the other two versions
	/// are the same then while the actual code may be different, it is nonetheless required to
	/// do the same thing.
	/// Non-consensus-breaking optimizations are about the only changes that could be made which
	/// would result in only the `impl_version` changing.
	pub impl_version: u32,

	/// All existing dispatches are fully compatible when this number doesn't change. If this
	/// number changes, then `spec_version` must change, also.
	///
	/// This number must change when an existing dispatchable (module ID, dispatch ID) is changed,
	/// either through an alteration in its user-level semantics, a parameter
	/// added/removed/changed, a dispatchable being removed, a module being removed, or a
	/// dispatchable/module changing its index.
	///
	/// It need *not* change when a new module is added or when a dispatchable is added.
	pub transaction_version: u32,
  1. The scheduleXcmpTaskThoughProxy dispatch has been removed.
  2. The dispatch ID for scheduleXcmpTask has been changed.
  3. scheduleXcmpTask dispatch has added scheduleAs and instructionSequence parameters.

Hence, the transaction_version and spec_version need to be changed.

Copy link
Member

@chrisli30 chrisli30 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. The spec_version needs to change because of the above reasons 👍

@imstar15 imstar15 merged commit 182258c into master Jan 9, 2024
3 checks passed
@imstar15 imstar15 deleted the version-v2.1.3 branch January 9, 2024 06:02
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.

2 participants