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

Replace mixnet backend with mixnet addon #615

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Replace mixnet backend with mixnet addon #615

merged 3 commits into from
Mar 19, 2024

Conversation

youngjoon-lee
Copy link
Contributor

@youngjoon-lee youngjoon-lee commented Mar 19, 2024

Summary

  • Removed the mixnet backend. Now we have only one network backend: libp2p.
  • Added the mixnet add-on (feature) for the libp2p network backend.

Details

I've felt that it's not natural to have the "mixnet network backend" separately from the libp2p network backend. Now I think it's more natural to have only one network backend (libp2p) for now and have mixnet as an addon for the libp2p network backend. There are several reasons:

  • If we want to adopt a new network backend such as waku or anything else, do we have to also add another mixnet network backend for that, as below?
    backend/
      libp2p/
      mixnet-libp2p/
      waku/
      mixnet-waku/
    
  • The APIs of both libp2p and mixnet network backend are 100% identical. So, I made the libp2p network adapters (for consensus, DA, and mempool) and other parts work for both libp2p and mixnet backend. It was simple to do that, thanks to the conditional compilation in Rust. But, I've felt that code looks unnatural. If both backends are identical, and if both are based on libp2p underneath, why should we have two network backend? I might be able to refactor network adapters to select one of network backends smoothly but I thought it's not really needed for now because we actually have only one networking mechanism (libp2p) now.

There might be several solutions, but I end up thinking about making the mixnet as an add-on for each network backend, so that we can enable/disable mixnet no matter which network backend we choose. Of course, we're going to make the mixnet mandatory in production, but please be remined that we've discussed to make it opt-in for easy development for the time being.
With this structure, we will have only one network backend (libp2p). If we build the nomos-node with only the libp2p feature, the mixnet is disabled.

cargo build --all --no-default-features --features libp2p

If we build it with both libp2p and mixnet feature, the mixnet is enabled for the libp2p network backend.

cargo build --all --no-default-features --features libp2p,mixnet 

@youngjoon-lee youngjoon-lee self-assigned this Mar 19, 2024
@youngjoon-lee youngjoon-lee marked this pull request as ready for review March 19, 2024 09:02
Copy link
Member

@bacv bacv left a comment

Choose a reason for hiding this comment

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

I agree that having mixnet as feature in libp2p feels less confusing, nice re-mix.

Copy link
Collaborator

@danielSanchezQ danielSanchezQ left a comment

Choose a reason for hiding this comment

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

This looks fair. It also makes sense, as we want plain libp2p (for testing mostly) and libp2p+mixnet (production).

@youngjoon-lee youngjoon-lee changed the base branch from mixnet-v1-integration-tests to master March 19, 2024 10:12
@youngjoon-lee youngjoon-lee merged commit d449114 into master Mar 19, 2024
13 checks passed
@youngjoon-lee youngjoon-lee deleted the re-mix branch March 19, 2024 10:15
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

3 participants