From 67c666b033d061ffcbb997b210b34f3632d5fe59 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sat, 28 Mar 2015 19:41:03 -0700 Subject: [PATCH] Clean up LedgerEntrySet and TransactionEngine: * Reduce public interfaces * Remove wrapper functions * Remove freeze timed cutover code * Return results directly instead of via ref parameters --- Builds/VisualStudio2013/RippleD.vcxproj | 8 - .../VisualStudio2013/RippleD.vcxproj.filters | 6 - src/ripple/app/consensus/LedgerConsensus.cpp | 15 +- src/ripple/app/ledger/Ledger.cpp | 30 +--- src/ripple/app/ledger/Ledger.h | 2 - src/ripple/app/ledger/LedgerEntrySet.cpp | 14 +- src/ripple/app/ledger/LedgerEntrySet.h | 12 +- src/ripple/app/ledger/LedgerMaster.cpp | 7 +- src/ripple/app/misc/NetworkOPs.cpp | 1 - src/ripple/app/paths/Pathfinder.cpp | 5 +- src/ripple/app/paths/RippleCalc.cpp | 2 +- src/ripple/app/tests/common_ledger.cpp | 41 +++-- src/ripple/app/transactors/CancelOffer.cpp | 2 +- src/ripple/app/transactors/Change.cpp | 12 +- src/ripple/app/transactors/CreateOffer.cpp | 14 +- src/ripple/app/transactors/CreateTicket.cpp | 4 +- src/ripple/app/transactors/Payment.cpp | 12 +- src/ripple/app/transactors/SetTrust.cpp | 10 +- src/ripple/app/transactors/Transactor.cpp | 4 +- src/ripple/app/tx/LocalTxs.cpp | 4 +- src/ripple/app/tx/TransactionCheck.cpp | 37 ----- src/ripple/app/tx/TransactionEngine.cpp | 147 ++++++++++-------- src/ripple/app/tx/TransactionEngine.h | 61 +++----- src/ripple/legacy/0.27/CreateOffer27.cpp | 10 +- src/ripple/unity/app4.cpp | 1 - 25 files changed, 173 insertions(+), 288 deletions(-) delete mode 100644 src/ripple/app/tx/TransactionCheck.cpp diff --git a/Builds/VisualStudio2013/RippleD.vcxproj b/Builds/VisualStudio2013/RippleD.vcxproj index 4c15666f0f..4f7ef5c535 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj +++ b/Builds/VisualStudio2013/RippleD.vcxproj @@ -2060,12 +2060,6 @@ - - True - True - ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) - ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) - True True @@ -4518,8 +4512,6 @@ - - diff --git a/Builds/VisualStudio2013/RippleD.vcxproj.filters b/Builds/VisualStudio2013/RippleD.vcxproj.filters index bf37a52627..9269636857 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2013/RippleD.vcxproj.filters @@ -2613,9 +2613,6 @@ ripple\app\tx - - ripple\app\tx - ripple\app\tx @@ -5289,9 +5286,6 @@ soci\src\core - - soci\src\core - sqlite diff --git a/src/ripple/app/consensus/LedgerConsensus.cpp b/src/ripple/app/consensus/LedgerConsensus.cpp index 7d8b854389..050c5e4136 100644 --- a/src/ripple/app/consensus/LedgerConsensus.cpp +++ b/src/ripple/app/consensus/LedgerConsensus.cpp @@ -1852,27 +1852,26 @@ int applyTransaction (TransactionEngine& engine try { - bool didApply; - TER result = engine.applyTransaction (*txn, parms, didApply); + auto result = engine.applyTransaction (*txn, parms); - if (didApply) + if (result.second) { WriteLog (lsDEBUG, LedgerConsensus) - << "Transaction success: " << transHuman (result); + << "Transaction applied: " << transHuman (result.first); return LedgerConsensusImp::resultSuccess; } - if (isTefFailure (result) || isTemMalformed (result) || - isTelLocal (result)) + if (isTefFailure (result.first) || isTemMalformed (result.first) || + isTelLocal (result.first)) { // failure WriteLog (lsDEBUG, LedgerConsensus) - << "Transaction failure: " << transHuman (result); + << "Transaction failure: " << transHuman (result.first); return LedgerConsensusImp::resultFail; } WriteLog (lsDEBUG, LedgerConsensus) - << "Transaction retry: " << transHuman (result); + << "Transaction retry: " << transHuman (result.first); return LedgerConsensusImp::resultRetry; } catch (...) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 0f65e9a318..ac550373d3 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -261,30 +261,6 @@ Ledger::~Ledger () } } -bool Ledger::enforceFreeze () const -{ - - // Temporarily, the freze code can run in either - // enforcing mode or non-enforcing mode. In - // non-enforcing mode, freeze flags can be - // manipulated, but freezing is not actually - // enforced. Once freeze enforcing has been - // enabled, this function can be removed - - // Let freeze enforcement be tested - // If you wish to test non-enforcing mode, - // you must remove this line - if (getConfig().RUN_STANDALONE) - return true; - - // Freeze enforcing date is September 15, 2014 - static std::uint32_t const enforceDate = - iToSeconds (boost::posix_time::ptime ( - boost::gregorian::date (2014, boost::gregorian::Sep, 15))); - - return mParentCloseTime >= enforceDate; -} - void Ledger::setImmutable () { // Updates the hash and marks the ledger and its maps immutable @@ -766,7 +742,7 @@ bool Ledger::saveValidatedLedger (bool current) << "Transaction in ledger " << mLedgerSeq << " affects no accounts"; - *db << + *db << (STTx::getMetaSQLInsertReplaceHeader () + vt.second->getTxn ()->getMetaSQL ( getLedgerSeq (), vt.second->getEscMeta ()) + ";"); @@ -953,7 +929,7 @@ bool Ledger::getHashesByIndex ( auto db = getApp().getLedgerDB ().checkoutDb (); boost::optional lhO, phO; - + *db << "SELECT LedgerHash,PrevHash FROM Ledgers " "INDEXED BY SeqLedger Where LedgerSeq = :ls;", soci::into (lhO), @@ -1024,7 +1000,7 @@ Ledger::pointer Ledger::getLastFullLedger () if (!ledger) return ledger; - + ledger->setClosed (); if (getApp().getOPs ().haveLedger (ledgerSeq)) diff --git a/src/ripple/app/ledger/Ledger.h b/src/ripple/app/ledger/Ledger.h index 48d33f0404..42630f0b39 100644 --- a/src/ripple/app/ledger/Ledger.h +++ b/src/ripple/app/ledger/Ledger.h @@ -181,8 +181,6 @@ public: mAccountStateMap->setLedgerSeq (mLedgerSeq); } - bool enforceFreeze () const; - // ledger signature operations void addRaw (Serializer & s) const; void setRaw (Serializer & s, bool hasPrefix); diff --git a/src/ripple/app/ledger/LedgerEntrySet.cpp b/src/ripple/app/ledger/LedgerEntrySet.cpp index 02f9190788..521c0ab6ce 100644 --- a/src/ripple/app/ledger/LedgerEntrySet.cpp +++ b/src/ripple/app/ledger/LedgerEntrySet.cpp @@ -122,16 +122,6 @@ SLE::pointer LedgerEntrySet::entryCache (LedgerEntryType letType, uint256 const& return sleEntry; } -LedgerEntryAction LedgerEntrySet::hasEntry (uint256 const& index) const -{ - std::map::const_iterator it = mEntries.find (index); - - if (it == mEntries.end ()) - return taaNONE; - - return it->second.mAction; -} - void LedgerEntrySet::entryCache (SLE::ref sle) { assert (mLedger); @@ -1187,7 +1177,7 @@ STAmount LedgerEntrySet::accountHolds ( bool LedgerEntrySet::isGlobalFrozen (Account const& issuer) { - if (!enforceFreeze () || isXRP (issuer)) + if (isXRP (issuer)) return false; SLE::pointer sle = entryCache (ltACCOUNT_ROOT, getAccountRootIndex (issuer)); @@ -1204,7 +1194,7 @@ bool LedgerEntrySet::isFrozen( Currency const& currency, Account const& issuer) { - if (!enforceFreeze () || isXRP (currency)) + if (isXRP (currency)) return false; SLE::pointer sle = entryCache (ltACCOUNT_ROOT, getAccountRootIndex (issuer)); diff --git a/src/ripple/app/ledger/LedgerEntrySet.h b/src/ripple/app/ledger/LedgerEntrySet.h index 1f98b4c303..3db9e29a7f 100644 --- a/src/ripple/app/ledger/LedgerEntrySet.h +++ b/src/ripple/app/ledger/LedgerEntrySet.h @@ -126,11 +126,6 @@ public: return mSeq; } - TransactionEngineParams getParams () const - { - return mParams; - } - void bumpSeq () { ++mSeq; @@ -146,14 +141,9 @@ public: return mLedger; } - bool enforceFreeze () const - { - return mLedger->enforceFreeze (); - } - // basic entry functions SLE::pointer getEntry (uint256 const& index, LedgerEntryAction&); - LedgerEntryAction hasEntry (uint256 const& index) const; + void entryCache (SLE::ref); // Add this entry to the cache void entryCreate (SLE::ref); // This entry will be created void entryDelete (SLE::ref); // This entry will be deleted diff --git a/src/ripple/app/ledger/LedgerMaster.cpp b/src/ripple/app/ledger/LedgerMaster.cpp index 574601530c..1b3f995638 100644 --- a/src/ripple/app/ledger/LedgerMaster.cpp +++ b/src/ripple/app/ledger/LedgerMaster.cpp @@ -344,10 +344,9 @@ public: if (getApp().getHashRouter ().addSuppressionFlags (it.first.getTXID (), SF_SIGGOOD)) tepFlags = static_cast (tepFlags | tapNO_CHECK_SIGN); - bool didApply; - engine.applyTransaction (*it.second, tepFlags, didApply); + auto ret = engine.applyTransaction (*it.second, tepFlags); - if (didApply) + if (ret.second) ++recovers; // If a transaction is recovered but hasn't been relayed, @@ -390,7 +389,7 @@ public: ScopedLockType sl (m_mutex); ledger = mCurrentLedger.getMutable (); engine.setLedger (ledger); - result = engine.applyTransaction (*txn, params, didApply); + std::tie(result, didApply) = engine.applyTransaction (*txn, params); } if (didApply) { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 92c59e3564..f3b6ba49b5 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1045,7 +1045,6 @@ Transaction::pointer NetworkOPsImp::processTransactionCb ( if (callback) callback (trans, r); - if (r == tefFAILURE) throw Fault (IO_ERROR); diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 2950abd85a..8a41f995b2 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -710,8 +710,7 @@ int Pathfinder::getPathsOut ( int aFlags = sleAccount->getFieldU32 (sfFlags); bool const bAuthRequired = (aFlags & lsfRequireAuth) != 0; - bool const bFrozen = ((aFlags & lsfGlobalFreeze) != 0) - && mLedger->enforceFreeze (); + bool const bFrozen = ((aFlags & lsfGlobalFreeze) != 0); int count = 0; @@ -741,7 +740,7 @@ int Pathfinder::getPathsOut ( { // This probably isn't a useful path out } - else if (rspEntry->getFreezePeer () && mLedger->enforceFreeze ()) + else if (rspEntry->getFreezePeer ()) { // Not a useful path out } diff --git a/src/ripple/app/paths/RippleCalc.cpp b/src/ripple/app/paths/RippleCalc.cpp index 446521539f..c47600982c 100644 --- a/src/ripple/app/paths/RippleCalc.cpp +++ b/src/ripple/app/paths/RippleCalc.cpp @@ -109,7 +109,7 @@ bool RippleCalc::addPathState(STPath const& path, TER& resultCode) if (pathState->status() == tesSUCCESS) pathState->checkNoRipple (uDstAccountID_, uSrcAccountID_); - if (pathState->status() == tesSUCCESS && mActiveLedger.enforceFreeze ()) + if (pathState->status() == tesSUCCESS) pathState->checkFreeze (); pathState->setIndex (pathStateList_.size ()); diff --git a/src/ripple/app/tests/common_ledger.cpp b/src/ripple/app/tests/common_ledger.cpp index f2bc27670e..d76a708f4a 100644 --- a/src/ripple/app/tests/common_ledger.cpp +++ b/src/ripple/app/tests/common_ledger.cpp @@ -128,15 +128,12 @@ void applyTransaction(Ledger::pointer const& ledger, STTx const& tx, bool check) { TransactionEngine engine(ledger); - bool didApply = false; - auto r = engine.applyTransaction(tx, tapOPEN_LEDGER | (check ? tapNONE : tapNO_CHECK_SIGN), - didApply); - if (r != tesSUCCESS) - throw std::runtime_error( - "r != tesSUCCESS"); - if (!didApply) - throw std::runtime_error( - "didApply"); + auto r = engine.applyTransaction(tx, + tapOPEN_LEDGER | (check ? tapNONE : tapNO_CHECK_SIGN)); + if (r.first != tesSUCCESS) + throw std::runtime_error("r != tesSUCCESS"); + if (!r.second) + throw std::runtime_error("didApply"); } // Create genesis ledger from a start amount in drops, and the public @@ -173,7 +170,7 @@ createAccount(std::string const& passphrase, KeyType keyType) } TestAccount -createAndFundAccount(TestAccount& from, std::string const& passphrase, +createAndFundAccount(TestAccount& from, std::string const& passphrase, KeyType keyType, std::uint64_t amountDrops, Ledger::pointer const& ledger, bool sign) { @@ -198,14 +195,14 @@ createAndFundAccounts(TestAccount& from, std::vector passphrases, } std::map -createAndFundAccountsWithFlags(TestAccount& from, +createAndFundAccountsWithFlags(TestAccount& from, std::vector passphrases, KeyType keyType, std::uint64_t amountDrops, - Ledger::pointer& ledger, + Ledger::pointer& ledger, Ledger::pointer& LCL, const std::uint32_t flags, bool sign) { - auto accounts = createAndFundAccounts(from, + auto accounts = createAndFundAccounts(from, passphrases, keyType, amountDrops, ledger, sign); close_and_advance(ledger, LCL); setAllAccountFlags(accounts, ledger, flags); @@ -244,7 +241,7 @@ const std::uint32_t flags, bool sign) } } -void +void clearAccountFlags(TestAccount& account, Ledger::pointer const& ledger, const std::uint32_t flags, bool sign) { @@ -284,7 +281,7 @@ getPaymentTx(TestAccount& from, TestAccount const& to, std::uint64_t amountDrops, bool sign) { - Json::Value tx_json = getPaymentJson(from, to, + Json::Value tx_json = getPaymentJson(from, to, std::to_string(amountDrops)); return parseTransaction(from, tx_json, sign); } @@ -451,7 +448,7 @@ Json::Value findPath(Ledger::pointer ledger, TestAccount const& src, } log << "Source currencies: " << jvSrcCurrencies; - auto result = ripplePathFind(cache, src.pk, dest.pk, saDstAmount, + auto result = ripplePathFind(cache, src.pk, dest.pk, saDstAmount, ledger, jvSrcCurrencies, contextPaths, level); if(!result.first) throw std::runtime_error( @@ -461,12 +458,12 @@ Json::Value findPath(Ledger::pointer ledger, TestAccount const& src, } SLE::pointer -getLedgerEntryRippleState(Ledger::pointer ledger, - TestAccount const& account1, TestAccount const& account2, +getLedgerEntryRippleState(Ledger::pointer ledger, + TestAccount const& account1, TestAccount const& account2, Currency currency) { auto uNodeIndex = getRippleStateIndex( - account1.pk.getAccountID(), account2.pk.getAccountID(), + account1.pk.getAccountID(), account2.pk.getAccountID(), to_currency(currency.getCurrency())); if (!uNodeIndex.isNonZero()) @@ -477,10 +474,10 @@ getLedgerEntryRippleState(Ledger::pointer ledger, } void -verifyBalance(Ledger::pointer ledger, TestAccount const& account, +verifyBalance(Ledger::pointer ledger, TestAccount const& account, Amount const& amount) { - auto sle = getLedgerEntryRippleState(ledger, account, + auto sle = getLedgerEntryRippleState(ledger, account, amount.getIssuer(), amount.getCurrency()); if (!sle) throw std::runtime_error( @@ -500,4 +497,4 @@ verifyBalance(Ledger::pointer ledger, TestAccount const& account, } } -} \ No newline at end of file +} diff --git a/src/ripple/app/transactors/CancelOffer.cpp b/src/ripple/app/transactors/CancelOffer.cpp index 45e0c8775c..0ff4ff7d11 100644 --- a/src/ripple/app/transactors/CancelOffer.cpp +++ b/src/ripple/app/transactors/CancelOffer.cpp @@ -78,7 +78,7 @@ public: uint256 const offerIndex (getOfferIndex (mTxnAccountID, uOfferSequence)); - SLE::pointer sleOffer (mEngine->entryCache (ltOFFER, + SLE::pointer sleOffer (mEngine->view().entryCache (ltOFFER, offerIndex)); if (sleOffer) diff --git a/src/ripple/app/transactors/Change.cpp b/src/ripple/app/transactors/Change.cpp index 5e4dda2a9b..23fb3627e3 100644 --- a/src/ripple/app/transactors/Change.cpp +++ b/src/ripple/app/transactors/Change.cpp @@ -119,10 +119,10 @@ private: auto const index = getLedgerAmendmentIndex (); - SLE::pointer amendmentObject (mEngine->entryCache (ltAMENDMENTS, index)); + SLE::pointer amendmentObject (mEngine->view().entryCache (ltAMENDMENTS, index)); if (!amendmentObject) - amendmentObject = mEngine->entryCreate(ltAMENDMENTS, index); + amendmentObject = mEngine->view().entryCreate(ltAMENDMENTS, index); STVector256 amendments (amendmentObject->getFieldV256 (sfAmendments)); @@ -134,7 +134,7 @@ private: amendments.push_back (amendment); amendmentObject->setFieldV256 (sfAmendments, amendments); - mEngine->entryModify (amendmentObject); + mEngine->view().entryModify (amendmentObject); getApp().getAmendmentTable ().enable (amendment); @@ -148,10 +148,10 @@ private: { auto const index = getLedgerFeeIndex (); - SLE::pointer feeObject = mEngine->entryCache (ltFEE_SETTINGS, index); + SLE::pointer feeObject = mEngine->view().entryCache (ltFEE_SETTINGS, index); if (!feeObject) - feeObject = mEngine->entryCreate (ltFEE_SETTINGS, index); + feeObject = mEngine->view().entryCreate (ltFEE_SETTINGS, index); // VFALCO-FIXME this generates errors // m_journal.trace << @@ -166,7 +166,7 @@ private: feeObject->setFieldU32 ( sfReserveIncrement, mTxn.getFieldU32 (sfReserveIncrement)); - mEngine->entryModify (feeObject); + mEngine->view().entryModify (feeObject); // VFALCO-FIXME this generates errors // m_journal.trace << diff --git a/src/ripple/app/transactors/CreateOffer.cpp b/src/ripple/app/transactors/CreateOffer.cpp index 409a507ee1..33c366a593 100644 --- a/src/ripple/app/transactors/CreateOffer.cpp +++ b/src/ripple/app/transactors/CreateOffer.cpp @@ -49,7 +49,7 @@ private: // Only valid for custom currencies assert (!isXRP (issue.currency)); - SLE::pointer const issuerAccount = mEngine->entryCache ( + SLE::pointer const issuerAccount = mEngine->view().entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (issue.account)); if (!issuerAccount) @@ -65,7 +65,7 @@ private: if (issuerAccount->getFieldU32 (sfFlags) & lsfRequireAuth) { - SLE::pointer const trustLine (mEngine->entryCache ( + SLE::pointer const trustLine (mEngine->view().entryCache ( ltRIPPLE_STATE, getRippleStateIndex ( mTxnAccountID, issue.account, issue.currency))); @@ -486,14 +486,14 @@ public: } bool const bHaveExpiration (mTxn.isFieldPresent (sfExpiration)); - + if (bHaveExpiration && (mTxn.getFieldU32 (sfExpiration) == 0)) { if (m_journal.debug) m_journal.warning << "Malformed offer: bad expiration"; return temBAD_EXPIRATION; } - + bool const bHaveCancel (mTxn.isFieldPresent (sfOfferSequence)); if (bHaveCancel && (mTxn.getFieldU32 (sfOfferSequence) == 0)) @@ -604,7 +604,7 @@ public: view.bumpSeq (); // Begin ledger variance. - SLE::pointer sleCreator = mEngine->entryCache ( + SLE::pointer sleCreator = mEngine->view().entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (mTxnAccountID)); if (view.isGlobalFrozen (uPaysIssuerID) || view.isGlobalFrozen (uGetsIssuerID)) @@ -642,7 +642,7 @@ public: // Process a cancellation request that's passed along with an offer. if (bHaveCancel) { - SLE::pointer sleCancel = mEngine->entryCache (ltOFFER, + SLE::pointer sleCancel = mEngine->view().entryCache (ltOFFER, getOfferIndex (mTxnAccountID, uCancelSequence)); // It's not an error to not find the offer to cancel: it might have @@ -831,7 +831,7 @@ public: if (result == tesSUCCESS) { - SLE::pointer sleOffer (mEngine->entryCreate (ltOFFER, offer_index)); + SLE::pointer sleOffer (mEngine->view().entryCreate (ltOFFER, offer_index)); sleOffer->setFieldAccount (sfAccount, mTxnAccountID); sleOffer->setFieldU32 (sfSequence, uSequence); diff --git a/src/ripple/app/transactors/CreateTicket.cpp b/src/ripple/app/transactors/CreateTicket.cpp index 5f3294b594..e27ee0108c 100644 --- a/src/ripple/app/transactors/CreateTicket.cpp +++ b/src/ripple/app/transactors/CreateTicket.cpp @@ -80,7 +80,7 @@ public: return tesSUCCESS; } - SLE::pointer sleTicket = mEngine->entryCreate (ltTICKET, + SLE::pointer sleTicket = mEngine->view().entryCreate (ltTICKET, getTicketIndex (mTxnAccountID, mTxn.getSequence ())); sleTicket->setFieldAccount (sfAccount, mTxnAccountID); @@ -93,7 +93,7 @@ public: { Account const target_account (mTxn.getFieldAccount160 (sfTarget)); - SLE::pointer sleTarget = mEngine->entryCache (ltACCOUNT_ROOT, + SLE::pointer sleTarget = mEngine->view().entryCache (ltACCOUNT_ROOT, getAccountRootIndex (target_account)); // Destination account does not exist. diff --git a/src/ripple/app/transactors/Payment.cpp b/src/ripple/app/transactors/Payment.cpp index e4504b0ce6..32dd4ac576 100644 --- a/src/ripple/app/transactors/Payment.cpp +++ b/src/ripple/app/transactors/Payment.cpp @@ -66,11 +66,11 @@ public: bool const defaultPathsAllowed = !(uTxFlags & tfNoRippleDirect); bool const bPaths = mTxn.isFieldPresent (sfPaths); bool const bMax = mTxn.isFieldPresent (sfSendMax); - + STAmount const saDstAmount (mTxn.getFieldAmount (sfAmount)); - + STAmount maxSourceAmount; - + if (bMax) maxSourceAmount = mTxn.getFieldAmount (sfSendMax); else if (saDstAmount.isNative ()) @@ -192,7 +192,7 @@ public: // Open a ledger for editing. auto const index = getAccountRootIndex (uDstAccountID); - SLE::pointer sleDst (mEngine->entryCache (ltACCOUNT_ROOT, index)); + SLE::pointer sleDst (mEngine->view().entryCache (ltACCOUNT_ROOT, index)); if (!sleDst) { @@ -234,7 +234,7 @@ public: // Create the account. auto const newIndex = getAccountRootIndex (uDstAccountID); - sleDst = mEngine->entryCreate (ltACCOUNT_ROOT, newIndex); + sleDst = mEngine->view().entryCreate (ltACCOUNT_ROOT, newIndex); sleDst->setFieldAccount (sfAccount, uDstAccountID); sleDst->setFieldU32 (sfSequence, 1); } @@ -255,7 +255,7 @@ public: // Tell the engine that we are intending to change the the destination // account. The source account gets always charged a fee so it's always // marked as modified. - mEngine->entryModify (sleDst); + mEngine->view().entryModify (sleDst); } TER terResult; diff --git a/src/ripple/app/transactors/SetTrust.cpp b/src/ripple/app/transactors/SetTrust.cpp index 72fe83b373..4c2dd2d52b 100644 --- a/src/ripple/app/transactors/SetTrust.cpp +++ b/src/ripple/app/transactors/SetTrust.cpp @@ -82,7 +82,7 @@ public: // Check if destination makes sense. auto const& issuer = saLimitAmount.getIssuer (); - + if (!issuer || issuer == noAccount()) { if (m_journal.trace) m_journal.trace << @@ -153,7 +153,7 @@ public: // lines exist now, why not remove this code and simply // return an error? SLE::pointer selDelete ( - mEngine->entryCache (ltRIPPLE_STATE, + mEngine->view().entryCache (ltRIPPLE_STATE, getRippleStateIndex ( mTxnAccountID, uDstAccountID, currency))); @@ -173,7 +173,7 @@ public: } } - SLE::pointer sleDst (mEngine->entryCache ( + SLE::pointer sleDst (mEngine->view().entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (uDstAccountID))); if (!sleDst) @@ -186,7 +186,7 @@ public: STAmount saLimitAllow = saLimitAmount; saLimitAllow.setIssuer (mTxnAccountID); - SLE::pointer sleRippleState (mEngine->entryCache (ltRIPPLE_STATE, + SLE::pointer sleRippleState (mEngine->view().entryCache (ltRIPPLE_STATE, getRippleStateIndex (mTxnAccountID, uDstAccountID, currency))); if (sleRippleState) @@ -391,7 +391,7 @@ public: } else { - mEngine->entryModify (sleRippleState); + mEngine->view().entryModify (sleRippleState); m_journal.trace << "Modify ripple line"; } diff --git a/src/ripple/app/transactors/Transactor.cpp b/src/ripple/app/transactors/Transactor.cpp index 5637882142..e55eb8614d 100644 --- a/src/ripple/app/transactors/Transactor.cpp +++ b/src/ripple/app/transactors/Transactor.cpp @@ -266,7 +266,7 @@ TER Transactor::apply () return terResult; // Find source account - mTxnAccount = mEngine->entryCache (ltACCOUNT_ROOT, + mTxnAccount = mEngine->view().entryCache (ltACCOUNT_ROOT, getAccountRootIndex (mTxnAccountID)); calculateFee (); @@ -303,7 +303,7 @@ TER Transactor::apply () if (terResult != tesSUCCESS) return (terResult); if (mTxnAccount) - mEngine->entryModify (mTxnAccount); + mEngine->view().entryModify (mTxnAccount); return doApply (); } diff --git a/src/ripple/app/tx/LocalTxs.cpp b/src/ripple/app/tx/LocalTxs.cpp index 1a97f9ce3e..4e690b738f 100644 --- a/src/ripple/app/tx/LocalTxs.cpp +++ b/src/ripple/app/tx/LocalTxs.cpp @@ -156,9 +156,7 @@ public: { try { - TransactionEngineParams parms = tapOPEN_LEDGER; - bool didApply; - engine.applyTransaction (*it.second, parms, didApply); + engine.applyTransaction (*it.second, tapOPEN_LEDGER); } catch (...) { diff --git a/src/ripple/app/tx/TransactionCheck.cpp b/src/ripple/app/tx/TransactionCheck.cpp deleted file mode 100644 index 82478524c8..0000000000 --- a/src/ripple/app/tx/TransactionCheck.cpp +++ /dev/null @@ -1,37 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include - -namespace ripple { - -// VFALCO TODO move this into TransactionEngine.cpp - -// Double check a transaction's metadata to make sure no system invariants were broken - -bool TransactionEngine::checkInvariants (TER result, const STTx& txn, TransactionEngineParams params) -{ - // VFALCO I deleted a bunch of code that was wrapped in #if 0. - // If you need it, check the commit log. - - return true; -} - -} // ripple diff --git a/src/ripple/app/tx/TransactionEngine.cpp b/src/ripple/app/tx/TransactionEngine.cpp index 23ee386705..dc6cd07ded 100644 --- a/src/ripple/app/tx/TransactionEngine.cpp +++ b/src/ripple/app/tx/TransactionEngine.cpp @@ -80,15 +80,25 @@ void TransactionEngine::txnWrite () } } -TER TransactionEngine::applyTransaction ( +std::pair +TransactionEngine::applyTransaction ( STTx const& txn, - TransactionEngineParams params, - bool& didApply) + TransactionEngineParams params) { - WriteLog (lsTRACE, TransactionEngine) << "applyTransaction>"; - didApply = false; assert (mLedger); - mNodes.init (mLedger, txn.getTransactionID (), mLedger->getLedgerSeq (), params); + + WriteLog (lsTRACE, TransactionEngine) << "applyTransaction>"; + + uint256 const& txID = txn.getTransactionID (); + + if (!txID) + { + WriteLog (lsWARNING, TransactionEngine) << + "applyTransaction: invalid transaction id"; + return std::make_pair(temINVALID_FLAG, false); + } + + mNodes.init (mLedger, txID, mLedger->getLedgerSeq (), params); #ifdef BEAST_DEBUG if (1) @@ -109,22 +119,13 @@ TER TransactionEngine::applyTransaction ( } #endif - uint256 const& txID = txn.getTransactionID (); - - if (!txID) - { - WriteLog (lsWARNING, TransactionEngine) << - "applyTransaction: invalid transaction id"; - return temINVALID; - } - TER terResult = Transactor::transact (txn, params, this); if (terResult == temUNKNOWN) { WriteLog (lsWARNING, TransactionEngine) << "applyTransaction: Invalid transaction: unknown transaction type"; - return temUNKNOWN; + return std::make_pair(temUNKNOWN, false); } if (ShouldLog (lsDEBUG, TransactionEngine)) @@ -140,15 +141,16 @@ TER TransactionEngine::applyTransaction ( " : " << strHuman; } - if (isTesSuccess (terResult)) - didApply = true; - else if (isTecClaim (terResult) && !(params & tapRETRY)) + bool didApply = isTesSuccess (terResult); + + if (isTecClaim (terResult) && !(params & tapRETRY)) { // only claim the transaction fee - WriteLog (lsDEBUG, TransactionEngine) << "Reprocessing to only claim fee"; + WriteLog (lsDEBUG, TransactionEngine) << + "Reprocessing tx " << txID << " to only claim fee"; mNodes.clear (); - SLE::pointer txnAcct = entryCache (ltACCOUNT_ROOT, + SLE::pointer txnAcct = mNodes.entryCache (ltACCOUNT_ROOT, getAccountRootIndex (txn.getSourceAccount ())); if (!txnAcct) @@ -182,70 +184,69 @@ TER TransactionEngine::applyTransaction ( fee = balance; txnAcct->setFieldAmount (sfBalance, balance - fee); txnAcct->setFieldU32 (sfSequence, t_seq + 1); - entryModify (txnAcct); + mNodes.entryModify (txnAcct); didApply = true; } } } } - else + else if (!didApply) + { WriteLog (lsDEBUG, TransactionEngine) << "Not applying transaction " << txID; + } + + if (didApply && !checkInvariants (terResult, txn, params)) + { + WriteLog (lsFATAL, TransactionEngine) << + "Transaction violates invariants"; + WriteLog (lsFATAL, TransactionEngine) << + txn.getJson (0); + WriteLog (lsFATAL, TransactionEngine) << + transToken (terResult) << ": " << transHuman (terResult); + WriteLog (lsFATAL, TransactionEngine) << + mNodes.getJson (0); + didApply = false; + terResult = tefINTERNAL; + } if (didApply) { - if (!checkInvariants (terResult, txn, params)) + // Transaction succeeded fully or (retries are not allowed and the + // transaction could claim a fee) + Serializer m; + mNodes.calcRawMeta (m, terResult, mTxnSeq++); + + txnWrite (); + + Serializer s; + txn.add (s); + + if (params & tapOPEN_LEDGER) { - WriteLog (lsFATAL, TransactionEngine) << - "Transaction violates invariants"; - WriteLog (lsFATAL, TransactionEngine) << - txn.getJson (0); - WriteLog (lsFATAL, TransactionEngine) << - transToken (terResult) << ": " << transHuman (terResult); - WriteLog (lsFATAL, TransactionEngine) << - mNodes.getJson (0); - didApply = false; - terResult = tefINTERNAL; + if (!mLedger->addTransaction (txID, s)) + { + WriteLog (lsFATAL, TransactionEngine) << + "Duplicate transaction applied"; + assert (false); + throw std::runtime_error ("Duplicate transaction applied"); + } } else { - // Transaction succeeded fully or (retries are not allowed and the - // transaction could claim a fee) - Serializer m; - mNodes.calcRawMeta (m, terResult, mTxnSeq++); - - txnWrite (); - - Serializer s; - txn.add (s); - - if (params & tapOPEN_LEDGER) + if (!mLedger->addTransaction (txID, s, m)) { - if (!mLedger->addTransaction (txID, s)) - { - WriteLog (lsFATAL, TransactionEngine) << - "Tried to add transaction to open ledger that already had it"; - assert (false); - throw std::runtime_error ("Duplicate transaction applied"); - } + WriteLog (lsFATAL, TransactionEngine) << + "Duplicate transaction applied to closed ledger"; + assert (false); + throw std::runtime_error ("Duplicate transaction applied to closed ledger"); } - else - { - if (!mLedger->addTransaction (txID, s, m)) - { - WriteLog (lsFATAL, TransactionEngine) << - "Tried to add transaction to ledger that already had it"; - assert (false); - throw std::runtime_error ("Duplicate transaction applied to closed ledger"); - } - // Charge whatever fee they specified. - STAmount saPaid = txn.getTransactionFee (); - mLedger->destroyCoins (saPaid.getNValue ()); - } + // Charge whatever fee they specified. + STAmount saPaid = txn.getTransactionFee (); + mLedger->destroyCoins (saPaid.getNValue ()); } } - mTxnAccount.reset (); mNodes.clear (); if (!(params & tapOPEN_LEDGER) && isTemMalformed (terResult)) @@ -253,7 +254,19 @@ TER TransactionEngine::applyTransaction ( // XXX Malformed or failed transaction in closed ledger must bow out. } - return terResult; + return { terResult, didApply }; +} + +bool +TransactionEngine::checkInvariants ( + TER result, + STTx const& txn, + TransactionEngineParams params) +{ + // VFALCO I deleted a bunch of code that was wrapped in #if 0. + // If you need it, check the commit log. + + return true; } } // ripple diff --git a/src/ripple/app/tx/TransactionEngine.h b/src/ripple/app/tx/TransactionEngine.h index 5d831733b6..dbfdf5f694 100644 --- a/src/ripple/app/tx/TransactionEngine.h +++ b/src/ripple/app/tx/TransactionEngine.h @@ -22,6 +22,7 @@ #include #include +#include namespace ripple { @@ -37,69 +38,47 @@ public: static char const* getCountedObjectName () { return "TransactionEngine"; } private: - LedgerEntrySet mNodes; + LedgerEntrySet mNodes; - TER setAuthorized (const STTx & txn, bool bMustSetGenerator); - TER checkSig (const STTx & txn); + void txnWrite (); protected: - Ledger::pointer mLedger; - int mTxnSeq; - - Account mTxnAccountID; - SLE::pointer mTxnAccount; - - void txnWrite (); + Ledger::pointer mLedger; + int mTxnSeq = 0; public: - typedef std::shared_ptr pointer; + TransactionEngine () = default; - TransactionEngine () : mTxnSeq (0) - { - ; - } - TransactionEngine (Ledger::ref ledger) : mLedger (ledger), mTxnSeq (0) + TransactionEngine (Ledger::ref ledger) + : mLedger (ledger) { assert (mLedger); } - LedgerEntrySet& view () + LedgerEntrySet& + view () { return mNodes; } - Ledger::ref getLedger () + + Ledger::ref + getLedger () { return mLedger; } - void setLedger (Ledger::ref ledger) + + void + setLedger (Ledger::ref ledger) { assert (ledger); mLedger = ledger; } - // VFALCO TODO Remove these pointless wrappers - SLE::pointer entryCreate (LedgerEntryType type, uint256 const& index) - { - return mNodes.entryCreate (type, index); - } + std::pair + applyTransaction (STTx const&, TransactionEngineParams); - SLE::pointer entryCache (LedgerEntryType type, uint256 const& index) - { - return mNodes.entryCache (type, index); - } - - void entryDelete (SLE::ref sleEntry) - { - mNodes.entryDelete (sleEntry); - } - - void entryModify (SLE::ref sleEntry) - { - mNodes.entryModify (sleEntry); - } - - TER applyTransaction (const STTx&, TransactionEngineParams, bool & didApply); - bool checkInvariants (TER result, const STTx & txn, TransactionEngineParams params); + bool + checkInvariants (TER result, STTx const& txn, TransactionEngineParams params); }; inline TransactionEngineParams operator| (const TransactionEngineParams& l1, const TransactionEngineParams& l2) diff --git a/src/ripple/legacy/0.27/CreateOffer27.cpp b/src/ripple/legacy/0.27/CreateOffer27.cpp index e9ea2d68d9..8b4056f21c 100644 --- a/src/ripple/legacy/0.27/CreateOffer27.cpp +++ b/src/ripple/legacy/0.27/CreateOffer27.cpp @@ -39,7 +39,7 @@ CreateOffer::checkAcceptAsset(IssueRef issue) const /* Only valid for custom currencies */ assert (!isXRP (issue.currency)); - SLE::pointer const issuerAccount = mEngine->entryCache ( + SLE::pointer const issuerAccount = mEngine->view().entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (issue.account)); if (!issuerAccount) @@ -55,7 +55,7 @@ CreateOffer::checkAcceptAsset(IssueRef issue) const if (issuerAccount->getFieldU32 (sfFlags) & lsfRequireAuth) { - SLE::pointer const trustLine (mEngine->entryCache ( + SLE::pointer const trustLine (mEngine->view().entryCache ( ltRIPPLE_STATE, getRippleStateIndex ( mTxnAccountID, issue.account, issue.currency))); @@ -236,7 +236,7 @@ CreateOffer::doApply() view.bumpSeq (); // Begin ledger variance. - SLE::pointer sleCreator = mEngine->entryCache ( + SLE::pointer sleCreator = mEngine->view().entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (mTxnAccountID)); if (uTxFlags & tfOfferCreateMask) @@ -336,7 +336,7 @@ CreateOffer::doApply() { uint256 const uCancelIndex ( getOfferIndex (mTxnAccountID, uCancelSequence)); - SLE::pointer sleCancel = mEngine->entryCache (ltOFFER, uCancelIndex); + SLE::pointer sleCancel = mEngine->view().entryCache (ltOFFER, uCancelIndex); // It's not an error to not find the offer to cancel: it might have // been consumed or removed as we are processing. @@ -571,7 +571,7 @@ CreateOffer::doApply() saTakerGets.getHumanCurrency (); } - SLE::pointer sleOffer (mEngine->entryCreate (ltOFFER, uLedgerIndex)); + SLE::pointer sleOffer (mEngine->view().entryCreate (ltOFFER, uLedgerIndex)); sleOffer->setFieldAccount (sfAccount, mTxnAccountID); sleOffer->setFieldU32 (sfSequence, uSequence); diff --git a/src/ripple/unity/app4.cpp b/src/ripple/unity/app4.cpp index e8b8e25504..2bf73c6c88 100644 --- a/src/ripple/unity/app4.cpp +++ b/src/ripple/unity/app4.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include