-
Notifications
You must be signed in to change notification settings - Fork 22
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
Port SCP's hashes into 64-bytes #737
Conversation
Codecov Report
@@ Coverage Diff @@
## v0.x.x #737 +/- ##
==========================================
- Coverage 91.39% 91.36% -0.03%
==========================================
Files 63 63
Lines 5146 5120 -26
==========================================
- Hits 4703 4678 -25
+ Misses 443 442 -1
Continue to review full report at Codecov.
|
StellarHashFmt needs to be removed in SCPPrettyPrinter. |
45d66ef
to
3c8b876
Compare
Done. |
a9a264e
to
ab79669
Compare
source/scpp/src/scp/SCPDriver.cpp
Outdated
} | ||
uint512 t = h->finish(); | ||
|
||
// WARNING: Do not replace this back with SCP's original implementation. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ab79669
to
0d458bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments, looks nice
using uint256 = xdr::opaque_array<32>; | ||
using uint512 = xdr::opaque_array<64>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment above those types, explaining that they were modified on purpose ?
source/scpp/src/util/HashOfHash.cpp
Outdated
@@ -3,7 +3,6 @@ | |||
|
|||
namespace std | |||
{ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated
// Plain SHA256 | ||
uint256 | ||
sha256(ByteSlice const& bin) | ||
// Plain SHA512 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess you want to do the migration in two steps ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this could have been all part of the same PR. But I'll leave it as is then, and fix up the comments.
Also removed StellarHashFmt since we can just use prettify which supports 64-byte hashes already. Part of: bosagora#702
0d458bf
to
49cc23f
Compare
Fixed up, green. |
This is the first part of resolving #702