Change internal strong hashes to BLAKE2 #2795
Conversation
| @@ -0,0 +1,75 @@ | |||
| // Copyright 2020 Stellar Development Foundation and contributors. Licensed | |||
There was a problem hiding this comment.
need to update Visual Studio files
| @@ -915,7 +915,7 @@ void | |||
| Peer::recvSCPQuorumSet(StellarMessage const& msg) | |||
| { | |||
| ZoneScoped; | |||
| Hash hash = sha256(xdr::xdr_to_opaque(msg.qSet())); | |||
| Hash hash = xdrSha256(msg.qSet()); | |||
There was a problem hiding this comment.
this belongs to the previous commit
| @@ -58,7 +58,7 @@ bool | |||
| Floodgate::addRecord(StellarMessage const& msg, Peer::pointer peer, Hash& index) | |||
| { | |||
| ZoneScoped; | |||
| index = sha256(xdr::xdr_to_opaque(msg)); | |||
| index = xdrBlake2(msg); | |||
There was a problem hiding this comment.
you forgot to update Tracker::listen (so this commit breaks item fetcher)
There was a problem hiding this comment.
I .. don't think so! The item fetcher and tracker deals in public sha256 hashes -- the shared identities of txsets and qsets baked into the public protocol. We're not changing its hashes.
The floodmap is an entirely internal usage, the hashes that key it never leave it. They're private book-keeping, which is why it's safe to change them here (and changing them is unrelated to the hashes used elsewhere).
There was a problem hiding this comment.
no: look at how getPeersKnows (which now uses Blake2 hashes) is used: it's passing what was computed in Tracker::listen, which is computed as:
// NB: hash here is of StellarMessage
mWaitingEnvelopes.push_back(
std::make_pair(sha256(xdr::xdr_to_opaque(m)), env));
Those keys are only used in connection to the flood map from what I can tell. The entire code base, except for the actual download code only uses the values.
This should not be confused with what item fetcher is trying to download (which is also a hash but of the item that we're trying to get).
There was a problem hiding this comment.
Oh my goodness. Nice catch! Fixed (and added a comment).
graydon
force-pushed
the
change-internal-strong-hashes-to-blake2
branch
from
8cf9dc2
to
99421cb
Compare
graydon
force-pushed
the
change-internal-strong-hashes-to-blake2
branch
from
99421cb
to
e18b395
Compare
|
r+ e18b395 |
This switches two of the strong internal hashes (one on floodgate, one on signature checking cache, neither exposed as part of any protocol) from SHA256 to BLAKE2b, which runs about twice as fast and has equal-or-superior security properties. This also happens to be the default "generic" strong hash in libsodium, so not especially hard to hook up to.
Along the way it also changes a bunch of
sha256(xdr_to_opaque(...))calls to the non-allocating (and very slightly faster)xdrSha256andxdrBlake2variants