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

Configurable Config and Extra types #373

Merged
merged 12 commits into from
Jan 6, 2022
Merged

Configurable Config and Extra types #373

merged 12 commits into from
Jan 6, 2022

Conversation

ascjones
Copy link
Contributor

@ascjones ascjones commented Dec 16, 2021

Closes #331

Currently the Config and Extra trait implementations are hardcoded by the macro, which means that if the target runtime has different types or extrinsic signed extensions, then it will not work.

This PR enables the user to supply their own implementations of Config and Extra as parameters to the RuntimeApi type generated by the macro. E.g.

let api = ClientBuilder::new()
        .build()
        .await?
        .to_runtime_api::<runtime::RuntimeApi<subxt::DefaultConfig, subxt::DefaultExtra<subxt::DefaultConfig>>>();

Here we are using the built in default implementations as type parameters, but the user is now able to supply their own implementations in their place.

This may not be the final API, since we also need to consider how we can integrate the initialisation of some of the additional signed data (e.g. transaction payment), which currently have default values set. Also we will probably want to use metadata to have the macro generate the correct set of signed extensions, rather than requiring them to be hardcoded upfront.

However these things can be built upon this foundation, which at least allows the user the ability to customise these previously hardcoded types.

@jsdw jsdw requested review from a team and jsdw January 5, 2022 15:30
Self(account_id)
}
}

pub struct RuntimeApi<T: ::subxt::Config + ::subxt::ExtrinsicExtraData<T>> {
pub struct RuntimeApi<T: ::subxt::Config, E> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth defaulting these generic params eg

pub struct RuntimeApi<T: ::subxt::Config = ::subxt::DefaultConfig, E = ::subxt::DefaultExtra<::subxt::DefaultConfig>> {

so that we don't have to specify them when instantiating the RuntimeApi unless they differ? Or is the common case that you'll need to specify different ones, so it may as well be explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure TBH, on the one hand it is very convenient if you are using the defaults, on the other if they are hidden then there is not the clue that it is configurable.

I'd lean towards leaving it explicit for now, until the api has solidified and we have an idea of the common usage.

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me; basically a ton of threading a new generic param or two through things for that extra flexibility as far as I understand it. Nice one!

@ascjones ascjones merged commit ca5345c into master Jan 6, 2022
@ascjones ascjones deleted the aj-config branch January 6, 2022 09:39
0623forbidden pushed a commit to DEIPworld/substrate-subxt that referenced this pull request Feb 15, 2022
* WIP config

* WIP separate default config

* Separate trait impls on client

* WIP introduce new ApiConfig (to be renamed) trait

* Update generated polkadot codegen

* Allow configuring Config and Extra types independently

* Add extra default configuration

* Revert ir parsing of config attr

* Add default-features = false to substrate deps

* Revert "Add default-features = false to substrate deps"

This reverts commit 099d20c.
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.

Configurable extrinsic Extra
2 participants