Closed bufke closed 6 years ago
Yes, yes, yes, type definitions using flow and/or typescript would be really useful!
Here is a preview of it.
I'm not familiar with Buffer as seen in the readme Buffer.from
.
let nonce = sodium.randombytes_buf(sodium.crypto_box_NONCEBYTES);
console.log(nonce);
console.log(Buffer.from(nonce));
Both console.log as Uint8Array. I set type DefaultBinary = Uint8Array;
and am using that in many places. Does that sound right?
Are there any "private" functions that shouldn't be included? Here is a raw generated libsodium.d.ts file. I'm just copying from there and filling in any's as much as I can. I am manually adding in constants as they don't appear in my generated file.
Looks pretty good!
Here's something. I have all the functions and constants defined for sumo. There are still lots of any's. Do you want to include the type defs in this repo and npm package? That advantage of that is the types will "just work" when someone is using typescript. The disadvantage is probably that you'd be taking more ownership since it's coming from the official repo here. An alternative would be to publish a @types/libsodium.js npm package that contains just the types.
todos:
Should I open a pull request or try to submit to @types/libsodium.js
(the later doesn't require you do to anything).
To add on to this, I've been using a custom type definition for the subset of libsodium that my code depends on for a while. May or may not be helpful here: https://gist.github.com/buu700/039601a7b410474f0edba9977a8c6393
Your general structure is probably a bit more idiomatic, but maybe some of what's in mine can help fill in the any
s. Also, crypto_stream_chacha20
is actually only in this because my project adds it in a custom build, so you can ignore that unless it gets added to libsodium.js's default build at some point.
I updated them. I could probably guess on a lot of the any's but just putting in what I confirmed for myself and your additions. The output format could be updated to be more consistent.
@bufke I'd like to contribute to your work but unfortunately your project visibility is set to private:
404
The page could not be found or you don't have permission to view it.
ah I moved the repo. It's here.
I could make it it's own project if you'd like.
Also it needs updated for the 0.7.x release
I could make it it's own project if you'd like.
Yes, that would be great! I can start working on the 0.7.x release then.
https://gitlab.com/burke-software/libsodium.js-types
Open to ideas on project structure. Ideally it would have some form of test and either get merged into this project or @types on npm. The tests I've seen for type defs are just using the code and being able to compile without errors. I'm unfortunately not familiar with how to submit to @types.
Hey @bufke! Is there any update on the type definitions? Our cryptography implementation at Wire is strongly based on libsodium and as avid TypeScript coders we would welcome type definitions a lot. π€
To publish your definitions you could use a tool like Microsoft's types-publisher. Microsoft also provides guidelines on how to publish for @types.
Happy to see that you are working on it!
Best, Benny
Hello @bennyn this is still something I want to do - just time always is limited. Thanks for the links. I use wire actually so glad to be of any assistance.
I wonder - do you care about supporting the OutputFormat param? It makes the type defs a bit harder. It should be possible to support it using generics - I just need to figure out how to actually do that. Ideally passing the param like "base64" would let typescript infer the return will be string
. While leaving it default would be Uint8Array
Alternatively we could just default Uint8Array and document that if you use output format to pass in the return type generic like doSomething<string>(etc)
.
Without handling the output format param - I can't tell what the function should return or I have to make a possibly wrong assumption or return a union of types that will probably confuse people more than anything else.
outputFormat
is an unusual and confusing hack. You can probably stick to a single return type.
On mobile so I can't test this right now, but I'm pretty sure you can do something like this:
balls (input: Uint8Array, outputFormat?: 'uint8array') : Uint8Array;
balls (input: Uint8Array, outputFormat: 'base64'|'hex'|'text') : string;
I use wire actually so glad to be of any assistance.
Very pleased to hear that you are using Wire. π And even happier about the fact that you like to help the crypto community! This is just great!
I wonder - do you care about supporting the OutputFormat param?
Answering your question, we are not making use of the OutputFormat
. Here are some exemplary calls which we make with sodium
:
const cipherText = sodium.crypto_stream_chacha20_xor(plainText, typedNonce, typedKeyMaterial, 'uint8array');
const cipherTextInHex = sodium.to_hex(cipherText);
const aliceKeyPair = sodium.crypto_sign_keypair();
const curve25519_pk = sodium.crypto_sign_ed25519_pk_to_curve25519(aliceKeyPair.publicKey);
const curve25519_sk = sodium.crypto_sign_ed25519_sk_to_curve25519(aliceKeyPair.privateKey);
Can someone review this before I mimic it more?
type DefaultBinary = Uint8Array;
type Uint8ArrayOutputFormat = 'uint8array';
type StringOutputFormat = 'text' | 'hex' | 'base64';
function crypto_pwhash(keyLength: number, password: string, salt: DefaultBinary, opsLimit: number, memLimit: number, algorithm: number, outputFormat?: Uint8ArrayOutputFormat): DefaultBinary;
function crypto_pwhash(keyLength: number, password: string, salt: DefaultBinary, opsLimit: number, memLimit: number, algorithm: number, outputFormat: StringOutputFormat): string;
Test code
let hash: Uint8Array = sodium.crypto_pwhash(sodium.crypto_box_SEEDBYTES, password, salt, OPSLIMIT, MEMLIMIT, ALG);
hash = sodium.crypto_pwhash(sodium.crypto_box_SEEDBYTES, password, salt, OPSLIMIT, MEMLIMIT, ALG, 'uint8array');
const hashString: string = sodium.crypto_pwhash(sodium.crypto_box_SEEDBYTES, password, salt, OPSLIMIT, MEMLIMIT, ALG, 'text');
@bufke Thanks for pushing this forward!
We have a libsodium.js performance benchmark. The code of this benchmark can be easily converted to TypeScript to test the compatibility with your type definitions.
If time allows, we can kick-off a test already next week. π
Thanks for your work on the typings @bufke! At Wire we are in the progress of rewriting most of our libraries to TypeScript and thus rely strongly on typings for libsodium.js
. Since programmers are lazy, instead of maintaining the typings for libsodium, I wrote a type generator (which uses libsodium's symbol files to create the typings) to make things easier. Of course this is quite hacky because the symbol files are not streamlined and sometimes really unclear - and e.g. for now the interfaces are just hardcoded and not generated from libsodium's symbols.
And I was able to understand libsodium's symbols only with your help, so thanks! :slightly_smiling_face:
It would be awesome if we could work together with @jedisct1 to make parsing the functions easier (by streamlining the symbol files for example).
Anyway, feel free to fork and improve the tool - also looking forward to any kind of feedback! :+1:
As an example, here is the latest generated libsodium.d.ts
for the sumo version:
Hi @jedisct1 and @bufke, did you have a chance to test out my definitions file?
At Wire, we are using it already at wire-web-proteus and it works pretty well so far. Nevertheless, I'd like to read a second review before I publish it to DefinitelyTyped.
@ffflorian in libsodium.d.ts .ready
is a function instead of a simple Promise
Nice work! One other comment: it'd be convenient if all the union types that include null
also included undefined
, as that could cause headaches with strict null checks.
Along the same lines, doesn't matter much to me, but it may also be worth making null
a possible type of all the optional arguments β e.g. outputFormat?: Uint8ArrayOutputFormat
-> outputFormat?: Uint8ArrayOutputFormat|null
.
@webmaster128 You are right, thanks. I fixed it in https://github.com/ffflorian/libsodium-type-generator/commit/96afcec2f9cc8606366afb0c0eb583a1437b9d42.
@buu700 Thanks for the hints. Unfortunately I can't change a function like
function crypto_generichash_init(key: Uint8Array | null, hash_length: number): state_address;
to
function crypto_generichash_init(key?: Uint8Array | null, hash_length: number): state_address;
because hash_length
is required and a required argument can't follow an optional argument.
I fixed this by making key
mandatory but allowing it to be null
in https://github.com/ffflorian/libsodium-type-generator/compare/96afcec2f9cc8606366afb0c0eb583a1437b9d42...c3f97b2bc8f8d00a5661e2d4febbb535b1750413.
Please see version 1.0.9 for the updated declaration file.
I know a required argument can't follow an optional one; what I was suggesting was to explicitly add undefined
as an accepted input type. In that example, it would look like:
function crypto_generichash_init(key: Uint8Array | null | undefined, hash_length: number): state_address;
@buu700 Oh, of course. Thanks! Fixed in https://github.com/ffflorian/libsodium-type-generator/commit/24cfd62eddf6877335f8fc3132d5750a53a03681 and updated the declaration file in v1.0.10.
Awesome, thanks for the quick fix!
@ffflorian I was able to drop in your generated d.ts to my project and it works without any TS errors. Thanks, great work!
I haven't tested this myself yet, but just noticed something else: shouldn't all the instances of outputFormat?: StringOutputFormat
actually be required arguments (since calling a function with the output unspecified returns a byte array)?
Slightly more up to date type defs: https://gitlab.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/types/emberclear/libsodium.d.ts
@NullVoxPopuli Did you really check @ffflorian's latest work here https://github.com/ffflorian/libsodium-type-generator/releases? He does an incredibly great job with the types and responses very quickly and competent to the few edge case issues that may be left. You might want to contribute to his repo for the benefit of everyone
Speaking of which, is that ready for general use yet? And are there any plans to integrate it into either the main libsodium packages or separate @types packages, @ffflorian and @jedisct1?
I am happily using @ffflorian's types in two different projects by manually downloading the definition file from the release page into my project. The generator itself is not needed by users.
Awesome, that's great to hear.
@buu700 the types are - in my opinion - ready to use, but I didn't really find a good solution yet for the sumo version to include only the types for the extra functions.
Sooner or later I will create a PR here or @types, depending on how @jedisct1 sees it.
As in a way of having in the generated libsodium-wrappers-sumo
definition extend libsodium-wrappers
?
Either way is probably fine IMO since it's just generated code anyway, but from a quick test it should work if you explicitly add export
to everything (e.g. export const SODIUM_LIBRARY_VERSION_MAJOR: number;
) and then add export * from 'libsodium-wrappers';
to libsodium-wrappers-sumo
.
@buu700 Thank you for your input, I'll try adding it in the next days.
the types don't define a sodium method, so I had to do this:
@NullVoxPopuli your import statement imports a default export, but there is no default export in the module.
What you want to do is either import * as libsodiumWrapper from 'libsodium-wrappers'
or import sodium = require("libsodium-wrappers");
. I use the later because I was having problems with the first one (maybe related to https://github.com/jedisct1/libsodium.js/issues/148).
@webmaster128 I'm currently using broccoli for my js importer, so I've assigned everything to a default export. So, my typing issue was somewhat on me. lol
Should we close this at this point? Any issues with types could just be their own issue. I've been using the types for many months without issue.
I'd say so.
Type definitions can help noobs like myself understand the library while also catching type related errors. I'm starting some type defs. If anyone else is interested I'm happy to share and collaborate.
Sample of what I was thinking. Modify libsodium.js so that tsc can read it (basically remove the top header section. Generate the functions (that have mostly
any
). Manually make types and start replacing those any's. One challenge is supporting the outputFormat param that causes the type of the return to change.I'm happy both contributing the d.ts file to libsodium.js directly or just maintaining it separately myself.