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.

Sign up for GitHub

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

Jump to bottom

Add da service to nomos node #404

Merged
merged 5 commits into from
Sep 18, 2023
Merged

Add da service to nomos node #404

merged 5 commits into from
Sep 18, 2023

Conversation

zeegomo
Copy link
Contributor

@zeegomo zeegomo commented Sep 13, 2023

This PR:

  • updates the data availability service to work with generic da protocols
  • add the data availability service to the nomos node

Could actually split this but it's not too big and you can find the features grouped by commits anyway

@zeegomo zeegomo requested a review from al8n September 13, 2023 14:07
@zeegomo zeegomo added the da label Sep 13, 2023
@zeegomo zeegomo added this to the DA milestone Sep 13, 2023
danielSanchezQ
danielSanchezQ reviewed Sep 13, 2023
View reviewed changes
nomos-core/src/da/mod.rs

// Construct a new instance
fn new(settings: Self::Settings) -> Self;
Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, I always forget about this.

@danielSanchezQ
Copy link
Collaborator

I rebased it to solve some conflicts, but now it just do not work for waku. I think it was working before. Can you confirm what did I miss @zeegomo. Sorry!

youngjoon-lee
youngjoon-lee approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@youngjoon-lee youngjoon-lee left a comment

Choose a reason for hiding this comment

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

👍

@zeegomo
Copy link
Contributor Author

zeegomo commented Sep 18, 2023 •
edited

I rebased it to solve some conflicts, but now it just do not work for waku. I think it was working before. Can you confirm what did I miss @zeegomo. Sorry!

I guess some conditional compilation stuff as I only implemented it for the libp2p backend, let me check

@danielSanchezQ
Copy link
Collaborator

I rebased it to solve some conflicts, but now it just do not work for waku. I think it was working before. Can you confirm what did I miss @zeegomo. Sorry!

I guess some conditional compilation stuff as I only implemented it for the libp2p backend, let me check

I think at this point we can ditch waku in the nomos binary itself (where the problems are iirc).

@zeegomo
Copy link
Contributor Author

zeegomo commented Sep 18, 2023

For now it's just a fix of a few lines, if we find it's increasingly difficult we can remove it in a different PR

@zeegomo zeegomo merged commit 5e19492 into master Sep 18, 2023
11 checks passed
@jakubgs jakubgs deleted the add-da-service branch February 15, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielSanchezQ danielSanchezQ danielSanchezQ approved these changes

@youngjoon-lee youngjoon-lee youngjoon-lee approved these changes

@al8n al8n al8n approved these changes

Assignees

@zeegomo zeegomo

Labels
da
Projects
None yet
Milestone
DA
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@zeegomo @danielSanchezQ @youngjoon-lee @al8n