CBase58Data Refactoring:

* Remove unnecessary functions.
* Load from a base_uint const& - don't use void pointers.
* Free comparison functions.
* Explicitly specify encoding alphabet.
* Miscellaneous cleanups.

src/ripple/types/api/Base58.h

@@ -114,29 +114,23 @@ public:
         return raw_encode (&v.front(), &v.back()+1, alphabet, withCheck);
     }
 
-    // VFALCO NOTE Avoid this interface which uses globals, explicitly
-    //             pass the alphabet in the call to encode!
-    //
-    static Alphabet const& getCurrentAlphabet ();
-    static void setCurrentAlphabet (Alphabet const& alphabet);
-
     template <class Container>
     static std::string encode (Container const& container)
     {
         return encode (container.container.begin(), container.end(),
-            getCurrentAlphabet(), false);
+            getRippleAlphabet(), false);
     }
 
     template <class Container>
     static std::string encodeWithCheck (Container const& container)
     {
         return encode (&container.front(), &container.back()+1,
-            getCurrentAlphabet(), true);
+            getRippleAlphabet(), true);
     }
 
     static std::string encode (const unsigned char* pbegin, const unsigned char* pend)
     {
-        return encode (pbegin, pend, getCurrentAlphabet(), false);
+        return encode (pbegin, pend, getRippleAlphabet(), false);
     }
 
     //--------------------------------------------------------------------------
@@ -146,13 +140,10 @@ public:
     static bool raw_decode (char const* first, char const* last,
         void* dest, std::size_t size, bool checked, Alphabet const& alphabet);
 
-    static bool decode (const char* psz, Blob& vchRet, Alphabet const& alphabet = getCurrentAlphabet ());
+    static bool decode (const char* psz, Blob& vchRet, Alphabet const& alphabet = getRippleAlphabet ());
     static bool decode (const std::string& str, Blob& vchRet);
-    static bool decodeWithCheck (const char* psz, Blob& vchRet, Alphabet const& alphabet = getCurrentAlphabet());
-    static bool decodeWithCheck (const std::string& str, Blob& vchRet, Alphabet const& alphabet = getCurrentAlphabet());
-
-private:
-    static Alphabet const* s_currentAlphabet;
+    static bool decodeWithCheck (const char* psz, Blob& vchRet, Alphabet const& alphabet = getRippleAlphabet());
+    static bool decodeWithCheck (const std::string& str, Blob& vchRet, Alphabet const& alphabet = getRippleAlphabet());
 };
 
 }

src/ripple/types/impl/Base58.cpp

