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

Remove compilation feature disable-metadata-hash-check #833

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

haerdib
Copy link
Contributor

@haerdib haerdib commented Feb 5, 2025

  • Removes compilation feature disable-metadata-hash-check
  • Adds struct ExtrinsicParamsWithoutHashCheck to still provide an "easy" compatibility with older nodes.

@haerdib haerdib requested a review from Niederb February 5, 2025 13:01
@haerdib haerdib added F7-enhancement Enhances an already existing functionality E2-breaksapi labels Feb 5, 2025
@haerdib haerdib marked this pull request as ready for review February 5, 2025 13:01
@haerdib haerdib changed the title Remove compilation feature for hash check extrinsic params Remove compilation feature disable-metadata-hash-check Feb 5, 2025
mortality_checkpoint: Option<Hash>,
tip: Tip,
pub era: Era,
pub mortality_checkpoint: Option<Hash>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it's the best way to make this pub, but the normal getter function names are already taken as setter functions.. and since it does not provide any real functionality, it's basically just a data container. So I don't see the need to keep the inners private.

Happy to adapt if it's not ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about using pub(crate)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, because if we are able to reuse the struct in a different ExtrinsicParams implementation, then so could others with their specific Runtime implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point. Then we leave it as is, works for me.

Copy link
Contributor

@Niederb Niederb left a comment

Choose a reason for hiding this comment

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

Just one comment regarding pub

mortality_checkpoint: Option<Hash>,
tip: Tip,
pub era: Era,
pub mortality_checkpoint: Option<Hash>,
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using pub(crate)?

@haerdib haerdib merged commit 1f040e0 into master Feb 6, 2025
60 checks passed
@haerdib haerdib deleted the bh/remove-disable-metadata-hash-check branch February 6, 2025 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2-breaksapi F7-enhancement Enhances an already existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants