Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Base8 Spec #60

Merged
merged 1 commit into from
Jun 9, 2020
Merged

Conversation

fabianhjr
Copy link
Contributor

@fabianhjr fabianhjr commented Oct 2, 2019

This is a first throw at specifying a Base8 encoding, solves some issues of #59 and multiformats/go-multibase#26

Currently the test vectors mapped to the octal value of each byte so encoding and decoding would be harder than need be and encoded bytearray values would be larger (if each byte is mapped to 3 chars) or unpredictable (if, as currently, there are some chars encoded to different octal lengths)

For example, the first test vector was:

(In chunks of 3)
["104","312","615","453","347","216","230","266","151","364","624","403","127","314","534","474","564","320","645","563","162","044","1"]

(In chunks of 8)
["10431261","54533472","16230266","15136462","44031273","14534474","56432064","55631620","441"]

And 312 seems to be the encoding of e sometimes in some places like position ≡ {1,2} mod 3 .

rfcs/Base8.md Outdated Show resolved Hide resolved
rfcs/Base8.md Outdated Show resolved Hide resolved
rfcs/Base8.md Outdated Show resolved Hide resolved
hugomrdias added a commit to multiformats/js-multibase that referenced this pull request Jun 3, 2020
- 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
@fabianhjr fabianhjr force-pushed the add-base8-spec branch 4 times, most recently from 0813aaf to e88ecae Compare June 4, 2020 21:42
Copy link
Contributor

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

@fabianhjr so far so good! Can we add a ## Decoding section as well? Feel free to liberally C/P from the 4648 spec. Also won't hurt to link to it: https://tools.ietf.org/html/rfc4648

@ribasushi ribasushi requested a review from hugomrdias June 4, 2020 21:47
@ribasushi
Copy link
Contributor

@hugomrdias when time permits can you check that the spec-as-written makes sense? You were the last to implement stuff in this area...

@fabianhjr
Copy link
Contributor Author

I think it would be best to wait until the encoding is a go before trying to spec the decoding. As far as I know this encoding would fit what was implemented in multibase-js however will wait for @hugomrdias.

@fabianhjr
Copy link
Contributor Author

fabianhjr commented Jun 7, 2020

Thanks @hugomrdias.

The decoding section has been added @ribasushi.

rfcs/Base8.md Outdated Show resolved Hide resolved
@fabianhjr fabianhjr requested a review from Stebalien June 8, 2020 02:04
@fabianhjr
Copy link
Contributor Author

Hi @Stebalien, I think this is ready to be merged. UwU

@fabianhjr fabianhjr changed the title RFC: Add Base8 Spec Add Base8 Spec Jun 9, 2020
@Stebalien Stebalien merged commit b5750a6 into multiformats:master Jun 9, 2020
@Stebalien
Copy link
Member

🚀

@fabianhjr fabianhjr deleted the add-base8-spec branch June 9, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants