Annotate some core classes

Builds/VisualStudio2012/RippleD.vcxproj

@@ -1130,18 +1130,6 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
     </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_Job.cpp">
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
-    </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_JobQueue.cpp">
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
-    </ClCompile>
     <ClCompile Include="..\..\src\cpp\ripple\main.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
@@ -1266,12 +1254,6 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
     </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_LoadFeeTrack.cpp">
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
-    </ClCompile>
     <ClCompile Include="..\..\src\cpp\ripple\ripple_Peer.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
@@ -1781,8 +1763,6 @@
     <ClInclude Include="..\..\src\cpp\ripple\LedgerProposal.h" />
     <ClInclude Include="..\..\src\cpp\ripple\LedgerTiming.h" />
     <ClInclude Include="..\..\src\cpp\ripple\ripple_LoadManager.h" />
-    <ClInclude Include="..\..\src\cpp\ripple\ripple_Job.h" />
-    <ClInclude Include="..\..\src\cpp\ripple\ripple_JobQueue.h" />
     <ClInclude Include="..\..\src\cpp\ripple\ripple_InboundLedgers.h" />
     <ClInclude Include="..\..\src\cpp\ripple\NetworkOPs.h" />
     <ClInclude Include="..\..\src\cpp\ripple\ripple_NicknameState.h" />

Builds/VisualStudio2012/RippleD.vcxproj.filters

@@ -393,9 +393,6 @@
     <ClCompile Include="..\..\src\cpp\ripple\ripple_DatabaseCon.cpp">
       <Filter>1. Modules\ripple_app\refactored</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_LoadFeeTrack.cpp">
-      <Filter>1. Modules\ripple_app\refactored</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\src\cpp\ripple\ripple_Validations.cpp">
       <Filter>1. Modules\ripple_app\refactored</Filter>
     </ClCompile>
@@ -435,12 +432,6 @@
     <ClCompile Include="..\..\src\cpp\ripple\ripple_Config.cpp">
       <Filter>1. Modules\ripple_app\refactored</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_Job.cpp">
-      <Filter>1. Modules\ripple_app\refactored</Filter>
-    </ClCompile>
-    <ClCompile Include="..\..\src\cpp\ripple\ripple_JobQueue.cpp">
-      <Filter>1. Modules\ripple_app\refactored</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\src\cpp\ripple\HTTPRequest.cpp">
       <Filter>1. Modules\ripple_app\_unfactored\network</Filter>
     </ClCompile>
@@ -1250,12 +1241,6 @@
     <ClInclude Include="..\..\src\cpp\ripple\ripple_Config.h">
       <Filter>1. Modules\ripple_app\refactored</Filter>
     </ClInclude>
-    <ClInclude Include="..\..\src\cpp\ripple\ripple_Job.h">
-      <Filter>1. Modules\ripple_app\refactored</Filter>
-    </ClInclude>
-    <ClInclude Include="..\..\src\cpp\ripple\ripple_JobQueue.h">
-      <Filter>1. Modules\ripple_app\refactored</Filter>
-    </ClInclude>
     <ClInclude Include="..\..\src\cpp\ripple\HTTPRequest.h">
       <Filter>1. Modules\ripple_app\_unfactored\network</Filter>
     </ClInclude>

TODO.txt

@@ -2,6 +2,8 @@
 TODO
 --------------------------------------------------------------------------------
 
+- Remove "ENABLE_INSECURE" when the time is right.
+
 - Put all the Ripple code in the ripple namespace
   * boost unit tests create namespace problems
 

modules/ripple_basics/utility/ripple_ThreadName.cpp

@@ -65,6 +65,8 @@ extern void setCallingThreadName (const char* n)
         pName += " ";
     }
 
+    // VFALCO TODO Use beast::Thread::setCurrentThreadName here
+    //
     prctl (PR_SET_NAME, (pName + n).c_str (), 0, 0, 0);
 }
 #endif

modules/ripple_core/functional/ripple_ILoadFeeTrack.h

