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

share: Availability interface and implementation #171

Merged
merged 25 commits into from
Nov 10, 2021
Merged

share: Availability interface and implementation #171

merged 25 commits into from
Nov 10, 2021

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Nov 5, 2021 •
edited
Loading

Content

  • Extends share package with a new Availability interface to check SharesAvailable
  • Implements LightAvailability that uses DataAvailability sampling technic
  • Fixes tests flakiness and does some minor changes to the existing test suite in service/share

@Wondertan Wondertan self-assigned this Nov 5, 2021
@Wondertan Wondertan mentioned this pull request Nov 8, 2021
Closed
@Wondertan Wondertan changed the title das: DASer interface and implementation share: Availability interface and implementation Nov 9, 2021
This was referenced Nov 9, 2021
Merged
Closed
Closed
Merged
renaynay
renaynay requested changes Nov 9, 2021
View reviewed changes
das/daser.go Outdated Show resolved Hide resolved
das/daser.go Outdated Show resolved Hide resolved
ipld/write.go Outdated Show resolved Hide resolved
service/share/availability.go Outdated Show resolved Hide resolved
service/share/light_availability.go Outdated Show resolved Hide resolved
service/share/light_availability.go Outdated Show resolved Hide resolved
service/share/sample.go Outdated Show resolved Hide resolved
service/share/sample.go Outdated Show resolved Hide resolved
service/share/sample.go Outdated Show resolved Hide resolved
service/share/sample.go Show resolved Hide resolved
@Wondertan Wondertan force-pushed the hlib/daser branch 2 times, most recently from 16956e1 to 1bb2126 Compare November 9, 2021 15:12
renaynay
renaynay previously approved these changes Nov 9, 2021
View reviewed changes
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

👍🏻

adlerjohn
adlerjohn requested changes Nov 9, 2021
View reviewed changes
ipld/write.go Outdated Show resolved Hide resolved
service/share/light_availability.go Outdated Show resolved Hide resolved
service/share/sample.go Show resolved Hide resolved
adlerjohn
adlerjohn previously approved these changes Nov 9, 2021
View reviewed changes
@Wondertan Wondertan requested a review from renaynay November 9, 2021 20:24
@Wondertan
Copy link
Member Author

Extracted flaky test fixing commits into celestiaorg/celestia-app#1639

Wondertan and others added 17 commits November 10, 2021 16:04
@Wondertan
fix(das): use Share Root instead of header.DataAvailabilityHeader
03eab90 
.
@Wondertan
logs(service/share): add logger
0f67722
@Wondertan
fix(service/share): a better place for sampling logic
e4415f7
@Wondertan
feat(service/share): define Availability interface
5f0c8af
@Wondertan
feat(service/share): light availability
90058e8
@Wondertan
tests(service/share): light availability testing
4d89b3b
@Wondertan
das: move things out
bbbc487
@Wondertan
feat(service/share): integrate service with availability
7b67a49
@Wondertan
feat(node/services): integrate Availability
fba3362
@Wondertan
docs(service/share): add comments to Availability and lightAvailavility
a95feea
@Wondertan @renaynay
Update service/share/availability.go
1796568 
Co-authored-by: rene <[email protected]>
@Wondertan @renaynay
Update service/share/light_availability.go
6bd0fbd 
Co-authored-by: rene <[email protected]>
@Wondertan @renaynay
Update service/share/light_availability.go
8d9a6d4 
Co-authored-by: rene <[email protected]>
@Wondertan @renaynay
Update service/share/sample.go
68833b8 
Co-authored-by: rene <[email protected]>
@Wondertan @renaynay
Update service/share/sample.go
49563c7 
Co-authored-by: rene <[email protected]>
@Wondertan
fix(service/share): review fixes
75d586f
@Wondertan
fix(service/share): use RowRoots instead of ColumnRoots
65555ef
adlerjohn
adlerjohn approved these changes Nov 10, 2021
View reviewed changes
Copy link
Member

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

lgtm

renaynay
renaynay approved these changes Nov 10, 2021
View reviewed changes
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

lfg

@Wondertan Wondertan merged commit 2adcfc6 into main Nov 10, 2021
@Wondertan Wondertan deleted the hlib/daser branch November 10, 2021 15:44
liamsi
liamsi reviewed Nov 10, 2021
View reviewed changes
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.

Joining too late to the party. Saw this was already merged 💪🏼

node/services/service.go
func Share(lc fx.Lifecycle, dag ipld.DAGService) share.Service {
service := share.NewService(dag)
func Share(ctx context.Context, lc fx.Lifecycle, dag ipld.DAGService) share.Service {
avail := share.NewLightAvailability(merkledag.NewSession(fxutil.WithLifecycle(ctx, lc), dag))
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we should not directly use the fxutil.WithLifecycle(ctx, lc) as a param instead?

service/share/light_availability.go
getter format.NodeGetter
}

// NewLightAvailability creates a new Light DataAvailability.
Copy link
Member

Choose a reason for hiding this comment

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

~~What is a Light DataAvailability?~~NVM, just saw the comment on top of lightAvailability. The comment here is still somewaht confusing imo.

@Wondertan Wondertan mentioned this pull request Nov 13, 2021
Merged
@Wondertan Wondertan restored the hlib/daser branch January 7, 2022 14:40
@Wondertan Wondertan deleted the hlib/daser branch January 7, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamsi liamsi liamsi left review comments

@adlerjohn adlerjohn adlerjohn approved these changes

@renaynay renaynay renaynay approved these changes

@mattdf mattdf Awaiting requested review from mattdf

Assignees

@Wondertan Wondertan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Wondertan @adlerjohn @liamsi @renaynay