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

fix: zero padding reader incomplete reads #710

Merged
merged 3 commits into from
Jan 30, 2025
Merged

fix: zero padding reader incomplete reads #710

merged 3 commits into from
Jan 30, 2025

Conversation

th7nder
Copy link
Contributor

@th7nder th7nder commented Jan 30, 2025

Description

Fixes #709.
On bigger files read with ZeroPaddingReader it started giving non-deterministic results as it was padding with 0 when it should not have.

Checklist

  • Are there important points that reviewers should know?
    • If yes, which ones?
  • Make sure that you described what this change does.
  • Have you tested this solution?

@th7nder th7nder added the bug Something isn't working label Jan 30, 2025
@th7nder th7nder added this to the Phase 3 milestone Jan 30, 2025
@th7nder th7nder requested review from cernicc and jmg-duarte January 30, 2025 10:14
@th7nder th7nder self-assigned this Jan 30, 2025
jmg-duarte
jmg-duarte previously approved these changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

Great find, some would say "the best find", the bestest find even! 🚀

lib/polka-storage-proofs/src/zero_reader.rs Outdated Show resolved Hide resolved
@jmg-duarte jmg-duarte added the ready for review Review is needed label Jan 30, 2025
@jmg-duarte jmg-duarte enabled auto-merge (squash) January 30, 2025 10:21
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jan 30, 2025
@th7nder th7nder requested a review from jmg-duarte January 30, 2025 10:23
cernicc
cernicc previously approved these changes Jan 30, 2025
View reviewed changes
Copy link
Member

@cernicc cernicc left a comment

Choose a reason for hiding this comment

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

💪

jmg-duarte
jmg-duarte previously approved these changes Jan 30, 2025
View reviewed changes
@th7nder th7nder dismissed stale reviews from jmg-duarte and cernicc via 91e2cab January 30, 2025 11:04
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jan 30, 2025
@th7nder th7nder requested review from cernicc and jmg-duarte January 30, 2025 11:05
@jmg-duarte jmg-duarte merged commit 9a9b687 into develop Jan 30, 2025
6 checks passed
@jmg-duarte jmg-duarte deleted the fix/709/commp-incomplete-read branch January 30, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cernicc cernicc cernicc approved these changes

@jmg-duarte jmg-duarte jmg-duarte approved these changes

Assignees

@th7nder th7nder

Labels
bug Something isn't working ready for review Review is needed
Projects
None yet
Milestone
Phase 3
Development

Successfully merging this pull request may close these issues.

Cannot store arbitrary file because of padding
3 participants
@th7nder @cernicc @jmg-duarte