Modernize base_uint:

*  Add construction and assignment from a generic
   contiguous container.  Both compile-time and run time
   safety checks are made to ensure the safety of this
   conversion.

*  Remove base_uint::copyFrom.  The generic copy assignment
   operator now does this functionality with enhanced
   safety and better syntax.

*  Remove construction from and dedendence on Blob.
   The generic constructor and assignment now handle this
   functionality.

*  Fix client code to adhere to this new API.

*  Removed the use of fromVoid in PeerImp.cpp as it was
   an inappropriate use of this dangerous API.  The
   generic container constructors do it with enhanced
   safety and better syntax.

*  Rename data member pn to data_ and make it private.

*  Remove constraint from hash_append

*  Remove array_type alias
This commit is contained in:
Howard Hinnant
2019-05-23 19:29:25 -04:00
committed by Manoj doshi
parent de99e79bf1
commit 773dcd1d48
18 changed files with 105 additions and 74 deletions

View File

@@ -114,14 +114,10 @@ void OrderBookDB::update(
sle->getFieldH256 (sfRootIndex) == sle->key()) sle->getFieldH256 (sfRootIndex) == sle->key())
{ {
Book book; Book book;
book.in.currency.copyFrom(sle->getFieldH160( book.in.currency = sle->getFieldH160(sfTakerPaysCurrency);
sfTakerPaysCurrency)); book.in.account = sle->getFieldH160(sfTakerPaysIssuer);
book.in.account.copyFrom(sle->getFieldH160 ( book.out.account = sle->getFieldH160(sfTakerGetsIssuer);
sfTakerPaysIssuer)); book.out.currency = sle->getFieldH160(sfTakerGetsCurrency);
book.out.account.copyFrom(sle->getFieldH160(
sfTakerGetsIssuer));
book.out.currency.copyFrom (sle->getFieldH160(
sfTakerGetsCurrency));
uint256 index = getBookBase (book); uint256 index = getBookBase (book);
if (seen.insert (index).second) if (seen.insert (index).second)

View File

@@ -51,7 +51,7 @@ public:
void restart (bool multiQuality) void restart (bool multiQuality)
{ {
if (multiQuality) if (multiQuality)
current = 0; // Restart book searching. current = beast::zero; // Restart book searching.
else else
restartNeeded = true; // Restart at same quality. restartNeeded = true; // Restart at same quality.
} }
@@ -61,8 +61,8 @@ public:
if (current != beast::zero) if (current != beast::zero)
return false; return false;
current.copyFrom (getBookBase (book)); current = getBookBase(book);
next.copyFrom (getQualityNext (current)); next = getQualityNext(current);
// TODO(tom): it seems impossible that any actual offers with // TODO(tom): it seems impossible that any actual offers with
// quality == 0 could occur - we should disallow them, and clear // quality == 0 could occur - we should disallow them, and clear

View File

@@ -89,7 +89,7 @@ TER PathCursor::advanceNode (bool const bReverse) const
JLOG (j_.trace()) JLOG (j_.trace())
<< "advanceNode: No more offers."; << "advanceNode: No more offers.";
node().offerIndex_ = 0; node().offerIndex_ = beast::zero;
break; break;
} }
else else
@@ -177,7 +177,7 @@ TER PathCursor::advanceNode (bool const bReverse) const
{ {
// Ran off end of offers. // Ran off end of offers.
node().bEntryAdvance = false; // Done. node().bEntryAdvance = false; // Done.
node().offerIndex_ = 0; // Report no more entries. node().offerIndex_ = beast::zero; // Report no more entries.
} }
} }
else else

View File

