Closed rillian closed 9 months ago
NB we shouldn't deploy this to the star-randsrv app until after https://github.com/brave/brave-core/pull/18954 has made it into stable release.
The corresponding brave-core change is in 1.54 scheduled for stable release July 18. I'll hold off on merging until then to avoid blocking other work.
Could we please hold off for a few months? Even though 1.54 will be released in the coming weeks, it does take a while for the majority of clients to update (i.e. Android users)
Looking at data from last week, we're about halfway through the transition (300k supporting multihash vs 400k not), so we should revisit after the next release.
@DJAndries Are you ok with merging this now? The browser-side change has been deployed for ~7 months, and attestation has been disabled for about the same amount of time.
@DJAndries Are you ok with merging this now? The browser-side change has been deployed for ~7 months, and attestation has been disabled for about the same amount of time.
i'm cool with it :+1:
i'm cool with it 👍
Great. Could you mark it as approved please, so I can merge?
Because the
hashSeparator
character can occur within the binary hash digest values representing the https cert and optional app key, it's not useful for dividing the user_data value into separate fields.Instead use the multihash encoding, which prefixes two bytes. The first (0x12) define the hash digest as SHA-256 just as the previous "sha256:" prefix did. The second (0x20) marks the digest length. This more compactly delimits the values while allowing the same extensibility.
Resolves #23