Extract getting verification key for proof signing into VerificationKeyStrategy class

[mkempa]

Following the logic in PR 2235 it is also beneficial to have the strategy for getting verification key itself overridable by plugins.

Such a use case would be to use the universal resolver to get a DID Document and fetch a verkey identified by the key ID. Then search the wallet by the verkey to obtain DIDInfo.

[dbluhm]

@mkempa I'm finding this change a bit confusing. The added method is get_verification_key_for_did but the value actually returned is a DIDInfo rather than a representation of a verkey. This strikes at a deeper problem we have right now in ACA-Py: the 1:1 relationship between DIDs and keys. I think it would be better for code like the LD Proof handler to take an object representing a "verification method" rather than a DID Info object and then for the keys to be stored by verification method ID so they're easily recalled from the wallet. If we can achieve that construction, I don't think there would be a need to make the verkey retrieval pluggable.

There are details there that would need to be worked out still though. Thoughts, @swcurran @andrewwhitehead @usingtechnology?

[mkempa]

The added method is get_verification_key_for_did but the value actually returned is a DIDInfo rather than a representation of a verkey.

My first intention was to return the verkey or None, hence the method name. Then I went on the path of the least resistence. Otherwise I would have to modify signature of _get_suite and fix more unit tests, etc.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[dbluhm]

@PatStLouis is going to a take another look at this

[swcurran]

@PatStLouis -- any update on this? Thanks!

[PatStLouis]

The issue with using the verificationMethod object to find the keypair is that there could be different verificationMethod.types adding complexity.

When I create a did in the wallet, the current method of encoding the verkey is using the ED25519VerificationKey2018 encoding algorithm. I might want to represent this verkey using ED25519VerificationKey2020 in my did document, so finding the key from the verificationMethod might not always lead to a match.

I could also have multiple verificationMethod.id I want to link to the same keypair (like publishing the verkey with 2 different web did)

Since the did and the verkey value are used to find the matching keypair, they act as a label for this keypair. The simplest approach would be to have an optional issuance options field proofKey where I can provide the verkey as it's registered in the wallet and this would then take priority over using the issuer did value if present. This is how it worked with the /jsonld/sign endpoint and adds value to a controller.

However there is already an existing function get_local_did_for_verkey function available for this.

I'm not convinced in the added value of making this pluggable. I would rather have an optional field in the issuance options for now.

[swcurran]

@dbluhm — thoughts on this? I’m sort of getting this, but don’t know the data flow — what is/could be where in the DID and proof.

[dbluhm]

@PatStLouis is this superseded by having the keys in the wallet identifiable by kid/verification method id?

[PatStLouis]

@dbluhm it looks like it, identifying the wallet keys by verificationMethod.id seems like a more elegant way of achieving what is depicted in the use case. Regardless this PR is about to be a year old and the file structure changed. I'm okay with closing this PR unless @mkempa has objections.