-
Notifications
You must be signed in to change notification settings - Fork 347
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
Rewrite type-extrinsics #173
Conversation
000b247
to
015cfda
Compare
3c7d950
to
927e3c0
Compare
Related #161 |
927e3c0
to
39bf5e6
Compare
39bf5e6
to
94bcdbf
Compare
b270e27
to
689b9d1
Compare
expect(newExtrinsics.timestamp.set([10101]).toU8a()).toEqual( | ||
new Uint8Array([ | ||
// index | ||
// 3, 0, // FIXME This was taken from the old tests, should work, but doesn't |
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.
It is length encoded. The 40 is the length. Passing isBare = true should give the result you expect.
|
||
return new Extrinsic( | ||
u8aConcat( | ||
new Uint8Array([index, meta.id.toU8a()[1]]), |
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.
Not sure if that's the cleanest way
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.
Well, we get to 3,0... so... Just not sure this is u16 (not sure why it is not a u8) and the toU8a() encodes to little-endian, so the value we are actually interested in is in position 0?
i.e.
0x0004
-> Uint8Array([4, 0])
I would actually just toNumber()
it there. It is supposed to fit in a byte, if it goes over 255 we have bigger issues.
...expectedArgs.map((argument, index) => { | ||
const type = argument.type.toString(); // Argument type, as string | ||
|
||
return createType(type, args[index]).toU8a(true); |
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.
This should be encoded? In the test you to toU8a(true)
, this should be ready for submittion.
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.
Once we have the encoding/signing in-place, we can actually test this e2e, we have the test keyring, we can send stuff, Alice is nicely operational on a local network.
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. |
API:
extrinsics.timestamp.set(...)
-> Returns correctly encodedExtrinsic
.TODO:
extrinsics.balances.set_balance
->extrinsics.balances.setBalance
In another PR:
Notes:
type-primitives
codec/
andlegacy
folders should be removed asap