From c288c4cd25536f3326bb6fdc3466bc9a1bccef16 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Tue, 9 Jul 2013 09:38:50 -0700 Subject: [PATCH] Fix a bug that can destroy metadata. If we find destroyed metadata, fix it. --- src/cpp/ripple/Ledger.cpp | 4 +- src/cpp/ripple/NetworkOPs.cpp | 17 +++++--- src/cpp/ripple/Transaction.cpp | 40 ------------------- src/cpp/ripple/Transaction.h | 3 -- src/cpp/ripple/TransactionMaster.cpp | 5 +-- src/cpp/ripple/TransactionMaster.h | 2 +- .../ripple/ripple_SerializedTransaction.cpp | 20 ---------- src/cpp/ripple/ripple_SerializedTransaction.h | 2 - 8 files changed, 15 insertions(+), 78 deletions(-) diff --git a/src/cpp/ripple/Ledger.cpp b/src/cpp/ripple/Ledger.cpp index c4508cd44c..ac9ae3239e 100644 --- a/src/cpp/ripple/Ledger.cpp +++ b/src/cpp/ripple/Ledger.cpp @@ -373,7 +373,7 @@ Transaction::pointer Ledger::getTransaction (uint256 const& transID) const if (txn->getStatus () == NEW) txn->setStatus (mClosed ? COMMITTED : INCLUDED, mLedgerSeq); - getApp().getMasterTransaction ().canonicalize (txn, false); + getApp().getMasterTransaction ().canonicalize (txn); return txn; } @@ -452,7 +452,7 @@ bool Ledger::getTransaction (uint256 const& txID, Transaction::pointer& txn, Tra if (txn->getStatus () == NEW) txn->setStatus (mClosed ? COMMITTED : INCLUDED, mLedgerSeq); - getApp().getMasterTransaction ().canonicalize (txn, false); + getApp().getMasterTransaction ().canonicalize (txn); return true; } diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index dc43c7e789..7b9a78bb07 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -255,7 +255,7 @@ void NetworkOPs::runTransactionQueue () // transaction should be held WriteLog (lsDEBUG, NetworkOPs) << "QTransaction should be held: " << r; dbtx->setStatus (HELD); - getApp().getMasterTransaction ().canonicalize (dbtx, true); + getApp().getMasterTransaction ().canonicalize (dbtx); mLedgerMaster->addHeldTransaction (dbtx); } else if (r == tefPAST_SEQ) @@ -268,7 +268,7 @@ void NetworkOPs::runTransactionQueue () { WriteLog (lsINFO, NetworkOPs) << "QTransaction is now included in open ledger"; dbtx->setStatus (INCLUDED); - getApp().getMasterTransaction ().canonicalize (dbtx, true); + getApp().getMasterTransaction ().canonicalize (dbtx); } else { @@ -366,7 +366,7 @@ Transaction::pointer NetworkOPs::processTransaction (Transaction::pointer trans, { WriteLog (lsINFO, NetworkOPs) << "Transaction is now included in open ledger"; trans->setStatus (INCLUDED); - getApp().getMasterTransaction ().canonicalize (trans, true); + getApp().getMasterTransaction ().canonicalize (trans); } else if (r == tefPAST_SEQ) { @@ -381,7 +381,7 @@ Transaction::pointer NetworkOPs::processTransaction (Transaction::pointer trans, // transaction should be held WriteLog (lsDEBUG, NetworkOPs) << "Transaction should be held: " << r; trans->setStatus (HELD); - getApp().getMasterTransaction ().canonicalize (trans, true); + getApp().getMasterTransaction ().canonicalize (trans); mLedgerMaster->addHeldTransaction (trans); } } @@ -1228,7 +1228,12 @@ NetworkOPs::getAccountTxs (const RippleAddress& account, int32 minLedger, int32 else rawMeta.resize (metaSize); if (rawMeta.getLength() == 0) - { // FIXME metadata isn't in the table, update metadata and sequence in Transactions DB from ledger + { // Work around a bug that could leave the metadata missing + uint32 seq = static_cast(db->getBigInt("AccountTransactions.LedgerSeq")); + WriteLog(lsWARNING, NetworkOPs) << "Recovering ledger " << seq << ", txn " << txn->getID(); + Ledger::pointer ledger = getLedgerBySeq(seq); + if (ledger) + ledger->pendSave(false); } TransactionMetaSet::pointer meta = boost::make_shared (txn->getID (), txn->getLedger (), rawMeta.getData ()); @@ -2225,7 +2230,7 @@ void NetworkOPs::sweepFetchPack () void NetworkOPs::addFetchPack (uint256 const& hash, boost::shared_ptr< Blob >& data) { - mFetchPack.canonicalize (hash, data, false); + mFetchPack.canonicalize (hash, data); } bool NetworkOPs::getFetchPack (uint256 const& hash, Blob& data) diff --git a/src/cpp/ripple/Transaction.cpp b/src/cpp/ripple/Transaction.cpp index 0d71e23433..2ac415ec8d 100644 --- a/src/cpp/ripple/Transaction.cpp +++ b/src/cpp/ripple/Transaction.cpp @@ -121,46 +121,6 @@ void Transaction::setStatus (TransStatus ts, uint32 lseq) mInLedger = lseq; } -void Transaction::save () -{ - if ((mStatus == INVALID) || (mStatus == REMOVED)) - return; - - char status; - - switch (mStatus) - { - case NEW: - status = TXN_SQL_NEW; - break; - - case INCLUDED: - status = TXN_SQL_INCLUDED; - break; - - case CONFLICTED: - status = TXN_SQL_CONFLICT; - break; - - case COMMITTED: - status = TXN_SQL_VALIDATED; - break; - - case HELD: - status = TXN_SQL_HELD; - break; - - default: - status = TXN_SQL_UNKNOWN; - } - - Database* db = getApp().getTxnDB ()->getDB (); - ScopedLock dbLock (getApp().getTxnDB ()->getDBLock ()); - - // FIXME: This can destroy metadata - db->executeSQL (mTransaction->getSQLInsertReplaceHeader () + mTransaction->getSQL (getLedger (), status) + ";"); -} - Transaction::pointer Transaction::transactionFromSQL (Database* db, bool bValidate) { Serializer rawTxn; diff --git a/src/cpp/ripple/Transaction.h b/src/cpp/ripple/Transaction.h index 6753836c44..b9949bb453 100644 --- a/src/cpp/ripple/Transaction.h +++ b/src/cpp/ripple/Transaction.h @@ -134,9 +134,6 @@ public: mInLedger = ledger; } - // database functions - void save (); - bool operator< (const Transaction&) const; bool operator> (const Transaction&) const; bool operator== (const Transaction&) const; diff --git a/src/cpp/ripple/TransactionMaster.cpp b/src/cpp/ripple/TransactionMaster.cpp index 9a21807216..133fac7cab 100644 --- a/src/cpp/ripple/TransactionMaster.cpp +++ b/src/cpp/ripple/TransactionMaster.cpp @@ -80,7 +80,7 @@ SerializedTransaction::pointer TransactionMaster::fetch (SHAMapItem::ref item, S return txn; } -bool TransactionMaster::canonicalize (Transaction::pointer& txn, bool may_be_new) +bool TransactionMaster::canonicalize (Transaction::pointer& txn) { uint256 tid = txn->getID (); @@ -90,9 +90,6 @@ bool TransactionMaster::canonicalize (Transaction::pointer& txn, bool may_be_new if (mCache.canonicalize (tid, txn)) return true; - if (may_be_new) - txn->save (); - return false; } // vim:ts=4 diff --git a/src/cpp/ripple/TransactionMaster.h b/src/cpp/ripple/TransactionMaster.h index 05bd56c764..4bfce624bc 100644 --- a/src/cpp/ripple/TransactionMaster.h +++ b/src/cpp/ripple/TransactionMaster.h @@ -20,7 +20,7 @@ public: // return value: true = we had the transaction already bool inLedger (uint256 const& hash, uint32 ledger); - bool canonicalize (Transaction::pointer& txn, bool maybeNew); + bool canonicalize (Transaction::pointer& txn); void sweep (void) { mCache.sweep (); diff --git a/src/cpp/ripple/ripple_SerializedTransaction.cpp b/src/cpp/ripple/ripple_SerializedTransaction.cpp index 1a227bf13d..d93e8854cc 100644 --- a/src/cpp/ripple/ripple_SerializedTransaction.cpp +++ b/src/cpp/ripple/ripple_SerializedTransaction.cpp @@ -248,26 +248,6 @@ std::string SerializedTransaction::getMetaSQLValueHeader () return "(TransID, TransType, FromAcct, FromSeq, LedgerSeq, Status, RawTxn, TxnMeta)"; } -std::string SerializedTransaction::getSQLInsertHeader () -{ - return "INSERT INTO Transactions " + getSQLValueHeader () + " VALUES "; -} - -std::string SerializedTransaction::getSQLInsertIgnoreHeader () -{ - return "INSERT OR IGNORE INTO Transactions " + getSQLValueHeader () + " VALUES "; -} - -std::string SerializedTransaction::getSQLInsertReplaceHeader () -{ - return "INSERT OR REPLACE INTO Transactions " + getSQLValueHeader () + " VALUES "; -} - -std::string SerializedTransaction::getMetaSQLInsertHeader () -{ - return "INSERT INTO Transactions " + getMetaSQLValueHeader () + " VALUES "; -} - std::string SerializedTransaction::getMetaSQLInsertReplaceHeader () { return "INSERT OR REPLACE INTO Transactions " + getMetaSQLValueHeader () + " VALUES "; diff --git a/src/cpp/ripple/ripple_SerializedTransaction.h b/src/cpp/ripple/ripple_SerializedTransaction.h index 169c3753fc..31ecc83287 100644 --- a/src/cpp/ripple/ripple_SerializedTransaction.h +++ b/src/cpp/ripple/ripple_SerializedTransaction.h @@ -114,14 +114,12 @@ public: static std::string getSQLValueHeader (); static std::string getSQLInsertHeader (); static std::string getSQLInsertIgnoreHeader (); - static std::string getSQLInsertReplaceHeader (); std::string getSQL (std::string & sql, uint32 inLedger, char status) const; std::string getSQL (uint32 inLedger, char status) const; std::string getSQL (Serializer rawTxn, uint32 inLedger, char status) const; // SQL Functions with metadata static std::string getMetaSQLValueHeader (); - static std::string getMetaSQLInsertHeader (); static std::string getMetaSQLInsertReplaceHeader (); std::string getMetaSQL (uint32 inLedger, const std::string & escapedMetaData) const; std::string getMetaSQL (Serializer rawTxn, uint32 inLedger, char status, const std::string & escapedMetaData) const;