From 2c535940ac5dadd41fcecd9c893abebe2224ebb4 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 23 Dec 2012 17:42:04 -0800 Subject: [PATCH] Make the transaction engine report whether it added the transaction. --- src/cpp/ripple/LedgerConsensus.cpp | 33 ++++++++++++++++------------ src/cpp/ripple/LedgerMaster.cpp | 9 +++++--- src/cpp/ripple/TransactionEngine.cpp | 17 +++++++------- src/cpp/ripple/TransactionEngine.h | 2 +- 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/cpp/ripple/LedgerConsensus.cpp b/src/cpp/ripple/LedgerConsensus.cpp index 7953a11466..ee9c024784 100644 --- a/src/cpp/ripple/LedgerConsensus.cpp +++ b/src/cpp/ripple/LedgerConsensus.cpp @@ -1090,14 +1090,15 @@ void LedgerConsensus::playbackProposals() void LedgerConsensus::applyTransaction(TransactionEngine& engine, SerializedTransaction::ref txn, Ledger::ref ledger, CanonicalTXSet& failedTransactions, bool openLedger) -{ +{ // FIXME: Needs to handle partial success TransactionEngineParams parms = openLedger ? tapOPEN_LEDGER : tapNONE; #ifndef TRUST_NETWORK try { #endif - TER result = engine.applyTransaction(*txn, parms); - if (isTerRetry(result)) + bool didApply; + TER result = engine.applyTransaction(*txn, parms, didApply); + if (!didApply && !isTefFailure(result) && !isTemMalformed(result)) { cLog(lsINFO) << " retry"; assert(!ledger->hasTransaction(txn->getTransactionID())); @@ -1108,12 +1109,6 @@ void LedgerConsensus::applyTransaction(TransactionEngine& engine, SerializedTran cLog(lsTRACE) << " success"; assert(ledger->hasTransaction(txn->getTransactionID())); } - else if (isTemMalformed(result) || isTefFailure(result)) - { - cLog(lsINFO) << " hard fail"; - } - else - assert(false); #ifndef TRUST_NETWORK } catch (...) @@ -1160,14 +1155,24 @@ void LedgerConsensus::applyTransactions(SHAMap::ref set, Ledger::ref applyLedger { try { - TER result = engine.applyTransaction(*it->second, parms); - if (result <= 0) - { - if (result == 0) ++successes; + bool didApply; + TER result = engine.applyTransaction(*it->second, parms, didApply); + if (isTelLocal(result)) + { // should never happen + assert(false); + it = failedTransactions.erase(it); + } + else if (didApply) + { // transaction was applied, remove from set + ++successes; + it = failedTransactions.erase(it); + } + else if (isTefFailure(result) || isTemMalformed(result)) + { // transaction cannot apply. tef is expected, tem should never happen it = failedTransactions.erase(it); } else - { + { // try again ++it; } } diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index 00a60898f1..53041814ff 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -91,8 +91,9 @@ Ledger::pointer LedgerMaster::closeLedger(bool recover) { try { - TER result = mEngine.applyTransaction(*it->second, tapOPEN_LEDGER); - if (isTepSuccess(result)) + bool didApply; + mEngine.applyTransaction(*it->second, tapOPEN_LEDGER, didApply); + if (didApply) ++recovers; } catch (...) @@ -112,7 +113,9 @@ Ledger::pointer LedgerMaster::closeLedger(bool recover) TER LedgerMaster::doTransaction(const SerializedTransaction& txn, TransactionEngineParams params) { - TER result = mEngine.applyTransaction(txn, params); + bool didApply; + TER result = mEngine.applyTransaction(txn, params, didApply); + // CHECKME: Should we call this even on gross failures? theApp->getOPs().pubProposedTransaction(mEngine.getLedger(), txn, result); return result; } diff --git a/src/cpp/ripple/TransactionEngine.cpp b/src/cpp/ripple/TransactionEngine.cpp index 1d9b33cc74..cd6b1bd6a1 100644 --- a/src/cpp/ripple/TransactionEngine.cpp +++ b/src/cpp/ripple/TransactionEngine.cpp @@ -67,9 +67,11 @@ void TransactionEngine::txnWrite() } } -TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, TransactionEngineParams params) +TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, TransactionEngineParams params, + bool& didApply) { cLog(lsTRACE) << "applyTransaction>"; + didApply = false; assert(mLedger); mNodes.init(mLedger, txn.getTransactionID(), mLedger->getLedgerSeq()); @@ -110,12 +112,10 @@ TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, Transa cLog(lsINFO) << "applyTransaction: terResult=" << strToken << " : " << terResult << " : " << strHuman; - bool applyTransaction = false; - if (terResult == tesSUCCESS) - applyTransaction = true; + didApply = true; else if (isTepPartial(terResult) && !isSetBit(params, tapRETRY)) - applyTransaction = true; + didApply = true; else if (isTecClaim(terResult) && !isSetBit(params, tapRETRY)) { // only claim the transaction fee mNodes.clear(); @@ -146,14 +146,14 @@ TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, Transa { txnAcct->setFieldAmount(sfBalance, balance - fee); txnAcct->setFieldU32(sfSequence, t_seq + 1); - applyTransaction = true; + didApply = true; entryModify(txnAcct); } } } } - if (applyTransaction) + if (didApply) { // Transaction succeeded fully or (retries are not allowed and the transaction succeeded partially). Serializer m; @@ -183,8 +183,7 @@ TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, Transa mTxnAccount.reset(); mNodes.clear(); - if (!isSetBit(params, tapOPEN_LEDGER) - && (isTemMalformed(terResult) || isTefFailure(terResult))) + if (!isSetBit(params, tapOPEN_LEDGER) && isTemMalformed(terResult)) { // XXX Malformed or failed transaction in closed ledger must bow out. } diff --git a/src/cpp/ripple/TransactionEngine.h b/src/cpp/ripple/TransactionEngine.h index 0139c42918..67a53447b0 100644 --- a/src/cpp/ripple/TransactionEngine.h +++ b/src/cpp/ripple/TransactionEngine.h @@ -78,7 +78,7 @@ public: void entryDelete(SLE::ref sleEntry) { mNodes.entryDelete(sleEntry); } void entryModify(SLE::ref sleEntry) { mNodes.entryModify(sleEntry); } - TER applyTransaction(const SerializedTransaction&, TransactionEngineParams); + TER applyTransaction(const SerializedTransaction&, TransactionEngineParams, bool& didApply); }; inline TransactionEngineParams operator|(const TransactionEngineParams& l1, const TransactionEngineParams& l2)