Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixnet v1: Skeleton #570

Merged
merged 4 commits into from
Jan 31, 2024
Merged

Mixnet v1: Skeleton #570

merged 4 commits into from
Jan 31, 2024

Conversation

youngjoon-lee
Copy link
Contributor

@youngjoon-lee youngjoon-lee commented Jan 29, 2024

Added the skeleton of mixnet backend for the network service

From mixnet v1, we will abstract mixnet as a network service, so upper layers (e.g. consensus) just focus on sending/receiving messages. Upper layers don't need to take care of which transport/algorithm are used in the network service

@youngjoon-lee youngjoon-lee self-assigned this Jan 29, 2024
Comment on lines 24 to 27
pub enum RobustnessMsg {
/// Inject a new entropy that will be used for configuration update logic
SetEntropy(Box<u8>),
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now it is ok. But at some point when writing the new consensus we would probably have a consensus event system to subscribe to consensus events. I would like for consensus to not be responsible on when to actually call this. @zeegomo WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree. It would be great if consensus doesn't need to care about when this action should be called, as we've discussed before. If you guys have the same idea, I'll add a comment in the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should probably make a NewEntropy message available.
Apart from this, not sure the robustness layer should exist as its own separate service. It only makes sense with a mixnet, couldn't it be an implementation detail of the mixnet service?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @zeegomo , Rosbustness shouldn't be a service in nomos-nodo per se. As it is something just the mixnet needs. I think it should be embedded in the mixnet service itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our code structure, it makes sense to embed robustness in the mixnet service. But then I still have a doubt why we need to have the robustness layer separately in the spec (as designed when I, you, and Marcin discussed). Of course, I know it was the spec-level discussion. But, I thought the robustness layer will control other configs for other layers in the future, as a mediator of config correlation between layers. If not, I don’t fully understand yet why the robustness layer should exist separately. Maybe this is an implementation detail that doesn't need to be covered in the spec. I opened a discussion in Notion to double check with Marcin.

For v1, it is only for mixnet. I can embed it into the network service.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeegomo @danielSanchezQ @al8n
Okay, I don't need to think complicatedly. The robustness layer doesn't need to a 'service' in this implementation. The robustness layer can be implemented conceptually and clearly within a mixnet crate implementation. I just removed the robustness service 3136224. Also, I removed unclear commands and added a comment that the mixnet service should subscribe NewEntropy from the consensus service.

@youngjoon-lee youngjoon-lee marked this pull request as ready for review January 29, 2024 11:55
@youngjoon-lee youngjoon-lee changed the title Mixnet v1 skeleton Mixnet v1: Skeleton Jan 29, 2024
nomos-services/network/src/backends/mixnet/mod.rs Outdated Show resolved Hide resolved
Comment on lines 24 to 27
pub enum RobustnessMsg {
/// Inject a new entropy that will be used for configuration update logic
SetEntropy(Box<u8>),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should probably make a NewEntropy message available.
Apart from this, not sure the robustness layer should exist as its own separate service. It only makes sense with a mixnet, couldn't it be an implementation detail of the mixnet service?

@youngjoon-lee youngjoon-lee merged commit 1f66aa9 into mixnet-v1 Jan 31, 2024
7 checks passed
@youngjoon-lee youngjoon-lee deleted the mixnet-v1-skeleton branch January 31, 2024 08:25
youngjoon-lee added a commit that referenced this pull request Mar 12, 2024
* base

* Remove mixnet client from libp2p network backend (#572)

* Mixnet v1: Remove all mixnet legacies: mixnet crate, mixnode binary, tests, and docker (#573)

* Mixnet v1: Skeleton (#570)

* Use QUIC for libp2p (#580)

* Add Poisson interval function for Mixnet (#575)

* Mixnet network backend skeleton (#586)

* Libp2p stream read/write (#587)

* Emitting packets from mixclient using libp2p stream (#588)

* Handle outputs from mixnode using libp2p stream/gossipsub (#589)

* Refactor poisson (#590)

* Mix client Poisson emission (#591)

* Mix node packet handling (#592)

* Mix Packet / Fragment logic (#593)

* Move FisherYates to `nomos-utils` (#594)

* Mixnet topology (#595)

* Mix client/node unit tests (#596)

* change multiaddr from tcp to udp with quic-v1 (#607)

---------

Co-authored-by: Al Liu <scygliu1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants