From 74e6ed1af366b692c37f219807f29d3fc079826f Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 7 Oct 2021 16:12:31 -0400 Subject: [PATCH 01/19] Improve the readability of STBase-derived types * Increase the visibility of each type's API. * No functional changes. --- src/ripple/protocol/STAccount.h | 77 +- src/ripple/protocol/STAmount.h | 262 +++--- src/ripple/protocol/STArray.h | 260 +++--- src/ripple/protocol/STBase.h | 79 +- src/ripple/protocol/STBitString.h | 167 ++-- src/ripple/protocol/STBlob.h | 156 ++-- src/ripple/protocol/STInteger.h | 157 ++-- src/ripple/protocol/STLedgerEntry.h | 73 +- src/ripple/protocol/STObject.h | 893 +++++++++++---------- src/ripple/protocol/STPathSet.h | 569 +++++++------ src/ripple/protocol/STTx.h | 72 +- src/ripple/protocol/STValidation.h | 241 +++--- src/ripple/protocol/STVector256.h | 272 ++++--- src/ripple/protocol/impl/STAccount.cpp | 44 + src/ripple/protocol/impl/STAmount.cpp | 24 + src/ripple/protocol/impl/STArray.cpp | 24 + src/ripple/protocol/impl/STBase.cpp | 12 + src/ripple/protocol/impl/STBlob.cpp | 34 + src/ripple/protocol/impl/STLedgerEntry.cpp | 18 + src/ripple/protocol/impl/STObject.cpp | 45 +- src/ripple/protocol/impl/STPathSet.cpp | 28 +- src/ripple/protocol/impl/STTx.cpp | 23 +- src/ripple/protocol/impl/STValidation.cpp | 12 + src/ripple/protocol/impl/STVector256.cpp | 28 +- 24 files changed, 2145 insertions(+), 1425 deletions(-) diff --git a/src/ripple/protocol/STAccount.h b/src/ripple/protocol/STAccount.h index a7f655f91..708eb79a2 100644 --- a/src/ripple/protocol/STAccount.h +++ b/src/ripple/protocol/STAccount.h @@ -40,79 +40,62 @@ public: using value_type = AccountID; STAccount(); + STAccount(SField const& n); STAccount(SField const& n, Buffer&& v); STAccount(SerialIter& sit, SField const& name); STAccount(SField const& n, AccountID const& v); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } - + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; SerializedTypeID - getSType() const override - { - return STI_ACCOUNT; - } + getSType() const override; std::string getText() const override; void - add(Serializer& s) const override - { - assert(fName->isBinary()); - assert(fName->fieldType == STI_ACCOUNT); - - // Preserve the serialization behavior of an STBlob: - // o If we are default (all zeros) serialize as an empty blob. - // o Otherwise serialize 160 bits. - int const size = isDefault() ? 0 : uint160::bytes; - s.addVL(value_.data(), size); - } + add(Serializer& s) const override; bool - isEquivalent(const STBase& t) const override - { - auto const* const tPtr = dynamic_cast(&t); - return tPtr && (default_ == tPtr->default_) && (value_ == tPtr->value_); - } + isEquivalent(const STBase& t) const override; bool - isDefault() const override - { - return default_; - } + isDefault() const override; STAccount& - operator=(AccountID const& value) - { - setValue(value); - return *this; - } + operator=(AccountID const& value); AccountID - value() const noexcept - { - return value_; - } + value() const noexcept; void - setValue(AccountID const& v) - { - value_ = v; - default_ = false; - } + setValue(AccountID const& v); }; +inline STAccount& +STAccount::operator=(AccountID const& value) +{ + setValue(value); + return *this; +} + +inline AccountID +STAccount::value() const noexcept +{ + return value_; +} + +inline void +STAccount::setValue(AccountID const& v) +{ + value_ = v; + default_ = false; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 01bd4934b..3114cb0e1 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -145,94 +145,50 @@ public: STAmount(XRPAmount const& amount); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } - + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; //-------------------------------------------------------------------------- - -private: - static std::unique_ptr - construct(SerialIter&, SField const& name); - - void - set(std::int64_t v); - void - canonicalize(); - -public: - //-------------------------------------------------------------------------- // // Observers // //-------------------------------------------------------------------------- int - exponent() const noexcept - { - return mOffset; - } + exponent() const noexcept; + bool - native() const noexcept - { - return mIsNative; - } + native() const noexcept; + bool - negative() const noexcept - { - return mIsNegative; - } + negative() const noexcept; + std::uint64_t - mantissa() const noexcept - { - return mValue; - } + mantissa() const noexcept; + Issue const& - issue() const - { - return mIssue; - } + issue() const; // These three are deprecated Currency const& - getCurrency() const - { - return mIssue.currency; - } + getCurrency() const; + AccountID const& - getIssuer() const - { - return mIssue.account; - } + getIssuer() const; int - signum() const noexcept - { - return mValue ? (mIsNegative ? -1 : 1) : 0; - } + signum() const noexcept; /** Returns a zero value with the same issuer and currency. */ STAmount - zeroed() const - { - return STAmount(mIssue); - } + zeroed() const; void setJson(Json::Value&) const; STAmount const& - value() const noexcept - { - return *this; - } + value() const noexcept; //-------------------------------------------------------------------------- // @@ -240,28 +196,17 @@ public: // //-------------------------------------------------------------------------- - explicit operator bool() const noexcept - { - return *this != beast::zero; - } + explicit operator bool() const noexcept; STAmount& operator+=(STAmount const&); STAmount& operator-=(STAmount const&); - STAmount& operator=(beast::Zero) - { - clear(); - return *this; - } + STAmount& operator=(beast::Zero); STAmount& - operator=(XRPAmount const& amount) - { - *this = STAmount(amount); - return *this; - } + operator=(XRPAmount const& amount); //-------------------------------------------------------------------------- // @@ -270,42 +215,20 @@ public: //-------------------------------------------------------------------------- void - negate() - { - if (*this != beast::zero) - mIsNegative = !mIsNegative; - } + negate(); void - clear() - { - // The -100 is used to allow 0 to sort less than a small positive values - // which have a negative exponent. - mOffset = mIsNative ? 0 : -100; - mValue = 0; - mIsNegative = false; - } + clear(); // Zero while copying currency and issuer. void - clear(STAmount const& saTmpl) - { - clear(saTmpl.mIssue); - } + clear(STAmount const& saTmpl); void - clear(Issue const& issue) - { - setIssue(issue); - clear(); - } + clear(Issue const& issue); void - setIssuer(AccountID const& uIssuer) - { - mIssue.account = uIssuer; - setIssue(mIssue); - } + setIssuer(AccountID const& uIssuer); /** Set the Issue for this amount and update mIsNative. */ void @@ -318,10 +241,7 @@ public: //-------------------------------------------------------------------------- SerializedTypeID - getSType() const override - { - return STI_AMOUNT; - } + getSType() const override; std::string getFullText() const override; @@ -338,15 +258,21 @@ public: isEquivalent(const STBase& t) const override; bool - isDefault() const override - { - return (mValue == 0) && mIsNative; - } + isDefault() const override; XRPAmount xrp() const; IOUAmount iou() const; + +private: + static std::unique_ptr + construct(SerialIter&, SField const& name); + + void + set(std::int64_t v); + void + canonicalize(); }; //------------------------------------------------------------------------------ @@ -382,6 +308,122 @@ toSTAmount(STAmount const& a) // //------------------------------------------------------------------------------ +inline int +STAmount::exponent() const noexcept +{ + return mOffset; +} + +inline bool +STAmount::native() const noexcept +{ + return mIsNative; +} + +inline bool +STAmount::negative() const noexcept +{ + return mIsNegative; +} + +inline std::uint64_t +STAmount::mantissa() const noexcept +{ + return mValue; +} + +inline Issue const& +STAmount::issue() const +{ + return mIssue; +} + +inline Currency const& +STAmount::getCurrency() const +{ + return mIssue.currency; +} + +inline AccountID const& +STAmount::getIssuer() const +{ + return mIssue.account; +} + +inline int +STAmount::signum() const noexcept +{ + return mValue ? (mIsNegative ? -1 : 1) : 0; +} + +inline STAmount +STAmount::zeroed() const +{ + return STAmount(mIssue); +} + +inline STAmount::operator bool() const noexcept +{ + return *this != beast::zero; +} + +inline STAmount& STAmount::operator=(beast::Zero) +{ + clear(); + return *this; +} + +inline STAmount& +STAmount::operator=(XRPAmount const& amount) +{ + *this = STAmount(amount); + return *this; +} + +inline void +STAmount::negate() +{ + if (*this != beast::zero) + mIsNegative = !mIsNegative; +} + +inline void +STAmount::clear() +{ + // The -100 is used to allow 0 to sort less than a small positive values + // which have a negative exponent. + mOffset = mIsNative ? 0 : -100; + mValue = 0; + mIsNegative = false; +} + +// Zero while copying currency and issuer. +inline void +STAmount::clear(STAmount const& saTmpl) +{ + clear(saTmpl.mIssue); +} + +inline void +STAmount::clear(Issue const& issue) +{ + setIssue(issue); + clear(); +} + +inline void +STAmount::setIssuer(AccountID const& uIssuer) +{ + mIssue.account = uIssuer; + setIssue(mIssue); +} + +inline STAmount const& +STAmount::value() const noexcept +{ + return *this; +} + inline bool isLegalNet(STAmount const& value) { diff --git a/src/ripple/protocol/STArray.h b/src/ripple/protocol/STArray.h index 12c698649..5c4deae4e 100644 --- a/src/ripple/protocol/STArray.h +++ b/src/ripple/protocol/STArray.h @@ -38,161 +38,213 @@ public: using const_iterator = list_type::const_iterator; STArray() = default; - STArray(STArray&&); STArray(STArray const&) = default; + STArray& + operator=(STArray const&) = default; + STArray(STArray&&); + STArray& + operator=(STArray&&); + STArray(SField const& f, int n); STArray(SerialIter& sit, SField const& f, int depth = 0); explicit STArray(int n); explicit STArray(SField const& f); - STArray& - operator=(STArray const&) = default; - STArray& - operator=(STArray&&); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; STObject& - operator[](std::size_t j) - { - return v_[j]; - } + operator[](std::size_t j); STObject const& - operator[](std::size_t j) const - { - return v_[j]; - } + operator[](std::size_t j) const; STObject& - back() - { - return v_.back(); - } + back(); STObject const& - back() const - { - return v_.back(); - } + back() const; template void - emplace_back(Args&&... args) - { - v_.emplace_back(std::forward(args)...); - } + emplace_back(Args&&... args); void - push_back(STObject const& object) - { - v_.push_back(object); - } + push_back(STObject const& object); void - push_back(STObject&& object) - { - v_.push_back(std::move(object)); - } + push_back(STObject&& object); iterator - begin() - { - return v_.begin(); - } + begin(); iterator - end() - { - return v_.end(); - } + end(); const_iterator - begin() const - { - return v_.begin(); - } + begin() const; const_iterator - end() const - { - return v_.end(); - } + end() const; size_type - size() const - { - return v_.size(); - } + size() const; bool - empty() const - { - return v_.empty(); - } - void - clear() - { - v_.clear(); - } - void - reserve(std::size_t n) - { - v_.reserve(n); - } - void - swap(STArray& a) noexcept - { - v_.swap(a.v_); - } + empty() const; - virtual std::string + void + clear(); + + void + reserve(std::size_t n); + + void + swap(STArray& a) noexcept; + + std::string getFullText() const override; - virtual std::string + + std::string getText() const override; - virtual Json::Value + Json::Value getJson(JsonOptions index) const override; - virtual void + + void add(Serializer& s) const override; void sort(bool (*compare)(const STObject& o1, const STObject& o2)); bool - operator==(const STArray& s) const - { - return v_ == s.v_; - } - bool - operator!=(const STArray& s) const - { - return v_ != s.v_; - } + operator==(const STArray& s) const; - virtual SerializedTypeID - getSType() const override - { - return STI_ARRAY; - } - virtual bool + bool + operator!=(const STArray& s) const; + + SerializedTypeID + getSType() const override; + + bool isEquivalent(const STBase& t) const override; - virtual bool - isDefault() const override - { - return v_.empty(); - } + + bool + isDefault() const override; }; +inline STObject& +STArray::operator[](std::size_t j) +{ + return v_[j]; +} + +inline STObject const& +STArray::operator[](std::size_t j) const +{ + return v_[j]; +} + +inline STObject& +STArray::back() +{ + return v_.back(); +} + +inline STObject const& +STArray::back() const +{ + return v_.back(); +} + +template +inline void +STArray::emplace_back(Args&&... args) +{ + v_.emplace_back(std::forward(args)...); +} + +inline void +STArray::push_back(STObject const& object) +{ + v_.push_back(object); +} + +inline void +STArray::push_back(STObject&& object) +{ + v_.push_back(std::move(object)); +} + +inline STArray::iterator +STArray::begin() +{ + return v_.begin(); +} + +inline STArray::iterator +STArray::end() +{ + return v_.end(); +} + +inline STArray::const_iterator +STArray::begin() const +{ + return v_.begin(); +} + +inline STArray::const_iterator +STArray::end() const +{ + return v_.end(); +} + +inline STArray::size_type +STArray::size() const +{ + return v_.size(); +} + +inline bool +STArray::empty() const +{ + return v_.empty(); +} + +inline void +STArray::clear() +{ + v_.clear(); +} + +inline void +STArray::reserve(std::size_t n) +{ + v_.reserve(n); +} + +inline void +STArray::swap(STArray& a) noexcept +{ + v_.swap(a.v_); +} + +inline bool +STArray::operator==(const STArray& s) const +{ + return v_ == s.v_; +} + +inline bool +STArray::operator!=(const STArray& s) const +{ + return v_ != s.v_; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h index 4eb7b7cd9..b8f34005d 100644 --- a/src/ripple/protocol/STBase.h +++ b/src/ripple/protocol/STBase.h @@ -61,53 +61,34 @@ enum class JsonOptions { none = 0, include_date = 1 }; */ class STBase { + SField const* fName; + public: - STBase(); - - explicit STBase(SField const& n); - virtual ~STBase() = default; - - STBase(const STBase& t) = default; + STBase(); + STBase(const STBase&) = default; STBase& operator=(const STBase& t); + explicit STBase(SField const& n); + bool operator==(const STBase& t) const; bool operator!=(const STBase& t) const; virtual STBase* - copy(std::size_t n, void* buf) const - { - return emplace(n, buf, *this); - } - + copy(std::size_t n, void* buf) const; virtual STBase* - move(std::size_t n, void* buf) - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf); template D& - downcast() - { - D* ptr = dynamic_cast(this); - if (ptr == nullptr) - Throw(); - return *ptr; - } + downcast(); template D const& - downcast() const - { - D const* ptr = dynamic_cast(this); - if (ptr == nullptr) - Throw(); - return *ptr; - } + downcast() const; virtual SerializedTypeID getSType() const; @@ -142,17 +123,9 @@ public: addFieldID(Serializer& s) const; protected: - SField const* fName; - template static STBase* - emplace(std::size_t n, void* buf, T&& val) - { - using U = std::decay_t; - if (sizeof(U) > n) - return new U(std::forward(val)); - return new (buf) U(std::forward(val)); - } + emplace(std::size_t n, void* buf, T&& val); }; //------------------------------------------------------------------------------ @@ -160,6 +133,36 @@ protected: std::ostream& operator<<(std::ostream& out, const STBase& t); +template +D& +STBase::downcast() +{ + D* ptr = dynamic_cast(this); + if (ptr == nullptr) + Throw(); + return *ptr; +} + +template +D const& +STBase::downcast() const +{ + D const* ptr = dynamic_cast(this); + if (ptr == nullptr) + Throw(); + return *ptr; +} + +template +STBase* +STBase::emplace(std::size_t n, void* buf, T&& val) +{ + using U = std::decay_t; + if (sizeof(U) > n) + return new U(std::forward(val)); + return new (buf) U(std::forward(val)); +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STBitString.h b/src/ripple/protocol/STBitString.h index 06ace27d9..7ad271a7d 100644 --- a/src/ripple/protocol/STBitString.h +++ b/src/ripple/protocol/STBitString.h @@ -31,93 +31,88 @@ class STBitString final : public STBase public: using value_type = base_uint; +private: + value_type value_; + +public: STBitString() = default; - STBitString(SField const& n) : STBase(n) - { - } - - STBitString(const value_type& v) : value_(v) - { - } - - STBitString(SField const& n, const value_type& v) : STBase(n), value_(v) - { - } - - STBitString(SerialIter& sit, SField const& name) - : STBitString(name, sit.getBitString()) - { - } + STBitString(SField const& n); + STBitString(const value_type& v); + STBitString(SField const& n, const value_type& v); + STBitString(SerialIter& sit, SField const& name); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; SerializedTypeID getSType() const override; std::string - getText() const override - { - return to_string(value_); - } + getText() const override; bool - isEquivalent(const STBase& t) const override - { - const STBitString* v = dynamic_cast(&t); - return v && (value_ == v->value_); - } + isEquivalent(const STBase& t) const override; void - add(Serializer& s) const override - { - assert(fName->isBinary()); - assert(fName->fieldType == getSType()); - s.addBitString(value_); - } + add(Serializer& s) const override; + + bool + isDefault() const override; template void - setValue(base_uint const& v) - { - value_ = v; - } + setValue(base_uint const& v); value_type const& - value() const - { - return value_; - } + value() const; - operator value_type() const - { - return value_; - } - - bool - isDefault() const override - { - return value_ == beast::zero; - } - -private: - value_type value_; + operator value_type() const; }; using STHash128 = STBitString<128>; using STHash160 = STBitString<160>; using STHash256 = STBitString<256>; +template +inline STBitString::STBitString(SField const& n) : STBase(n) +{ +} + +template +inline STBitString::STBitString(const value_type& v) : value_(v) +{ +} + +template +inline STBitString::STBitString(SField const& n, const value_type& v) + : STBase(n), value_(v) +{ +} + +template +inline STBitString::STBitString(SerialIter& sit, SField const& name) + : STBitString(name, sit.getBitString()) +{ +} + +template +STBase* +STBitString::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +template +STBase* +STBitString::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + template <> inline SerializedTypeID STHash128::getSType() const @@ -139,6 +134,58 @@ STHash256::getSType() const return STI_HASH256; } +template +std::string +STBitString::getText() const +{ + return to_string(value_); +} + +template +bool +STBitString::isEquivalent(const STBase& t) const +{ + const STBitString* v = dynamic_cast(&t); + return v && (value_ == v->value_); +} + +template +void +STBitString::add(Serializer& s) const +{ + assert(getFName().isBinary()); + assert(getFName().fieldType == getSType()); + s.addBitString(value_); +} + +template +template +void +STBitString::setValue(base_uint const& v) +{ + value_ = v; +} + +template +typename STBitString::value_type const& +STBitString::value() const +{ + return value_; +} + +template +STBitString::operator value_type() const +{ + return value_; +} + +template +bool +STBitString::isDefault() const +{ + return value_ == beast::zero; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STBlob.h b/src/ripple/protocol/STBlob.h index 2afdc912d..3be671ae5 100644 --- a/src/ripple/protocol/STBlob.h +++ b/src/ripple/protocol/STBlob.h @@ -32,110 +32,116 @@ namespace ripple { // variable length byte string class STBlob : public STBase { + Buffer value_; + public: using value_type = Slice; STBlob() = default; - STBlob(STBlob const& rhs) : STBase(rhs), value_(rhs.data(), rhs.size()) - { - } - - STBlob(SField const& f, void const* data, std::size_t size) - : STBase(f), value_(data, size) - { - } - - STBlob(SField const& f, Buffer&& b) : STBase(f), value_(std::move(b)) - { - } - - STBlob(SField const& n) : STBase(n) - { - } + STBlob(STBlob const& rhs); + STBlob(SField const& f, void const* data, std::size_t size); + STBlob(SField const& f, Buffer&& b); + STBlob(SField const& n); STBlob(SerialIter&, SField const& name = sfGeneric); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; std::size_t - size() const - { - return value_.size(); - } + size() const; std::uint8_t const* - data() const - { - return reinterpret_cast(value_.data()); - } + data() const; SerializedTypeID - getSType() const override - { - return STI_VL; - } + getSType() const override; std::string getText() const override; void - add(Serializer& s) const override - { - assert(fName->isBinary()); - assert( - (fName->fieldType == STI_VL) || (fName->fieldType == STI_ACCOUNT)); - s.addVL(value_.data(), value_.size()); - } - - STBlob& - operator=(Slice const& slice) - { - value_ = Buffer(slice.data(), slice.size()); - return *this; - } - - value_type - value() const noexcept - { - return value_; - } - - STBlob& - operator=(Buffer&& buffer) - { - value_ = std::move(buffer); - return *this; - } - - void - setValue(Buffer&& b) - { - value_ = std::move(b); - } + add(Serializer& s) const override; bool isEquivalent(const STBase& t) const override; bool - isDefault() const override - { - return value_.empty(); - } + isDefault() const override; -private: - Buffer value_; + STBlob& + operator=(Slice const& slice); + + value_type + value() const noexcept; + + STBlob& + operator=(Buffer&& buffer); + + void + setValue(Buffer&& b); }; +inline STBlob::STBlob(STBlob const& rhs) + : STBase(rhs), value_(rhs.data(), rhs.size()) +{ +} + +inline STBlob::STBlob(SField const& f, void const* data, std::size_t size) + : STBase(f), value_(data, size) +{ +} + +inline STBlob::STBlob(SField const& f, Buffer&& b) + : STBase(f), value_(std::move(b)) +{ +} + +inline STBlob::STBlob(SField const& n) : STBase(n) +{ +} + +inline std::size_t +STBlob::size() const +{ + return value_.size(); +} + +inline std::uint8_t const* +STBlob::data() const +{ + return reinterpret_cast(value_.data()); +} + +inline STBlob& +STBlob::operator=(Slice const& slice) +{ + value_ = Buffer(slice.data(), slice.size()); + return *this; +} + +inline STBlob::value_type +STBlob::value() const noexcept +{ + return value_; +} + +inline STBlob& +STBlob::operator=(Buffer&& buffer) +{ + value_ = std::move(buffer); + return *this; +} + +inline void +STBlob::setValue(Buffer&& b) +{ + value_ = std::move(b); +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STInteger.h b/src/ripple/protocol/STInteger.h index e70e516c3..f19363956 100644 --- a/src/ripple/protocol/STInteger.h +++ b/src/ripple/protocol/STInteger.h @@ -30,27 +30,19 @@ class STInteger : public STBase public: using value_type = Integer; - explicit STInteger(Integer v) : value_(v) - { - } - - STInteger(SField const& n, Integer v = 0) : STBase(n), value_(v) - { - } +private: + Integer value_; +public: + explicit STInteger(Integer v); + STInteger(SField const& n, Integer v = 0); STInteger(SerialIter& sit, SField const& name); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; SerializedTypeID getSType() const override; @@ -61,52 +53,24 @@ public: getText() const override; void - add(Serializer& s) const override - { - assert(fName->isBinary()); - assert(fName->fieldType == getSType()); - s.addInteger(value_); - } - - STInteger& - operator=(value_type const& v) - { - value_ = v; - return *this; - } - - value_type - value() const noexcept - { - return value_; - } - - void - setValue(Integer v) - { - value_ = v; - } - - operator Integer() const - { - return value_; - } - - virtual bool - isDefault() const override - { - return value_ == 0; - } + add(Serializer& s) const override; bool - isEquivalent(const STBase& t) const override - { - const STInteger* v = dynamic_cast(&t); - return v && (value_ == v->value_); - } + isDefault() const override; -private: - Integer value_; + bool + isEquivalent(const STBase& t) const override; + + STInteger& + operator=(value_type const& v); + + value_type + value() const noexcept; + + void + setValue(Integer v); + + operator Integer() const; }; using STUInt8 = STInteger; @@ -114,6 +78,83 @@ using STUInt16 = STInteger; using STUInt32 = STInteger; using STUInt64 = STInteger; +template +inline STInteger::STInteger(Integer v) : value_(v) +{ +} + +template +inline STInteger::STInteger(SField const& n, Integer v) + : STBase(n), value_(v) +{ +} + +template +inline STBase* +STInteger::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +template +inline STBase* +STInteger::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +template +inline void +STInteger::add(Serializer& s) const +{ + assert(getFName().isBinary()); + assert(getFName().fieldType == getSType()); + s.addInteger(value_); +} + +template +inline bool +STInteger::isDefault() const +{ + return value_ == 0; +} + +template +inline bool +STInteger::isEquivalent(const STBase& t) const +{ + const STInteger* v = dynamic_cast(&t); + return v && (value_ == v->value_); +} + +template +inline STInteger& +STInteger::operator=(value_type const& v) +{ + value_ = v; + return *this; +} + +template +inline typename STInteger::value_type +STInteger::value() const noexcept +{ + return value_; +} + +template +inline void +STInteger::setValue(Integer v) +{ + value_ = v; +} + +template +inline STInteger::operator Integer() const +{ + return value_; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STLedgerEntry.h b/src/ripple/protocol/STLedgerEntry.h index 0a0e3a805..ce09ade24 100644 --- a/src/ripple/protocol/STLedgerEntry.h +++ b/src/ripple/protocol/STLedgerEntry.h @@ -29,7 +29,8 @@ class Invariants_test; class STLedgerEntry final : public STObject, public CountedObject { - friend Invariants_test; // this test wants access to the private type_ + uint256 key_; + LedgerEntryType type_; public: using pointer = std::shared_ptr; @@ -37,37 +38,19 @@ public: /** Create an empty object with the given key and type. */ explicit STLedgerEntry(Keylet const& k); - - STLedgerEntry(LedgerEntryType type, uint256 const& key) - : STLedgerEntry(Keylet(type, key)) - { - } - + STLedgerEntry(LedgerEntryType type, uint256 const& key); STLedgerEntry(SerialIter& sit, uint256 const& index); - STLedgerEntry(SerialIter&& sit, uint256 const& index) - : STLedgerEntry(sit, index) - { - } - + STLedgerEntry(SerialIter&& sit, uint256 const& index); STLedgerEntry(STObject const& object, uint256 const& index); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; SerializedTypeID - getSType() const override - { - return STI_LEDGERENTRY; - } + getSType() const override; std::string getFullText() const override; @@ -83,16 +66,10 @@ public: the SHAMap associative container. */ uint256 const& - key() const - { - return key_; - } + key() const; LedgerEntryType - getType() const - { - return type_; - } + getType() const; // is this a ledger entry that can be threaded bool @@ -112,13 +89,37 @@ private: void setSLEType(); -private: - uint256 key_; - LedgerEntryType type_; + friend Invariants_test; // this test wants access to the private type_ }; using SLE = STLedgerEntry; +inline STLedgerEntry::STLedgerEntry(LedgerEntryType type, uint256 const& key) + : STLedgerEntry(Keylet(type, key)) +{ +} + +inline STLedgerEntry::STLedgerEntry(SerialIter&& sit, uint256 const& index) + : STLedgerEntry(sit, index) +{ +} + +/** Returns the 'key' (or 'index') of this item. + The key identifies this entry's position in + the SHAMap associative container. +*/ +inline uint256 const& +STLedgerEntry::key() const +{ + return key_; +} + +inline LedgerEntryType +STLedgerEntry::getType() const +{ + return type_; +} + } // namespace ripple -#endif +#endif \ No newline at end of file diff --git a/src/ripple/protocol/STObject.h b/src/ripple/protocol/STObject.h index 3d2a72b02..4868cbc61 100644 --- a/src/ripple/protocol/STObject.h +++ b/src/ripple/protocol/STObject.h @@ -50,187 +50,13 @@ throwFieldNotFound(SField const& field) class STObject : public STBase, public CountedObject { -private: // Proxy value for a STBase derived class template - class Proxy - { - protected: - using value_type = typename T::value_type; - - STObject* st_; - SOEStyle style_; - TypedField const* f_; - - Proxy(Proxy const&) = default; - Proxy(STObject* st, TypedField const* f); - value_type - value() const; - T const* - find() const; - - template - void - assign(U&& u); - }; - + class Proxy; template - class ValueProxy : private Proxy - { - private: - using value_type = typename T::value_type; - - public: - ValueProxy(ValueProxy const&) = default; - ValueProxy& - operator=(ValueProxy const&) = delete; - - template - std::enable_if_t, ValueProxy&> - operator=(U&& u); - - operator value_type() const; - - private: - friend class STObject; - - ValueProxy(STObject* st, TypedField const* f); - }; - + class ValueProxy; template - class OptionalProxy : private Proxy - { - private: - using value_type = typename T::value_type; - - using optional_type = - std::optional::type>; - - public: - OptionalProxy(OptionalProxy const&) = default; - OptionalProxy& - operator=(OptionalProxy const&) = delete; - - /** Returns `true` if the field is set. - - Fields with soeDEFAULT and set to the - default value will return `true` - */ - explicit operator bool() const noexcept; - - /** Return the contained value - - Throws: - - STObject::FieldErr if !engaged() - */ - value_type - operator*() const; - - operator optional_type() const; - - /** Explicit conversion to std::optional */ - optional_type - operator~() const; - - friend bool - operator==(OptionalProxy const& lhs, std::nullopt_t) noexcept - { - return !lhs.engaged(); - } - - friend bool - operator==(std::nullopt_t, OptionalProxy const& rhs) noexcept - { - return rhs == std::nullopt; - } - - friend bool - operator==(OptionalProxy const& lhs, optional_type const& rhs) noexcept - { - if (!lhs.engaged()) - return !rhs; - if (!rhs) - return false; - return *lhs == *rhs; - } - - friend bool - operator==(optional_type const& lhs, OptionalProxy const& rhs) noexcept - { - return rhs == lhs; - } - - friend bool - operator==(OptionalProxy const& lhs, OptionalProxy const& rhs) noexcept - { - if (lhs.engaged() != rhs.engaged()) - return false; - return !lhs.engaged() || *lhs == *rhs; - } - - friend bool - operator!=(OptionalProxy const& lhs, std::nullopt_t) noexcept - { - return !(lhs == std::nullopt); - } - - friend bool - operator!=(std::nullopt_t, OptionalProxy const& rhs) noexcept - { - return !(rhs == std::nullopt); - } - - friend bool - operator!=(OptionalProxy const& lhs, optional_type const& rhs) noexcept - { - return !(lhs == rhs); - } - - friend bool - operator!=(optional_type const& lhs, OptionalProxy const& rhs) noexcept - { - return !(lhs == rhs); - } - - friend bool - operator!=(OptionalProxy const& lhs, OptionalProxy const& rhs) noexcept - { - return !(lhs == rhs); - } - - // Emulate std::optional::value_or - value_type - value_or(value_type val) const - { - return engaged() ? this->value() : val; - } - - OptionalProxy& - operator=(std::nullopt_t const&); - OptionalProxy& - operator=(optional_type&& v); - OptionalProxy& - operator=(optional_type const& v); - - template - std::enable_if_t, OptionalProxy&> - operator=(U&& u); - - private: - friend class STObject; - - OptionalProxy(STObject* st, TypedField const* f); - - bool - engaged() const noexcept; - - void - disengage(); - - optional_type - optional_value() const; - }; + class OptionalProxy; struct Transform { @@ -240,10 +66,7 @@ private: using result_type = STBase; STBase const& - operator()(detail::STVar const& e) const - { - return e.get(); - } + operator()(detail::STVar const& e) const; }; using list_type = std::vector; @@ -255,188 +78,128 @@ public: using iterator = boost:: transform_iterator; - class FieldErr : public std::runtime_error - { - using std::runtime_error::runtime_error; - }; - - STObject(STObject&&); + virtual ~STObject() = default; STObject(STObject const&) = default; - STObject(const SOTemplate& type, SField const& name); - STObject( - const SOTemplate& type, - SerialIter& sit, - SField const& name) noexcept(false); - STObject(SerialIter& sit, SField const& name, int depth = 0) noexcept( - false); - STObject(SerialIter&& sit, SField const& name) noexcept(false) - : STObject(sit, name) - { - } STObject& operator=(STObject const&) = default; + STObject(STObject&&); STObject& operator=(STObject&& other); + STObject(const SOTemplate& type, SField const& name); + STObject(const SOTemplate& type, SerialIter& sit, SField const& name); + STObject(SerialIter& sit, SField const& name, int depth = 0); + STObject(SerialIter&& sit, SField const& name); explicit STObject(SField const& name); - virtual ~STObject() = default; + STBase* + copy(std::size_t n, void* buf) const override; STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } - - STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; iterator - begin() const - { - return iterator(v_.begin()); - } + begin() const; iterator - end() const - { - return iterator(v_.end()); - } + end() const; bool - empty() const - { - return v_.empty(); - } + empty() const; void - reserve(std::size_t n) - { - v_.reserve(n); - } + reserve(std::size_t n); void - applyTemplate(const SOTemplate& type) noexcept(false); + applyTemplate(const SOTemplate& type); void - applyTemplateFromSField(SField const&) noexcept(false); + applyTemplateFromSField(SField const&); bool - isFree() const - { - return mType == nullptr; - } + isFree() const; void set(const SOTemplate&); + bool set(SerialIter& u, int depth = 0); - virtual SerializedTypeID - getSType() const override - { - return STI_OBJECT; - } - virtual bool - isEquivalent(const STBase& t) const override; - virtual bool - isDefault() const override - { - return v_.empty(); - } + SerializedTypeID + getSType() const override; - virtual void - add(Serializer& s) const override - { - add(s, withAllFields); // just inner elements - } + bool + isEquivalent(const STBase& t) const override; + + bool + isDefault() const override; void - addWithoutSigningFields(Serializer& s) const - { - add(s, omitSigningFields); - } + add(Serializer& s) const override; - // VFALCO NOTE does this return an expensive copy of an object with a - // dynamic buffer? - // VFALCO TODO Remove this function and fix the few callers. - Serializer - getSerializer() const - { - Serializer s; - add(s, withAllFields); - return s; - } - - virtual std::string + std::string getFullText() const override; - virtual std::string + + std::string getText() const override; // TODO(tom): options should be an enum. - virtual Json::Value + Json::Value getJson(JsonOptions options) const override; + void + addWithoutSigningFields(Serializer& s) const; + + Serializer + getSerializer() const; + template std::size_t - emplace_back(Args&&... args) - { - v_.emplace_back(std::forward(args)...); - return v_.size() - 1; - } + emplace_back(Args&&... args); int - getCount() const - { - return v_.size(); - } + getCount() const; bool setFlag(std::uint32_t); bool clearFlag(std::uint32_t); bool isFlag(std::uint32_t) const; + std::uint32_t getFlags() const; uint256 getHash(HashPrefix prefix) const; + uint256 getSigningHash(HashPrefix prefix) const; const STBase& - peekAtIndex(int offset) const - { - return v_[offset].get(); - } + peekAtIndex(int offset) const; + STBase& - getIndex(int offset) - { - return v_[offset].get(); - } + getIndex(int offset); + const STBase* - peekAtPIndex(int offset) const - { - return &v_[offset].get(); - } + peekAtPIndex(int offset) const; + STBase* - getPIndex(int offset) - { - return &v_[offset].get(); - } + getPIndex(int offset); int getFieldIndex(SField const& field) const; + SField const& getFieldSType(int index) const; const STBase& peekAtField(SField const& field) const; + STBase& getField(SField const& field); + const STBase* peekAtPField(SField const& field) const; + STBase* getPField(SField const& field, bool createOkay = false); @@ -605,22 +368,7 @@ public: template void - setFieldH160(SField const& field, base_uint<160, Tag> const& v) - { - STBase* rf = getPField(field, true); - - if (!rf) - throwFieldNotFound(field); - - if (rf->getSType() == STI_NOTPRESENT) - rf = makeFieldPresent(field); - - using Bits = STBitString<160>; - if (auto cf = dynamic_cast(rf)) - cf->setValue(v); - else - Throw("Wrong field type"); - } + setFieldH160(SField const& field, base_uint<160, Tag> const& v); STObject& peekFieldObject(SField const& field); @@ -644,10 +392,9 @@ public: bool operator==(const STObject& o) const; bool - operator!=(const STObject& o) const - { - return !(*this == o); - } + operator!=(const STObject& o) const; + + class FieldErr; private: enum WhichFields : bool { @@ -676,25 +423,7 @@ private: typename V = typename std::remove_cv().value())>::type>::type> V - getFieldByValue(SField const& field) const - { - const STBase* rf = peekAtPField(field); - - if (!rf) - throwFieldNotFound(field); - - SerializedTypeID id = rf->getSType(); - - if (id == STI_NOTPRESENT) - return V(); // optional field not present - - const T* cf = dynamic_cast(rf); - - if (!cf) - Throw("Wrong field type"); - - return cf->value(); - } + getFieldByValue(SField const& field) const; // Implementations for getting (most) fields that return by const reference. // @@ -703,94 +432,210 @@ private: // 'empty' value we return in that circumstance. template V const& - getFieldByConstRef(SField const& field, V const& empty) const - { - const STBase* rf = peekAtPField(field); - - if (!rf) - throwFieldNotFound(field); - - SerializedTypeID id = rf->getSType(); - - if (id == STI_NOTPRESENT) - return empty; // optional field not present - - const T* cf = dynamic_cast(rf); - - if (!cf) - Throw("Wrong field type"); - - return *cf; - } + getFieldByConstRef(SField const& field, V const& empty) const; // Implementation for setting most fields with a setValue() method. template void - setFieldUsingSetValue(SField const& field, V value) - { - static_assert(!std::is_lvalue_reference::value, ""); - - STBase* rf = getPField(field, true); - - if (!rf) - throwFieldNotFound(field); - - if (rf->getSType() == STI_NOTPRESENT) - rf = makeFieldPresent(field); - - T* cf = dynamic_cast(rf); - - if (!cf) - Throw("Wrong field type"); - - cf->setValue(std::move(value)); - } + setFieldUsingSetValue(SField const& field, V value); // Implementation for setting fields using assignment template void - setFieldUsingAssignment(SField const& field, T const& value) - { - STBase* rf = getPField(field, true); - - if (!rf) - throwFieldNotFound(field); - - if (rf->getSType() == STI_NOTPRESENT) - rf = makeFieldPresent(field); - - T* cf = dynamic_cast(rf); - - if (!cf) - Throw("Wrong field type"); - - (*cf) = value; - } + setFieldUsingAssignment(SField const& field, T const& value); // Implementation for peeking STObjects and STArrays template T& - peekField(SField const& field) - { - STBase* rf = getPField(field, true); - - if (!rf) - throwFieldNotFound(field); - - if (rf->getSType() == STI_NOTPRESENT) - rf = makeFieldPresent(field); - - T* cf = dynamic_cast(rf); - - if (!cf) - Throw("Wrong field type"); - - return *cf; - } + peekField(SField const& field); }; //------------------------------------------------------------------------------ +template +class STObject::Proxy +{ +protected: + using value_type = typename T::value_type; + + STObject* st_; + SOEStyle style_; + TypedField const* f_; + + Proxy(Proxy const&) = default; + + Proxy(STObject* st, TypedField const* f); + + value_type + value() const; + + T const* + find() const; + + template + void + assign(U&& u); +}; + +template +class STObject::ValueProxy : private Proxy +{ +private: + using value_type = typename T::value_type; + +public: + ValueProxy(ValueProxy const&) = default; + ValueProxy& + operator=(ValueProxy const&) = delete; + + template + std::enable_if_t, ValueProxy&> + operator=(U&& u); + + operator value_type() const; + +private: + friend class STObject; + + ValueProxy(STObject* st, TypedField const* f); +}; + +template +class STObject::OptionalProxy : private Proxy +{ +private: + using value_type = typename T::value_type; + + using optional_type = std::optional::type>; + +public: + OptionalProxy(OptionalProxy const&) = default; + OptionalProxy& + operator=(OptionalProxy const&) = delete; + + /** Returns `true` if the field is set. + + Fields with soeDEFAULT and set to the + default value will return `true` + */ + explicit operator bool() const noexcept; + + /** Return the contained value + + Throws: + + STObject::FieldErr if !engaged() + */ + value_type + operator*() const; + + operator optional_type() const; + + /** Explicit conversion to std::optional */ + optional_type + operator~() const; + + friend bool + operator==(OptionalProxy const& lhs, std::nullopt_t) noexcept + { + return !lhs.engaged(); + } + + friend bool + operator==(std::nullopt_t, OptionalProxy const& rhs) noexcept + { + return rhs == std::nullopt; + } + + friend bool + operator==(OptionalProxy const& lhs, optional_type const& rhs) noexcept + { + if (!lhs.engaged()) + return !rhs; + if (!rhs) + return false; + return *lhs == *rhs; + } + + friend bool + operator==(optional_type const& lhs, OptionalProxy const& rhs) noexcept + { + return rhs == lhs; + } + + friend bool + operator==(OptionalProxy const& lhs, OptionalProxy const& rhs) noexcept + { + if (lhs.engaged() != rhs.engaged()) + return false; + return !lhs.engaged() || *lhs == *rhs; + } + + friend bool + operator!=(OptionalProxy const& lhs, std::nullopt_t) noexcept + { + return !(lhs == std::nullopt); + } + + friend bool + operator!=(std::nullopt_t, OptionalProxy const& rhs) noexcept + { + return !(rhs == std::nullopt); + } + + friend bool + operator!=(OptionalProxy const& lhs, optional_type const& rhs) noexcept + { + return !(lhs == rhs); + } + + friend bool + operator!=(optional_type const& lhs, OptionalProxy const& rhs) noexcept + { + return !(lhs == rhs); + } + + friend bool + operator!=(OptionalProxy const& lhs, OptionalProxy const& rhs) noexcept + { + return !(lhs == rhs); + } + + // Emulate std::optional::value_or + value_type + value_or(value_type val) const; + + OptionalProxy& + operator=(std::nullopt_t const&); + OptionalProxy& + operator=(optional_type&& v); + OptionalProxy& + operator=(optional_type const& v); + + template + std::enable_if_t, OptionalProxy&> + operator=(U&& u); + +private: + friend class STObject; + + OptionalProxy(STObject* st, TypedField const* f); + + bool + engaged() const noexcept; + + void + disengage(); + + optional_type + optional_value() const; +}; + +class STObject::FieldErr : public std::runtime_error +{ + using std::runtime_error::runtime_error; +}; + template STObject::Proxy::Proxy(STObject* st, TypedField const* f) : st_(st), f_(f) { @@ -973,8 +818,113 @@ STObject::OptionalProxy::optional_value() const -> optional_type return this->value(); } +template +typename STObject::OptionalProxy::value_type +STObject::OptionalProxy::value_or(value_type val) const +{ + return engaged() ? this->value() : val; +} + //------------------------------------------------------------------------------ +inline STBase const& +STObject::Transform::operator()(detail::STVar const& e) const +{ + return e.get(); +} + +//------------------------------------------------------------------------------ + +inline STObject::STObject(SerialIter&& sit, SField const& name) + : STObject(sit, name) +{ +} + +inline STObject::iterator +STObject::begin() const +{ + return iterator(v_.begin()); +} + +inline STObject::iterator +STObject::end() const +{ + return iterator(v_.end()); +} + +inline bool +STObject::empty() const +{ + return v_.empty(); +} + +inline void +STObject::reserve(std::size_t n) +{ + v_.reserve(n); +} + +inline bool +STObject::isFree() const +{ + return mType == nullptr; +} + +inline void +STObject::addWithoutSigningFields(Serializer& s) const +{ + add(s, omitSigningFields); +} + +// VFALCO NOTE does this return an expensive copy of an object with a +// dynamic buffer? +// VFALCO TODO Remove this function and fix the few callers. +inline Serializer +STObject::getSerializer() const +{ + Serializer s; + add(s, withAllFields); + return s; +} + +template +inline std::size_t +STObject::emplace_back(Args&&... args) +{ + v_.emplace_back(std::forward(args)...); + return v_.size() - 1; +} + +inline int +STObject::getCount() const +{ + return v_.size(); +} + +inline const STBase& +STObject::peekAtIndex(int offset) const +{ + return v_[offset].get(); +} + +inline STBase& +STObject::getIndex(int offset) +{ + return v_[offset].get(); +} + +inline const STBase* +STObject::peekAtPIndex(int offset) const +{ + return &v_[offset].get(); +} + +inline STBase* +STObject::getPIndex(int offset) +{ + return &v_[offset].get(); +} + template typename T::value_type STObject::operator[](TypedField const& f) const @@ -1063,6 +1013,145 @@ STObject::at(OptionaledField const& of) -> OptionalProxy return OptionalProxy(this, of.f); } +template +void +STObject::setFieldH160(SField const& field, base_uint<160, Tag> const& v) +{ + STBase* rf = getPField(field, true); + + if (!rf) + throwFieldNotFound(field); + + if (rf->getSType() == STI_NOTPRESENT) + rf = makeFieldPresent(field); + + using Bits = STBitString<160>; + if (auto cf = dynamic_cast(rf)) + cf->setValue(v); + else + Throw("Wrong field type"); +} + +inline bool +STObject::operator!=(const STObject& o) const +{ + return !(*this == o); +} + +template +V +STObject::getFieldByValue(SField const& field) const +{ + const STBase* rf = peekAtPField(field); + + if (!rf) + throwFieldNotFound(field); + + SerializedTypeID id = rf->getSType(); + + if (id == STI_NOTPRESENT) + return V(); // optional field not present + + const T* cf = dynamic_cast(rf); + + if (!cf) + Throw("Wrong field type"); + + return cf->value(); +} + +// Implementations for getting (most) fields that return by const reference. +// +// If an absent optional field is deserialized we don't have anything +// obvious to return. So we insist on having the call provide an +// 'empty' value we return in that circumstance. +template +V const& +STObject::getFieldByConstRef(SField const& field, V const& empty) const +{ + const STBase* rf = peekAtPField(field); + + if (!rf) + throwFieldNotFound(field); + + SerializedTypeID id = rf->getSType(); + + if (id == STI_NOTPRESENT) + return empty; // optional field not present + + const T* cf = dynamic_cast(rf); + + if (!cf) + Throw("Wrong field type"); + + return *cf; +} + +// Implementation for setting most fields with a setValue() method. +template +void +STObject::setFieldUsingSetValue(SField const& field, V value) +{ + static_assert(!std::is_lvalue_reference::value, ""); + + STBase* rf = getPField(field, true); + + if (!rf) + throwFieldNotFound(field); + + if (rf->getSType() == STI_NOTPRESENT) + rf = makeFieldPresent(field); + + T* cf = dynamic_cast(rf); + + if (!cf) + Throw("Wrong field type"); + + cf->setValue(std::move(value)); +} + +// Implementation for setting fields using assignment +template +void +STObject::setFieldUsingAssignment(SField const& field, T const& value) +{ + STBase* rf = getPField(field, true); + + if (!rf) + throwFieldNotFound(field); + + if (rf->getSType() == STI_NOTPRESENT) + rf = makeFieldPresent(field); + + T* cf = dynamic_cast(rf); + + if (!cf) + Throw("Wrong field type"); + + (*cf) = value; +} + +// Implementation for peeking STObjects and STArrays +template +T& +STObject::peekField(SField const& field) +{ + STBase* rf = getPField(field, true); + + if (!rf) + throwFieldNotFound(field); + + if (rf->getSType() == STI_NOTPRESENT) + rf = makeFieldPresent(field); + + T* cf = dynamic_cast(rf); + + if (!cf) + Throw("Wrong field type"); + + return *cf; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STPathSet.h b/src/ripple/protocol/STPathSet.h index 804cf818d..95d0317c2 100644 --- a/src/ripple/protocol/STPathSet.h +++ b/src/ripple/protocol/STPathSet.h @@ -32,6 +32,14 @@ namespace ripple { class STPathElement { + unsigned int mType; + AccountID mAccountID; + Currency mCurrencyID; + AccountID mIssuerID; + + bool is_offer_; + std::size_t hash_value_; + public: enum Type { typeNone = 0x00, @@ -44,204 +52,89 @@ public: // Combination of all types. }; -private: - static std::size_t - get_hash(STPathElement const& element); + STPathElement(); + STPathElement(STPathElement const&) = default; + STPathElement& + operator=(STPathElement const&) = default; -public: STPathElement( std::optional const& account, std::optional const& currency, - std::optional const& issuer) - : mType(typeNone) - { - if (!account) - { - is_offer_ = true; - } - else - { - is_offer_ = false; - mAccountID = *account; - mType |= typeAccount; - assert(mAccountID != noAccount()); - } - - if (currency) - { - mCurrencyID = *currency; - mType |= typeCurrency; - } - - if (issuer) - { - mIssuerID = *issuer; - mType |= typeIssuer; - assert(mIssuerID != noAccount()); - } - - hash_value_ = get_hash(*this); - } + std::optional const& issuer); STPathElement( AccountID const& account, Currency const& currency, AccountID const& issuer, - bool forceCurrency = false) - : mType(typeNone) - , mAccountID(account) - , mCurrencyID(currency) - , mIssuerID(issuer) - , is_offer_(isXRP(mAccountID)) - { - if (!is_offer_) - mType |= typeAccount; - - if (forceCurrency || !isXRP(currency)) - mType |= typeCurrency; - - if (!isXRP(issuer)) - mType |= typeIssuer; - - hash_value_ = get_hash(*this); - } + bool forceCurrency = false); STPathElement( unsigned int uType, AccountID const& account, Currency const& currency, - AccountID const& issuer) - : mType(uType) - , mAccountID(account) - , mCurrencyID(currency) - , mIssuerID(issuer) - , is_offer_(isXRP(mAccountID)) - { - hash_value_ = get_hash(*this); - } - - STPathElement() : mType(typeNone), is_offer_(true) - { - hash_value_ = get_hash(*this); - } - - STPathElement(STPathElement const&) = default; - STPathElement& - operator=(STPathElement const&) = default; + AccountID const& issuer); auto - getNodeType() const - { - return mType; - } + getNodeType() const; bool - isOffer() const - { - return is_offer_; - } + isOffer() const; bool - isAccount() const - { - return !isOffer(); - } + isAccount() const; bool - hasIssuer() const - { - return getNodeType() & STPathElement::typeIssuer; - } + hasIssuer() const; bool - hasCurrency() const - { - return getNodeType() & STPathElement::typeCurrency; - } + hasCurrency() const; bool - isNone() const - { - return getNodeType() == STPathElement::typeNone; - } + isNone() const; // Nodes are either an account ID or a offer prefix. Offer prefixs denote a // class of offers. AccountID const& - getAccountID() const - { - return mAccountID; - } + getAccountID() const; Currency const& - getCurrency() const - { - return mCurrencyID; - } + getCurrency() const; AccountID const& - getIssuerID() const - { - return mIssuerID; - } + getIssuerID() const; bool - operator==(const STPathElement& t) const - { - return (mType & typeAccount) == (t.mType & typeAccount) && - hash_value_ == t.hash_value_ && mAccountID == t.mAccountID && - mCurrencyID == t.mCurrencyID && mIssuerID == t.mIssuerID; - } + operator==(const STPathElement& t) const; bool - operator!=(const STPathElement& t) const - { - return !operator==(t); - } + operator!=(const STPathElement& t) const; private: - unsigned int mType; - AccountID mAccountID; - Currency mCurrencyID; - AccountID mIssuerID; - - bool is_offer_; - std::size_t hash_value_; + static std::size_t + get_hash(STPathElement const& element); }; class STPath { + std::vector mPath; + public: STPath() = default; - STPath(std::vector p) : mPath(std::move(p)) - { - } + STPath(std::vector p); std::vector::size_type - size() const - { - return mPath.size(); - } + size() const; bool - empty() const - { - return mPath.empty(); - } + empty() const; void - push_back(STPathElement const& e) - { - mPath.push_back(e); - } + push_back(STPathElement const& e); template void - emplace_back(Args&&... args) - { - mPath.emplace_back(std::forward(args)...); - } + emplace_back(Args&&... args); bool hasSeen( @@ -252,55 +145,28 @@ public: Json::Value getJson(JsonOptions) const; std::vector::const_iterator - begin() const - { - return mPath.begin(); - } + begin() const; std::vector::const_iterator - end() const - { - return mPath.end(); - } + end() const; bool - operator==(STPath const& t) const - { - return mPath == t.mPath; - } + operator==(STPath const& t) const; std::vector::const_reference - back() const - { - return mPath.back(); - } + back() const; std::vector::const_reference - front() const - { - return mPath.front(); - } + front() const; STPathElement& - operator[](int i) - { - return mPath[i]; - } + operator[](int i); const STPathElement& - operator[](int i) const - { - return mPath[i]; - } + operator[](int i) const; void - reserve(size_t s) - { - mPath.reserve(s); - } - -private: - std::vector mPath; + reserve(size_t s); }; //------------------------------------------------------------------------------ @@ -308,26 +174,19 @@ private: // A set of zero or more payment paths class STPathSet final : public STBase { + std::vector value; + public: STPathSet() = default; - STPathSet(SField const& n) : STBase(n) - { - } - + STPathSet(SField const& n); STPathSet(SerialIter& sit, SField const& name); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; void add(Serializer& s) const override; @@ -335,10 +194,7 @@ public: Json::Value getJson(JsonOptions) const override; SerializedTypeID - getSType() const override - { - return STI_PATHSET; - } + getSType() const override; bool assembleAdd(STPath const& base, STPathElement const& tail); @@ -347,64 +203,317 @@ public: isEquivalent(const STBase& t) const override; bool - isDefault() const override - { - return value.empty(); - } + isDefault() const override; // std::vector like interface: std::vector::const_reference - operator[](std::vector::size_type n) const - { - return value[n]; - } + operator[](std::vector::size_type n) const; std::vector::reference - operator[](std::vector::size_type n) - { - return value[n]; - } + operator[](std::vector::size_type n); std::vector::const_iterator - begin() const - { - return value.begin(); - } + begin() const; std::vector::const_iterator - end() const - { - return value.end(); - } + end() const; std::vector::size_type - size() const - { - return value.size(); - } + size() const; bool - empty() const - { - return value.empty(); - } + empty() const; void - push_back(STPath const& e) - { - value.push_back(e); - } + push_back(STPath const& e); template void - emplace_back(Args&&... args) + emplace_back(Args&&... args); +}; + +// ------------ STPathElement ------------ + +inline STPathElement::STPathElement() : mType(typeNone), is_offer_(true) +{ + hash_value_ = get_hash(*this); +} + +inline STPathElement::STPathElement( + std::optional const& account, + std::optional const& currency, + std::optional const& issuer) + : mType(typeNone) +{ + if (!account) { - value.emplace_back(std::forward(args)...); + is_offer_ = true; + } + else + { + is_offer_ = false; + mAccountID = *account; + mType |= typeAccount; + assert(mAccountID != noAccount()); } -private: - std::vector value; -}; + if (currency) + { + mCurrencyID = *currency; + mType |= typeCurrency; + } + + if (issuer) + { + mIssuerID = *issuer; + mType |= typeIssuer; + assert(mIssuerID != noAccount()); + } + + hash_value_ = get_hash(*this); +} + +inline STPathElement::STPathElement( + AccountID const& account, + Currency const& currency, + AccountID const& issuer, + bool forceCurrency) + : mType(typeNone) + , mAccountID(account) + , mCurrencyID(currency) + , mIssuerID(issuer) + , is_offer_(isXRP(mAccountID)) +{ + if (!is_offer_) + mType |= typeAccount; + + if (forceCurrency || !isXRP(currency)) + mType |= typeCurrency; + + if (!isXRP(issuer)) + mType |= typeIssuer; + + hash_value_ = get_hash(*this); +} + +inline STPathElement::STPathElement( + unsigned int uType, + AccountID const& account, + Currency const& currency, + AccountID const& issuer) + : mType(uType) + , mAccountID(account) + , mCurrencyID(currency) + , mIssuerID(issuer) + , is_offer_(isXRP(mAccountID)) +{ + hash_value_ = get_hash(*this); +} + +inline auto +STPathElement::getNodeType() const +{ + return mType; +} + +inline bool +STPathElement::isOffer() const +{ + return is_offer_; +} + +inline bool +STPathElement::isAccount() const +{ + return !isOffer(); +} + +inline bool +STPathElement::hasIssuer() const +{ + return getNodeType() & STPathElement::typeIssuer; +} + +inline bool +STPathElement::hasCurrency() const +{ + return getNodeType() & STPathElement::typeCurrency; +} + +inline bool +STPathElement::isNone() const +{ + return getNodeType() == STPathElement::typeNone; +} + +// Nodes are either an account ID or a offer prefix. Offer prefixs denote a +// class of offers. +inline AccountID const& +STPathElement::getAccountID() const +{ + return mAccountID; +} + +inline Currency const& +STPathElement::getCurrency() const +{ + return mCurrencyID; +} + +inline AccountID const& +STPathElement::getIssuerID() const +{ + return mIssuerID; +} + +inline bool +STPathElement::operator==(const STPathElement& t) const +{ + return (mType & typeAccount) == (t.mType & typeAccount) && + hash_value_ == t.hash_value_ && mAccountID == t.mAccountID && + mCurrencyID == t.mCurrencyID && mIssuerID == t.mIssuerID; +} + +inline bool +STPathElement::operator!=(const STPathElement& t) const +{ + return !operator==(t); +} + +// ------------ STPath ------------ + +inline STPath::STPath(std::vector p) : mPath(std::move(p)) +{ +} + +inline std::vector::size_type +STPath::size() const +{ + return mPath.size(); +} + +inline bool +STPath::empty() const +{ + return mPath.empty(); +} + +inline void +STPath::push_back(STPathElement const& e) +{ + mPath.push_back(e); +} + +template +inline void +STPath::emplace_back(Args&&... args) +{ + mPath.emplace_back(std::forward(args)...); +} + +inline std::vector::const_iterator +STPath::begin() const +{ + return mPath.begin(); +} + +inline std::vector::const_iterator +STPath::end() const +{ + return mPath.end(); +} + +inline bool +STPath::operator==(STPath const& t) const +{ + return mPath == t.mPath; +} + +inline std::vector::const_reference +STPath::back() const +{ + return mPath.back(); +} + +inline std::vector::const_reference +STPath::front() const +{ + return mPath.front(); +} + +inline STPathElement& +STPath::operator[](int i) +{ + return mPath[i]; +} + +inline const STPathElement& +STPath::operator[](int i) const +{ + return mPath[i]; +} + +inline void +STPath::reserve(size_t s) +{ + mPath.reserve(s); +} + +// ------------ STPathSet ------------ + +inline STPathSet::STPathSet(SField const& n) : STBase(n) +{ +} + +// std::vector like interface: +inline std::vector::const_reference +STPathSet::operator[](std::vector::size_type n) const +{ + return value[n]; +} + +inline std::vector::reference +STPathSet::operator[](std::vector::size_type n) +{ + return value[n]; +} + +inline std::vector::const_iterator +STPathSet::begin() const +{ + return value.begin(); +} + +inline std::vector::const_iterator +STPathSet::end() const +{ + return value.end(); +} + +inline std::vector::size_type +STPathSet::size() const +{ + return value.size(); +} + +inline bool +STPathSet::empty() const +{ + return value.empty(); +} + +inline void +STPathSet::push_back(STPath const& e) +{ + value.push_back(e); +} + +template +inline void +STPathSet::emplace_back(Args&&... args) +{ + value.emplace_back(std::forward(args)...); +} } // namespace ripple diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index 706a99359..af710761a 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -42,23 +42,21 @@ enum TxnSql : char { class STTx final : public STObject, public CountedObject { + uint256 tid_; + TxType tx_type_; + public: static std::size_t const minMultiSigners = 1; static std::size_t const maxMultiSigners = 8; -public: STTx() = delete; + STTx(STTx const& other) = default; STTx& operator=(STTx const& other) = delete; - STTx(STTx const& other) = default; - - explicit STTx(SerialIter& sit) noexcept(false); - explicit STTx(SerialIter&& sit) noexcept(false) : STTx(sit) - { - } - - explicit STTx(STObject&& object) noexcept(false); + explicit STTx(SerialIter& sit); + explicit STTx(SerialIter&& sit); + explicit STTx(STObject&& object); /** Constructs a transaction. @@ -69,23 +67,15 @@ public: STTx(TxType type, std::function assembler); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; // STObject functions. SerializedTypeID - getSType() const override - { - return STI_TRANSACTION; - } + getSType() const override; + std::string getFullText() const override; @@ -97,16 +87,10 @@ public: getSigningHash() const; TxType - getTxnType() const - { - return tx_type_; - } + getTxnType() const; Blob - getSigningPubKey() const - { - return getFieldVL(sfSigningPubKey); - } + getSigningPubKey() const; SeqProxy getSeqProxy() const; @@ -115,10 +99,7 @@ public: getMentionedAccounts() const; uint256 - getTransactionID() const - { - return tid_; - } + getTransactionID() const; Json::Value getJson(JsonOptions options) const override; @@ -156,9 +137,6 @@ private: Expected checkMultiSign(RequireFullyCanonicalSig requireCanonicalSig) const; - - uint256 tid_; - TxType tx_type_; }; bool @@ -178,6 +156,28 @@ sterilize(STTx const& stx); bool isPseudoTx(STObject const& tx); +inline STTx::STTx(SerialIter&& sit) : STTx(sit) +{ +} + +inline TxType +STTx::getTxnType() const +{ + return tx_type_; +} + +inline Blob +STTx::getSigningPubKey() const +{ + return getFieldVL(sfSigningPubKey); +} + +inline uint256 +STTx::getTransactionID() const +{ + return tid_; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index a73604e62..d6ae93e23 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -43,6 +43,21 @@ constexpr std::uint32_t vfFullyCanonicalSig = 0x80000000; class STValidation final : public STObject, public CountedObject { + bool mTrusted = false; + + // Determines the validity of the signature in this validation; unseated + // optional if we haven't yet checked it, a boolean otherwise. + mutable std::optional valid_; + + // The public key associated with the key used to sign this validation + PublicKey const signingPubKey_; + + // The ID of the validator that issued this validation. For validators + // that use manifests this will be derived from the master public key. + NodeID const nodeID_; + + NetClock::time_point seenTime_ = {}; + public: /** Construct a STValidation from a peer from serialized data. @@ -61,27 +76,7 @@ public: STValidation( SerialIter& sit, LookupNodeID&& lookupNodeID, - bool checkSignature) - : STObject(validationFormat(), sit, sfValidation) - , signingPubKey_([this]() { - auto const spk = getFieldVL(sfSigningPubKey); - - if (publicKeyType(makeSlice(spk)) != KeyType::secp256k1) - Throw("Invalid public key in validation"); - - return PublicKey{makeSlice(spk)}; - }()) - , nodeID_(lookupNodeID(signingPubKey_)) - { - if (checkSignature && !isValid()) - { - JLOG(debugLog().error()) << "Invalid signature in validation: " - << getJson(JsonOptions::none); - Throw("Invalid signature in validation"); - } - - assert(nodeID_.isNonZero()); - } + bool checkSignature); /** Construct, sign and trust a new STValidation issued by this node. @@ -97,54 +92,13 @@ public: PublicKey const& pk, SecretKey const& sk, NodeID const& nodeID, - F&& f) - : STObject(validationFormat(), sfValidation) - , signingPubKey_(pk) - , nodeID_(nodeID) - , seenTime_(signTime) - { - assert(nodeID_.isNonZero()); - - // First, set our own public key: - if (publicKeyType(pk) != KeyType::secp256k1) - LogicError( - "We can only use secp256k1 keys for signing validations"); - - setFieldVL(sfSigningPubKey, pk.slice()); - setFieldU32(sfSigningTime, signTime.time_since_epoch().count()); - - // Perform additional initialization - f(*this); - - // Finally, sign the validation and mark it as trusted: - setFlag(vfFullyCanonicalSig); - setFieldVL(sfSignature, signDigest(pk, sk, getSigningHash())); - setTrusted(); - - // Check to ensure that all required fields are present. - for (auto const& e : validationFormat()) - { - if (e.style() == soeREQUIRED && !isFieldPresent(e.sField())) - LogicError( - "Required field '" + e.sField().getName() + - "' missing from validation."); - } - - // We just signed this, so it should be valid. - valid_ = true; - } + F&& f); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; // Hash of the validated ledger uint256 @@ -161,16 +115,10 @@ public: getSeenTime() const noexcept; PublicKey const& - getSignerPublic() const noexcept - { - return signingPubKey_; - } + getSignerPublic() const noexcept; NodeID const& - getNodeID() const noexcept - { - return nodeID_; - } + getNodeID() const noexcept; bool isValid() const noexcept; @@ -179,31 +127,19 @@ public: isFull() const noexcept; bool - isTrusted() const noexcept - { - return mTrusted; - } + isTrusted() const noexcept; uint256 getSigningHash() const; void - setTrusted() - { - mTrusted = true; - } + setTrusted(); void - setUntrusted() - { - mTrusted = false; - } + setUntrusted(); void - setSeen(NetClock::time_point s) - { - seenTime_ = s; - } + setSeen(NetClock::time_point s); Blob getSerialized() const; @@ -214,23 +150,120 @@ public: private: static SOTemplate const& validationFormat(); - - bool mTrusted = false; - - // Determines the validity of the signature in this validation; unseated - // optional if we haven't yet checked it, a boolean otherwise. - mutable std::optional valid_; - - // The public key associated with the key used to sign this validation - PublicKey const signingPubKey_; - - // The ID of the validator that issued this validation. For validators - // that use manifests this will be derived from the master public key. - NodeID const nodeID_; - - NetClock::time_point seenTime_ = {}; }; +template +STValidation::STValidation( + SerialIter& sit, + LookupNodeID&& lookupNodeID, + bool checkSignature) + : STObject(validationFormat(), sit, sfValidation) + , signingPubKey_([this]() { + auto const spk = getFieldVL(sfSigningPubKey); + + if (publicKeyType(makeSlice(spk)) != KeyType::secp256k1) + Throw("Invalid public key in validation"); + + return PublicKey{makeSlice(spk)}; + }()) + , nodeID_(lookupNodeID(signingPubKey_)) +{ + if (checkSignature && !isValid()) + { + JLOG(debugLog().error()) << "Invalid signature in validation: " + << getJson(JsonOptions::none); + Throw("Invalid signature in validation"); + } + + assert(nodeID_.isNonZero()); +} + +/** Construct, sign and trust a new STValidation issued by this node. + + @param signTime When the validation is signed + @param publicKey The current signing public key + @param secretKey The current signing secret key + @param nodeID ID corresponding to node's public master key + @param f callback function to "fill" the validation with necessary data +*/ +template +STValidation::STValidation( + NetClock::time_point signTime, + PublicKey const& pk, + SecretKey const& sk, + NodeID const& nodeID, + F&& f) + : STObject(validationFormat(), sfValidation) + , signingPubKey_(pk) + , nodeID_(nodeID) + , seenTime_(signTime) +{ + assert(nodeID_.isNonZero()); + + // First, set our own public key: + if (publicKeyType(pk) != KeyType::secp256k1) + LogicError("We can only use secp256k1 keys for signing validations"); + + setFieldVL(sfSigningPubKey, pk.slice()); + setFieldU32(sfSigningTime, signTime.time_since_epoch().count()); + + // Perform additional initialization + f(*this); + + // Finally, sign the validation and mark it as trusted: + setFlag(vfFullyCanonicalSig); + setFieldVL(sfSignature, signDigest(pk, sk, getSigningHash())); + setTrusted(); + + // Check to ensure that all required fields are present. + for (auto const& e : validationFormat()) + { + if (e.style() == soeREQUIRED && !isFieldPresent(e.sField())) + LogicError( + "Required field '" + e.sField().getName() + + "' missing from validation."); + } + + // We just signed this, so it should be valid. + valid_ = true; +} + +inline PublicKey const& +STValidation::getSignerPublic() const noexcept +{ + return signingPubKey_; +} + +inline NodeID const& +STValidation::getNodeID() const noexcept +{ + return nodeID_; +} + +inline bool +STValidation::isTrusted() const noexcept +{ + return mTrusted; +} + +inline void +STValidation::setTrusted() +{ + mTrusted = true; +} + +inline void +STValidation::setUntrusted() +{ + mTrusted = false; +} + +inline void +STValidation::setSeen(NetClock::time_point s) +{ + seenTime_ = s; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/STVector256.h b/src/ripple/protocol/STVector256.h index 0a4b90b09..ff59b6336 100644 --- a/src/ripple/protocol/STVector256.h +++ b/src/ripple/protocol/STVector256.h @@ -28,43 +28,26 @@ namespace ripple { class STVector256 : public STBase { + std::vector mValue; + public: using value_type = std::vector const&; STVector256() = default; - explicit STVector256(SField const& n) : STBase(n) - { - } - - explicit STVector256(std::vector const& vector) : mValue(vector) - { - } - - STVector256(SField const& n, std::vector const& vector) - : STBase(n), mValue(vector) - { - } - + explicit STVector256(SField const& n); + explicit STVector256(std::vector const& vector); + STVector256(SField const& n, std::vector const& vector); STVector256(SerialIter& sit, SField const& name); STBase* - copy(std::size_t n, void* buf) const override - { - return emplace(n, buf, *this); - } + copy(std::size_t n, void* buf) const override; STBase* - move(std::size_t n, void* buf) override - { - return emplace(n, buf, std::move(*this)); - } + move(std::size_t n, void* buf) override; SerializedTypeID - getSType() const override - { - return STI_VECTOR256; - } + getSType() const override; void add(Serializer& s) const override; @@ -75,131 +58,200 @@ public: isEquivalent(const STBase& t) const override; bool - isDefault() const override - { - return mValue.empty(); - } + isDefault() const override; STVector256& - operator=(std::vector const& v) - { - mValue = v; - return *this; - } + operator=(std::vector const& v); STVector256& - operator=(std::vector&& v) - { - mValue = std::move(v); - return *this; - } + operator=(std::vector&& v); void - setValue(const STVector256& v) - { - mValue = v.mValue; - } + setValue(const STVector256& v); /** Retrieve a copy of the vector we contain */ - explicit operator std::vector() const - { - return mValue; - } + explicit operator std::vector() const; std::size_t - size() const - { - return mValue.size(); - } + size() const; void - resize(std::size_t n) - { - return mValue.resize(n); - } + resize(std::size_t n); bool - empty() const - { - return mValue.empty(); - } + empty() const; std::vector::reference - operator[](std::vector::size_type n) - { - return mValue[n]; - } + operator[](std::vector::size_type n); std::vector::const_reference - operator[](std::vector::size_type n) const - { - return mValue[n]; - } + operator[](std::vector::size_type n) const; std::vector const& - value() const - { - return mValue; - } + value() const; std::vector::iterator - insert(std::vector::const_iterator pos, uint256 const& value) - { - return mValue.insert(pos, value); - } + insert(std::vector::const_iterator pos, uint256 const& value); std::vector::iterator - insert(std::vector::const_iterator pos, uint256&& value) - { - return mValue.insert(pos, std::move(value)); - } + insert(std::vector::const_iterator pos, uint256&& value); void - push_back(uint256 const& v) - { - mValue.push_back(v); - } + push_back(uint256 const& v); std::vector::iterator - begin() - { - return mValue.begin(); - } + begin(); std::vector::const_iterator - begin() const - { - return mValue.begin(); - } + begin() const; std::vector::iterator - end() - { - return mValue.end(); - } + end(); std::vector::const_iterator - end() const - { - return mValue.end(); - } + end() const; std::vector::iterator - erase(std::vector::iterator position) - { - return mValue.erase(position); - } + erase(std::vector::iterator position); void - clear() noexcept - { - return mValue.clear(); - } - -private: - std::vector mValue; + clear() noexcept; }; +inline STVector256::STVector256(SField const& n) : STBase(n) +{ +} + +inline STVector256::STVector256(std::vector const& vector) + : mValue(vector) +{ +} + +inline STVector256::STVector256( + SField const& n, + std::vector const& vector) + : STBase(n), mValue(vector) +{ +} + +inline STVector256& +STVector256::operator=(std::vector const& v) +{ + mValue = v; + return *this; +} + +inline STVector256& +STVector256::operator=(std::vector&& v) +{ + mValue = std::move(v); + return *this; +} + +inline void +STVector256::setValue(const STVector256& v) +{ + mValue = v.mValue; +} + +/** Retrieve a copy of the vector we contain */ +inline STVector256::operator std::vector() const +{ + return mValue; +} + +inline std::size_t +STVector256::size() const +{ + return mValue.size(); +} + +inline void +STVector256::resize(std::size_t n) +{ + return mValue.resize(n); +} + +inline bool +STVector256::empty() const +{ + return mValue.empty(); +} + +inline std::vector::reference +STVector256::operator[](std::vector::size_type n) +{ + return mValue[n]; +} + +inline std::vector::const_reference +STVector256::operator[](std::vector::size_type n) const +{ + return mValue[n]; +} + +inline std::vector const& +STVector256::value() const +{ + return mValue; +} + +inline std::vector::iterator +STVector256::insert( + std::vector::const_iterator pos, + uint256 const& value) +{ + return mValue.insert(pos, value); +} + +inline std::vector::iterator +STVector256::insert(std::vector::const_iterator pos, uint256&& value) +{ + return mValue.insert(pos, std::move(value)); +} + +inline void +STVector256::push_back(uint256 const& v) +{ + mValue.push_back(v); +} + +inline std::vector::iterator +STVector256::begin() +{ + return mValue.begin(); +} + +inline std::vector::const_iterator +STVector256::begin() const +{ + return mValue.begin(); +} + +inline std::vector::iterator +STVector256::end() +{ + return mValue.end(); +} + +inline std::vector::const_iterator +STVector256::end() const +{ + return mValue.end(); +} + +inline std::vector::iterator +STVector256::erase(std::vector::iterator position) +{ + return mValue.erase(position); +} + +inline void +STVector256::clear() noexcept +{ + return mValue.clear(); +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/impl/STAccount.cpp b/src/ripple/protocol/impl/STAccount.cpp index 9453e990c..5881ae5b2 100644 --- a/src/ripple/protocol/impl/STAccount.cpp +++ b/src/ripple/protocol/impl/STAccount.cpp @@ -59,6 +59,50 @@ STAccount::STAccount(SField const& n, AccountID const& v) { } +STBase* +STAccount::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STAccount::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +SerializedTypeID +STAccount::getSType() const +{ + return STI_ACCOUNT; +} + +void +STAccount::add(Serializer& s) const +{ + assert(getFName().isBinary()); + assert(getFName().fieldType == STI_ACCOUNT); + + // Preserve the serialization behavior of an STBlob: + // o If we are default (all zeros) serialize as an empty blob. + // o Otherwise serialize 160 bits. + int const size = isDefault() ? 0 : uint160::bytes; + s.addVL(value_.data(), size); +} + +bool +STAccount::isEquivalent(const STBase& t) const +{ + auto const* const tPtr = dynamic_cast(&t); + return tPtr && (default_ == tPtr->default_) && (value_ == tPtr->value_); +} + +bool +STAccount::isDefault() const +{ + return default_; +} + std::string STAccount::getText() const { diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 40132e3b4..d764eb00d 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -292,6 +292,18 @@ STAmount::construct(SerialIter& sit, SField const& name) return std::make_unique(sit, name); } +STBase* +STAmount::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STAmount::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + //------------------------------------------------------------------------------ // // Conversion @@ -485,6 +497,12 @@ STAmount::setJson(Json::Value& elem) const // //------------------------------------------------------------------------------ +SerializedTypeID +STAmount::getSType() const +{ + return STI_AMOUNT; +} + std::string STAmount::getFullText() const { @@ -638,6 +656,12 @@ STAmount::isEquivalent(const STBase& t) const return v && (*v == *this); } +bool +STAmount::isDefault() const +{ + return (mValue == 0) && mIsNative; +} + //------------------------------------------------------------------------------ // amount = mValue * [10 ^ mOffset] diff --git a/src/ripple/protocol/impl/STArray.cpp b/src/ripple/protocol/impl/STArray.cpp index 229b3700a..7b1e5d9f2 100644 --- a/src/ripple/protocol/impl/STArray.cpp +++ b/src/ripple/protocol/impl/STArray.cpp @@ -89,6 +89,18 @@ STArray::STArray(SerialIter& sit, SField const& f, int depth) : STBase(f) } } +STBase* +STArray::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STArray::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + std::string STArray::getFullText() const { @@ -153,6 +165,12 @@ STArray::add(Serializer& s) const } } +SerializedTypeID +STArray::getSType() const +{ + return STI_ARRAY; +} + bool STArray::isEquivalent(const STBase& t) const { @@ -160,6 +178,12 @@ STArray::isEquivalent(const STBase& t) const return v != nullptr && v_ == v->v_; } +bool +STArray::isDefault() const +{ + return v_.empty(); +} + void STArray::sort(bool (*compare)(const STObject&, const STObject&)) { diff --git a/src/ripple/protocol/impl/STBase.cpp b/src/ripple/protocol/impl/STBase.cpp index f447b1fe2..b49b65bf2 100644 --- a/src/ripple/protocol/impl/STBase.cpp +++ b/src/ripple/protocol/impl/STBase.cpp @@ -53,6 +53,18 @@ STBase::operator!=(const STBase& t) const return (getSType() != t.getSType()) || !isEquivalent(t); } +STBase* +STBase::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STBase::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + SerializedTypeID STBase::getSType() const { diff --git a/src/ripple/protocol/impl/STBlob.cpp b/src/ripple/protocol/impl/STBlob.cpp index ab24079c9..871c535e9 100644 --- a/src/ripple/protocol/impl/STBlob.cpp +++ b/src/ripple/protocol/impl/STBlob.cpp @@ -27,12 +27,40 @@ STBlob::STBlob(SerialIter& st, SField const& name) { } +STBase* +STBlob::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STBlob::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +SerializedTypeID +STBlob::getSType() const +{ + return STI_VL; +} + std::string STBlob::getText() const { return strHex(value_); } +void +STBlob::add(Serializer& s) const +{ + assert(getFName().isBinary()); + assert( + (getFName().fieldType == STI_VL) || + (getFName().fieldType == STI_ACCOUNT)); + s.addVL(value_.data(), value_.size()); +} + bool STBlob::isEquivalent(const STBase& t) const { @@ -40,4 +68,10 @@ STBlob::isEquivalent(const STBase& t) const return v && (value_ == v->value_); } +bool +STBlob::isDefault() const +{ + return value_.empty(); +} + } // namespace ripple diff --git a/src/ripple/protocol/impl/STLedgerEntry.cpp b/src/ripple/protocol/impl/STLedgerEntry.cpp index 8694782c7..8ad5e1fb5 100644 --- a/src/ripple/protocol/impl/STLedgerEntry.cpp +++ b/src/ripple/protocol/impl/STLedgerEntry.cpp @@ -88,6 +88,24 @@ STLedgerEntry::getFullText() const return ret; } +STBase* +STLedgerEntry::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STLedgerEntry::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +SerializedTypeID +STLedgerEntry::getSType() const +{ + return STI_LEDGERENTRY; +} + std::string STLedgerEntry::getText() const { diff --git a/src/ripple/protocol/impl/STObject.cpp b/src/ripple/protocol/impl/STObject.cpp index e05b1cb9a..0d5500738 100644 --- a/src/ripple/protocol/impl/STObject.cpp +++ b/src/ripple/protocol/impl/STObject.cpp @@ -40,10 +40,7 @@ STObject::STObject(SOTemplate const& type, SField const& name) : STBase(name) set(type); } -STObject::STObject( - SOTemplate const& type, - SerialIter& sit, - SField const& name) noexcept(false) +STObject::STObject(SOTemplate const& type, SerialIter& sit, SField const& name) : STBase(name) { v_.reserve(type.size()); @@ -60,6 +57,36 @@ STObject::STObject(SerialIter& sit, SField const& name, int depth) noexcept( set(sit, depth); } +STBase* +STObject::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STObject::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +SerializedTypeID +STObject::getSType() const +{ + return STI_OBJECT; +} + +bool +STObject::isDefault() const +{ + return v_.empty(); +} + +void +STObject::add(Serializer& s) const +{ + add(s, withAllFields); // just inner elements +} + STObject& STObject::operator=(STObject&& other) { @@ -86,7 +113,7 @@ STObject::set(const SOTemplate& type) } void -STObject::applyTemplate(const SOTemplate& type) noexcept(false) +STObject::applyTemplate(const SOTemplate& type) { auto throwFieldErr = [](std::string const& field, char const* description) { std::stringstream ss; @@ -140,7 +167,7 @@ STObject::applyTemplate(const SOTemplate& type) noexcept(false) } void -STObject::applyTemplateFromSField(SField const& sField) noexcept(false) +STObject::applyTemplateFromSField(SField const& sField) { SOTemplate const* elements = InnerObjectFormats::getInstance().findSOTemplateBySField(sField); @@ -150,7 +177,7 @@ STObject::applyTemplateFromSField(SField const& sField) noexcept(false) // return true = terminated with end-of-object bool -STObject::set(SerialIter& sit, int depth) noexcept(false) +STObject::set(SerialIter& sit, int depth) { bool reachedEndOfObject = false; @@ -229,9 +256,9 @@ STObject::getFullText() const std::string ret; bool first = true; - if (fName->hasName()) + if (getFName().hasName()) { - ret = fName->getName(); + ret = getFName().getName(); ret += " = {"; } else diff --git a/src/ripple/protocol/impl/STPathSet.cpp b/src/ripple/protocol/impl/STPathSet.cpp index 620cbaab0..cc1e367ba 100644 --- a/src/ripple/protocol/impl/STPathSet.cpp +++ b/src/ripple/protocol/impl/STPathSet.cpp @@ -101,6 +101,18 @@ STPathSet::STPathSet(SerialIter& sit, SField const& name) : STBase(name) } } +STBase* +STPathSet::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STPathSet::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + bool STPathSet::assembleAdd(STPath const& base, STPathElement const& tail) { // assemble base+tail and add it to the set if it's not a duplicate @@ -129,6 +141,12 @@ STPathSet::isEquivalent(const STBase& t) const return v && (value == v->value); } +bool +STPathSet::isDefault() const +{ + return value.empty(); +} + bool STPath::hasSeen( AccountID const& account, @@ -181,11 +199,17 @@ STPathSet::getJson(JsonOptions options) const return ret; } +SerializedTypeID +STPathSet::getSType() const +{ + return STI_PATHSET; +} + void STPathSet::add(Serializer& s) const { - assert(fName->isBinary()); - assert(fName->fieldType == STI_PATHSET); + assert(getFName().isBinary()); + assert(getFName().fieldType == STI_PATHSET); bool first = true; for (auto const& spPath : value) diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index a7a45912e..c8e05c742 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -56,14 +56,14 @@ getTxFormat(TxType type) return format; } -STTx::STTx(STObject&& object) noexcept(false) : STObject(std::move(object)) +STTx::STTx(STObject&& object) : STObject(std::move(object)) { tx_type_ = safe_cast(getFieldU16(sfTransactionType)); applyTemplate(getTxFormat(tx_type_)->getSOTemplate()); // may throw tid_ = getHash(HashPrefix::transactionID); } -STTx::STTx(SerialIter& sit) noexcept(false) : STObject(sfTransaction) +STTx::STTx(SerialIter& sit) : STObject(sfTransaction) { int length = sit.getBytesLeft(); @@ -97,6 +97,25 @@ STTx::STTx(TxType type, std::function assembler) tid_ = getHash(HashPrefix::transactionID); } +STBase* +STTx::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STTx::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +// STObject functions. +SerializedTypeID +STTx::getSType() const +{ + return STI_TRANSACTION; +} + std::string STTx::getFullText() const { diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index 54e6ebffd..8d9b3563c 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -25,6 +25,18 @@ namespace ripple { +STBase* +STValidation::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STValidation::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + SOTemplate const& STValidation::validationFormat() { diff --git a/src/ripple/protocol/impl/STVector256.cpp b/src/ripple/protocol/impl/STVector256.cpp index 0b1281f8a..f74670ac0 100644 --- a/src/ripple/protocol/impl/STVector256.cpp +++ b/src/ripple/protocol/impl/STVector256.cpp @@ -41,11 +41,35 @@ STVector256::STVector256(SerialIter& sit, SField const& name) : STBase(name) } } +STBase* +STVector256::copy(std::size_t n, void* buf) const +{ + return emplace(n, buf, *this); +} + +STBase* +STVector256::move(std::size_t n, void* buf) +{ + return emplace(n, buf, std::move(*this)); +} + +SerializedTypeID +STVector256::getSType() const +{ + return STI_VECTOR256; +} + +bool +STVector256::isDefault() const +{ + return mValue.empty(); +} + void STVector256::add(Serializer& s) const { - assert(fName->isBinary()); - assert(fName->fieldType == STI_VECTOR256); + assert(getFName().isBinary()); + assert(getFName().fieldType == STI_VECTOR256); s.addVL(mValue.begin(), mValue.end(), mValue.size() * (256 / 8)); } From 0c13676d5f8a49842cdaffe8e427e0a6ad3fa6c7 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Tue, 19 Oct 2021 14:02:30 -0400 Subject: [PATCH 02/19] Restrict access to the virtual functions move and copy * These are meant to be used by detail::STVar only and are otherwise error-prone to call. --- src/ripple/protocol/STAccount.h | 13 ++++++++----- src/ripple/protocol/STAmount.h | 12 +++++++----- src/ripple/protocol/STArray.h | 14 ++++++++------ src/ripple/protocol/STBase.h | 17 ++++++++++++----- src/ripple/protocol/STBitString.h | 14 ++++++++------ src/ripple/protocol/STBlob.h | 14 ++++++++------ src/ripple/protocol/STInteger.h | 14 ++++++++------ src/ripple/protocol/STLedgerEntry.h | 13 +++++++------ src/ripple/protocol/STObject.h | 13 +++++++------ src/ripple/protocol/STPathSet.h | 14 ++++++++------ src/ripple/protocol/STTx.h | 13 +++++++------ src/ripple/protocol/STValidation.h | 13 +++++++------ src/ripple/protocol/STVector256.h | 14 ++++++++------ 13 files changed, 103 insertions(+), 75 deletions(-) diff --git a/src/ripple/protocol/STAccount.h b/src/ripple/protocol/STAccount.h index 708eb79a2..fb6d1c81b 100644 --- a/src/ripple/protocol/STAccount.h +++ b/src/ripple/protocol/STAccount.h @@ -46,11 +46,6 @@ public: STAccount(SerialIter& sit, SField const& name); STAccount(SField const& n, AccountID const& v); - STBase* - copy(std::size_t n, void* buf) const override; - STBase* - move(std::size_t n, void* buf) override; - SerializedTypeID getSType() const override; @@ -74,6 +69,14 @@ public: void setValue(AccountID const& v); + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; inline STAccount& diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 3114cb0e1..0f3023aaf 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -144,11 +144,6 @@ public: STAmount(IOUAmount const& amount, Issue const& issue); STAmount(XRPAmount const& amount); - STBase* - copy(std::size_t n, void* buf) const override; - STBase* - move(std::size_t n, void* buf) override; - //-------------------------------------------------------------------------- // // Observers @@ -273,6 +268,13 @@ private: set(std::int64_t v); void canonicalize(); + + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/STArray.h b/src/ripple/protocol/STArray.h index 5c4deae4e..8a38928cb 100644 --- a/src/ripple/protocol/STArray.h +++ b/src/ripple/protocol/STArray.h @@ -50,12 +50,6 @@ public: explicit STArray(int n); explicit STArray(SField const& f); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - STObject& operator[](std::size_t j); @@ -134,6 +128,14 @@ public: bool isDefault() const override; + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; inline STObject& diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h index b8f34005d..914a3e0f6 100644 --- a/src/ripple/protocol/STBase.h +++ b/src/ripple/protocol/STBase.h @@ -33,6 +33,10 @@ namespace ripple { enum class JsonOptions { none = 0, include_date = 1 }; +namespace detail { +class STVar; +} + // VFALCO TODO fix this restriction on copy assignment. // // CAUTION: Do not create a vector (or similar container) of any object derived @@ -77,11 +81,6 @@ public: bool operator!=(const STBase& t) const; - virtual STBase* - copy(std::size_t n, void* buf) const; - virtual STBase* - move(std::size_t n, void* buf); - template D& downcast(); @@ -126,6 +125,14 @@ protected: template static STBase* emplace(std::size_t n, void* buf, T&& val); + +private: + virtual STBase* + copy(std::size_t n, void* buf) const; + virtual STBase* + move(std::size_t n, void* buf); + + friend class detail::STVar; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/STBitString.h b/src/ripple/protocol/STBitString.h index 7ad271a7d..2e242a4b2 100644 --- a/src/ripple/protocol/STBitString.h +++ b/src/ripple/protocol/STBitString.h @@ -42,12 +42,6 @@ public: STBitString(SField const& n, const value_type& v); STBitString(SerialIter& sit, SField const& name); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - SerializedTypeID getSType() const override; @@ -71,6 +65,14 @@ public: value() const; operator value_type() const; + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; using STHash128 = STBitString<128>; diff --git a/src/ripple/protocol/STBlob.h b/src/ripple/protocol/STBlob.h index 3be671ae5..58a61206b 100644 --- a/src/ripple/protocol/STBlob.h +++ b/src/ripple/protocol/STBlob.h @@ -45,12 +45,6 @@ public: STBlob(SField const& n); STBlob(SerialIter&, SField const& name = sfGeneric); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - std::size_t size() const; @@ -83,6 +77,14 @@ public: void setValue(Buffer&& b); + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; inline STBlob::STBlob(STBlob const& rhs) diff --git a/src/ripple/protocol/STInteger.h b/src/ripple/protocol/STInteger.h index f19363956..9d385fd98 100644 --- a/src/ripple/protocol/STInteger.h +++ b/src/ripple/protocol/STInteger.h @@ -38,12 +38,6 @@ public: STInteger(SField const& n, Integer v = 0); STInteger(SerialIter& sit, SField const& name); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - SerializedTypeID getSType() const override; @@ -71,6 +65,14 @@ public: setValue(Integer v); operator Integer() const; + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; using STUInt8 = STInteger; diff --git a/src/ripple/protocol/STLedgerEntry.h b/src/ripple/protocol/STLedgerEntry.h index ce09ade24..14732ff5c 100644 --- a/src/ripple/protocol/STLedgerEntry.h +++ b/src/ripple/protocol/STLedgerEntry.h @@ -43,12 +43,6 @@ public: STLedgerEntry(SerialIter&& sit, uint256 const& index); STLedgerEntry(STObject const& object, uint256 const& index); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - SerializedTypeID getSType() const override; @@ -90,6 +84,13 @@ private: setSLEType(); friend Invariants_test; // this test wants access to the private type_ + + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; using SLE = STLedgerEntry; diff --git a/src/ripple/protocol/STObject.h b/src/ripple/protocol/STObject.h index 4868cbc61..97bc2b4e5 100644 --- a/src/ripple/protocol/STObject.h +++ b/src/ripple/protocol/STObject.h @@ -92,12 +92,6 @@ public: STObject(SerialIter&& sit, SField const& name); explicit STObject(SField const& name); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - iterator begin() const; @@ -448,6 +442,13 @@ private: template T& peekField(SField const& field); + + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/STPathSet.h b/src/ripple/protocol/STPathSet.h index 95d0317c2..3ac2c0797 100644 --- a/src/ripple/protocol/STPathSet.h +++ b/src/ripple/protocol/STPathSet.h @@ -182,12 +182,6 @@ public: STPathSet(SField const& n); STPathSet(SerialIter& sit, SField const& name); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - void add(Serializer& s) const override; @@ -230,6 +224,14 @@ public: template void emplace_back(Args&&... args); + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; // ------------ STPathElement ------------ diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index af710761a..ca33abf8a 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -66,12 +66,6 @@ public: */ STTx(TxType type, std::function assembler); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - // STObject functions. SerializedTypeID getSType() const override; @@ -137,6 +131,13 @@ private: Expected checkMultiSign(RequireFullyCanonicalSig requireCanonicalSig) const; + + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; bool diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index d6ae93e23..edd922e7b 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -94,12 +94,6 @@ public: NodeID const& nodeID, F&& f); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - // Hash of the validated ledger uint256 getLedgerHash() const; @@ -150,6 +144,13 @@ public: private: static SOTemplate const& validationFormat(); + + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; template diff --git a/src/ripple/protocol/STVector256.h b/src/ripple/protocol/STVector256.h index ff59b6336..87d65036a 100644 --- a/src/ripple/protocol/STVector256.h +++ b/src/ripple/protocol/STVector256.h @@ -40,12 +40,6 @@ public: STVector256(SField const& n, std::vector const& vector); STVector256(SerialIter& sit, SField const& name); - STBase* - copy(std::size_t n, void* buf) const override; - - STBase* - move(std::size_t n, void* buf) override; - SerializedTypeID getSType() const override; @@ -116,6 +110,14 @@ public: void clear() noexcept; + +private: + STBase* + copy(std::size_t n, void* buf) const override; + STBase* + move(std::size_t n, void* buf) override; + + friend class detail::STVar; }; inline STVector256::STVector256(SField const& n) : STBase(n) From df02eb125f2ac01d329731b3562ee6f693182638 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Thu, 21 Oct 2021 14:31:58 -0700 Subject: [PATCH 03/19] Prefer using a local Sandbox over using the Context's view --- src/ripple/app/tx/impl/CreateOffer.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 624d93b7a..833d18d88 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -637,7 +637,7 @@ CreateOffer::takerCross( Sandbox& sbCancel, Amounts const& takerAmount) { - NetClock::time_point const when = ctx_.view().parentCloseTime(); + NetClock::time_point const when = sb.parentCloseTime(); beast::WrappedSink takerSink(j_, "Taker "); @@ -957,7 +957,7 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) // Expiration is defined in terms of the close time of the parent ledger, // because we definitively know the time that it closed but we do not // know the closing time of the ledger that is under construction. - if (expiration && (ctx_.view().parentCloseTime() >= tp{d{*expiration}})) + if (expiration && (sb.parentCloseTime() >= tp{d{*expiration}})) { // If the offer has expired, the transaction has successfully // done nothing, so short circuit from here. @@ -965,13 +965,12 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) // The return code change is attached to featureDepositPreauth as a // convenience. The change is not big enough to deserve a fix code. TER const ter{ - ctx_.view().rules().enabled(featureDepositPreauth) - ? TER{tecEXPIRED} - : TER{tesSUCCESS}}; + sb.rules().enabled(featureDepositPreauth) ? TER{tecEXPIRED} + : TER{tesSUCCESS}}; return {ter, true}; } - bool const bOpenLedger = ctx_.view().open(); + bool const bOpenLedger = sb.open(); bool crossed = false; if (result == tesSUCCESS) @@ -1129,8 +1128,8 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) return {tefINTERNAL, false}; { - XRPAmount reserve = ctx_.view().fees().accountReserve( - sleCreator->getFieldU32(sfOwnerCount) + 1); + XRPAmount reserve = + sb.fees().accountReserve(sleCreator->getFieldU32(sfOwnerCount) + 1); if (mPriorBalance < reserve) { From c2a08a1f263abcf41e16f147d0c04e680db01dcf Mon Sep 17 00:00:00 2001 From: John Freeman Date: Tue, 16 Nov 2021 10:55:52 -0600 Subject: [PATCH 04/19] Simplify the Job Queue: This is a refactor aimed at cleaning up and simplifying the existing job queue. As of now, all jobs are cancelled at the same time and in the same way, so this commit removes the per-job cancellation token. If the need for such support is demonstrated, support can be re-added. * Revise documentation for ClosureCounter and Workers. * Simplify code, removing unnecessary function arguments and deduplicating expressions * Restructure job handlers to no longer need to pass a job's handle to the job. --- src/ripple/app/consensus/RCLConsensus.cpp | 8 +-- src/ripple/app/consensus/RCLValidations.cpp | 2 +- src/ripple/app/ledger/ConsensusTransSetSF.cpp | 7 +- src/ripple/app/ledger/Ledger.cpp | 7 +- src/ripple/app/ledger/LedgerMaster.h | 4 +- src/ripple/app/ledger/OrderBookDB.cpp | 2 +- src/ripple/app/ledger/impl/InboundLedger.cpp | 2 +- src/ripple/app/ledger/impl/InboundLedgers.cpp | 4 +- .../app/ledger/impl/LedgerDeltaAcquire.cpp | 2 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 21 +++--- src/ripple/app/ledger/impl/TimeoutCounter.cpp | 2 +- .../app/ledger/impl/TransactionAcquire.cpp | 2 +- src/ripple/app/main/Application.cpp | 2 +- src/ripple/app/main/NodeStoreScheduler.cpp | 2 +- src/ripple/app/misc/NetworkOPs.cpp | 24 ++++--- src/ripple/app/paths/PathRequests.cpp | 8 +-- src/ripple/app/paths/PathRequests.h | 5 +- src/ripple/core/ClosureCounter.h | 16 ++--- src/ripple/core/Coro.ipp | 2 +- src/ripple/core/Job.h | 25 +------ src/ripple/core/JobQueue.h | 22 +------ src/ripple/core/impl/Job.cpp | 29 +-------- src/ripple/core/impl/JobQueue.cpp | 65 ++++++++----------- src/ripple/core/impl/SociDB.cpp | 2 +- src/ripple/core/impl/Workers.h | 31 ++++++--- src/ripple/net/impl/RPCSub.cpp | 2 +- src/ripple/overlay/impl/PeerImp.cpp | 28 ++++---- src/ripple/overlay/impl/PeerImp.h | 2 +- src/ripple/rpc/impl/ShardArchiveHandler.cpp | 65 +++++++++---------- src/test/core/Coroutine_test.cpp | 4 +- src/test/core/JobQueue_test.cpp | 9 ++- 31 files changed, 164 insertions(+), 242 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 859c66d31..be8f2af8c 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -131,9 +131,7 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash) acquiringLedger_ = hash; app_.getJobQueue().addJob( - jtADVANCE, - "getConsensusLedger", - [id = hash, &app = app_](Job&) { + jtADVANCE, "getConsensusLedger", [id = hash, &app = app_]() { app.getInboundLedgers().acquire( id, 0, InboundLedger::Reason::CONSENSUS); }); @@ -423,9 +421,7 @@ RCLConsensus::Adaptor::onAccept( Json::Value&& consensusJson) { app_.getJobQueue().addJob( - jtACCEPT, - "acceptLedger", - [=, cj = std::move(consensusJson)](auto&) mutable { + jtACCEPT, "acceptLedger", [=, cj = std::move(consensusJson)]() mutable { // Note that no lock is held or acquired during this job. // This is because generic Consensus guarantees that once a ledger // is accepted, the consensus results and capture by reference state diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index 0fcf0660b..ab9391385 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -135,7 +135,7 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash) Application* pApp = &app_; app_.getJobQueue().addJob( - jtADVANCE, "getConsensusLedger", [pApp, hash](Job&) { + jtADVANCE, "getConsensusLedger", [pApp, hash]() { pApp->getInboundLedgers().acquire( hash, 0, InboundLedger::Reason::CONSENSUS); }); diff --git a/src/ripple/app/ledger/ConsensusTransSetSF.cpp b/src/ripple/app/ledger/ConsensusTransSetSF.cpp index 881424ff1..997a2aee1 100644 --- a/src/ripple/app/ledger/ConsensusTransSetSF.cpp +++ b/src/ripple/app/ledger/ConsensusTransSetSF.cpp @@ -62,10 +62,9 @@ ConsensusTransSetSF::gotNode( auto stx = std::make_shared(std::ref(sit)); assert(stx->getTransactionID() == nodeHash.as_uint256()); auto const pap = &app_; - app_.getJobQueue().addJob( - jtTRANSACTION, "TXS->TXN", [pap, stx](Job&) { - pap->getOPs().submitTransaction(stx); - }); + app_.getJobQueue().addJob(jtTRANSACTION, "TXS->TXN", [pap, stx]() { + pap->getOPs().submitTransaction(stx); + }); } catch (std::exception const&) { diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 4a12a3999..cac9c40bb 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -981,10 +981,9 @@ pendSaveValidated( // See if we can use the JobQueue. if (!isSynchronous && - app.getJobQueue().addJob( - jobType, jobName, [&app, ledger, isCurrent](Job&) { - saveValidatedLedger(app, ledger, isCurrent); - })) + app.getJobQueue().addJob(jobType, jobName, [&app, ledger, isCurrent]() { + saveValidatedLedger(app, ledger, isCurrent); + })) { return true; } diff --git a/src/ripple/app/ledger/LedgerMaster.h b/src/ripple/app/ledger/LedgerMaster.h index eb296f0de..dbb01f54a 100644 --- a/src/ripple/app/ledger/LedgerMaster.h +++ b/src/ripple/app/ledger/LedgerMaster.h @@ -301,7 +301,7 @@ private: setPubLedger(std::shared_ptr const& l); void - tryFill(Job& job, std::shared_ptr ledger); + tryFill(std::shared_ptr ledger); void getFetchPack(LedgerIndex missing, InboundLedger::Reason reason); @@ -326,7 +326,7 @@ private: findNewLedgersToPublish(std::unique_lock&); void - updatePaths(Job& job); + updatePaths(); // Returns true if work started. Always called with m_mutex locked. // The passed lock is a reminder to callers. diff --git a/src/ripple/app/ledger/OrderBookDB.cpp b/src/ripple/app/ledger/OrderBookDB.cpp index 4b598de1b..b9f72b715 100644 --- a/src/ripple/app/ledger/OrderBookDB.cpp +++ b/src/ripple/app/ledger/OrderBookDB.cpp @@ -68,7 +68,7 @@ OrderBookDB::setup(std::shared_ptr const& ledger) update(ledger); else app_.getJobQueue().addJob( - jtUPDATE_PF, "OrderBookDB::update", [this, ledger](Job&) { + jtUPDATE_PF, "OrderBookDB::update", [this, ledger]() { update(ledger); }); } diff --git a/src/ripple/app/ledger/impl/InboundLedger.cpp b/src/ripple/app/ledger/impl/InboundLedger.cpp index f961d28b7..979c14544 100644 --- a/src/ripple/app/ledger/impl/InboundLedger.cpp +++ b/src/ripple/app/ledger/impl/InboundLedger.cpp @@ -527,7 +527,7 @@ InboundLedger::done() // We hold the PeerSet lock, so must dispatch app_.getJobQueue().addJob( - jtLEDGER_DATA, "AcquisitionDone", [self = shared_from_this()](Job&) { + jtLEDGER_DATA, "AcquisitionDone", [self = shared_from_this()]() { if (self->complete_ && !self->failed_) { self->app_.getLedgerMaster().checkAccept(self->getLedger()); diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index 8ee3443a2..76681ea0a 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -183,7 +183,7 @@ public: // dispatch if (ledger->gotData(std::weak_ptr(peer), packet)) app_.getJobQueue().addJob( - jtLEDGER_DATA, "processLedgerData", [ledger](Job&) { + jtLEDGER_DATA, "processLedgerData", [ledger]() { ledger->runData(); }); @@ -198,7 +198,7 @@ public: if (packet->type() == protocol::liAS_NODE) { app_.getJobQueue().addJob( - jtLEDGER_DATA, "gotStaleData", [this, packet](Job&) { + jtLEDGER_DATA, "gotStaleData", [this, packet]() { gotStaleData(packet); }); } diff --git a/src/ripple/app/ledger/impl/LedgerDeltaAcquire.cpp b/src/ripple/app/ledger/impl/LedgerDeltaAcquire.cpp index 122915eed..3c19c6ee1 100644 --- a/src/ripple/app/ledger/impl/LedgerDeltaAcquire.cpp +++ b/src/ripple/app/ledger/impl/LedgerDeltaAcquire.cpp @@ -240,7 +240,7 @@ LedgerDeltaAcquire::onLedgerBuilt( app_.getJobQueue().addJob( jtREPLAY_TASK, "onLedgerBuilt", - [=, ledger = this->fullLedger_, &app = this->app_](Job&) { + [=, ledger = this->fullLedger_, &app = this->app_]() { for (auto reason : reasons) { switch (reason) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 4cc195815..0dd0ba1ee 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -699,7 +699,7 @@ LedgerMaster::getEarliestFetch() } void -LedgerMaster::tryFill(Job& job, std::shared_ptr ledger) +LedgerMaster::tryFill(std::shared_ptr ledger) { std::uint32_t seq = ledger->info().seq; uint256 prevHash = ledger->info().parentHash; @@ -710,7 +710,7 @@ LedgerMaster::tryFill(Job& job, std::shared_ptr ledger) std::uint32_t maxHas = seq; NodeStore::Database& nodeStore{app_.getNodeStore()}; - while (!job.shouldCancel() && seq > 0) + while (!app_.getJobQueue().isStopping() && seq > 0) { { std::lock_guard ml(m_mutex); @@ -1453,7 +1453,7 @@ LedgerMaster::tryAdvance() if (!mAdvanceThread && !mValidLedger.empty()) { mAdvanceThread = true; - app_.getJobQueue().addJob(jtADVANCE, "advanceLedger", [this](Job&) { + app_.getJobQueue().addJob(jtADVANCE, "advanceLedger", [this]() { std::unique_lock sl(m_mutex); assert(!mValidLedger.empty() && mAdvanceThread); @@ -1476,7 +1476,7 @@ LedgerMaster::tryAdvance() } void -LedgerMaster::updatePaths(Job& job) +LedgerMaster::updatePaths() { { std::lock_guard ml(m_mutex); @@ -1487,7 +1487,7 @@ LedgerMaster::updatePaths(Job& job) } } - while (!job.shouldCancel()) + while (!app_.getJobQueue().isStopping()) { std::shared_ptr lastLedger; { @@ -1527,8 +1527,7 @@ LedgerMaster::updatePaths(Job& job) try { - app_.getPathRequests().updateAll( - lastLedger, job.getCancelCallback()); + app_.getPathRequests().updateAll(lastLedger); } catch (SHAMapMissingNode const& mn) { @@ -1591,7 +1590,7 @@ LedgerMaster::newPFWork( if (mPathFindThread < 2) { if (app_.getJobQueue().addJob( - jtUPDATE_PF, name, [this](Job& j) { updatePaths(j); })) + jtUPDATE_PF, name, [this]() { updatePaths(); })) { ++mPathFindThread; } @@ -1942,8 +1941,8 @@ LedgerMaster::fetchForHistory( mFillInProgress = seq; } app_.getJobQueue().addJob( - jtADVANCE, "tryFill", [this, ledger](Job& j) { - tryFill(j, ledger); + jtADVANCE, "tryFill", [this, ledger]() { + tryFill(ledger); }); } } @@ -2124,7 +2123,7 @@ LedgerMaster::gotFetchPack(bool progress, std::uint32_t seq) { if (!mGotFetchPackThread.test_and_set(std::memory_order_acquire)) { - app_.getJobQueue().addJob(jtLEDGER_DATA, "gotFetchPack", [&](Job&) { + app_.getJobQueue().addJob(jtLEDGER_DATA, "gotFetchPack", [&]() { app_.getInboundLedgers().gotFetchPack(); mGotFetchPackThread.clear(std::memory_order_release); }); diff --git a/src/ripple/app/ledger/impl/TimeoutCounter.cpp b/src/ripple/app/ledger/impl/TimeoutCounter.cpp index ff43a0e1a..9ea20c063 100644 --- a/src/ripple/app/ledger/impl/TimeoutCounter.cpp +++ b/src/ripple/app/ledger/impl/TimeoutCounter.cpp @@ -83,7 +83,7 @@ TimeoutCounter::queueJob(ScopedLockType& sl) app_.getJobQueue().addJob( queueJobParameter_.jobType, queueJobParameter_.jobName, - [wptr = pmDowncast()](Job&) { + [wptr = pmDowncast()]() { if (auto sptr = wptr.lock(); sptr) sptr->invokeOnTimer(); }); diff --git a/src/ripple/app/ledger/impl/TransactionAcquire.cpp b/src/ripple/app/ledger/impl/TransactionAcquire.cpp index 3cd4a301e..7d958cba8 100644 --- a/src/ripple/app/ledger/impl/TransactionAcquire.cpp +++ b/src/ripple/app/ledger/impl/TransactionAcquire.cpp @@ -81,7 +81,7 @@ TransactionAcquire::done() // just updates the consensus and related structures when we acquire // a transaction set. No need to update them if we're shutting down. app_.getJobQueue().addJob( - jtTXN_DATA, "completeAcquire", [pap, hash, map](Job&) { + jtTXN_DATA, "completeAcquire", [pap, hash, map]() { pap->getInboundTransactions().giveSet(hash, map, true); }); } diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 8040decea..ac0b6c947 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1085,7 +1085,7 @@ public: if (e.value() == boost::system::errc::success) { m_jobQueue->addJob( - jtSWEEP, "sweep", [this](Job&) { doSweep(); }); + jtSWEEP, "sweep", [this]() { doSweep(); }); } // Recover as best we can if an unexpected error occurs. if (e.value() != boost::system::errc::success && diff --git a/src/ripple/app/main/NodeStoreScheduler.cpp b/src/ripple/app/main/NodeStoreScheduler.cpp index 0e6c14adb..0ac890964 100644 --- a/src/ripple/app/main/NodeStoreScheduler.cpp +++ b/src/ripple/app/main/NodeStoreScheduler.cpp @@ -32,7 +32,7 @@ NodeStoreScheduler::scheduleTask(NodeStore::Task& task) if (jobQueue_.isStopped()) return; - if (!jobQueue_.addJob(jtWRITE, "NodeObject::store", [&task](Job&) { + if (!jobQueue_.addJob(jtWRITE, "NodeObject::store", [&task]() { task.performScheduledTask(); })) { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 574ce040b..2b9c5f316 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -949,7 +949,7 @@ NetworkOPsImp::setHeartbeatTimer() heartbeatTimer_, mConsensus.parms().ledgerGRANULARITY, [this]() { - m_job_queue.addJob(jtNETOP_TIMER, "NetOPs.heartbeat", [this](Job&) { + m_job_queue.addJob(jtNETOP_TIMER, "NetOPs.heartbeat", [this]() { processHeartbeatTimer(); }); }, @@ -964,7 +964,7 @@ NetworkOPsImp::setClusterTimer() clusterTimer_, 10s, [this]() { - m_job_queue.addJob(jtNETOP_CLUSTER, "NetOPs.cluster", [this](Job&) { + m_job_queue.addJob(jtNETOP_CLUSTER, "NetOPs.cluster", [this]() { processClusterTimer(); }); }, @@ -1153,7 +1153,7 @@ NetworkOPsImp::submitTransaction(std::shared_ptr const& iTrans) auto tx = std::make_shared(trans, reason, app_); - m_job_queue.addJob(jtTRANSACTION, "submitTxn", [this, tx](Job&) { + m_job_queue.addJob(jtTRANSACTION, "submitTxn", [this, tx]() { auto t = tx; processTransaction(t, false, false, FailHard::no); }); @@ -1224,9 +1224,8 @@ NetworkOPsImp::doTransactionAsync( if (mDispatchState == DispatchState::none) { - if (m_job_queue.addJob(jtBATCH, "transactionBatch", [this](Job&) { - transactionBatch(); - })) + if (m_job_queue.addJob( + jtBATCH, "transactionBatch", [this]() { transactionBatch(); })) { mDispatchState = DispatchState::scheduled; } @@ -1262,10 +1261,9 @@ NetworkOPsImp::doTransactionSync( if (mTransactions.size()) { // More transactions need to be applied, but by another job. - if (m_job_queue.addJob( - jtBATCH, "transactionBatch", [this](Job&) { - transactionBatch(); - })) + if (m_job_queue.addJob(jtBATCH, "transactionBatch", [this]() { + transactionBatch(); + })) { mDispatchState = DispatchState::scheduled; } @@ -2941,7 +2939,7 @@ NetworkOPsImp::reportFeeChange() if (f != mLastFeeSummary) { m_job_queue.addJob( - jtCLIENT_FEE_CHANGE, "reportFeeChange->pubServer", [this](Job&) { + jtCLIENT_FEE_CHANGE, "reportFeeChange->pubServer", [this]() { pubServer(); }); } @@ -2953,7 +2951,7 @@ NetworkOPsImp::reportConsensusStateChange(ConsensusPhase phase) m_job_queue.addJob( jtCLIENT_CONSENSUS, "reportConsensusStateChange->pubConsensus", - [this, phase](Job&) { pubConsensus(phase); }); + [this, phase]() { pubConsensus(phase); }); } inline void @@ -3346,7 +3344,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) app_.getJobQueue().addJob( jtCLIENT_ACCT_HIST, "AccountHistoryTxStream", - [this, dbType = databaseType, subInfo](Job&) { + [this, dbType = databaseType, subInfo]() { auto const& accountId = subInfo.index_->accountId_; auto& lastLedgerSeq = subInfo.index_->historyLastLedgerSeq_; auto& txHistoryIndex = subInfo.index_->historyTxIndex_; diff --git a/src/ripple/app/paths/PathRequests.cpp b/src/ripple/app/paths/PathRequests.cpp index fe78f360f..50e591eb1 100644 --- a/src/ripple/app/paths/PathRequests.cpp +++ b/src/ripple/app/paths/PathRequests.cpp @@ -55,9 +55,7 @@ PathRequests::getLineCache( } void -PathRequests::updateAll( - std::shared_ptr const& inLedger, - Job::CancelCallback shouldCancel) +PathRequests::updateAll(std::shared_ptr const& inLedger) { auto event = app_.getJobQueue().makeLoadEvent(jtPATH_FIND, "PathRequest::updateAll"); @@ -84,7 +82,7 @@ PathRequests::updateAll( { for (auto const& wr : requests) { - if (shouldCancel()) + if (app_.getJobQueue().isStopping()) break; auto request = wr.lock(); @@ -174,7 +172,7 @@ PathRequests::updateAll( requests = requests_; cache = getLineCache(cache->getLedger(), false); } - } while (!shouldCancel()); + } while (!app_.getJobQueue().isStopping()); JLOG(mJournal.debug()) << "updateAll complete: " << processed << " processed and " << removed << " removed"; diff --git a/src/ripple/app/paths/PathRequests.h b/src/ripple/app/paths/PathRequests.h index d95ab9c5d..75b852867 100644 --- a/src/ripple/app/paths/PathRequests.h +++ b/src/ripple/app/paths/PathRequests.h @@ -47,12 +47,9 @@ public: /** Update all of the contained PathRequest instances. @param ledger Ledger we are pathfinding in. - @param shouldCancel Invocable that returns whether to cancel. */ void - updateAll( - std::shared_ptr const& ledger, - Job::CancelCallback shouldCancel); + updateAll(std::shared_ptr const& ledger); std::shared_ptr getLineCache( diff --git a/src/ripple/core/ClosureCounter.h b/src/ripple/core/ClosureCounter.h index 7c0d36d9f..9f5bc3ee6 100644 --- a/src/ripple/core/ClosureCounter.h +++ b/src/ripple/core/ClosureCounter.h @@ -31,21 +31,21 @@ namespace ripple { /** * The role of a `ClosureCounter` is to assist in shutdown by letting callers - * wait for the completion of callbacks (of a single type signature) that they - * previously scheduled. The lifetime of a `ClosureCounter` consists of two + * wait for the completion of closures (of a specific type signature) that they + * previously registered. These closures are typically callbacks for + * asynchronous operations. The lifetime of a `ClosureCounter` consists of two * phases: the initial expanding "fork" phase, and the subsequent shrinking * "join" phase. * - * In the fork phase, callers register a callback by passing the callback and + * In the fork phase, callers register a closure by passing the closure and * receiving a substitute in return. The substitute has the same callable - * interface as the callback, and it informs the `ClosureCounter` whenever it + * interface as the closure, and it informs the `ClosureCounter` whenever it * is copied or destroyed, so that it can keep an accurate count of copies. * * The transition to the join phase is made by a call to `join`. In this - * phase, every substitute returned going forward will be empty, signaling to - * the caller that they should just drop the callback and cancel their - * asynchronous operation. `join` blocks until all existing callback - * substitutes are destroyed. + * phase, every substitute returned going forward will be null, signaling to + * the caller that they should drop the closure and cancel their operation. + * `join` blocks until all existing closure substitutes are destroyed. * * \tparam Ret_t The return type of the closure. * \tparam Args_t The argument types of the closure. diff --git a/src/ripple/core/Coro.ipp b/src/ripple/core/Coro.ipp index 0234f9092..2b07c5a45 100644 --- a/src/ripple/core/Coro.ipp +++ b/src/ripple/core/Coro.ipp @@ -77,7 +77,7 @@ JobQueue::Coro::post() // sp keeps 'this' alive if (jq_.addJob( - type_, name_, [this, sp = shared_from_this()](Job&) { resume(); })) + type_, name_, [this, sp = shared_from_this()]() { resume(); })) { return true; } diff --git a/src/ripple/core/Job.h b/src/ripple/core/Job.h index 66aa0d051..0f3bb718b 100644 --- a/src/ripple/core/Job.h +++ b/src/ripple/core/Job.h @@ -109,43 +109,25 @@ public: // Job(); - // Job (Job const& other); - Job(JobType type, std::uint64_t index); - /** A callback used to check for canceling a job. */ - using CancelCallback = std::function; - // VFALCO TODO try to remove the dependency on LoadMonitor. Job(JobType type, std::string const& name, std::uint64_t index, LoadMonitor& lm, - std::function const& job, - CancelCallback cancelCallback); - - // Job& operator= (Job const& other); + std::function const& job); JobType getType() const; - CancelCallback - getCancelCallback() const; - /** Returns the time when the job was queued. */ clock_type::time_point const& queue_time() const; - /** Returns `true` if the running job should make a best-effort cancel. */ - bool - shouldCancel() const; - void doJob(); - void - rename(std::string const& n); - // These comparison operators make the jobs sort in priority order // in the job set bool @@ -158,16 +140,15 @@ public: operator>=(const Job& j) const; private: - CancelCallback m_cancelCallback; JobType mType; std::uint64_t mJobIndex; - std::function mJob; + std::function mJob; std::shared_ptr m_loadEvent; std::string mName; clock_type::time_point m_queue_time; }; -using JobCounter = ClosureCounter; +using JobCounter = ClosureCounter; } // namespace ripple diff --git a/src/ripple/core/JobQueue.h b/src/ripple/core/JobQueue.h index a93d68b85..a9d541baa 100644 --- a/src/ripple/core/JobQueue.h +++ b/src/ripple/core/JobQueue.h @@ -138,7 +138,7 @@ public: join(); }; - using JobFunction = std::function; + using JobFunction = std::function; JobQueue( int threadCount, @@ -160,7 +160,7 @@ public: template < typename JobHandler, typename = std::enable_if_t()(std::declval())), + decltype(std::declval()()), void>::value>> bool addJob(JobType type, std::string const& name, JobHandler&& jobHandler) @@ -259,7 +259,6 @@ private: int nSuspend_ = 0; Workers m_workers; - Job::CancelCallback m_cancelCallback; // Statistics tracking perf::PerfLog& perfLog_; @@ -288,23 +287,6 @@ private: std::string const& name, JobFunction const& func); - // Signals an added Job for processing. - // - // Pre-conditions: - // The JobType must be valid. - // The Job must exist in mJobSet. - // The Job must not have previously been queued. - // - // Post-conditions: - // Count of waiting jobs of that type will be incremented. - // If JobQueue exists, and has at least one thread, Job will eventually - // run. - // - // Invariants: - // The calling thread owns the JobLock - void - queueJob(Job const& job, std::lock_guard const& lock); - // Returns the next Job we should run now. // // RunnableJob: diff --git a/src/ripple/core/impl/Job.cpp b/src/ripple/core/impl/Job.cpp index a9b82ccf3..780a9f49c 100644 --- a/src/ripple/core/impl/Job.cpp +++ b/src/ripple/core/impl/Job.cpp @@ -36,10 +36,8 @@ Job::Job( std::string const& name, std::uint64_t index, LoadMonitor& lm, - std::function const& job, - CancelCallback cancelCallback) - : m_cancelCallback(cancelCallback) - , mType(type) + std::function const& job) + : mType(type) , mJobIndex(index) , mJob(job) , mName(name) @@ -54,27 +52,12 @@ Job::getType() const return mType; } -Job::CancelCallback -Job::getCancelCallback() const -{ - assert(m_cancelCallback); - return m_cancelCallback; -} - Job::clock_type::time_point const& Job::queue_time() const { return m_queue_time; } -bool -Job::shouldCancel() const -{ - if (m_cancelCallback) - return m_cancelCallback(); - return false; -} - void Job::doJob() { @@ -82,19 +65,13 @@ Job::doJob() m_loadEvent->start(); m_loadEvent->setName(mName); - mJob(*this); + mJob(); // Destroy the lambda, otherwise we won't include // its duration in the time measurement mJob = nullptr; } -void -Job::rename(std::string const& newName) -{ - mName = newName; -} - bool Job::operator>(const Job& j) const { diff --git a/src/ripple/core/impl/JobQueue.cpp b/src/ripple/core/impl/JobQueue.cpp index 85dcec1ca..28947300c 100644 --- a/src/ripple/core/impl/JobQueue.cpp +++ b/src/ripple/core/impl/JobQueue.cpp @@ -35,7 +35,6 @@ JobQueue::JobQueue( , m_invalidJobData(JobTypes::instance().getInvalid(), collector, logs) , m_processCount(0) , m_workers(*this, &perfLog, "JobQueue", threadCount) - , m_cancelCallback(std::bind(&JobQueue::isStopping, this)) , perfLog_(perfLog) , m_collector(collector) { @@ -100,9 +99,27 @@ JobQueue::addRefCountedJob( { std::lock_guard lock(m_mutex); - auto result = m_jobSet.emplace( - type, name, ++m_lastJob, data.load(), func, m_cancelCallback); - queueJob(*result.first, lock); + auto result = + m_jobSet.emplace(type, name, ++m_lastJob, data.load(), func); + auto const& job = *result.first; + + JobType const type(job.getType()); + assert(type != jtINVALID); + assert(m_jobSet.find(job) != m_jobSet.end()); + perfLog_.jobQueue(type); + + JobTypeData& data(getJobTypeData(type)); + + if (data.waiting + data.running < getJobLimit(type)) + { + m_workers.addTask(); + } + else + { + // defer the task until we go below the limit + ++data.deferred; + } + ++data.waiting; } return true; } @@ -282,29 +299,6 @@ JobQueue::isStopped() const return stopped_; } -void -JobQueue::queueJob(Job const& job, std::lock_guard const& lock) -{ - JobType const type(job.getType()); - assert(type != jtINVALID); - assert(m_jobSet.find(job) != m_jobSet.end()); - perfLog_.jobQueue(type); - - JobTypeData& data(getJobTypeData(type)); - - if (data.waiting + data.running < getJobLimit(type)) - { - m_workers.addTask(); - } - else - { - // defer the task until we go below the limit - // - ++data.deferred; - } - ++data.waiting; -} - void JobQueue::getNextJob(Job& job) { @@ -313,30 +307,25 @@ JobQueue::getNextJob(Job& job) std::set::const_iterator iter; for (iter = m_jobSet.begin(); iter != m_jobSet.end(); ++iter) { - JobTypeData& data(getJobTypeData(iter->getType())); + JobType const type = iter->getType(); + assert(type != jtINVALID); - assert(data.running <= getJobLimit(data.type())); + JobTypeData& data(getJobTypeData(type)); + assert(data.running <= getJobLimit(type)); // Run this job if we're running below the limit. if (data.running < getJobLimit(data.type())) { assert(data.waiting > 0); + --data.waiting; + ++data.running; break; } } assert(iter != m_jobSet.end()); - - JobType const type = iter->getType(); - JobTypeData& data(getJobTypeData(type)); - - assert(type != jtINVALID); - job = *iter; m_jobSet.erase(iter); - - --data.waiting; - ++data.running; } void diff --git a/src/ripple/core/impl/SociDB.cpp b/src/ripple/core/impl/SociDB.cpp index 419784c12..2d1332636 100644 --- a/src/ripple/core/impl/SociDB.cpp +++ b/src/ripple/core/impl/SociDB.cpp @@ -254,7 +254,7 @@ public: // There is a separate check in `checkpoint` for a valid // connection in the rare case when the DatabaseCon is destroyed // after locking this weak_ptr - [wp = std::weak_ptr{shared_from_this()}](Job&) { + [wp = std::weak_ptr{shared_from_this()}]() { if (auto self = wp.lock()) self->checkpoint(); })) diff --git a/src/ripple/core/impl/Workers.h b/src/ripple/core/impl/Workers.h index 0abbbe429..88ab8fa27 100644 --- a/src/ripple/core/impl/Workers.h +++ b/src/ripple/core/impl/Workers.h @@ -37,24 +37,35 @@ class PerfLog; /** * `Workers` is effectively a thread pool. The constructor takes a "callback" * that has a `void processTask(int instance)` method, and a number of - * workers. It creates that many Workers and then waits for calls to + * workers. It creates that many `Worker`s and then waits for calls to * `Workers::addTask()`. It holds a semaphore that counts the number of - * waiting tasks, and a condition variable for the event when the last worker - * pauses itself. + * pending "tasks", and a condition variable for the event when the last + * worker pauses itself. + * + * A "task" is just a call to the callback's `processTask` method. + * "Adding a task" means calling that method now, or remembering to call it in + * the future. + * This is implemented with a semaphore. + * If there are any workers waiting when a task is added, then one will be + * woken to claim the task. + * If not, then the next worker to wait on the semaphore will claim the task. * * Creating a `Worker` creates a thread that calls `Worker::run()`. When that * thread enters `Worker::run`, it increments the count of active workers in - * the parent `Workers` object and then blocks on the semaphore if there are - * no waiting tasks. It will be unblocked whenever the number of waiting tasks - * is incremented. That only happens in two circumstances: (1) when + * the parent `Workers` object and then tries to claim a task, which blocks if + * there are none pending. + * It will be unblocked whenever the semaphore is notified (i.e. when the + * number of pending tasks is incremented). + * That only happens in two circumstances: (1) when * `Workers::addTask` is called and (2) when `Workers` wants to pause some * workers ("pause one worker" is considered one task), which happens when * someone wants to stop the workers or shrink the threadpool. No worker * threads are ever destroyed until `Workers` is destroyed; it merely pauses * workers until then. * - * When an idle worker is woken, it checks whether `Workers` is trying to pause - * workers. If so, it adds itself to the set of paused workers and blocks on + * When a waiting worker is woken, it checks whether `Workers` is trying to + * pause workers. If so, it changes its status from active to paused and + * blocks on * its own condition variable. If not, then it calls `processTask` on the * "callback" held by `Workers`. * @@ -62,8 +73,8 @@ class PerfLog; * to exit is only set in the destructor of `Worker`, which unblocks the * paused thread and waits for it to exit. A `Worker::run` thread checks * whether it needs to exit only when it is woken from a pause (not when it is - * woken from idle). This is why the destructor for `Workers` pauses all the - * workers before destroying them. + * woken from waiting). This is why the destructor for `Workers` pauses all + * the workers before destroying them. */ class Workers { diff --git a/src/ripple/net/impl/RPCSub.cpp b/src/ripple/net/impl/RPCSub.cpp index f65f9a361..8b052e817 100644 --- a/src/ripple/net/impl/RPCSub.cpp +++ b/src/ripple/net/impl/RPCSub.cpp @@ -96,7 +96,7 @@ public: JLOG(j_.info()) << "RPCCall::fromNetwork start"; mSending = m_jobQueue.addJob( - jtCLIENT_SUBSCRIBE, "RPCSub::sendThread", [this](Job&) { + jtCLIENT_SUBSCRIBE, "RPCSub::sendThread", [this]() { sendThread(); }); } diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 7a1510996..6f0532821 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1070,7 +1070,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // VFALCO What's the right job type? auto that = shared_from_this(); app_.getJobQueue().addJob( - jtVALIDATION_ut, "receiveManifests", [this, that, m](Job&) { + jtVALIDATION_ut, "receiveManifests", [this, that, m]() { overlay_.onManifests(m, that); }); } @@ -1608,7 +1608,7 @@ PeerImp::handleTransaction( [weak = std::weak_ptr(shared_from_this()), flags, checkSignature, - stx](Job&) { + stx]() { if (auto peer = weak.lock()) peer->checkTransaction(flags, checkSignature, stx); }); @@ -1711,7 +1711,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // Queue a job to process the request std::weak_ptr weak = shared_from_this(); - app_.getJobQueue().addJob(jtLEDGER_REQ, "recvGetLedger", [weak, m](Job&) { + app_.getJobQueue().addJob(jtLEDGER_REQ, "recvGetLedger", [weak, m]() { if (auto peer = weak.lock()) peer->processLedgerRequest(m); }); @@ -1730,7 +1730,7 @@ PeerImp::onMessage(std::shared_ptr const& m) fee_ = Resource::feeMediumBurdenPeer; std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( - jtREPLAY_REQ, "recvProofPathRequest", [weak, m](Job&) { + jtREPLAY_REQ, "recvProofPathRequest", [weak, m]() { if (auto peer = weak.lock()) { auto reply = @@ -1779,7 +1779,7 @@ PeerImp::onMessage(std::shared_ptr const& m) fee_ = Resource::feeMediumBurdenPeer; std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( - jtREPLAY_REQ, "recvReplayDeltaRequest", [weak, m](Job&) { + jtREPLAY_REQ, "recvReplayDeltaRequest", [weak, m]() { if (auto peer = weak.lock()) { auto reply = @@ -1900,7 +1900,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { std::weak_ptr weak{shared_from_this()}; app_.getJobQueue().addJob( - jtTXN_DATA, "recvPeerData", [weak, ledgerHash, m](Job&) { + jtTXN_DATA, "recvPeerData", [weak, ledgerHash, m]() { if (auto peer = weak.lock()) { peer->app_.getInboundTransactions().gotData( @@ -2013,9 +2013,9 @@ PeerImp::onMessage(std::shared_ptr const& m) app_.getJobQueue().addJob( isTrusted ? jtPROPOSAL_t : jtPROPOSAL_ut, "recvPropose->checkPropose", - [weak, m, proposal](Job& job) { + [weak, isTrusted, m, proposal]() { if (auto peer = weak.lock()) - peer->checkPropose(job, m, proposal); + peer->checkPropose(isTrusted, m, proposal); }); } @@ -2602,7 +2602,7 @@ PeerImp::onMessage(std::shared_ptr const& m) app_.getJobQueue().addJob( isTrusted ? jtVALIDATION_t : jtVALIDATION_ut, "recvValidation->checkValidation", - [weak, val, m](Job&) { + [weak, val, m]() { if (auto peer = weak.lock()) peer->checkValidation(val, m); }); @@ -2655,7 +2655,7 @@ PeerImp::onMessage(std::shared_ptr const& m) std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( - jtREQUESTED_TXN, "doTransactions", [weak, m](Job&) { + jtREQUESTED_TXN, "doTransactions", [weak, m]() { if (auto peer = weak.lock()) peer->doTransactions(m); }); @@ -2795,7 +2795,7 @@ PeerImp::onMessage(std::shared_ptr const& m) std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( - jtMISSING_TXN, "handleHaveTransactions", [weak, m](Job&) { + jtMISSING_TXN, "handleHaveTransactions", [weak, m]() { if (auto peer = weak.lock()) peer->handleHaveTransactions(m); }); @@ -2975,7 +2975,7 @@ PeerImp::doFetchPack(const std::shared_ptr& packet) auto elapsed = UptimeClock::now(); auto const pap = &app_; app_.getJobQueue().addJob( - jtPACK, "MakeFetchPack", [pap, weak, packet, hash, elapsed](Job&) { + jtPACK, "MakeFetchPack", [pap, weak, packet, hash, elapsed]() { pap->getLedgerMaster().makeFetchPack(weak, packet, hash, elapsed); }); } @@ -3111,12 +3111,10 @@ PeerImp::checkTransaction( // Called from our JobQueue void PeerImp::checkPropose( - Job& job, + bool isTrusted, std::shared_ptr const& packet, RCLCxPeerPos peerPos) { - bool isTrusted = (job.getType() == jtPROPOSAL_t); - JLOG(p_journal_.trace()) << "Checking " << (isTrusted ? "trusted" : "UNTRUSTED") << " proposal"; diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index f7bfbbc9d..8bed64e72 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -619,7 +619,7 @@ private: void checkPropose( - Job& job, + bool isTrusted, std::shared_ptr const& packet, RCLCxPeerPos peerPos); diff --git a/src/ripple/rpc/impl/ShardArchiveHandler.cpp b/src/ripple/rpc/impl/ShardArchiveHandler.cpp index c52c1b501..efa422c8b 100644 --- a/src/ripple/rpc/impl/ShardArchiveHandler.cpp +++ b/src/ripple/rpc/impl/ShardArchiveHandler.cpp @@ -360,7 +360,7 @@ ShardArchiveHandler::next(std::lock_guard const& l) // to prevent holding up the lock if the downloader // sleeps. auto const& url{archives_.begin()->second}; - auto wrapper = jobCounter_.wrap([this, url, dstDir](Job&) { + auto wrapper = jobCounter_.wrap([this, url, dstDir]() { auto const ssl = (url.scheme == "https"); auto const defaultPort = ssl ? 443 : 80; @@ -416,42 +416,41 @@ ShardArchiveHandler::complete(path dstPath) } // Make lambdas mutable captured vars can be moved from - auto wrapper = - jobCounter_.wrap([=, dstPath = std::move(dstPath)](Job&) mutable { - if (stopping_) - return; + auto wrapper = jobCounter_.wrap([=, + dstPath = std::move(dstPath)]() mutable { + if (stopping_) + return; - // If not synced then defer and retry - auto const mode{app_.getOPs().getOperatingMode()}; - if (mode != OperatingMode::FULL) - { - std::lock_guard lock(m_); - timer_.expires_from_now(static_cast( - (static_cast(OperatingMode::FULL) - - static_cast(mode)) * - 10)); + // If not synced then defer and retry + auto const mode{app_.getOPs().getOperatingMode()}; + if (mode != OperatingMode::FULL) + { + std::lock_guard lock(m_); + timer_.expires_from_now(static_cast( + (static_cast(OperatingMode::FULL) - + static_cast(mode)) * + 10)); - auto wrapper = timerCounter_.wrap( - [=, dstPath = std::move(dstPath)]( - boost::system::error_code const& ec) mutable { - if (ec != boost::asio::error::operation_aborted) - complete(std::move(dstPath)); - }); + auto wrapper = timerCounter_.wrap( + [=, dstPath = std::move(dstPath)]( + boost::system::error_code const& ec) mutable { + if (ec != boost::asio::error::operation_aborted) + complete(std::move(dstPath)); + }); - if (!wrapper) - onClosureFailed( - "failed to wrap closure for operating mode timer", - lock); - else - timer_.async_wait(*wrapper); - } + if (!wrapper) + onClosureFailed( + "failed to wrap closure for operating mode timer", lock); else - { - process(dstPath); - std::lock_guard lock(m_); - removeAndProceed(lock); - } - }); + timer_.async_wait(*wrapper); + } + else + { + process(dstPath); + std::lock_guard lock(m_); + removeAndProceed(lock); + } + }); if (!wrapper) { diff --git a/src/test/core/Coroutine_test.cpp b/src/test/core/Coroutine_test.cpp index 944602784..8937255a7 100644 --- a/src/test/core/Coroutine_test.cpp +++ b/src/test/core/Coroutine_test.cpp @@ -127,7 +127,7 @@ public: BEAST_EXPECT(*lv == -1); gate g; - jq.addJob(jtCLIENT, "LocalValue-Test", [&](auto const& job) { + jq.addJob(jtCLIENT, "LocalValue-Test", [&]() { this->BEAST_EXPECT(*lv == -1); *lv = -2; this->BEAST_EXPECT(*lv == -2); @@ -166,7 +166,7 @@ public: c->join(); } - jq.addJob(jtCLIENT, "LocalValue-Test", [&](auto const& job) { + jq.addJob(jtCLIENT, "LocalValue-Test", [&]() { this->BEAST_EXPECT(*lv == -2); g.signal(); }); diff --git a/src/test/core/JobQueue_test.cpp b/src/test/core/JobQueue_test.cpp index d0c698099..cba021767 100644 --- a/src/test/core/JobQueue_test.cpp +++ b/src/test/core/JobQueue_test.cpp @@ -37,10 +37,9 @@ class JobQueue_test : public beast::unit_test::suite { // addJob() should run the Job (and return true). std::atomic jobRan{false}; - BEAST_EXPECT( - jQueue.addJob(jtCLIENT, "JobAddTest1", [&jobRan](Job&) { - jobRan = true; - }) == true); + BEAST_EXPECT(jQueue.addJob(jtCLIENT, "JobAddTest1", [&jobRan]() { + jobRan = true; + }) == true); // Wait for the Job to run. while (jobRan == false) @@ -58,7 +57,7 @@ class JobQueue_test : public beast::unit_test::suite // Not recommended for the faint of heart... bool unprotected; BEAST_EXPECT( - jQueue.addJob(jtCLIENT, "JobAddTest2", [&unprotected](Job&) { + jQueue.addJob(jtCLIENT, "JobAddTest2", [&unprotected]() { unprotected = false; }) == false); } From 06e87e0f6add5b880d647e14ab3d950decfcf416 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 28 Dec 2021 15:34:03 -0800 Subject: [PATCH 05/19] Fix deletion of orphan nodestore directories: Orphaned nodestore directories should only be deleted if the proper nodestore directories are confirmed to exist. --- src/ripple/app/misc/SHAMapStoreImp.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index 3930e04bf..56a817934 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -485,6 +485,7 @@ SHAMapStoreImp::dbPaths() bool writableDbExists = false; bool archiveDbExists = false; + std::vector pathsToDelete; for (boost::filesystem::directory_iterator it(dbPath); it != boost::filesystem::directory_iterator(); ++it) @@ -494,7 +495,7 @@ SHAMapStoreImp::dbPaths() else if (!state.archiveDb.compare(it->path().string())) archiveDbExists = true; else if (!dbPrefix_.compare(it->path().stem().string())) - boost::filesystem::remove_all(it->path()); + pathsToDelete.push_back(it->path()); } if ((!writableDbExists && state.writableDb.size()) || @@ -524,6 +525,10 @@ SHAMapStoreImp::dbPaths() Throw("state db error"); } + + // The necessary directories exist. Now, remove any others. + for (boost::filesystem::path& p : pathsToDelete) + boost::filesystem::remove_all(p); } std::unique_ptr From dc213a4fab213924ebce8163db7ef5f530d621a1 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Fri, 14 Jan 2022 15:13:09 -0800 Subject: [PATCH 06/19] Make gateway_balances admin-only in reporting mode --- src/ripple/rpc/impl/Handler.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ripple/rpc/impl/Handler.cpp b/src/ripple/rpc/impl/Handler.cpp index 7c193f554..3613bf723 100644 --- a/src/ripple/rpc/impl/Handler.cpp +++ b/src/ripple/rpc/impl/Handler.cpp @@ -82,7 +82,11 @@ Handler const handlerArray[]{ Role::USER, NO_CONDITION}, {"download_shard", byRef(&doDownloadShard), Role::ADMIN, NO_CONDITION}, +#ifdef RIPPLED_REPORTING + {"gateway_balances", byRef(&doGatewayBalances), Role::ADMIN, NO_CONDITION}, +#else {"gateway_balances", byRef(&doGatewayBalances), Role::USER, NO_CONDITION}, +#endif {"get_counts", byRef(&doGetCounts), Role::ADMIN, NO_CONDITION}, {"feature", byRef(&doFeature), Role::ADMIN, NO_CONDITION}, {"fee", byRef(&doFee), Role::USER, NEEDS_CURRENT_LEDGER}, From 8f82b62e0d19f77cfbcbd034443e63d77e8a5460 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Wed, 26 Jan 2022 19:08:25 -0800 Subject: [PATCH 07/19] Use CIDR notation for `admin` and `secure_gateway` --- cfg/rippled-example.cfg | 25 +++- src/ripple/rpc/Role.h | 6 +- src/ripple/rpc/impl/Role.cpp | 50 +++++-- src/ripple/rpc/impl/ServerHandlerImp.cpp | 8 +- src/ripple/rpc/impl/WSInfoSub.h | 3 +- src/ripple/server/Port.h | 15 +- src/ripple/server/impl/Port.cpp | 167 +++++++++++++++-------- src/test/core/Config_test.cpp | 4 +- src/test/jtx/envconfig.h | 4 + src/test/jtx/impl/envconfig.cpp | 18 +++ src/test/rpc/Roles_test.cpp | 24 ++++ 11 files changed, 241 insertions(+), 83 deletions(-) diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index b9407d0da..f87ad44c7 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -200,9 +200,19 @@ # # admin = [ IP, IP, IP, ... ] # -# A comma-separated list of IP addresses. +# A comma-separated list of IP addresses or subnets. Subnets +# should be represented in "slash" notation, such as: +# 10.0.0.0/8 +# 172.16.0.0/12 +# 192.168.0.0/16 +# Those examples are ipv4, but ipv6 is also supported. +# When configuring subnets, the address must match the +# underlying network address. Otherwise, the desired IP range is +# ambiguous. For example, 10.1.2.3/24 has a network address of +# 10.1.2.0. Therefore, that subnet should be configured as +# 10.1.2.0/24. # -# When set, grants administrative command access to the specified IP +# When set, grants administrative command access to the specified # addresses. These commands may be issued over http, https, ws, or wss # if configured on the port. If not provided, the default is to not allow # administrative commands. @@ -233,9 +243,10 @@ # # secure_gateway = [ IP, IP, IP, ... ] # -# A comma-separated list of IP addresses. +# A comma-separated list of IP addresses or subnets. See the +# details for the "admin" option above. # -# When set, allows the specified IP addresses to pass HTTP headers +# When set, allows the specified addresses to pass HTTP headers # containing username and remote IP address for each session. If a # non-empty username is passed in this way, then resource controls # such as often resulting in "tooBusy" errors will be lifted. However, @@ -250,9 +261,9 @@ # proxies. Since rippled trusts these hosts, they must be # responsible for properly authenticating the remote user. # -# The same IP address cannot be used in both "admin" and "secure_gateway" -# lists for the same port. In this case, rippled will abort with an error -# message to the console shortly after startup +# If some IP addresses are included for both "admin" and +# "secure_gateway" connections, then they will be treated as +# "admin" addresses. # # ssl_key = # ssl_cert = diff --git a/src/ripple/rpc/Role.h b/src/ripple/rpc/Role.h index 1e71b7ecf..c4f1f730c 100644 --- a/src/ripple/rpc/Role.h +++ b/src/ripple/rpc/Role.h @@ -25,8 +25,11 @@ #include #include #include +#include +#include #include #include +#include #include namespace ripple { @@ -79,7 +82,8 @@ isUnlimited(Role const& role); bool ipAllowed( beast::IP::Address const& remoteIp, - std::vector const& adminIp); + std::vector const& nets4, + std::vector const& nets6); boost::string_view forwardedFor(http_request_type const& request); diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 3bfc7d56a..1807ecc20 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -23,13 +23,14 @@ #include #include #include +#include namespace ripple { bool passwordUnrequiredOrSentCorrect(Port const& port, Json::Value const& params) { - assert(!port.admin_ip.empty()); + assert(!(port.admin_nets_v4.empty() && port.admin_nets_v6.empty())); bool const passwordRequired = (!port.admin_user.empty() || !port.admin_password.empty()); @@ -43,14 +44,40 @@ passwordUnrequiredOrSentCorrect(Port const& port, Json::Value const& params) bool ipAllowed( beast::IP::Address const& remoteIp, - std::vector const& adminIp) + std::vector const& nets4, + std::vector const& nets6) { - return std::find_if( - adminIp.begin(), - adminIp.end(), - [&remoteIp](beast::IP::Address const& ip) { - return ip.is_unspecified() || ip == remoteIp; - }) != adminIp.end(); + // To test whether the remoteIP is part of one of the configured + // subnets, first convert it to a subnet definition. For ipv4, + // this means appending /32. For ipv6, /128. Then based on protocol + // check for whether the resulting network is either a subnet of or + // equal to each configured subnet, based on boost::asio's reasoning. + // For example, 10.1.2.3 is a subnet of 10.1.2.0/24, but 10.1.2.0 is + // not. However, 10.1.2.0 is equal to the network portion of 10.1.2.0/24. + + std::string addrString = remoteIp.to_string(); + if (remoteIp.is_v4()) + { + addrString += "/32"; + auto ipNet = boost::asio::ip::make_network_v4(addrString); + for (auto const& net : nets4) + { + if (ipNet.is_subnet_of(net) || ipNet == net) + return true; + } + } + else + { + addrString += "/128"; + auto ipNet = boost::asio::ip::make_network_v6(addrString); + for (auto const& net : nets6) + { + if (ipNet.is_subnet_of(net) || ipNet == net) + return true; + } + } + + return false; } bool @@ -59,7 +86,7 @@ isAdmin( Json::Value const& params, beast::IP::Address const& remoteIp) { - return ipAllowed(remoteIp, port.admin_ip) && + return ipAllowed(remoteIp, port.admin_nets_v4, port.admin_nets_v6) && passwordUnrequiredOrSentCorrect(port, params); } @@ -77,7 +104,10 @@ requestRole( if (required == Role::ADMIN) return Role::FORBID; - if (ipAllowed(remoteIp.address(), port.secure_gateway_ip)) + if (ipAllowed( + remoteIp.address(), + port.secure_gateway_nets_v4, + port.secure_gateway_nets_v6)) { if (user.size()) return Role::IDENTIFIED; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 3aed45382..3ac5f04a9 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -1060,10 +1060,6 @@ to_Port(ParsedPort const& parsed, std::ostream& log) Throw(); } p.port = *parsed.port; - if (parsed.admin_ip) - p.admin_ip = *parsed.admin_ip; - if (parsed.secure_gateway_ip) - p.secure_gateway_ip = *parsed.secure_gateway_ip; if (parsed.protocol.empty()) { @@ -1083,6 +1079,10 @@ to_Port(ParsedPort const& parsed, std::ostream& log) p.pmd_options = parsed.pmd_options; p.ws_queue_limit = parsed.ws_queue_limit; p.limit = parsed.limit; + p.admin_nets_v4 = parsed.admin_nets_v4; + p.admin_nets_v6 = parsed.admin_nets_v6; + p.secure_gateway_nets_v4 = parsed.secure_gateway_nets_v4; + p.secure_gateway_nets_v6 = parsed.secure_gateway_nets_v6; return p; } diff --git a/src/ripple/rpc/impl/WSInfoSub.h b/src/ripple/rpc/impl/WSInfoSub.h index 4bc80d575..8f386c8eb 100644 --- a/src/ripple/rpc/impl/WSInfoSub.h +++ b/src/ripple/rpc/impl/WSInfoSub.h @@ -45,7 +45,8 @@ public: if (ipAllowed( beast::IPAddressConversion::from_asio(ws->remote_endpoint()) .address(), - ws->port().secure_gateway_ip)) + ws->port().secure_gateway_nets_v4, + ws->port().secure_gateway_nets_v6)) { auto it = h.find("X-User"); if (it != h.end()) diff --git a/src/ripple/server/Port.h b/src/ripple/server/Port.h index ae260f679..9dccfdf9c 100644 --- a/src/ripple/server/Port.h +++ b/src/ripple/server/Port.h @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include #include @@ -30,6 +32,7 @@ #include #include #include +#include namespace boost { namespace asio { @@ -50,8 +53,10 @@ struct Port boost::asio::ip::address ip; std::uint16_t port = 0; std::set protocol; - std::vector admin_ip; - std::vector secure_gateway_ip; + std::vector admin_nets_v4; + std::vector admin_nets_v6; + std::vector secure_gateway_nets_v4; + std::vector secure_gateway_nets_v6; std::string user; std::string password; std::string admin_user; @@ -108,8 +113,10 @@ struct ParsedPort std::optional ip; std::optional port; - std::optional> admin_ip; - std::optional> secure_gateway_ip; + std::vector admin_nets_v4; + std::vector admin_nets_v6; + std::vector secure_gateway_nets_v4; + std::vector secure_gateway_nets_v6; }; void diff --git a/src/ripple/server/impl/Port.cpp b/src/ripple/server/impl/Port.cpp index 5691e7bd0..25d86025b 100644 --- a/src/ripple/server/impl/Port.cpp +++ b/src/ripple/server/impl/Port.cpp @@ -21,8 +21,9 @@ #include #include #include - #include +#include +#include namespace ripple { @@ -47,18 +48,34 @@ operator<<(std::ostream& os, Port const& p) { os << "'" << p.name << "' (ip=" << p.ip << ":" << p.port << ", "; - if (!p.admin_ip.empty()) + if (p.admin_nets_v4.size() || p.admin_nets_v6.size()) { - os << "admin IPs:"; - for (auto const& ip : p.admin_ip) - os << ip.to_string() << ", "; + os << "admin nets:"; + for (auto const& net : p.admin_nets_v4) + { + os << net.to_string(); + os << ", "; + } + for (auto const& net : p.admin_nets_v6) + { + os << net.to_string(); + os << ", "; + } } - if (!p.secure_gateway_ip.empty()) + if (p.secure_gateway_nets_v4.size() || p.secure_gateway_nets_v6.size()) { - os << "secure_gateway IPs:"; - for (auto const& ip : p.secure_gateway_ip) - os << ip.to_string() << ", "; + os << "secure_gateway nets:"; + for (auto const& net : p.secure_gateway_nets_v4) + { + os << net.to_string(); + os << ", "; + } + for (auto const& net : p.secure_gateway_nets_v6) + { + os << net.to_string(); + os << ", "; + } } os << p.protocols() << ")"; @@ -72,65 +89,108 @@ populate( Section const& section, std::string const& field, std::ostream& log, - std::optional>& ips, - bool allowAllIps, - std::vector const& admin_ip) + std::vector& nets4, + std::vector& nets6) { auto const optResult = section.get(field); - if (optResult) + if (!optResult) + return; + + std::stringstream ss(*optResult); + std::string ip; + + while (std::getline(ss, ip, ',')) { - std::stringstream ss(*optResult); - std::string ip; - bool has_any(false); + boost::algorithm::trim(ip); + bool v4; + boost::asio::ip::network_v4 v4Net; + boost::asio::ip::network_v6 v6Net; - ips.emplace(); - while (std::getline(ss, ip, ',')) + try { + // First, check to see if 0.0.0.0 or ipv6 equivalent was configured, + // which means all IP addresses. auto const addr = beast::IP::Endpoint::from_string_checked(ip); - if (!addr) + if (addr) { - log << "Invalid value '" << ip << "' for key '" << field - << "' in [" << section.name() << "]"; - Throw(); - } - - if (is_unspecified(*addr)) - { - if (!allowAllIps) + if (is_unspecified(*addr)) { - log << addr->address() << " not allowed'" - << "' for key '" << field << "' in [" << section.name() - << "]"; - Throw(); + nets4.push_back( + boost::asio::ip::make_network_v4("0.0.0.0/0")); + nets6.push_back(boost::asio::ip::make_network_v6("::/0")); + // No reason to allow more IPs--it would be redundant. + break; + } + + // The configured address is a single IP (or else addr would + // be unset). We need this to be a subnet, so append + // the number of network bits to make a subnet of 1, + // depending on type. + v4 = addr->is_v4(); + std::string addressString = addr->to_string(); + if (v4) + { + addressString += "/32"; + v4Net = boost::asio::ip::make_network_v4(addressString); } else { - has_any = true; + addressString += "/128"; + v6Net = boost::asio::ip::make_network_v6(addressString); + } + } + else + { + // Since addr is empty, assume that the entry is + // for a subnet which includes trailing /0-32 or /0-128 + // depending on ip type. + // First, see if it's an ipv4 subnet. If not, try ipv6. + // If that throws, then there's nothing we can do with + // the entry. + try + { + v4Net = boost::asio::ip::make_network_v4(ip); + v4 = true; + } + catch (boost::system::system_error const& e) + { + v6Net = boost::asio::ip::make_network_v6(ip); + v4 = false; } } - if (has_any && !ips->empty()) + // Confirm that the address entry is the same as the subnet's + // underlying network address. + // 10.1.2.3/24 makes no sense. The underlying network address + // is 10.1.2.0/24. + if (v4) { - log << "IP specified along with " << addr->address() << " '" - << ip << "' for key '" << field << "' in [" - << section.name() << "]"; - Throw(); + if (v4Net != v4Net.canonical()) + { + log << "The configured subnet " << v4Net.to_string() + << " is not the same as the network address, which is " + << v4Net.canonical().to_string(); + Throw(); + } + nets4.push_back(v4Net); } - - auto const& address = addr->address(); - if (std::find_if( - admin_ip.begin(), - admin_ip.end(), - [&address](beast::IP::Address const& a) { - return address == a; - }) != admin_ip.end()) + else { - log << "IP specified for " << field << " is also for " - << "admin: " << ip << " in [" << section.name() << "]"; - Throw(); + if (v6Net != v6Net.canonical()) + { + log << "The configured subnet " << v6Net.to_string() + << " is not the same as the network address, which is " + << v6Net.canonical().to_string(); + Throw(); + } + nets6.push_back(v6Net); } - - ips->emplace_back(addr->address()); + } + catch (boost::system::system_error const& e) + { + log << "Invalid value '" << ip << "' for key '" << field << "' in [" + << section.name() << "]: " << e.what(); + Throw(); } } } @@ -232,14 +292,13 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log) } } - populate(section, "admin", log, port.admin_ip, true, {}); + populate(section, "admin", log, port.admin_nets_v4, port.admin_nets_v6); populate( section, "secure_gateway", log, - port.secure_gateway_ip, - false, - port.admin_ip.value_or(std::vector{})); + port.secure_gateway_nets_v4, + port.secure_gateway_nets_v6); set(port.user, "user", section); set(port.password, "password", section); diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index 09685d32c..45afaf6cb 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -810,11 +810,11 @@ trustthesevalidators.gov ParsedPort rpc; if (!unexcept([&]() { parse_Port(rpc, conf["port_rpc"], log); })) return; - BEAST_EXPECT(rpc.admin_ip && (rpc.admin_ip.value().size() == 2)); + BEAST_EXPECT(rpc.admin_nets_v4.size() + rpc.admin_nets_v6.size() == 2); ParsedPort wss; if (!unexcept([&]() { parse_Port(wss, conf["port_wss_admin"], log); })) return; - BEAST_EXPECT(wss.admin_ip && (wss.admin_ip.value().size() == 1)); + BEAST_EXPECT(wss.admin_nets_v4.size() + wss.admin_nets_v6.size() == 1); } void diff --git a/src/test/jtx/envconfig.h b/src/test/jtx/envconfig.h index 755a6aa86..59b881539 100644 --- a/src/test/jtx/envconfig.h +++ b/src/test/jtx/envconfig.h @@ -84,6 +84,10 @@ std::unique_ptr no_admin(std::unique_ptr); std::unique_ptr secure_gateway(std::unique_ptr); +std::unique_ptr admin_localnet(std::unique_ptr); + +std::unique_ptr secure_gateway_localnet(std::unique_ptr); + /// @brief adjust configuration with params needed to be a validator /// /// this is intended for use with envconfig, as in diff --git a/src/test/jtx/impl/envconfig.cpp b/src/test/jtx/impl/envconfig.cpp index b29e13ab8..f34a444fa 100644 --- a/src/test/jtx/impl/envconfig.cpp +++ b/src/test/jtx/impl/envconfig.cpp @@ -83,6 +83,24 @@ secure_gateway(std::unique_ptr cfg) return cfg; } +std::unique_ptr +admin_localnet(std::unique_ptr cfg) +{ + (*cfg)["port_rpc"].set("admin", "127.0.0.0/8"); + (*cfg)["port_ws"].set("admin", "127.0.0.0/8"); + return cfg; +} + +std::unique_ptr +secure_gateway_localnet(std::unique_ptr cfg) +{ + (*cfg)["port_rpc"].set("admin", ""); + (*cfg)["port_ws"].set("admin", ""); + (*cfg)["port_rpc"].set("secure_gateway", "127.0.0.0/8"); + (*cfg)["port_ws"].set("secure_gateway", "127.0.0.0/8"); + return cfg; +} + auto constexpr defaultseed = "shUwVw52ofnCUX5m7kPTKzJdr4HEH"; std::unique_ptr diff --git a/src/test/rpc/Roles_test.cpp b/src/test/rpc/Roles_test.cpp index a56120740..079fd2f9f 100644 --- a/src/test/rpc/Roles_test.cpp +++ b/src/test/rpc/Roles_test.cpp @@ -269,6 +269,30 @@ class Roles_test : public beast::unit_test::suite BEAST_EXPECT(rpcRes["ip"] == "::11:22:33:44:45.55.65.75"); BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString())); } + + { + Env env{*this, envconfig(admin_localnet)}; + BEAST_EXPECT(env.rpc("ping")["result"]["role"] == "admin"); + BEAST_EXPECT(makeWSClient(env.app().config()) + ->invoke("ping")["result"]["unlimited"] + .asBool()); + } + + { + Env env{*this, envconfig(secure_gateway_localnet)}; + BEAST_EXPECT(env.rpc("ping")["result"]["role"] == "proxied"); + auto wsRes = + makeWSClient(env.app().config())->invoke("ping")["result"]; + BEAST_EXPECT( + !wsRes.isMember("unlimited") || !wsRes["unlimited"].asBool()); + + std::unordered_map headers; + headers["X-Forwarded-For"] = "12.34.56.78"; + Json::Value rpcRes = env.rpc(headers, "ping")["result"]; + BEAST_EXPECT(rpcRes["role"] == "proxied"); + BEAST_EXPECT(rpcRes["ip"] == "12.34.56.78"); + BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString())); + } } void From c0cb389b2014867d05649052a905b97a05ffd1ce Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Wed, 9 Feb 2022 10:53:57 -0800 Subject: [PATCH 08/19] Fallback to normal sync if fast loading is not possible: If fast loading is enabled but the last persisted ledger is not entirely on disk, the server would fail to start without manual intervention by the server operator. This commit allows the server to detect this scenario and attempt to automatically recover. --- src/ripple/app/ledger/Ledger.cpp | 2 +- src/ripple/shamap/SHAMap.h | 2 +- src/ripple/shamap/impl/SHAMapDelta.cpp | 73 ++++++++++++++++---------- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index cac9c40bb..66bea5682 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -774,7 +774,7 @@ Ledger::walkLedger(beast::Journal j, bool parallel) const else { if (parallel) - stateMap_->walkMapParallel(missingNodes1, 32); + return stateMap_->walkMapParallel(missingNodes1, 32); else stateMap_->walkMap(missingNodes1, 32); } diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index f5a108523..76e27bf1e 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -328,7 +328,7 @@ public: void walkMap(std::vector& missingNodes, int maxMissing) const; - void + bool walkMapParallel( std::vector& missingNodes, int maxMissing) const; diff --git a/src/ripple/shamap/impl/SHAMapDelta.cpp b/src/ripple/shamap/impl/SHAMapDelta.cpp index eb6da19e3..896678ef9 100644 --- a/src/ripple/shamap/impl/SHAMapDelta.cpp +++ b/src/ripple/shamap/impl/SHAMapDelta.cpp @@ -290,13 +290,13 @@ SHAMap::walkMap(std::vector& missingNodes, int maxMissing) } } -void +bool SHAMap::walkMapParallel( std::vector& missingNodes, int maxMissing) const { if (!root_->isInner()) // root_ is only node, and we have it - return; + return false; using StackEntry = std::shared_ptr; std::array, 16> topChildren; @@ -311,6 +311,8 @@ SHAMap::walkMapParallel( } std::vector workers; workers.reserve(16); + std::vector exceptions; + exceptions.reserve(16); std::array>, 16> nodeStacks; @@ -329,45 +331,62 @@ SHAMap::walkMapParallel( JLOG(journal_.debug()) << "starting worker " << rootChildIndex; workers.push_back(std::thread( - [&m, &missingNodes, &maxMissing, this]( + [&m, &missingNodes, &maxMissing, &exceptions, this]( std::stack> nodeStack) { - while (!nodeStack.empty()) + try { - std::shared_ptr node = - std::move(nodeStack.top()); - assert(node); - nodeStack.pop(); - - for (int i = 0; i < 16; ++i) + while (!nodeStack.empty()) { - if (node->isEmptyBranch(i)) - continue; - std::shared_ptr nextNode = - descendNoStore(node, i); + std::shared_ptr node = + std::move(nodeStack.top()); + assert(node); + nodeStack.pop(); - if (nextNode) + for (int i = 0; i < 16; ++i) { - if (nextNode->isInner()) - nodeStack.push( - std::static_pointer_cast( - nextNode)); - } - else - { - std::lock_guard l{m}; - missingNodes.emplace_back( - type_, node->getChildHash(i)); - if (--maxMissing <= 0) - return; + if (node->isEmptyBranch(i)) + continue; + std::shared_ptr nextNode = + descendNoStore(node, i); + + if (nextNode) + { + if (nextNode->isInner()) + nodeStack.push(std::static_pointer_cast< + SHAMapInnerNode>(nextNode)); + } + else + { + std::lock_guard l{m}; + missingNodes.emplace_back( + type_, node->getChildHash(i)); + if (--maxMissing <= 0) + return; + } } } } + catch (SHAMapMissingNode const& e) + { + std::lock_guard l(m); + exceptions.push_back(e); + } }, std::move(nodeStacks[rootChildIndex]))); } for (std::thread& worker : workers) worker.join(); + + std::lock_guard l(m); + if (exceptions.empty()) + return true; + std::stringstream ss; + ss << "Exception(s) in ledger load: "; + for (auto const& e : exceptions) + ss << e.what() << ", "; + JLOG(journal_.error()) << ss.str(); + return false; } } // namespace ripple From a529b218f307621a3bbb83fa5a0d798456ba6e83 Mon Sep 17 00:00:00 2001 From: Ikko Ashimine Date: Fri, 14 Jan 2022 01:06:34 +0900 Subject: [PATCH 09/19] Fix typo in ReportingETL.cpp respresent -> represent --- src/ripple/app/reporting/ReportingETL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/reporting/ReportingETL.cpp b/src/ripple/app/reporting/ReportingETL.cpp index 0ffa8392f..15eda7b27 100644 --- a/src/ripple/app/reporting/ReportingETL.cpp +++ b/src/ripple/app/reporting/ReportingETL.cpp @@ -155,7 +155,7 @@ ReportingETL::loadInitialLedger(uint32_t startingSequence) // Once the below call returns, all data has been pushed into the queue loadBalancer_.loadInitialLedger(startingSequence, writeQueue); - // null is used to respresent the end of the queue + // null is used to represent the end of the queue std::shared_ptr null; writeQueue.push(null); // wait for the writer to finish From 0623a40f0281a6084d9764e62922dc976e383d09 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Wed, 12 Jan 2022 10:53:34 -0500 Subject: [PATCH 10/19] Refactor to fix levelization: * Remove Application & Database dependency in PerfLog. Replace it with a callback passed into the constructor. * Fixes the circular dependency between ripple/nodestore and ripple/basics --- Builds/CMake/RippledCore.cmake | 6 +++++- Builds/levelization/README.md | 13 +++++++------ Builds/levelization/results/loops.txt | 6 ------ Builds/levelization/results/ordering.txt | 9 +++++++++ src/ripple/app/main/Application.cpp | 2 -- src/ripple/{basics => perflog}/impl/PerfLogImp.cpp | 3 ++- src/ripple/{basics => perflog}/impl/PerfLogImp.h | 0 7 files changed, 23 insertions(+), 16 deletions(-) rename src/ripple/{basics => perflog}/impl/PerfLogImp.cpp (99%) rename src/ripple/{basics => perflog}/impl/PerfLogImp.h (100%) diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index e8fc5327d..2d6daf47a 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -442,7 +442,6 @@ target_sources (rippled PRIVATE #]===============================] src/ripple/basics/impl/Archive.cpp src/ripple/basics/impl/BasicConfig.cpp - src/ripple/basics/impl/PerfLogImp.cpp src/ripple/basics/impl/ResolverAsio.cpp src/ripple/basics/impl/UptimeClock.cpp src/ripple/basics/impl/make_SSLContext.cpp @@ -640,6 +639,11 @@ target_sources (rippled PRIVATE src/ripple/rpc/impl/ShardVerificationScheduler.cpp src/ripple/rpc/impl/Status.cpp src/ripple/rpc/impl/TransactionSign.cpp + #[===============================[ + main sources: + subdir: perflog + #]===============================] + src/ripple/perflog/impl/PerfLogImp.cpp #[===============================[ main sources: diff --git a/Builds/levelization/README.md b/Builds/levelization/README.md index ef5aa378e..4ff3a5423 100644 --- a/Builds/levelization/README.md +++ b/Builds/levelization/README.md @@ -36,12 +36,13 @@ listed later. | 07 | ripple/shamap ripple/overlay | 08 | ripple/app | 09 | ripple/rpc -| 10 | test/jtx test/beast test/csf -| 11 | test/unit_test -| 12 | test/crypto test/conditions test/json test/resource test/shamap test/peerfinder test/basics test/overlay -| 13 | test -| 14 | test/net test/protocol test/ledger test/consensus test/core test/server test/nodestore -| 15 | test/rpc test/app +| 10 | ripple/perflog +| 11 | test/jtx test/beast test/csf +| 12 | test/unit_test +| 13 | test/crypto test/conditions test/json test/resource test/shamap test/peerfinder test/basics test/overlay +| 14 | test +| 15 | test/net test/protocol test/ledger test/consensus test/core test/server test/nodestore +| 16 | test/rpc test/app (Note that `test` levelization is *much* less important and *much* less strictly enforced than `ripple` levelization, other than the requirement diff --git a/Builds/levelization/results/loops.txt b/Builds/levelization/results/loops.txt index c6c561721..d1838c55c 100644 --- a/Builds/levelization/results/loops.txt +++ b/Builds/levelization/results/loops.txt @@ -28,15 +28,9 @@ Loop: ripple.basics ripple.core Loop: ripple.basics ripple.json ripple.json ~= ripple.basics -Loop: ripple.basics ripple.nodestore - ripple.nodestore > ripple.basics - Loop: ripple.basics ripple.protocol ripple.protocol > ripple.basics -Loop: ripple.basics ripple.rpc - ripple.rpc > ripple.basics - Loop: ripple.core ripple.net ripple.net > ripple.core diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt index 65281daad..ed6b4e57c 100644 --- a/Builds/levelization/results/ordering.txt +++ b/Builds/levelization/results/ordering.txt @@ -29,6 +29,7 @@ ripple.net > ripple.beast ripple.net > ripple.json ripple.net > ripple.protocol ripple.net > ripple.resource +ripple.nodestore > ripple.basics ripple.nodestore > ripple.beast ripple.nodestore > ripple.core ripple.nodestore > ripple.json @@ -46,6 +47,13 @@ ripple.peerfinder > ripple.basics ripple.peerfinder > ripple.beast ripple.peerfinder > ripple.core ripple.peerfinder > ripple.protocol +ripple.perflog > ripple.basics +ripple.perflog > ripple.beast +ripple.perflog > ripple.core +ripple.perflog > ripple.json +ripple.perflog > ripple.nodestore +ripple.perflog > ripple.protocol +ripple.perflog > ripple.rpc ripple.protocol > ripple.beast ripple.protocol > ripple.crypto ripple.protocol > ripple.json @@ -53,6 +61,7 @@ ripple.resource > ripple.basics ripple.resource > ripple.beast ripple.resource > ripple.json ripple.resource > ripple.protocol +ripple.rpc > ripple.basics ripple.rpc > ripple.beast ripple.rpc > ripple.core ripple.rpc > ripple.crypto diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index ac0b6c947..933c49391 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1339,8 +1339,6 @@ ApplicationImp::setup() { // Fall back to syncing from the network, such as // when there's no existing data. - if (startUp == Config::NETWORK && !config_->standalone()) - m_networkOPs->setNeedNetworkLedger(); startGenesisLedger(); } else diff --git a/src/ripple/basics/impl/PerfLogImp.cpp b/src/ripple/perflog/impl/PerfLogImp.cpp similarity index 99% rename from src/ripple/basics/impl/PerfLogImp.cpp rename to src/ripple/perflog/impl/PerfLogImp.cpp index 17d2242ec..db5a188fc 100644 --- a/src/ripple/basics/impl/PerfLogImp.cpp +++ b/src/ripple/perflog/impl/PerfLogImp.cpp @@ -17,8 +17,9 @@ */ //============================================================================== +#include + #include -#include #include #include #include diff --git a/src/ripple/basics/impl/PerfLogImp.h b/src/ripple/perflog/impl/PerfLogImp.h similarity index 100% rename from src/ripple/basics/impl/PerfLogImp.h rename to src/ripple/perflog/impl/PerfLogImp.h From 255bf829ca4df59aac41db0a50475085f5cef4f8 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Mon, 22 Nov 2021 15:18:23 -0500 Subject: [PATCH 11/19] Fix Travis CI MacOS builds: * Update boost version. * Use latest macOS image. * Credit to @donovanhide in #4025 for starting this ball rolling, and inspiring a boost update. --- .travis.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index c967a2453..73c995aa0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,9 +36,9 @@ env: - NIH_CACHE_ROOT=${CACHE_DIR}/nih_c - PARALLEL_TESTS=true # this is NOT used by linux container based builds (which already have boost installed) - - BOOST_URL='https://boostorg.jfrog.io/artifactory/main/release/1.70.0/source/boost_1_70_0.tar.gz' + - BOOST_URL='https://boostorg.jfrog.io/artifactory/main/release/1.75.0/source/boost_1_75_0.tar.gz' # Alternate dowload location - - BOOST_URL2='https://downloads.sourceforge.net/project/boost/boost/1.70.0/boost_1_70_0.tar.bz2?r=&ts=1594393912&use_mirror=newcontinuum' + - BOOST_URL2='https://downloads.sourceforge.net/project/boost/boost/1.75.0/boost_1_75_0.tar.bz2?r=&ts=1594393912&use_mirror=newcontinuum' # Travis downloader doesn't seem to have updated certs. Using this option # introduces obvious security risks, but they're Travis's risks. # Note that this option is only used if the "normal" build fails. @@ -299,15 +299,15 @@ matrix: if: commit_message !~ /travis_run_/ OR commit_message =~ /travis_run_mac/ stage: build os: osx - osx_image: xcode11.2 - name: xcode11.2, debug + osx_image: xcode13.1 + name: xcode13.1, debug env: # put NIH in non-cache location since it seems to # cause failures when homebrew updates - NIH_CACHE_ROOT=${TRAVIS_BUILD_DIR}/nih_c - BLD_CONFIG=Debug - TEST_EXTRA_ARGS="" - - BOOST_ROOT=${CACHE_DIR}/boost_1_70_0 + - BOOST_ROOT=${CACHE_DIR}/boost_1_75_0 - >- CMAKE_ADD=" -DBOOST_ROOT=${BOOST_ROOT}/_INSTALLED_ @@ -338,7 +338,7 @@ matrix: - travis_wait ${MAX_TIME_MIN} cmake --build . --parallel --verbose - ./rippled --unittest --quiet --unittest-log --unittest-jobs ${NUM_PROCESSORS} ${TEST_EXTRA_ARGS} - <<: *macos - name: xcode11.2, release + name: xcode13.1, release before_script: - export BLD_CONFIG=Release - export CMAKE_EXTRA_ARGS="${CMAKE_EXTRA_ARGS} -Dassert=ON" @@ -347,8 +347,8 @@ matrix: before_script: - export TEST_EXTRA_ARGS="--unittest-ipv6" - <<: *macos - osx_image: xcode11.2 - name: xcode11.2, debug + osx_image: xcode13.1 + name: xcode13.1, debug # windows - &windows if: commit_message !~ /travis_run_/ OR commit_message =~ /travis_run_win/ @@ -361,13 +361,13 @@ matrix: - NIH_CACHE_ROOT=${TRAVIS_BUILD_DIR}/nih_c - VCPKG_DEFAULT_TRIPLET="x64-windows-static" - MATRIX_EVAL="CC=cl.exe && CXX=cl.exe" - - BOOST_ROOT=${CACHE_DIR}/boost_1_70 + - BOOST_ROOT=${CACHE_DIR}/boost_1_75 - >- CMAKE_ADD=" -DCMAKE_PREFIX_PATH=${BOOST_ROOT}/_INSTALLED_ -DBOOST_ROOT=${BOOST_ROOT}/_INSTALLED_ -DBoost_ROOT=${BOOST_ROOT}/_INSTALLED_ - -DBoost_DIR=${BOOST_ROOT}/_INSTALLED_/lib/cmake/Boost-1.70.0 + -DBoost_DIR=${BOOST_ROOT}/_INSTALLED_/lib/cmake/Boost-1.75.0 -DBoost_COMPILER=vc141 -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_TOOLCHAIN_FILE=${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake From 90326bf75641748b980a38b2014e759f8c023801 Mon Sep 17 00:00:00 2001 From: natenichols Date: Wed, 26 Jan 2022 11:18:58 -0600 Subject: [PATCH 12/19] Proxy validation_quorum when in reporting mode --- src/ripple/rpc/handlers/ServerInfo.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ripple/rpc/handlers/ServerInfo.cpp b/src/ripple/rpc/handlers/ServerInfo.cpp index 80fde471e..aad3d9988 100644 --- a/src/ripple/rpc/handlers/ServerInfo.cpp +++ b/src/ripple/rpc/handlers/ServerInfo.cpp @@ -44,6 +44,8 @@ doServerInfo(RPC::JsonContext& context) { Json::Value const proxied = forwardToP2p(context); auto const lf = proxied[jss::result][jss::info][jss::load_factor]; + auto const vq = proxied[jss::result][jss::info][jss::validation_quorum]; + ret[jss::info][jss::validation_quorum] = vq.isNull() ? 1 : vq; ret[jss::info][jss::load_factor] = lf.isNull() ? 1 : lf; } return ret; From f326f019bf78f051017850d4e1c5766839f41895 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Wed, 9 Jun 2021 20:36:44 +0000 Subject: [PATCH 13/19] force build with c++17 --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee210b726..97cbe8783 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,7 @@ if (POLICY CMP0074) endif () project (rippled) +set (CMAKE_CXX_STANDARD 17) # make GIT_COMMIT_HASH define available to all sources find_package(Git) From 11ca9a946c4b98c57c93e9059986307b915d0cac Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 28 Jan 2022 16:47:37 -0500 Subject: [PATCH 14/19] Add successor information to clio ETL messages * Allow clio to ask for object successors and predecessors from rippled * Add lower_bound and last_below to SHAMap --- CMakeLists.txt | 4 +- .../proto/org/xrpl/rpc/v1/get_ledger.proto | 16 ++ src/ripple/proto/org/xrpl/rpc/v1/ledger.proto | 21 ++ src/ripple/rpc/handlers/LedgerHandler.cpp | 89 ++++++ src/ripple/shamap/SHAMap.h | 27 ++ src/ripple/shamap/impl/SHAMap.cpp | 74 ++++- src/test/ledger/View_test.cpp | 268 ++++++++++++++++++ src/test/rpc/ReportingETL_test.cpp | 146 +++++++++- 8 files changed, 628 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 97cbe8783..3c369d980 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,7 +5,9 @@ if (POLICY CMP0074) endif () project (rippled) -set (CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_EXTENSIONS OFF) +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) # make GIT_COMMIT_HASH define available to all sources find_package(Git) diff --git a/src/ripple/proto/org/xrpl/rpc/v1/get_ledger.proto b/src/ripple/proto/org/xrpl/rpc/v1/get_ledger.proto index c3147b1f1..0df9ca5ce 100644 --- a/src/ripple/proto/org/xrpl/rpc/v1/get_ledger.proto +++ b/src/ripple/proto/org/xrpl/rpc/v1/get_ledger.proto @@ -32,6 +32,10 @@ message GetLedgerRequest // coming from a secure_gateway host, then the client is not subject to // resource controls string user = 6; + + // For every object in the diff, get the object's predecessor and successor + // in the state map. Only used if get_objects is also true. + bool get_object_neighbors = 7; } message GetLedgerResponse @@ -58,6 +62,18 @@ message GetLedgerResponse // True if request was exempt from resource controls bool is_unlimited = 7; + + // True if the response contains the state map diff + bool objects_included = 8; + + // True if the response contains key of objects adjacent to objects in state + // map diff + bool object_neighbors_included = 9; + + + // Successor information for book directories modified as part of this + // ledger + repeated BookSuccessor book_successors = 10; } message TransactionHashList diff --git a/src/ripple/proto/org/xrpl/rpc/v1/ledger.proto b/src/ripple/proto/org/xrpl/rpc/v1/ledger.proto index 70b0a3562..78e4211f5 100644 --- a/src/ripple/proto/org/xrpl/rpc/v1/ledger.proto +++ b/src/ripple/proto/org/xrpl/rpc/v1/ledger.proto @@ -54,7 +54,14 @@ message RawLedgerObject DELETED = 3; } + // Whether the object was created, modified or deleted ModificationType mod_type = 3; + + // Key of the object preceding this object in the desired ledger + bytes predecessor = 4; + + // Key of the object succeeding this object in the desired ledger + bytes successor = 5; } message RawLedgerObjects @@ -62,3 +69,17 @@ message RawLedgerObjects repeated RawLedgerObject objects = 1; } +// Successor information for book directories. The book base is (usually) not +// an actual object, yet we need to be able to ask for the successor to the +// book base. +message BookSuccessor { + + // Base of the book in question + bytes book_base = 1; + + // First book directory in the book. An empty value here means the entire + // book is deleted + bytes first_book = 2; + +}; + diff --git a/src/ripple/rpc/handlers/LedgerHandler.cpp b/src/ripple/rpc/handlers/LedgerHandler.cpp index c358f2761..b2e4cb8dd 100644 --- a/src/ripple/rpc/handlers/LedgerHandler.cpp +++ b/src/ripple/rpc/handlers/LedgerHandler.cpp @@ -105,6 +105,7 @@ LedgerHandler::check() std::pair doLedgerGrpc(RPC::GRPCContext& context) { + auto begin = std::chrono::system_clock::now(); org::xrpl::rpc::v1::GetLedgerRequest& request = context.params; org::xrpl::rpc::v1::GetLedgerResponse response; grpc::Status status = grpc::Status::OK; @@ -212,13 +213,101 @@ doLedgerGrpc(RPC::GRPCContext& context) obj->set_mod_type(org::xrpl::rpc::v1::RawLedgerObject::DELETED); else obj->set_mod_type(org::xrpl::rpc::v1::RawLedgerObject::CREATED); + auto const blob = inDesired ? inDesired->slice() : inBase->slice(); + auto const objectType = + static_cast(blob[1] << 8 | blob[2]); + + if (request.get_object_neighbors()) + { + if (!(inBase && inDesired)) + { + auto lb = desired->stateMap().lower_bound(k); + auto ub = desired->stateMap().upper_bound(k); + if (lb != desired->stateMap().end()) + obj->set_predecessor( + lb->key().data(), lb->key().size()); + if (ub != desired->stateMap().end()) + obj->set_successor(ub->key().data(), ub->key().size()); + if (objectType == ltDIR_NODE) + { + auto sle = std::make_shared(SerialIter{blob}, k); + if (!sle->isFieldPresent(sfOwner)) + { + auto bookBase = keylet::quality({ltDIR_NODE, k}, 0); + if (!inBase && inDesired) + { + auto firstBook = + desired->stateMap().upper_bound( + bookBase.key); + if (firstBook != desired->stateMap().end() && + firstBook->key() < + getQualityNext(bookBase.key) && + firstBook->key() == k) + { + auto succ = response.add_book_successors(); + succ->set_book_base( + bookBase.key.data(), + bookBase.key.size()); + succ->set_first_book( + firstBook->key().data(), + firstBook->key().size()); + } + } + if (inBase && !inDesired) + { + auto oldFirstBook = + base->stateMap().upper_bound(bookBase.key); + if (oldFirstBook != base->stateMap().end() && + oldFirstBook->key() < + getQualityNext(bookBase.key) && + oldFirstBook->key() == k) + { + auto succ = response.add_book_successors(); + succ->set_book_base( + bookBase.key.data(), + bookBase.key.size()); + auto newFirstBook = + desired->stateMap().upper_bound( + bookBase.key); + + if (newFirstBook != + desired->stateMap().end() && + newFirstBook->key() < + getQualityNext(bookBase.key)) + { + succ->set_first_book( + newFirstBook->key().data(), + newFirstBook->key().size()); + } + } + } + } + } + } + } } + response.set_objects_included(true); + response.set_object_neighbors_included(request.get_object_neighbors()); response.set_skiplist_included(true); } response.set_validated( RPC::isValidated(context.ledgerMaster, *ledger, context.app)); + auto end = std::chrono::system_clock::now(); + auto duration = + std::chrono::duration_cast(end - begin) + .count() * + 1.0; + JLOG(context.j.warn()) + << __func__ << " - Extract time = " << duration + << " - num objects = " << response.ledger_objects().objects_size() + << " - num txns = " << response.transactions_list().transactions_size() + << " - ms per obj " + << duration / response.ledger_objects().objects_size() + << " - ms per txn " + << duration / response.transactions_list().transactions_size(); + return {response, status}; } } // namespace ripple diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index 76e27bf1e..b913bd5b1 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -210,9 +210,17 @@ public: peekItem(uint256 const& id, SHAMapHash& hash) const; // traverse functions + + // finds the object in the tree with the smallest object id greater than the + // input id const_iterator upper_bound(uint256 const& id) const; + // finds the object in the tree with the greatest object id smaller than the + // input id + const_iterator + lower_bound(uint256 const& id) const; + /** Visit every node in this SHAMap @param function called with every node visited. @@ -400,12 +408,31 @@ private: std::shared_ptr writeNode(NodeObjectType t, std::shared_ptr node) const; + // returns the first item at or below this node SHAMapLeafNode* firstBelow( std::shared_ptr, SharedPtrNodeStack& stack, int branch = 0) const; + // returns the last item at or below this node + SHAMapLeafNode* + lastBelow( + std::shared_ptr node, + SharedPtrNodeStack& stack, + int branch = branchFactor) const; + + // helper function for firstBelow and lastBelow + SHAMapLeafNode* + belowHelper( + std::shared_ptr node, + SharedPtrNodeStack& stack, + int branch, + std::tuple< + int, + std::function, + std::function> const& loopParams) const; + // Simple descent // Get a child of the specified node SHAMapTreeNode* diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 2bd1c85f3..27547aaec 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -437,12 +437,14 @@ SHAMap::unshareNode(std::shared_ptr node, SHAMapNodeID const& nodeID) } SHAMapLeafNode* -SHAMap::firstBelow( +SHAMap::belowHelper( std::shared_ptr node, SharedPtrNodeStack& stack, - int branch) const + int branch, + std::tuple, std::function> const& + loopParams) const { - // Return the first item at or below this node + auto& [init, cmp, incr] = loopParams; if (node->isLeaf()) { auto n = std::static_pointer_cast(node); @@ -454,7 +456,7 @@ SHAMap::firstBelow( stack.push({inner, SHAMapNodeID{}}); else stack.push({inner, stack.top().second.getChildNodeID(branch)}); - for (int i = 0; i < branchFactor;) + for (int i = init; cmp(i);) { if (!inner->isEmptyBranch(i)) { @@ -468,14 +470,37 @@ SHAMap::firstBelow( } inner = std::static_pointer_cast(node); stack.push({inner, stack.top().second.getChildNodeID(branch)}); - i = 0; // scan all 16 branches of this new node + i = init; // descend and reset loop } else - ++i; // scan next branch + incr(i); // scan next branch } return nullptr; } +SHAMapLeafNode* +SHAMap::lastBelow( + std::shared_ptr node, + SharedPtrNodeStack& stack, + int branch) const +{ + auto init = branchFactor - 1; + auto cmp = [](int i) { return i >= 0; }; + auto incr = [](int& i) { --i; }; + return belowHelper(node, stack, branch, {init, cmp, incr}); +} +SHAMapLeafNode* +SHAMap::firstBelow( + std::shared_ptr node, + SharedPtrNodeStack& stack, + int branch) const +{ + auto init = 0; + auto cmp = [](int i) { return i <= branchFactor; }; + auto incr = [](int& i) { ++i; }; + + return belowHelper(node, stack, branch, {init, cmp, incr}); +} static const std::shared_ptr no_item; std::shared_ptr const& @@ -619,6 +644,43 @@ SHAMap::upper_bound(uint256 const& id) const } return end(); } +SHAMap::const_iterator +SHAMap::lower_bound(uint256 const& id) const +{ + SharedPtrNodeStack stack; + walkTowardsKey(id, &stack); + while (!stack.empty()) + { + auto [node, nodeID] = stack.top(); + if (node->isLeaf()) + { + auto leaf = static_cast(node.get()); + if (leaf->peekItem()->key() < id) + return const_iterator( + this, leaf->peekItem().get(), std::move(stack)); + } + else + { + auto inner = std::static_pointer_cast(node); + for (int branch = selectBranch(nodeID, id) - 1; branch >= 0; + --branch) + { + if (!inner->isEmptyBranch(branch)) + { + node = descendThrow(inner, branch); + auto leaf = lastBelow(node, stack, branch); + if (!leaf) + Throw(type_, id); + return const_iterator( + this, leaf->peekItem().get(), std::move(stack)); + } + } + } + stack.pop(); + } + // TODO: what to return here? + return end(); +} bool SHAMap::hasItem(uint256 const& id) const diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index 3d86ad227..45c8f007e 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -384,6 +384,273 @@ class View_test : public beast::unit_test::suite return std::vector({uint256(args)...}); } + void + testUpperAndLowerBound() + { + using namespace jtx; + Env env(*this); + Config config; + std::shared_ptr const genesis = std::make_shared( + create_genesis, + config, + std::vector{}, + env.app().getNodeFamily()); + auto const ledger = std::make_shared( + *genesis, env.app().timeKeeper().closeTime()); + + auto setup = [&ledger](std::vector const& vec) { + wipe(*ledger); + for (auto x : vec) + { + ledger->rawInsert(sle(x)); + } + }; + { + setup({1, 2, 3}); + BEAST_EXPECT(sles(*ledger) == list(1, 2, 3)); + auto e = ledger->stateMap().end(); + auto b1 = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(1)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(2)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(3)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(4)) == b1); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(5)) == b1); + b1 = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(0)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(1)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(2)) == b1); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(3)) == e); + } + + { + setup({2, 4, 6}); + BEAST_EXPECT(sles(*ledger) == list(2, 4, 6)); + auto e = ledger->stateMap().end(); + auto b1 = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(1)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(2)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(3)) == b1); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(4)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(5)) == b1); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(6)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(7)) == b1); + b1 = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(1)) == b1); + ++b1; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(2)) == b1); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(3)) == b1); + ++b1; + + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(4)) == b1); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(5)) == b1); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(6)) == e); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(7)) == e); + } + { + setup({2, 3, 5, 6, 10, 15}); + BEAST_EXPECT(sles(*ledger) == list(2, 3, 5, 6, 10, 15)); + auto e = ledger->stateMap().end(); + auto b = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(1)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(2)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(3)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(4)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(5)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(6)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(7)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(8)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(9)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(10)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(11)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(12)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(13)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(14)) == b); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(15)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(16)) == b); + b = ledger->stateMap().begin(); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(0)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(1)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(2)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(3)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(4)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(5)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(6)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(7)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(8)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(9)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(10)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(11)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(12)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(13)) == b); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(14)) == b); + ++b; + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(15)) == e); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(16)) == e); + } + { + // some full trees, some empty trees, etc + setup({0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, + 13, 14, 15, 16, 20, 25, 30, 32, 33, 34, 35, 36, 37, + 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 66, 100}); + BEAST_EXPECT( + sles(*ledger) == + list( + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 20, + 25, + 30, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 66, + 100)); + auto b = ledger->stateMap().begin(); + auto e = ledger->stateMap().end(); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(0)) == e); + BEAST_EXPECT(ledger->stateMap().lower_bound(uint256(1)) == b); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(5))->key() == + uint256(4)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(15))->key() == + uint256(14)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(16))->key() == + uint256(15)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(19))->key() == + uint256(16)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(20))->key() == + uint256(16)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(24))->key() == + uint256(20)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(31))->key() == + uint256(30)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(32))->key() == + uint256(30)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(40))->key() == + uint256(39)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(47))->key() == + uint256(46)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(48))->key() == + uint256(47)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(64))->key() == + uint256(48)); + + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(90))->key() == + uint256(66)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(96))->key() == + uint256(66)); + BEAST_EXPECT( + ledger->stateMap().lower_bound(uint256(100))->key() == + uint256(66)); + + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(0))->key() == + uint256(1)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(5))->key() == + uint256(6)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(15))->key() == + uint256(16)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(16))->key() == + uint256(20)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(18))->key() == + uint256(20)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(20))->key() == + uint256(25)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(31))->key() == + uint256(32)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(32))->key() == + uint256(33)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(47))->key() == + uint256(48)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(48))->key() == + uint256(66)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(53))->key() == + uint256(66)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(66))->key() == + uint256(100)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(70))->key() == + uint256(100)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(85))->key() == + uint256(100)); + BEAST_EXPECT( + ledger->stateMap().upper_bound(uint256(98))->key() == + uint256(100)); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(100)) == e); + BEAST_EXPECT(ledger->stateMap().upper_bound(uint256(155)) == e); + } + } + void testSles() { @@ -811,6 +1078,7 @@ class View_test : public beast::unit_test::suite testStacked(); testContext(); testSles(); + testUpperAndLowerBound(); testFlags(); testTransferRate(); testAreCompatible(); diff --git a/src/test/rpc/ReportingETL_test.cpp b/src/test/rpc/ReportingETL_test.cpp index 8678e2b9b..36b2f9b0b 100644 --- a/src/test/rpc/ReportingETL_test.cpp +++ b/src/test/rpc/ReportingETL_test.cpp @@ -70,20 +70,22 @@ class ReportingETL_test : public beast::unit_test::suite auto sequence, bool transactions, bool expand, - bool get_objects) { + bool get_objects, + bool get_object_neighbors) { GrpcLedgerClient grpcClient{grpcPort}; grpcClient.request.mutable_ledger()->set_sequence(sequence); grpcClient.request.set_transactions(transactions); grpcClient.request.set_expand(expand); grpcClient.request.set_get_objects(get_objects); + grpcClient.request.set_get_object_neighbors(get_object_neighbors); grpcClient.GetLedger(); return std::make_pair(grpcClient.status, grpcClient.reply); }; { - auto [status, reply] = grpcLedger(3, false, false, false); + auto [status, reply] = grpcLedger(3, false, false, false, false); BEAST_EXPECT(status.ok()); BEAST_EXPECT(reply.validated()); @@ -119,7 +121,7 @@ class ReportingETL_test : public beast::unit_test::suite addRaw(ledger->info(), s, true); { - auto [status, reply] = grpcLedger(4, true, false, false); + auto [status, reply] = grpcLedger(4, true, false, false, false); BEAST_EXPECT(status.ok()); BEAST_EXPECT(reply.validated()); BEAST_EXPECT(reply.has_hashes_list()); @@ -145,7 +147,7 @@ class ReportingETL_test : public beast::unit_test::suite } { - auto [status, reply] = grpcLedger(4, true, true, false); + auto [status, reply] = grpcLedger(4, true, true, false, false); BEAST_EXPECT(status.ok()); BEAST_EXPECT(reply.validated()); @@ -209,7 +211,7 @@ class ReportingETL_test : public beast::unit_test::suite } { - auto [status, reply] = grpcLedger(4, true, true, true); + auto [status, reply] = grpcLedger(4, true, true, true, false); BEAST_EXPECT(status.ok()); BEAST_EXPECT(reply.validated()); @@ -295,6 +297,112 @@ class ReportingETL_test : public beast::unit_test::suite ++idx; } } + { + auto [status, reply] = grpcLedger(4, true, true, true, true); + + BEAST_EXPECT(status.ok()); + BEAST_EXPECT(reply.validated()); + BEAST_EXPECT(!reply.has_hashes_list()); + BEAST_EXPECT(reply.object_neighbors_included()); + + BEAST_EXPECT(reply.has_transactions_list()); + BEAST_EXPECT(reply.transactions_list().transactions_size() == 4); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(0) + .transaction_blob()) == + transactions[0]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(0) + .metadata_blob()) == + metas[0]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(1) + .transaction_blob()) == + transactions[1]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(1) + .metadata_blob()) == + metas[1]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(2) + .transaction_blob()) == + transactions[2]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(2) + .metadata_blob()) == + metas[2]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(3) + .transaction_blob()) == + transactions[3]->getSerializer().slice()); + + BEAST_EXPECT( + makeSlice(reply.transactions_list() + .transactions(3) + .metadata_blob()) == + metas[3]->getSerializer().slice()); + BEAST_EXPECT(reply.skiplist_included()); + + BEAST_EXPECT(s.slice() == makeSlice(reply.ledger_header())); + + auto parent = env.app().getLedgerMaster().getLedgerBySeq(3); + + SHAMap::Delta differences; + + int maxDifferences = std::numeric_limits::max(); + + bool res = parent->stateMap().compare( + ledger->stateMap(), differences, maxDifferences); + BEAST_EXPECT(res); + + size_t idx = 0; + + for (auto& [k, v] : differences) + { + auto obj = reply.ledger_objects().objects(idx); + BEAST_EXPECT(k == uint256::fromVoid(obj.key().data())); + if (v.second) + { + BEAST_EXPECT(v.second->slice() == makeSlice(obj.data())); + } + else + BEAST_EXPECT(obj.data().size() == 0); + + if (!(v.first && v.second)) + { + auto succ = ledger->stateMap().upper_bound(k); + auto pred = ledger->stateMap().lower_bound(k); + + if (succ != ledger->stateMap().end()) + BEAST_EXPECT( + succ->key() == + uint256::fromVoid(obj.successor().data())); + else + BEAST_EXPECT(obj.successor().size() == 0); + if (pred != ledger->stateMap().end()) + BEAST_EXPECT( + pred->key() == + uint256::fromVoid(obj.predecessor().data())); + else + BEAST_EXPECT(obj.predecessor().size() == 0); + } + ++idx; + } + } // Delete an account @@ -312,7 +420,7 @@ class ReportingETL_test : public beast::unit_test::suite { auto [status, reply] = - grpcLedger(env.closed()->seq(), true, true, true); + grpcLedger(env.closed()->seq(), true, true, true, true); BEAST_EXPECT(status.ok()); BEAST_EXPECT(reply.validated()); @@ -333,16 +441,34 @@ class ReportingETL_test : public beast::unit_test::suite size_t idx = 0; for (auto& [k, v] : differences) { - BEAST_EXPECT( - k == - uint256::fromVoid( - reply.ledger_objects().objects(idx).key().data())); + auto obj = reply.ledger_objects().objects(idx); + BEAST_EXPECT(k == uint256::fromVoid(obj.key().data())); if (v.second) { BEAST_EXPECT( v.second->slice() == makeSlice(reply.ledger_objects().objects(idx).data())); } + else + BEAST_EXPECT(obj.data().size() == 0); + if (!(v.first && v.second)) + { + auto succ = base->stateMap().upper_bound(k); + auto pred = base->stateMap().lower_bound(k); + + if (succ != base->stateMap().end()) + BEAST_EXPECT( + succ->key() == + uint256::fromVoid(obj.successor().data())); + else + BEAST_EXPECT(obj.successor().size() == 0); + if (pred != base->stateMap().end()) + BEAST_EXPECT( + pred->key() == + uint256::fromVoid(obj.predecessor().data())); + else + BEAST_EXPECT(obj.predecessor().size() == 0); + } ++idx; } From a01cadbfd522e75ff9b817194782ca7e52430761 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Mon, 20 Dec 2021 23:47:01 -0800 Subject: [PATCH 15/19] Move Beast & fix `#include` paths --- Builds/CMake/RippledCore.cmake | 34 ++-- src/beast/extras/README.md | 3 - src/beast/extras/beast/doc_debug.hpp | 170 ------------------ src/ripple/app/main/GRPCServer.cpp | 2 +- src/ripple/app/main/Main.cpp | 2 +- src/ripple/basics/CountedObject.h | 2 +- .../beast/test/fail_counter.hpp | 0 .../beast/test/fail_stream.hpp | 0 .../beast/test/pipe_stream.hpp | 0 .../extras => ripple}/beast/test/sig_wait.hpp | 0 .../beast/test/string_iostream.hpp | 0 .../beast/test/string_istream.hpp | 0 .../beast/test/string_ostream.hpp | 0 .../beast/test/test_allocator.hpp | 0 .../extras => ripple}/beast/test/yield_to.hpp | 0 src/ripple/beast/unit_test.h | 20 +-- .../beast/unit_test/amount.hpp | 0 .../unit_test/detail/const_container.hpp | 0 .../beast/unit_test/dstream.hpp | 0 .../beast/unit_test/global_suites.hpp | 2 +- .../beast/unit_test/main.cpp | 79 ++++---- .../beast/unit_test/match.hpp | 2 +- .../beast/unit_test/recorder.hpp | 4 +- .../beast/unit_test/reporter.hpp | 4 +- .../beast/unit_test/results.hpp | 2 +- .../beast/unit_test/runner.hpp | 2 +- .../beast/unit_test/suite.hpp | 4 +- .../beast/unit_test/suite_info.hpp | 0 .../beast/unit_test/suite_list.hpp | 5 +- .../beast/unit_test/thread.hpp | 2 +- src/ripple/protocol/KnownFormats.h | 2 +- src/test/basics/DetectCrash_test.cpp | 2 +- .../beast/beast_io_latency_probe_test.cpp | 2 +- src/test/beast/define_print.cpp | 6 +- src/test/nodestore/Timing_test.cpp | 2 +- src/test/server/ServerStatus_test.cpp | 2 +- src/test/unit_test/multi_runner.cpp | 2 +- src/test/unit_test/multi_runner.h | 4 +- 38 files changed, 87 insertions(+), 274 deletions(-) delete mode 100644 src/beast/extras/README.md delete mode 100644 src/beast/extras/beast/doc_debug.hpp rename src/{beast/extras => ripple}/beast/test/fail_counter.hpp (100%) rename src/{beast/extras => ripple}/beast/test/fail_stream.hpp (100%) rename src/{beast/extras => ripple}/beast/test/pipe_stream.hpp (100%) rename src/{beast/extras => ripple}/beast/test/sig_wait.hpp (100%) rename src/{beast/extras => ripple}/beast/test/string_iostream.hpp (100%) rename src/{beast/extras => ripple}/beast/test/string_istream.hpp (100%) rename src/{beast/extras => ripple}/beast/test/string_ostream.hpp (100%) rename src/{beast/extras => ripple}/beast/test/test_allocator.hpp (100%) rename src/{beast/extras => ripple}/beast/test/yield_to.hpp (100%) rename src/{beast/extras => ripple}/beast/unit_test/amount.hpp (100%) rename src/{beast/extras => ripple}/beast/unit_test/detail/const_container.hpp (100%) rename src/{beast/extras => ripple}/beast/unit_test/dstream.hpp (100%) rename src/{beast/extras => ripple}/beast/unit_test/global_suites.hpp (95%) rename src/{beast/extras => ripple}/beast/unit_test/main.cpp (57%) rename src/{beast/extras => ripple}/beast/unit_test/match.hpp (98%) rename src/{beast/extras => ripple}/beast/unit_test/recorder.hpp (94%) rename src/{beast/extras => ripple}/beast/unit_test/reporter.hpp (98%) rename src/{beast/extras => ripple}/beast/unit_test/results.hpp (98%) rename src/{beast/extras => ripple}/beast/unit_test/runner.hpp (99%) rename src/{beast/extras => ripple}/beast/unit_test/suite.hpp (99%) rename src/{beast/extras => ripple}/beast/unit_test/suite_info.hpp (100%) rename src/{beast/extras => ripple}/beast/unit_test/suite_list.hpp (93%) rename src/{beast/extras => ripple}/beast/unit_test/thread.hpp (98%) diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 2d6daf47a..e0c627e2b 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -120,12 +120,8 @@ add_library (Ripple::xrpl_core ALIAS xrpl_core) target_include_directories (xrpl_core PUBLIC $ - $ - # this one is for beast/legacy files: - $ $) - target_compile_definitions(xrpl_core PUBLIC BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT @@ -295,23 +291,23 @@ install ( if (tests) install ( FILES - src/beast/extras/beast/unit_test/amount.hpp - src/beast/extras/beast/unit_test/dstream.hpp - src/beast/extras/beast/unit_test/global_suites.hpp - src/beast/extras/beast/unit_test/match.hpp - src/beast/extras/beast/unit_test/recorder.hpp - src/beast/extras/beast/unit_test/reporter.hpp - src/beast/extras/beast/unit_test/results.hpp - src/beast/extras/beast/unit_test/runner.hpp - src/beast/extras/beast/unit_test/suite.hpp - src/beast/extras/beast/unit_test/suite_info.hpp - src/beast/extras/beast/unit_test/suite_list.hpp - src/beast/extras/beast/unit_test/thread.hpp - DESTINATION include/beast/unit_test) + src/ripple/beast/unit_test/amount.hpp + src/ripple/beast/unit_test/dstream.hpp + src/ripple/beast/unit_test/global_suites.hpp + src/ripple/beast/unit_test/match.hpp + src/ripple/beast/unit_test/recorder.hpp + src/ripple/beast/unit_test/reporter.hpp + src/ripple/beast/unit_test/results.hpp + src/ripple/beast/unit_test/runner.hpp + src/ripple/beast/unit_test/suite.hpp + src/ripple/beast/unit_test/suite_info.hpp + src/ripple/beast/unit_test/suite_list.hpp + src/ripple/beast/unit_test/thread.hpp + DESTINATION include/ripple/beast/extras/unit_test) install ( FILES - src/beast/extras/beast/unit_test/detail/const_container.hpp - DESTINATION include/beast/unit_test/detail) + src/ripple/beast/unit_test/detail/const_container.hpp + DESTINATION include/ripple/beast/unit_test/detail) endif () #tests #[===================================================================[ rippled executable diff --git a/src/beast/extras/README.md b/src/beast/extras/README.md deleted file mode 100644 index ea6ffce9d..000000000 --- a/src/beast/extras/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# Extras - -These are not part of the official public Beast interface but they are used by the tests and some third party programs. diff --git a/src/beast/extras/beast/doc_debug.hpp b/src/beast/extras/beast/doc_debug.hpp deleted file mode 100644 index 7023c8a2c..000000000 --- a/src/beast/extras/beast/doc_debug.hpp +++ /dev/null @@ -1,170 +0,0 @@ -// -// Copyright (c) 2013-2017 Vinnie Falco (vinnie dot falco at gmail dot com) -// -// Distributed under the Boost Software License, Version 1.0. (See accompanying -// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -// - -#ifndef BEAST_DOC_DEBUG_HPP -#define BEAST_DOC_DEBUG_HPP - -namespace beast { - -#if BEAST_DOXYGEN - -/// doc type (documentation debug helper) -using doc_type = int; - -/// doc enum (documentation debug helper) -enum doc_enum -{ - /// One (documentation debug helper) - one, - - /// Two (documentation debug helper) - two -}; - -/// doc enum class (documentation debug helper) -enum class doc_enum_class : unsigned -{ - /// one (documentation debug helper) - one, - - /// two (documentation debug helper) - two -}; - -/// doc func (documentation debug helper) -void doc_func(); - -/// doc class (documentation debug helper) -struct doc_class -{ - /// doc class member func (documentation debug helper) - void func(); -}; - -/// (documentation debug helper) -namespace nested { - -/// doc type (documentation debug helper) -using nested_doc_type = int; - -/// doc enum (documentation debug helper) -enum nested_doc_enum -{ - /// One (documentation debug helper) - one, - - /// Two (documentation debug helper) - two -}; - -/// doc enum class (documentation debug helper) -enum class nested_doc_enum_class : unsigned -{ - /// one (documentation debug helper) - one, - - /// two (documentation debug helper) - two -}; - -/// doc func (documentation debug helper) -void nested_doc_func(); - -/// doc class (documentation debug helper) -struct nested_doc_class -{ - /// doc class member func (documentation debug helper) - void func(); -}; - -} // nested - -/** This is here to help troubleshoot doc/reference.xsl problems - - Embedded references: - - @li type @ref doc_type - - @li enum @ref doc_enum - - @li enum item @ref doc_enum::one - - @li enum_class @ref doc_enum_class - - @li enum_class item @ref doc_enum_class::one - - @li func @ref doc_func - - @li class @ref doc_class - - @li class func @ref doc_class::func - - @li nested type @ref nested::nested_doc_type - - @li nested enum @ref nested::nested_doc_enum - - @li nested enum item @ref nested::nested_doc_enum::one - - @li nested enum_class @ref nested::nested_doc_enum_class - - @li nested enum_class item @ref nested::nested_doc_enum_class::one - - @li nested func @ref nested::nested_doc_func - - @li nested class @ref nested::nested_doc_class - - @li nested class func @ref nested::nested_doc_class::func -*/ -void doc_debug(); - -namespace nested { - -/** This is here to help troubleshoot doc/reference.xsl problems - - Embedded references: - - @li type @ref doc_type - - @li enum @ref doc_enum - - @li enum item @ref doc_enum::one - - @li enum_class @ref doc_enum_class - - @li enum_class item @ref doc_enum_class::one - - @li func @ref doc_func - - @li class @ref doc_class - - @li class func @ref doc_class::func - - @li nested type @ref nested_doc_type - - @li nested enum @ref nested_doc_enum - - @li nested enum item @ref nested_doc_enum::one - - @li nested enum_class @ref nested_doc_enum_class - - @li nested enum_class item @ref nested_doc_enum_class::one - - @li nested func @ref nested_doc_func - - @li nested class @ref nested_doc_class - - @li nested class func @ref nested_doc_class::func -*/ -void nested_doc_debug(); - -} // nested - -#endif - -} // beast - -#endif diff --git a/src/ripple/app/main/GRPCServer.cpp b/src/ripple/app/main/GRPCServer.cpp index ddaea14c2..aef2612c0 100644 --- a/src/ripple/app/main/GRPCServer.cpp +++ b/src/ripple/app/main/GRPCServer.cpp @@ -22,7 +22,7 @@ #include #include -#include +#include namespace ripple { diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 34ec2989b..14befa63e 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -35,7 +35,7 @@ #include #ifdef ENABLE_TESTS -#include +#include #include #endif // ENABLE_TESTS diff --git a/src/ripple/basics/CountedObject.h b/src/ripple/basics/CountedObject.h index a5bccf89e..690841fe1 100644 --- a/src/ripple/basics/CountedObject.h +++ b/src/ripple/basics/CountedObject.h @@ -20,8 +20,8 @@ #ifndef RIPPLE_BASICS_COUNTEDOBJECT_H_INCLUDED #define RIPPLE_BASICS_COUNTEDOBJECT_H_INCLUDED +#include #include -#include #include #include #include diff --git a/src/beast/extras/beast/test/fail_counter.hpp b/src/ripple/beast/test/fail_counter.hpp similarity index 100% rename from src/beast/extras/beast/test/fail_counter.hpp rename to src/ripple/beast/test/fail_counter.hpp diff --git a/src/beast/extras/beast/test/fail_stream.hpp b/src/ripple/beast/test/fail_stream.hpp similarity index 100% rename from src/beast/extras/beast/test/fail_stream.hpp rename to src/ripple/beast/test/fail_stream.hpp diff --git a/src/beast/extras/beast/test/pipe_stream.hpp b/src/ripple/beast/test/pipe_stream.hpp similarity index 100% rename from src/beast/extras/beast/test/pipe_stream.hpp rename to src/ripple/beast/test/pipe_stream.hpp diff --git a/src/beast/extras/beast/test/sig_wait.hpp b/src/ripple/beast/test/sig_wait.hpp similarity index 100% rename from src/beast/extras/beast/test/sig_wait.hpp rename to src/ripple/beast/test/sig_wait.hpp diff --git a/src/beast/extras/beast/test/string_iostream.hpp b/src/ripple/beast/test/string_iostream.hpp similarity index 100% rename from src/beast/extras/beast/test/string_iostream.hpp rename to src/ripple/beast/test/string_iostream.hpp diff --git a/src/beast/extras/beast/test/string_istream.hpp b/src/ripple/beast/test/string_istream.hpp similarity index 100% rename from src/beast/extras/beast/test/string_istream.hpp rename to src/ripple/beast/test/string_istream.hpp diff --git a/src/beast/extras/beast/test/string_ostream.hpp b/src/ripple/beast/test/string_ostream.hpp similarity index 100% rename from src/beast/extras/beast/test/string_ostream.hpp rename to src/ripple/beast/test/string_ostream.hpp diff --git a/src/beast/extras/beast/test/test_allocator.hpp b/src/ripple/beast/test/test_allocator.hpp similarity index 100% rename from src/beast/extras/beast/test/test_allocator.hpp rename to src/ripple/beast/test/test_allocator.hpp diff --git a/src/beast/extras/beast/test/yield_to.hpp b/src/ripple/beast/test/yield_to.hpp similarity index 100% rename from src/beast/extras/beast/test/yield_to.hpp rename to src/ripple/beast/test/yield_to.hpp diff --git a/src/ripple/beast/unit_test.h b/src/ripple/beast/unit_test.h index 3352abb6c..b80c188de 100644 --- a/src/ripple/beast/unit_test.h +++ b/src/ripple/beast/unit_test.h @@ -20,16 +20,16 @@ #ifndef BEAST_UNIT_TEST_H_INCLUDED #define BEAST_UNIT_TEST_H_INCLUDED -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #ifndef BEAST_EXPECT #define BEAST_EXPECT_S1(x) #x diff --git a/src/beast/extras/beast/unit_test/amount.hpp b/src/ripple/beast/unit_test/amount.hpp similarity index 100% rename from src/beast/extras/beast/unit_test/amount.hpp rename to src/ripple/beast/unit_test/amount.hpp diff --git a/src/beast/extras/beast/unit_test/detail/const_container.hpp b/src/ripple/beast/unit_test/detail/const_container.hpp similarity index 100% rename from src/beast/extras/beast/unit_test/detail/const_container.hpp rename to src/ripple/beast/unit_test/detail/const_container.hpp diff --git a/src/beast/extras/beast/unit_test/dstream.hpp b/src/ripple/beast/unit_test/dstream.hpp similarity index 100% rename from src/beast/extras/beast/unit_test/dstream.hpp rename to src/ripple/beast/unit_test/dstream.hpp diff --git a/src/beast/extras/beast/unit_test/global_suites.hpp b/src/ripple/beast/unit_test/global_suites.hpp similarity index 95% rename from src/beast/extras/beast/unit_test/global_suites.hpp rename to src/ripple/beast/unit_test/global_suites.hpp index 3fbf79589..b641ed7cb 100644 --- a/src/beast/extras/beast/unit_test/global_suites.hpp +++ b/src/ripple/beast/unit_test/global_suites.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_GLOBAL_SUITES_HPP #define BEAST_UNIT_TEST_GLOBAL_SUITES_HPP -#include +#include namespace beast { namespace unit_test { diff --git a/src/beast/extras/beast/unit_test/main.cpp b/src/ripple/beast/unit_test/main.cpp similarity index 57% rename from src/beast/extras/beast/unit_test/main.cpp rename to src/ripple/beast/unit_test/main.cpp index 5762b5102..aa1a0f536 100644 --- a/src/beast/extras/beast/unit_test/main.cpp +++ b/src/ripple/beast/unit_test/main.cpp @@ -5,12 +5,12 @@ // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) // -#include -#include -#include -#include -#include -#include +#include +#include +#include +#include +#include +#include #include #include #include @@ -18,74 +18,66 @@ #include #ifdef BOOST_MSVC -# ifndef WIN32_LEAN_AND_MEAN // VC_EXTRALEAN -# define WIN32_LEAN_AND_MEAN -# include -# undef WIN32_LEAN_AND_MEAN -# else -# include -# endif +#ifndef WIN32_LEAN_AND_MEAN // VC_EXTRALEAN +#define WIN32_LEAN_AND_MEAN +#include +#undef WIN32_LEAN_AND_MEAN +#else +#include +#endif #endif namespace beast { namespace unit_test { -static -std::string +static std::string prefix(suite_info const& s) { - if(s.manual()) + if (s.manual()) return "|M| "; return " "; } -static -void +static void print(std::ostream& os, suite_list const& c) { std::size_t manual = 0; - for(auto const& s : c) + for (auto const& s : c) { os << prefix(s) << s.full_name() << '\n'; - if(s.manual()) + if (s.manual()) ++manual; } - os << - amount(c.size(), "suite") << " total, " << - amount(manual, "manual suite") << - '\n' - ; + os << amount(c.size(), "suite") << " total, " + << amount(manual, "manual suite") << '\n'; } // Print the list of suites // Used with the --print command line option -static -void +static void print(std::ostream& os) { os << "------------------------------------------\n"; print(os, global_suites()); - os << "------------------------------------------" << - std::endl; + os << "------------------------------------------" << std::endl; } -} // unit_test -} // beast +} // namespace unit_test +} // namespace beast // Simple main used to produce stand // alone executables that run unit tests. -int main(int ac, char const* av[]) +int +main(int ac, char const* av[]) { using namespace std; using namespace beast::unit_test; namespace po = boost::program_options; po::options_description desc("Options"); - desc.add_options() - ("help,h", "Produce a help message") - ("print,p", "Print the list of available test suites") - ("suites,s", po::value(), "suites to run") - ; + desc.add_options()("help,h", "Produce a help message")( + "print,p", "Print the list of available test suites")( + "suites,s", po::value(), "suites to run"); po::positional_options_description p; po::variables_map vm; @@ -95,27 +87,26 @@ int main(int ac, char const* av[]) dstream log(std::cerr); std::unitbuf(log); - if(vm.count("help")) + if (vm.count("help")) { log << desc << std::endl; } - else if(vm.count("print")) + else if (vm.count("print")) { print(log); } else { std::string suites; - if(vm.count("suites") > 0) + if (vm.count("suites") > 0) suites = vm["suites"].as(); reporter r(log); bool failed; - if(! suites.empty()) - failed = r.run_each_if(global_suites(), - match_auto(suites)); + if (!suites.empty()) + failed = r.run_each_if(global_suites(), match_auto(suites)); else failed = r.run_each(global_suites()); - if(failed) + if (failed) return EXIT_FAILURE; return EXIT_SUCCESS; } diff --git a/src/beast/extras/beast/unit_test/match.hpp b/src/ripple/beast/unit_test/match.hpp similarity index 98% rename from src/beast/extras/beast/unit_test/match.hpp rename to src/ripple/beast/unit_test/match.hpp index 7c164b7b1..859eb1f71 100644 --- a/src/beast/extras/beast/unit_test/match.hpp +++ b/src/ripple/beast/unit_test/match.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_MATCH_HPP #define BEAST_UNIT_TEST_MATCH_HPP -#include +#include #include namespace beast { diff --git a/src/beast/extras/beast/unit_test/recorder.hpp b/src/ripple/beast/unit_test/recorder.hpp similarity index 94% rename from src/beast/extras/beast/unit_test/recorder.hpp rename to src/ripple/beast/unit_test/recorder.hpp index dd5fcbe9a..c7c543889 100644 --- a/src/beast/extras/beast/unit_test/recorder.hpp +++ b/src/ripple/beast/unit_test/recorder.hpp @@ -8,8 +8,8 @@ #ifndef BEAST_UNIT_TEST_RECORDER_HPP #define BEAST_UNIT_TEST_RECORDER_HPP -#include -#include +#include +#include namespace beast { namespace unit_test { diff --git a/src/beast/extras/beast/unit_test/reporter.hpp b/src/ripple/beast/unit_test/reporter.hpp similarity index 98% rename from src/beast/extras/beast/unit_test/reporter.hpp rename to src/ripple/beast/unit_test/reporter.hpp index cb2e33177..844c321a0 100644 --- a/src/beast/extras/beast/unit_test/reporter.hpp +++ b/src/ripple/beast/unit_test/reporter.hpp @@ -8,8 +8,8 @@ #ifndef BEAST_UNIT_TEST_REPORTER_HPP #define BEAST_UNIT_TEST_REPORTER_HPP -#include -#include +#include +#include #include #include #include diff --git a/src/beast/extras/beast/unit_test/results.hpp b/src/ripple/beast/unit_test/results.hpp similarity index 98% rename from src/beast/extras/beast/unit_test/results.hpp rename to src/ripple/beast/unit_test/results.hpp index 6ae014af5..58d6e9c8a 100644 --- a/src/beast/extras/beast/unit_test/results.hpp +++ b/src/ripple/beast/unit_test/results.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_RESULTS_HPP #define BEAST_UNIT_TEST_RESULTS_HPP -#include +#include #include #include diff --git a/src/beast/extras/beast/unit_test/runner.hpp b/src/ripple/beast/unit_test/runner.hpp similarity index 99% rename from src/beast/extras/beast/unit_test/runner.hpp rename to src/ripple/beast/unit_test/runner.hpp index 366ee3e37..17fa0b9d7 100644 --- a/src/beast/extras/beast/unit_test/runner.hpp +++ b/src/ripple/beast/unit_test/runner.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_RUNNER_H_INCLUDED #define BEAST_UNIT_TEST_RUNNER_H_INCLUDED -#include +#include #include #include #include diff --git a/src/beast/extras/beast/unit_test/suite.hpp b/src/ripple/beast/unit_test/suite.hpp similarity index 99% rename from src/beast/extras/beast/unit_test/suite.hpp rename to src/ripple/beast/unit_test/suite.hpp index c4e0cf26c..befc9a281 100644 --- a/src/beast/extras/beast/unit_test/suite.hpp +++ b/src/ripple/beast/unit_test/suite.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_SUITE_HPP #define BEAST_UNIT_TEST_SUITE_HPP -#include +#include #include #include #include @@ -684,7 +684,7 @@ run(runner& r) #define BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Class,Module,Library,Priority) #else -#include +#include #define BEAST_DEFINE_TESTSUITE(Class,Module,Library) \ BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,false,0) #define BEAST_DEFINE_TESTSUITE_MANUAL(Class,Module,Library) \ diff --git a/src/beast/extras/beast/unit_test/suite_info.hpp b/src/ripple/beast/unit_test/suite_info.hpp similarity index 100% rename from src/beast/extras/beast/unit_test/suite_info.hpp rename to src/ripple/beast/unit_test/suite_info.hpp diff --git a/src/beast/extras/beast/unit_test/suite_list.hpp b/src/ripple/beast/unit_test/suite_list.hpp similarity index 93% rename from src/beast/extras/beast/unit_test/suite_list.hpp rename to src/ripple/beast/unit_test/suite_list.hpp index 41526a2d0..1a7e241e9 100644 --- a/src/beast/extras/beast/unit_test/suite_list.hpp +++ b/src/ripple/beast/unit_test/suite_list.hpp @@ -8,8 +8,8 @@ #ifndef BEAST_UNIT_TEST_SUITE_LIST_HPP #define BEAST_UNIT_TEST_SUITE_LIST_HPP -#include -#include +#include +#include #include #include #include @@ -76,4 +76,3 @@ suite_list::insert( } // beast #endif - diff --git a/src/beast/extras/beast/unit_test/thread.hpp b/src/ripple/beast/unit_test/thread.hpp similarity index 98% rename from src/beast/extras/beast/unit_test/thread.hpp rename to src/ripple/beast/unit_test/thread.hpp index 8f3263af3..9f02ba5ee 100644 --- a/src/beast/extras/beast/unit_test/thread.hpp +++ b/src/ripple/beast/unit_test/thread.hpp @@ -8,7 +8,7 @@ #ifndef BEAST_UNIT_TEST_THREAD_HPP #define BEAST_UNIT_TEST_THREAD_HPP -#include +#include #include #include #include diff --git a/src/ripple/protocol/KnownFormats.h b/src/ripple/protocol/KnownFormats.h index aa99d2ee9..b59742259 100644 --- a/src/ripple/protocol/KnownFormats.h +++ b/src/ripple/protocol/KnownFormats.h @@ -21,10 +21,10 @@ #define RIPPLE_PROTOCOL_KNOWNFORMATS_H_INCLUDED #include +#include #include #include #include -#include #include namespace ripple { diff --git a/src/test/basics/DetectCrash_test.cpp b/src/test/basics/DetectCrash_test.cpp index c02100117..300c94852 100644 --- a/src/test/basics/DetectCrash_test.cpp +++ b/src/test/basics/DetectCrash_test.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include #include diff --git a/src/test/beast/beast_io_latency_probe_test.cpp b/src/test/beast/beast_io_latency_probe_test.cpp index 6baf5b986..9ae6a1341 100644 --- a/src/test/beast/beast_io_latency_probe_test.cpp +++ b/src/test/beast/beast_io_latency_probe_test.cpp @@ -19,7 +19,7 @@ #include #include -#include +#include #include #include diff --git a/src/test/beast/define_print.cpp b/src/test/beast/define_print.cpp index dbfb5243a..af51aeee5 100644 --- a/src/test/beast/define_print.cpp +++ b/src/test/beast/define_print.cpp @@ -5,9 +5,9 @@ // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) // -#include -#include -#include +#include +#include +#include #include // Include this .cpp in your project to gain access to the printing suite diff --git a/src/test/nodestore/Timing_test.cpp b/src/test/nodestore/Timing_test.cpp index 8694631a0..931ad7988 100644 --- a/src/test/nodestore/Timing_test.cpp +++ b/src/test/nodestore/Timing_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/server/ServerStatus_test.cpp b/src/test/server/ServerStatus_test.cpp index c6548bc81..684532c4f 100644 --- a/src/test/server/ServerStatus_test.cpp +++ b/src/test/server/ServerStatus_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -30,7 +31,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/unit_test/multi_runner.cpp b/src/test/unit_test/multi_runner.cpp index 6659301ad..5cd69111b 100644 --- a/src/test/unit_test/multi_runner.cpp +++ b/src/test/unit_test/multi_runner.cpp @@ -19,7 +19,7 @@ #include -#include +#include #include diff --git a/src/test/unit_test/multi_runner.h b/src/test/unit_test/multi_runner.h index e9c1ed765..57d6d33f9 100644 --- a/src/test/unit_test/multi_runner.h +++ b/src/test/unit_test/multi_runner.h @@ -20,9 +20,9 @@ #ifndef TEST_UNIT_TEST_MULTI_RUNNER_H #define TEST_UNIT_TEST_MULTI_RUNNER_H +#include +#include #include -#include -#include #include #include From 297def5ed3e0f543ab1c516c82f242d1015e775c Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Mon, 20 Dec 2021 21:50:17 -0800 Subject: [PATCH 16/19] Update boost max to 1.77 --- Builds/CMake/deps/FindBoost.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Builds/CMake/deps/FindBoost.cmake b/Builds/CMake/deps/FindBoost.cmake index b100f09ae..121e72641 100644 --- a/Builds/CMake/deps/FindBoost.cmake +++ b/Builds/CMake/deps/FindBoost.cmake @@ -969,7 +969,7 @@ function(_Boost_COMPONENT_DEPENDENCIES component _ret) set(_Boost_WAVE_DEPENDENCIES filesystem serialization thread chrono date_time atomic) set(_Boost_WSERIALIZATION_DEPENDENCIES serialization) endif() - if(NOT Boost_VERSION_STRING VERSION_LESS 1.71.0) + if(NOT Boost_VERSION_STRING VERSION_LESS 1.77.0) message(WARNING "New Boost version may have incorrect or missing dependencies and imported targets") endif() endif() From eb576790859d6b66b4ddd792f9bcb5aa671d6c4d Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Mon, 28 Feb 2022 12:48:43 -0800 Subject: [PATCH 17/19] Update cmake_minimum_required to 3.16, Ubuntu 20.04's version --- Builds/CMake/RippledSanity.cmake | 8 ++++---- CMakeLists.txt | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Builds/CMake/RippledSanity.cmake b/Builds/CMake/RippledSanity.cmake index 20a61796c..4aaa2e5f8 100644 --- a/Builds/CMake/RippledSanity.cmake +++ b/Builds/CMake/RippledSanity.cmake @@ -39,14 +39,14 @@ endif () if ("${CMAKE_CXX_COMPILER_ID}" MATCHES ".*Clang") # both Clang and AppleClang set (is_clang TRUE) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" AND - CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0) - message (FATAL_ERROR "This project requires clang 7 or later") + CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0) + message (FATAL_ERROR "This project requires clang 8 or later") endif () # TODO min AppleClang version check ? elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set (is_gcc TRUE) - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0) - message (FATAL_ERROR "This project requires GCC 7 or later") + if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0) + message (FATAL_ERROR "This project requires GCC 8 or later") endif () endif () if (CMAKE_GENERATOR STREQUAL "Xcode") diff --git a/CMakeLists.txt b/CMakeLists.txt index 3c369d980..ade87d349 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required (VERSION 3.9.0) +cmake_minimum_required (VERSION 3.16) if (POLICY CMP0074) cmake_policy(SET CMP0074 NEW) From 6a8180c967198a7e5c7baed756898f28a707f013 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Thu, 3 Mar 2022 16:32:29 -0800 Subject: [PATCH 18/19] Use 3.16 as min cmake --- .travis.yml | 2 +- src/ripple/protocol/STInteger.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 73c995aa0..d8cbf4344 100644 --- a/.travis.yml +++ b/.travis.yml @@ -282,7 +282,7 @@ matrix: env: - MATRIX_EVAL="CC=gcc-8 && CXX=g++-8" - BUILD_TYPE=Debug - - CMAKE_EXE=/opt/local/cmake-3.9/bin/cmake + - CMAKE_EXE=/opt/local/cmake/bin/cmake - SKIP_TESTS=true # validator keys project as subproj of rippled - <<: *linux diff --git a/src/ripple/protocol/STInteger.h b/src/ripple/protocol/STInteger.h index 9d385fd98..fcd007e45 100644 --- a/src/ripple/protocol/STInteger.h +++ b/src/ripple/protocol/STInteger.h @@ -72,7 +72,7 @@ private: STBase* move(std::size_t n, void* buf) override; - friend class detail::STVar; + friend class ripple::detail::STVar; }; using STUInt8 = STInteger; From 1a8eb5e6e3b1c844ae36bb3b7d363d76269ea450 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sat, 22 Jan 2022 20:39:15 -0800 Subject: [PATCH 19/19] Set version to 1.9.0-b1 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 79b40a849..cb749b481 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.8.5" +char const* const versionString = "1.9.0-b1" // clang-format on #if defined(DEBUG) || defined(SANITIZER)