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

Revert writing to local dag during proposal #436

Closed
3 of 4 tasks
liamsi opened this issue Jun 25, 2021 · 3 comments · Fixed by #493
Closed
3 of 4 tasks

Revert writing to local dag during proposal #436

liamsi opened this issue Jun 25, 2021 · 3 comments · Fixed by #493

Comments

@liamsi
Copy link
Member

liamsi commented Jun 25, 2021

Summary

Previously, we wanted to store on ipld (and provide to the underlying dht) as early as possible so that validators could do DAS. Hence, for instance the proposer calls PutBlock as early as possible. While it still could make sense to call putting and providing earlier, I would recommend treating the ipld store as a replacement for the tendermint block store instead. This means removing the calls to the ipld/write methods during proposal.

Details

It is not (consensus) critical to store the block ipfs earlier. Hence, we can simply store the block (during consensus) during finalizeCommit. We can remove the the earlier storing to ipfs for proposals now. Everything else is already part of separate issues/PRs.
Once below action items are tackled we have half the story of a "full node based" storage node (see #435). Although I agree that we should write an ADR describing them in more detail still. This issue is mostly about getting rid of storing to ipfs during proposal only. Otherwise proposers will store the block twice (once during proposal and once when finalizing the commit; at least currently).

Action Items

Update: We decided to move the IPLD/IPFS logic into a separate repository. Hence, this issue boils down to remove calling PutBlock during decideProposal.

References

Saving the block data to IPFS was also already done in: #374
Storing the block data exclusively in the ipld store will done in: #218
Related discussion: https://github.com/celestiaorg/lazyledger-core/pull/374/files#r645077665

@Wondertan
Copy link
Member

Wondertan commented Jun 25, 2021

@liamsi, During the proposal stage, nodes must verify the integrity of a block received from the proposer. To verify that they need to erasure code the block and build IPLD/NMT trees of it. If we want to decouple saving from the proposal stage, we need to do verification twice: during proposal and saving, where in the first case, we do not pass the visitor option. IMO, we should avoid double verification, though I agree that saving in finalizing makes more sense.

@liamsi
Copy link
Member Author

liamsi commented Jun 25, 2021

Yeah, good point. I did not mention the fact that we should still compute the EDS to be able to compute the data root correctly (sth that is currently done via storing). We need to think of ways to make this efficient. I think the erasure coding is the expensive part. Computing the eds could be "cached" (e.g. in a private field in the block) to only happen once. Computing the tree twice is probably very cheap; storing or even network IO is the expensive part.

(Note that if we switch block gossiping to using shares or even rows like in #434 we would be computing the tree or subtrees for verification as well and the latter quite often; not only during saving).

@Wondertan
Copy link
Member

Let's introduce caching. I’ll try caching ExtendedDataSqaure in on block in #427

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 a pull request may close this issue.

2 participants