-
Notifications
You must be signed in to change notification settings - Fork 248
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
wip: Remove subxt Config associated types by relying on enriched metadata (v16) #1566
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
// Present in metadata but this PoC needs to add | ||
// fn specific per name of type to impl the hashing fn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thought I had for this sort of thing was originally going to be more like:
type Hasher = DynamicHasher
And then this hasher type (DynamicHasher
here) is given the metadata when it tries to hash something, and can inspect the name of the hashing function and do the correct hashing logic itself.
In either case though, you need it to line up with the type Hash
I think, which would be interesting/messy to do. It might be that we can infer and generate the Hash
type based on what we see that the Hasher
is though, rather than trying to extract it from the metadata. So maybe you'd end up with:
// DynamicHasher would spit out hashes like this, and these would encode
// to their inner bytes only (ignoring the variant byte):
enum DynamicHash {
// an enum that supports the common hash types we know about
BlakeTwo256([u8; 32]),
TwoX128([u8; 16])
}
// ...
type Hash = DynamicHash;
type Hasher = DynamicHasher;
But we could theoretically do it all with codegen instead and extend the Subxt macro so that we can provide the necessary config types or override existing ones, but by default it will codegen relevant types like Hasher
and Hash
based on the metadata!
This would have the advantage of being faster/more correct and the disadvantage of needing more different Config
impls for different chains rather than potentially being able to rely on one Config
impl for multiple chains (or customising it for one chain).
Something to ponder :)
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Updated the PoC to use build on top of the latest PRs:
I'll still need to think about the dynamic hashers/types, wanted so far to check that everything is exposed from the substrate side |
I haven't looked at this yet but just wanted to leave this thought form yesterdays call before I forget: It might be nice to have both:
This combo would allow for things to "just work" in the ismple case using I think at the moment, if we just did one of those, I'd vote for (2) ie make |
This PR exposes the custom types present in the enriched metadata (possibly the next V16).
This stands as a PoC to pave the way towards the V16 and check the viability of exposing chain specific types to reduce the amount of configuration subxt users need to handle.
This aims to make subxt more robust and ideally allow users to communicate to any chain without relying on preset code configs. At the same time, this reduces the burden on developers to add said preset code configs and instead relies on the metadata to expose the appropriate types.
This metadata PoC exposes 7 out of 8 configuration parameters needed by subxt to communicate with chains.
Hasher
Header
andSignature
need extra functionality on the subxt::codegen sidetype Hasher = Blake2Hasher
can be deduced to be blake2 and therefore the said functionality introduced at compile-time via codegen)ExtrinsicParams
needs further investigation since the metadata cannot safely export code functionalityCustom Types Exposed
This is what the codegen will produce after inspecting the metadata:
Metadata Config Example
Running `metadata_config` Balance transfer success: Transfer { from: AccountId32([212, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26, 189, 4, 169, 159, 214, 130, 44, 133, 88, 133, 76, 205, 227, 154, 86, 132, 231, 165, 109, 162, 125]), to: AccountId32([142, 175, 4, 21, 22, 135, 115, 99, 38, 201, 254, 161, 126, 37, 252, 82, 135, 97, 54, 147, 201, 18, 144, 156, 178, 38, 170, 71, 148, 242, 106, 72]), amount: 10000 }