@@ -4,14 +4,24 @@
 */
 //==============================================================================
 
-#ifndef RIPPLE_ILOADFEETRACK_H
+#ifndef RIPPLE_ILOADFEETRACK_RIPPLEHEADER
-#define RIPPLE_ILOADFEETRACK_H
+#define RIPPLE_ILOADFEETRACK_RIPPLEHEADER
 
-/** Tracks the current fee and load schedule.
+/** Manages the current fee schedule.
+
+    The "base" fee is the cost to send a reference transaction under no load,
+    expressed in millionths of one XRP.
+
+    The "load" fee is how much the local server currently charges to send a
+    reference transaction. This fee fluctuates based on the load of the
+    server.
 */
+// VFALCO TODO Rename "load" to "current".
 class ILoadFeeTrack
 {
 public:
+    /** Create a new tracker.
+    */
     static ILoadFeeTrack* New ();
 
     virtual ~ILoadFeeTrack () { }
@@ -22,20 +32,20 @@ public:
     // Scale using load as well as base rate
     virtual uint64 scaleFeeLoad (uint64 fee, uint64 baseFee, uint32 referenceFeeUnits, bool bAdmin) = 0;
 
-    virtual uint32 getRemoteFee () = 0;
+    // VFALCO NOTE These appear to be unused, so I'm hiding the declarations.
-    virtual uint32 getLocalFee () = 0;
+    //
+    //virtual uint32 getRemoteFee () = 0;
+    //virtual uint32 getLocalFee () = 0;
+    //virtual void setRemoteFee (uint32) = 0;
 
     virtual uint32 getLoadBase () = 0;
     virtual uint32 getLoadFactor () = 0;
 
     virtual Json::Value getJson (uint64 baseFee, uint32 referenceFeeUnits) = 0;
 
-    virtual void setRemoteFee (uint32) = 0;
     virtual bool raiseLocalFee () = 0;
     virtual bool lowerLocalFee () = 0;
     virtual bool isLoaded () = 0;
 };
 
 #endif
-
-// vim:ts=4

modules/ripple_core/functional/ripple_JobQueue.cpp

@@ -152,7 +152,7 @@ Json::Value JobQueue::getJson (int)
     return ret;
 }
 
-int JobQueue::isOverloaded ()
+bool JobQueue::isOverloaded ()
 {
     int count = 0;
     boost::mutex::scoped_lock sl (mJobLock);
@@ -161,7 +161,7 @@ int JobQueue::isOverloaded ()
         if (mJobLoads[i].isOver ())
             ++count;
 
-    return count;
+    return count > 0;
 }
 
 void JobQueue::shutdown ()

modules/ripple_core/functional/ripple_JobQueue.h

@@ -38,7 +38,7 @@ public:
         return LoadEvent::autoptr (new LoadEvent (mJobLoads[t], name, true));
     }
 
-    int isOverloaded ();
+    bool isOverloaded ();
     Json::Value getJson (int c = 0);
 
 private:

modules/ripple_core/functional/ripple_LoadFeeTrack.cpp

@@ -8,30 +8,6 @@ class LoadManager;
 
 class LoadFeeTrack : public ILoadFeeTrack
 {
-private:
-    static const int lftNormalFee = 256;        // 256 is the minimum/normal load factor
-    static const int lftFeeIncFraction = 16;    // increase fee by 1/16
-    static const int lftFeeDecFraction = 4;     // decrease fee by 1/4
-    static const int lftFeeMax = lftNormalFee * 1000000;
-
-    uint32 mLocalTxnLoadFee;        // Scale factor, lftNormalFee = normal fee
-    uint32 mRemoteTxnLoadFee;       // Scale factor, lftNormalFee = normal fee
-    int raiseCount;
-
-    boost::mutex mLock;
-
-    // VFALCO TODO Move this function to some "math utilities" file
-    // compute (value)*(mul)/(div) - avoid overflow but keep precision
-    uint64 mulDiv (uint64 value, uint32 mul, uint64 div)
-    {
-        static uint64 boundary = (0x00000000FFFFFFFF);
-
-        if (value > boundary)                           // Large value, avoid overflow
-            return (value / div) * mul;
-        else                                            // Normal value, preserve accuracy
-            return (value * mul) / div;
-    }
-
 public:
     LoadFeeTrack ()
         : mLocalTxnLoadFee (lftNormalFee)
@@ -171,6 +147,33 @@ public:
 
         return j;
     }
