Cleanups:

- Cleanup VotableInteger class and remove unused duplicate
 - Remove obsolete function and move to std functions
 - Fix typos
 - Make isMemoOkay a free function
 - Sanitize error returns
This commit is contained in:
Nik Bougalis
2014-03-03 13:37:08 -08:00
parent fc129e43fd
commit 569b3a46a1
10 changed files with 102 additions and 157 deletions

View File

@@ -498,57 +498,6 @@ Json::Value Features::getJson (uint256 const& feature)
return ret;
}
template<typename INT> class VotableInteger
{
protected:
INT mCurrent; // The current setting
INT mTarget; // The setting we want
std::map<INT, int> mVoteMap;
public:
VotableInteger (INT current, INT target) : mCurrent (current), mTarget (target)
{
++mVoteMap[mTarget]; // Add our vote
}
bool mayVote ()
{
return mCurrent != mTarget; // If we love the current setting, we will not vote
}
void addVote (INT vote)
{
++mVoteMap[vote];
}
void noVote ()
{
addVote (mCurrent);
}
INT getVotes ()
{
INT ourVote = mCurrent;
int weight = 0;
typedef typename std::map<INT, int>::value_type mapVType;
BOOST_FOREACH (const mapVType & value, mVoteMap)
{
// Take most voted value between current and target, inclusive
// FIXME: Should take best value that can get a significant majority
if ((value.first <= std::max (mTarget, mCurrent)) &&
(value.first >= std::min (mTarget, mCurrent)) &&
(value.second > weight))
{
ourVote = value.first;
weight = value.second;
}
}
return ourVote;
}
};
IFeatures* IFeatures::New (uint32 majorityTime, int majorityFraction)
{
return new Features (majorityTime, majorityFraction);

View File

@@ -24,22 +24,26 @@ class Features;
class FeeVote : public IFeeVote
{
private:
// VFALCO TODO rename template parameter (wtf, looks like a macro)
template <typename INT>
template <typename Integer>
class VotableInteger
{
public:
VotableInteger (INT current, INT target) : mCurrent (current), mTarget (target)
VotableInteger (Integer current, Integer target)
: mCurrent (current)
, mTarget (target)
{
++mVoteMap[mTarget]; // Add our vote
// Add our vote
++mVoteMap[mTarget];
}
bool mayVote ()
bool mayVote () const
{
return mCurrent != mTarget; // If we love the current setting, we will not vote
// If we love the current setting, we will not vote
return mCurrent != mTarget;
}
void addVote (INT vote)
void addVote (Integer vote)
{
++mVoteMap[vote];
}
@@ -49,12 +53,12 @@ private:
addVote (mCurrent);
}
INT getVotes ()
Integer getVotes ()
{
INT ourVote = mCurrent;
Integer ourVote = mCurrent;
int weight = 0;
typedef typename std::map<INT, int>::value_type mapVType;
typedef typename std::map<Integer, int>::value_type mapVType;
BOOST_FOREACH (const mapVType & value, mVoteMap)
{
// Take most voted value between current and target, inclusive
@@ -71,10 +75,11 @@ private:
}
private:
INT mCurrent; // The current setting
INT mTarget; // The setting we want
std::map<INT, int> mVoteMap;
Integer mCurrent; // The current setting
Integer mTarget; // The setting we want
std::map<Integer, int> mVoteMap;
};
public:
FeeVote (uint64 targetBaseFee, uint32 targetReserveBase, uint32 targetReserveIncrement)
: mTargetBaseFee (targetBaseFee)

View File

@@ -733,7 +733,7 @@ void NetworkOPsImp::submitTransaction (Job&, SerializedTransaction::pointer iTra
{
try
{
if (!trans->isMemoOkay () || !trans->checkSign ())
if (!isMemoOkay (*trans) || !trans->checkSign ())
{
m_journal.warning << "Submitted transaction has bad signature";
getApp().getHashRouter ().setFlag (suppress, SF_BAD);

View File

@@ -307,17 +307,19 @@ std::string SerializedTransaction::getMetaSQL (Serializer rawTxn, uint32 inLedge
% getSequence () % inLedger % status % rTxn % escapedMetaData);
}
bool SerializedTransaction::isMemoOkay ()
//------------------------------------------------------------------------------
bool isMemoOkay (STObject const& st)
{
bool ret = true;
if (isFieldPresent (sfMemos))
{
Serializer s (2048); // Try to avoid allocate/copy/free's
getFieldArray (sfMemos).add (s);
if (s.getDataLength () > 1024)
ret = false;
}
return ret;
if (!st.isFieldPresent (sfMemos))
return true;
// The number 2048 is a preallocation hint, not a hard limit
// to avoid allocate/copy/free's
Serializer s (2048);
st.getFieldArray (sfMemos).add (s);
// FIXME move the memo limit into a config tunable
if (s.getDataLength () > 1024)
return false;
return true;
}
//------------------------------------------------------------------------------

View File

@@ -124,8 +124,6 @@ public:
mSigBad = true;
}
bool isMemoOkay ();
// SQL Functions
static std::string getSQLValueHeader ();
static std::string getSQLInsertHeader ();
@@ -153,5 +151,7 @@ private:
mutable bool mSigBad;
};
bool isMemoOkay (STObject const& st);
#endif
// vim:ts=4

View File

@@ -338,7 +338,8 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
if (!bOffline)
{
SLE::pointer sleAccountRoot = mNetOps->getSLEi (lSnapshot, Ledger::getAccountRootIndex (raSrcAddressID.getAccountID ()));
SLE::pointer sleAccountRoot = mNetOps->getSLEi (lSnapshot,
Ledger::getAccountRootIndex (raSrcAddressID.getAccountID ()));
if (!sleAccountRoot)
{
@@ -350,23 +351,29 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
bool bHaveAuthKey = false;
RippleAddress naAuthorizedPublic;
RippleAddress naSecret = RippleAddress::createSeedGeneric (params["secret"].asString ());
RippleAddress naMasterGenerator = RippleAddress::createGeneratorPublic (naSecret);
RippleAddress naSecret = RippleAddress::createSeedGeneric (
params["secret"].asString ());
RippleAddress naMasterGenerator = RippleAddress::createGeneratorPublic (
naSecret);
// Find the index of Account from the master generator, so we can generate the public and private keys.
RippleAddress naMasterAccountPublic;
unsigned int iIndex = 0;
bool bFound = false;
// Find the index of Account from the master generator, so we can generate
// the public and private keys.
RippleAddress naMasterAccountPublic;
unsigned int iIndex = 0;
bool bFound = false;
// Don't look at ledger entries to determine if the account exists. Don't want to leak to thin server that these accounts are
// related.
// Don't look at ledger entries to determine if the account exists.
// Don't want to leak to thin server that these accounts are related.
while (!bFound && iIndex != getConfig ().ACCOUNT_PROBE_MAX)
{
naMasterAccountPublic.setAccountPublic (naMasterGenerator, iIndex);
WriteLog (lsWARNING, RPCHandler) << "authorize: " << iIndex << " : " << naMasterAccountPublic.humanAccountID () << " : " << raSrcAddressID.humanAccountID ();
WriteLog (lsWARNING, RPCHandler) <<
"authorize: " << iIndex <<
" : " << naMasterAccountPublic.humanAccountID () <<
" : " << raSrcAddressID.humanAccountID ();
bFound = raSrcAddressID.getAccountID () == naMasterAccountPublic.getAccountID ();
bFound = raSrcAddressID.getAccountID () == naMasterAccountPublic.getAccountID ();
if (!bFound)
++iIndex;
@@ -378,9 +385,12 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
}
// Use the generator to determine the associated public and private keys.
RippleAddress naGenerator = RippleAddress::createGeneratorPublic (naSecret);
RippleAddress naAccountPublic = RippleAddress::createAccountPublic (naGenerator, iIndex);
RippleAddress naAccountPrivate = RippleAddress::createAccountPrivate (naGenerator, naSecret, iIndex);
RippleAddress naGenerator = RippleAddress::createGeneratorPublic (
naSecret);
RippleAddress naAccountPublic = RippleAddress::createAccountPublic (
naGenerator, iIndex);
RippleAddress naAccountPrivate = RippleAddress::createAccountPrivate (
naGenerator, naSecret, iIndex);
if (bHaveAuthKey
// The generated pair must match authorized...
@@ -395,7 +405,7 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
return rpcError (rpcSRC_ACT_NOT_FOUND);
}
std::unique_ptr<STObject> sopTrans;
std::unique_ptr<STObject> sopTrans;
{
STParsedJSON parsed ("tx_json", tx_json);
@@ -424,30 +434,27 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
}
catch (std::exception& e)
{
jvResult["error"] = "invalidTransaction";
jvResult["error_exception"] = e.what ();
return jvResult;
return RPC::make_error (rpcINTERNAL,
"Exception occurred during transaction");
}
if (!stpTrans->isMemoOkay ())
if (!isMemoOkay (*stpTrans))
{
jvResult["error"] = "invalidTransaction";
jvResult["error_exception"] = "overlong memos";
return jvResult;
return RPC::make_error (rpcINVALID_PARAMS,
"The memo exceeds the maximum allowed size.");
}
if (params.isMember ("debug_signing"))
{
jvResult["tx_unsigned"] = strHex (stpTrans->getSerializer ().peekData ());
jvResult["tx_unsigned"] = strHex (
stpTrans->getSerializer ().peekData ());
jvResult["tx_signing_hash"] = stpTrans->getSigningHash ().ToString ();
}
// FIXME: For performance, transactions should not be signed in this code path.
stpTrans->sign (naAccountPrivate);
Transaction::pointer tpTrans;
Transaction::pointer tpTrans;
try
{
@@ -455,37 +462,33 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
}
catch (std::exception& e)
{
jvResult["error"] = "internalTransaction";
jvResult["error_exception"] = e.what ();
return jvResult;
return RPC::make_error (rpcINTERNAL,
"Exception occurred during transaction");
}
try
{
// FIXME: For performance, should use asynch interface
tpTrans = mNetOps->submitTransactionSync (tpTrans, mRole == Config::ADMIN, bFailHard, bSubmit);
tpTrans = mNetOps->submitTransactionSync (tpTrans,
mRole == Config::ADMIN, bFailHard, bSubmit);
if (!tpTrans)
{
jvResult["error"] = "invalidTransaction";
jvResult["error_exception"] = "Unable to sterilize transaction.";
return jvResult;
return RPC::make_error (rpcINTERNAL,
"Unable to sterilize transaction.");
}
}
catch (std::exception& e)
{
jvResult["error"] = "internalSubmit";
jvResult["error_exception"] = e.what ();
return jvResult;
return RPC::make_error (rpcINTERNAL,
"Exception occurred during transaction submission.");
}
try
{
jvResult["tx_json"] = tpTrans->getJson (0);
jvResult["tx_blob"] = strHex (tpTrans->getSTransaction ()->getSerializer ().peekData ());
jvResult["tx_json"] = tpTrans->getJson (0);
jvResult["tx_blob"] = strHex (
tpTrans->getSTransaction ()->getSerializer ().peekData ());
if (temUNCERTAIN != tpTrans->getResult ())
{
@@ -503,10 +506,8 @@ Json::Value RPCHandler::transactionSign (Json::Value params,
}
catch (std::exception& e)
{
jvResult["error"] = "internalJson";
jvResult["error_exception"] = e.what ();
return jvResult;
return RPC::make_error (rpcINTERNAL,
"Exception occurred during JSON handling.");
}
}
@@ -535,7 +536,7 @@ Json::Value RPCHandler::getMasterGenerator (Ledger::ref lrLedger, const RippleAd
if (vucMasterGenerator.empty ())
{
return rpcError (rpcFAIL_GEN_DECRPYT);
return rpcError (rpcFAIL_GEN_DECRYPT);
}
naMasterGenerator.setGenerator (vucMasterGenerator);
@@ -693,7 +694,7 @@ Json::Value RPCHandler::accountFromString (Ledger::ref lrLedger, RippleAddress&
if (vucMasterGenerator.empty ())
{
rpcError (rpcNO_GEN_DECRPYT);
rpcError (rpcNO_GEN_DECRYPT);
}
naGenerator.setGenerator (vucMasterGenerator);

View File

@@ -31,7 +31,7 @@ Transaction::Transaction (SerializedTransaction::ref sit, bool bValidate)
return;
}
if (!bValidate || (mTransaction->isMemoOkay () && checkSign ()))
if (!bValidate || (isMemoOkay (*mTransaction) && checkSign ()))
mStatus = NEW;
}
@@ -350,29 +350,17 @@ Json::Value Transaction::getJson (int options, bool binary) const
return ret;
}
//
// Obsolete
//
static bool isHex (char j)
{
if ((j >= '0') && (j <= '9')) return true;
if ((j >= 'A') && (j <= 'F')) return true;
if ((j >= 'a') && (j <= 'f')) return true;
return false;
}
bool Transaction::isHexTxID (const std::string& txid)
{
if (txid.size () != 64) return false;
if (txid.size () != 64)
return false;
for (int i = 0; i < 64; ++i)
if (!isHex (txid[i])) return false;
auto const ret = std::find_if_not (txid.begin (), txid.end (),
std::bind (
std::isxdigit <std::string::value_type>,
std::placeholders::_1,
std::locale ()));
return true;
return (ret == txid.end ());
}
// vim:ts=4

View File

@@ -175,18 +175,18 @@ struct send_if_pred
{
typedef void return_type;
PackedMessage::pointer const& msg;
Predicate const& predicate;
PackedMessage::pointer const& msg;
Predicate const& predicate;
send_if_pred(PackedMessage::pointer const& m, Predicate const& p)
: msg(m), predicate(p)
{ }
send_if_pred(PackedMessage::pointer const& m, Predicate const& p)
: msg(m), predicate(p)
{ }
void operator()(Peer::ref peer) const
{
{
if (predicate (peer))
peer->sendPacket (msg, false);
}
}
};
/** Helper function to aid in type deduction */
@@ -206,7 +206,7 @@ struct send_if_not_pred
{
typedef void return_type;
PackedMessage::pointer const& msg;
PackedMessage::pointer const& msg;
Predicate const& predicate;
send_if_not_pred(PackedMessage::pointer const& m, Predicate const& p)

View File

@@ -108,10 +108,10 @@ enum error_code_i
// Internal error (should never happen)
rpcINTERNAL, // Generic internal error.
rpcFAIL_GEN_DECRPYT,
rpcFAIL_GEN_DECRYPT,
rpcNOT_IMPL,
rpcNOT_SUPPORTED,
rpcNO_GEN_DECRPYT,
rpcNO_GEN_DECRYPT,
};
//------------------------------------------------------------------------------

View File

@@ -65,7 +65,7 @@ public:
add (rpcDST_AMT_MALFORMED, "dstAmtMalformed", "Destination amount/currency/issuer is malformed.");
add (rpcDST_ISR_MALFORMED, "dstIsrMalformed", "Destination issuer is malformed.");
add (rpcFORBIDDEN, "forbidden", "Bad credentials.");
add (rpcFAIL_GEN_DECRPYT, "failGenDecrypt", "Failed to decrypt generator.");
add (rpcFAIL_GEN_DECRYPT, "failGenDecrypt", "Failed to decrypt generator.");
add (rpcGETS_ACT_MALFORMED, "getsActMalformed", "Gets account malformed.");
add (rpcGETS_AMT_MALFORMED, "getsAmtMalformed", "Gets amount malformed.");
add (rpcHOST_IP_MALFORMED, "hostIpMalformed", "Host IP is malformed.");
@@ -84,7 +84,7 @@ public:
add (rpcNO_CLOSED, "noClosed", "Closed ledger is unavailable.");
add (rpcNO_CURRENT, "noCurrent", "Current ledger is unavailable.");
add (rpcNO_EVENTS, "noEvents", "Current transport does not support events.");
add (rpcNO_GEN_DECRPYT, "noGenDectypt", "Password failed to decrypt master public generator.");
add (rpcNO_GEN_DECRYPT, "noGenDecrypt", "Password failed to decrypt master public generator.");
add (rpcNO_NETWORK, "noNetwork", "Network not available.");
add (rpcNO_PATH, "noPath", "Unable to find a ripple path.");
add (rpcNO_PERMISSION, "noPermission", "You don't have permission for this command.");