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.
This commit is contained in:
JoelKatz
2013-02-24 16:48:54 -08:00
parent d301cc6128
commit 167d13cf40
3 changed files with 9 additions and 17 deletions

View File

@@ -119,9 +119,8 @@ Ledger::pointer LedgerMaster::closeLedger(bool recover)
return closingLedger; 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); TER result = mEngine.applyTransaction(txn, params, didApply);
// CHECKME: Should we call this even on gross failures? // CHECKME: Should we call this even on gross failures?
theApp->getOPs().pubProposedTransaction(mEngine.getLedger(), txn, result); theApp->getOPs().pubProposedTransaction(mEngine.getLedger(), txn, result);

View File

@@ -75,7 +75,7 @@ public:
// The published ledger is the last fully validated ledger // The published ledger is the last fully validated ledger
Ledger::ref getValidatedLedger() { return mPubLedger; } 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 newLedger);
void pushLedger(Ledger::ref newLCL, Ledger::ref newOL, bool fromConsensus); void pushLedger(Ledger::ref newLCL, Ledger::ref newOL, bool fromConsensus);

View File

@@ -246,7 +246,9 @@ void NetworkOPs::runTransactionQueue()
Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(txn->getID(), true); Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(txn->getID(), true);
assert(dbtx); 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); dbtx->setResult(r);
if (isTemMalformed(r)) // malformed, cache bad if (isTemMalformed(r)) // malformed, cache bad
@@ -255,21 +257,17 @@ void NetworkOPs::runTransactionQueue()
theApp->isNewFlag(txn->getID(), SF_RETRY); theApp->isNewFlag(txn->getID(), SF_RETRY);
bool relay = true;
if (isTerRetry(r)) if (isTerRetry(r))
{ // transaction should be held { // transaction should be held
cLog(lsDEBUG) << "Transaction should be held: " << r; cLog(lsDEBUG) << "Transaction should be held: " << r;
dbtx->setStatus(HELD); dbtx->setStatus(HELD);
theApp->getMasterTransaction().canonicalize(dbtx, true); theApp->getMasterTransaction().canonicalize(dbtx, true);
mLedgerMaster->addHeldTransaction(dbtx); mLedgerMaster->addHeldTransaction(dbtx);
relay = false;
} }
else if (r == tefPAST_SEQ) else if (r == tefPAST_SEQ)
{ // duplicate or conflict { // duplicate or conflict
cLog(lsINFO) << "Transaction is obsolete"; cLog(lsINFO) << "Transaction is obsolete";
dbtx->setStatus(OBSOLETE); dbtx->setStatus(OBSOLETE);
relay = false;
} }
else if (r == tesSUCCESS) else if (r == tesSUCCESS)
{ {
@@ -280,12 +278,10 @@ void NetworkOPs::runTransactionQueue()
else else
{ {
cLog(lsDEBUG) << "Status other than success " << r; cLog(lsDEBUG) << "Status other than success " << r;
if (mMode == omFULL)
relay = false;
dbtx->setStatus(INVALID); dbtx->setStatus(INVALID);
} }
if (relay) if (didApply || (mMode != omFULL))
{ {
std::set<uint64> peers; std::set<uint64> peers;
if (theApp->getSuppression().swapSet(txn->getID(), peers, SF_RELAYED)) 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()); boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock());
Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(trans->getID(), true); 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); trans->setResult(r);
if (isTemMalformed(r)) // malformed, cache bad if (isTemMalformed(r)) // malformed, cache bad
@@ -372,8 +369,6 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans,
return trans; return trans;
} }
bool relay = true;
if (r == tesSUCCESS) if (r == tesSUCCESS)
{ {
cLog(lsINFO) << "Transaction is now included in open ledger"; cLog(lsINFO) << "Transaction is now included in open ledger";
@@ -383,12 +378,10 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans,
else else
{ {
cLog(lsDEBUG) << "Status other than success " << r; cLog(lsDEBUG) << "Status other than success " << r;
if (mMode == omFULL)
relay = false;
trans->setStatus(INVALID); trans->setStatus(INVALID);
} }
if (relay) if (didApply || (mMode != omFULL))
{ {
std::set<uint64> peers; std::set<uint64> peers;
if (theApp->getSuppression().swapSet(trans->getID(), peers, SF_RELAYED)) if (theApp->getSuppression().swapSet(trans->getID(), peers, SF_RELAYED))