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

Fetch constants from runtime metadata? #493

Closed
ascjones opened this issue Mar 29, 2022 · 4 comments · Fixed by #494
Closed

Fetch constants from runtime metadata? #493

ascjones opened this issue Mar 29, 2022 · 4 comments · Fixed by #494

Comments

@ascjones
Copy link
Member

ascjones commented Mar 29, 2022

Constants are currently quite literally constant, since they are embedded in the code generated from the metadata provided at compile time.

However, I believe it is more useful to read the constants from the metadata fetched at runtime from the node. This is for the case where the constant values differ in the runtime targeted node, which is legitimate when building an app which can target different compatible chains.

I came across this when debugging povstats which reads the maximum block weight, and I was adjusting the block weight on the node side which wasn't being reflected on the client side.

Rel #402

@lexnv
Copy link
Contributor

lexnv commented Mar 29, 2022

One possible approach would be to have both options of static and dynamic / runtime constant APIs.

In such a case, we could introduce similarly to the pub struct ConstantsApi; an extended

pub struct ConstantsApiRunTime<'a, T: ::subxt::Config> {
    client: &'a ::subxt::Client<T>,
}

Implementation would need to be extended for each pallet as well, to something similar to:

impl ConstantsApiRunTime {
    pub fn block_weights(&self) -> ...
    {
        Ok(::subxt::codec::Decode::decode(
        
            self.client.metadata().pallet("system").constant("block_weights").value
        
        )?)
    }

These changes would reflect in the API experience from

let max_weight = api.constants().system().block_weights().unwrap();

to

let max_weight = api.constants_runtime().system().block_weights().unwrap();

Another option would be to change the ConstantsApi to utilize the client's runtime constants directly.

The latter also implies a few changes in static metadata validation: constants would have to be removed from pallet validation, as no such constants would exist in the static version (although they are obtained dynamically, but potentially from a different node). We could avoid this limitation via pre-hashing existing constants and embedding just a hash in the generated API.

What option do you think would work the best for most use-cases? 😄

@ascjones
Copy link
Member Author

Only seeing your post now, I just put in a quick implementation which changes the codegen to read from the runtime metadata.

In general I can't think of when I would want to query the static constants: when given the choice of both I would always choose the runtime obtained metadata; it will be more accurate if I am trying to do something with that value e.g. make a calculation.

I didn't consider metadata validation when I looked at this and haven't looked properly at your PR yet so I can't make an informed comment about how that fits in there.

@sander2
Copy link
Contributor

sander2 commented Mar 30, 2022

In general I can't think of when I would want to query the static constants: when given the choice of both I would always choose the runtime obtained metadata; it will be more accurate if I am trying to do something with that value e.g. make a calculation.

We use different metadatas for different chains, so in our case using the dynamic query wouldn't add anything, and would indeed be a little bit less convenient to use: there is a performance impact and the calling function needs to become async. Of course, we could easily just fetch these constants on startup and then cache them, so it's not a huge issue.

@ascjones
Copy link
Member Author

ascjones commented Mar 30, 2022

See my PR #494, it actually reads it from the already fetched at initialization metadata, so doesn't need to be async and shouldn't any slower since it still performs the decode from already in memory data.

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 a pull request may close this issue.

3 participants