+
+private:
+    // VFALCO TODO Move this function to some "math utilities" file
+    // compute (value)*(mul)/(div) - avoid overflow but keep precision
+    uint64 mulDiv (uint64 value, uint32 mul, uint64 div)
+    {
+        // VFALCO TODO replace with beast::literal64bitUnsigned ()
+        //
+        static uint64 boundary = (0x00000000FFFFFFFF);
+
+        if (value > boundary)                           // Large value, avoid overflow
+            return (value / div) * mul;
+        else                                            // Normal value, preserve accuracy
+            return (value * mul) / div;
+    }
+
+private:
+    static const int lftNormalFee = 256;        // 256 is the minimum/normal load factor
+    static const int lftFeeIncFraction = 16;    // increase fee by 1/16
+    static const int lftFeeDecFraction = 4;     // decrease fee by 1/4
+    static const int lftFeeMax = lftNormalFee * 1000000;
+
+    uint32 mLocalTxnLoadFee;        // Scale factor, lftNormalFee = normal fee
+    uint32 mRemoteTxnLoadFee;       // Scale factor, lftNormalFee = normal fee
+    int raiseCount;
+
+    boost::mutex mLock;
 };
 
 //------------------------------------------------------------------------------

modules/ripple_core/functional/ripple_LoadMonitor.h

@@ -4,8 +4,8 @@
 */
 //==============================================================================
 
-#ifndef RIPPLE_LOADMONITOR_H
+#ifndef RIPPLE_LOADMONITOR_RIPPLEHEADER
-#define RIPPLE_LOADMONITOR_H
+#define RIPPLE_LOADMONITOR_RIPPLEHEADER
 
 // Monitors load levels and response times
 

src/cpp/ripple/RPCHandler.h

@@ -139,6 +139,9 @@ public:
     static Json::Value runHandler (const std::string& name, const Json::Value& params);
 
 private:
+    // VFALCO TODO Replace with a singleton with a well defined interface and
+    //             a lock free stack (if necessary).
+    //
     static RPCInternalHandler*  sHeadHandler;
 
     RPCInternalHandler*         mNextHandler;

src/cpp/ripple/ripple_LoadManager.cpp

