[fix] Multibase public key decoding incorrectly

[gmulhearn]

The PublicKeyField::key_decoded (i.e. "give me the raw public key bytes") was returning the wrong bytes for the publicKeyMultibase verification method field. Particularly, it was returning the [multikey-type + public key bytes] (i.e. the bytes prefixed with the key type), whereas we want it to just return the public key bytes (to align with what the other PublicKeyField variants decode to).

The unit tests were previously self-fulfilling in testing this, so it was not spotted. An easy way to spot the error is looking at what the PUBLIC_KEY_HEX test vector used to be; a 34 byte long array.. when's the last time you've seen a public key that is 34 bytes long! (it's because we were leaving the 2 byte multikey type prefix in there [236, 1] (a.k.a. 0xEC: https://github.com/multiformats/multicodec/blob/master/table.csv#L97)).

I also removed the key_decoded solution for JWK, as it was incorrect too; it was just returning the JSON serialized JWK as if it is the public key bytes. IMO the to_vec method of the JsonWebKey is misleading for this reason, so i removed it (i was expecting it to return the public key bytes..).

[gmulhearn]

fixed up some numalgo 2 tests. IMO the previous test behaviour highlights how it was confusing/inconsistent to return key_decoded with the prefix in the multibase case.

[JamesKEbert]

So, I'll admit I am a little weaker in this area specifically, so please correct any assumptions or knowledge gaps.

I think it makes sense to return the actual 32 bytes we're looking for in the majority of cases, as the 2 leading bytes are only there really for transport within the DID Doc (so that it can be explicit vs contextually determined, which is the purpose behind the Multibase Encoding Scheme as I understand it).

However, I could maybe see there being use cases where it's better to get the 'raw' value present in the DID Doc and pass that along still containing the additional context of the encoding scheme and multicodec (such as: an API that just returns the publicKeyMultibase and it's removed from the context of the original DIDDoc, so the end developer could use the the contextual info). So, if that's even a valid concern, is there a way to get the 'raw' value (so including the first two bytes)?

Baring that singular question, the PR looks good. :+1:

[gmulhearn]

Thanks for your review @JamesKEbert, yes there's some ways to do this. So in the case that you are holding the VerificationMethod struct from the DIDDoc you've resolved, there are a few things you could do:

• call .public_key on the VerificationMethod struct. This will return our public_key::Key struct. This contains: a typed KeyType and the raw public key bytes. The Key struct also has methods on it to get the multibase encoded key (with the prefix): Key::fingerprint, which will return the string that would be publicKeyMultibase.
• you could also pattern match on the PublicKeyField within the VerificationMethod to pull out the original public_key_multibase. i.e. something like

  key_field: PublicKeyField
  let PublicKeyField::Multibase { public_key_multibase } = key_field else { panic!("") };
  // public_key_multibase is the original string

in my experience of using this part of the crate, i kind the first option of using the .public_key method the most useful, as you get the keytype + key bytes decoded, and you can easily reencode it to whatever you want (e.g. publicKeyMultibase) using the Key methods.

I actually don't really need the fix in this PR anymore, bcus i realised .public_key was the better option for what i was doing... however i think the fix here still results in the key_decoded API being more consistent; always returning the raw (unprefixed) key bytes, no matter what variant the PublicKeyField enum was

[JamesKEbert]

Cool, glad to hear there's routes if someone would want to get access to the keytype + key bytes.

i think the fix here still results in the key_decoded API being more consistent; always returning the raw (unprefixed) key bytes, no matter what variant the PublicKeyField enum was

Agreed