{"action":"created","issue":{"url":"https://api.github.com/repos/nim-lang/rfcs/issues/38","repository_url":"https://api.github.com/repos/nim-lang/rfcs","labels_url":"https://api.github.com/repos/nim-lang/rfcs/issues/38/labels{/name}","comments_url":"https://api.github.com/repos/nim-lang/rfcs/issues/38/comments","events_url":"https://api.github.com/repos/nim-lang/rfcs/issues/38/events","html_url":"https://github.com/nim-lang/rfcs/issues/38","id":370167192,"node_id":"MDU6SXNzdWUzNzAxNjcxOTI=","number":38,"title":"RFC: Standardize API of all crypto/hashes packages  ","user":{"login":"narimiran","id":16516130,"node_id":"MDQ6VXNlcjE2NTE2MTMw","avatar_url":"https://avatars2.githubusercontent.com/u/16516130?v=4","gravatar_id":"","url":"https://api.github.com/users/narimiran","html_url":"https://github.com/narimiran","followers_url":"https://api.github.com/users/narimiran/followers","following_url":"https://api.github.com/users/narimiran/following{/other_user}","gists_url":"https://api.github.com/users/narimiran/gists{/gist_id}","starred_url":"https://api.github.com/users/narimiran/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/narimiran/subscriptions","organizations_url":"https://api.github.com/users/narimiran/orgs","repos_url":"https://api.github.com/users/narimiran/repos","events_url":"https://api.github.com/users/narimiran/events{/privacy}","received_events_url":"https://api.github.com/users/narimiran/received_events","type":"User","site_admin":false},"labels":[{"id":1091665951,"node_id":"MDU6TGFiZWwxMDkxNjY1OTUx","url":"https://api.github.com/repos/nim-lang/rfcs/labels/RFC","name":"RFC","color":"1d1d1d","default":false},{"id":1091677081,"node_id":"MDU6TGFiZWwxMDkxNjc3MDgx","url":"https://api.github.com/repos/nim-lang/rfcs/labels/Stdlib","name":"Stdlib","color":"eb6420","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-10-15T13:37:00Z","updated_at":"2018-10-15T13:37:03Z","closed_at":null,"author_association":"COLLABORATOR","body":"<a href=\"https://github.com/mratsim\"><img src=\"https://avatars3.githubusercontent.com/u/22738317?v=4\" align=\"left\" width=\"96\" height=\"96\" hspace=\"10\"></img></a> **Issue by [mratsim](https://github.com/mratsim)**\n_Thursday Mar 15, 2018 at 12:33 GMT_\n_Originally opened as https://github.com/nim-lang/Nim/issues/7337_\n\n----\n\nAs we're working with crypto at Status, we regularly get bitten by the different types in Nim libraries to represent binary data.\r\n\r\n# Context:\r\n\r\nCrypto and hashing libraries in the wild uses arrays or seq of uint8, char, byte, object, uint32, cuchar and also strings to represent the binary hashes. I have noted the following issues:\r\n\r\nA. **Output hash** type is inconsistent:\r\n  - array[N, char]:\r\n    - [nimSHA2](https://github.com/jangko/nimSHA2/blob/210db67d942b5fbefefb0dc2886542f7d97debc5/nimSHA2.nim#L105-L108)\r\n  - array[N, uint8]:\r\n    - [md5](https://nim-lang.org/docs/md5.html)\r\n  - object-wrapped array[bits div 8, uint8] (number of bits is part of the type):\r\n    - [keccak_tiny](https://github.com/status-im/nim-keccak-tiny/blob/394d9f4ea6cb2a36e8defda93d5b349a356591e1/keccak_tiny.nim#L8) (Status)\r\n    - [nimcrypto](https://github.com/cheatfate/nimcrypto/blob/64543272aeae07d021c7819f8c2803ea087fc128/hash.nim#L8) (Status)\r\n  - array[N, byte]:\r\n    - [Eth-keys](https://github.com/status-im/nim-eth-keys/blob/36fe3e56b46ae2547faa62a4f8bdf4067315e497/src/private/lowlevel_types.nim#L22) (Status)\r\n  - distinct array[N, uint32]:\r\n    - [SHA1](https://nim-lang.org/docs/sha1.html)\r\n  - string\r\n    - [libsodium](https://github.com/FedericoCeratto/nim-libsodium/blob/0e2de3f9b97a0624703b6d6bb8a07035f94b63a4/libsodium/sodium.nim#L169-L186)\r\n  - ptr cuchar + len\r\n    - [nimssl](https://github.com/genotrance/nimssl/blob/13b6f7e3dcc2fb9d99207c1bf25ac2808b464b61/tests/shatest.nim#L5-L6) (bindings autogenerated on install so not visible on Github)\r\n\r\nB. 0.18 introduction of **`$` for arrays** broke libraries:\r\n  - https://github.com/jangko/nimSHA2/issues/2\r\n  - [Araq on IRC](https://irclogs.nim-lang.org/15-03-2018.html#10:47:21)\r\n\r\nC. **Input type** is inconsistent:\r\n  - a string (binary buffer)\r\n    - [nimSHA2](https://github.com/jangko/nimSHA2/blob/210db67d942b5fbefefb0dc2886542f7d97debc5/nimSHA2.nim#L383-L386)\r\n    - [libsodium](https://github.com/FedericoCeratto/nim-libsodium/blob/0e2de3f9b97a0624703b6d6bb8a07035f94b63a4/libsodium/sodium.nim#L321-L332)\r\n  - a hex string or a binary buffer (any kind: string, arrays, sequences)\r\n    - [keccak_tiny](https://github.com/status-im/nim-keccak-tiny/blob/394d9f4ea6cb2a36e8defda93d5b349a356591e1/keccak_tiny.nim#L41-L45) (Status) and\r\n      its dependency [memRange](https://github.com/status-im/nim-ranges/blob/master/ranges/memranges.nim) package\r\n  - a hex string\r\n    - [eth_keys](https://github.com/status-im/nim-eth-keys/blob/36fe3e56b46ae2547faa62a4f8bdf4067315e497/src/datatypes_interface.nim#L30-L32) (Status)\r\n  - ptr cuchar + len\r\n    - [nimssl](https://github.com/genotrance/nimssl/blob/13b6f7e3dcc2fb9d99207c1bf25ac2808b464b61/tests/shatest.nim#L5-L6) (bindings autogenerated on install so not visible on Github)\r\n  - ptr uint8 + len\r\n    - [nimcrypto](https://github.com/cheatfate/nimcrypto/blob/c8ed7a87eade1b5741c2d4f211a6e057993fa71d/hash.nim#L17-L23). Further overloads are coming.\r\n\r\nD. **`$` Conversion of the hash type** is inconsistent between string, hex or binary representation:\r\n  - [nimSHA2](https://github.com/jangko/nimSHA2/blob/210db67d942b5fbefefb0dc2886542f7d97debc5/nimSHA2.nim#L388-L395) `$` on array[N, char] (N = 28, 32, 48, 64 only) returns a binary string\r\n    - every other size returns the string representation of the numbers\r\n  - [libsodium](https://github.com/FedericoCeratto/nim-libsodium/blob/0e2de3f9b97a0624703b6d6bb8a07035f94b63a4/libsodium/sodium.nim#L116-L154) also returns a binary string and requires `bin2hex` for the hex representation.\r\n  - [keccak_tiny](https://github.com/status-im/nim-keccak-tiny/blob/394d9f4ea6cb2a36e8defda93d5b349a356591e1/keccak_tiny.nim#L10-L13) (Status) returns the hex representation\r\n  - [md5](https://github.com/nim-lang/Nim/blob/2a8d2ad4cde1c942a9dbc44598f65d4acf1e0aa6/lib/pure/md5.nim#L214-L220) returns the hex representation\r\n  - [nim-eth-keys](https://github.com/status-im/nim-eth-keys/blob/36fe3e56b46ae2547faa62a4f8bdf4067315e497/src/private/lowlevel_types.nim#L102) (Status) uses the default `$` for array of bytes (returns a string of uint8) and require `toHex` for the hex representation\r\n  - [nimssl](https://github.com/genotrance/nimssl/blob/13b6f7e3dcc2fb9d99207c1bf25ac2808b464b61/tests/shatest.nim#L9-L14) doesn't have `$`(it's a pointer). It requires `toHex` for the hex representation.\r\n\r\nNote that we are currently having this discussion internally, but since Araq mentionned his pain working with hashes following 0.18, I believe the whole Nim ecosystem would benefit.\r\n\r\n# Stakes\r\n\r\n[Nimbus](https://github.com/status-im/nimbus/wiki/Nimbus-Roadmap) (A client for Ethereum 2.0) will bring a lot of attention to Nim crypto ecosystem and it's in our best interest to adopt healthy common standards.\r\nBeyond the internal representations, I think a common set of APIs is what matters\r\n\r\n# Questions and considerations for API:\r\n\r\n**1. \"String\" input**\r\n\r\n`proc hash1234(s: string): Hash = ...`, what should `s` be, a hex string or binary data?\r\n\r\n**2. Variable size input**\r\n\r\nWhich one(s) among\r\n  * `proc hash1234(s: seq[uint8]): Hash = ...`\r\n  * `proc hash1234(s: seq[byte]): Hash = ...`\r\n  * `proc hash1234(s: seq[char]): Hash = ...`\r\n  * `proc hash1234(s: string): Hash = ...`\r\n\r\n**3. `$` behaviour**\r\n\r\nWhich one among:\r\n  * An error\r\n  * The hex representation\r\n  * The uint8/byte numbers\r\n  * Convert to binary blob\r\n\r\n**4. Chaining**\r\n\r\n  `hash1234(hash1234(foo))` should be allowed without casting/conversion\r\n\r\n**5. Hex output**\r\n\r\nWhich one among:\r\n  * `$`\r\n  * `toHex`\r\n\r\n# Opinions\r\n\r\nFor consistency one type between byte/uint8/char should be used.\r\nAt the same time one type between seq[byte]/seq[uint8]/seq[char] and string should be used.\r\n\r\nNote: Those are only my opinions and does not represent the opinion of all Status devs\r\n\r\n**Opinion 1**: `$` is very error prone right now. From an ergonomic point of view, it makes sense to convert binary blob/hashes to their hex representation.\r\n\r\n**Opinion 2**: `$` converting to the string repr of seq[uint8] can be avoided by using `distinct` types or wrapping Hash = array[N, byte|uint8] in an object.\r\n\r\n**Opinion 3**: The internal representation should be **byte**.\r\n\r\nRationale:\r\n- **byte** is for data or binary blob that are read/written\r\n- **uint8** is for numbers (add, multiply, divide, modulo ...)\r\n- **char**  is for text that can be printed.\r\n           \r\nA counterpoint can be made that **byte** (or **char**) hides the actual size of the type.\r\nHowever Windows and POSIX require 1 byte = 8-bit. The only systems with 1 byte != uint8 are DSP chips (they use 1 byte = 12, 16, 24 bit), see [here](https://stackoverflow.com/questions/5516044/system-where-1-byte-8-bit).\r\n\r\n**Opinion 4**: Let's nail it! :fireworks:\r\n\r\nInviting: @jangko, @cheatfate, @Araq, @zah, @yglukhov to the discussion.\r\n\n"},"comment":{"url":"https://api.github.com/repos/nim-lang/rfcs/issues/comments/429855220","html_url":"https://github.com/nim-lang/rfcs/issues/38#issuecomment-429855220","issue_url":"https://api.github.com/repos/nim-lang/rfcs/issues/38","id":429855220,"node_id":"MDEyOklzc3VlQ29tbWVudDQyOTg1NTIyMA==","user":{"login":"narimiran","id":16516130,"node_id":"MDQ6VXNlcjE2NTE2MTMw","avatar_url":"https://avatars2.githubusercontent.com/u/16516130?v=4","gravatar_id":"","url":"https://api.github.com/users/narimiran","html_url":"https://github.com/narimiran","followers_url":"https://api.github.com/users/narimiran/followers","following_url":"https://api.github.com/users/narimiran/following{/other_user}","gists_url":"https://api.github.com/users/narimiran/gists{/gist_id}","starred_url":"https://api.github.com/users/narimiran/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/narimiran/subscriptions","organizations_url":"https://api.github.com/users/narimiran/orgs","repos_url":"https://api.github.com/users/narimiran/repos","events_url":"https://api.github.com/users/narimiran/events{/privacy}","received_events_url":"https://api.github.com/users/narimiran/received_events","type":"User","site_admin":false},"created_at":"2018-10-15T13:37:03Z","updated_at":"2018-10-15T13:37:03Z","author_association":"COLLABORATOR","body":"<a href=\"https://github.com/PMunch\"><img src=\"https://avatars0.githubusercontent.com/u/2845482?v=4\" align=\"left\" width=\"48\" height=\"48\" hspace=\"10\"></img></a> **Comment by [PMunch](https://github.com/PMunch)**\n_Thursday Mar 15, 2018 at 13:03 GMT_\n\n----\n\nYes! This has been annoying me for quite some while now. Another thing that should be taken into consideration is reading of data from a stream and writing to one. Whatever solution this comes to, the corresponding base type should be added to the streams module as well so as to facilitate easy input -> en-/decrypt -> output.\r\n\r\nPersonally I agree with the rationale offered, and while the DSP argument is valid Nim already has things like `int` which can take on multiple sizes. Doing the same for byte makes sense. I could write a crypto procedure that takes `byte8` and the compiler will check that code using `byte` actually has the right sized byte.\n"}}
{"actor":{"display_login":"narimiran"}}