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

DA ready consensus #205

Closed
wants to merge 2 commits into from
Closed

DA ready consensus #205

wants to merge 2 commits into from

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Mar 10, 2021

Description(TODO)

Depends on:

Closes: #181, closes #215, closes #85, closes #184

NOTE:
The PR is big and touches multiple issues at once, as I would like it to fully serve its goal in the name. Also, I'll do my best to make it easy to review via commit-by-commit reviewing, while preserving history.

@Wondertan Wondertan self-assigned this Mar 10, 2021
@celestiaorg celestiaorg deleted a comment from lgtm-com bot Mar 15, 2021
@liamsi
Copy link
Member

liamsi commented Mar 22, 2021

I'd recommend to think about ways to split this up into multiple Pull Requests instead. At least two, maybe even three or more pull requests. Roughly this could look like the following approach:

  1. one PR that (only) adds all LL specific fields / data structures to the consensus messages according to the spec (DA ready consensus #205)
  2. another one that makes use of them, additionally (but optionally) to the existing tendermint logic (partly dependent on Implement reading from IPLD merkle dag #194)
  3. one PR that deprecates the now obsolete tendermint fields: PartSetHeader, Parts etc (depends on Implement reading from IPLD merkle dag #194)
  4. One PR that changes the hash in the BlockID to a simple hash (instead of merkelization)

The nice thing about this approach is that 1. and somewhat 2. as well are independent of #194, #85, and #182 etc. and can be reviewed and merged independently and faster. Only after 3. we will have parity with the Spec and can close #181. In between we would have data structures that kinda support both: vanilla tendermint and LazyLedger main chain logic.

@liamsi liamsi mentioned this pull request Mar 24, 2021
@github-actions
Copy link

github-actions bot commented Apr 4, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Apr 4, 2021
@liamsi
Copy link
Member

liamsi commented Apr 6, 2021

@Wondertan I'll close this PR but will leave the branch around.

@liamsi liamsi closed this Apr 6, 2021
@Wondertan
Copy link
Member Author

@liamsi, ok. I was thinking about closing it as well at some point, as it does things in the wrong way.

@rootulp rootulp deleted the hlib/da-consensus branch September 22, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants