Upgrade DID multiformat #587
Conversation
Could we break out the format into its own type as another |
| , publicKey :: Key.Public | ||
| } deriving (Show, Eq) | ||
| data DID | ||
| = Key Key.Public |
There was a problem hiding this comment.
RE the comment about getting this mergable:
In fact (and this is maybe controversial), we could potentially put old styles in this sum
There was a problem hiding this comment.
Something like:
data DID
= Key Key.Public
| OldstyleRSA OldstyleThere was a problem hiding this comment.
🤔 Yeah we could do that.
However, I think getting this mergable is easier by switching DID generation to the old style, and keeping the new style around in an unused function/comment.
There was a problem hiding this comment.
I guess, what's the advantage of having both old & newstyle available, if we only ever use one of the two styles in call sites? Then we can just change it in the, uh.. definition site? (What's the opposite of call site? :D )
There was a problem hiding this comment.
Having both available means that old UCANs continue to work — we can decode them, even if we only generate new ones
There was a problem hiding this comment.
Oh, old UCANs already continue working even in this PR. We simply allow both formats and parse them to the same DID.
There was a problem hiding this comment.
Ah okay. Clearly I need to look at the code clearer! I'm just in a meeting; will look in more detail in a bit. Apologies 🙏
| (0x85 : 0x24 : rsaASNPublicKeyW8s) -> | ||
| case Public.decodeASN1DERRSAPublicKey $ BS.pack rsaASNPublicKeyW8s of | ||
| Right pk -> pure $ RSAPublicKey pk | ||
| Left err -> fail err | ||
|
|
||
| -- Backwards compatibility | ||
| (0x00 : 0xF5 : 0x02 : rsaKeyW8s) -> | ||
| RSAPublicKey <$> parseKeyW8s (BS64.encode $ BS.pack rsaKeyW8s) |
There was a problem hiding this comment.
This is what I'm talking about: We just add the ability to interpret another multiformat prefix.
| RSAPublicKey rsa -> 0x00 : 0xF5 : 0x02 : BS.unpack (BS64.decodeLenient . encodeUtf8 $ textDisplay rsa) | ||
| {- ^ ^ ^ | ||
| | | | | ||
| | "expect 373 Bytes", encoded in the mixed-endian format | ||
| "raw" | ||
| -} |
There was a problem hiding this comment.
Buuuuut, we've removed the old format generation code. If we put this back in we can merge this into main I think.
There was a problem hiding this comment.
Awesome; let's put it back!
We actually did this with the Oldstyle DIDs last time, too. It worked well! IIRC we "just" wrapped calls to emitted DIDs in DID.Oldstyle, and it serialized in the old format. After the browser was able to handle the new format, we removed that wrapper, but kept the ability to deserialize old UCANs.
Does that make sense?
There was a problem hiding this comment.
Yup! I've taken a close look at Oldstyle recently, actually. It's still there and we're using it to announce the fission server's DID in old-style IIRC.
I think we had a discussion about that and we realized we can just remove the ... "old oldstyle" and replace it with something else, since webnative doesn't actually "read" the DID, it only takes it verbatim as the audience for its UCANs. But that's something for another PR - I'll put that into an issue.
Split out from #586
This upgrades the DID multiformat to the new version. We can't deploy this yet, because this PR will have the CLI generate new-style DIDs and browsers need to be upgraded to understand that format first.
What we could merge is support for reading the new DID RSA format, but generating the old RSA format.
However for now this is just some reviewable commits extracted from #586, so we can split reviewing into rounds.