From 167d13cf40d3bff7a80685ad446ecdf726854ee0 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 24 Feb 2013 16:48:54 -0800 Subject: [PATCH] I think this is the underlying issue. In some cases where the tx return value wasn't full success, we applied to our open ledger but didn't relay. This caused disputes resolved only by dispute relaying. --- src/cpp/ripple/LedgerMaster.cpp | 3 +-- src/cpp/ripple/LedgerMaster.h | 2 +- src/cpp/ripple/NetworkOPs.cpp | 21 +++++++-------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index fbcd2e9567..a3f35cdf26 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -119,9 +119,8 @@ Ledger::pointer LedgerMaster::closeLedger(bool recover) return closingLedger; } -TER LedgerMaster::doTransaction(const SerializedTransaction& txn, TransactionEngineParams params) +TER LedgerMaster::doTransaction(const SerializedTransaction& txn, TransactionEngineParams params, bool& didApply) { - 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); diff --git a/src/cpp/ripple/LedgerMaster.h b/src/cpp/ripple/LedgerMaster.h index 458dd3980b..700f8f2803 100644 --- a/src/cpp/ripple/LedgerMaster.h +++ b/src/cpp/ripple/LedgerMaster.h @@ -75,7 +75,7 @@ public: // The published ledger is the last fully validated ledger Ledger::ref getValidatedLedger() { return mPubLedger; } - TER doTransaction(const SerializedTransaction& txn, TransactionEngineParams params); + TER doTransaction(const SerializedTransaction& txn, TransactionEngineParams params, bool& didApply); void pushLedger(Ledger::ref newLedger); void pushLedger(Ledger::ref newLCL, Ledger::ref newOL, bool fromConsensus); diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index 3e8f6f100a..74df6e366d 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -246,7 +246,9 @@ void NetworkOPs::runTransactionQueue() Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(txn->getID(), true); assert(dbtx); - TER r = mLedgerMaster->doTransaction(*dbtx->getSTransaction(), tapOPEN_LEDGER | tapNO_CHECK_SIGN); + bool didApply; + TER r = mLedgerMaster->doTransaction(*dbtx->getSTransaction(), + tapOPEN_LEDGER | tapNO_CHECK_SIGN, didApply); dbtx->setResult(r); if (isTemMalformed(r)) // malformed, cache bad @@ -255,21 +257,17 @@ void NetworkOPs::runTransactionQueue() theApp->isNewFlag(txn->getID(), SF_RETRY); - bool relay = true; - if (isTerRetry(r)) { // transaction should be held cLog(lsDEBUG) << "Transaction should be held: " << r; dbtx->setStatus(HELD); theApp->getMasterTransaction().canonicalize(dbtx, true); mLedgerMaster->addHeldTransaction(dbtx); - relay = false; } else if (r == tefPAST_SEQ) { // duplicate or conflict cLog(lsINFO) << "Transaction is obsolete"; dbtx->setStatus(OBSOLETE); - relay = false; } else if (r == tesSUCCESS) { @@ -280,12 +278,10 @@ void NetworkOPs::runTransactionQueue() else { cLog(lsDEBUG) << "Status other than success " << r; - if (mMode == omFULL) - relay = false; dbtx->setStatus(INVALID); } - if (relay) + if (didApply || (mMode != omFULL)) { std::set peers; if (theApp->getSuppression().swapSet(txn->getID(), peers, SF_RELAYED)) @@ -335,7 +331,8 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(trans->getID(), true); - TER r = mLedgerMaster->doTransaction(*trans->getSTransaction(), tapOPEN_LEDGER | tapNO_CHECK_SIGN); + bool didApply; + TER r = mLedgerMaster->doTransaction(*trans->getSTransaction(), tapOPEN_LEDGER | tapNO_CHECK_SIGN, didApply); trans->setResult(r); if (isTemMalformed(r)) // malformed, cache bad @@ -372,8 +369,6 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, return trans; } - bool relay = true; - if (r == tesSUCCESS) { cLog(lsINFO) << "Transaction is now included in open ledger"; @@ -383,12 +378,10 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, else { cLog(lsDEBUG) << "Status other than success " << r; - if (mMode == omFULL) - relay = false; trans->setStatus(INVALID); } - if (relay) + if (didApply || (mMode != omFULL)) { std::set peers; if (theApp->getSuppression().swapSet(trans->getID(), peers, SF_RELAYED))