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

[DO NOT REVIEW] feat: update to polkadot v1.1.0 release #1157

Closed
wants to merge 21 commits into from

Conversation

ashutoshvarma
Copy link
Member

@ashutoshvarma ashutoshvarma commented Jan 26, 2024

Pull Request Summary
This PR has been superseded by #1182

@ashutoshvarma ashutoshvarma self-assigned this Jan 26, 2024
@ashutoshvarma ashutoshvarma added shiden related to shiden runtime astar Related to Astar shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. labels Jan 26, 2024
runtime/astar/Cargo.toml Outdated Show resolved Hide resolved
runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
run_to_block::<T>(state.next_era_start - 1);
DappStaking::<T>::on_finalize(state.next_era_start - 1);
System::<T>::set_block_number(state.next_era_start);
run_to_block::<T>((state.next_era_start - 1).into());
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps the helper functions should do this conversion?
E.g. change function argument to u64.

Seems like could would be much more readable.

Comment on lines +27 to +30
pub(super) fn run_to_block<T: Config>(n: BlockNumberFor<T>)
where
BlockNumberFor<T>: IsType<BlockNumber>,
{
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it can be solved by my previous comment, but we should really consider defining the custom type to be sued here, to avoid repeating this lengthy type description everywhere.

Comment on lines +38 to +41
#[benchmarks(
where
BlockNumberFor<T>: IsType<BlockNumber>,
)]
Copy link
Member

Choose a reason for hiding this comment

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

A general comment about this, since it's repeating really a lot - astar-primitives were introduced to avoid having such code snippets, to cut down on the code repetition. And now we find ourselves going in the separate direction 🙂.

Should we get rid of the BlockNumber from the astar-primitives?
Should we instead define our custom type, which would cover the new BlockNumberFor requirements?

We'd all prefer I guess not to have to repeat the where statement everywhere.

Comment on lines +788 to +792
Some(Subcommand::TryRuntime) => Err("The `try-runtime` subcommand has been migrated to a \
standalone CLI (https://github.com/paritytech/try-runtime-cli). It is no longer \
being maintained here and will be removed entirely some time after January 2024. \
Please remove this subcommand from your runtime and use the standalone CLI."
.into()),
Copy link
Member

Choose a reason for hiding this comment

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

Why not just remove it now?

bin/collator/src/local/service.rs Show resolved Hide resolved
@ashutoshvarma ashutoshvarma changed the title feat: update to polkadot v1.1.0 release [DO NOT REVIEW] feat: update to polkadot v1.1.0 release Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. shibuya related to shibuya shiden related to shiden runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants