diff --git a/src/ripple/module/app/consensus/DisputedTx.h b/src/ripple/module/app/consensus/DisputedTx.h index 7278088b3..a36474268 100644 --- a/src/ripple/module/app/consensus/DisputedTx.h +++ b/src/ripple/module/app/consensus/DisputedTx.h @@ -82,8 +82,13 @@ private: ripple::unordered_map mVotes; }; -#define LEDGER_TOTAL_PASSES 8 -#define LEDGER_RETRY_PASSES 5 +// How many total extra passes we make +// We must ensure we make at least one non-retriable pass +#define LEDGER_TOTAL_PASSES 3 + +// How many extra retry passes we +// make if the previous retry pass made changes +#define LEDGER_RETRY_PASSES 1 } // ripple diff --git a/src/ripple/module/app/consensus/LedgerConsensus.cpp b/src/ripple/module/app/consensus/LedgerConsensus.cpp index eda4b3877..0029b7411 100644 --- a/src/ripple/module/app/consensus/LedgerConsensus.cpp +++ b/src/ripple/module/app/consensus/LedgerConsensus.cpp @@ -972,6 +972,7 @@ private: // Apply disputed transactions that didn't get in TransactionEngine engine (newOL); + bool anyDisputes = false; for (auto& it : mDisputes) { if (!it.second->getOurVote ()) @@ -986,10 +987,8 @@ private: SerializedTransaction::pointer txn = std::make_shared(sit); - if (applyTransaction (engine, txn, newOL, true, false)) - { - failedTransactions.push_back (txn); - } + failedTransactions.push_back (txn); + anyDisputes = true; } catch (...) { @@ -998,12 +997,22 @@ private: } } } + if (anyDisputes) + { + applyTransactions (std::shared_ptr(), + newOL, newLCL, failedTransactions, true); + } - WriteLog (lsDEBUG, LedgerConsensus) - << "Applying transactions from current open ledger"; - applyTransactions (getApp().getLedgerMaster ().getCurrentLedger - ()->peekTransactionMap (), newOL, newLCL, - failedTransactions, true); + { + Ledger::pointer oldOL = getApp().getLedgerMaster().getCurrentLedger(); + if (oldOL->peekTransactionMap()->getHash().isNonZero ()) + { + WriteLog (lsDEBUG, LedgerConsensus) + << "Applying transactions from current open ledger"; + applyTransactions (oldOL->peekTransactionMap (), + newOL, newLCL, failedTransactions, true); + } + } { TransactionEngine engine (newOL); @@ -1252,6 +1261,7 @@ private: { TransactionEngine engine (applyLedger); + if (set) for (SHAMapItem::pointer item = set->peekFirstItem (); !!item; item = set->peekNextItem (item->getTag ())) if (!checkLedger->hasTransaction (item->getTag ())) @@ -1334,6 +1344,10 @@ private: if ((!changes) || (pass >= LEDGER_RETRY_PASSES)) certainRetry = false; } + + // If there are any transactions left, we must have + // tried them in at least one final pass + assert (failedTransactions.empty() || !certainRetry); } /** Apply a transaction to a ledger diff --git a/src/ripple/module/app/transactors/Transactor.cpp b/src/ripple/module/app/transactors/Transactor.cpp index 47c7a4f6f..b6b470ed2 100644 --- a/src/ripple/module/app/transactors/Transactor.cpp +++ b/src/ripple/module/app/transactors/Transactor.cpp @@ -111,19 +111,29 @@ TER Transactor::payFee () if (saPaid < zero || !saPaid.isNative ()) return temBAD_FEE; - if (!saPaid) return tesSUCCESS; + if (!saPaid) + return tesSUCCESS; - // Deduct the fee, so it's not available during the transaction. - // Will only write the account back, if the transaction succeeds. if (mSourceBalance < saPaid) { m_journal.trace << "Insufficient balance:" << " balance=" << mSourceBalance.getText () << " paid=" << saPaid.getText (); + if ((mSourceBalance > zero) && (!(mParams & tapOPEN_LEDGER))) + { + // Closed ledger, non-zero balance, less than fee + mSourceBalance.clear (); + mTxnAccount->setFieldAmount (sfBalance, mSourceBalance); + return tecINSUFF_FEE; + } + return terINSUF_FEE_B; } + // Deduct the fee, so it's not available during the transaction. + // Will only write the account back, if the transaction succeeds. + mSourceBalance -= saPaid; mTxnAccount->setFieldAmount (sfBalance, mSourceBalance); diff --git a/src/ripple/module/app/tx/TransactionEngine.cpp b/src/ripple/module/app/tx/TransactionEngine.cpp index 67274705d..eed0e949f 100644 --- a/src/ripple/module/app/tx/TransactionEngine.cpp +++ b/src/ripple/module/app/tx/TransactionEngine.cpp @@ -149,10 +149,19 @@ TER TransactionEngine::applyTransaction (const SerializedTransaction& txn, Trans STAmount fee = txn.getTransactionFee (); STAmount balance = txnAcct->getFieldAmount (sfBalance); - if (balance < fee) + // We retry/reject the transaction if the account + // balance is zero or we're applying against an open + // ledger and the balance is less than the fee + if ((balance == zero) || + ((params & tapOPEN_LEDGER) && (balance < fee))) + { + // Account has no funds or ledger is open terResult = terINSUF_FEE_B; + } else { + if (fee > balance) + fee = balance; txnAcct->setFieldAmount (sfBalance, balance - fee); txnAcct->setFieldU32 (sfSequence, t_seq + 1); entryModify (txnAcct); diff --git a/src/ripple/module/data/protocol/TER.cpp b/src/ripple/module/data/protocol/TER.cpp index 99cbb109c..d2523c0e1 100644 --- a/src/ripple/module/data/protocol/TER.cpp +++ b/src/ripple/module/data/protocol/TER.cpp @@ -50,6 +50,7 @@ bool transResultInfo (TER terCode, std::string& strToken, std::string& strHuman) { tecNO_ISSUER, "tecNO_ISSUER", "Issuer account does not exist." }, { tecNO_AUTH, "tecNO_AUTH", "Not authorized to hold asset." }, { tecNO_LINE, "tecNO_LINE", "No such line." }, + { tecINSUFF_FEE, "tecINSUFF_FEE", "Insufficient balance to pay fee." }, { tefALREADY, "tefALREADY", "The exact transaction was already in this ledger." }, { tefBAD_ADD_AUTH, "tefBAD_ADD_AUTH", "Not authorized to add account." }, diff --git a/src/ripple/module/data/protocol/TER.h b/src/ripple/module/data/protocol/TER.h index 5acfd55bc..cf2c98b91 100644 --- a/src/ripple/module/data/protocol/TER.h +++ b/src/ripple/module/data/protocol/TER.h @@ -183,6 +183,7 @@ enum TER // aka TransactionEngineResult tecNO_ISSUER = 133, tecNO_AUTH = 134, tecNO_LINE = 135, + tecINSUFF_FEE = 136, }; inline bool isTelLocal(TER x)