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

Lift PoV size configuration restrictions #533

Conversation

s0me0ne-unkn0wn
Copy link
Contributor

This PR bumps the polkadot-runtime-parachains crate version to include the backport paritytech/polkadot-sdk#6672 into the runtimes.

We were going to make this happen before EoY 2024, but it was delayed because of the delayed crates release. Therefore, this PR also proposes to release this change ASAP as a point release to the 1.3 release lane.

@s0me0ne-unkn0wn
Copy link
Contributor Author

For some reason, it downgrades staging-xcm and xcm-procedural 🤔

@acatangiu could you please have a look, do you have an idea why that happens, and is it okay or not?

@bkchr
Copy link
Contributor

bkchr commented Jan 9, 2025

Why can this not just be released with the next normal release?

@eskimor
Copy link
Contributor

eskimor commented Jan 9, 2025

Why can this not just be released with the next normal release?

When will the next normal release get cut realistically/pessimistically?

@bkchr
Copy link
Contributor

bkchr commented Jan 9, 2025

We are working on it. Latest end of january.

@eskimor
Copy link
Contributor

eskimor commented Jan 9, 2025

We are working on it. Latest end of january.

Assuming we could get a point release out in a few days (or less), I would rather not wait then.

  1. We promised this EoY 2024 - already overdue.
  2. 2.5 Meg PoVs are kind of ridiculous for chains with state size in the millions of accounts. I want to have this limit lifted, before chains run into limitations, not after.
  3. We have another spammening coming in, (non public run planned end of January/beginning February) where larger PoV sizes would also be helpful.

@bkchr
Copy link
Contributor

bkchr commented Jan 9, 2025

@acatangiu could you please have a look, do you have an idea why that happens, and is it okay or not?

Not okay. Please fix this.

Assuming we could get a point release out in a few days (or less), I would rather not wait then.

Not really convinced, but fine.

@acatangiu
Copy link
Contributor

@acatangiu could you please have a look, do you have an idea why that happens, and is it okay or not?

They should not be downgraded, no.

Not clear why it happens, you should check the branch where polkadot-runtime-parachains was released, against the branch of the previous version, and make sure it is the same "base" for staging-xcm and xcm-procedural.

@acatangiu
Copy link
Contributor

staging-xcm is indeed on version = "14.0.3" on the v1.14.0 release so the question is then, when and why were the XCM crates updated to 14.1.0.

@s0me0ne-unkn0wn
Copy link
Contributor Author

@acatangiu seems like we've found how it happened :/

Citing @Morganamilo:

yeah that was a quick hacky fix i had to apply because it seems some breaking changes were made that were not labeled as breaking

I pinned the crate to the xcm version that existed at the time of the original 1.14 release to make it self consistent otherwise it would be impossible to release

when i was investigating it seemed polkadot-runtime-parachains (or some dep) was using xcm_procedural::impl_conversion_functions_for_multilocation_v2 which was removed without being labeled breaking. meanwhile current xcm calls xcm_procedural::impl_conversion_functions_for_junctions_v4 which doesn't exist in the version of xcm_procedural that also contains v2. So both had to be pinned to the older version

you can see here the v2 stuff was removed and v4 stuff added

https://docs.rs/xcm-procedural/10.0.1/xcm_procedural/?search=impl_conversion_functions_for_
https://docs.rs/xcm-procedural/10.2.0/xcm_procedural/?search=impl_conversion_functions_for_

the fix for this (besides upgrding sdk version) would be to either add the v2 stuff back to xcm_procedural or migrate whatever is calling the v2 functions to use a newer version

an xcm dev will need to decide and implement that

@franciscoaguirre
Copy link
Contributor

franciscoaguirre commented Jan 10, 2025

I'm not understanding why that version of polkadot-runtime-parachains is depending on that version of xcm-procedural.
It's hard to navigate through the releases and crate versions.

I can add the v2 conversion functions back to xcm-procedural 10.2.0 if that's what we need.
I could also "migrate whatever is calling the v2 functions to use a newer version" but I don't know what's calling it.

Also, the functions are available in xcm-procedural 10.1.0 so it shouldn't have downgraded?

@eskimor
Copy link
Contributor

eskimor commented Jan 10, 2025

Ok. If this is getting complicated, time is likely better spent in getting the next major release out quickly (assuming the problem does not exist there, which sounds like it.)

@acatangiu
Copy link
Contributor

Ok. If this is getting complicated, time is likely better spent in getting the next major release out quickly (assuming the problem does not exist there, which sounds like it.)

Fully agree, at this point there's only a few PRs (all trivial or relatively simple) that just need reviews: #527

Then we can just do the major release and avoid two incremental releases (and runtime upgrades).

@s0me0ne-unkn0wn
Copy link
Contributor Author

I've just made sure the 1.4.0 will use the crate version 17.0.1, which does contain the backport lifting the restrictions, so we're good to go and no additional actions are needed.

@s0me0ne-unkn0wn s0me0ne-unkn0wn deleted the release-v1.3.5 branch January 10, 2025 18:56
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.

5 participants