refactor(p2p): use async/await syntax on peer discovery

[glevco]

Motivation

While studying for another project, I found out that our version of Twisted actually supports using the async/await python syntax, and even recommends it (docs):

Unless your code supports Python 2 (and therefore needs compatibility with older versions of Twisted), writing coroutines with the functionality described in “Coroutines with async/await” is preferred over inlineCallbacks. Coroutines are supported by dedicated Python syntax, are compatible with asyncio, and provide higher performance.

This has multiple benefits, improving readability, static type checking, and may even boost performance, according to the docs above.

We should aim to gradually convert this code, where it makes sense. Mostly, it makes sense to convert it everywhere the @inlineCallbacksdecorator is used, as the conversion is pretty straightforward. Places using direct Deferred callback manipulation require a bit more thought so we may not want to convert them. Ideally, new could should always use the new syntax.

Here's a small cheat sheet for conversion:

• For decorated functions/methods, simply change @inlineCallbacks to async and yield to await (Deferreds are Awaitable). Also, change the return type from Generator[_, _, T] to T.
• Deferreds are eager (they start running as soon as they're created) while coroutines (async functions) are lazy (they start running when they're awaited). This means that currently we have places in code where synchronous code ("non-inlineCallbacks") create a Deferred without any callbacks, so the behavior is "fire and forget". Since coroutines cannot be awaited in "non-async" functions, we need to use Deferred.fromCoroutine() in order to convert and automatically make them run.

It is my understanding that we could have only a single Deferred.fromCoroutine() in the code, in the outermost async function, and all subsequent usages could leverage the async/await syntax directly. This is currently not possible because of those "fire and forget" cases. They may indicate that we should explicitly handle those deferred's callbacks, improving robustness (since they're currently ignored), and then they could be converted to async/awaits too. That would bubble up async functions all the way to the startup of our code. This is a larger refactor and we could do it incrementally.

Acceptance Criteria

• Change all @inlineCallbacks/yield functions and methods related to peer discovery to use async/await instead. Also, update return types accordingly.
• Update HathorProtocol and BaseState to accept coroutines as commands.
• Update ConnectionsManager to fire and forget a coroutine.
• No behavior should be changed by this PR.

Checklist

• If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (a575945) 85.28% compared to head (4359181) 85.30%.

❗ Current head 4359181 differs from pull request most recent head f326512. Consider uploading reports for the commit f326512 to get more accurate results

Files	Patch %	Lines
hathor/p2p/peer_discovery.py	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
+ Coverage   85.28%   85.30%   +0.01%     
==========================================
  Files         283      282       -1     
  Lines       22421    22416       -5     
  Branches     3400     3399       -1     
==========================================
- Hits        19122    19121       -1     
  Misses       2610     2610              
+ Partials      689      685       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.