Skip to content

DA ready consensus#205

Closed
Wondertan wants to merge 2 commits intomasterfrom
hlib/da-consensus
Closed

DA ready consensus#205
Wondertan wants to merge 2 commits intomasterfrom
hlib/da-consensus

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