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.

Sign up for GitHub

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

Jump to bottom

wip/PoC: Enrich metadata custom types with essential types for chain communications #4358

Closed
wants to merge 24 commits into from
Closed

wip/PoC: Enrich metadata custom types with essential types for chain communications #4358

wants to merge 24 commits into from

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented May 2, 2024

This PR enriches the metadata's custom types via captured types needed by users to communicate with the chain.
Ideally, this will naturally evolve into a strongly typed metadata V16.

Subxt relies on a preset called Config to interact with the chain.
This Config present contains types such as AccountId, Hasher and more needed to craft transactions.
Subxt has implemented a SubstrateConfig and PolkadotConfig.

This is the subxt config that can be almost entirely replaced by metadata types.

The end goal of this PoC is to remove as many types needed by those configs and rely on the metadata provided information instead.

For more details, see paritytech/subxt#1566.

lexnv added 11 commits May 1, 2024 15:24
@lexnv
metadata/ir: Extend IR with custom types
f0ea720 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
metadata/ir: Convert custom IR types to concrete metadata
fecdd47 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Populate custom types with runtime call of system config
1279594 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Expose AccountId to metadata
ab4f6cc 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Optionally expose the AssetId of the asset pallet
e7bee31 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Use runtime instance for extracting AssetId type
d0986cf 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Extract address ty
c8061aa 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Extract signature ty
e63dae6 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Extend with Hash and Hashing types
c666a6f 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Extract block header to custom types
fbd75b0 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
frame/metadata: Remove call enum ty from custom types
0fedb4d 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv added A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). R0-silent Changes should not be mentioned in any release notes I5-enhancement An additional feature request. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels May 2, 2024
@lexnv lexnv self-assigned this May 2, 2024
@lexnv lexnv requested a review from a team as a code owner May 2, 2024 12:55
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6111092

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6111093

@jsdw
Copy link
Contributor

jsdw commented May 2, 2024

I'd leave this as a draft for now since we'll prob want to think about and iterate on it a bit etc :)

@jsdw jsdw marked this pull request as draft May 2, 2024 15:16
bkchr
bkchr requested changes May 2, 2024
View reviewed changes
substrate/frame/support/procedural/src/construct_runtime/mod.rs
Comment on lines +229 to +232
/// The fixed name of the ForeignAssets pallet.
const FOREIGN_ASSETS_PALLET_NAME: &str = "ForeignAssets";
/// The fixed name of the Assets pallet.
const ASSETS_PALLET_NAME: &str = "Assets";
Copy link
Member

Choose a reason for hiding this comment

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

This is nothing we want to integrate. Users will have no fucking idea that they should call the pallets this way. These pallets can also have multiple instances. This doesn't makes any sense and we should not start doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that makes sense! Its a bit of a hack to see how far I can get in subxt to remove those config types :D

I think a better approach may be to collect the associated types from pallet's configs. However, I'll have to look how much that will increase the metadata size and if we run into limitations with it (maybe not all types impl TypeInfo for us to expose these), will think about it! Thanks for the review! 🙏

bkchr
bkchr reviewed May 2, 2024
View reviewed changes
substrate/frame/support/procedural/src/construct_runtime/expand/metadata.rs
Comment on lines +171 to +178
(
"Address".into(),
address_ty,
),
(
"Signature".into(),
signature_ty,
),
Copy link
Member

Choose a reason for hiding this comment

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

They already exist in the extrinsic metadata?

lexnv added 7 commits May 16, 2024 19:18
@lexnv lexnv mentioned this pull request May 20, 2024
Closed
@lexnv lexnv mentioned this pull request Jun 5, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 8, 2024
@lexnv @bkchr @gupnik
frame/proc-macro: Refactor code for better readability (#4712)
07cfcf0 
Small refactoring PR to improve the readability of the proc macros.
- small improvement in docs
- use new `let Some(..) else` expression
- removed extra indentations by early returns

Discovered during metadata v16 poc, extracted from:
#4358

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: gupnik <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
@lexnv @bkchr
@gupnik @TarekkMA
frame/proc-macro: Refactor code for better readability (paritytech#4712)
8e6a0e2 
Small refactoring PR to improve the readability of the proc macros.
- small improvement in docs
- use new `let Some(..) else` expression
- removed extra indentations by early returns

Discovered during metadata v16 poc, extracted from:
paritytech#4358

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: gupnik <[email protected]>
@lexnv lexnv mentioned this pull request Aug 7, 2024
Merged
@lexnv
Copy link
Contributor Author

lexnv commented Aug 7, 2024

Superseded by: #5274

@lexnv lexnv closed this Aug 7, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 16, 2024
@lexnv @bkchr
@gui1117 @shawntabrizi
Metadata V16 (unstable): Enrich metadata with associated types of con…
b649f4a 
…fig traits (#5274)

This feature is part of the upcoming metadata V16. The associated types
of the `Config` trait that require the `TypeInfo` or `Parameter` bounds
are included in the metadata of the pallet. The metadata is not yet
exposed to the end-user, however the metadata intermediate
representation (IR) contains these types.

Developers can opt out of metadata collection of the associated types by
specifying `without_metadata` optional attribute to the
`#[pallet::config]`.

Furthermore, the `without_metadata` argument can be used in combination
with the newly added `#[pallet::include_metadata]` attribute to
selectively include only certain associated types in the metadata
collection.

### API Design

- There is nothing to collect from the associated types:

```rust
#[pallet::config]
pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;
	}
```

- Default automatic collection of associated types that require TypeInfo
or Parameter bounds:

```rust
	#[pallet::config]
	pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;

		// Associated type included by default, because it requires TypeInfo bound.
		/// Nonce doc.
		type Nonce: TypeInfo;

		// Associated type included by default, because it requires
		// Parameter bound (indirect TypeInfo).
		type AccountData: Parameter;

		// Associated type without metadata bounds, not included.
		type NotIncluded: From<u8>;
	}
```

- Disable automatic collection

```rust
// Associated types are not collected by default.
	#[pallet::config(without_metadata)]
	pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;

		// Explicitly include associated types.
		#[pallet::include_metadata]
		type Nonce: TypeInfo;

		type AccountData: Parameter;

		type NotIncluded: From<u8>;
	}
```

Builds on top of the PoC:
#4358
Closes: #4519

cc @paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Guillaume Thiolliere <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkchr bkchr bkchr requested changes

@acatangiu acatangiu Awaiting requested review from acatangiu acatangiu will be requested when the pull request is marked ready for review acatangiu is a code owner

Assignees

@lexnv lexnv

Labels
A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request. R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lexnv @paritytech-cicd-pr @jsdw @bkchr