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

Hide nonce implementation details in metadata #6562

Merged
merged 12 commits into from
Nov 26, 2024

Conversation

re-gius
Copy link
Contributor

@re-gius re-gius commented Nov 20, 2024

See polkadot-fellows/runtimes#248 : using TypeWithDefault having derived TypeInfo for Nonce causes a breaking change in metadata for nonce type because it's no longer u64.

Adding a default implementation of TypeInfo for TypeWithDefault to restore the original type info in metadata.

@re-gius re-gius added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Nov 20, 2024
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/11937561288
Failed job name: cargo-clippy

@re-gius re-gius marked this pull request as ready for review November 22, 2024 10:28
@@ -565,5 +577,11 @@ mod tests {
}
type U128WithDefault = TypeWithDefault<u128, Getu128>;
impl WrapAtLeast32Bit for U128WithDefault {}

assert_eq!(U8WithDefault::type_info(), <u8 as TypeInfo>::type_info());
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, this was not passing before this PR, right

Copy link
Contributor Author

@re-gius re-gius Nov 25, 2024

Choose a reason for hiding this comment

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

Exactly, I just double-checked: it does not pass with the derived TypeInfo

@@ -40,7 +40,7 @@ use serde::{Deserialize, Serialize};
/// A type that wraps another type and provides a default value.
///
/// Passes through arithmetical and many other operations to the inner value.
#[derive(Encode, Decode, TypeInfo, Debug, MaxEncodedLen)]
#[derive(Encode, Decode, Debug, MaxEncodedLen)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should update the doc of the type to specify this nuance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 06212c9

@kianenigma kianenigma requested review from jsdw and ggwpez November 25, 2024 13:56
@kianenigma
Copy link
Contributor

The only minor downside of this is that now we would have to wait for the next release, and then update the fellowship repo as well

Set your reminders @re-gius :)

Copy link
Contributor

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

LGTM!

@re-gius re-gius added this pull request to the merge queue Nov 26, 2024
Merged via the queue into master with commit fc315ac Nov 26, 2024
196 of 201 checks passed
@re-gius re-gius deleted the re-gius/hide-type-info-for-nonce branch November 26, 2024 17:18
@re-gius re-gius added the A4-needs-backport Pull request must be backported to all maintained releases. label Nov 28, 2024
@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2407:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6562-to-stable2407
git worktree add --checkout .worktree/backport-6562-to-stable2407 backport-6562-to-stable2407
cd .worktree/backport-6562-to-stable2407
git reset --hard HEAD^
git cherry-pick -x fc315ac5979e9bf1cc56b58ab6f364b6b2689635
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2409:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6562-to-stable2409
git worktree add --checkout .worktree/backport-6562-to-stable2409 backport-6562-to-stable2409
cd .worktree/backport-6562-to-stable2409
git reset --hard HEAD^
git cherry-pick -x fc315ac5979e9bf1cc56b58ab6f364b6b2689635
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2412:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6562-to-stable2412
git worktree add --checkout .worktree/backport-6562-to-stable2412 backport-6562-to-stable2412
cd .worktree/backport-6562-to-stable2412
git reset --hard HEAD^
git cherry-pick -x fc315ac5979e9bf1cc56b58ab6f364b6b2689635
git push --force-with-lease

@re-gius re-gius had a problem deploying to subsystem-benchmarks November 28, 2024 17:36 — with GitHub Actions Failure
EgorPopelyaev added a commit that referenced this pull request Dec 3, 2024
Backport #6562 into `stable2412` from re-gius.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

---------

Co-authored-by: Giuseppe Re <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-needs-backport Pull request must be backported to all maintained releases. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants