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

feat(shwap/ods_file): tail padding trim #3620

Merged
merged 3 commits into from
Aug 15, 2024
Merged

feat(shwap/ods_file): tail padding trim #3620

merged 3 commits into from
Aug 15, 2024

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Aug 3, 2024

Implements tail padding share trimming a.k.a. compression with supporting changes. The actual trimming logic turned out extremely simple and does not even require any form of indexing. (896bdd3)

This PR is reviewable commit-by-commit and is supposed to merge as so.

Depends on #3643

@Wondertan Wondertan added kind:testing Related to unit tests kind:feat Attached to feature PRs kind:refactor Attached to refactoring PRs shwap labels Aug 3, 2024
@Wondertan Wondertan self-assigned this Aug 3, 2024
share/share.go Outdated Show resolved Hide resolved
Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

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

Nice trick with io.EOF triggering padding!

share/new_eds/read.go Outdated Show resolved Hide resolved
share/new_eds/testing.go Outdated Show resolved Hide resolved
share/new_eds/testing.go Show resolved Hide resolved
store/file/ods.go Outdated Show resolved Hide resolved
@Wondertan Wondertan marked this pull request as draft August 11, 2024 13:49
@Wondertan Wondertan changed the base branch from shwap to shwap-decouple-files August 11, 2024 13:50
@Wondertan Wondertan force-pushed the shwap-decouple-files branch from 4cccb18 to 5a4dff4 Compare August 11, 2024 14:00
@Wondertan Wondertan force-pushed the shwap-decouple-files branch from 5a4dff4 to 5b76ed5 Compare August 11, 2024 18:12
@Wondertan Wondertan force-pushed the shwap-trim branch 2 times, most recently from 5ccec35 to e242456 Compare August 11, 2024 18:20
@Wondertan Wondertan force-pushed the shwap-decouple-files branch 6 times, most recently from 21198cc to 9e6bcb9 Compare August 13, 2024 17:41
@Wondertan Wondertan force-pushed the shwap-decouple-files branch 6 times, most recently from a8c83e7 to 4ee148f Compare August 14, 2024 17:02
Base automatically changed from shwap-decouple-files to shwap August 14, 2024 17:24
@Wondertan Wondertan marked this pull request as ready for review August 14, 2024 17:53
share/new_eds/read.go Outdated Show resolved Hide resolved
share/new_eds/read.go Show resolved Hide resolved
func testEDSes(t *testing.T, size int) map[string]*rsmt2d.ExtendedDataSquare {
fullEDS := edstest.RandEDS(t, size)

padding := rand.IntN(size * size)
Copy link
Member

Choose a reason for hiding this comment

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

Should be at least 1 padding share

Copy link
Member Author

Choose a reason for hiding this comment

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

done

store/file/header.go Show resolved Hide resolved
for i := range shares {
shares[i] = axsData[i*shrLn : (i+1)*shrLn]
if i > shrsRead-1 {
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, where is -1 comes from?

Copy link
Member Author

@Wondertan Wondertan Aug 15, 2024

Choose a reason for hiding this comment

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

number of read shares starts from 1, while i from 0, so shrsRead has to be shifted

Comment on lines +109 to +112
if share.GetNamespace(shr).Equals(share.TailPaddingNamespace) {
return nil
}

Copy link
Member

Choose a reason for hiding this comment

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

Need test for ods file to ensure file store was actually trimmed

@Wondertan Wondertan force-pushed the shwap-trim branch 2 times, most recently from 37648e7 to 2e27c60 Compare August 15, 2024 12:54
Besides it parrallelizes the test to run them quicker
Implements tail padding share trimming a.k.a. compression with supporting changes.
The actual trimming logic turned out extremely simple and does not even require any form of indexing.
@Wondertan Wondertan merged commit 01b1655 into shwap Aug 15, 2024
28 of 29 checks passed
@Wondertan Wondertan deleted the shwap-trim branch August 15, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feat Attached to feature PRs kind:refactor Attached to refactoring PRs kind:testing Related to unit tests shwap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants