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

service/block: Implement block store #113

Merged
merged 7 commits into from
Oct 11, 2021
Merged

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Oct 5, 2021

This PR implements the block store which allows block data to be stored and retrieved via the provided DAGService. The block.Service receives an ipld.DAGService upon construction and uses it for writing and reading block data. Block data can be retrieved via the DataAvailabilityHeader.

Merging blocked by #110.

Resolves #47.
Related to #25.

@renaynay renaynay added area:block Raw blocks and erasure coded blocks area:repository Issues related to Node's or Core's repository labels Oct 5, 2021
@renaynay renaynay force-pushed the block-store branch 2 times, most recently from e9b49f8 to 133367f Compare October 6, 2021 11:32
@renaynay
Copy link
Member Author

renaynay commented Oct 6, 2021

A question this PR raises is how to best make the most recent (arbitrary number) number of blocks quickly available for sampling as those are most likely going to be sampled from the most following a new block event in the network. Do we store the block data in mem for a while and then pop it out after X number of new blocks?

@mattdf
Copy link

mattdf commented Oct 8, 2021

Do we store the block data in mem for a while and then pop it out after X number of new blocks?

This sounds reasonable to me, perhaps do what geth does and allow the user to specify a command line argument that sets the max RAM allowed to be used by the cache, so that very big blocks don't DoS nodes in the future. If a node has to restart for whatever reason, then you'd also have to rebuild this cache by reading the disk. I think some kind of ring buffer struct would be fine here since for sampling you'll want to access the entire block.

Current PR LGTM.

mattdf
mattdf previously approved these changes Oct 8, 2021
@renaynay renaynay requested a review from mattdf October 8, 2021 12:44
mattdf
mattdf previously approved these changes Oct 8, 2021
liamsi
liamsi previously approved these changes Oct 8, 2021
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

👍🏼

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
@renaynay renaynay dismissed stale reviews from liamsi and mattdf via 00d80f6 October 8, 2021 13:01
@renaynay renaynay requested review from liamsi and mattdf October 8, 2021 13:32
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

☀️🕺🏻🎉🍾

@renaynay renaynay merged commit 924ba74 into celestiaorg:main Oct 11, 2021
@renaynay renaynay deleted the block-store branch October 11, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:block Raw blocks and erasure coded blocks area:repository Issues related to Node's or Core's repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

block: Implement BlockStore
4 participants