aries-vcx icon indicating copy to clipboard operation
aries-vcx copied to clipboard

refactor: improve didcore VerificationMethod in-memory representation

Open xprazak2 opened this issue 2 years ago • 1 comments

I followed the approach outlined in https://github.com/hyperledger/aries-vcx/issues/1163#issue-2202258364

  • removed PublicKeyField in 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
  • VerificationMethod is 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 avatar Mar 28 '24 08:03 xprazak2

@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!

Patrik-Stas avatar Apr 04 '24 01:04 Patrik-Stas