@@ -24,8 +24,6 @@
 
 namespace ripple {
 
-Base58::Alphabet const* Base58::s_currentAlphabet = &Base58::getRippleAlphabet ();
-
 void Base58::fourbyte_hash256 (void* out, void const* in, std::size_t bytes)
 {
     unsigned char const* const p (
@@ -88,16 +86,6 @@ std::string Base58::raw_encode (
     return str;
 }
 
-Base58::Alphabet const& Base58::getCurrentAlphabet ()
-{
-    return *s_currentAlphabet;
-}
-
-void Base58::setCurrentAlphabet (Alphabet const& alphabet)
-{
-    s_currentAlphabet = &alphabet;
-}
-
 //------------------------------------------------------------------------------
 
 bool Base58::raw_decode (char const* first, char const* last, void* dest,

src/ripple_data/crypto/Base58Data.cpp

@@ -39,34 +39,17 @@ CBase58Data::CBase58Data ()
 
 CBase58Data::~CBase58Data ()
 {
-    if (!vchData.empty ())
-        memset (&vchData[0], 0, vchData.size ());
+    // Ensures that any potentially sensitive data is cleared from memory
+    std::fill (std::begin (vchData), std::end(vchData), 0);
 }
 
-void CBase58Data::SetData (int nVersionIn, Blob const& vchDataIn)
-{
-    nVersion    = nVersionIn;
-    vchData     = vchDataIn;
-}
-
-void CBase58Data::SetData (int nVersionIn, const void* pdata, size_t nSize)
-{
-    nVersion = nVersionIn;
-    vchData.resize (nSize);
-
-    if (nSize)
-        memcpy (&vchData[0], pdata, nSize);
-}
-
-void CBase58Data::SetData (int nVersionIn, const unsigned char* pbegin, const unsigned char* pend)
-{
-    SetData (nVersionIn, (void*)pbegin, pend - pbegin);
-}
-
-bool CBase58Data::SetString (const char* psz, unsigned char version, Base58::Alphabet const& alphabet)
+bool CBase58Data::SetString (
+    std::string const& str,
+    unsigned char version, 
+    Base58::Alphabet const& alphabet)
 {
     Blob vchTemp;
-    Base58::decodeWithCheck (psz, vchTemp, alphabet);
+    Base58::decodeWithCheck (str.c_str (), vchTemp, alphabet);
 
     if (vchTemp.empty () || vchTemp[0] != version)
     {
@@ -76,20 +59,15 @@ bool CBase58Data::SetString (const char* psz, unsigned char version, Base58::Alp
     }
 
     nVersion = vchTemp[0];
-    vchData.resize (vchTemp.size () - 1);
 
-    if (!vchData.empty ())
-        memcpy (&vchData[0], &vchTemp[1], vchData.size ());
+    vchData.assign (vchTemp.begin () + 1, vchTemp.end ());
+
+    // Ensures that any potentially sensitive data is cleared from memory
+    std::fill (vchTemp.begin(), vchTemp.end(), 0);
 
-    memset (&vchTemp[0], 0, vchTemp.size ());
     return true;
 }
 
-bool CBase58Data::SetString (const std::string& str, unsigned char version)
-{
-    return SetString (str.c_str (), version);
-}
-
 std::string CBase58Data::ToString () const
 {
     Blob vch (1, nVersion);
@@ -99,44 +77,6 @@ std::string CBase58Data::ToString () const
     return Base58::encodeWithCheck (vch);
 }
 
-int CBase58Data::CompareTo (const CBase58Data& b58) const
-{
-    if (nVersion < b58.nVersion) return -1;
-
-    if (nVersion > b58.nVersion) return  1;
-
-    if (vchData < b58.vchData)   return -1;
-
-    if (vchData > b58.vchData)   return  1;
-
-    return 0;
-}
-
-bool CBase58Data::operator== (const CBase58Data& b58) const
-{
-    return CompareTo (b58) == 0;
-}
-bool CBase58Data::operator!= (const CBase58Data& b58) const
-{
-    return CompareTo (b58) != 0;
-}
-bool CBase58Data::operator<= (const CBase58Data& b58) const
-{
-    return CompareTo (b58) <= 0;
-}
-bool CBase58Data::operator>= (const CBase58Data& b58) const
-{
-    return CompareTo (b58) >= 0;
-}
-bool CBase58Data::operator< (const CBase58Data& b58) const
-{
-    return CompareTo (b58) <  0;
-}
-bool CBase58Data::operator> (const CBase58Data& b58) const
-{
-    return CompareTo (b58) >  0;
-}
-
 std::size_t hash_value (const CBase58Data& b58)
 {
     std::size_t seed = HashMaps::getInstance ().getNonce <size_t> ()

src/ripple_data/crypto/Base58Data.h

@@ -32,40 +32,102 @@
 #ifndef RIPPLE_BASE58DATA_H
 #define RIPPLE_BASE58DATA_H
 
-#include ".././ripple/types/api/Base58.h"
+#include "../../ripple/types/api/Base58.h"
+#include "../../ripple/types/api/base_uint.h"
 
 namespace ripple {
 
 class CBase58Data
 {
 protected:
+    // NIKB TODO: combine nVersion into vchData so that CBase58Data becomes
+    //            unnecessary and is replaced by a couple of helper functions
+    //            that operate on a Blob.
+
     unsigned char nVersion;
     Blob vchData;
 
     CBase58Data ();
     ~CBase58Data ();
 
-    void SetData (int nVersionIn, Blob const& vchDataIn);
-    void SetData (int nVersionIn, const void* pdata, size_t nSize);
-    void SetData (int nVersionIn, const unsigned char* pbegin, const unsigned char* pend);
+    void SetData (int version, Blob const& vchDataIn)
+    {
+        nVersion = version;
+        vchData = vchDataIn;
+    }
+
+    template <size_t Bits, class Tag>
+    void SetData (int version, base_uint<Bits, Tag> const& from)
+    {
+        nVersion = version;
+
+        vchData.resize (from.size ());
+
+        std::copy (std::begin (from), std::end(from), std::begin (vchData));
+    }
 
 public:
-    bool SetString (const char* psz, unsigned char version, Base58::Alphabet const& alphabet = Base58::getCurrentAlphabet ());
-    bool SetString (const std::string& str, unsigned char version);
+    bool SetString (std::string const& str, unsigned char version,
+        Base58::Alphabet const& alphabet);
 
     std::string ToString () const;
-    int CompareTo (const CBase58Data& b58) const;
 
-    bool operator== (const CBase58Data& b58) const;
-    bool operator!= (const CBase58Data& b58) const;
-    bool operator<= (const CBase58Data& b58) const;
-    bool operator>= (const CBase58Data& b58) const;
-    bool operator< (const CBase58Data& b58) const;
-    bool operator> (const CBase58Data& b58) const;
+    int compare (const CBase58Data& b58) const
+    {
+        if (nVersion < b58.nVersion)
+            return -1;
+
+        if (nVersion > b58.nVersion)
+            return  1;
+
+        if (vchData < b58.vchData)
+            return -1;
+
+        if (vchData > b58.vchData)
+            return  1;
+
+        return 0;
+    }
 
     friend std::size_t hash_value (const CBase58Data& b58);
 };
 
+inline bool
+operator== (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) == 0;
+}
+
+inline bool
+operator!= (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) != 0;
+}
+
+inline bool
+operator< (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) <  0;
+}
+
+inline bool
+operator<= (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) <= 0;
+}
+
+inline bool
+operator> (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) > 0;
+}
+
+inline bool
+operator>= (CBase58Data const& lhs, CBase58Data const& rhs)
+{
+    return lhs.compare (rhs) >= 0;
+}
+
 extern std::size_t hash_value (const CBase58Data& b58);
 
 } // ripple

src/ripple_data/crypto/CKeyDeterministic.cpp

@@ -215,7 +215,15 @@ static BIGNUM* makeHash (const RippleAddress& pubGen, int seq, BIGNUM* order)
 EC_KEY* CKey::GeneratePublicDeterministicKey (const RippleAddress& pubGen, int seq)
 {
     // publicKey(n) = rootPublicKey EC_POINT_+ Hash(pubHash|seq)*point
-    EC_KEY*         rootKey     = CKey::GenerateRootPubKey (pubGen.getGeneratorBN ());
+    BIGNUM* generator = BN_bin2bn (
+        pubGen.getGenerator ().data (),
+        pubGen.getGenerator ().size (),
+        nullptr);
+
+    if (generator == nullptr)
+        return nullptr;
+
+    EC_KEY*         rootKey     = CKey::GenerateRootPubKey (generator);
     const EC_POINT* rootPubKey  = EC_KEY_get0_public_key (rootKey);
     BN_CTX*         ctx         = BN_CTX_new ();
     EC_KEY*         pkey        = EC_KEY_new_by_curve_name (NID_secp256k1);

src/ripple_data/protocol/RippleAddress.cpp

@@ -153,7 +153,7 @@ std::string RippleAddress::humanNodePublic () const
 
 bool RippleAddress::setNodePublic (const std::string& strPublic)
 {
-    mIsValid        = SetString (strPublic.c_str (), VER_NODE_PUBLIC);
+    mIsValid = SetString (strPublic, VER_NODE_PUBLIC, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
@@ -256,23 +256,23 @@ std::string RippleAddress::humanNodePrivate () const
 
 bool RippleAddress::setNodePrivate (const std::string& strPrivate)
 {
-    mIsValid        = SetString (strPrivate.c_str (), VER_NODE_PRIVATE);
+    mIsValid = SetString (strPrivate, VER_NODE_PRIVATE, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
 
 void RippleAddress::setNodePrivate (Blob const& vPrivate)
 {
-    mIsValid        = true;
+    mIsValid = true;
 
     SetData (VER_NODE_PRIVATE, vPrivate);
 }
 
 void RippleAddress::setNodePrivate (uint256 hash256)
 {
-    mIsValid        = true;
+    mIsValid = true;
 
-    SetData (VER_NODE_PRIVATE, hash256.begin (), 32);
+    SetData (VER_NODE_PRIVATE, hash256);
 }
 
 void RippleAddress::signNodePrivate (uint256 const& hash, Blob& vchSig) const
@@ -324,7 +324,8 @@ std::string RippleAddress::humanAccountID () const
     case VER_ACCOUNT_ID:
     {
         StaticScopedLockType sl (s_lock);
-        ripple::unordered_map< Blob , std::string >::iterator it = rncMap.find (vchData);
+
+        auto it = rncMap.find (vchData);
 
         if (it != rncMap.end ())
             return it->second;
@@ -363,7 +364,7 @@ bool RippleAddress::setAccountID (const std::string& strAccountID, Base58::Alpha
     }
     else
     {
-        mIsValid    = SetString (strAccountID.c_str (), VER_ACCOUNT_ID, alphabet);
+        mIsValid = SetString (strAccountID, VER_ACCOUNT_ID, alphabet);
     }
 
     return mIsValid;
@@ -373,7 +374,7 @@ void RippleAddress::setAccountID (const uint160& hash160)
 {
     mIsValid        = true;
 
-    SetData (VER_ACCOUNT_ID, hash160.begin (), 20);
+    SetData (VER_ACCOUNT_ID, hash160);
 }
 
 //
@@ -429,14 +430,14 @@ std::string RippleAddress::humanAccountPublic () const
 
 bool RippleAddress::setAccountPublic (const std::string& strPublic)
 {
-    mIsValid        = SetString (strPublic.c_str (), VER_ACCOUNT_PUBLIC);
+    mIsValid = SetString (strPublic, VER_ACCOUNT_PUBLIC, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
 
 void RippleAddress::setAccountPublic (Blob const& vPublic)
 {
-    mIsValid        = true;
+    mIsValid = true;
 
     SetData (VER_ACCOUNT_PUBLIC, vPublic);
 }
@@ -522,7 +523,7 @@ std::string RippleAddress::humanAccountPrivate () const
 
 bool RippleAddress::setAccountPrivate (const std::string& strPrivate)
 {
-    mIsValid        = SetString (strPrivate.c_str (), VER_ACCOUNT_PRIVATE);
+    mIsValid = SetString (strPrivate, VER_ACCOUNT_PRIVATE, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
@@ -536,9 +537,9 @@ void RippleAddress::setAccountPrivate (Blob const& vPrivate)
 
 void RippleAddress::setAccountPrivate (uint256 hash256)
 {
-    mIsValid        = true;
+    mIsValid = true;
 
-    SetData (VER_ACCOUNT_PRIVATE, hash256.begin (), 32);
+    SetData (VER_ACCOUNT_PRIVATE, hash256);
 }
 
 void RippleAddress::setAccountPrivate (const RippleAddress& naGenerator, const RippleAddress& naSeed, int seq)
@@ -572,27 +573,6 @@ bool RippleAddress::accountPrivateSign (uint256 const& uHash, Blob& vucSig) cons
     return bResult;
 }
 
-#if 0
-bool RippleAddress::accountPrivateVerify (uint256 const& uHash, Blob const& vucSig) const
-{
-    CKey        ckPrivate;
-    bool        bVerified;
-
-    if (!ckPrivate.SetPrivateKeyU (getAccountPrivate ()))
-    {
-        // Bad private key.
-        WriteLog (lsWARNING, RippleAddress) << "accountPrivateVerify: Bad private key.";
-        bVerified   = false;
-    }
-    else
-    {
-        bVerified   = ckPrivate.Verify (uHash, vucSig);
-    }
-
-    return bVerified;
-}
-#endif
-
 Blob RippleAddress::accountPrivateEncrypt (const RippleAddress& naPublicTo, Blob const& vucPlainText) const
 {
     CKey                        ckPrivate;
@@ -659,27 +639,6 @@ Blob RippleAddress::accountPrivateDecrypt (const RippleAddress& naPublicFrom, Bl
 // Generators
 //
 
-BIGNUM* RippleAddress::getGeneratorBN () const
-{
-    // returns the public generator
-    switch (nVersion)
-    {
-    case VER_NONE:
-        throw std::runtime_error ("unset source - getGeneratorBN");
-
-    case VER_FAMILY_GENERATOR:
-        // Do nothing.
-        break;
-
-    default:
-        throw std::runtime_error (str (boost::format ("bad source: %d") % int (nVersion)));
-    }
-
-    BIGNUM* ret = BN_bin2bn (&vchData[0], vchData.size (), nullptr);
-    assert (ret);
-    return ret;
-}
-
 Blob const& RippleAddress::getGenerator () const
 {
     // returns the public generator
@@ -714,7 +673,7 @@ std::string RippleAddress::humanGenerator () const
 
 bool RippleAddress::setGenerator (const std::string& strGenerator)
 {
-    mIsValid        = SetString (strGenerator.c_str (), VER_FAMILY_GENERATOR);
+    mIsValid = SetString (strGenerator, VER_FAMILY_GENERATOR, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
@@ -816,7 +775,7 @@ int RippleAddress::setSeed1751 (const std::string& strHuman1751)
 
 bool RippleAddress::setSeed (const std::string& strSeed)
 {
-    mIsValid        = SetString (strSeed.c_str (), VER_FAMILY_SEED);
+    mIsValid = SetString (strSeed, VER_FAMILY_SEED, Base58::getRippleAlphabet ());
 
     return mIsValid;
 }
@@ -861,9 +820,9 @@ bool RippleAddress::setSeedGeneric (const std::string& strText)
 
 void RippleAddress::setSeed (uint128 hash128)
 {
-    mIsValid        = true;
+    mIsValid = true;
 
-    SetData (VER_FAMILY_SEED, hash128.begin (), 16);
+    SetData (VER_FAMILY_SEED, hash128);
 }
 
 void RippleAddress::setSeedRandom ()

src/ripple_data/protocol/RippleAddress.h

@@ -31,9 +31,6 @@
 #include "../ripple/types/api/RipplePublicKeyHash.h"
 #include "../ripple/sslutil/api/ECDSACanonical.h"
 
-struct bignum_st;
-typedef struct bignum_st BIGNUM;
-
 namespace ripple {
 
 //
@@ -110,7 +107,7 @@ public:
 
     std::string humanAccountID () const;
 
-    bool setAccountID (const std::string& strAccountID, Base58::Alphabet const& alphabet = Base58::getCurrentAlphabet ());
+    bool setAccountID (const std::string& strAccountID, Base58::Alphabet const& alphabet = Base58::getRippleAlphabet());
     void setAccountID (const uint160& hash160In);
 
     static RippleAddress createAccountID (const std::string& strAccountID)
@@ -175,7 +172,6 @@ public:
     void setAccountPrivate (const RippleAddress& naGenerator, const RippleAddress& naSeed, int seq);
 
     bool accountPrivateSign (uint256 const& uHash, Blob& vucSig) const;
-    // bool accountPrivateVerify(uint256 const& uHash, Blob const& vucSig) const;
 
     // Encrypt a message.
     Blob accountPrivateEncrypt (const RippleAddress& naPublicTo, Blob const& vucPlainText) const;
@@ -203,7 +199,6 @@ public:
     // Generators
     // Use to generate a master or regular family.
     //
-    BIGNUM* getGeneratorBN () const; // DEPRECATED
     Blob const& getGenerator () const;
 
     std::string humanGenerator () const;