@@ -202,40 +202,63 @@ void LoadManager::threadEntry ()
 {
     setCallingThreadName ("loadmgr");
 
-    // VFALCO TODO replace this with a beast Time object
+    // VFALCO TODO replace this with a beast Time object?
+    //
+    // Initialize the clock to the current time.
     boost::posix_time::ptime t = boost::posix_time::microsec_clock::universal_time ();
 
-    while (1)
+    for (;;)
     {
         {
+            // VFALCO NOTE What is this lock protecting?
             boost::mutex::scoped_lock sl (mLock);
 
+            // Check for the shutdown flag.
             if (mShutdown)
             {
+                // VFALCO NOTE Why clear the flag now?
                 mShutdown = false;
                 return;
             }
 
+            // VFALCO NOTE I think this is to reduce calls to the operating system
+            //             for retrieving the current time.
+            //
+            //        TODO Instead of incrementing can't we just retrieve the current
+            //             time again?
+            //
+            // Manually update the timer.
             UptimeTimer::getInstance ().incrementElapsedTime ();
 
-            int dlTime = UptimeTimer::getInstance ().getElapsedSeconds () - mDeadLock;
+            // Measure the amount of time we have been deadlocked, in seconds.
+            //
+            // VFALCO NOTE mDeadLock is a canary for detecting the condition.
+            int const timeSpentDeadlocked = UptimeTimer::getInstance ().getElapsedSeconds () - mDeadLock;
 
-            if (mArmed && (dlTime >= 10))
+            if (mArmed && (timeSpentDeadlocked >= 10))
             {
-                if ((dlTime % 10) == 0)
+                // Report the deadlocked condition every 10 seconds
+                if ((timeSpentDeadlocked % 10) == 0)
                 {
-                    boost::thread (BIND_TYPE (&LogDeadLock, dlTime)).detach ();
+                    // VFALCO TODO Replace this with a dedicated thread with call queue.
+                    //
+                    boost::thread (BIND_TYPE (&LogDeadLock, timeSpentDeadlocked)).detach ();
                 }
 
-                assert (dlTime < 500);
+                // If we go over 500 seconds spent deadlocked, it means that the
+                // deadlock resolution code has failed, which qualifies as undefined
+                // behavior.
+                //
+                assert (timeSpentDeadlocked < 500);
             }
 
         }
 
-        // VFALCO TODO Eliminate the dependence on the Application object by
-        //         constructing with the job queue and the fee tracker.
         bool change;
 
+        // VFALCO TODO Eliminate the dependence on the Application object.
+        //             Choices include constructing with the job queue / feetracker.
+        //             Another option is using an observer pattern to invert the dependency.
         if (theApp->getJobQueue ().isOverloaded ())
         {
             WriteLog (lsINFO, LoadManager) << theApp->getJobQueue ().getJson (0);

src/cpp/ripple/ripple_LoadManager.h

@@ -40,7 +40,23 @@ enum
     LC_Network = 4
 };
 
-// a single endpoint that can impose load
+/** Tracks the consumption of resources at an endpoint.
+
+    To prevent monopolization of server resources or attacks on servers,
+    resource consumption is monitored at each endpoint. When consumption
+    exceeds certain thresholds, costs are imposed. Costs include charging
+    additional XRP for transactions, requiring a proof of work to be
+    performed, or simply disconnecting the endpoint.
+
+    Currently, consumption endpoints include:
+
+    - WebSocket connections
+    - Peer connections
+
+    @note Although RPC connections consume resources, they are transient and
+          cannot be rate limited. It is advised not to expose RPC interfaces
+          to the general public.
+*/
 class LoadSource
 {
 private:
@@ -53,6 +69,13 @@ public:
     static const int lsfOutbound    = 2; // outbound connection
 
 public:
+    /** Construct a load source.
+
+        Sources with admin privileges have relaxed or no restrictions
+        on resource consumption.
+
+        @param admin    `true` if the source has admin privileges.
+    */
     explicit LoadSource (bool admin)
         : mBalance (0)
         , mFlags (admin ? lsfPrivileged : 0)
@@ -142,22 +165,6 @@ public:
 
     void init ();
 
-    int getCreditRate () const;
-
-    int getCreditLimit () const;
-
-    int getDebitWarn () const;
-
-    int getDebitLimit () const;
-
-    void setCreditRate (int);
-
-    void setCreditLimit (int);
-
-    void setDebitWarn (int);
-
-    void setDebitLimit (int);
-
     bool shouldWarn (LoadSource&) const;
 
     bool shouldCutoff (LoadSource&) const;
@@ -182,6 +189,17 @@ public:
         mArmed = true;
     }
 
+private:
+    // VFALCO TODO These used to be public but are apparently not used. Find out why.
+    int getCreditRate () const;
+    int getCreditLimit () const;
+    int getDebitWarn () const;
+    int getDebitLimit () const;
+    void setCreditRate (int);
+    void setCreditLimit (int);
+    void setDebitWarn (int);
+    void setDebitLimit (int);
+
 private:
     class Cost
     {
@@ -201,7 +219,7 @@ private:
         }
 
     public:
-        // VFALCO TODO Make these private
+        // VFALCO TODO Make these private and const
         LoadType    mType;
         int         mCost;
         int         mCategories;