-
Notifications
You must be signed in to change notification settings - Fork 7
feat(commp): add binary unmarshall/marshall #29
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
==========================================
+ Coverage 41.58% 46.17% +4.58%
==========================================
Files 3 5 +2
Lines 392 444 +52
==========================================
+ Hits 163 205 +42
+ Misses 225 202 -23
- Partials 4 37 +33
🚀 New features to boost your workflow:
|
|
Permissions question is WIP (slow) here filecoin-project/github-mgmt#47, hard because there's basically never been a clean-up and we have so many teams contributing in the org so it's a bit tedious pulling all those threads together and figuring out canonical lists. Will try and get someone, maybe me, to review this soon. Sadly the changes are big enough that it's going to be an investment. |
|
we don't need a merge... it's more just contribute back :). but also: what concerns me is I have access to push and merge this PR without any additional approval, and that says to me a bunch of people have my access, and I can't imagine what someone with my level access could do to the network if they pushed and merged a PR to a core hashing algorithm that was malicious, either cause they had a grudge or their account got hacked. I know a lot of hashing is in rust code but I bet people have access there too. |
|
kudos to attemps to clean up the tooling around commp! It's both fairly complex and fairly straightforward - complex because there's a lot of intricate details - simple because it's "just hashing something". Glad to see any activity towards improvement! |
|
@ribasushi if you're willing to review this then I'd very much appreciate it. I too imagined that you'd moved on enough to not want to be bothered with this but it would be great if you could. |

Goals
Support binary marshalling of hash calculation state, so that you can pause/resume a calculation. This is supported on sha256 and the simd version used in this package.
It's certainly a bunch of complexity so no pressure to merge, but we needed it.
Also I'm a long time contributor but should I really still have commit privileges to all the filecoin-project repos? Cause maybe there are people like me who are now hostile actors? or people who could get hacked?
Implementation
For discussion
I'm pretty sure this is the first time ANYONE has worked on this repo in a minute and the code is complicated. I recommend some additional docs and/or knowledge transfer. happy to pass on what I learned, @ribasushi is off to better pastures.