SCPDriver: Make hashing routine user-definable

This introduces one new virtual method `getHashOf`, which takes a vector of a vector of bytes - to be able to hash a concatenation of parameters. It returns a `Hash`, which is defined in `Stellar-types.x`. Implementors of SCPDriver can define their own hash type by redefining it in this XDR header file. Co-authored-by: MonsieurNicolas <nicolas@stellar.org> Resolves #2724
stellar · Oct 7, 2020 · 2a91d38 · 2a91d38 · latobarita · Oct 7, 2020
1 parent 077574b
commit 2a91d38
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 23 deletions.
diff --git a/src/herder/HerderSCPDriver.cpp b/src/herder/HerderSCPDriver.cpp
@@ -32,6 +32,17 @@ namespace stellar
 uint32_t const HerderSCPDriver::FIRST_PROTOCOL_WITH_TXSET_CLOSETIME_AFFINITY =
     14;
 
+Hash
+HerderSCPDriver::getHashOf(std::vector<xdr::opaque_vec<>> const& vals) const
+{
+    SHA256 hasher;
+    for (auto const& v : vals)
+    {
+        hasher.add(v);
+    }
+    return hasher.finish();
+}
+
 HerderSCPDriver::SCPMetrics::SCPMetrics(Application& app)
     : mEnvelopeSign(
           app.getMetrics().NewMeter({"scp", "envelope", "sign"}, "envelope"))

diff --git a/src/herder/HerderSCPDriver.h b/src/herder/HerderSCPDriver.h
@@ -112,6 +112,9 @@ class HerderSCPDriver : public SCPDriver
                     std::chrono::milliseconds timeout,
                     std::function<void()> cb) override;
 
