Refactor NodeStore

TODO.txt

@@ -11,6 +11,8 @@ Vinnie's Short List (Changes day to day)
 
 --------------------------------------------------------------------------------
 
+- Replace base_uint and uintXXX with UnsignedInteger
+
 - Rewrite boost program_options in Beast
 
 - Examples for different backend key/value config settings

modules/ripple_app/node/ripple_NodeObject.cpp

@@ -32,6 +32,80 @@ NodeObject::NodeObject (
 {
 }
 
+NodeObject::NodeObject (void const* key, void const* value, int valueBytes)
+{
+    DecodedBlob decoded (key, value, valueBytes);
+
+    if (decoded.success)
+    {
+        mType = decoded.objectType;
+        mHash = uint256 (key);
+        mLedgerIndex = decoded.ledgerIndex;
+        mData = Blob (decoded.objectData, decoded.objectData + decoded.dataBytes);
+    }
+    else
+    {
+        // VFALCO TODO Write the hex version of key to the string for diagnostics.
+        String s;
+        s << "NodeStore:: DecodedBlob failed";
+        Throw (s);
+    }
+}
+
+NodeObject::DecodedBlob::DecodedBlob (void const* key, void const* value, int valueBytes)
+{
+    /*  Data format:
+
+        Bytes
+
+        0...3       LedgerIndex     32-bit big endian integer
+        4...7       Unused?         An unused copy of the LedgerIndex
+        8           char            One of NodeObjectType
+        9...end                     The body of the object data
+    */
+
+    success = false;
+    key = key;
+    // VFALCO NOTE Ledger indexes should have started at 1
+    ledgerIndex = LedgerIndex (-1);
+    objectType = hotUNKNOWN;
+    objectData = nullptr;
+    dataBytes = bmin (0, valueBytes - 9);
+
+    if (dataBytes > 4)
+    {
+        LedgerIndex const* index = static_cast <LedgerIndex const*> (value);
+        ledgerIndex = ByteOrder::swapIfLittleEndian (*index);
+    }
+
+    // VFALCO NOTE What about bytes 4 through 7 inclusive?
+
+    if (dataBytes > 8)
+    {
+        unsigned char const* byte = static_cast <unsigned char const*> (value);
+        objectType = static_cast <NodeObjectType> (byte [8]);
+    }
+
+    if (dataBytes > 9)
+    {
+        objectData = static_cast <unsigned char const*> (value) + 9;
+
+        switch (objectType)
+        {
+        case hotUNKNOWN:
+        default:
+            break;
+
+        case hotLEDGER:
+        case hotTRANSACTION:
+        case hotACCOUNT_NODE:
+        case hotTRANSACTION_NODE:
+            success = true;
+            break;
+        }
+    }
+}
+
 NodeObjectType NodeObject::getType () const
 {
     return mType;

modules/ripple_app/node/ripple_NodeObject.h

@@ -34,6 +34,14 @@ class NodeObject : public CountedObject <NodeObject>
 public:
     static char const* getCountedObjectName () { return "NodeObject"; }
 
+    /** The type used to hold the hash.
+
+        The hahes are fixed size, SHA256.
+
+        @note The key size can be retrieved with `Hash::sizeInBytes`
+    */
+    typedef UnsignedInteger <32> Hash;
+
     typedef boost::shared_ptr <NodeObject> pointer;
     typedef pointer const& ref;
 
@@ -56,6 +64,40 @@ public:
                 int bytesInBuffer,
                 uint256 const & hash);
 
+    /** Create from a key/value blob.
+
+        This is the format in which a NodeObject is stored in the
+        persistent storage layer.
+
+        @see NodeStore
+    */
+    NodeObject (void const* key, void const* value, int valueBytes);
+
+    /** Parsed key/value blob into NodeObject components.
+
+        This will extract the information required to construct
+        a NodeObject. It also does consistency checking and returns
+        the result, so it is possible to determine if the data
+        is corrupted without throwing an exception. Note all forms
+        of corruption are detected so further analysis will be
+        needed to eliminate false positives.
+
+        This is the format in which a NodeObject is stored in the
+        persistent storage layer.
+    */
+    struct DecodedBlob
+    {
+        DecodedBlob (void const* key, void const* value, int valueBytes);
+
+        bool success;
+
+        void const* key;
+        LedgerIndex ledgerIndex;
+        NodeObjectType objectType;
+        unsigned char const* objectData;
+        int dataBytes;
+    };
+        
     /** Retrieve the type of this object.
     */
     NodeObjectType getType () const;
@@ -74,10 +116,10 @@ public:
     Blob const& getData () const;
 
 private:
-    NodeObjectType const mType;
-    uint256 const mHash;
-    LedgerIndex const mLedgerIndex;
-    Blob const mData;
+    NodeObjectType mType;
+    uint256 mHash;
+    LedgerIndex mLedgerIndex;
+    Blob mData;
 };
 
 #endif

modules/ripple_app/node/ripple_NodeStore.cpp

