-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use did:key flag #1029
feat: use did:key flag #1029
Conversation
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #1029 +/- ##
==========================================
+ Coverage 88.66% 88.69% +0.03%
==========================================
Files 520 521 +1
Lines 12099 12115 +16
Branches 1996 2001 +5
==========================================
+ Hits 10727 10745 +18
+ Misses 1310 1308 -2
Partials 62 62
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@andrewwhitehead -- heads up on this. We probably want to have ACA-Py doing the same thing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
|
||
export type ConnectionMetadata = { | ||
[ConnectionMetadataKeys.UseDidKeysForProtocol]: { | ||
[protocolUri: string]: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specific reason for making this a key/value object instead of an array with string uri's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal of this metadata key is to override agent settings, based on what we learn from the connection. So if our agent is set to always use did:key by default and we learn that the other side is not using it for a certain protocol, we'll set the key for that protocol to false.
And we could also have the opposite case: we don't use did:key but it happens that the other side is using them, so we set the key for that protocol to true. Also we might change our agent settings afterwards, and it will still take into account the metadata key for the given connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I thought we would only use it to say a protocol does support did:key, but there is also the case where we explicitly store a protocol doesn't store it.
Is this also updated autmoatically when this changes? E.g. we have stored false so we keep sending base58 keys, but then the other agent sends a did:key, do we update it automatically? Or is it set once and kept like this forever?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this also updated autmoatically when this changes? E.g. we have stored false so we keep sending base58 keys, but then the other agent sends a did:key, do we update it automatically? Or is it set once and kept like this forever?
I think it depends on each protocol/service but generally the best would be to update it automatically. As of current AFJ support for mediator role, it does not add so much value because we only support Mediation Grant and Keylist Update request/response. However, considering a future support of Keylist queries and responses it's better to handle it right from the start.
So I'm adding automatic handling for both recipient and mediator in the messages we currently support. Hopefully in 0.3.x we'll support all of them 😄
const connectionUsesDidKey = messageContext.message.routingKeys.some(isDidKey) | ||
|
||
const useDidKeysForProtocol = connection.metadata.get(ConnectionMetadataKeys.UseDidKeysForProtocol) ?? {} | ||
useDidKeysForProtocol[MediationGrantMessage.type.protocolUri] = connectionUsesDidKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work with minor version updates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tied to a specific protocol major.minor version. If a new interaction is started with a newer protocol version, a new key/value will be added. Not super nice in terms of storage optimization, but I guess this will be mostly limited to specific connections (such as mediators), so should not represent a big issue 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, something to look out for. We maybe don't need to store it separtely per minor version (however if ACA-Py updated to mediator coordinator 1.1 and 1.0 already supported did:key I would assume the newer version also does).
@@ -125,9 +134,9 @@ export class MediationRecipientService { | |||
// update keylist in mediationRecord | |||
for (const update of keylist) { | |||
if (update.action === KeylistUpdateAction.add) { | |||
mediationRecord.addRecipientKey(update.recipientKey) | |||
mediationRecord.addRecipientKey(verkeyToDidKey(update.recipientKey)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a migration script to update non did keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, actually it was supposed to be a fix for the case where we receive keys in did:key format and it made it worst 😝.
It should use didKeyToVerkey
instead. I'll add a test to MediationRecipientService.test.ts
to make sure it stores all keys in base58 and it behaves exactly as current wallets.
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Ready to merge? |
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
* feat: OOB public did (#930) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * feat(routing): manual mediator pickup lifecycle management (#989) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * docs(demo): faber creates invitation (#995) Signed-off-by: conanoc <conanoc@gmail.com> * chore(release): v0.2.3 (#999) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(question-answer): question answer protocol state/role check (#1001) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: Action Menu protocol (Aries RFC 0509) implementation (#974) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * fix(ledger): remove poolConnected on pool close (#1011) Signed-off-by: Niall Shaw <niall.shaw@absa.africa> * fix(ledger): check taa version instad of aml version (#1013) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * chore: add @janrtvld to maintainers (#1016) Signed-off-by: Timo Glastra <timo@animo.id> * feat(routing): add settings to control back off strategy on mediator reconnection (#1017) Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> * fix: avoid crash when an unexpected message arrives (#1019) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * chore(release): v0.2.4 (#1024) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * style: fix some lint errors Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: use did:key flag (#1029) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * ci: set default rust version (#1036) Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> * fix(oob): allow encoding in content type header (#1037) Signed-off-by: Timo Glastra <timo@animo.id> * feat: connection type (#994) Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> * chore(module-tenants): match package versions Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: improve sending error handling (#1045) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: expose findAllByQuery method in modules and services (#1044) Signed-off-by: Jim Ezesinachi <jim@animo.id> * feat: possibility to set masterSecretId inside of WalletConfig (#1043) Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> * fix(oob): set connection alias when creating invitation (#1047) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * build: fix missing parameter Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: conanoc <conanoc@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: Niall Shaw <niall.shaw@absa.africa> Signed-off-by: Jakub Koci <jakub.koci@gmail.com> Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> Signed-off-by: Jim Ezesinachi <jim@animo.id> Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> Co-authored-by: Iskander508 <pavel.zarecky@seznam.cz> Co-authored-by: conanoc <conanoc@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Niall Shaw <100220424+niallshaw-absa@users.noreply.github.com> Co-authored-by: jakubkoci <jakub.koci@gmail.com> Co-authored-by: Timo Glastra <timo@animo.id> Co-authored-by: Sergi Garreta Serra <garretaserra@gmail.com> Co-authored-by: Sai Ranjit Tummalapalli <34263716+sairanjit@users.noreply.github.com> Co-authored-by: KolbyRKunz <KolbyKunz@yahoo.com> Co-authored-by: Jim Ezesinachi <ezesinachijim@gmail.com> Co-authored-by: an-uhryn <55444541+an-uhryn@users.noreply.github.com>
* feat: OOB public did (openwallet-foundation#930) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * feat(routing): manual mediator pickup lifecycle management (openwallet-foundation#989) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * docs(demo): faber creates invitation (openwallet-foundation#995) Signed-off-by: conanoc <conanoc@gmail.com> * chore(release): v0.2.3 (openwallet-foundation#999) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(question-answer): question answer protocol state/role check (openwallet-foundation#1001) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: Action Menu protocol (Aries RFC 0509) implementation (openwallet-foundation#974) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * fix(ledger): remove poolConnected on pool close (openwallet-foundation#1011) Signed-off-by: Niall Shaw <niall.shaw@absa.africa> * fix(ledger): check taa version instad of aml version (openwallet-foundation#1013) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * chore: add @janrtvld to maintainers (openwallet-foundation#1016) Signed-off-by: Timo Glastra <timo@animo.id> * feat(routing): add settings to control back off strategy on mediator reconnection (openwallet-foundation#1017) Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> * fix: avoid crash when an unexpected message arrives (openwallet-foundation#1019) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * chore(release): v0.2.4 (openwallet-foundation#1024) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * style: fix some lint errors Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: use did:key flag (openwallet-foundation#1029) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * ci: set default rust version (openwallet-foundation#1036) Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> * fix(oob): allow encoding in content type header (openwallet-foundation#1037) Signed-off-by: Timo Glastra <timo@animo.id> * feat: connection type (openwallet-foundation#994) Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> * chore(module-tenants): match package versions Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: improve sending error handling (openwallet-foundation#1045) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: expose findAllByQuery method in modules and services (openwallet-foundation#1044) Signed-off-by: Jim Ezesinachi <jim@animo.id> * feat: possibility to set masterSecretId inside of WalletConfig (openwallet-foundation#1043) Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> * fix(oob): set connection alias when creating invitation (openwallet-foundation#1047) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * build: fix missing parameter Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: conanoc <conanoc@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: Niall Shaw <niall.shaw@absa.africa> Signed-off-by: Jakub Koci <jakub.koci@gmail.com> Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> Signed-off-by: Jim Ezesinachi <jim@animo.id> Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> Co-authored-by: Iskander508 <pavel.zarecky@seznam.cz> Co-authored-by: conanoc <conanoc@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Niall Shaw <100220424+niallshaw-absa@users.noreply.github.com> Co-authored-by: jakubkoci <jakub.koci@gmail.com> Co-authored-by: Timo Glastra <timo@animo.id> Co-authored-by: Sergi Garreta Serra <garretaserra@gmail.com> Co-authored-by: Sai Ranjit Tummalapalli <34263716+sairanjit@users.noreply.github.com> Co-authored-by: KolbyRKunz <KolbyKunz@yahoo.com> Co-authored-by: Jim Ezesinachi <ezesinachijim@gmail.com> Co-authored-by: an-uhryn <55444541+an-uhryn@users.noreply.github.com>
An initial step to support Aries RFC 0360 by adding an optional configuration flag
useDidKeyInProtocols
that will define how agent should format keys in protocols where keys are exchanged: 'naked' (base58) or did:key encoded.This setting will be used unless the other party has already sent us keys (for instance, a Mediator sent us the routing key in a Mediation Grant message). In such case, their format will be used in subsequent messages for that protocol (e.g. KeyList Update). This is achieved by adding a specific metadata key
UseDidKeysForProtocol
to the related connection record, which intended to track the other party support of did:key in different RFCs.Currently, only Coordinate Mediation (RFC 0211) uses this feature, but in further versions other protocols will use this metadata/settings key (such as Pickup V2 - RFC 0685).
For key reception, the agent will always accept both formats and internally store in base58.