+    // hashing support
+    Hash getHashOf(std::vector<xdr::opaque_vec<>> const& vals) const override;
+
     // core SCP
     ValueWrapperPtr
     combineCandidates(uint64_t slotIndex,

diff --git a/src/scp/LocalNode.cpp b/src/scp/LocalNode.cpp
@@ -25,14 +25,15 @@ LocalNode::LocalNode(NodeID const& nodeID, bool isValidator,
     : mNodeID(nodeID), mIsValidator(isValidator), mQSet(qSet), mSCP(scp)
 {
     normalizeQSet(mQSet);
-    mQSetHash = sha256(xdr::xdr_to_opaque(mQSet));
+    auto const& scpDriver = mSCP->getDriver();
+    mQSetHash = scpDriver.getHashOf({xdr::xdr_to_opaque(mQSet)});
 
     CLOG(INFO, "SCP") << "LocalNode::LocalNode"
                       << "@" << KeyUtils::toShortString(mNodeID)
                       << " qSet: " << hexAbbrev(mQSetHash);
 
     mSingleQSet = std::make_shared<SCPQuorumSet>(buildSingletonQSet(mNodeID));
-    gSingleQSetHash = sha256(xdr::xdr_to_opaque(*mSingleQSet));
+    gSingleQSetHash = scpDriver.getHashOf({xdr::xdr_to_opaque(*mSingleQSet)});
 }
 
 SCPQuorumSet
@@ -48,7 +49,7 @@ void
 LocalNode::updateQuorumSet(SCPQuorumSet const& qSet)
 {
     ZoneScoped;
-    mQSetHash = sha256(xdr::xdr_to_opaque(qSet));
+    mQSetHash = mSCP->getDriver().getHashOf({xdr::xdr_to_opaque(qSet)});
     mQSet = qSet;
 }
 

diff --git a/src/scp/SCPDriver.cpp b/src/scp/SCPDriver.cpp
@@ -8,7 +8,6 @@
 
 #include "crypto/Hex.h"
 #include "crypto/KeyUtils.h"
-#include "crypto/SHA.h"
 #include "crypto/SecretKey.h"
 #include "xdrpp/marshal.h"
 
@@ -56,7 +55,7 @@ SCPDriver::wrapValue(Value const& value)
 std::string
 SCPDriver::getValueString(Value const& v) const
 {
-    uint256 valueHash = sha256(xdr::xdr_to_opaque(v));
+    Hash valueHash = getHashOf({xdr::xdr_to_opaque(v)});
 
     return hexAbbrev(valueHash);
 }
@@ -78,15 +77,16 @@ static const uint32 hash_N = 1;
 static const uint32 hash_P = 2;
 static const uint32 hash_K = 3;
 
-static uint64
-hashHelper(uint64 slotIndex, Value const& prev,
-           std::function<void(SHA256&)> extra)
+uint64
+SCPDriver::hashHelper(
+    uint64 slotIndex, Value const& prev,
+    std::function<void(std::vector<xdr::opaque_vec<>>&)> extra)
 {
-    SHA256 h;
-    h.add(xdr::xdr_to_opaque(slotIndex));
-    h.add(xdr::xdr_to_opaque(prev));
-    extra(h);
-    uint256 t = h.finish();
+    std::vector<xdr::opaque_vec<>> vals;
+    vals.emplace_back(xdr::xdr_to_opaque(slotIndex));
+    vals.emplace_back(xdr::xdr_to_opaque(prev));
+    extra(vals);
+    Hash t = getHashOf(vals);
     uint64 res = 0;
     for (size_t i = 0; i < sizeof(res); i++)
     {
@@ -99,22 +99,24 @@ uint64
 SCPDriver::computeHashNode(uint64 slotIndex, Value const& prev, bool isPriority,
                            int32_t roundNumber, NodeID const& nodeID)
 {
-    return hashHelper(slotIndex, prev, [&](SHA256& h) {
-        h.add(xdr::xdr_to_opaque(isPriority ? hash_P : hash_N));
-        h.add(xdr::xdr_to_opaque(roundNumber));
-        h.add(xdr::xdr_to_opaque(nodeID));
-    });
+    return hashHelper(
+        slotIndex, prev, [&](std::vector<xdr::opaque_vec<>>& vals) {
+            vals.emplace_back(xdr::xdr_to_opaque(isPriority ? hash_P : hash_N));
+            vals.emplace_back(xdr::xdr_to_opaque(roundNumber));
+            vals.emplace_back(xdr::xdr_to_opaque(nodeID));
+        });
 }
 
 uint64
 SCPDriver::computeValueHash(uint64 slotIndex, Value const& prev,
                             int32_t roundNumber, Value const& value)
 {
-    return hashHelper(slotIndex, prev, [&](SHA256& h) {
-        h.add(xdr::xdr_to_opaque(hash_K));
-        h.add(xdr::xdr_to_opaque(roundNumber));
-        h.add(xdr::xdr_to_opaque(value));
-    });
+    return hashHelper(slotIndex, prev,
+                      [&](std::vector<xdr::opaque_vec<>>& vals) {
+                          vals.emplace_back(xdr::xdr_to_opaque(hash_K));
+                          vals.emplace_back(xdr::xdr_to_opaque(roundNumber));
+                          vals.emplace_back(xdr::xdr_to_opaque(value));
+                      });
 }
 
 static const int MAX_TIMEOUT_SECONDS = (30 * 60);

diff --git a/src/scp/SCPDriver.h b/src/scp/SCPDriver.h
@@ -147,6 +147,10 @@ class SCPDriver
     // `toShortString` converts to the common name of a key if found
     virtual std::string toShortString(PublicKey const& pk) const;
 
+    // `getHashOf` computes the hash for the given vector of byte vector
+    virtual Hash
+    getHashOf(std::vector<xdr::opaque_vec<>> const& vals) const = 0;
+
     // `computeHashNode` is used by the nomination protocol to
     // randomize the order of messages between nodes.
     virtual uint64 computeHashNode(uint64 slotIndex, Value const& prev,
@@ -234,5 +238,10 @@ class SCPDriver
     ballotDidHearFromQuorum(uint64 slotIndex, SCPBallot const& ballot)
     {
     }
+
+  private:
+    uint64
+    hashHelper(uint64 slotIndex, Value const& prev,
+               std::function<void(std::vector<xdr::opaque_vec<>>&)> extra);
 };
 }
diff --git a/src/scp/test/SCPTests.cpp b/src/scp/test/SCPTests.cpp
@@ -158,6 +158,17 @@ class TestSCP : public SCPDriver
     std::set<Value> mExpectedCandidates;
     Value mCompositeValue;
 
+    Hash
+    getHashOf(std::vector<xdr::opaque_vec<>> const& vals) const override
+    {
+        SHA256 hasher;
+        for (auto const& v : vals)
+        {
+            hasher.add(v);
+        }
+        return hasher.finish();
+    }
+
     // override the internal hashing scheme in order to make tests
     // more predictable.
     uint64

diff --git a/src/scp/test/SCPUnitTests.cpp b/src/scp/test/SCPUnitTests.cpp
@@ -117,6 +117,17 @@ class TestNominationSCP : public SCPDriver
         static Value const emptyValue{};
         return emptyValue;
     }
+
+    Hash
+    getHashOf(std::vector<xdr::opaque_vec<>> const& vals) const override
+    {
+        SHA256 hasher;
+        for (auto const& v : vals)
+        {
+            hasher.add(v);
+        }
+        return hasher.finish();
+    }
 };
 
 class NominationTestHandler : public NominationProtocol