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

Blake3 incorrect implementation? #1

Closed
sergeevabc opened this issue Mar 30, 2021 · 6 comments
Closed

Blake3 incorrect implementation? #1

sergeevabc opened this issue Mar 30, 2021 · 6 comments
Assignees
Labels
bug Houston, we've got a problem

Comments

@sergeevabc
Copy link

sergeevabc commented Mar 30, 2021

Windows 7, Milva 1.1.0, b3sum 0.3.7, hashsum 1.0

$ milva.exe --blake3 milva.exe
milva.exe: af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262

$ b3sum.exe milva.exe
21bc827a71ac67d907aa51e0ae4d3f26e3448a40c5aaed5df298cc75d1cdd55a  milva.exe

$ hashsum_win64.exe -v -t b3 milva.exe                 
21bc827a71ac67d907aa51e0ae4d3f26e3448a40c5aaed5df298cc75d1cdd55a milva.exe 
@samuel-lucas6
Copy link
Owner

samuel-lucas6 commented Mar 31, 2021

That's not good. It looks like I'm hashing nothing. I'll take a proper look later.

Nice catch! Thanks for reporting this.

Edit: Correction. The README example for Blake3.NET just confused me because it's using a new MemoryStream.

@samuel-lucas6 samuel-lucas6 added the bug Houston, we've got a problem label Mar 31, 2021
@samuel-lucas6 samuel-lucas6 self-assigned this Mar 31, 2021
@samuel-lucas6
Copy link
Owner

Fixed in release v1.2.0. It turns out I was right initially - Blake3.NET does do things differently, hence the silly mistake.

@sergeevabc
Copy link
Author

sergeevabc commented Mar 31, 2021

Also, this implementation suffers from a bad memory management, which leads to disk trashing and errors. You’re not alone here: enc_file by @LazyEmpiricist and b3sum by @oconnor663 show the same symptoms. On the other hand, RapidCRC by @OV2, HashCheck fork by @idrassi, and the recently appeared Hashsum by @ddulesov work just fine.

$ wmic ComputerSystem get TotalPhysicalMemory
4293378048

$ stat -c %s CityLights1931.mkv
4658941558

$ milva --blake3 CityLights1931.mkv
CityLights1931.mkv - Error: System.IO.IOException

@samuel-lucas6
Copy link
Owner

@sergeevabc Blake3.NET uses the official Rust implementation of BLAKE3. Therefore, there's nothing I can do about that if that's a legitimate problem.

I suggest discussing your problem in detail in the BLAKE3 repo in a new issue rather than in an unrelated issue. Also based on some of your comments in other repos, I urge you to please try and be more constructive and helpful with your criticism since a few of your comments do come across as trashing someone's hard work, which is something maintainers never enjoy reading.

@sergeevabc
Copy link
Author

sergeevabc commented Mar 31, 2021

@samuel-lucas6
I do provide a verbose feedback and am always ready for further troubleshooting. For example, laziness aside, fixing Unicode issue of xxHash even led me to booting up a virtual machine with Windows XP, not to mention I once helped Frank Denis (the very author you cited) with DNSProxy. So if you’re about to mentor me on how to contribute, dig my profile deeper.

However, sometimes developers become detached from reality, follow “can’t reproduce, therefore nothing to worry about” policy, and evade responsibility to make a solution instead of more-or-less coherently glued pieces of code (like you do by redirecting me to upstream and not caring that you polluted the community with a partly broken tool). When you share something, expect to process various feedback, not exclusively positive, because there is a certain level of expectations set by the achievements of ancestors. The feedback includes bashing, because there are more and more quitters who fuel expectations but fail to deliver.

@samuel-lucas6
Copy link
Owner

@sergeevabc I'm just saying that in some cases you come across as a little harsh. You also put that criticism of age in an unrelated issue rather than creating a new issue, which didn't help your case either.

If the developer can't replicate the problem, then it's likely harder, if not impossible in some cases, to fix the bug. I'm not 'polluting the community with a partly broken tool' (see what I mean about coming across as harsh), I'm using the official BLAKE3 implementation. Perhaps it has problems at present, but I'm sure they'll be ironed out in a future release. Complaining to me about a problem with another program won't help the situation.

Finally, I do expect various forms of feedback, but as Frank discussed in that blog post, being overwhelmed with criticism can have a negative impact on maintainers, and there's a polite way of going about providing feedback. I understand that these sorts of issues are frustrating, but saying things like 'polluting the community with a partly broken tool' will increase the likelihood of maintainers ignoring your feedback since that sort of phrasing is more of an insult than feedback.

I don't think there's any more to discuss, so I'm going to lock this issue. You know who to discuss the BLAKE3 issue with, and I'm hoping you realise that you could phrase some of your comments more politely in the future.

Repository owner locked as resolved and limited conversation to collaborators Apr 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Houston, we've got a problem
Projects
None yet
Development

No branches or pull requests

2 participants