Clean and harden Transaction.

* Replace boolean parameter with enumerated type.
* Get rid of std::ref.
* 80-column cleanups.
* Replace an std::bin with a lambda.
This commit is contained in:
Tom Ritchford
2014-09-26 14:22:18 -04:00
committed by Vinnie Falco
parent f54280aaad
commit 4241dbb600
11 changed files with 157 additions and 98 deletions

View File

@@ -467,7 +467,7 @@ Transaction::pointer Ledger::getTransaction (uint256 const& transID) const
return txn;
if (type == SHAMapTreeNode::tnTRANSACTION_NM)
txn = Transaction::sharedTransaction (item->peekData (), true);
txn = Transaction::sharedTransaction (item->peekData (), Validate::YES);
else if (type == SHAMapTreeNode::tnTRANSACTION_MD)
{
Blob txnData;
@@ -476,7 +476,7 @@ Transaction::pointer Ledger::getTransaction (uint256 const& transID) const
if (!item->peekSerializer ().getVL (txnData, 0, txnLength))
return Transaction::pointer ();
txn = Transaction::sharedTransaction (txnData, false);
txn = Transaction::sharedTransaction (txnData, Validate::NO);
}
else
{
@@ -551,7 +551,10 @@ bool Ledger::getTransaction (
meta.reset ();
if (!txn)
txn = Transaction::sharedTransaction (item->peekData (), true);
{
txn = Transaction::sharedTransaction (
item->peekData (), Validate::YES);
}
}
else if (type == SHAMapTreeNode::tnTRANSACTION_MD)
{
@@ -560,7 +563,7 @@ bool Ledger::getTransaction (
txn = getApp().getMasterTransaction ().fetch (txID, false);
if (!txn)
txn = Transaction::sharedTransaction (it.getVL (), true);
txn = Transaction::sharedTransaction (it.getVL (), Validate::YES);
else
it.getVL (); // skip transaction

View File

@@ -861,9 +861,13 @@ void NetworkOPsImp::submitTransaction (
}
m_job_queue.addJob (jtTRANSACTION, "submitTxn",
std::bind (&NetworkOPsImp::processTransactionCbVoid, this,
std::make_shared<Transaction> (trans, false),
false, false, false, callback));
std::bind (&NetworkOPsImp::processTransactionCbVoid,
this,
std::make_shared<Transaction> (trans, Validate::NO),
false,
false,
false,
callback));
}
// Sterilize transaction through serialization.
@@ -875,7 +879,8 @@ Transaction::pointer NetworkOPsImp::submitTransactionSync (
Serializer s;
tpTrans->getSTransaction ()->add (s);
auto tpTransNew = Transaction::sharedTransaction (s.getData (), true);
auto tpTransNew = Transaction::sharedTransaction (
s.getData (), Validate::YES);
if (!tpTransNew)
{
@@ -1963,7 +1968,7 @@ NetworkOPs::AccountTxs NetworkOPsImp::getAccountTxs (
SQL_FOREACH (db, sql)
{
auto txn = Transaction::transactionFromSQL (db, false);
auto txn = Transaction::transactionFromSQL (db, Validate::NO);
Serializer rawMeta;
int metaSize = 2048;
@@ -2130,7 +2135,7 @@ NetworkOPsImp::AccountTxs NetworkOPsImp::getTxsAccount (
if (foundResume)
{
auto txn = Transaction::transactionFromSQL (db, false);
auto txn = Transaction::transactionFromSQL (db, Validate::NO);
Serializer rawMeta;
int metaSize = 2048;
@@ -2683,7 +2688,7 @@ Json::Value NetworkOPsImp::transJson(
if (account != amount.issue ().account)
{
LedgerEntrySet les (lpCurrent, tapNONE, true);
auto const ownerFunds (les.accountFunds (account, amount, fhIGNORE_FREEZE));
auto const ownerFunds (les.accountFunds (account, amount, fhIGNORE_FREEZE));
jvObj[jss::transaction][jss::owner_funds] = ownerFunds.getText ();
}

View File

@@ -42,9 +42,9 @@ public:
typedef const std::shared_ptr<SerializedTransaction>& ref;
public:
SerializedTransaction (SerializerIterator & sit);
SerializedTransaction (TxType type);
SerializedTransaction (const STObject & object);
explicit SerializedTransaction (SerializerIterator & sit);
explicit SerializedTransaction (TxType type);
explicit SerializedTransaction (const STObject & object);
// STObject functions
SerializedTypeID getSType () const
@@ -85,6 +85,7 @@ public:
}
void setSigningPubKey (const RippleAddress & naSignPubKey);
void setSourceAccount (const RippleAddress & naSource);
std::string getTransactionType () const
{
return mFormat->getName ();
@@ -130,15 +131,26 @@ public:
static std::string getSQLValueHeader ();
static std::string getSQLInsertHeader ();
static std::string getSQLInsertIgnoreHeader ();
std::string getSQL (std::string & sql, std::uint32_t inLedger, char status) const;
std::string getSQL (std::uint32_t inLedger, char status) const;
std::string getSQL (Serializer rawTxn, std::uint32_t inLedger, char status) const;
std::string getSQL (
std::string & sql, std::uint32_t inLedger, char status) const;
std::string getSQL (
std::uint32_t inLedger, char status) const;
std::string getSQL (
Serializer rawTxn, std::uint32_t inLedger, char status) const;
// SQL Functions with metadata
static std::string getMetaSQLValueHeader ();
static std::string getMetaSQLInsertReplaceHeader ();
std::string getMetaSQL (std::uint32_t inLedger, std::string const& escapedMetaData) const;
std::string getMetaSQL (Serializer rawTxn, std::uint32_t inLedger, char status, std::string const& escapedMetaData) const;
std::string getMetaSQL (
std::uint32_t inLedger, std::string const& escapedMetaData) const;
std::string getMetaSQL (
Serializer rawTxn,
std::uint32_t inLedger,
char status,
std::string const& escapedMetaData) const;
private:
TxType mType;

View File

@@ -82,7 +82,7 @@ public:
SLE::pointer sleTarget = mEngine->entryCache (ltACCOUNT_ROOT,
Ledger::getAccountRootIndex (target_account));
// Destination account does not exist.
if (!sleTarget)
return tecNO_TARGET;
@@ -95,11 +95,15 @@ public:
std::uint64_t hint;
TER result = mEngine->view ().dirAdd (hint,
Ledger::getOwnerDirIndex (mTxnAccountID), sleTicket->getIndex (),
std::bind (&Ledger::ownerDirDescriber,
std::placeholders::_1, std::placeholders::_2,
mTxnAccountID));
auto describer = [&](SLE::pointer p, bool b)
{
Ledger::ownerDirDescriber(p, b, mTxnAccountID);
};
TER result = mEngine->view ().dirAdd (
hint,
Ledger::getOwnerDirIndex (mTxnAccountID),
sleTicket->getIndex (),
describer);
m_journal.trace <<
"Creating ticket " << to_string (sleTicket->getIndex ()) <<

View File

@@ -19,8 +19,11 @@
namespace ripple {
Transaction::Transaction (SerializedTransaction::ref sit, bool bValidate)
: mInLedger (0), mStatus (INVALID), mResult (temUNCERTAIN), mTransaction (sit)
Transaction::Transaction (SerializedTransaction::ref sit, Validate validate)
: mInLedger (0),
mStatus (INVALID),
mResult (temUNCERTAIN),
mTransaction (sit)
{
try
{
@@ -33,20 +36,24 @@ Transaction::Transaction (SerializedTransaction::ref sit, bool bValidate)
return;
}
if (!bValidate || (passesLocalChecks (*mTransaction) && checkSign ()))
if (validate == Validate::NO ||
(passesLocalChecks (*mTransaction) && checkSign ()))
{
mStatus = NEW;
}
}
Transaction::pointer Transaction::sharedTransaction (Blob const& vucTransaction, bool bValidate)
Transaction::pointer Transaction::sharedTransaction (
Blob const& vucTransaction, Validate validate)
{
try
{
Serializer s (vucTransaction);
SerializerIterator sit (s);
Serializer s (vucTransaction);
SerializerIterator sit (s);
SerializedTransaction::pointer st = std::make_shared<SerializedTransaction> (std::ref (sit));
return std::make_shared<Transaction> (st, bValidate);
return std::make_shared<Transaction> (
std::make_shared<SerializedTransaction> (sit),
validate);
}
catch (...)
{
@@ -65,12 +72,16 @@ Transaction::Transaction (
RippleAddress const& naSourceAccount,
std::uint32_t uSeq,
STAmount const& saFee,
std::uint32_t uSourceTag) :
mAccountFrom (naSourceAccount), mFromPubKey (naPublicKey), mInLedger (0), mStatus (NEW), mResult (temUNCERTAIN)
std::uint32_t uSourceTag)
: mAccountFrom (naSourceAccount),
mFromPubKey (naPublicKey),
mInLedger (0),
mStatus (NEW),
mResult (temUNCERTAIN)
{
assert (mFromPubKey.isValid ());
mTransaction = std::make_shared<SerializedTransaction> (ttKind);
mTransaction = std::make_shared<SerializedTransaction> (ttKind);
mTransaction->setSigningPubKey (mFromPubKey);
mTransaction->setSourceAccount (mAccountFrom);
@@ -86,45 +97,36 @@ Transaction::Transaction (
bool Transaction::sign (RippleAddress const& naAccountPrivate)
{
bool bResult = true;
bool bResult = naAccountPrivate.isValid ();
if (!naAccountPrivate.isValid ())
if (!bResult)
{
WriteLog (lsWARNING, Ledger) << "No private key for signing";
bResult = false;
}
getSTransaction ()->sign (naAccountPrivate);
if (bResult)
{
updateID ();
}
else
{
mStatus = INCOMPLETE;
}
return bResult;
}
//
// Misc.
//
bool Transaction::checkSign () const
{
if (!mFromPubKey.isValid ())
{
WriteLog (lsWARNING, Ledger) << "Transaction has bad source public key";
return false;
}
if (mFromPubKey.isValid ())
return mTransaction->checkSign (mFromPubKey);
WriteLog (lsWARNING, Ledger) << "Transaction has bad source public key";
return false;
return mTransaction->checkSign (mFromPubKey);
}
void Transaction::setStatus (TransStatus ts, std::uint32_t lseq)
@@ -133,7 +135,8 @@ void Transaction::setStatus (TransStatus ts, std::uint32_t lseq)
mInLedger = lseq;
}
Transaction::pointer Transaction::transactionFromSQL (Database* db, bool bValidate)
Transaction::pointer Transaction::transactionFromSQL (
Database* db, Validate validate)
{
Serializer rawTxn;
std::string status;
@@ -155,8 +158,8 @@ Transaction::pointer Transaction::transactionFromSQL (Database* db, bool bValida
rawTxn.resize (txSize);
SerializerIterator it (rawTxn);
SerializedTransaction::pointer txn = std::make_shared<SerializedTransaction> (std::ref (it));
Transaction::pointer tr = std::make_shared<Transaction> (txn, bValidate);
auto txn = std::make_shared<SerializedTransaction> (it);
auto tr = std::make_shared<Transaction> (txn, validate);
TransStatus st (INVALID);
@@ -213,7 +216,8 @@ Transaction::pointer Transaction::transactionFromSQL (std::string const& sql)
db->getStr ("Status", status);
inLedger = db->getInt ("LedgerSeq");
txSize = db->getBinary ("RawTxn", &*rawTxn.begin (), rawTxn.getLength ());
txSize = db->getBinary (
"RawTxn", &*rawTxn.begin (), rawTxn.getLength ());
if (txSize > rawTxn.getLength ())
{
@@ -226,8 +230,8 @@ Transaction::pointer Transaction::transactionFromSQL (std::string const& sql)
rawTxn.resize (txSize);
SerializerIterator it (rawTxn);
SerializedTransaction::pointer txn = std::make_shared<SerializedTransaction> (std::ref (it));
Transaction::pointer tr = std::make_shared<Transaction> (txn, true);
auto txn = std::make_shared<SerializedTransaction> (it);
auto tr = std::make_shared<Transaction> (txn, Validate::YES);
TransStatus st (INVALID);
@@ -268,17 +272,25 @@ Transaction::pointer Transaction::transactionFromSQL (std::string const& sql)
Transaction::pointer Transaction::load (uint256 const& id)
{
std::string sql = "SELECT LedgerSeq,Status,RawTxn FROM Transactions WHERE TransID='";
std::string sql = "SELECT LedgerSeq,Status,RawTxn "
"FROM Transactions WHERE TransID='";
sql.append (to_string (id));
sql.append ("';");
return transactionFromSQL (sql);
}
bool Transaction::convertToTransactions (std::uint32_t firstLedgerSeq, std::uint32_t secondLedgerSeq,
bool checkFirstTransactions, bool checkSecondTransactions, const SHAMap::Delta& inMap,
std::map<uint256, std::pair<Transaction::pointer, Transaction::pointer> >& outMap)
bool Transaction::convertToTransactions (
std::uint32_t firstLedgerSeq,
std::uint32_t secondLedgerSeq,
Validate checkFirstTransactions,
Validate checkSecondTransactions,
const SHAMap::Delta& inMap,
std::map<uint256,
std::pair<Transaction::pointer, Transaction::pointer> >& outMap)
{
// convert a straight SHAMap payload difference to a transaction difference table
// Convert a straight SHAMap payload difference to a transaction difference
// table.
//
// return value: true=ledgers are valid, false=a ledger is invalid
for (auto it = inMap.begin (); it != inMap.end (); ++it)

View File

@@ -23,7 +23,8 @@
namespace ripple {
//
// Transactions should be constructed in JSON with. Use STObject::parseJson to obtain a binary version.
// Transactions should be constructed in JSON with. Use STObject::parseJson to
// obtain a binary version.
//
class Database;
@@ -41,7 +42,10 @@ enum TransStatus
INCOMPLETE = 8 // needs more signatures
};
// This class is for constructing and examining transactions. Transactions are static so manipulation functions are unnecessary.
enum class Validate {NO, YES};
// This class is for constructing and examining transactions.
// Transactions are static so manipulation functions are unnecessary.
class Transaction
: public std::enable_shared_from_this<Transaction>
, public CountedObject <Transaction>
@@ -53,18 +57,19 @@ public:
typedef const pointer& ref;
public:
Transaction (SerializedTransaction::ref st, bool bValidate);
Transaction (SerializedTransaction::ref, Validate);
static Transaction::pointer sharedTransaction (Blob const & vucTransaction, bool bValidate);
static Transaction::pointer transactionFromSQL (Database * db, bool bValidate);
static Transaction::pointer sharedTransaction (Blob const&, Validate);
static Transaction::pointer transactionFromSQL (Database*, Validate);
Transaction (
TxType ttKind,
const RippleAddress & naPublicKey, // To prove transaction is consistent and authorized.
const RippleAddress & naSourceAccount, // To identify the paying account.
std::uint32_t uSeq, // To order transactions.
const STAmount & saFee, // Transaction fee.
std::uint32_t uSourceTag); // User call back value.
RippleAddress const& naPublicKey, // To prove transaction is
// consistent and authorized.
RippleAddress const& naSourceAccount, // To identify the paying account.
std::uint32_t uSeq, // To order transactions.
STAmount const& saFee, // Transaction fee.
std::uint32_t uSourceTag); // User call back value.
bool sign (const RippleAddress & naAccountPrivate);
@@ -161,18 +166,18 @@ public:
static Transaction::pointer load (uint256 const& id);
// conversion function
static bool convertToTransactions (std::uint32_t ourLedgerSeq,
std::uint32_t otherLedgerSeq,
bool checkFirstTransactions,
bool checkSecondTransactions,
const SHAMap::Delta & inMap,
std::map<uint256, std::pair<Transaction::pointer,
Transaction::pointer> >& outMap);
static bool convertToTransactions (
std::uint32_t ourLedgerSeq,
std::uint32_t otherLedgerSeq,
Validate checkFirstTransactions,
Validate checkSecondTransactions,
const SHAMap::Delta & inMap,
std::map<uint256, std::pair<pointer, pointer> >& outMap);
static bool isHexTxID (std::string const&);
protected:
static Transaction::pointer transactionFromSQL (std::string const& statement);
static Transaction::pointer transactionFromSQL (std::string const&);
private:
uint256 mTransactionID;

View File

@@ -994,7 +994,12 @@ private:
}
}
static void checkTransaction (Job&, int flags, SerializedTransaction::pointer stx, std::weak_ptr<Peer> peer)
// TODO(tom): duplicates code in handshake_analyzer::checkTransaction.
static void checkTransaction (
Job&,
int flags,
SerializedTransaction::pointer stx,
std::weak_ptr<Peer> peer)
{
try
{
@@ -1007,9 +1012,8 @@ private:
return;
}
bool const needCheck = !(flags & SF_SIGGOOD);
Transaction::pointer tx =
std::make_shared<Transaction> (stx, needCheck);
auto validate = (flags & SF_SIGGOOD) ? Validate::NO : Validate::YES;
auto tx = std::make_shared<Transaction> (stx, validate);
if (tx->getStatus () == INVALID)
{

View File

@@ -1076,7 +1076,12 @@ private:
}
}
static void checkTransaction (Job&, int flags, SerializedTransaction::pointer stx, std::weak_ptr<Peer> peer)
// TODO(tom): duplicates code in PeerImp::checkTransaction.
static void checkTransaction (
Job&,
int flags,
SerializedTransaction::pointer stx,
std::weak_ptr<Peer> peer)
{
try
{
@@ -1090,18 +1095,21 @@ private:
return;
}
bool const needCheck = !(flags & SF_SIGGOOD);
Transaction::pointer tx =
std::make_shared<Transaction> (stx, needCheck);
auto validate = (flags & SF_SIGGOOD) ? Validate::NO : Validate::YES;
auto tx = std::make_shared<Transaction> (stx, validate);
if (tx->getStatus () == INVALID)
{
getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_BAD);
getApp().getHashRouter ().setFlag (
stx->getTransactionID (), SF_BAD);
charge (peer, Resource::feeInvalidSignature);
return;
}
else
getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_SIGGOOD);
{
getApp().getHashRouter ().setFlag (
stx->getTransactionID (), SF_SIGGOOD);
}
bool const trusted (flags & SF_TRUSTED);
getApp().getOPs ().processTransaction (tx, trusted, false, false);
@@ -1115,9 +1123,15 @@ private:
}
// Called from our JobQueue
static void checkPropose (Job& job, Overlay* pPeers, std::shared_ptr<protocol::TMProposeSet> packet,
LedgerProposal::pointer proposal, uint256 consensusLCL, RippleAddress nodePublic,
std::weak_ptr<Peer> peer, bool fromCluster)
static void checkPropose (
Job& job,
Overlay* pPeers,
std::shared_ptr<protocol::TMProposeSet> packet,
LedgerProposal::pointer proposal,
uint256 consensusLCL,
RippleAddress nodePublic,
std::weak_ptr<Peer> peer,
bool fromCluster)
{
bool sigGood = false;
bool isTrusted = (job.getType () == jtPROPOSAL_t);

View File

@@ -65,7 +65,7 @@ Json::Value doSubmit (RPC::Context& context)
try
{
tpTrans = std::make_shared<Transaction> (stpTrans, true);
tpTrans = std::make_shared<Transaction> (stpTrans, Validate::YES);
}
catch (std::exception& e)
{

View File

@@ -51,7 +51,7 @@ Json::Value doTxHistory (RPC::Context& context)
SQL_FOREACH (db, sql)
{
if (auto trans = Transaction::transactionFromSQL (db, false))
if (auto trans = Transaction::transactionFromSQL (db, Validate::NO))
txs.append (trans->getJson (0));
}
}

View File

@@ -381,7 +381,7 @@ Json::Value transactionSign (
try
{
tpTrans = std::make_shared<Transaction> (stpTrans, false);
tpTrans = std::make_shared<Transaction> (stpTrans, Validate::NO);
}
catch (std::exception&)
{