Added Base8 Implementation #26
Conversation
|
Should creating a |
|
ping @Stebalien |
There was a problem hiding this comment.
We'll need to define what base8 means for us before we implement it. This implementation expects that each group of 3 digits maps to a single byte. However, 0o777 (7777 in multibase) is 511 (doesn't fit in a single byte).
The simplest mapping I know of is 8 octal digits onto 3 bytes. However, if we go with that, we'll probably want to accept octal strings that aren't a multiple of 8 (automatically padding out to a multiple of 8).
Regardless, you'll need to submit a PR to https://github.com/multiformats/multibase/ describing the encoding (unless you can fine some existing RFC).
(I'd also kind of like to know why you need octal before we go through the process of defining and implementing it)
multibase_test.go
Outdated
| @@ -81,8 +83,9 @@ func TestDecode(t *testing.T) { | |||
| func TestRoundTrip(t *testing.T) { | |||
| buf := make([]byte, 17) | |||
| rand.Read(buf) | |||
| fmt.Println(buf) | |||
There was a problem hiding this comment.
Best to avoid printf in tests. If you want to log this, please use t.Log.
There was a problem hiding this comment.
This was by a mistake. Removed now.
base8.go
Outdated
|
|
||
| // decodeOctalString takes an octal string and gives byte array of decimals | ||
| func decodeOctalString(s string) ([]byte, error) { | ||
| data := make([]byte, len(s)/3) |
There was a problem hiding this comment.
Nit: I'd move this after the length check.
|
I've fixed the recommended changes in code.
I was reading the code and found out base8 was missing and I had some time so. 😕 |
Fair enough. That's also why we have base 2... But we'll still need some spec first. |
|
Totally understandable. Thanks for your time. |
Stebalien
added
the
status/blocked
|
@Stebalien: Can I implement base2 encoding in reference with RFCs? |
|
Go ahead. |
* upstream/master: ci: standardize test implementation against test-vectors gx: ignore tests when publishing as well add the spec as a submodule
|
Regarding spec for base 8, I gave this some thought. For bases that evenly break bytes into characters, pad to full bytes, this is base 2 (8 chars), base 4 (4 chars), and base 16 (2 chars). It makes sense for these to treat as bitstreams and pad out to whole bytes. But for other power of two bases that don't evenly fit into a byte, use optional padding at the end. Examples are base64 (3 bytes = 4 chars), and base32 (5 bytes = 8 chars). Base 8 (3 bytes = 8 chars) could fit in this style encoding. Another option and the one JS currently uses is to convert to a large number and make leading zeroes represent null bytes similar to base 10 and base 58. |
|
I wrote a base-8 codec that works similar to base-32 and base-64 where you give it an alphabet and an optional padding character. https://github.com/filecoin-project/lua-filecoin/blob/master/base-8.lua For example base8 with
But if I instead use base-x (which is what the JS implementation currently does), it looks closer to the current test vectors, but different leading zeroes:
|
|
@creationix I'm fine with either but I'd like to go with what's commonly used in the community. Have you found any other users of base8? |
|
I've not seen any others. I don't know if there is a common encoding for this. Logically, the same style as base-64 and base-32 makes the most sense. |
|
So, the real question is, should we even bother? A viable option is to just drop base8. |
|
Personally I see base-2 and base-8 both as unneeded. Is there any use case where they are the correct solution? Base-16 works everywhere and encodes much shorter and easier than them. My recommendation is to either drop them to reduce the maintenance overhead for implementations or to go with the logical encoding as I've suggested if we must keep base-8. |
No description provided.