Upgrade to stable futures (#2). #100
Conversation
Using a LocalPool is also much closer to the previous tokio-based code which used the current-thread executor for all tasks.
|
A note on the test failure in this run of I can either take a closer look at that in the context of this PR or create a ticket for it, whatever is preferable. |
|
A separate ticket for the test failure sounds good, since it's not caused by this PR. |
| } | ||
| } | ||
| } | ||
|
|
||
| impl<H, N, E: Environment<H, N>> Unpin for BackgroundRound<H, N, E> where |
There was a problem hiding this comment.
I'm surprised this isn't an auto impl
|
To avoid any divergence between GRANDPA master and whatever Substrate is pointing to, I'd prefer to have a PR to Substrate ready to go before merging this. Pierre also started on one of those in paritytech/substrate#3909. When that or a similar PR is at least ready for review with CI passing, we can merge this |
|
I might have a go at updating Pierre's PR. |
This is a new version of #81 which was later reverted in #89. Compared to that PR, the following has changed:
master.futures-0.3.1andfutures_timer-2.0.2LocalPoolinstead of aThreadPool. The introduction of theThreadPoolin the tests in Update to stables futures #81 caused at least one test to be subject to a race condition (import_commit_for_any_round). While that can of course be addressed in the test, using aLocalPoolis much closer to the current tokio behaviour in the tests, where all futures spawned within thelazyblock are only polled once thelazyblock has completed, since these spawns happen in the context of a current-thread executor (although this was not even guaranteed by tokio). In any case, it makes the tests less fragile to reordering of statements, hence I went with theLocalPoolto simplify the tests.To not introduce too many changes (and because I want to rebase #93 in turn on top of this PR), I did not try to make use of async/await anywhere.