@@ -193,7 +193,7 @@ NodeObject::pointer NodeStore::retrieve (uint256 const& hash)
 
     if (m_fastBackend)
     {
-        obj = m_fastBackend->retrieve (hash);
+        obj = retrieve (m_fastBackend, hash);
 
         if (obj)
         {
@@ -203,16 +203,26 @@ NodeObject::pointer NodeStore::retrieve (uint256 const& hash)
     }
 
     {
-        LoadEvent::autoptr event (getApp().getJobQueue ().getLoadEventAP (jtHO_READ, "HOS::retrieve"));
-        obj = m_backend->retrieve(hash);
+        // m_hooks->onRetrieveBegin ()
 
-        if (!obj)
+        // VFALCO TODO Why is this an autoptr? Why can't it just be a plain old object?
+        //
+        LoadEvent::autoptr event (getApp().getJobQueue ().getLoadEventAP (jtHO_READ, "HOS::retrieve"));
+
+        obj = retrieve (m_backend, hash);
+
+        if (obj == nullptr)
         {
             m_negativeCache.add (hash);
-            return obj;
-        }
+
+            // VFALCO TODO Eliminate return from middle of function
+
+            return obj; // VFALCO NOTE This is nullptr, why return obj? 
         }
 
+    }
+
+    // VFALCO NOTE What does this do?
     m_cache.canonicalize (hash, obj);
 
     if (m_fastBackend)
@@ -223,6 +233,13 @@ NodeObject::pointer NodeStore::retrieve (uint256 const& hash)
     return obj;
 }
 
+//------------------------------------------------------------------------------
+
+NodeObject::pointer NodeStore::retrieve (Backend* backend, uint256 const& hash)
+{
+    return backend->retrieve (hash);
+}
+
 void NodeStore::importVisitor (
     std::vector <NodeObject::pointer>& objects,
     NodeObject::pointer object)

modules/ripple_app/node/ripple_NodeStore.h

@@ -19,13 +19,18 @@ public:
         */
         // VFALCO TODO Make this a tunable parameter in the key value pairs
         bulkWriteBatchSize = 128
+
+        /** Size of the fixed keys, in bytes.
+        */
+        ,keyBytes = 32 // 256 bit hash
     };
 
     /** Interface to inform callers of cetain activities.
     */
     class Hooks
     {
-        virtual void on
+        virtual void onRetrieveBegin () { }
+        virtual void onRetrieveEnd () { }
     };
 
     /** Back end used for the store.
@@ -43,11 +48,19 @@ public:
         //             It should just deal with a fixed key and raw data.
         //
         virtual bool store (NodeObject::ref);
+        //virtual bool put (void const* key, void const* value, int valueBytes) { return false; }
 
         /** Retrieve an individual object.
         */
         virtual NodeObject::pointer retrieve (uint256 const &hash) = 0;
 
+        struct GetCallback
+        {
+            virtual void* getBufferForValue (int valueBytes) = 0;
+        };
+
+        virtual bool get (void const* key, GetCallback* callback) { return false; }
+
         // Visit every object in the database
         // This function will only be called during an import operation
         //
@@ -161,6 +174,8 @@ public:
     int import (String sourceBackendParameters);
 
 private:
+    NodeObject::pointer retrieve (Backend* backend, uint256 const& hash);
+
     void importVisitor (std::vector <NodeObject::pointer>& objects, NodeObject::pointer object);
     
     static Backend* createBackend (String const& parameters);

modules/ripple_basics/containers/ripple_TaggedCache.h

@@ -343,6 +343,7 @@ bool TaggedCache<c_Key, c_Data, Timer>::del (const key_type& key, bool valid)
     return ret;
 }
 
+// VFALCO NOTE What does it mean to canonicalize the data?
 template<typename c_Key, typename c_Data, class Timer>
 bool TaggedCache<c_Key, c_Data, Timer>::canonicalize (const key_type& key, boost::shared_ptr<c_Data>& data, bool replace)
 {

modules/ripple_basics/types/ripple_UInt256.h

@@ -19,6 +19,10 @@ inline int Testuint256AdHoc (std::vector<std::string> vArg);
 
 // We have to keep a separate base class without constructors
 // so the compiler will let us use it in a union
+//
+// VFALCO NOTE This class produces undefined behavior when
+//             BITS is not a multiple of 32!!!
+//
 template<unsigned int BITS>
 class base_uint
 {
@@ -30,6 +34,22 @@ protected:
     unsigned int pn[WIDTH];
 
 public:
+    base_uint ()
+    {
+    }
+
+    /** Construct from a raw pointer.
+    
+        The buffer pointed to by `data` must be at least 32 bytes.
+    */
+    explicit base_uint (void const* data)
+    {
+        // BITS must be a multiple of 32
+        static_bassert ((BITS % 32) == 0);
+
+        memcpy (&pn [0], data, BITS / 8);
+    }
+
     bool isZero () const
     {
         for (int i = 0; i < WIDTH; i++)
@@ -474,6 +494,11 @@ public:
         *this = b;
     }
 
+    explicit uint256 (void const* data)
+        : base_uint256 (data)
+    {
+    }
+
     uint256& operator= (uint64 uHost)
     {
         zero ();
@@ -590,7 +615,7 @@ template<unsigned int BITS> inline std::ostream& operator<< (std::ostream& out,
 
 inline int Testuint256AdHoc (std::vector<std::string> vArg)
 {
-    uint256 g (0);
+    uint256 g (uint64 (0));
 
     printf ("%s\n", g.ToString ().c_str ());
     --g;

src/cpp/ripple/ripple_SHAMapTreeNode.cpp

@@ -4,8 +4,14 @@
 */
 //==============================================================================
 
-SHAMapTreeNode::SHAMapTreeNode (uint32 seq, const SHAMapNode& nodeID) : SHAMapNode (nodeID), mHash (0),
-    mSeq (seq), mAccessSeq (seq), mType (tnERROR), mIsBranch (0), mFullBelow (false)
+SHAMapTreeNode::SHAMapTreeNode (uint32 seq, const SHAMapNode& nodeID)
+    : SHAMapNode (nodeID)
+    , mHash (uint64(0))
+    , mSeq (seq)
+    , mAccessSeq (seq)
+    , mType (tnERROR)
+    , mIsBranch (0)
+    , mFullBelow (false)
 {
 }