Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

feat: add flow annotations #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having this file (.babelrc) and .flowconfig, could we have these in a separate repository and import here? Would be a lot better, otherwise this is gonna be duplicated everywhere.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we should move these into AEgir

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly .flowconfig isn't js or json so you can't really pull it from elsewhere (unless you're thinking of git submodules which aren't worth the hassle IMO).

As of .babelrc indeed this probably should end up in AEgir.

"env": {
"development": {
"sourceMaps": "inline",
"comments": false,
"presets": [
[
"env",
{
"targets": {
"node": "current"
}
}
],
"flow-node"
]
},
"umd": {
"comments": false,
"presets": [
[
"env",
{
"modules": false,
"targets": {
"browsers": "last 2 versions"
}
}
],
"flow-node"
]
}
}
}
9 changes: 9 additions & 0 deletions .flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[ignore]
.*/node_modules/documentation/*

[libs]

[include]

[options]
suppress_comment= \\(.\\|\n\\)*\\@FlowIgnore
36 changes: 31 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,38 @@
"description": "multiple hash functions",
"main": "src/index.js",
"scripts": {
"lint": "aegir lint",
"build": "aegir build",
"test": "aegir test -t node -t browser -t webworker",
"lint": "lint-staged && npm run type-check",
"type-check": "flow check",
"build": "npm run build:node && BABEL_ENV=umd aegir build",
"test": "npm run build:node && aegir test -t node -t browser -t webworker",
"test:node": "aegir test -t node",
"test:browser": "aegir test -t browser",
"test:webworker": "aegir test -t webworker",
"release": "aegir release -t node -t browser",
"release-minor": "aegir release --type minor -t node -t browser",
"release-major": "aegir release --type major -t node -t browser",
"coverage": "aegir coverage",
"coverage-publish": "aegir coverage --provider coveralls"
"coverage-publish": "aegir coverage --provider coveralls",
"build:types": "flow-copy-source --verbose src lib",
"build:lib": "babel --out-dir lib src",
"build:node": "npm run build:types && npm run build:lib",
"start": "flow-copy-source --watch --verbose src lib & babel --watch --out-dir lib src"
},
"lint-staged": {
"*.js": [
"prettier --no-semi --write",
"git add"
]
},
"pre-commit": [
"lint",
"test"
],
"standard": {
"ignore": [
"dist"
]
},
"repository": {
"type": "git",
"url": "git://github.com/multiformats/js-multihashing.git"
Expand All @@ -33,16 +49,26 @@
"url": "https://github.com/multiformats/js-multihashing/issues"
},
"dependencies": {
"babel-cli": "^6.26.0",
"babel-core": "^6.26.0",
"babel-loader": "^7.1.4",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those 3 are devDependencies aren't they?

"blakejs": "^1.1.0",
"js-sha3": "^0.7.0",
"multihashes": "~0.4.12",
"webcrypto": "~0.1.1"
},
"devDependencies": {
"aegir": "^12.3.0",
"babel-preset-flow-node": "^2.0.1",
"chai": "^4.1.2",
"dirty-chai": "^2.0.1",
"pre-commit": "^1.2.2"
"flow-bin": "^0.69.0",
"flow-copy-source": "^1.3.0",
"lint-staged": "^7.0.2",
"pre-commit": "^1.2.2",
"prettier": "^1.11.1",
"rollup.config.flow": "^1.0.0",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as multiformats/js-multihash#47 (review). No need for rollup I guess.

"source-map-support": "^0.5.4"
},
"homepage": "https://github.com/multiformats/js-multihashing",
"contributors": [
Expand Down
65 changes: 44 additions & 21 deletions src/blake.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,79 @@
'use strict'
// @flow
"use strict"

const blake = require('blakejs')
const minB = 0xb201
const minS = 0xb241
import * as blake from "blakejs"
import type { Hash, HashTable, HashBuilder } from "./types"
import type { Code } from "multihashes/lib/constants"

var blake2b = {
const minB: Code = 0xb201
const minS: Code = 0xb241

type BlakeCtx = {
b: Uint8Array,
h: Uint32Array,
t: number,
c: number,
outlen: number
}
type Blake2Hash = Buffer
type BlakeHasher = {
init(size: number, key: ?number): BlakeCtx,
update(ctx: BlakeCtx, input: Uint8Array): void,
digest(ctx: BlakeCtx): Uint8Array
}

const blake2b: BlakeHasher = {
init: blake.blake2bInit,
update: blake.blake2bUpdate,
digest: blake.blake2bFinal
}

var blake2s = {
blake2b.init(1, 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this init call intended here ? I don't see it in original code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that was a mistake. I was testing the flow checking.


const blake2s: BlakeHasher = {
init: blake.blake2sInit,
update: blake.blake2sUpdate,
digest: blake.blake2sFinal
}

class B2Hash {
constructor (size, hashFunc) {
class B2Hash implements Hash {
ctx: BlakeCtx | null
hf: BlakeHasher

constructor(size, hashFunc) {
this.hf = hashFunc
this.ctx = this.hf.init(size, null)
}

update (buf) {
update(buf: Buffer): Hash {
if (this.ctx === null) {
throw new Error('blake2 context is null. (already called digest?)')
throw new Error("blake2 context is null. (already called digest?)")
}
this.hf.update(this.ctx, buf)
return this
}

digest () {
digest(): Blake2Hash {
const ctx = this.ctx
this.ctx = null
if (ctx === null) {
throw Error("blake2 context is null. (already called digest?)")
}
return Buffer.from(this.hf.digest(ctx))
}
}

function addFuncs (table) {
function mkFunc (size, hashFunc) {
return () => new B2Hash(size, hashFunc)
export const addFuncs = (table: HashTable) => {
const mkFunc = (size: number, hashFunc: BlakeHasher): HashBuilder => {
return (): Hash => new B2Hash(size, hashFunc)
}

var i
// I don't like using any here but the only way I could get the types to work here.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other way to do it would be to add another table in js-multihash which would be {[number]:Code} and then do const code = thatTable[minB + i]. That being said it's probably fine the way it is here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More out of curiosity - what was the problem? Shouldn't Code be just an alias for number?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were opaque type aliases which behave like nominal types. In that module context any number can be treated as Code but outside of that context Code is a subtype of number and only way to get hold of value of that tipe is by getting it from that module either from exposed constant annotated as Code or via function that returns Code. This gives you a way to say have parseCode : number -> ?Code function that can return passed value back but ensuring that it’s a valid Code. Kind of typically you’d have isCode function but such that type checker then enforce typings

let i
for (i = 0; i < 64; i++) {
table[minB + i] = mkFunc(i + 1, blake2b)
table[(minB + i: any)] = mkFunc(i + 1, blake2b)
}
for (i = 0; i < 32; i++) {
table[minS + i] = mkFunc(i + 1, blake2s)
table[(minS + i: any)] = mkFunc(i + 1, blake2s)
}
}

module.exports = {
addFuncs: addFuncs
}
53 changes: 32 additions & 21 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
'use strict'
// @flow
"use strict"

const multihash = require('multihashes')
const blake = require('./blake')
const sha3 = require('./sha3')
const crypto = require('webcrypto')
import * as multihash from "multihashes"
import type { Multihash } from "multihashes"
import type { Name, Code } from "multihashes/lib/constants"
import type { HashTable, Hash } from "./types"
import * as blake from "./blake"
import * as sha3 from "./sha3"
import * as crypto from "webcrypto"

const mh = module.exports = Multihashing
const mh = (module.exports = Multihashing)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest export default Multihashing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I had export default Multihashing but that changes the api of this library for commonjs because instead of doing const multihashing = require('multihashing') a user now have to do const mh = require('multihashing').default which I don't think we want to do with the initial flow conversion.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garrensmith In fact flow-node babel preset takes care of this as it includes add-module-exports plugin only thing to watch out for is that you can either single default export or multiple non default exports (as there is no way around it with common-js style).


mh.Buffer = Buffer // for browser things

function Multihashing (buf, func, length) {
function Multihashing(
buf: Buffer,
func: Name | Code,
length: number
): Multihash {
return multihash.encode(mh.digest(buf, func, length), func, length)
}

// expose multihash itself, to avoid silly double requires.
mh.multihash = multihash

mh.digest = function (buf, func, length) {
let digest = mh.createHash(func).update(buf).digest()
mh.digest = function(buf: Buffer, func: Name | Code, length: ?number): Buffer {
let digest = mh
.createHash(func)
.update(buf)
.digest()

if (length) {
digest = digest.slice(0, length)
Expand All @@ -26,38 +37,38 @@ mh.digest = function (buf, func, length) {
return digest
}

mh.createHash = function (func, length) {
mh.createHash = function(func: Name | Code): Hash {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is aside comment more on the API design. I would very much encourage to either have two different functions one that works with codes the other with names or better yet settle on canonical representation and make coercion a consumer's concern. The fact that IPFS libs tend to take unions of things made by far most difficult to figure out what's being passed around (although types would solve this) and also choose representation to use myself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmx I think we should open up an issue around this. Something that could be added in another PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Please open issues if you find weirdness in the API. Adding types is surely a good reason to review the APIs.

func = multihash.coerceCode(func)
if (!mh.functions[func]) {
throw new Error('multihash function ' + func + ' not yet supported')
throw new Error("multihash function " + func + " not yet supported")
}

return mh.functions[func]()
}

mh.verify = function (hash, buf) {
mh.verify = function verify(hash: Multihash, buf: Buffer): boolean {
const decoded = multihash.decode(hash)
const encoded = mh(buf, decoded.name, decoded.length)
return encoded.equals(hash)
}

mh.functions = {
0x11: gsha1,
0x12: gsha2256,
0x13: gsha2512
[0x11]: gsha1,
[0x12]: gsha2256,
[0x13]: gsha2512
}

blake.addFuncs(mh.functions)
sha3.addFuncs(mh.functions)

function gsha1 () {
return crypto.createHash('sha1')
function gsha1(): Hash {
return crypto.createHash("sha1")
}

function gsha2256 () {
return crypto.createHash('sha256')
function gsha2256(): Hash {
return crypto.createHash("sha256")
}

function gsha2512 () {
return crypto.createHash('sha512')
function gsha2512(): Hash {
return crypto.createHash("sha512")
}
49 changes: 30 additions & 19 deletions src/sha3.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict'
// @flow
"use strict"

const sha3 = require('js-sha3')
import * as sha3 from "js-sha3"
import type { Hash, HashTable } from "./types"

const functions = [
[0x14, sha3.sha3_512],
Expand All @@ -9,44 +11,53 @@ const functions = [
[0x17, sha3.sha3_224],
[0x18, sha3.shake128, 256],
[0x19, sha3.shake256, 512],
[0x1A, sha3.keccak224],
[0x1B, sha3.keccak256],
[0x1C, sha3.keccak384],
[0x1D, sha3.keccak512]
[0x1a, sha3.keccak224],
[0x1b, sha3.keccak256],
[0x1c, sha3.keccak384],
[0x1d, sha3.keccak512]
]

class Hasher {
constructor (hashFunc, arg) {
type HexString = string
type Sha3Hash = Buffer
type ShaHasher = (input: string | Buffer, length?: number) => HexString

class ShaHash implements Hash {
hf: ShaHasher
input: Buffer | null
arg: number

constructor(hashFunc, arg?: number) {
this.hf = hashFunc
this.arg = arg
if (arg) {
this.arg = arg
}
this.input = null
}

update (buf) {
update(buf: Buffer): Hash {
this.input = buf
return this
}

digest () {
digest(): Sha3Hash {
if (!this.input) {
throw Error("Missing an input to hash")
}
Copy link

@Gozala Gozala Apr 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I would really recommend encoding such invariants in a type system as primary benefit of type checking is to eliminate possibility of runtime errors. I don't know exact specifics of how this is used so my specific suggestion may not necessarily be applicable here, but it might still clarify what I mean. Usually such invariants can be encoded at the type level by making invalid state unrepresentable. Here is an example:

interface HashUpdate {
  update(buf: Buffer): Hash;
}

interface Hash extends HashUpdate {
  digest(): Buffer;
}


class ShaHash implements Hash {
  // Assumbtion is that this static function is exposed and not the class so that consumer can only create
  // HashUpdate and can't create `Hash` without an input. Since our `HashUpdate` interface does not have
  // a `digest` attempt to call will be reported by a type checker.
  static new(hashFunc, arg?: number):HashUpdate {
    return new ShaHash(hashFunc, arg)
  }
  // Since `update` returns a `Hash` that has both `digest` and `update` methods
  // calling `digest` on returned value will be by fine by type checker. Please also note
  // that even though it's the same instance from type checker perspective former still
  // has not `update` and later does.
  update(buf: Buffer): Hash {
    this.input = buf
    return this
  }
  // It is necessary to check this as `input` can be `null` but either way it's useful to cover
  // consumers that don't use type-checker
  digest(): Sha3Hash {
    if (!this.input) {
      throw Error("Missing an input to hash")
    }
    //...
  }
}

const input = this.input
const arg = this.arg
return Buffer.from(this.hf(input, arg), 'hex')
return Buffer.from(this.hf(input, arg), "hex")
}
}

function addFuncs (table) {
export const addFuncs = (table: HashTable) => {
for (const info of functions) {
const code = info[0]
const fn = info[1]

if (info.length === 3) {
table[code] = () => new Hasher(fn, info[2])
table[code] = () => new ShaHash(fn, info[2])
} else {
table[code] = () => new Hasher(fn)
table[code] = () => new ShaHash(fn)
}
}
}

module.exports = {
addFuncs: addFuncs
}
Loading