-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore(upgrade): v1.10.0 to v1.11.0 #2117
chore(upgrade): v1.10.0 to v1.11.0 #2117
Conversation
8e515fd
to
118cc6d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
2ad5baf
to
3e72965
Compare
5336cf4
to
5b6de32
Compare
f3f6b24
to
d172134
Compare
d172134
to
f5f10cc
Compare
I’ve split off the Omni Benchmarker CI updates into a follow-up PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed weights changes, and ran make start
. I ran all the rust tests. I could not run make start-paseo-relay
because of broken docker files (not a regression).
I could not get e2e tests to run locally and they would not recognize a running local frequency instance:
Unable to find or start a Frequency node; aborting.
@@ -101,8 +101,8 @@ impl<T: frame_system::Config> pallet_preimage::WeightInfo for SubstrateWeight<T> | |||
// Proof Size summary in bytes: | |||
// Measured: `243` | |||
// Estimated: `5041` | |||
// Minimum execution time: 47_210_000 picoseconds. | |||
Weight::from_parts(48_322_000, 5041) | |||
// Minimum execution time: 66_888_000 picoseconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting large weight changes throughout this file.
get_account_id_from_seed::<sr25519::Public>("Bob"), | ||
get_account_id_from_seed::<sr25519::Public>("Dave"), | ||
], | ||
development_invulnerables(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
237200e
to
29997b2
Compare
29997b2
to
8ac302c
Compare
Thanks for double checking. How are you running the e2e tests? I was able to run them using make e2e-tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read through the changes
- Mostly appear to be updates/refactor
- Existing comments are useful and can be addressed
Lgtm
- Upgrade Polkadot-sdk 1.10.0 to 1.11.0 - Update weights to reflect the new version. Notable Changes: - [Apply patch `run_with_spec`](paritytech/polkadot-sdk#3512) - [Genesis Presets](paritytech/polkadot-sdk#2714) - [Integrate LiteP2P](paritytech/polkadot-sdk#2944) - [Template Simplification](https://github.com/paritytech/polkadot-sdk/pull/3801/files) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.11.0) issue-1957
8ac302c
to
f1e3247
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Pull branch
- Built & ran unit tests
- Built & ran e2e tests
Looks good!
/// Build the import queue for the parachain runtime. | ||
#[cfg(not(feature = "frequency-no-relay"))] | ||
/// Build the import queue for the parachain runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of these is not needed:
/// Build the import queue for the parachain runtime. | |
#[cfg(not(feature = "frequency-no-relay"))] | |
/// Build the import queue for the parachain runtime. | |
#[cfg(not(feature = "frequency-no-relay"))] | |
/// Build the import queue for the parachain runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! I’ll update this in my follow-up PR to avoid triggering the CI again.
@@ -482,15 +471,3 @@ fn start_consensus( | |||
|
|||
Ok(()) | |||
} | |||
|
|||
/// Start a parachain node. | |||
#[cfg(any(not(feature = "frequency-no-relay"), feature = "frequency-lint-check"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup.
@@ -192,7 +192,7 @@ impl WeightToFeeTrait for TransactionByteFee { | |||
|
|||
impl pallet_transaction_payment::Config for Test { | |||
type RuntimeEvent = RuntimeEvent; | |||
type OnChargeTransaction = CurrencyAdapter<Balances, ()>; | |||
type OnChargeTransaction = FungibleAdapter<Balances, ()>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bye, bye Currency. 🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Read through Polkadot Release notes.
- Read through code changes.
Looks good, nice work!
🚢 it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like my test run is a local problem. Approved!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read the code and looked good. Just some questions.
@@ -48,7 +48,7 @@ sp-inherents = { workspace = true } | |||
sp-keyring = { workspace = true } | |||
sp-runtime = { workspace = true } | |||
sp-timestamp = { workspace = true } | |||
try-runtime-cli = { workspace = true, optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Would you mind explaining why is this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I missed including that in the PR description. There’s now a standalone CLI, which is a result of this Polkadot-sdk issue.
|
||
info!("Parachain id: {:?}", id); | ||
info!("Parachain Account: {}", parachain_account); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no more parachain account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this might be leftover code since it was only used for logging. The PR description where Parity removed it doesn’t provide a clear explanation.
Link to the PR.
- Upgrade Polkadot-sdk 1.10.0 to 1.11.0 - Update weights to reflect the new version. Notable Changes: - [Apply patch `run_with_spec`](paritytech/polkadot-sdk#3512) - [Genesis Presets](paritytech/polkadot-sdk#2714) - [Integrate LiteP2P](paritytech/polkadot-sdk#2944) - [Template Simplification](https://github.com/paritytech/polkadot-sdk/pull/3801/files) - [Remove try-runtime-cli](https://github.com/paritytech/polkadot-sdk/pull/4017/files#diff-3a3aa5e088741f8ab1ca38fbe314c7a150d18b7466426f4ad2569113017e8439) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.11.0)
Notable Changes:
run_with_spec
For more details, please refer to:
Release Notes
#1957