aries-vcx
aries-vcx copied to clipboard
refactor: improve didcore VerificationMethod in-memory representation
I followed the approach outlined in https://github.com/hyperledger/aries-vcx/issues/1163#issue-2202258364
- removed
PublicKeyFieldin favour of bytes - introduced a new flag that determines the serialization format
- implemented custom serialization
I do not think that this is really an improvement since it has its own downsides:
- we now have 2 separate attributes to determine the format of a single value
-
VerificationMethodis now responsible for implementing how to serialize other type - key should know how to serialize itself - while bytes can represent various key types, having just raw bytes us not that useful for working with the key. Usually, those bytes are used to construct a type that has methods for key operations
So I think we might go from:
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
#[serde(untagged)]
#[serde(deny_unknown_fields)]
pub enum PublicKeyField {
#[serde(rename_all = "camelCase")]
Multibase { public_key_multibase: String },
#[serde(rename_all = "camelCase")]
Jwk { public_key_jwk: JsonWebKey },
#[serde(rename_all = "camelCase")]
Base58 { public_key_base58: String },
#[serde(rename_all = "camelCase")]
Base64 { public_key_base64: String },
#[serde(rename_all = "camelCase")]
Hex { public_key_hex: String },
#[serde(rename_all = "camelCase")]
Pem { public_key_pem: String },
#[serde(rename_all = "camelCase")]
Pgp { public_key_pgp: String },
}
to something like:
#[derive(Clone, Debug, PartialEq)]
pub enum PublicKeyField {
Multibase(public_key::Key),
Jwk (JsonWebKey),
Base58(public_key::Key),
Base64(public_key::Key),
Hex(public_key::Key),
Pem(pem::Pem),
Pgp(pgp::Pgp),
}
impl Serialize for PublicKeyField {...}
The key type is responsible for holding the key bytes and PublicKeyField knows how to de/serialize itself. Your thoughts?
@xprazak2 thanks for the insights! You have good points.
The key type is responsible for holding the key bytes and PublicKeyField knows how to de/serialize itself. Your thoughts?
I think this is good idea, lets do it!