fix(l2): proof_coordinator genserver locked by tcp listen loop

[LeanSerra]

Motivation

Currently the proof_coordinator Genserver can only handle a single cast message Listen { listener: Arc<TcpListener> } as the handler for this messages starts an infinite loop that prevents other cast messages to be handled. This PR fixes that issue by moving the tcp loop into its own task.

Description

• Move tcp listening loop to spawned spawn_listener task
• Remove ConnectionHandler genserver
• Now the spawn_listener task will send a cast message to the proof_coordinator genserver, which will process the request and send the response.

Closes #3316

[github-actions]

Lines of code report

Total lines added: 0
Total lines removed: 58
Total lines changed: 58

Detailed view

+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/proof_coordinator.rs | 517   | -58  |
+-------------------------------------------------+-------+------+

[MegaRedHand]

LGTM

[MegaRedHand]

Why do we need to wrap the stream in an Arc here?

[LeanSerra]

Basically because ProofCordInMessage needs to implement Clone and TcpStream is not clonable.

[MegaRedHand]

You're right. After the question I checked in spawned and GenServer messages need to be Clone 😅
Well, at least until lambdaclass/spawned#56 is merged 😎

[Oppen]

And are we sure there are no good reasons for that to not be Clone? A TCP stream is not a datagram oriented socket, meaning you could race to read and write if accessing from several tasks or interleaved inside a task.
I suspect we just want the ProofCoordinator to spawn a task per connection instead. That task would do the infinite loop for as long as the connection is alive, while the ProofCoordinator is free to handle the next cast message.

[LeanSerra]

You are right! I made some changes in this commit.

Now we spawn a task that loops doing listener.accept().await. When it successfully accepts a connection it deserializes the request and then passes it and the stream over to the genserver to handle the response.
Right now I still need to wrap it in an Arc but, if the Clone requirements are removed from spawned, we can be 100% sure that only one task can have ownership of the stream at a given time so we have no races.

It might be better to spawn a new task per connection, each doing the deserializing of the request, and sending those requests to the genserver.

[MegaRedHand]

Looks better now