primitive-types/impl-codec: Bump to include new parity_scale_codec::MaxEncodedLen impls

[Xanewok]

Part of paritytech/substrate#9163.

Needs paritytech/scale-info#102 to be released first.

The gist of this PR is to implement parity_scale_codec::MaxEncodedLen for fixed hashes as required by Substrate (and other uints, which is done for consistency) and to update the rest of the crates using the new release candidate of the SCALE codec since Cargo requires us to explicitly opt-in for the RCs.

[ordian]

unfortunately, updating scale-info is a breaking change, so it has to 0.10.0, which will result in major bump in most of the crates in the workspace, sorry about that :/

[ascjones]

paritytech/scale-info#105

[Xanewok]

Hm, I'm beginning to question our approach of using a release candidate of parity-scale-codec to have more freedom in experimenting... This looks like will need to bump every version used here for the sake of explicitly selecting the release candidate for Cargo.
Also ideally the "pre-releaseness" should be viral and every crate here modified to depend on a pre-release should be a pre-release itself because the usage of parity-scale-codec is so pervasive that selecting both versions in the crate graph surely leads to build breakages and so it should be treated like a peer dependency in this case.

@coriolinus do you think we'll experiment with the newly introduced MaxEncodedLen and the derives while working on the PoV benchmarking enough so that it warrants a release candidate?
Maybe we can get away with just publishing the 2.2.0 and introducing tweaks to potentially changed attributes as patch-level "bug fixes"?
I know that might not be a perfect engineering approach but I'm worried that what we should ideally do here does not scale so well and we'll end up with -rc.N versions of everything here (and in turn in Substrate) which will probably be a whole another can of worms...

I'm open to suggestions because I'm not sure what should be the correct way forward.

[ordian]

Re breaking versions cascading, see #556 (comment)

[ascjones]

every crate here modified to depend on a pre-release should be a pre-release itself

Yes exactly, if there's a pre-release requirement for a dependency then we should really do a pre-release here, as I did in scale-info. Otherwise it forces all other crates with a dependency on parity-scale-codec to satisfy that pre-release requirement.

I'm open to suggestions because I'm not sure what should be the correct way forward.

Not sure exactly what the goal is, but could you somehow use a "local registry source" for the experimentation and just do "releases" there of all the crates in the tree which depend on parity-scale-codec? Then you don't have to release all those crates publicy to crates.io while you experiment.

[Xanewok]

Context

Not sure exactly what the goal is

To give more context on the rationale, we want to unbreak release checking process in Substrate in paritytech/substrate#9140 and paritytech/substrate#9163.

While the check was broken, a new workspace-local max-encoded-len(-crate) crate was introduced with a new trait MaxEncodedLen: parity_scale_codec::Encode that's used in Substrate. To resume publishing, we needed either to publish that crate temporarily or upstream this implementation into parity_scale_codec (as previously agreed upon). There were some concerns that we might not want to migrate it to SCALE until Substrate migrates first, because we might need a breaking change for trait definition, which is why I decided to push a pre-release, thinking it might affect at most 3-4 other crates (oh the blissful ignorance!) in addition to what's in the Substrate workspace.

Solution

Thinking about it some more, I think that the best solution would be to actually release a 2.2 but with MaxEncodedLen gated behind an additional feature flag (max-encoded-len) with an explicit note that it's exempt from the usual SemVer guarantees and considered experimental.

Since it's used pervasively, most crate dependents just care about a single Encode and Decode trait being available, so things will continue working as they were (with ^2 or ^2.1 dependency bounds, as is in most cases). However, if a crate decides to opt-in into MaxEncodedLen, then they have to guarantee that stuff keeps working until this new feature flag is considered stable.

The only crate apart from the Substrate ones that will be using this new trait is primitive-types to implement it manually for its primitives. However, we mostly want to keep more flexibility for other helper attributes like #[codec(crate = ...)] or #[mel_bound(...)] so that's what will probably change, rather than the trait definition. This means, that breakage here will probably not happen here but it will allow us to keep experimenting with the attributes in the Substrate repository.

WDYT?

cc @bkchr @shawntabrizi (discussion is somewhat scattered, sorry for the spam!)

[ascjones]

Thinking about it some more, I think that the best solution would be to actually release a 2.2 but with MaxEncodedLen gated behind an additional feature flag (max-encoded-len) with an explicit note that it's exempt from the usual SemVer guarantees and considered experimental.

Sounds good to me, it will allow you to conduct the experiments without the cascading pre-release requirement which causes friction and headaches.

That said there is always the danger that users will start to depend upon features even if they are advertised as unstable, so it's a tradeoff.

[ordian]

LGTM once the patch section is removed

[coriolinus]

@coriolinus do you think we'll experiment with the newly introduced MaxEncodedLen and the derives while working on the PoV benchmarking enough so that it warrants a release candidate?

Sorry for the delay, missed this message somehow. The trait itself has not been substantially edited since it was first written; so far at least, there have been no issues raised which would require any changes. We updated the derive at one point, but I'm comfortable calling it stable now.

[Xanewok]

Okay, this is now reduced to a minimum patch, which is just an impl-codec minor bump which pulls in a new feature of parity-scale-codec.

The end result we're interested in is that the types defined by primitive-types use that feature but with the way we have it structured via intermediate impl-* crates it's enough to just bump the impl-codec in the end-user workspace, e.g. Substrate.

We could, for clarity, also bump a minor level for primitive-types that'd also bump the required version of impl-codec. I don't have strong feelings on this personally and would just like to publish some working solution to unbreak paritytech/substrate#9163.

Can I get a re-review of this final patch? Thanks!

[dvdplm]

I think a minor bump to primitive-types would be good.