Decode runtime metadata #162
Conversation
packages/api-format/src/codec/U16.ts
Outdated
|
|
||
| import BaseNumber from './base/Number'; | ||
|
|
||
| export default class U16 extends BaseNumber { |
There was a problem hiding this comment.
Only have u16 implemented here since it is all we need for this specific one. (u8, u32, u64, u128 is basically exactly the same as this one, just with the bitlength updated)
Should probablybring those in, however wanted to keep this specific to this specific need.
| constructor () { | ||
| super({ | ||
| name: String, | ||
| arguments: class extends CodecArray<String> { |
There was a problem hiding this comment.
This approach looks funny, however have kept strictly the the structures defined in the Rust codebase. So instead of extra classes, use them inline for arrays and (some) structures. Naming of each class here maps 1-to-1 with Rust.
| import String from './String'; | ||
| import U16 from './U16'; | ||
|
|
||
| class EventMetadata extends CodecStruct { |
There was a problem hiding this comment.
This file you probably want to read from the bottom-up, i.e. the RuntimeMetadata is exported right at the end and building the structures using the other classes in here.
|
I need to add the comments in this one, i.e. thinking behind stuff. (Added some comments in the PR, but should probably just expand specifically in the base classes). I think it is a start that will solve the short-term goals, but do believe there is an lot of room for improvement here. |
|
|
||
| export default class StringCompact extends String { | ||
| constructor (value: string = '') { | ||
| super(value, LengthCompact); |
There was a problem hiding this comment.
Same a string, however using the new compact length encoding - this should go in in the next day or so and then this should be the default. Same with Arrays.
| return this; | ||
| } | ||
|
|
||
| map <O> (fn: (entry: T, index?: number) => O): Array<O> { |
There was a problem hiding this comment.
We implement the base Array functions, length, map, filter, reduce, forEach so this class can be used like an array. So our own Array-like. (Probably need some more, so it can be used an an iterator)
|
|
||
| export default class CodecArrayCompact <T extends Base<any>> extends BaseArray<T> { | ||
| constructor (Type: { new(): T }, value: Array<T> = [] as Array<T>) { | ||
| super(Type, value, LengthCompact); |
There was a problem hiding this comment.
As per StringCompact comment, this will become the default.
|
|
||
| import { Base } from '../types'; | ||
|
|
||
| export default class CodecEnum implements Base<number> { |
There was a problem hiding this comment.
This takes a byte and maps it to a number. We actually really should do a TS-like enum impl for these. Not sure how that looks though. Probably pass enum as type and then do some magic?
|
|
||
| import { Base } from '../types'; | ||
|
|
||
| export default class CodecEnumType implements Base<Base<any>> { |
There was a problem hiding this comment.
This is something that based on an enum, it decodes to a different type. The same comments on the enum applies, but it really should probably actually include enum instead of re-doing the logic. (composition?)
|
|
||
| import LengthCompact from './LengthCompact'; | ||
|
|
||
| describe('Compact', () => { |
There was a problem hiding this comment.
TODO. Port the Rust tests...
|
|
||
| import { Base } from '../types'; | ||
|
|
||
| export default class CodecOption implements Base<Base<any> | null> { |
There was a problem hiding this comment.
Based on the length of the first byte, it determines if something follows or not. If 0, nothing follows, if 1, we have actual data f a specific type.
| const metaDecode = (input: Array<number>) => { | ||
| const metadata = new Metadata(); | ||
|
|
||
| try { |
There was a problem hiding this comment.
try/catch block here for coding debugging. To be removed.
| }); | ||
| }); | ||
|
|
||
| it('retrieves the wasm metadata', () => { |
| } | ||
|
|
||
| byteLength (): number { | ||
| return 4; |
| return bnToHex(this.raw, this.bitLength); | ||
| } | ||
|
|
||
| toBn (): BN { |
There was a problem hiding this comment.
We need some functions get the get values out, toBn and toNumber does that. (We don't really want to encourage access to raw directly, it should be moved to _raw and made at the very least protected)
| ); | ||
| } | ||
|
|
||
| static with <O extends Base<any>> (Type: { new(value?: any): O }): { new(value?: any): CodecArray<O> } { |
There was a problem hiding this comment.
Shortcut function so we can have less noise when actually constructing these inline
| this.raw = value; | ||
| } | ||
|
|
||
| byteLength (): number { |
There was a problem hiding this comment.
As with Arrays, we don't have length, in any of the classes. (Well, we do have in String and Array). byteLength has the total length including the length of the prefixes, where length is just the number of elements. (So in coding use byteLength, when working with data, just a straight length is fine)
| arguments: CodecArray<String>, | ||
| documentation: CodecArray<String> | ||
| }> { | ||
| constructor () { |
There was a problem hiding this comment.
I really tried to not have to pass through the type and a structure that defines the classes (that will be used upon construction). I failed at my first attempt. So there is room for improvement, since what gets passed in the constructor is the same (more-or-less) as the generic type.
There was a problem hiding this comment.
Somewhere there must be a solution between key and lookup types, to not pass the type in, but just the struct in the constructor - https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#keyof-and-lookup-types
| constructor (value?: any) { | ||
| super({ | ||
| outerEvent: OuterEventMetadata, | ||
| modules: CodecArray.with(RuntimeModuleMetadata) |
There was a problem hiding this comment.
CodecArray.with are helper functions that just makes it simpler to define an array (and friends) in-place without having to do yet another class definition for a nested type.
| "modifier": 0, | ||
| "type": { | ||
| "key": "PropIndex", | ||
| "value": "(T::Balance, Vec<T::AccountId>)" |
There was a problem hiding this comment.
This effective translates to -
{
balance: Balance,
vecAccountId: Array<AccountId>
}
or rather as a CodecStruct -
extends class CodecStruct<{
balance: Balance,
vecAccountId: CodecArray<AccountId>
}> {
constructor (value?: any) {
super({
balance: Balance,
vecAccountId: CodecArray.with(AccountId)
});
}
}
And sadly, looking at the above it is pretty straight forward to generate, however... since we stll have the issue earlier with the getters not being automagically defined, it is a bit of an issue. Definition is one thing, but we really don't want people to go instance.raw.balance.toNumber() when they can do instance.balance.toNumber() - if no other way, maybe trim raw to something like v, i.e. shorter.
Dunno, just thinking through.
| { | ||
| "name": "LaunchPeriod", | ||
| "modifier": 2, | ||
| "type": "T::BlockNumber", |
There was a problem hiding this comment.
These T::Type stuff is driving me a bit batty. The Rust basically bleeds through. I'm in favour of stripping any T:: stuff and just not having it at all. (Especially with events not having them and even having things duplicated as T::. And then even in the modules, there are some without T, and really T::AccountId === AccountId for the API purposes)
| "type": { | ||
| "key": "(ReferendumIndex, T::AccountId)", | ||
| "value": "bool" | ||
| }, |
There was a problem hiding this comment.
Where these type: { key: ..., value: ... } are found, it is actually a Map. so key becomes the param that is passed, and value is the result type, i.e. type.
There was a problem hiding this comment.
Oooh, and I picked an interesting one - the map key is actually a struct as well (it is ok, when we call it gets encoded, so ends up just being a u8a)
| this._Type = Type; | ||
| } | ||
|
|
||
| static with <O extends CodecBase> (Type: { new(value?: any): O }): { new(value?: any): CodecArray<O> } { |
There was a problem hiding this comment.
OK, didn't understand the DecodeDifferent on the rust side, but this syntax is clear.
| // i.e. while it wraps an array, it provides a `length` property to get the actual length, `at(index)` | ||
| // to retrieve a specific item. Additionally the helper functions `map`, `filter`, `forEach` and | ||
| // `reduce` is exposed on the interface. | ||
| export default class CodecArray < |
There was a problem hiding this comment.
Can this extend Array and remove this.raw?
There was a problem hiding this comment.
I would love that. No idea how to implement it though. (It would make it much easier since we won't need to do the map, filter, reduce, etc.
| return this.raw.forEach(fn); | ||
| } | ||
|
|
||
| fromJSON (input: any): CodecArray<T> { |
There was a problem hiding this comment.
Really small detail about syntax:
Uint8Array.from(...)-> returnsnew Uint8Array()Array.from(...)-> returnsnew Array()
So the natural (for me) syntax is to have a static method here that returns a new instance.
There was a problem hiding this comment.
Agreed actually. However the reason I went down this route is twofold -
-
For the stuff where we need to know the actual type, it is probably easier constructing with the class and then where we from it can be used.
-
... erm, I started with it that way around. However TS basically doesn't allow statics in interfaces (where I started) and it became one big hack to try and work around this.
Not 100% crazy about the above, happy to see it changed.
| // | ||
| // TODO | ||
| // - This could probably be abstract, as long as we have the functions abstract as well | ||
| export default class CodecBase <T = any> { |
There was a problem hiding this comment.
Yeap, needs to be done as per the TODO.
|
Related to #161 |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fails on CI since e2e is active. Currently being tested with e2e -
yarn test packages/api-provider/test/e2e.basics.test.jsWhile it decodes fully, there are a couple of issues with the implementation as it stands from a usability perspective.