Tidy up LedgerEntrySet

RefactoringNotes.txt

@@ -49,6 +49,10 @@ Ledger "Skip List"
 
   Is not really a skip list data structure
 
+Duplicate Code
+
+  LedgerEntryFormat and TxFormat
+
 --------------------------------------------------------------------------------
 
 Interfaces

modules/ripple_client/ripple_client.cpp

@@ -41,7 +41,7 @@
 #include    "src/cpp/ripple/ripple_AccountState.h"
 #include    "src/cpp/ripple/ripple_NicknameState.h"
 #include     "src/cpp/ripple/Ledger.h"
-#include   "src/cpp/ripple/LedgerEntrySet.h"
+#include   "src/cpp/ripple/ripple_LedgerEntrySet.h"
 #include    "src/cpp/ripple/TransactionEngine.h"
 #include "src/cpp/ripple/ripple_LoadManager.h"
 #include  "src/cpp/ripple/ripple_Peer.h"

modules/ripple_data/protocol/ripple_HashPrefix.h

@@ -22,6 +22,10 @@
 
     @ingroup protocol
 */
+// VFALCO NOTE there are ledger entry prefixes too but they are only
+//             1 byte, find out why they are different. Maybe we should
+//             group them all together?
+//
 struct HashPrefix
 {
     // VFALCO TODO Make these Doxygen comments and expand the

modules/ripple_data/protocol/ripple_LedgerFormat.h

@@ -7,23 +7,57 @@
 #ifndef RIPPLE_LEDGERFORMAT_H
 #define RIPPLE_LEDGERFORMAT_H
 
+/**
+
+    These are stored in serialized data.
+
+    @ingroup protocol
+*/
 // Used as the type of a transaction or the type of a ledger entry.
 enum LedgerEntryType
 {
     ltINVALID           = -1,
+
     ltACCOUNT_ROOT      = 'a',
+
+    /** Directory node.
+
+        A directory is a vector 256-bit values. Usually they represent
+        hashes of other objects in the ledger.
+
+        Used in an append-only fashion.
+
+        (There's a little more information than this, see the template)
+    */
     ltDIR_NODE          = 'd',
+
     ltGENERATOR_MAP     = 'g',
+
+    /** Describes a trust line.
+    */
+    // VFALCO TODO Rename to TrustLine or something similar.
     ltRIPPLE_STATE      = 'r',
+
+    /** Deprecated.
+    */
     ltNICKNAME          = 'n',
+
     ltOFFER             = 'o',
+
     ltCONTRACT          = 'c',
+
     ltLEDGER_HASHES     = 'h',
+
     ltFEATURES          = 'f',
+
     ltFEE_SETTINGS      = 's',
 };
 
+/**
+    @ingroup protocol
+*/
 // Used as a prefix for computing ledger indexes (keys).
+// VFALCO TODO Why are there a separate set of prefixes? i.e. class HashPrefix
 enum LedgerNameSpace
 {
     spaceAccount        = 'a',
@@ -40,6 +74,9 @@ enum LedgerNameSpace
     spaceFee            = 'e',
 };
 
+/**
+    @ingroup protocol
+*/
 enum LedgerSpecificFlags
 {
     // ltACCOUNT_ROOT
@@ -59,6 +96,8 @@ enum LedgerSpecificFlags
     lsfHighAuth         = 0x00080000,
 };
 
+// VFALCO TODO See if we can merge LedgerEntryFormat with TxFormat
+//
 class LedgerEntryFormat
 {
 public:

modules/ripple_main/ripple_main.cpp

@@ -179,7 +179,7 @@
 #include "src/cpp/ripple/ripple_AccountItems.h"
 #include "src/cpp/ripple/ripple_AcceptedLedgerTx.h"
 #include "src/cpp/ripple/ripple_AcceptedLedger.h"
-#include "src/cpp/ripple/LedgerEntrySet.h"
+#include "src/cpp/ripple/ripple_LedgerEntrySet.h"
 #include "src/cpp/ripple/TransactionEngine.h"
 #include "src/cpp/ripple/ripple_CanonicalTXSet.h"
 
@@ -277,7 +277,7 @@ static const uint64 tenTo17m1 = tenTo17 - 1;
 #include "src/cpp/ripple/HttpsClient.cpp"
 #include "src/cpp/ripple/Interpreter.cpp" // no log
 #include "src/cpp/ripple/Ledger.cpp"
-#include "src/cpp/ripple/LedgerEntrySet.cpp"
+#include "src/cpp/ripple/ripple_LedgerEntrySet.cpp"
 #include "src/cpp/ripple/LedgerMaster.cpp"
 #include "src/cpp/ripple/LedgerProposal.cpp" // no log
 #include "src/cpp/ripple/LedgerTiming.cpp"

newcoin.vcxproj

@@ -891,7 +891,7 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
     </ClCompile>
-    <ClCompile Include="src\cpp\ripple\LedgerEntrySet.cpp">
+    <ClCompile Include="src\cpp\ripple\ripple_LedgerEntrySet.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>
@@ -1772,7 +1772,7 @@
     <ClInclude Include="src\cpp\ripple\Ledger.h" />
     <ClInclude Include="src\cpp\ripple\ripple_LedgerAcquire.h" />
     <ClInclude Include="src\cpp\ripple\ripple_LedgerConsensus.h" />
-    <ClInclude Include="src\cpp\ripple\LedgerEntrySet.h" />
+    <ClInclude Include="src\cpp\ripple\ripple_LedgerEntrySet.h" />
     <ClInclude Include="src\cpp\ripple\ripple_LedgerHistory.h" />
     <ClInclude Include="src\cpp\ripple\LedgerMaster.h" />
     <ClInclude Include="src\cpp\ripple\LedgerProposal.h" />

newcoin.vcxproj.filters

@@ -633,9 +633,6 @@
     <ClCompile Include="src\cpp\ripple\Ledger.cpp">
       <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
     </ClCompile>
-    <ClCompile Include="src\cpp\ripple\LedgerEntrySet.cpp">
-      <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
-    </ClCompile>
     <ClCompile Include="src\cpp\ripple\LedgerMaster.cpp">
       <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
     </ClCompile>
@@ -885,6 +882,9 @@
     <ClCompile Include="src\cpp\ripple\ripple_LocalCredentials.cpp">
       <Filter>1. Modules\ripple_main\refactored</Filter>
     </ClCompile>
+    <ClCompile Include="src\cpp\ripple\ripple_LedgerEntrySet.cpp">
+      <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="database\sqlite3ext.h">
@@ -1397,9 +1397,6 @@
     <ClInclude Include="src\cpp\ripple\Ledger.h">
       <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
     </ClInclude>
-    <ClInclude Include="src\cpp\ripple\LedgerEntrySet.h">
-      <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
-    </ClInclude>
     <ClInclude Include="src\cpp\ripple\LedgerMaster.h">
       <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
     </ClInclude>
@@ -1649,6 +1646,9 @@
     <ClInclude Include="src\cpp\ripple\ripple_Version.h">
       <Filter>1. Modules\ripple_main\refactored</Filter>
     </ClInclude>
+    <ClInclude Include="src\cpp\ripple\ripple_LedgerEntrySet.h">
+      <Filter>1. Modules\ripple_main\_unfactored\ledger</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <None Include="SConstruct" />

src/cpp/ripple/Ledger.h

@@ -358,6 +358,9 @@ public:
     //
 
     // Index of node which is the ripple state between two accounts for a currency.
+    // VFALCO NOTE Rename these to make it clear they are simple functions that
+    //             don't access global variables. e.g. "calculateKeyFromRippleStateAndAddress"
+    //
     static uint256 getRippleStateIndex (const RippleAddress & naA, const RippleAddress & naB, const uint160 & uCurrency);
     static uint256 getRippleStateIndex (const uint160 & uiA, const uint160 & uiB, const uint160 & uCurrency)
     {

src/cpp/ripple/ripple_LedgerEntrySet.cpp

@@ -11,6 +11,9 @@ DECLARE_INSTANCE (LedgerEntrySet)
 
 // #define META_DEBUG
 
+// VFALCO TODO Replace this macro with a documented language constant
+//        NOTE Is this part of the protocol?
+//
 #define DIR_NODE_MAX        32
 
 void LedgerEntrySet::init (Ledger::ref ledger, uint256 const& transactionID,

src/cpp/ripple/ripple_LedgerEntrySet.h

@@ -4,8 +4,8 @@
 */
 //==============================================================================
 
-#ifndef __LEDGERENTRYSET__
+#ifndef RIPPLE_LEDGERENTRYSET_H
-#define __LEDGERENTRYSET__
+#define RIPPLE_LEDGERENTRYSET_H
 
 DEFINE_INSTANCE (LedgerEntrySetEntry);
 DEFINE_INSTANCE (LedgerEntrySet);
@@ -42,9 +42,11 @@ public:
     LedgerEntryAction   mAction;
     int                 mSeq;
 
-    LedgerEntrySetEntry (SLE::ref e, LedgerEntryAction a, int s) : mEntry (e), mAction (a), mSeq (s)
+    LedgerEntrySetEntry (SLE::ref e, LedgerEntryAction a, int s)
+        : mEntry (e)
+        , mAction (a)
+        , mSeq (s)
     {
-        ;
     }
 };
 
@@ -62,12 +64,10 @@ public:
     LedgerEntrySet (Ledger::ref ledger, TransactionEngineParams tep, bool immutable = false) :
         mLedger (ledger), mParams (tep), mSeq (0), mImmutable (immutable)
     {
-        ;
     }
 
     LedgerEntrySet () : mParams (tapNONE), mSeq (0), mImmutable (false)
     {
-        ;
     }
 
     // set functions
@@ -75,17 +75,23 @@ public:
     {
         mImmutable = true;
     }
+
     bool isImmutable () const
     {
         return mImmutable;
     }
+
     LedgerEntrySet duplicate () const;  // Make a duplicate of this set
+
     void setTo (const LedgerEntrySet&); // Set this set to have the same contents as another
+
     void swapWith (LedgerEntrySet&);    // Swap the contents of two sets
+
     void invalidate ()
     {
         mLedger.reset ();
     }
+
     bool isValid () const
     {
         return !!mLedger;
@@ -95,21 +101,26 @@ public:
     {
         return mSeq;
     }
+
     TransactionEngineParams getParams () const
     {
         return mParams;
     }
+
     void bumpSeq ()
     {
         ++mSeq;
     }
+
     void init (Ledger::ref ledger, uint256 const & transactionID, uint32 ledgerID, TransactionEngineParams params);
+
     void clear ();
 
     Ledger::pointer& getLedger ()
     {
         return mLedger;
     }
+
     Ledger::ref getLedgerRef () const
     {
         return mLedger;