Feature cleanup

RefactoringNotes.txt

@@ -0,0 +1,51 @@
+--------------------------------------------------------------------------------
+
+Naming
+
+Some names don't make sense.
+
+  LedgerAcquire
+    Not a noun.
+    Is it really an InboundLedger ?
+    Does it continue to exist after the ledger is received?
+
+Inconsistent names
+
+  We have full names like SerializedType and then acronyms like STObject
+  Two names for some things, e.g. SerializedLedgerEntry and SLE
+
+  Shared/Smart pointer typedefs in classes have a variety of different names
+  for the same thing. e.g. "pointer", "ptr", "ptr_t", "wptr"
+
+Verbose names
+
+  The prefix "Flat" is more appealing than "Serialized" because its shorter and
+    easier to pronounce.
+
+Ledger "Skip List"
+
+  Is not really a skip list
+
+--------------------------------------------------------------------------------
+
+Interfaces
+
+  Serializer
+
+    Upon analysis this class does two incompatible things. Flattening, and
+    unflattening. The interface should be reimplemented as two distinct
+    abstract classes, InputStream and OutputStream with suitable implementations
+    such as to and from a block of memory or dynamically allocated buffer.
+
+    The name and conflation of dual roles serves to confuse code at the point
+    of call. Does set(Serializer& s) flatten or unflatten the data? This
+    would be more clear:
+        bool write (OutputStream& stream);
+
+
+
+Implementation
+
+  LoadManager
+
+    What is going on in the destructor

StyleCheatSheet.txt

@@ -0,0 +1,7 @@
+- One class per header file.
+- Place each data member on its own line.
+- Place each ctor-initializer on its own line.
+- Create typedefs for primitive types to describe them.
+- Return descriptive local variables instead of constants.
+- Use "explicit" for single-argument ctors
+

modules/ripple_basics/ripple_basics.h

