-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Metadata V15 breaking change: Features to include in V15 #12939
Comments
It will be great if we can avoid breaking changes. |
Hi, Thanks @xlc for providing input on this issue. Regarding exposing multiple flavors of the metadata, we have discussed something similar in issue: #12370.
Although it would be a benefit from the user perspective to not have a breaking change, @bkchr what do you think? |
There is no filter required. I am asking to add a new API I am really against breaking change of metadata because it will break EVERYTHING. Every exchanges, bots, hardware wallet, wallets, dApps, blockexplorer, scripts etc have to upgrade. Many of them are running perfectly otherwise. This is going to cost a lot of dev resources & money. And what do we gain from this breaking change? Yeah runtime API metadata is super useful. But 99% of the current code doesn't need it and it is just a bad idea to force those code to upgrade for no benefits. |
I have to agree with @xlc here. Breaking will be too bad for the ecosystem and we should not do it unless absolutely necessary. Additionally, I wouldn't call it metadata "flavors" but versioned metadata. IMHO we should move to version 15 but still provide v14 under its original location. We should a an additional fallible In this case supporting the old version is not hard as it is just the new version minus the runtime API section. |
Then we can also just introduce metadata v15 format "today" and then wait 3 months until we start returning it from any runtime. The entire metadata is versioned from almost the beginning and the argument "any change will take quite a lot of dev resources" is a bad argument. This means we could never change anything and this is really bad. We are still soo early and people need to be able to adapt. If we give people 3 months or maybe more to update, it should be possible to adapt all your stuff to the new metadata and have all software updated. I mean we could maybe come up with something like, only one metadata version per year and always collect stuff, but completely stopping is a really bad idea. This would prevent any kind of innovation and I will not support this. |
To what we should add:
|
Why? The old runtime API will still exist. Downstream tools will start using the new runtime API at their own pace. We will remove the old version of the metadata and runtime API once we are convinced that everybody who is willing to keep up has switched. I am not saying we need to support old stuff forever. I agree that it is too early for that. It is substrate's big advantage that it actually can change. I am merely advocating to allow for a transition period where we have the old and the new one supported at the same time. That will make it much much less of a world breaking event. |
Metadata with Runtime APIDevelopers add runtime API in the following manner:
The first step contains helpful information as the documentation. At the same time, the second step is where we know which traits (runtime APIs) are implemented for which runtime. This example declares a substrate/primitives/api/src/lib.rs Lines 733 to 737 in ed3f055
Users that want to call the The function name is obtained by concatenating the trait name with the method name (note: there is also an API versioning that we should take into account). Frame-MetadataThe frame-metadata crate stores the type of metadata and the latest version is v14. The v15 metadata extends the v14 with the The runtime field is a collection of
SubstrateThe All three macros generate crate access: The For populating the metadata, the
Those crates are not available in the The To allow a smooth transition for developers and avoid a sudden breaking change in the ecosystem, the After a few months, the SummaryFrame-metadata:
Substrate:
PoC
// CC: @jsdw @niklasad1 @bkchr @ascjones @athei |
One problem of doing this is that it would prevent of "merklized metadata" where we include the metadata storage root in the signing process to ensure that no one can fake the metadata. If we would have different flavours of the metadata.. While writing this I realized that we could just include both versions of the metadata storage roots in the signing process.
Okay, I'm releasing my blocking on this :P I'm fine with doing this now :P |
Why do not use the more generalized API I proposed? Adding
Not updated clients only know about about the old storage root. So they cannot include both (or all currently valid metadata versions). Instead, a runtime needs to check the signature against all currently supported runtime versions (usually 2).
Nice |
We could return the metadata storage roots of all supported metadata versions. The client only needs to ensure that one of the storage roots matches the metadata it supports. |
I'm ok with either, but the pros and cons of adding methods like Pros:
Cons:
So on balance I think I prefer adding an explicit method per new version unless there is a reason I've overlooked that makes it a horrible idea :) Particularly It would be great if Alex could work on adding and merging Runtime API support behind an |
It is. Because every runtime needs to implement them. While with a general API they do that once and get automatically upgraded when FRAME adds this version. It is an additional layer of friction. We need to make the transition as easy as possible or we will lock ourselves into a corner. We will not be able to evolve the format because we get to afraid of the backlash.
Add another function which returns the list of supported versions then.
I don't see that as a strong argument. Just don't call
The friction of copy pasting those runtime impls into every single runtime. Boilerplate will be the end of us. |
Would we need to maintain all the runtime functions that we have in place? Presume we have: /// Substrate perspective
impl_runtime macro generates
Runtime::get_next_metadata() // we modify this as we see fit for the V15, V16 and onwards format
/// User/Runtime perspective
impl_runtime {
Metadata {
fn metadata() ... // Same as before
/// Upon updates, the users will modify s/v15/v16
/// Or name this `get_next_unstable_metadata` and never have to modify things
fn metadata_v15() -> Metadata {
Runtime::get_next_metadata()
}
} Just out of curiosity, would removing functions cause any issues with substrate? VersioningAnother thing that I wanted to point out is the trait version generated via |
On the topic of what to add to the metadata, pallet documentation (as discussed in paritytech/frame-metadata#47) would be very useful for building documentation sites (as we're planning to for capi). |
I don't get why you are so keen on encoding the version in the name of the runtime API. Can you explain? This will me a massive headache because the runtime authors need to implement those APIs based on whether FRAME exposes those APIs. This is just friction. For what? It doesn't change anything about the amount of versions you need to maintain. It is the same thing. Just without all the boilerplate of adding and removing runtime APIs. |
I was exploring maybe removing the I don't quite have the whole context into the runtime, yet so I might be wrong: Would a method that returns just the latest next metadata suffice for our use cases? fn get_next_unstable_metadata() -> Option<OpaqueMetadata> Tho, I have no strong feelings about this, and the |
If there are two methods for getting the metadata along the lines of let metadata1 = get_metadata();
if (is_supported_version(metadata1)) {
return metadata1;
}
let metadata2 = get_next_unstable_metadata();
if(is_supported_version(metadata2)) {
return metadata2;
}
// error, unsupported version Which could end up sending two copies of the metadata over the network. In contrast, |
A side note: applications may very well support multiple versions of the metadata, so having a method which specifies a version range (at which point the chain would send the highest version it supports in range) could be beneficial. |
I agree we should not use fn metadata_at_version(version: u32) -> Option<Metadata>;
fn metadata_versions() -> Vec<u32>; The original metadata version function will stick around as long as we support version We need to think about how these functions will be consumed. I see two different users:
Will call
I don't think it is much harder than having separate functions. Just a simple dispatcher based on the supplied version.
See above. Node returns supported versions and the client selects. Let's not go all TLS on that and end up in a complicated negotiation protocol. |
That is also what I would have proposed and I think this is the best way forward. A new method per metadata version isn't smart. As @athei already said, with these two functions we can hide the rest in FRAME and the user doesn't care. They only need to enable the features in |
Cool! That makes a lot of sense! Thanks for the clarifications!
Regarding the versioning of the runtime APIs themself, do you believe it may be of use to call an older version? (ie substrate/primitives/consensus/babe/src/lib.rs Lines 384 to 391 in 9015061
|
Or more specifically; should the metadata include information about older versions of calls? I think for
Ok; the arguments against a new method per version make sense (and I'm happy with this suggestion)! And I'd assume that |
No. Each runtime always only supports one version of the runtime function. The earlier versions are present in earlier runtimes and can not be used anymore in runtimes which implement the newest version. |
In the |
To the node side, yes. But on the runtime side you only have one. For the node side this is important to be able to call the old version and have some way of versioning the calls. But the metadata is always from the view of the current runtime and the current runtime only supports one version of the call.
|
I guess I had in mind that, like the How would you see the new metadata work (adding the new runtime APIs and the macro work to build the metadata up etc) being done in Substrate? Would it be hidden behind some unstable feature flag (that when enabled would also enable some frame-metadata unstable feature flag)? |
I see. It is just to import historical blocks. Agreed then we only include the most recent version in the metadata.
I agree. We should return even unstable versions there. Because code that automatically uses the newest version doesn't make sense even without the existence of unstable function: If a new stable version appears it will still break their code. The discoverability is only for UIs to support older versions not newer ones. |
Will we really need to have this on master to test the new metadata with subxt and whatever? I mean when the new metadata version is added to the crate, we will need to update in Substrate and start using it. In your runtime you can then enable the feature the |
I agree. And I don't think anything contradicting this was written here. Mainly that any production tool would not use the latest unstable version. When you are testing against the new version you would of course supply the unstable version. It is available but any sensible client would not use it for anything but testing.
Please don't. No more feature flags. We just document that the version is unstable. Or maybe even just give it a temporary number like |
I agree with all of this. I'd prefer a new version not be exposed from
I guess I had the following workflow in mind for developing this stuff:
|
I agree. We need a way to get incrementally build a new metadata version on |
Okay fine ;) |
Suggestion: From paritytech/polkadot-sdk#182 I had this idea since very beginning. How about just support custom metadata? In that way we can extend metadata without extend the metadata format. |
TBH, just sounds like a band tape on the shitty process we have. This should go into the release notes. No need to put this kind of stuff into the metadata where people will also just ignore it. |
Yeah I agree putting depreciation notice in metadata isn’t really the right thing to do. Still, I want custom metadata so I can annotate them to emulate things like paritytech/polkadot-sdk#349 |
I agree that the deprecation process should also include having release notes perhaps even starting a forum discussion to increase visibility. Hopefully paritytech/polkadot-sdk#182 will improve the current deprecation process. On the other hand, I think that allowing developers to "subscribe" to deprecation through metadata could be a good complement to the process. |
In principle I'm not opposed to either the deprecation thing or custom attrs in metadata if there was consensus around adding them and what they would look like.
enum State {
#[codec(index = 0)]
Stable,
#[codec(index = 1)]
Deprecated { message: Sting }
}
// and then attach a `State` to each call, constant and storage entry.
// Potential to add other states without breaking the interface, too
// (in one direction at least).
custom: BTreeMap<String, { type: u32, value: Vec<u8> }> It'd be up to parachains to inject values and type_ids (which reference types to be included in the metadata type registry to help tooling decode the value appropriately). Come to think of it, Things like Subxt could generate some code to then find and decode these arbitrary values too. Even if there was quick agreement and we could implement these things, I'm not concerned if these do or don't make it into V15 metadata (see the forum post; the proposed cutoff to stabilise this is end of June), because it wouldn't be awfully hard to put out and stabilise a V16 metadata etc going forwards anyway. But that said, given agreement I think we could probably sneak at least the custom attributes in, which is a quite trivial addition that leaves the "how to actually get them into metadata" as a separate concern. |
@jsdw. fwiw, I think this is a solid plan. We could leave the discussion and eventual standarisation of the deprecation tag in the metadata for later metadata formats, since the V15 release is approaching fast and currently there is no consensus on if and how to add it. |
Sounds like a plan! @lexnv will put a PR into |
We would like to include the Runtime API info in the metadata and create a V15 release.
The metadata has been stable for a while and would like to minimize the number
of breaking changes from the user's perspective.
Considering this is a breaking change for the metadata, what other features would we
like to include in the metadata V15 before making a release?
Metadata with Runtime API
To help client-side applications make runtime API calls, we would like to include
the runtime API information in the metadata (for context: #11648).
This feature would help tools like subxt, polkadot-js, and capi provide a uniform API into the runtime.
Furthermore, after merging the RPC Spec V2 API into Substrate, the current RPC methods
calling into the runtime will be replaced by
chainHead_unstable_call
(the equivalent of thecurrent
state_call
method). One example is fetching the metadata viastate_getMetadata
that would be replaced by a runtime API call to
Metadata_metadata
without parameters.Discovering which runtime function is available, providing the parameters, and interpreting the
result into the proper shape are tedious and prone to error steps. Including the runtime API info
in the metadata would improve the user experience.
The
decl_runtime_apis
from Substrate will be modified to include the runtime API information in the metadata.// CC: @jsdw @niklasad1 @bkchr @ascjones @athei @harrysolovay
The text was updated successfully, but these errors were encountered: