Bug fix waiting for non completing tasks to return

[ogenev]

I was unsuccessfully trying to connect trin-state with ddht using this guide.
After some debugging with netcat, the sockets and udp ports looked fine. Then I started to look into the code and realized that tokio;;join! is waiting for process_discv5_requests and process_jsonrpc_requests to complete before executing p2p.ping_bootnodes().await.unwrap();.

I'm new to tokio, but I think this is probably the simplest fix for now.

[KolbyML]

The changes look good :)

[mrferris]

This change also looks good to me. I don't see how the bootnode pinging could have been reached without your change.

If my git blame analysis is correct, it looks like it used to used spawn but @njgheorghita switched it to use join. Nick, any thoughts we haven't considered on this change?

[SamWilsn]

From a quick glance, this should probably be two spawns (to run the tasks in the background) and a select (to exit as soon as the first task exits) instead of a join.

[njgheorghita]

Yup this checks out with me, can't remember exactly why I switched it to join, but seems like it wasn't the right move.

[ogenev]

From a quick glance, this should probably be two spawns (to run the tasks in the background) and a select (to exit as soon as the first task exits) instead of a join.

I don't understand why we need tokio::select!, I guess we don't expect any of the tasks to exit?

[SamWilsn]

I don't understand why we need tokio::select!, I guess we don't expect any of the tasks to exit?

I don't think we strictly need select, but if either task panics, we probably want the whole application to exit (or possibly restart the task, or log, etc.)

[ogenev]

I don't think we strictly need select, but if either task panics, we probably want the whole application to exit (or possibly restart the task, or log, etc.)

This totally makes sense. I'm thinking of merging the PR as it is for now, because I'm not sure if we should dive into proper error handling at this time....