Fix routing in set public did #2288
Conversation
Signed-off-by: Matus Kempa <[email protected]>
Signed-off-by: Matus Kempa <[email protected]>
Signed-off-by: Matus Kempa <[email protected]>
Signed-off-by: Matus Kempa <[email protected]>
Signed-off-by: Matus Kempa <[email protected]>
|
|
||
| routing_keys, mediator_endpoint = await route_manager.routing_info( | ||
| profile, | ||
| None, |
There was a problem hiding this comment.
Passed None as endpoint because it is later set to the default_endpoint in the promote_wallet_to_public
There was a problem hiding this comment.
To clarify, we set the value to None here but the returned mediator_endpoint will be set if there's a base mediator; only if there is no mediator in use would the mediator_endpoint come back as a None and then promote_wallet_public_did will overwrite the none with the default endpoint from our settings. Sound right to you?
There was a problem hiding this comment.
Yes, that's the case I was trying to describe. I was referring to the line 630 in this very file
There was a problem hiding this comment.
I think I'd like to see this changed up a bit; the logic would be easier to follow if the promote_wallet_public_did just accepted an endpoint (rather than a mediator_endpoint) and then when we call the route_manager.routing_info, we pass in the default endpoint value from settings. Fewer magical None values. However, I think that can wait for a different PR. Thanks for the clarification 🙂
|
Kudos, SonarCloud Quality Gate passed!
|
|
@usingtechnology @TimoGlastra This one is pretty straightforward; the set public DID endpoint was overlooked during recent passes improving handling of mediation for multitenancy. Timo, you've been around this code recently with #2243; your thoughts would be appreciated if you get a chance but, given the straightforwardness, I'm content to merge without additional feedback. |
Having a multitenancy mode and a base wallet mediator, when a DID was being promoted to public the base wallet mediator was not taken into consideration - the returned mediation record was always None. As a result, the agent's endpoint is written to ledger instead of the mediator's.
This PR fixes this issue.