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

Added README files in v2 proto directories. #812

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

cody-littley
Copy link
Contributor

Why are these changes needed?

Adds README files in directories containing API not yet in use.

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@cody-littley cody-littley self-assigned this Oct 16, 2024
Copy link
Contributor

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for this. would personally like to have a README in api/proto/README.md to describe v1 vs v2. Like an entrypoint README, that could link to the others.


# What is "v2"?

The EigenDA v2 Architecture is a fundamental redesign of the disperser, and by extension systems that interact
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: ... redesign of the protocol...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wording updated

# What is "v2"?

The EigenDA v2 Architecture is a fundamental redesign of the disperser, and by extension systems that interact
with the disperser. The v2 Architecture is an important stepping stone towards reducing centralization and for
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The v2 architecture improves robustness, efficiency, and compatibility with upcoming features such as permissionless disperser and data availability sampling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wording updated

@cody-littley
Copy link
Contributor Author

@samlaf

LGTM! thanks for this. would personally like to have a README in api/proto/README.md to describe v1 vs v2. Like an entrypoint README, that could link to the others.

I added a readme to the proto directory. Not sure if it makes sense to link it to all of the other ones. The README files are basically all just copies of one another. They are currently spread around in multiple locations to make it more likely that somebody will think to look inside it when they are peeking into the v2 files.

Signed-off-by: Cody Littley <[email protected]>
Copy link
Contributor

@ian-shim ian-shim left a comment

Choose a reason for hiding this comment

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

I feel like having one README at api/proto is sufficient, but I'll leave it up to you

@@ -0,0 +1,15 @@
# Should I use the v2 API?
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no indication api/proto/relay is part of v2. Maybe this README should be specific to relay or say something like relay is only applicable with v2 architecture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note that the code in the relay directory is experimental.

Signed-off-by: Cody Littley <[email protected]>
@cody-littley
Copy link
Contributor Author

I feel like having one README at api/proto is sufficient, but I'll leave it up to you

I've collapsed these README files into a single file.

@cody-littley cody-littley merged commit fca2539 into Layr-Labs:master Oct 21, 2024
7 checks passed
@cody-littley cody-littley deleted the v2_readme branch October 21, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants