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.

Sign up for GitHub

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

Jump to bottom

feat(shwap): Add eds store #3545

Merged
merged 20 commits into from
Jul 18, 2024
Merged

feat(shwap): Add eds store #3545

merged 20 commits into from
Jul 18, 2024

Conversation

walldiss
Copy link
Member

@walldiss walldiss commented Jul 2, 2024

Add eds store engine
Based on #3543

@walldiss walldiss added kind:feat Attached to feature PRs shwap labels Jul 2, 2024
@walldiss walldiss self-assigned this Jul 2, 2024
@walldiss walldiss changed the base branch from main to shwap July 2, 2024 14:53
@walldiss walldiss force-pushed the add-eds-store branch 3 times, most recently from 5476dea to 265bee2 Compare July 4, 2024 14:40
@walldiss walldiss marked this pull request as ready for review July 4, 2024 14:40
@walldiss walldiss mentioned this pull request Jul 6, 2024
Merged
@walldiss walldiss requested a review from cristaloleg as a code owner July 10, 2024 08:15
@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 53.93586% with 158 lines in your changes missing coverage. Please review.

Please upload report for BASE (shwap@198f24e). Learn more about missing BASE report.

Files Patch % Lines
store/store.go 71.50% 31 Missing and 24 partials ⚠️
store/metrics.go 22.38% 52 Missing ⚠️
store/file/q1q4_file.go 6.66% 14 Missing ⚠️
store/cache/doublecache.go 0.00% 10 Missing ⚠️
share/new_eds/rsmt2d.go 0.00% 5 Missing ⚠️
store/cache/noop.go 0.00% 5 Missing ⚠️
share/new_eds/close_once.go 0.00% 4 Missing ⚠️
store/store_options.go 60.00% 2 Missing and 2 partials ⚠️
store/file/header.go 25.00% 2 Missing and 1 partial ⚠️
share/new_eds/proofs_cache.go 0.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files 
@@           Coverage Diff            @@
##             shwap    #3545   +/-   ##
========================================
  Coverage         ?   47.55%           
========================================
  Files            ?      316           
  Lines            ?    18561           
  Branches         ?        0           
========================================
  Hits             ?     8827           
  Misses           ?     8606           
  Partials         ?     1128

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store_options.go Outdated Show resolved Hide resolved
store/store_options.go Outdated Show resolved Hide resolved
share/new_eds/accessor.go Outdated Show resolved Hide resolved
share/new_eds/rsmt2d.go Outdated Show resolved Hide resolved
store/store_options.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
share/new_eds/accessor.go Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store_options.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/file/ods.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
state/core_access.go Outdated Show resolved Hide resolved
store/file/ods.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store_test.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store_test.go Show resolved Hide resolved
store/store_test.go Outdated Show resolved Hide resolved
store/store.go Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store.go Outdated Show resolved Hide resolved
store/store.go Show resolved Hide resolved
Wondertan
Wondertan reviewed Jul 17, 2024
View reviewed changes
store/store.go Outdated Show resolved Hide resolved
@walldiss
Copy link
Member Author

We both want to shwap sooner, but lets keep using review to discuss changes first. instead of committing your ideas to PR first and discussing afterwards.

@Wondertan
Copy link
Member

Yeah, doing in a diff PR pointing to this PR would be better

@walldiss walldiss merged commit 2e80fc6 into celestiaorg:shwap Jul 18, 2024
25 checks passed
@Wondertan Wondertan mentioned this pull request Jul 22, 2024
Merged
Wondertan added a commit that referenced this pull request Jul 22, 2024
@Wondertan
refactor(shwap): revert DataRoot -> DataHash (#3585)
d807843 
In #3545, I requested to rename `DataHash` to `DataRoot` to match the name of the original name in the Core's RawHeader. However, I realized it is called `DataHash` in the header, so this PR reverts that change. I think this confusion comes from the fact that we are always referring to this field as `DataRoot` in our discussions, so it feels like this is the correct phrasing, but it's not. 
It actually does not matter how it's called, honestly, and both are correct in some way, but it's good to be consistent in the code and avoid confusion with tautology.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wondertan Wondertan Wondertan approved these changes

@renaynay renaynay Awaiting requested review from renaynay

@vgonkivs vgonkivs Awaiting requested review from vgonkivs

@distractedm1nd distractedm1nd Awaiting requested review from distractedm1nd

@ramin ramin Awaiting requested review from ramin

@cristaloleg cristaloleg Awaiting requested review from cristaloleg

Assignees

@walldiss walldiss

Labels
kind:feat Attached to feature PRs shwap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@walldiss @codecov-commenter @Wondertan