@@ -25,9 +25,9 @@
#ifndef RIPPLE_BASICS_BASE_UINT_H_INCLUDED #ifndef RIPPLE_BASICS_BASE_UINT_H_INCLUDED
#define RIPPLE_BASICS_BASE_UINT_H_INCLUDED #define RIPPLE_BASICS_BASE_UINT_H_INCLUDED
#include <ripple/basics/Blob.h>
#include <ripple/basics/strHex.h> #include <ripple/basics/strHex.h>
#include <ripple/basics/hardened_hash.h> #include <ripple/basics/hardened_hash.h>
#include <ripple/beast/cxx17/type_traits.h>
#include <ripple/beast/utility/Zero.h> #include <ripple/beast/utility/Zero.h>
#include <boost/endian/conversion.hpp> #include <boost/endian/conversion.hpp>
#include <boost/functional/hash.hpp> #include <boost/functional/hash.hpp>
@@ -37,6 +37,27 @@
namespace ripple { namespace ripple {
namespace detail
{
template <class Container, class = std::void_t<>>
struct is_contiguous_container
: std::false_type
{};
template <class Container>
struct is_contiguous_container<Container,
std::void_t
<
decltype(std::declval<Container const>().size()),
decltype(std::declval<Container const>().data()),
typename Container::value_type
>>
: std::true_type
{};
} // namespace detail
// This class stores its values internally in big-endian form // This class stores its values internally in big-endian form
template <std::size_t Bits, class Tag = void> template <std::size_t Bits, class Tag = void>
@@ -48,14 +69,12 @@ class base_uint
static_assert (Bits >= 64, static_assert (Bits >= 64,
"The length of a base_uint in bits must be at least 64."); "The length of a base_uint in bits must be at least 64.");
protected:
static constexpr std::size_t WIDTH = Bits / 32; static constexpr std::size_t WIDTH = Bits / 32;
// This is really big-endian in byte order. // This is really big-endian in byte order.
// We sometimes use std::uint32_t for speed. // We sometimes use std::uint32_t for speed.
using array_type = std::array<std::uint32_t, WIDTH>; std::array<std::uint32_t, WIDTH> data_;
array_type pn;
public: public:
//-------------------------------------------------------------------------- //--------------------------------------------------------------------------
@@ -64,7 +83,7 @@ public:
// //
static std::size_t constexpr bytes = Bits / 8; static std::size_t constexpr bytes = Bits / 8;
static_assert(sizeof(array_type) == bytes, ""); static_assert(sizeof(data_) == bytes, "");
using size_type = std::size_t; using size_type = std::size_t;
using difference_type = std::ptrdiff_t; using difference_type = std::ptrdiff_t;
@@ -79,8 +98,8 @@ public:
using const_reverse_iterator = std::reverse_iterator<const_iterator>; using const_reverse_iterator = std::reverse_iterator<const_iterator>;
using tag_type = Tag; using tag_type = Tag;
pointer data() { return reinterpret_cast<pointer>(pn.data ()); } pointer data() { return reinterpret_cast<pointer>(data_.data ()); }
const_pointer data() const { return reinterpret_cast<const_pointer>(pn.data ()); } const_pointer data() const { return reinterpret_cast<const_pointer>(data_.data ()); }
iterator begin() { return data(); } iterator begin() { return data(); }
@@ -112,7 +131,7 @@ private:
explicit base_uint (void const* data, VoidHelper) explicit base_uint (void const* data, VoidHelper)
{ {
memcpy (pn.data (), data, bytes); memcpy (data_.data (), data, bytes);
} }
public: public:
@@ -126,25 +145,35 @@ public:
*this = beast::zero; *this = beast::zero;
} }
explicit base_uint (Blob const& vch)
{
assert (vch.size () == size ());
if (vch.size () == size ())
memcpy (pn.data (), vch.data (), size ());
else
*this = beast::zero;
}
explicit base_uint (std::uint64_t b) explicit base_uint (std::uint64_t b)
{ {
*this = b; *this = b;
} }
template <class OtherTag> template <class Container,
void copyFrom (base_uint<Bits, OtherTag> const& other) class = std::enable_if_t
<
detail::is_contiguous_container<Container>::value &&
std::is_trivially_copyable<typename Container::value_type>::value
>>
explicit base_uint(Container const& c)
{ {
memcpy (pn.data (), other.data(), bytes); assert(c.size()*sizeof(typename Container::value_type) == size());
std::memcpy(data_.data(), c.data(), size());
}
template <class Container>
std::enable_if_t
<
detail::is_contiguous_container<Container>::value &&
std::is_trivially_copyable<typename Container::value_type>::value,
base_uint&
>
operator=(Container const& c)
{
assert(c.size()*sizeof(typename Container::value_type) == size());
std::memcpy(data_.data(), c.data(), size());
return *this;
} }
/* Construct from a raw pointer. /* Construct from a raw pointer.
@@ -159,7 +188,7 @@ public:
int signum() const int signum() const
{ {
for (int i = 0; i < WIDTH; i++) for (int i = 0; i < WIDTH; i++)
if (pn[i] != 0) if (data_[i] != 0)
return 1; return 1;
return 0; return 0;
@@ -175,7 +204,7 @@ public:
base_uint ret; base_uint ret;
for (int i = 0; i < WIDTH; i++) for (int i = 0; i < WIDTH; i++)
ret.pn[i] = ~pn[i]; ret.data_[i] = ~data_[i];
return ret; return ret;
} }
@@ -190,15 +219,15 @@ public:
}; };
// Put in least significant bits. // Put in least significant bits.
ul = boost::endian::native_to_big(uHost); ul = boost::endian::native_to_big(uHost);
pn[WIDTH-2] = u[0]; data_[WIDTH-2] = u[0];
pn[WIDTH-1] = u[1]; data_[WIDTH-1] = u[1];
return *this; return *this;
} }
base_uint& operator^= (const base_uint& b) base_uint& operator^= (const base_uint& b)
{ {
for (int i = 0; i < WIDTH; i++) for (int i = 0; i < WIDTH; i++)
pn[i] ^= b.pn[i]; data_[i] ^= b.data_[i];
return *this; return *this;
} }
@@ -206,7 +235,7 @@ public:
base_uint& operator&= (const base_uint& b) base_uint& operator&= (const base_uint& b)
{ {
for (int i = 0; i < WIDTH; i++) for (int i = 0; i < WIDTH; i++)
pn[i] &= b.pn[i]; data_[i] &= b.data_[i];
return *this; return *this;
} }
@@ -214,7 +243,7 @@ public:
base_uint& operator|= (const base_uint& b) base_uint& operator|= (const base_uint& b)
{ {
for (int i = 0; i < WIDTH; i++) for (int i = 0; i < WIDTH; i++)
pn[i] |= b.pn[i]; data_[i] |= b.data_[i];
return *this; return *this;
} }
@@ -224,9 +253,9 @@ public:
// prefix operator // prefix operator
for (int i = WIDTH - 1; i >= 0; --i) for (int i = WIDTH - 1; i >= 0; --i)
{ {
pn[i] = boost::endian::native_to_big (boost::endian::big_to_native(pn[i]) + 1); data_[i] = boost::endian::native_to_big
(boost::endian::big_to_native(data_[i]) + 1);
if (pn[i] != 0) if (data_[i] != 0)
break; break;
} }
@@ -246,8 +275,9 @@ public:
{ {
for (int i = WIDTH - 1; i >= 0; --i) for (int i = WIDTH - 1; i >= 0; --i)
{ {
auto prev = pn[i]; auto prev = data_[i];
pn[i] = boost::endian::native_to_big (boost::endian::big_to_native(pn[i]) - 1); data_[i] =
boost::endian::native_to_big(boost::endian::big_to_native(data_[i]) - 1);
if (prev != 0) if (prev != 0)
break; break;
@@ -271,23 +301,22 @@ public:
for (int i = WIDTH; i--;) for (int i = WIDTH; i--;)
{ {
std::uint64_t n = carry + boost::endian::big_to_native(pn[i]) + std::uint64_t n = carry + boost::endian::big_to_native(data_[i]) +
boost::endian::big_to_native(b.pn[i]); boost::endian::big_to_native(b.data_[i]);
pn[i] = boost::endian::native_to_big (static_cast<std::uint32_t>(n)); data_[i] = boost::endian::native_to_big (static_cast<std::uint32_t>(n));
carry = n >> 32; carry = n >> 32;
} }
return *this; return *this;
} }
template <class Hasher, template <class Hasher>
class = std::enable_if_t<Hasher::endian != beast::endian::native>>
friend void hash_append( friend void hash_append(
Hasher& h, base_uint const& a) noexcept Hasher& h, base_uint const& a) noexcept
{ {
// Do not allow any endian transformations on this memory // Do not allow any endian transformations on this memory
h(a.pn.data (), sizeof(a.pn)); h(a.data_.data (), sizeof(a.data_));
} }
/** Parse a hex string into a base_uint /** Parse a hex string into a base_uint
@@ -298,7 +327,7 @@ public:
{ {
unsigned char* pOut = begin (); unsigned char* pOut = begin ();
for (int i = 0; i < sizeof (pn); ++i) for (int i = 0; i < sizeof (data_); ++i)
{ {
auto hi = charUnHex(*psz++); auto hi = charUnHex(*psz++);
if (hi == -1) if (hi == -1)
@@ -391,7 +420,7 @@ public:
base_uint<Bits, Tag>& operator=(beast::Zero) base_uint<Bits, Tag>& operator=(beast::Zero)
{ {
pn.fill(0); data_.fill(0);
return *this; return *this;
} }

View File

@@ -26,6 +26,7 @@
#define RIPPLE_CRYPTO_GENERATEDETERMINISTICKEY_H_INCLUDED #define RIPPLE_CRYPTO_GENERATEDETERMINISTICKEY_H_INCLUDED
#include <ripple/basics/base_uint.h> #include <ripple/basics/base_uint.h>
#include <ripple/basics/Blob.h>
namespace ripple { namespace ripple {

View File

@@ -20,6 +20,7 @@
#ifndef RIPPLE_NODESTORE_NODEOBJECT_H_INCLUDED #ifndef RIPPLE_NODESTORE_NODEOBJECT_H_INCLUDED
#define RIPPLE_NODESTORE_NODEOBJECT_H_INCLUDED #define RIPPLE_NODESTORE_NODEOBJECT_H_INCLUDED
#include <ripple/basics/Blob.h>
#include <ripple/basics/CountedObject.h> #include <ripple/basics/CountedObject.h>
#include <ripple/protocol/Protocol.h> #include <ripple/protocol/Protocol.h>

View File

@@ -1578,8 +1578,8 @@ PeerImp::onMessage (std::shared_ptr <protocol::TMProposeSet> const& m)
return; return;
} }
auto const proposeHash = uint256::fromVoid(set.currenttxhash().data()); uint256 const proposeHash{set.currenttxhash()};
auto const prevLedger = uint256::fromVoid(set.previousledger().data()); uint256 const prevLedger{set.previousledger()};
PublicKey const publicKey {makeSlice(set.nodepubkey())}; PublicKey const publicKey {makeSlice(set.nodepubkey())};
NetClock::time_point const closeTime { NetClock::duration{set.closetime()} }; NetClock::time_point const closeTime { NetClock::duration{set.closetime()} };

View File

@@ -34,7 +34,7 @@ private:
// But an STAccount is always 160 bits, so we can store it with less // But an STAccount is always 160 bits, so we can store it with less
// overhead in a ripple::uint160. However, so the serialized format of the // overhead in a ripple::uint160. However, so the serialized format of the
// STAccount stays unchanged, we serialize and deserialize like an STBlob. // STAccount stays unchanged, we serialize and deserialize like an STBlob.
uint160 value_; AccountID value_;
bool default_; bool default_;
public: public:
@@ -101,14 +101,12 @@ public:
AccountID AccountID
value() const noexcept value() const noexcept
{ {
AccountID result; return value_;
result.copyFrom (value_);
return result;
} }
void setValue (AccountID const& v) void setValue (AccountID const& v)
{ {
value_.copyFrom (v); value_ = v;
default_ = false; default_ = false;
} }
}; };

View File

@@ -102,7 +102,7 @@ public:
template <typename Tag> template <typename Tag>
void setValue (base_uint<Bits, Tag> const& v) void setValue (base_uint<Bits, Tag> const& v)
{ {
value_.copyFrom(v); value_ = v;
} }
value_type const& value_type const&

View File

@@ -21,6 +21,7 @@
#define RIPPLE_PROTOCOL_SERIALIZER_H_INCLUDED #define RIPPLE_PROTOCOL_SERIALIZER_H_INCLUDED
#include <ripple/basics/base_uint.h> #include <ripple/basics/base_uint.h>
#include <ripple/basics/Blob.h>
#include <ripple/basics/contract.h> #include <ripple/basics/contract.h>
#include <ripple/basics/Buffer.h> #include <ripple/basics/Buffer.h>
#include <ripple/basics/safe_cast.h> #include <ripple/basics/safe_cast.h>

View File

@@ -148,7 +148,7 @@ calcAccountID (PublicKey const& pk)
AccountID const& AccountID const&
xrpAccount() xrpAccount()
{ {
static AccountID const account(0); static AccountID const account(beast::zero);
return account; return account;
} }

View File

@@ -60,9 +60,9 @@ STAccount::STAccount (SerialIter& sit, SField const& name)
STAccount::STAccount (SField const& n, AccountID const& v) STAccount::STAccount (SField const& n, AccountID const& v)
: STBase (n) : STBase (n)
, value_(v)
, default_ (false) , default_ (false)
{ {
value_.copyFrom (v);
} }
std::string STAccount::getText () const std::string STAccount::getText () const

View File

@@ -106,12 +106,12 @@ STAmount::STAmount(SerialIter& sit, SField const& name)
} }
Issue issue; Issue issue;
issue.currency.copyFrom (sit.get160 ()); issue.currency = sit.get160();
if (isXRP (issue.currency)) if (isXRP (issue.currency))
Throw<std::runtime_error> ("invalid native currency"); Throw<std::runtime_error> ("invalid native currency");
issue.account.copyFrom (sit.get160 ()); issue.account = sit.get160();
if (isXRP (issue.account)) if (isXRP (issue.account))
Throw<std::runtime_error> ("invalid native account"); Throw<std::runtime_error> ("invalid native account");

View File

@@ -91,13 +91,13 @@ STPathSet::STPathSet (SerialIter& sit, SField const& name)
AccountID issuer; AccountID issuer;
if (hasAccount) if (hasAccount)
account.copyFrom (sit.get160 ()); account = sit.get160();
if (hasCurrency) if (hasCurrency)
currency.copyFrom (sit.get160 ()); currency = sit.get160();
if (hasIssuer) if (hasIssuer)
issuer.copyFrom (sit.get160 ()); issuer = sit.get160();
path.emplace_back (account, currency, issuer, hasCurrency); path.emplace_back (account, currency, issuer, hasCurrency);
} }

View File

@@ -110,7 +110,7 @@ Currency to_currency(std::string const& code)
Currency const& xrpCurrency() Currency const& xrpCurrency()
{ {
static Currency const currency(0); static Currency const currency(beast::zero);
return currency; return currency;
} }

View File

@@ -18,9 +18,11 @@
//============================================================================== //==============================================================================
#include <ripple/basics/base_uint.h> #include <ripple/basics/base_uint.h>
#include <ripple/basics/Blob.h>
#include <ripple/basics/hardened_hash.h> #include <ripple/basics/hardened_hash.h>
#include <ripple/beast/unit_test.h> #include <ripple/beast/unit_test.h>
#include <boost/algorithm/string.hpp> #include <boost/algorithm/string.hpp>
#include <complex>
#include <type_traits> #include <type_traits>
@@ -57,6 +59,8 @@ struct base_uint_test : beast::unit_test::suite
void run() override void run() override
{ {
static_assert(!std::is_constructible<test96, std::complex<double>>::value, "");
static_assert(!std::is_assignable<test96&, std::complex<double>>::value, "");
// used to verify set insertion (hashing required) // used to verify set insertion (hashing required)
std::unordered_set<test96, hardened_hash<>> uset; std::unordered_set<test96, hardened_hash<>> uset;

View File

@@ -77,7 +77,7 @@ public:
{ {
blob b; blob b;
hex_to_binary (s.begin (), s.end (), b); hex_to_binary (s.begin (), s.end (), b);
return uint256::fromVoid(b.data()); return uint256{b};
} }
static static

View File

@@ -22,6 +22,7 @@
#include <ripple/beast/xor_shift_engine.h> #include <ripple/beast/xor_shift_engine.h>
#include <ripple/beast/unit_test.h> #include <ripple/beast/unit_test.h>
#include <algorithm> #include <algorithm>
#include <array>
#include <chrono> #include <chrono>
#include <cmath> #include <cmath>
#include <numeric> #include <numeric>
@@ -115,12 +116,12 @@ public:
digest_test () digest_test ()
{ {
beast::xor_shift_engine g(19207813); beast::xor_shift_engine g(19207813);
std::uint8_t buf[32]; std::array<std::uint8_t, 32> buf;
for (int i = 0; i < 1000000; i++) for (int i = 0; i < 1000000; i++)
{ {
beast::rngfill (buf, sizeof(buf), g); beast::rngfill (buf.data(), buf.size(), g);
dataset1.push_back (uint256::fromVoid (buf)); dataset1.push_back (uint256{buf});
} }
} }