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

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 a team, achingbrain, lidel, mikeal, rvagg and vmx and removed request for a team June 3, 2020 17:47
@hugomrdias hugomrdias requested review from rvagg and vmx 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 ?

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

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
@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
@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

Development

Successfully merging this pull request may close these issues.

Implement base36 Export the names and codes as object (instead of array) More description error text Test against spec

5 participants