From d318ab612adc86f1fd8527a50af232f377ca89ef Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sun, 31 Jul 2022 23:10:03 -0700 Subject: [PATCH] Avoid unnecessary copying and dynamic memory allocations Co-authored-by: Chenna Keshava B S --- src/ripple/app/consensus/RCLConsensus.cpp | 11 ++----- src/ripple/app/consensus/RCLCxPeerPos.cpp | 38 ++++++----------------- src/ripple/app/consensus/RCLCxPeerPos.h | 32 ++++++------------- src/ripple/consensus/ConsensusProposal.h | 29 +++++++++++++++-- 4 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index a4a4e0989a..31c851eb81 100644 --- 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 index 709e789880..ee5a45b943 100644 --- 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( - 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 index 9d448aac48..e82a85d422 100644 --- a/src/ripple/app/consensus/RCLCxPeerPos.h +++ b/src/ripple/app/consensus/RCLCxPeerPos.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -61,10 +62,6 @@ public: 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,27 +70,27 @@ public: 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 @@ -101,21 +98,10 @@ public: getJson() const; private: - struct Data : public CountedObject - { - PublicKey publicKey_; - Buffer signature_; - uint256 suppression_; - Proposal proposal_; - - Data( - PublicKey const& publicKey, - Slice const& signature, - uint256 const& suppress, - Proposal&& proposal); - }; - - std::shared_ptr data_; + PublicKey publicKey_; + uint256 suppression_; + Proposal proposal_; + boost::container::static_vector signature_; template void diff --git a/src/ripple/consensus/ConsensusProposal.h b/src/ripple/consensus/ConsensusProposal.h index a3eccbb016..c5103cfe0d 100644 --- 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 #include #include +#include #include #include +#include namespace ripple { /** Represents a proposed position taken during a round of consensus. @@ -169,6 +172,7 @@ public: NetClock::time_point newCloseTime, NetClock::time_point now) { + signingHash_.reset(); position_ = newPosition; closeTime_ = newCloseTime; time_ = now; @@ -185,6 +189,7 @@ public: void bowOut(NetClock::time_point now) { + signingHash_.reset(); time_ = now; proposeSeq_ = seqLeave; } @@ -210,6 +215,23 @@ public: 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 @@ private: //! The identifier of the node taking this position NodeID_t nodeID_; + + //! The signing hash for this proposal + mutable std::optional signingHash_; }; template