reth icon indicating copy to clipboard operation
reth copied to clipboard

Unblock all `Stream` and `Future` impl in `net` crate

Open emhane opened this issue 1 year ago • 3 comments

Describe the feature

Firstly all poll or poll_next functions in the net crate should be inside an implementation of Future or Stream, preferably Stream if the behaviour is stream-like.

Secondly, replace any loops in Stream and Future implementations in the net crate that risk blocking, for example all while let .. loops. Replace these with the preemption design pattern if prioritisation should apply (used in NetworkManager and TransactionsManager), otherwise let tokio decided how tasks are interleaved by just removing the loop, i.e. while let .. becomes if let ... Caution in Future impl with loops! They are likely not polled in a loop, rather spawned in a thread and not polled again other than behind the scenes by tokio. Here cx.waker().wake_by_ref() must be called before returning Poll::Pending.

Additional context

No response

### Tasks
- [ ] https://github.com/paradigmxyz/reth/issues/6336
- [ ] https://github.com/paradigmxyz/reth/pull/6151
- [ ] https://github.com/paradigmxyz/reth/issues/6589
- [ ] https://github.com/paradigmxyz/reth/pull/6640
- [ ] https://github.com/paradigmxyz/reth/issues/6643
- [ ] https://github.com/paradigmxyz/reth/issues/6642
- [ ] https://github.com/paradigmxyz/reth/pull/6639
- [ ] https://github.com/paradigmxyz/reth/pull/6651
- [ ] https://github.com/paradigmxyz/reth/pull/6656
- [ ] https://github.com/paradigmxyz/reth/issues/6686
- [ ] https://github.com/paradigmxyz/reth/issues/7008

emhane avatar Feb 10 '24 20:02 emhane

most of them don't benefit from being stream.

the network future has preemption on the NetworkManager top layer

mattsse avatar Feb 10 '24 23:02 mattsse

most of them don't benefit from being stream.

ok, noted.

the network future has preemption on the NetworkManager top layer

which is the network future? let's add this to docs of the network future then to improve maintainability for anyone else wondering about this in the future

emhane avatar Feb 11 '24 11:02 emhane

polling underlying streams in the Stream impl for Swarm in a potentially blocking way, won't be intercepted by the budget in NetworkManager

emhane avatar Feb 11 '24 12:02 emhane

completed/not-planned

mattsse avatar Aug 20 '24 09:08 mattsse