andyleap / go-ssb

GNU General Public License v3.0
42 stars 13 forks source link

Make Ref.Raw() and Signature.Raw() return []byte, not string #1

Closed keks closed 7 years ago

keks commented 7 years ago

Pretty much that. The test doesn't pass but I see the same error on master so I don't think that's my fault.

Also I added feeds.db to .gitignore so there is no useless red line in every git status.

Edit: FWIW on both master and rawbytes the test error I see is

feed_test.go:69: Error: expected previous %FMNFrlyeO5Z//fY5FbLVTNzhbqppGoLs0Bq4oR8450A=.sha256 but found %gmrb5/tjdgmHqMpZiyFL5T3uNUzKBOdv786WuamBybM=.sha256
andyleap commented 7 years ago

Issue with this is that it ties down the encoding (base64 "standard") and makes it so everything is that. I'd prefer to have that be at least a little customizable, but it is something to discuss.

That error is something isn't hashing correctly. Unfortunately, node.js and Go have slightly different standards for encoding, and a 1 character difference will, of course, throw the hash completely off. I'll have to look into that one to see what's up.

keks commented 7 years ago

Issue with this is that it ties down the encoding (base64 "standard") and makes it so everything is that.

Well, if they are not and there is e.g. a %hexhash.sha256, the best we can do is guess. Introducing such a format would be nuts because you want to be able to parse the thing with certainty. What could happen is that e.g. there is a hash like %.x$hexhash.sha256, with .x specifying that the hash is in hex format, but if we allow for different formats, the format must be specified in the Ref itself. In that case we can add some more logic to parse the format first.

in %cx4sDkZEor3BJWlF6GfwXCStXSul3fPbxlLG+MDv2yQ=.sha256 @dominictarr suggested using a different base64 symbol set, but we can just blindly search and replace +,/ to -,_ before decoding so that isn't a show stopper. Other than that I don't expect much changes.

That error is something isn't hashing correctly. Unfortunately, node.js and Go have slightly different standards for encoding

Ah, I see!

dominictarr commented 7 years ago

maybe it's time to harden up the spec for what the ref means a bit. we kinda stuck the sigil on the front so lexiographic indexes would group things correctly, and so you could trigger auto suggester ui. The extention on the end because we figured we'd be glad we did one day. base64 because node has a base64 thing built in and implemented in c so it's fast. base58 does copy-paste nicer, but requires big number math, and I was worried that would be implemented in js and slow.

I figure, for a change I suggested it's unambigious whether it's a friendly64 or not, and any way a one-to-one conversion. if we where to hypothetically use hex or base58 which could be ambigious (except for length) but then we could have different length hashes too. I think incorporate that into the extention? maybe even .foo.bar extentions?

We should also create json test vectors for this, so you can have the same tests that ssb-ref uses

andyleap commented 7 years ago

Shouldn't be too big a deal to introduce more decoding formats as "tag extension" like .hexsha256, so I'm accepting this as is for now, it'll just need tweaking if that does happen.