feat: support account link pallet update#557
Conversation
packages/types/src/DidDetails.ts
Outdated
| signature: string | ||
| } | ||
|
|
||
| export type JsonEnum<K extends string, V> = K extends any |
There was a problem hiding this comment.
This looks just like Record. What does it do differently and is it important enough to have a new type for it? Because it makes things more confusing.
There was a problem hiding this comment.
ok, I played around with a bit and Record would not work here.
The only solution I could come up with is:
export type JsonEnum<K extends string, V> = {
[P in K]?: V
}
Which I like more, because it is more straightforward and understandable. Only downside is, that it allows empty objects, but I don't see a way around that. On the other side,makeJsonEnum would not be needed anymore, which currently uses type assertion with as and is needed, because it would not be possible to assign to the JsonEnum type without it.
There was a problem hiding this comment.
AFAIK it's the only way to turn 'a' | 'b' into something like {a: any} | {b: any}, which is the sort of type that polkadot requires for the creation of enums. What you proposed gives you {a?: any, b?: any} which besides empty objects also allows {a: 1, b: 2}. And it is not assignable to the polkadot type, so I don't think that will work.
There was a problem hiding this comment.
Another way is to list the types explicitly: {a: any} | {b: any}. It is wordier and longer (a downside) but also explicit (an upside).
There was a problem hiding this comment.
but what does that solve? if variable a is typed as a union of string literals, typescript interprets {[a]: any} as {[key: string]: any} and will not accept it for a signature that requires {a: any} | {b: any}
I can see that! I hope we can develop a decent concept though for abstracting over the different storage versions, because if we ever want to query old state, we would need that again. |
|
Seeing that there are efforts to find better ways to deal with polkadot/api's requirements for enum encoding, I'm merging this now so we have a better basis on which to continue discussions - because this feature makes quite some use of this pattern too, and has different requirements with regards to key type unions. |
3 participants
fixes KILTProtocol/ticket#2048
This adds discovery of whether the account linking pallet has updated to support ethereum alongside substrate addresses.
Extrinsic and query parameter encoding as well as decoding of results will adapt automatically.
How to test:
As long as KILTprotocol/kilt-node#355 is not yet merged, testing will have to be done against an image built from source on that branch.
Tests for ethereum address linking have therefore been set up to be skipped.
Integration tests against latest & latest-develop images should nevertheless still work.
Checklist: