Add endpoint to get managed addresses #556
Conversation
arun-koshy
requested review from
huitseeker,
patrickkuo,
sblackshear and
oxade
| rqctx: Arc<RequestContext<ServerContext>>, | ||
| ) -> Result<HttpResponseOk<GetAddressResponse>, HttpError> { | ||
| let server_context = rqctx.context(); | ||
| let wallet_context = server_context.wallet_context.lock().unwrap().take(); |
There was a problem hiding this comment.
The good thing is after this line you now own the contents of wallet_context. The bad thing is you just replaced the original by None, so if you do an early return anywhere before you hit the line where you place the wallet_context back where you found it:
*server_context.wallet_context.lock().unwrap() = Some(wallet_context);then any further actions don't have a wallet_context any more!
This doesn't matter for the very next error (conditioned on wallet_context.is_none()), but the two following errors (out of get_or_create_client_state and sync_client_state) are a concern.
There was a problem hiding this comment.
I ran into just this issue yesterday in some other endpoints! I have been trying to find a solution where I don't have to add that line to every endpoints early exit scenario (which gets really repetitive in future endpoints). However have yet to find anything outside of take() that doesn't cause the function change we saw earlier.
I will add the line to put wallet context back on an early exit and add a todo to find a better solution for this.
Thank you for the continued pro rust tips! |
This is the second of a series of PR's that will implement the client service API described as part of the GDC Eng Deliverables
The endpoints that are included in this PR are:
Get Addresses: Retrieve all managed accounts.
curl --location --request GET 'http://localhost:5000/wallet/addresses'