@@ -134,7 +134,7 @@ namespace boost {
 #include "containers/ripple_SecureAllocator.h"
 #include "containers/ripple_TaggedCache.h"
 
-#include "types/ripple_Types.h"
+#include "types/ripple_BasicTypes.h"
 #include "utility/ripple_ByteOrder.h"
 #include "utility/ripple_DiffieHellmanUtil.h"
 #include "utility/ripple_InstanceCounter.h"

modules/ripple_basics/types/ripple_UInt256.cpp

@@ -1,5 +1,5 @@
 
-std::size_t hash_value(const uint256& u)
+std::size_t hash_value(uint256 const& u)
 {
 	std::size_t seed = HashMaps::getInstance ().getNonce <size_t> ();
 

modules/ripple_basics/types/ripple_UInt256.h

@@ -485,26 +485,26 @@ class uint256 : public base_uint256
 };
 
 
-inline bool operator==(const uint256& a, uint64 b)						   { return (base_uint256)a == b; }
-inline bool operator!=(const uint256& a, uint64 b)						   { return (base_uint256)a != b; }
+inline bool operator==(uint256 const& a, uint64 b)						   { return (base_uint256)a == b; }
+inline bool operator!=(uint256 const& a, uint64 b)						   { return (base_uint256)a != b; }
 inline const uint256 operator^(const base_uint256& a, const base_uint256& b) { return uint256(a) ^= b; }
 inline const uint256 operator&(const base_uint256& a, const base_uint256& b) { return uint256(a) &= b; }
 inline const uint256 operator|(const base_uint256& a, const base_uint256& b) { return uint256(a) |= b; }
-inline bool operator==(const base_uint256& a, const uint256& b)		 { return (base_uint256)a == (base_uint256)b; }
-inline bool operator!=(const base_uint256& a, const uint256& b)		 { return (base_uint256)a != (base_uint256)b; }
-inline const uint256 operator^(const base_uint256& a, const uint256& b) { return (base_uint256)a ^  (base_uint256)b; }
-inline const uint256 operator&(const base_uint256& a, const uint256& b) { return (base_uint256)a &  (base_uint256)b; }
-inline const uint256 operator|(const base_uint256& a, const uint256& b) { return (base_uint256)a |  (base_uint256)b; }
-inline bool operator==(const uint256& a, const base_uint256& b)		 { return (base_uint256)a == (base_uint256)b; }
-inline bool operator!=(const uint256& a, const base_uint256& b)		 { return (base_uint256)a != (base_uint256)b; }
-inline const uint256 operator^(const uint256& a, const base_uint256& b) { return (base_uint256)a ^  (base_uint256)b; }
-inline const uint256 operator&(const uint256& a, const base_uint256& b) { return uint256(a) &= b; }
-inline const uint256 operator|(const uint256& a, const base_uint256& b) { return (base_uint256)a |  (base_uint256)b; }
-inline bool operator==(const uint256& a, const uint256& b)			  { return (base_uint256)a == (base_uint256)b; }
-inline bool operator!=(const uint256& a, const uint256& b)			  { return (base_uint256)a != (base_uint256)b; }
-inline const uint256 operator^(const uint256& a, const uint256& b)	  { return (base_uint256)a ^  (base_uint256)b; }
-inline const uint256 operator&(const uint256& a, const uint256& b)	  { return (base_uint256)a &  (base_uint256)b; }
-inline const uint256 operator|(const uint256& a, const uint256& b)	  { return (base_uint256)a |  (base_uint256)b; }
+inline bool operator==(const base_uint256& a, uint256 const& b)		 { return (base_uint256)a == (base_uint256)b; }
+inline bool operator!=(const base_uint256& a, uint256 const& b)		 { return (base_uint256)a != (base_uint256)b; }
+inline const uint256 operator^(const base_uint256& a, uint256 const& b) { return (base_uint256)a ^  (base_uint256)b; }
+inline const uint256 operator&(const base_uint256& a, uint256 const& b) { return (base_uint256)a &  (base_uint256)b; }
+inline const uint256 operator|(const base_uint256& a, uint256 const& b) { return (base_uint256)a |  (base_uint256)b; }
+inline bool operator==(uint256 const& a, const base_uint256& b)		 { return (base_uint256)a == (base_uint256)b; }
+inline bool operator!=(uint256 const& a, const base_uint256& b)		 { return (base_uint256)a != (base_uint256)b; }
+inline const uint256 operator^(uint256 const& a, const base_uint256& b) { return (base_uint256)a ^  (base_uint256)b; }
+inline const uint256 operator&(uint256 const& a, const base_uint256& b) { return uint256(a) &= b; }
+inline const uint256 operator|(uint256 const& a, const base_uint256& b) { return (base_uint256)a |  (base_uint256)b; }
+inline bool operator==(uint256 const& a, uint256 const& b)			  { return (base_uint256)a == (base_uint256)b; }
+inline bool operator!=(uint256 const& a, uint256 const& b)			  { return (base_uint256)a != (base_uint256)b; }
+inline const uint256 operator^(uint256 const& a, uint256 const& b)	  { return (base_uint256)a ^  (base_uint256)b; }
+inline const uint256 operator&(uint256 const& a, uint256 const& b)	  { return (base_uint256)a &  (base_uint256)b; }
+inline const uint256 operator|(uint256 const& a, uint256 const& b)	  { return (base_uint256)a |  (base_uint256)b; }
 
 template<unsigned int BITS> inline std::ostream& operator<<(std::ostream& out, const base_uint<BITS>& u)
 {
@@ -737,7 +737,7 @@ inline const std::string strHex(const uint160& ui)
 
 extern std::size_t hash_value (const uint160&);
 
-extern std::size_t hash_value (const uint256&);
+extern std::size_t hash_value (uint256 const& );
 
 #endif
 

modules/ripple_data/crypto/ripple_Base58.cpp

@@ -103,7 +103,7 @@ std::string Base58::encodeWithCheck (Blob const& vchIn)
 	return encode (vch);
 }
 
-bool Base58::decode (const char* psz, Blob & vchRet, const char* pAlpha)
+bool Base58::decode (const char* psz, Blob& vchRet, const char* pAlpha)
 {
 	assert (pAlpha != 0);
 
@@ -151,12 +151,12 @@ bool Base58::decode (const char* psz, Blob & vchRet, const char* pAlpha)
 	return true;
 }
 
-bool Base58::decode (const std::string& str, Blob & vchRet)
+bool Base58::decode (const std::string& str, Blob& vchRet)
 {
 	return decode (str.c_str(), vchRet);
 }
 
-bool Base58::decodeWithCheck (const char* psz, Blob & vchRet, const char* pAlphabet)
+bool Base58::decodeWithCheck (const char* psz, Blob& vchRet, const char* pAlphabet)
 {
 	assert (pAlphabet != NULL);
 
@@ -177,7 +177,7 @@ bool Base58::decodeWithCheck (const char* psz, Blob & vchRet, const char* pAlpha
 	return true;
 }
 
-bool Base58::decodeWithCheck (const std::string& str, Blob & vchRet, const char* pAlphabet)
+bool Base58::decodeWithCheck (const std::string& str, Blob& vchRet, const char* pAlphabet)
 {
 	return decodeWithCheck (str.c_str(), vchRet, pAlphabet);
 }

modules/ripple_data/crypto/ripple_Base58.h

@@ -48,10 +48,10 @@ class Base58
 	static std::string encode (Blob const& vch);
 	static std::string encodeWithCheck (Blob const& vchIn);
 
-	static bool decode (const char* psz, Blob & vchRet, const char* pAlphabet=getCurrentAlphabet ());
-	static bool decode (const std::string& str, Blob & vchRet);
-	static bool decodeWithCheck (const char* psz, Blob & vchRet, const char* pAlphabet=getCurrentAlphabet ());
-	static bool decodeWithCheck (const std::string& str, Blob & vchRet, const char* pAlphabet);
+	static bool decode (const char* psz, Blob& vchRet, const char* pAlphabet=getCurrentAlphabet ());
+	static bool decode (const std::string& str, Blob& vchRet);
+	static bool decodeWithCheck (const char* psz, Blob& vchRet, const char* pAlphabet=getCurrentAlphabet ());
+	static bool decodeWithCheck (const std::string& str, Blob& vchRet, const char* pAlphabet);
 
 private:
 	static char const* s_currentAlphabet;

modules/ripple_data/crypto/ripple_CBigNum.cpp

@@ -139,7 +139,7 @@ void CBigNum::setuint64(uint64 n)
 #endif
 }
 
-void CBigNum::setuint256(const uint256& n)
+void CBigNum::setuint256(uint256 const& n)
 {
 	BN_bin2bn(n.begin(), n.size(), NULL);
 }

modules/ripple_data/crypto/ripple_CBigNum.h

@@ -79,7 +79,7 @@ class CBigNum : public BIGNUM
 	void setint64(int64 n);
 	uint64 getuint64() const;
 	void setuint64(uint64 n);
-	void setuint256(const uint256& n);
+	void setuint256(uint256 const& n);
 	uint256 getuint256();
 	void setvch(Blob const& vch);
 	Blob getvch() const;

modules/ripple_data/crypto/ripple_CKey.h

@@ -126,7 +126,7 @@ class CKey
 	static EC_KEY* GenerateRootPubKey(BIGNUM* pubGenerator);
 	static EC_KEY* GeneratePublicDeterministicKey(const RippleAddress& generator, int n);
 	static EC_KEY* GeneratePrivateDeterministicKey(const RippleAddress& family, const BIGNUM* rootPriv, int n);
-	static EC_KEY* GeneratePrivateDeterministicKey(const RippleAddress& family, const uint256& rootPriv, int n);
+	static EC_KEY* GeneratePrivateDeterministicKey(const RippleAddress& family, uint256 const& rootPriv, int n);
 
 	CKey(const uint128& passPhrase) : fSet(false)
 	{
@@ -149,7 +149,7 @@ class CKey
 		assert(pkey);
 	}
 
-	CKey(const uint256& privateKey) : pkey(NULL), fSet(false)
+	CKey(uint256 const& privateKey) : pkey(NULL), fSet(false)
 	{
 		// XXX Broken pkey is null.
 		SetPrivateKeyU(privateKey);
@@ -194,7 +194,7 @@ class CKey
 		BN_bn2bin(bn, privKey.begin() + (privKey.size() - BN_num_bytes(bn)));
 	}
 
-	bool SetPrivateKeyU(const uint256& key, bool bThrow=false)
+	bool SetPrivateKeyU(uint256 const& key, bool bThrow=false)
 	{
 		// XXX Broken if pkey is not set.
 		BIGNUM* bn			= BN_bin2bn(key.begin(), key.size(), NULL);
@@ -248,7 +248,7 @@ class CKey
 		return vchPubKey;
 	}
 
-	bool Sign(const uint256& hash, Blob & vchSig)
+	bool Sign(uint256 const& hash, Blob& vchSig)
 	{
 		unsigned char pchSig[10000];
 		unsigned int nSize = 0;
@@ -264,20 +264,20 @@ class CKey
 		return true;
 	}
 
-	bool Verify(const uint256& hash, const void *sig, size_t sigLen) const
+	bool Verify(uint256 const& hash, const void *sig, size_t sigLen) const
 	{
 		// -1 = error, 0 = bad sig, 1 = good
 		if (ECDSA_verify(0, hash.begin(), hash.size(), (const unsigned char *) sig, sigLen, pkey) != 1)
 			return false;
 		return true;
 	}
 
-	bool Verify(const uint256& hash, Blob const& vchSig) const
+	bool Verify(uint256 const& hash, Blob const& vchSig) const
 	{
 		return Verify(hash, &vchSig[0], vchSig.size());
 	}
 
-	bool Verify(const uint256& hash, const std::string& sig) const
+	bool Verify(uint256 const& hash, const std::string& sig) const
 	{
 		return Verify(hash, sig.data(), sig.size());
 	}

modules/ripple_data/crypto/ripple_CKeyDeterministic.cpp

@@ -221,7 +221,7 @@ EC_KEY* CKey::GeneratePublicDeterministicKey(const RippleAddress& pubGen, int se
 	return success ? pkey : NULL;
 }
 
-EC_KEY* CKey::GeneratePrivateDeterministicKey(const RippleAddress& pubGen, const uint256& u, int seq)
+EC_KEY* CKey::GeneratePrivateDeterministicKey(const RippleAddress& pubGen, uint256 const& u, int seq)
 {
 	CBigNum bn(u);
 	return GeneratePrivateDeterministicKey(pubGen, static_cast<BIGNUM*>(&bn), seq);

modules/ripple_data/protocol/ripple_FieldNames.h

@@ -23,6 +23,11 @@ enum SerializedTypeID
 	STI_VALIDATION	= 10003,
 };
 
+/** Identifies fields.
+
+    Fields are necessary to tag data in signed transactions so that
+    the binary format of the transaction can be canonicalized.
+*/
 // VFALCO TODO rename this to NamedField
 class SField
 {
@@ -43,40 +48,58 @@ class SField
 	const int				fieldCode;		// (type<<16)|index
 	const SerializedTypeID	fieldType;		// STI_*
 	const int				fieldValue;		// Code number for protocol
-	std::string				fieldName;
+	std::string	            fieldName;
 	int						fieldMeta;
 	int						fieldNum;
 	bool					signingField;
 
-	SField(int fc, SerializedTypeID tid, int fv, const char* fn) : 
-		fieldCode(fc), fieldType(tid), fieldValue(fv), fieldName(fn), fieldMeta(sMD_Default), signingField(true)
+	SField (int fc, SerializedTypeID tid, int fv, const char* fn)
+        : fieldCode (fc)
+        , fieldType (tid)
+        , fieldValue (fv)
+        , fieldName (fn)
+        , fieldMeta (sMD_Default)
+        , signingField (true)
 	{
 		boost::mutex::scoped_lock sl(mapMutex);
-		codeToField[fieldCode] = this;
-		fieldNum = ++num;
+
+        codeToField[fieldCode] = this;
+
+        fieldNum = ++num;
 	}
 
-	SField(SerializedTypeID tid, int fv, const char *fn) :
-		fieldCode(FIELD_CODE(tid, fv)), fieldType(tid), fieldValue(fv), fieldName(fn),
-		fieldMeta(sMD_Default), signingField(true)
+	SField (SerializedTypeID tid, int fv, const char *fn)
+        : fieldCode (FIELD_CODE (tid, fv))
+        , fieldType (tid)
+        , fieldValue (fv)
+        , fieldName (fn)
+        , fieldMeta (sMD_Default)
+        , signingField (true)
 	{
 		boost::mutex::scoped_lock sl(mapMutex);
-		codeToField[fieldCode] = this;
-		fieldNum = ++num;
+
+        codeToField[fieldCode] = this;
+
+        fieldNum = ++num;
 	}
 
-	SField(int fc) : fieldCode(fc), fieldType(STI_UNKNOWN), fieldValue(0), fieldMeta(sMD_Never), signingField(true)
+	explicit SField (int fc)
+        : fieldCode (fc)
+        , fieldType (STI_UNKNOWN)
+        , fieldValue (0)
+        , fieldMeta (sMD_Never)
+        , signingField (true)
 	{
 		boost::mutex::scoped_lock sl(mapMutex);
 		fieldNum = ++num;
 	}
 
-	~SField();
+	~SField ();
 
-	static SField::ref getField(int fieldCode);
-	static SField::ref getField(const std::string& fieldName);
-	static SField::ref getField(int type, int value)				{ return getField(FIELD_CODE(type, value)); }
-	static SField::ref getField(SerializedTypeID type, int value)	{ return getField(FIELD_CODE(type, value)); }
+	static SField::ref getField (int fieldCode);
+	static SField::ref getField (const std::string& fieldName);
+	static SField::ref getField (int type, int value)				{ return getField(FIELD_CODE(type, value)); }
+	static SField::ref getField (SerializedTypeID type, int value)	{ return getField(FIELD_CODE(type, value)); }
 
 	std::string getName() const;
 	bool hasName() const		{ return !fieldName.empty(); }
@@ -86,8 +109,11 @@ class SField
 	bool isUseful() const		{ return fieldCode > 0; }
 	bool isKnown() const		{ return fieldType != STI_UNKNOWN; }
 	bool isBinary() const		{ return fieldValue < 256; }
-	bool isDiscardable() const	{ return fieldValue > 256; }
-	int getCode() const			{ return fieldCode; }
+
+    // VFALCO NOTE What is a discardable field?
+    bool isDiscardable() const	{ return fieldValue > 256; }
+
+    int getCode() const			{ return fieldCode; }
 	int getNum() const			{ return fieldNum; }
 	static int getNumFields()	{ return num; }
 
@@ -99,8 +125,9 @@ class SField
 	bool shouldInclude(bool withSigningField) const
 		{ return (fieldValue < 256) && (withSigningField || signingField); }
 
-	bool operator==(const SField& f) const { return fieldCode == f.fieldCode; }
-	bool operator!=(const SField& f) const { return fieldCode != f.fieldCode; }
+	bool operator== (const SField& f) const { return fieldCode == f.fieldCode; }
+
+    bool operator!= (const SField& f) const { return fieldCode != f.fieldCode; }
 
 	static int compare(SField::ref f1, SField::ref f2);
 
@@ -110,7 +137,7 @@ class SField
 	static boost::mutex			mapMutex;
 	static int 					num;
 
-	SField(SerializedTypeID id, int val);
+	SField (SerializedTypeID id, int val);
 };
 
 extern SField sfInvalid, sfGeneric, sfLedgerEntry, sfTransaction, sfValidation;