Avoid unnecessary copying and dynamic memory allocations

Co-authored-by: Chenna Keshava B S <ckbs.keshava56@gmail.com>
XRPLF · Aug 25, 2022 · d318ab6 · d318ab6
1 parent 9aaa0df
commit d318ab6
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 61 deletions.
diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp
@@ -211,15 +211,10 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal)
     prop.set_nodepubkey(
         validatorKeys_.publicKey.data(), validatorKeys_.publicKey.size());
 
-    auto signingHash = sha512Half(
-        HashPrefix::proposal,
-        std::uint32_t(proposal.proposeSeq()),
-        proposal.closeTime().time_since_epoch().count(),
-        proposal.prevLedger(),
-        proposal.position());
-
     auto sig = signDigest(
-        validatorKeys_.publicKey, validatorKeys_.secretKey, signingHash);
+        validatorKeys_.publicKey,
+        validatorKeys_.secretKey,
+        proposal.signingHash());
 
     prop.set_signature(sig.data(), sig.size());
 

diff --git a/src/ripple/app/consensus/RCLCxPeerPos.cpp b/src/ripple/app/consensus/RCLCxPeerPos.cpp
@@ -32,29 +32,23 @@ RCLCxPeerPos::RCLCxPeerPos(
     Slice const& signature,
     uint256 const& suppression,
     Proposal&& proposal)
-    : data_{std::make_shared<Data>(
-          publicKey,
-          signature,
-          suppression,
-          std::move(proposal))}
+    : publicKey_(publicKey)
+    , suppression_(suppression)
+    , proposal_(std::move(proposal))
 {
-}
+    // The maximum allowed size of a signature is 72 bytes; we verify
+    // this elsewhere, but we want to be extra careful here:
+    assert(signature.size() != 0 && signature.size() <= signature_.capacity());
 
-uint256
-RCLCxPeerPos::signingHash() const
-{
-    return sha512Half(
-        HashPrefix::proposal,
-        std::uint32_t(proposal().proposeSeq()),
-        proposal().closeTime().time_since_epoch().count(),
-        proposal().prevLedger(),
-        proposal().position());
+    if (signature.size() != 0 && signature.size() <= signature_.capacity())
+        signature_.assign(signature.begin(), signature.end());
 }
 
 bool
 RCLCxPeerPos::checkSign() const
 {
-    return verifyDigest(publicKey(), signingHash(), signature(), false);
+    return verifyDigest(
+        publicKey(), proposal_.signingHash(), signature(), false);
 }
 
 Json::Value
@@ -88,16 +82,4 @@ proposalUniqueId(
     return s.getSHA512Half();
 }
 
-RCLCxPeerPos::Data::Data(
-    PublicKey const& publicKey,
-    Slice const& signature,
-    uint256 const& suppress,
-    Proposal&& proposal)
-    : publicKey_{publicKey}
-    , signature_{signature}
-    , suppression_{suppress}
-    , proposal_{std::move(proposal)}
-{
-}
-
 }  // namespace ripple
diff --git a/src/ripple/app/consensus/RCLCxPeerPos.h b/src/ripple/app/consensus/RCLCxPeerPos.h
@@ -28,6 +28,7 @@
 #include <ripple/protocol/HashPrefix.h>
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/SecretKey.h>
+#include <boost/container/static_vector.hpp>
 #include <chrono>
 #include <cstdint>
 #include <string>
@@ -61,10 +62,6 @@ class RCLCxPeerPos
         uint256 const& suppress,
         Proposal&& proposal);
 
-    //! Create the signing hash for the proposal
-    uint256
-    signingHash() const;
-
     //! Verify the signing hash of the proposal
     bool
     checkSign() const;
@@ -73,49 +70,38 @@ class RCLCxPeerPos
     Slice
     signature() const
     {
-        return data_->signature_;
+        return {signature_.data(), signature_.size()};
     }
 
     //! Public key of peer that sent the proposal
     PublicKey const&
     publicKey() const
     {
-        return data_->publicKey_;
+        return publicKey_;
     }
 
     //! Unique id used by hash router to suppress duplicates
     uint256 const&
     suppressionID() const
     {
-        return data_->suppression_;
+        return suppression_;
     }
 
     Proposal const&
     proposal() const
     {
-        return data_->proposal_;
+        return proposal_;
     }
 
     //! JSON representation of proposal
     Json::Value
     getJson() const;
 
 private:
-    struct Data : public CountedObject<Data>
-    {
-        PublicKey publicKey_;
-        Buffer signature_;
-        uint256 suppression_;
-        Proposal proposal_;
-
-        Data(
-            PublicKey const& publicKey,
-            Slice const& signature,
-            uint256 const& suppress,
-            Proposal&& proposal);
-    };
-
-    std::shared_ptr<Data> data_;
+    PublicKey publicKey_;
+    uint256 suppression_;
+    Proposal proposal_;
+    boost::container::static_vector<std::uint8_t, 72> signature_;
 
     template <class Hasher>
     void

diff --git a/src/ripple/consensus/ConsensusProposal.h b/src/ripple/consensus/ConsensusProposal.h
@@ -16,13 +16,16 @@
     OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */
 //==============================================================================
-#ifndef RIPPLE_CONSENSUS_ConsensusProposal_H_INCLUDED
-#define RIPPLE_CONSENSUS_ConsensusProposal_H_INCLUDED
+#ifndef RIPPLE_CONSENSUS_CONSENSUSPROPOSAL_H_INCLUDED
+#define RIPPLE_CONSENSUS_CONSENSUSPROPOSAL_H_INCLUDED
 
+#include <ripple/basics/base_uint.h>
 #include <ripple/basics/chrono.h>
 #include <ripple/json/json_value.h>
+#include <ripple/protocol/HashPrefix.h>
 #include <ripple/protocol/jss.h>
 #include <cstdint>
+#include <optional>
 
 namespace ripple {
 /** Represents a proposed position taken during a round of consensus.
@@ -169,6 +172,7 @@ class ConsensusProposal
         NetClock::time_point newCloseTime,
         NetClock::time_point now)
     {
+        signingHash_.reset();
         position_ = newPosition;
         closeTime_ = newCloseTime;
         time_ = now;
@@ -185,6 +189,7 @@ class ConsensusProposal
     void
     bowOut(NetClock::time_point now)
     {
+        signingHash_.reset();
         time_ = now;
         proposeSeq_ = seqLeave;
     }
@@ -210,6 +215,23 @@ class ConsensusProposal
         return ret;
     }
 
+    //! The digest for this proposal, used for signing purposes.
+    uint256 const&
+    signingHash() const
+    {
+        if (!signingHash_)
+        {
+            signingHash_ = sha512Half(
+                HashPrefix::proposal,
+                std::uint32_t(proposeSeq()),
+                closeTime().time_since_epoch().count(),
+                prevLedger(),
+                position());
+        }
+
+        return signingHash_.value();
+    }
+
 private:
     //! Unique identifier of prior ledger this proposal is based on
     LedgerID_t previousLedger_;
@@ -228,6 +250,9 @@ class ConsensusProposal
 
     //! The identifier of the node taking this position
     NodeID_t nodeID_;
+
+    //! The signing hash for this proposal
+    mutable std::optional<uint256> signingHash_;
 };
 
 template <class NodeID_t, class LedgerID_t, class Position_t>