chore: Da dispersal unit tests update

[romanzac]

Batch of tests for data availability - dispersal to improve coverage.

[romanzac]

Hi @bacv, I've been trying to write error cases for behavior, after difficulties moved to swarm testing. Let's review test_dispersal_with_swarms and see what to do next ?

[bacv]

After dialing, executor needs to signal the other end about new stream:

let executor_open_stream_sender = executor.open_stream_sender();
executor_open_stream_sender.send(*validator_peer).unwrap();

To have open_stream_sender you'd need to target the da-integration-tests branch.

[romanzac]

I could not wrap my head around one problem. When I call executor_disperse_blob_sender.send(), handle_dispersal_event() is never called from run().loop. What else should I do ?

[romanzac]

I feel like our Swarm abstractions are only wrappers around libp2p's concepts? And we just forward all good and bad into upper layers.

[danielSanchezQ]

I feel like our Swarm abstractions are only wrappers around libp2p's concepts? And we just forward all good and bad into upper layers.

Well, they are. This is just a networking layer, it should not do anything else that forward/received messages according to the specs.

[bacv]

I could not wrap my head around one problem. When I call executor_disperse_blob_sender.send(), handle_dispersal_event() is never called from run().loop. What else should I do ?

If you inspect the executor dispersal behaviour, you'll see that before the blob is sent, the peer_id for desired subnetwork need to be available (.../executor/behaviour.rs:289). For this reason try to apply the changes I suggested and call open_stream_sender().

[romanzac]

Let me invest one more day and finish this exercise. Perhaps it will be useful for error case tests which require deep understanding of streams.

[romanzac]

I feel like our Swarm abstractions are only wrappers around libp2p's concepts? And we just forward all good and bad into upper layers.

Well, they are. This is just a networking layer, it should not do anything else that forward/received messages according to the specs.

Let me study Swarms in lipp2p more. Both go-waku and nwaku don't use them, that's why I need to have a deeper look.

You can think of Swarm as a hook or controller for all the underlying behaviours.

I feel uncomfortable we bring channels/hooks into higher levels. Channels are like pipes, no house has them visible unless it is for aestetical reasons :)

Here is an example how things get confusing:
Once dispersal_broadcast_sender became dispersal_events_sender. Just one level away we already start forgetting and shifting. Let my un-smartness be a guide here.

nomos-node/nomos-cli/src/da/network/swarm.rs

Line 47 in 9f4f139

dispersal_broadcast_sender: UnboundedSender<DispersalEvent>,

nomos-node/nomos-cli/src/da/network/backend.rs

Line 95 in 9f4f139

let (dispersal_events_sender, dispersal_events_receiver) = unbounded_channel();

@danielSanchezQ Could not we have something like Event Manager which understands messages, errors? Backend user could just drop data and it will be taken care of ? Any performance penalty for this one more redirect ? WDYT ?

[danielSanchezQ]

Just one level away we already start forgetting and shifting
what do you mean by this?

[romanzac]

Just one level away we already start forgetting and shifting
what do you mean by this?

Channel with the same purpose has different names.

[danielSanchezQ]

Just one level away we already start forgetting and shifting
what do you mean by this?

Channel with the same purpose has different names.

Well, yeah, we could make it more consistent in naming. But having the channels go away the layer should be good. I may be missing the point, but I do not think having an extra layer (event manager) would solve anything.

We usually have 3 layers:

-----------------------------
 Core (behaviours + Swarm)
-----------------------------
Backend (network backend service)
-----------------------------
Service
-----------------------------

The most important one is the Service, as the API should not change at all if possible. Imagine we change the core for something else (my most famous exaple: Pigeons). We just need to create a new (pigeons) backend while the service is left untouched. That makes pieces to be changed easily, is basically changing the type in the final overwatch app.

What I wanted to say with all this is that:

• Wrapping types is ok for the service and backend, if changing types is just a matter of rewrap (into the stream from whatever).
• Escaping types are not, otherwise is difficult to refactor.

[bacv]

It would be great if you could update the asserts part to listen for responses, we should consider test successful if for all dispersed blobs we'd get DispersalEvent::DispersalSuccess.

[romanzac]

Fixes after pair coding session at 3bdf9a2

[bacv]

Looks good! Thank you!