-
Notifications
You must be signed in to change notification settings - Fork 8
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
feedFormats and encryptionFormats as plugins #347
Conversation
@arj03 All tests pass except the ones related to box2 simply because of key management that I haven't worked on yet. But box2 is supported and you can see that from To use this in ssb-ebt for buttwoo, you'll have to do something like: db => network
network => db
If you need some other data, let me know. |
Also, 547df25 may look interesting, it creates the files
If you add a new encryption format named |
thoughts
I think this looks good but I find it quite hard to get a feel for it without visualising these in use? Maybe a conversation with that new pipeline diagram and pointing at the different parts and then naming the functions used would help contextualise. Also I assume some of the function signatures are not really pinned, because I assume things like |
Nice work btw 🏇 🦎 🍈 |
Hey @mixmix thanks for taking a look at this:
In this PR I'm using msgId, and I can try to be consistent with that elsewhere as well. msgKey is ambiguous because in so many places we use the word "keys" for "cryptographic key pair". In fact I think I bumped into one box2 related module that had msgKey as a variable name meaning "read key of a box2 boxed msg". So I'm trying to avoid that ambiguity. The exception, which I would preserve, is a KVT, because it's not so easy to change that object shape.
I'm open to renaming this function such that we always reserve the word "recipient" for sigils and SSB URIs. But I don't what other word to use. Suggestions welcome. I surely want to avoid the generic word "key". |
This is quite a beast, but overall a really nice and needed refactor :-) I have gone through a first pass of the code now. Some overall comments:
This is really nice!
Much better name, and really like how this is now just the core stuff.
We should probably discuss how we define these group names/keys sooner rather than later. |
@arj03 My idea with the rename is that you could use ssb-db2 without anything extra, i.e. |
Yeah that is really nice. At some we can move out migrate as well, that will make package.json a lot leaner :) |
I'm a tiny bit conflicted, and could move back to canDecrypt. But I'm going to try a bit more on this one: the index tells the records that we have successfully decrypted before, and the word is symmetric with "encrypted" in But I'll give this some more time, let's see. |
Beautiful 🥳 f68c43b |
@staltz re "getRecipients" do you mean it to be "getEnctptionKeys"? You didn't say what the buffers were |
I'm gonna fix the benchmarks before this is ready for review. |
Some review notes:
Do you remember why we pin ssb-friends to 5.1.0? |
Looking at create and the |
Overall I think this looks really good. I love how core.js is now really streamlined. It's hard to review such a big PR, not really a fault, just mentioning it to say that we might find bugs when we start upgrading other modules to work with this. I'm happy that we have a relatively good test-suite (and benchmarks :)). So in order to get move things along and really test I'll say it's fine to merge this in once the minor things I mentioned are fixed together with the benchmarks as you mentioned. It is after all a major version change. |
Thanks @arj03 for taking a look, I'm working on those. Two notes:
|
I think it's good to include both box1 and 2 in index.js. just that the default would be box1 of you don't specify in create. It is still the case that most clients can't decide box2. |
Yes, done
Yes, done
Oh, that's just a devDep and it's only used for the benchmarks (and actually those specific benchmarks are |
This actually made a lot of sense, and I made the change. But then some tests broke. Specifically, if you use I'm inclined to use |
I think that is for the best actually as publish is legacy anyway |
Yeah, I'm a bit conflicted. On one hand, removing the "try encryption formats until one of them works" is inconsistent with the default setting for encoding and feed format. On the other hand, now with And on the third (??) hand, since encryption is important and you don't want to screw it up, it's important to explicitly mention box2 not leaving it implicit. |
Okay but I implemented it anyway. Ready for Final FINAL review. |
I think it will be for the best this way :) Nice to see the last 2 commits actually removed a bunch of lines |
Can you create an issue for the benchmark that was commented out? |
Benchmark results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this. Super excited about the new ability to more easily add and structure feed formats 👍 ⭐ 🥇
MERGEDDDDDDD |
Benchmark results
|
Context
We are adding buttwoo as a feed format, and it works differently than other formats, so we would introduce a lot of complexity. At the same time, we need to improve the situation of support for box2 before we move on with "ssb-tribes2" on ssb-db2.
Solution
sbot.db.installFeedFormat()
andsbot.db.installEncryptionFormat()
and encodings.These are now orthogonal concerns, so any encryption format can work with any feed format, which can support different encodings.
🔵 Encoding
An "encoding" is a way of representing a feed message. This PR supports two encodings:
'js'
(JavaScript objects) and'bipf'
(BIPF buffers).🟡 Feed Format
A feed format defines how to create messages that follow a schema and a spec. Every feed format is a plugin-like object with:
name
stringencodings
array of supported encoding namesnewNativeMsg(opts)
toNativeMsg(msgVal, encoding)
fromNativeMsg(nativeMsg, encoding)
fromDecryptedNativeMsg(plaintextBuf, nativeMsg, encoding)
toPlaintextBuffer(opts)
getX()
andisX()
getFeedId(nativeMsg)
getMsgId(nativeMsg)
isNativeMsg(x)
isAuthor(author)
validateSingle()
validateBatch()
validateOOOBatch()
⭐ "Native Msg" this concept is new and important. Every feed format has a "native message format", which is a shape for the message that is specific to the feed format. The feed format fully controls its own "nativeMsg" shape, and it can be anything.
msgVal
.⚫ Encryption Format
An encryption format specifies how to encrypt and decrypt JavaScript buffers intended for some specific recipients. Every encryption format is a plugin-like object with:
name
string, also used for the suffix on the ciphertextsetup(config, cb)
OPTIONALencrypt(plaintextBuf, opts) => ciphertextBuf
decrypt(ciphertextBuf, opts) => plaintextBuf
TODO
isPrivate(encryptionFormat)
for querying canDecryptpost
Obv to compatfeedFormat.getSequence()
(needed for ebt)encryptionFormat.getRecipients()
=>getEncryptionKeys()
canDecrypt.index
naming?create()
onMsgAdded()
installFeedFormat()
installEncryptionFormat()
encrypted.index
/canDecrypt.index
setPost
hack for partial replication in the browserisPrivate()
=>isDecrypted()
/isEncrypted()
post
withonMsgAdded
post
withonMsgAdded