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

feat: more encoding, errors, spec tests #55

Merged
merged 11 commits into from
Jun 9, 2020
Merged

Conversation

hugomrdias
Copy link
Member

@hugomrdias hugomrdias commented Jun 3, 2020

This module now only uses 2 base encoding implementations, 1 generalised rfc4648 and 1 generalised btc like.

Note:
base8 deviates from the spec tests outputs but aligns with multiformats/multibase#60

closes #49
closes #38
closes #46
closes #53
closes #26

Benchmarks

new

identity x 1,225,963 ops/sec ±0.54% (83 runs sampled)
base2 x 282,486 ops/sec ±0.41% (89 runs sampled)
base8 x 564,588 ops/sec ±0.50% (88 runs sampled)
base10 x 131,183 ops/sec ±0.49% (87 runs sampled)
base16 x 665,055 ops/sec ±0.50% (86 runs sampled)
base16upper x 664,900 ops/sec ±0.42% (88 runs sampled)
base32hex x 746,598 ops/sec ±0.48% (87 runs sampled)
base32hexupper x 747,017 ops/sec ±0.57% (89 runs sampled)
base32hexpad x 740,950 ops/sec ±0.51% (87 runs sampled)
base32hexpadupper x 744,495 ops/sec ±0.54% (87 runs sampled)
base32 x 745,253 ops/sec ±0.51% (88 runs sampled)
base32upper x 744,605 ops/sec ±0.54% (90 runs sampled)
base32pad x 740,862 ops/sec ±0.48% (86 runs sampled)
base32padupper x 740,459 ops/sec ±0.53% (88 runs sampled)
base32z x 737,197 ops/sec ±0.49% (87 runs sampled)
base36 x 190,946 ops/sec ±0.52% (87 runs sampled)
base36upper x 190,731 ops/sec ±0.46% (91 runs sampled)
base58btc x 209,785 ops/sec ±0.52% (88 runs sampled)
base58flickr x 209,151 ops/sec ±0.61% (86 runs sampled)
base64 x 812,407 ops/sec ±0.49% (89 runs sampled)
base64pad x 785,924 ops/sec ±0.53% (89 runs sampled)
base64url x 815,979 ops/sec ±0.50% (89 runs sampled)
base64urlpad x 781,622 ops/sec ±0.50% (87 runs sampled)
Fastest is identity

old

base1:
base2 x 30,876 ops/sec ±0.80% (86 runs sampled)
base8 x 78,758 ops/sec ±0.64% (89 runs sampled)
base10 x 87,526 ops/sec ±0.55% (89 runs sampled)
base16 x 367,510 ops/sec ±0.78% (83 runs sampled)
base32 x 267,862 ops/sec ±0.62% (90 runs sampled)
base32pad x 255,571 ops/sec ±0.68% (87 runs sampled)
base32hex x 267,402 ops/sec ±0.60% (88 runs sampled)
base32hexpad x 254,547 ops/sec ±0.52% (87 runs sampled)
base32z x 257,585 ops/sec ±0.54% (90 runs sampled)
base58flickr x 135,862 ops/sec ±0.44% (87 runs sampled)
base58btc x 135,780 ops/sec ±0.66% (89 runs sampled)
base64 x 400,185 ops/sec ±0.60% (87 runs sampled)
base64pad x 378,530 ops/sec ±0.56% (87 runs sampled)
base64url x 376,370 ops/sec ±0.58% (86 runs sampled)
base64urlpad x 384,648 ops/sec ±0.94% (88 runs sampled)
Fastest is base64

/cc @Gozala @ribasushi

BREAKING CHANGE: names and codes export the full object that maps names/codes to base instances instead of just the keys

- adds support for all the encoding in https://github.com/multiformats/multibase/blob/master/multibase.csv
-  better errors showing the invalid chars and inputs
- `names` and `codes` export the full object that maps names/codes to base instances
- two news methods exported, `encoding` and `encodingFromData`
- added all the spec tests https://github.com/multiformats/multibase/tree/master/tests

This module now only uses 2 base encoding implementations, 1 generalised rfc4648 and 1 generalised btc like.

Note:
`base8` deviates from the spec tests outputs but aligns with multiformats/multibase#60
@hugomrdias hugomrdias requested review from achingbrain, lidel, rvagg, a team, mikeal and vmx and removed request for a team June 3, 2020 17:47
src/rfc4648.js Outdated Show resolved Hide resolved
src/rfc4648.js Outdated Show resolved Hide resolved
src/rfc4648.js Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@hugomrdias hugomrdias requested review from vmx and rvagg June 4, 2020 11:04
Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

I haven't reviewed the actual base encoding algorithm, but it seems like @rvagg has and also the test vectors pass, so I don't have much concerns about that.

@hugomrdias
Copy link
Member Author

@rvagg can you have another look before i merge/release ?

test/spec-test4.spec.js Outdated Show resolved Hide resolved
Copy link

@ribasushi ribasushi left a comment

Choose a reason for hiding this comment

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

@hugomrdias suggesting stripping a number of no-longer-relevant comments

test/spec-test1.spec.js Outdated Show resolved Hide resolved
test/spec-test2.spec.js Outdated Show resolved Hide resolved
test/spec-test3.spec.js Outdated Show resolved Hide resolved
test/spec-test4.spec.js Outdated Show resolved Hide resolved
test/spec-test5.spec.js Outdated Show resolved Hide resolved
test/spec-test6.spec.js Outdated Show resolved Hide resolved
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

+1 along with the removal of those comments about spec differences that @ribasushi is pointing out

@hugomrdias hugomrdias requested a review from vmx June 7, 2020 17:37
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
test/spec-test6.spec.js Outdated Show resolved Hide resolved
@fabianhjr
Copy link

Hi, multiformats/multibase#60 has been merged so the changed test vectors should now match with master.

@hugomrdias hugomrdias requested a review from vmx June 9, 2020 11:00
.travis.yml Show resolved Hide resolved
@hugomrdias hugomrdias merged commit 613363b into master Jun 9, 2020
@hugomrdias hugomrdias deleted the feat/more-bases branch June 9, 2020 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants