-
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
Add Compact{16,32,64,128,256} #219
Conversation
jacogr
commented
Oct 6, 2018
- Closes Add Compact{8,16,32,64,128,256} coders #197
- Related Rework codecs & extrinsics for poc-3 compatibility #161
} | ||
|
||
fromU8a (input: Uint8Array): Bytes { | ||
const [offset, length] = Compact.decode(input); | ||
const [offset, length] = Compact.decodeU8a(input, DEFAULT_LENGTH_BITS); |
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.
Instead of adding DEFAULT_LENGTH_BITS
as 2nd arg everywhere, you could add a default value for this 2nd arg in the function definition? (you'll probably make codeclimate happy too)
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 was that way originally, but it creates issues and bugs that look correct.
i.e. for numbers, you want the default to probaly be 64, but for lengths it needs to be 32. So made it explicit everywhere. (The issue is not an issue yet, but will be - all numbers will be compact encoded in the system, so want to make sure we pass the param through and not just forget about it.)
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.
OK!
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. |