Conversation
…ial with a base58 light DID
ntn-x2
force-pushed
the
aa-light-did-encoding-fix
branch
from
0bacec2 to
5c4ac77
Compare
tjwelde
left a comment
tjwelde
left a comment
There was a problem hiding this comment.
Overall: would work, but we should talk, if we want to introduce solution, which hints on the encoding
| let encoding: 'base58' | 'base64' | ||
| try { | ||
| // Try decoding details as base58 | ||
| deserialized = base58Decode(rawInput) |
There was a problem hiding this comment.
Since there can be collisions between base64 and base58 encoding, this might not throw on a base64 string.
There was a problem hiding this comment.
I did dig deeper into this and it turns out that since we are always including { e: { publickey: [...], type: "..." }}, base64 encoded string always starts with oWFlomlwdWJsaWNrZXnYQFhA and ends with R0eXBlZ3NyMjU1MTk=, which include characters not allowed in base58.
So it would actually work, but might not be the best solution here.
We should maybe think about something like https://github.com/multiformats/multibase
There was a problem hiding this comment.
Also we should think about reducing the characters which are the same for every encoded string
|
Superseded by #449. |
2 participants
Change default light DID details encoding to Base58
This PR fixes https://github.com/KILTprotocol/ticket/issues/1745.
Problem
Base64 charset contains characters that are not allowed in the DID Core spec. This means that whenever a light DID is created using more than a simple authentication key, i.e., when additional details must be encoded, the resulting DID string is not compliant with the standard anymore.
This PR changes the encoding to base58, while still allowing base64-encoded DIDs to be successfully decoded and used in all the places where a DID is used.
Implications
The
LightDidDetailsconstructor takes an additional parameterdetailsEncoding: 'base58' | 'base64'. For light DIDs that have been used to obtain credentials and 1. include an additional encryption key or service endpoints and 2. have not yet been migrated, the initialisation process using keys previously stored in the keystore must be done usingdetailsEncoding = 'base64', to allow for the credential to still be used. New light DIDs will usedetailsEncoding = 'base58'by default.An alternative would be to allow the wallet, i.e., only Sporran so far, to change so that credentials issued to unmigrated light DIDs would be re-requested for the new DID generated from the same keys and service endpoints. Issues such as SocialKYC would also need to be updated to support the new base58 encoding, but this is a much less breaking change SocialKYC is not interested in the specific details encoding of a light DID.
Changes
The new base58 encoding/decoding support was added, and a bunch of additional test cases to verify that none of the existing features have been broken, as long as the same light DID encoding is used. A negative test capturing the case of a base58-encoded DID using a credential issued to its base64-encoded representation has been added as well in the unit tests for
Credential.spec.ts.Note about failing integration tests
A change was recently made to the chain logic where the
delegation.remove()extrinsic via a DID call is only considered valid if the submitter is the delegation owner, and it will now fail if this condition is not met. The integration test of the delegation module have been updated to reflect this change, sodevelopintegration tests are passing, butlatest-rcones are now failing and will keep doing so until the new change is merged intomaster.