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

ADR: Block Data Propagation #389

Closed
liamsi opened this issue Jun 3, 2021 · 6 comments
Closed

ADR: Block Data Propagation #389

liamsi opened this issue Jun 3, 2021 · 6 comments

Comments

@liamsi
Copy link
Member

liamsi commented Jun 3, 2021

Summary

A lot was discussed and said about block propagation (during consensus and in other cases) already. We should capture the decision on how to move forward with the implementation in an ADR.

Expectation / Details

The main purpose of this ADR is to focus on how to get the MVP out of the door (#381). It's OK if the initial design takes some shortcuts to get there faster if this does not introduce (substantial) technical debt.

Additionally, it can and should try to capture as much as possible from the different discussed approaches and tradeoffs. Similarly, it could touch upon future optimizations for iterations right after the MVP (nice-to-have).

If helpful, explorative implementation in parallel to the ADR is encouraged. At this stage, ideally a tracer-bullet and not a prototype. Where necessary a quickly written spike to verify the feasibility of the proposed solution can also be a good idea.

Some topics that could be part of this adr:

Difference to ADR 001

ADR 001 was written under the assumption that validators will also do DAS. Hence, every node-type could have used the ipld API RetrieveBlockData. Now, that we pivoted to removing that node-type (celestiaorg/celestia-specs#177 (review)), block propagation can and should be revisted. Particularly, during consensus, where it remains important that validators receive the block in a timely manner.

References and Context

@liamsi liamsi added this to To do in LazyLedger MVP via automation Jun 3, 2021
@liamsi liamsi added this to Sprint Pool (Unassigned) in Weekly Sprint (core/app/libs) via automation Jun 3, 2021
@liamsi liamsi moved this from Sprint Pool (Unassigned) to Assigned & Part of this Iteration in Weekly Sprint (core/app/libs) Jun 3, 2021
@liamsi liamsi changed the title ADR: Block Data Propagation ADR: Block Data Propagation during consensus Jun 3, 2021
@liamsi liamsi changed the title ADR: Block Data Propagation during consensus ADR: Block Data Propagation Jun 3, 2021
@Wondertan
Copy link
Member

@liamsi, regarding the granularity of the gossiped data. Some time ago you mentioned that gossiping BlockParts in Tendermint wasn't the best idea. The thing I wonder about is full-block data gossiping vs rows. I think the BlockParts thing is related here, so can you pls share your insights on that.

@liamsi
Copy link
Member Author

liamsi commented Jun 8, 2021

Some time ago you mentioned that gossiping BlockParts in Tendermint wasn't the best idea.

Yeah, gossiping block parts, at least in its current form, is not the best idea because we chunk the block differently already (into erasure coded shares). So IMO, we should stick to one approach.

The thing I wonder about is full-block data gossiping vs rows.

My intuition here is that gossiping rows is the best granularity: on the receiving side, you could check if the row you got matches a CID in the DAHeader (otherwise someone is giving you garbage data, e.g. to DoS your node). Remember that blocks could become potentially very large and we don't want to wait for the whole block until making the decision if the received data matches the commitment in the DAHeader or not.

An alternative to gossiping rows could be gossiping shares. This would be more similar to the current BlockParts. But then, we would need to give the inclusion proof with each share to prevent the above's scenario that someone feeds garbage blocks into the network and this would only be detected after downloading all garbage. This would have the major downside that a lot of duplicate data needs to be gossiped (all those inner proof nodes).

@liamsi
Copy link
Member Author

liamsi commented Jun 8, 2021

Actually, another idea is to only send half the rows: namely those parts that represent the original data. So the smallest piece or "part" that would be gossiped could be [original part of row] (and maybe a single [proof node] for the other half). From this the receiving part could already compute the other half (this is currently not supported by rsmt2d but we could change that and it plays into your thinking here btw: celestiaorg/rsmt2d#60).

@Wondertan
Copy link
Member

Yeah, gossiping block parts, at least in its current form, is not the best idea because we chunk the block differently already (into erasure coded shares).

Ok, but maybe I got your original concern wrongly. I thought the block parts are not a good idea in terms of tendermimt; for our case, it's obviously not a way to go for the reason you mentioned.

@liamsi
Copy link
Member Author

liamsi commented Jun 9, 2021

I thought the block parts are not a good idea in terms of tendermimt

There is nothing wrong with blockparts in tendermint. The only questionable thing is that the PartSetHeader is part of BlockID in the header and BlockID is used everywhere. So basically an implementation detail on how to get the block data to other nodes spills everywhere. There are several issues on how to get rid of PartSetHeader in the header; see for instance tendermint/spec#78, and tendermint/tendermint#7922

@liamsi liamsi moved this from Assigned & Part of this Iteration to Sprint Pool (Unassigned) in Weekly Sprint (core/app/libs) Jun 9, 2021
@liamsi liamsi moved this from Sprint Pool (Unassigned) to Assigned & Part of this Iteration in Weekly Sprint (core/app/libs) Jun 9, 2021
@Wondertan Wondertan moved this from Assigned & Part of this Iteration to Sprint Pool (Unassigned) in Weekly Sprint (core/app/libs) Jun 23, 2021
@liamsi
Copy link
Member Author

liamsi commented Aug 16, 2021

Closing as this is no longer relevant now. IMO we should merge the ADR that @Wondertan wrote (after this is briefly addressed).

@liamsi liamsi closed this as completed Aug 16, 2021
LazyLedger MVP automation moved this from To do to Done Aug 16, 2021
Weekly Sprint (core/app/libs) automation moved this from Sprint Pool (Unassigned) to Done Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

2 participants