From 289bc7deb335d8493f00c27578ee3ab4273217ed Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 4 Nov 2015 18:18:26 -0800 Subject: [PATCH] Reduce interface to STAccount (RIPD-994): Since a non-default STAccount is now guaranteed to always be 160 bits, it was possible to reduce the number of methods that it provides. In the process of narrowing the STAccount interface it became reasonable to remove some methods that duplicated functionality. A few classes offered both a value() and a getValue() method. The getValue() method is removed from those classes. --- src/ripple/ledger/impl/TxMeta.cpp | 12 ++--- src/ripple/protocol/STAccount.h | 26 ++-------- src/ripple/protocol/STBitString.h | 6 --- src/ripple/protocol/STBlob.h | 6 --- src/ripple/protocol/STExchange.h | 2 +- src/ripple/protocol/STInteger.h | 6 --- src/ripple/protocol/STObject.h | 8 +-- src/ripple/protocol/impl/STObject.cpp | 53 ++++---------------- src/ripple/protocol/impl/STTx.cpp | 9 ++-- src/ripple/protocol/tests/STAccount.test.cpp | 7 --- 10 files changed, 25 insertions(+), 110 deletions(-) diff --git a/src/ripple/ledger/impl/TxMeta.cpp b/src/ripple/ledger/impl/TxMeta.cpp index af929b77b8..b823350301 100644 --- a/src/ripple/ledger/impl/TxMeta.cpp +++ b/src/ripple/ledger/impl/TxMeta.cpp @@ -136,15 +136,11 @@ TxMeta::getAffectedAccounts() const { for (auto const& field : *inner) { - STAccount const* sa = - dynamic_cast (&field); - - if (sa) + if (auto sa = dynamic_cast (&field)) { - AccountID id; - assert(sa->isValueH160()); - if (sa->getValueH160(id)) - list.insert(id); + assert (! sa->isDefault()); + if (! sa->isDefault()) + list.insert(sa->value()); } else if ((field.getFName () == sfLowLimit) || (field.getFName () == sfHighLimit) || (field.getFName () == sfTakerPays) || (field.getFName () == sfTakerGets)) diff --git a/src/ripple/protocol/STAccount.h b/src/ripple/protocol/STAccount.h index aabe666509..fc0101e61a 100644 --- a/src/ripple/protocol/STAccount.h +++ b/src/ripple/protocol/STAccount.h @@ -22,7 +22,6 @@ #include #include -#include #include namespace ripple { @@ -95,7 +94,7 @@ public: STAccount& operator= (AccountID const& value) { - setValueH160(value); + setValue (value); return *this; } @@ -103,34 +102,15 @@ public: value() const noexcept { AccountID result; - getValueH160(result); + result.copyFrom (value_); return result; } - template - void setValueH160 (base_uint<160, Tag> const& v) + void setValue (AccountID const& v) { value_.copyFrom (v); default_ = false; } - - // VFALCO This is a clumsy interface, it should return - // the value. And it should not be possible to - // have anything other than a uint160 in here. - // The base_uint tag should always be `AccountIDTag`. - template - bool getValueH160 (base_uint<160, Tag>& v) const - { - bool const success = isValueH160(); - if (success) - v.copyFrom (value_); - return success; - } - - bool isValueH160 () const - { - return ! isDefault(); - } }; } // ripple diff --git a/src/ripple/protocol/STBitString.h b/src/ripple/protocol/STBitString.h index c7acf6a19d..1981b9e6df 100644 --- a/src/ripple/protocol/STBitString.h +++ b/src/ripple/protocol/STBitString.h @@ -98,12 +98,6 @@ public: s.addBitString (value_); } - const value_type& - getValue () const - { - return value_; - } - template void setValue (base_uint const& v) { diff --git a/src/ripple/protocol/STBlob.h b/src/ripple/protocol/STBlob.h index 3e4dbcc4bc..a34cfaab59 100644 --- a/src/ripple/protocol/STBlob.h +++ b/src/ripple/protocol/STBlob.h @@ -147,12 +147,6 @@ public: return value_; } - Buffer - getValue () const - { - return Buffer(value_.data (), value_.size ()); - } - void setValue (Buffer&& b) { diff --git a/src/ripple/protocol/STExchange.h b/src/ripple/protocol/STExchange.h index 99a5a10fb3..7b44fb0410 100644 --- a/src/ripple/protocol/STExchange.h +++ b/src/ripple/protocol/STExchange.h @@ -50,7 +50,7 @@ struct STExchange, T> get (boost::optional& t, STInteger const& u) { - t = u.getValue(); + t = u.value(); } static diff --git a/src/ripple/protocol/STInteger.h b/src/ripple/protocol/STInteger.h index 63f5f8a6a3..5f127701fe 100644 --- a/src/ripple/protocol/STInteger.h +++ b/src/ripple/protocol/STInteger.h @@ -71,12 +71,6 @@ public: s.addInteger (value_); } - Integer - getValue () const - { - return value_; - } - STInteger& operator= (value_type const& v) { value_ = v; diff --git a/src/ripple/protocol/STObject.h b/src/ripple/protocol/STObject.h index 53141932f1..892a68871f 100644 --- a/src/ripple/protocol/STObject.h +++ b/src/ripple/protocol/STObject.h @@ -487,7 +487,7 @@ public: operator[](OptionaledField const& of) const; /** Return a modifiable field value. - + Throws: missing_field_error if the field is @@ -585,11 +585,11 @@ private: // Implementation for getting (most) fields that return by value. // // The remove_cv and remove_reference are necessitated by the STBitString - // types. Their getValue returns by const ref. We return those types + // types. Their value() returns by const ref. We return those types // by value. template ().getValue ())>::type >::type > + decltype (std::declval ().value ())>::type >::type > V getFieldByValue (SField const& field) const { const STBase* rf = peekAtPField (field); @@ -607,7 +607,7 @@ private: if (!cf) throw std::runtime_error ("Wrong field type"); - return cf->getValue (); + return cf->value (); } // Implementations for getting (most) fields that return by const reference. diff --git a/src/ripple/protocol/impl/STObject.cpp b/src/ripple/protocol/impl/STObject.cpp index fc226566db..c4d960378f 100644 --- a/src/ripple/protocol/impl/STObject.cpp +++ b/src/ripple/protocol/impl/STObject.cpp @@ -18,18 +18,12 @@ //============================================================================== #include -#include -#include -#include -#include +#include #include -#include #include #include -#include -#include -#include -#include +#include +#include namespace ripple { @@ -431,7 +425,7 @@ bool STObject::setFlag (std::uint32_t f) if (!t) return false; - t->setValue (t->getValue () | f); + t->setValue (t->value () | f); return true; } @@ -442,7 +436,7 @@ bool STObject::clearFlag (std::uint32_t f) if (!t) return false; - t->setValue (t->getValue () & ~f); + t->setValue (t->value () & ~f); return true; } @@ -458,7 +452,7 @@ std::uint32_t STObject::getFlags (void) const if (!t) return 0; - return t->getValue (); + return t->value (); } STBase* STObject::makeFieldPresent (SField const& field) @@ -560,22 +554,7 @@ uint256 STObject::getFieldH256 (SField const& field) const AccountID STObject::getAccountID (SField const& field) const { - auto rf = peekAtPField (field); - if (!rf) - throw std::runtime_error ("Field not found"); - - AccountID account; - if (rf->getSType () != STI_NOTPRESENT) - { - const STAccount* cf = dynamic_cast (rf); - - if (!cf) - throw std::runtime_error ("Wrong field type"); - - cf->getValueH160 (account); - } - - return account; + return getFieldByValue (field); } Blob STObject::getFieldVL (SField const& field) const @@ -670,26 +649,12 @@ void STObject::setFieldV256 (SField const& field, STVector256 const& v) void STObject::setAccountID (SField const& field, AccountID const& v) { - STBase* rf = getPField (field, true); - - if (!rf) - throw std::runtime_error ("Field not found"); - - if (rf->getSType () == STI_NOTPRESENT) - rf = makeFieldPresent (field); - - STAccount* cf = dynamic_cast (rf); - - if (!cf) - throw std::runtime_error ("Wrong field type"); - - cf->setValueH160 (v); + setFieldUsingSetValue (field, v); } void STObject::setFieldVL (SField const& field, Blob const& v) { - setFieldUsingSetValue - (field, Buffer(v.data (), v.size ())); + setFieldUsingSetValue (field, Buffer(v.data (), v.size ())); } void STObject::setFieldAmount (SField const& field, STAmount const& v) diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index ec9d7f387d..506b7942cc 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -134,10 +134,9 @@ STTx::getMentionedAccounts () const { if (auto sa = dynamic_cast (&it)) { - AccountID id; - assert(sa->isValueH160()); - if (sa->getValueH160(id)) - list.insert(id); + assert(! sa->isDefault()); + if (! sa->isDefault()) + list.insert(sa->value()); } else if (auto sa = dynamic_cast (&it)) { @@ -488,7 +487,7 @@ isAccountFieldOkay (STObject const& st) for (int i = 0; i < st.getCount(); ++i) { auto t = dynamic_cast(st.peekAtPIndex (i)); - if (t && !t->isValueH160 ()) + if (t && t->isDefault ()) return false; } diff --git a/src/ripple/protocol/tests/STAccount.test.cpp b/src/ripple/protocol/tests/STAccount.test.cpp index 96f239cf95..8582c30367 100644 --- a/src/ripple/protocol/tests/STAccount.test.cpp +++ b/src/ripple/protocol/tests/STAccount.test.cpp @@ -35,7 +35,6 @@ struct STAccount_test : public beast::unit_test::suite expect (defaultAcct.getText() == ""); expect (defaultAcct.isDefault() == true); expect (defaultAcct.value() == AccountID {}); - expect (! defaultAcct.isValueH160()); { #ifdef NDEBUG // Qualified because the serialization asserts in a debug build. Serializer s; @@ -45,7 +44,6 @@ struct STAccount_test : public beast::unit_test::suite SerialIter sit (s.slice ()); STAccount const deserializedDefault (sit, sfAccount); expect (deserializedDefault.isEquivalent (defaultAcct)); - expect (! deserializedDefault.isValueH160()); #endif // NDEBUG } { @@ -55,7 +53,6 @@ struct STAccount_test : public beast::unit_test::suite SerialIter sit (s.slice ()); STAccount const deserializedDefault (sit, sfAccount); expect (deserializedDefault.isEquivalent (defaultAcct)); - expect (! deserializedDefault.isValueH160()); } // Test constructor from SField. @@ -64,7 +61,6 @@ struct STAccount_test : public beast::unit_test::suite expect (sfAcct.getText() == ""); expect (sfAcct.isDefault()); expect (sfAcct.value() == AccountID {}); - expect (! sfAcct.isValueH160()); expect (sfAcct.isEquivalent (defaultAcct)); { Serializer s; @@ -74,7 +70,6 @@ struct STAccount_test : public beast::unit_test::suite SerialIter sit (s.slice ()); STAccount const deserializedSf (sit, sfAccount); expect (deserializedSf.isEquivalent(sfAcct)); - expect (! deserializedSf.isValueH160()); } // Test constructor from SField and AccountID. @@ -82,7 +77,6 @@ struct STAccount_test : public beast::unit_test::suite expect (zeroAcct.getText() == "rrrrrrrrrrrrrrrrrrrrrhoLvTp"); expect (! zeroAcct.isDefault()); expect (zeroAcct.value() == AccountID {0}); - expect (zeroAcct.isValueH160()); expect (! zeroAcct.isEquivalent (defaultAcct)); expect (! zeroAcct.isEquivalent (sfAcct)); { @@ -94,7 +88,6 @@ struct STAccount_test : public beast::unit_test::suite SerialIter sit (s.slice ()); STAccount const deserializedZero (sit, sfAccount); expect (deserializedZero.isEquivalent (zeroAcct)); - expect (deserializedZero.isValueH160()); } { // Construct from a VL that is not exactly 160 bits.