ssbc / ssb-bfe-spec

9 stars 1 forks source link

Clarify null values for messages #4

Closed arj03 closed 3 years ago

arj03 commented 3 years ago

@cryptix I noticed the spec was loosely defined on this point as you have a different definition: [1 1] + 32 zero bytes, than mine: [1 1] for classic messages. Does the same size requirement make it a lot easier to do stuff in Go? @mycognosist do you have any input on this from rust land?

mycognosist commented 3 years ago

@arj03

I think you must have snuck the msg == null code in when I wasn't looking ;) I don't yet have that case accounted for in the Rust implementation.

The size of the array doesn't make much difference to me (whether it's 2 bytes or padded with 32 zero bytes).

cryptix commented 3 years ago

I'd prefer to have a different extension code (like we have for bool) for null values instead of some hashes that can be empty.

It's less a question of what's possible or easier but consistency.

arj03 commented 3 years ago

We had a chat about this. Summary:

We need to figure out exactly how tribes handles these null messages, because they have production data that might be affected by whatever we end up with here.

arj03 commented 3 years ago

In tribes they have used the second option. So I think we should stick with that and not overthink the other cases.

staltz commented 3 years ago

Okay, I can finally join this thread after reading a bunch of things.

Currently the BFE-spec says [6 2] is for nil and undefined, and in JS ssb-bfe, it's exclusively for undefined. I don't think that's useful, because JSON.stringify({a: undefined}) evaluates to {}, but JSON.stringify({a:null}) evaluates to {"a":null}.

So it seems like [6 2] could be far more valuable to encode nulls than to encode undefineds. In other languages, there is just one "empty primitive", and those languages could use [6 2]. In JavaScript, the official "empty primitive" should be null, guided by how JSON.stringify behaves.

In tribes they have used the second option. So I think we should stick with that and not overthink the other cases.

In ssb-tribes they do that for a null Key in the T-F-Key triplet, which is very different to our case. In our case, [1 0 bytesbytesbytes] is supposed to represent a msg id, but that's not what we are encoding. We are encoding a primitive value ("value types"), so it should be at least [6 _ _].

If we were trying to encode null msg id, then yes I agree it should be [1 0 zerobyteszerobytes] but that's semantically not what we are encoding. I recommend we stick to strong semantic associations and use [6 2] for null and remove any mention of "undefined" from the spec.

staltz commented 3 years ago

To handle the case where box2 (envelope-spec) looks up prev_msg_id (and expects it to be a Buffer of the same size as a typical msg id), I think the box2 implementation can treat this just-in-time.

In JavaScript

This line

https://github.com/ssbc/envelope-js/blob/2d2bd576466850310a5261b8d7cc8591935ad961/util/derive-secret.js#L13

could be changed to

    const info = [
      toBuffer('envelope'),
      feed_id,
-     prev_msg_id,
+     prev_msg_id || Buffer.concat([Buffer.from([1,0]), Buffer.alloc(32)]),
      ...(labels.map(toBuffer))
    ]

right before info is encoded with slp-encode.

In Go

go-ssb can do something similar here (I suppose):

https://github.com/cryptoscope/ssb/blob/b985833c4f12ed0275bef1878b49a848579093ed/private/box2/boxer.go#L60

arj03 commented 3 years ago

Right, that makes undefined unsuitable so we need to use null for value type. What we really need is something like the option type in rust ;) I'll comment on the other JS PR now.