fix: mediation webhook routing keys #2502
Conversation
anwalker293
changed the title
Signed-off-by: Alex Walker <[email protected]>
Signed-off-by: Alex Walker <[email protected]>
Signed-off-by: Alex Walker <[email protected]>
Signed-off-by: Alex Walker <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
dbluhm
force-pushed
the
fix/mediation-webhook-routing-keys
branch
from
86ad52e to
3f0d1a6
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| v0.11.0: | ||
| update_routing_keys: true |
There was a problem hiding this comment.
Assuming this is merged in for 0.11.0, I believe this is the correct way to specify that our upgrade routine should be run? @shaangill025
| async def update_routing_keys(profile: Profile): | ||
| """Update routing keys previously stored in wallet. | ||
|
|
||
| This update step will transform the stored routing keys into did:key values | ||
| from raw base58 encoded values. | ||
|
|
||
| Steps: | ||
| for each mediation record stored in the wallet: | ||
| ensure the stored routing_keys list is formated as did:keys | ||
| save the record if modified | ||
| for each routing record stored in the wallet: | ||
| ensure the stored recipient keys are formated as did:keys | ||
| save the record if modified | ||
| """ |
There was a problem hiding this comment.
This is the routine we've added for updating the routing keys. The goal of doing the upgrade this way was to ensure that we don't have to keep base58 -> did:key translation code on the MediationRecord and RouteRecord hydration methods from now until the rest of forever. Is this the right approach? Do we anticipate the upgrade script will continue to be used in this way in the future? Does this logic belong here or should it go somewhere else?
|
Marking as draft temporarily: I need to review a couple of instances of these values being used. |
|
I believe I was overzealous with my recommended changes. To address the linked issues, we need only modify the mediation record. It is probably a good idea to limit this PR to just addressing the mediation record routing key representation. Changing route record would support changing over to sending and receiving |
|
Closing this PR; the upgrade process was an interesting exercise but ultimately the two issues are better addressed by a smaller change set found in #2516. We'll hold onto these changes in this branch in case they become relevant (if we needed to update the |
Re: #2357 and #2492
Indicio PR: Indicio-tech#153
Tagging and credit due to @dbluhm