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

name changes to types/format #12

Merged
merged 10 commits into from
Aug 15, 2021
Merged

name changes to types/format #12

merged 10 commits into from
Aug 15, 2021

Conversation

mixmix
Copy link
Member

@mixmix mixmix commented Aug 11, 2021

⚠️ assumes #11 is merged

context

on an @arj03 @mixmix @keks call discussing proposed changes with POBox, we got stuck into asking "what is a type, what is a format".
I think we arrived at an idea like:

  • a "type" is like a domain, a grouping based on similar functions that you would want to perform
  • a "format" is a associated with a specific "recipe" in that type/domain
    • this recipe includes algorithm AND context (e.g. this is a signature of a bendy but content field)
    • it's not useful / safe enough to just say "ed25519"

we also realised that some type/formats are locked in by use, but the naming of it is still flexible. This means that e.g. type 3, format 0, which was "diffie-hellman, ed25519" is being proposed to rename to "key, box2-dh".
This reflects the realisation that "diffie-hellman" is waaaay too vague and what we actually want to be able to talk about are encryption keys, and which exact recipes (format) they were used in

naming of fields

so naming is going to change some because we've realised some of the types/ formats were not tight enough.

I am also interested in changing names so that we can easily use names programatically when importing bfe.json.

Use cases:

  • we're using bfe.json in ssb-bfe and we have functions which depend on the type/format string names
  • I want to write functions which use this bfe.json definition to generate SSB-URIs for some types.

proposals

  1. we use the same name in README.md as in bfe.json
    • e.g. if type 0 = msg we use msg in the docs
  2. we don't use names with / or \s in them
    • this makes creating and parsing a URI harder
  3. we choose relatively short names
    • prettier, shorter URIs
    • if this is a challenge we could split out e.g. type.type into `type.textHandle" and "type.description"
      • if we do this, we could relax (1)

This PR executes these proposals to show more explicitly what I mean

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@arj03
Copy link
Member

arj03 commented Aug 12, 2021

Some suggestions that would be good to take into consideration before merging this.

README.md Outdated
|:-----------:| ------------------ |
| 0 | feed |
| 1 | msg |
| 2 | blob |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to preserve what was before, i.e. uppercase. Otherwise it feels like it's undoing what I did here 11f7d16

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following the principle of keeping the string the new ad the steering names in the bfe.json
So that people using NAMED_TYPES can just look at this table and type it without having to guess the case, spaces, hyphens

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that people using NAMED_TYPES can just look at this table and type it without having to guess the case, spaces, hyphens

There is bfe.json for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm i at what you mean. I think i was being paranoid expecting people to get confused. But if you think it's fine I'm happy for readme to be more human descriptive

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you know what? We could have a third column that reflects the name used in bfe.json

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
|-----------|-------------|-------------|-------------|-----------------|
| 5 | 0 | Arbitrary | box | [private box] |
|:---------:|:-----------:|-------------|-------------|-----------------|
| 5 | 0 | Arbitrary | box1 | [private box] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one still confuses me, which one is it, box1 or box? I honestly don't know. I know that once code broke everywhere because we were using box1 and it had to be box. But I think (??) ssb-tribes uses box1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ssb tribes uses box2
The original is box, but since the advent of box2 we talked about box saying "box1"... but the suffix is the OG box

At least I think that's how it went down

README.md Outdated
| 6 | 2 | 0 bytes | Nil | [null pointer] |
| 6 | 3 | Arbitrary | Arbitrary bytes | N/A |
|:---------:|:-----------:|-------------|-------------|-------------------------------|
| 6 | 0 | Arbitrary | string-UTF8 | [UTF8] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would appreciate if this is reverted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please respond to my values /proposals
Specifically, I don't want spaces in the name because it makes uris have to have some rule for dealing with them (or you could propose one)

Copy link
Member

@staltz staltz Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response to values/proposals is that this markdown document is for humans to read, so let's use formal English, while the json is for computers to parse. In the markdown we should be trying to explain to people what this is, and this table doesn't look like it's explaining, looks like it's stating raw data.

As for URIs, we're not going to have SSB URIs for these Generic data formats, so I don't understand what's the need for the hyphen.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree about ssb uri for generic types. I reckon that can be reverted happily then

bfe.json Outdated Show resolved Hide resolved
bfe.json Outdated
]
},
{
"code": 3,
"type": "diffie-hellman",
"type": "key",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "encryption key" (was that what we agreed on in the meeting? I don't accurately remember)? "key" is a very generic word.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct but in the types key is only ever going to mean a key for unlocking.

See also "no spaces / short words for ssb uri"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR then automatically means you're proposing the SSB URI ssb:keys/box2-dm-dh/ and I believe ssb:keys/ is going to confuse a lot of people on what it means (given the existence of ssb-keys). Why not ssb:encryption-keys/box2-dm-dh ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly because it's hella long, and I wasn't to save my bytes!

I'm up for something different to reduce confusion. Couldn't think of anything right though. Maybe emoji? Joking

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not prohibitively long. Remember SSB URIs are for human readability (while still computer parse-able), and we have other long SSB URIs such as ssb:experimental?action=claim-http-invite&.....

@staltz
Copy link
Member

staltz commented Aug 12, 2021

1. we use the same name in `README.md` as in `bfe.json`

I think most of my inline reviews are related to this. I don't like the idea of using same terms in README and bfe.json. The former is clearly documentation meant for humans to read (what if people don't know that "msg" refers to "message"?) while the latter is a programmatic helper, meant for computers to parse. Different purposes. Otherwise soon we're at the point where ssb-bfe would import literally README.md and parse it.

@mixmix
Copy link
Member Author

mixmix commented Aug 12, 2021

With this PR I would like us all to own it
I started this because it was an Action from the meeting

I took what I remembered, and integrated what I think will be good, making in mind plans to build ssb uris with this code. Keen to figure out if that's what we all want, and if so how best to do it. This is just a starting point I threw down

Base automatically changed from add_json to master August 12, 2021 22:38
README.md Outdated Show resolved Hide resolved
mixmix and others added 3 commits August 13, 2021 20:12
add identity type, with po-box format
add pobox diffie-hellman key type
Co-authored-by: André Staltz <andre@staltz.com>
@staltz
Copy link
Member

staltz commented Aug 13, 2021

Hold on, I'll make a small PR targeting this name_changes branch.

@mixmix
Copy link
Member Author

mixmix commented Aug 13, 2021

This is now turning into a mega branch. What do we want to do before merging?
(I feel like I merged the small PRs in the wrong order)

staltz and others added 2 commits August 14, 2021 09:13
@mixmix mixmix merged commit 29e3330 into master Aug 15, 2021
@mixmix mixmix deleted the name_changes branch August 15, 2021 22:58
@mixmix
Copy link
Member Author

mixmix commented Aug 15, 2021

@staltz I've made that change from key > encryption-key, and merge squashed this.

Thanks for the collaboration on this. I noticed that this is a kinda unique thing in SSB - it's very rare that we force ourselves to commit to a shared specification (though I wish we did this more in some cases!)

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.

None yet

3 participants