From 72832c0fa20c6cacfa931f65b662ee4bb9f9ca2c Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 19 Jun 2015 15:53:02 -0700 Subject: [PATCH 01/12] More robust call to get the valid ledger index --- src/ripple/app/misc/NetworkOPs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 2455a00bcd..d7dccde6ef 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -921,7 +921,7 @@ bool NetworkOPsImp::isValidated (std::uint32_t seq) { // use when ledger was retrieved by seq return haveLedger (seq) && - seq <= m_ledgerMaster.getValidatedLedger ()->getLedgerSeq (); + seq <= m_ledgerMaster.getValidLedgerIndex (); } void NetworkOPsImp::submitTransaction (Job&, STTx::pointer iTrans) From 6ec5fa9caed45d170d35f468ac3397191d725355 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 19 Jun 2015 15:53:08 -0700 Subject: [PATCH 02/12] Fix null pointer in ec wrapper --- src/ripple/crypto/impl/ECDSA.cpp | 2 +- src/ripple/crypto/impl/ECDSAKey.cpp | 15 +++++++------- src/ripple/crypto/impl/ec_key.cpp | 2 -- src/ripple/crypto/impl/ec_key.h | 31 +++++++++++++++-------------- src/ripple/crypto/impl/openssl.cpp | 2 +- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/ripple/crypto/impl/ECDSA.cpp b/src/ripple/crypto/impl/ECDSA.cpp index 671ef044d0..4da7effabf 100644 --- a/src/ripple/crypto/impl/ECDSA.cpp +++ b/src/ripple/crypto/impl/ECDSA.cpp @@ -71,7 +71,7 @@ static bool ECDSAVerify (uint256 const& hash, std::uint8_t const* sig, size_t si static bool ECDSAVerify (uint256 const& hash, Blob const& sig, const openssl::ec_key& key) { - return ECDSAVerify (hash, sig.data(), sig.size(), (EC_KEY*) key.get()); + return key.valid() && ECDSAVerify (hash, sig.data(), sig.size(), (EC_KEY*) key.get()); } bool ECDSAVerify (uint256 const& hash, diff --git a/src/ripple/crypto/impl/ECDSAKey.cpp b/src/ripple/crypto/impl/ECDSAKey.cpp index 96dd06209d..afa05095e8 100644 --- a/src/ripple/crypto/impl/ECDSAKey.cpp +++ b/src/ripple/crypto/impl/ECDSAKey.cpp @@ -55,33 +55,34 @@ ec_key ECDSAPrivateKey (uint256 const& serialized) } EC_KEY* key = new_initialized_EC_KEY(); + ec_key::pointer_t ptr = nullptr; const bool ok = EC_KEY_set_private_key (key, bn); BN_clear_free (bn); - if (! ok) - { + if (ok) + ptr = (ec_key::pointer_t) key; + else EC_KEY_free (key); - } - return ec_key::acquire ((ec_key::pointer_t) key); + return ec_key(ptr); } ec_key ECDSAPublicKey (std::uint8_t const* data, std::size_t size) { EC_KEY* key = new_initialized_EC_KEY(); + ec_key::pointer_t ptr = nullptr; if (o2i_ECPublicKey (&key, &data, size) != nullptr) { EC_KEY_set_conv_form (key, POINT_CONVERSION_COMPRESSED); + ptr = (ec_key::pointer_t) key; } else - { EC_KEY_free (key); - } - return ec_key::acquire ((ec_key::pointer_t) key); + return ec_key(ptr); } ec_key ECDSAPublicKey (Blob const& serialized) diff --git a/src/ripple/crypto/impl/ec_key.cpp b/src/ripple/crypto/impl/ec_key.cpp index 6b8fc985a0..b81e1a7ee3 100644 --- a/src/ripple/crypto/impl/ec_key.cpp +++ b/src/ripple/crypto/impl/ec_key.cpp @@ -34,8 +34,6 @@ static inline EC_KEY* get_EC_KEY (const ec_key& that) return (EC_KEY*) that.get(); } -const ec_key ec_key::invalid = ec_key::acquire (nullptr); - ec_key::ec_key (const ec_key& that) { if (that.ptr == nullptr) diff --git a/src/ripple/crypto/impl/ec_key.h b/src/ripple/crypto/impl/ec_key.h index 9c2aaba96b..217fbfe578 100644 --- a/src/ripple/crypto/impl/ec_key.h +++ b/src/ripple/crypto/impl/ec_key.h @@ -31,32 +31,28 @@ class ec_key public: using pointer_t = struct opaque_EC_KEY*; -private: - pointer_t ptr; - - void destroy(); + ec_key () : ptr(nullptr) + { + } ec_key (pointer_t raw) : ptr(raw) { } -public: - static const ec_key invalid; - - static ec_key acquire (pointer_t raw) { return ec_key (raw); } - - //ec_key() : ptr() {} - - ec_key (const ec_key&); - ec_key& operator= (const ec_key&) = delete; - ~ec_key() { destroy(); } + bool valid() const + { + return ptr != nullptr; + } + pointer_t get() const { return ptr; } + ec_key (const ec_key&); + pointer_t release() { pointer_t released = ptr; @@ -66,7 +62,12 @@ public: return released; } - bool valid() const { return ptr != nullptr; } +private: + pointer_t ptr; + + void destroy(); + + ec_key& operator= (const ec_key&) = delete; }; } // openssl diff --git a/src/ripple/crypto/impl/openssl.cpp b/src/ripple/crypto/impl/openssl.cpp index 768560ed58..7305162ac7 100644 --- a/src/ripple/crypto/impl/openssl.cpp +++ b/src/ripple/crypto/impl/openssl.cpp @@ -133,7 +133,7 @@ static ec_key ec_key_new_secp256k1_compressed() EC_KEY_set_conv_form (key, POINT_CONVERSION_COMPRESSED); - return ec_key::acquire ((ec_key::pointer_t) key); + return ec_key((ec_key::pointer_t) key); } void serialize_ec_point (ec_point const& point, std::uint8_t* ptr) From 2d6af1da1d6e17e8879f3e3f397da046545025ab Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sat, 13 Jun 2015 14:48:56 -0700 Subject: [PATCH 03/12] Add SigningPubKey regression test --- Builds/VisualStudio2013/RippleD.vcxproj | 4 ++ .../VisualStudio2013/RippleD.vcxproj.filters | 3 ++ src/ripple/app/tx/tests/Regression_test.cpp | 47 +++++++++++++++++++ src/ripple/unity/app_tx.cpp | 1 + 4 files changed, 55 insertions(+) create mode 100644 src/ripple/app/tx/tests/Regression_test.cpp diff --git a/Builds/VisualStudio2013/RippleD.vcxproj b/Builds/VisualStudio2013/RippleD.vcxproj index 3ca3c12677..8a79ef1d01 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj +++ b/Builds/VisualStudio2013/RippleD.vcxproj @@ -1837,6 +1837,10 @@ True True + + True + True + True True diff --git a/Builds/VisualStudio2013/RippleD.vcxproj.filters b/Builds/VisualStudio2013/RippleD.vcxproj.filters index ef5c986d5e..c789cfa020 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2013/RippleD.vcxproj.filters @@ -2541,6 +2541,9 @@ ripple\app\tx\tests + + ripple\app\tx\tests + ripple\app\tx\tests diff --git a/src/ripple/app/tx/tests/Regression_test.cpp b/src/ripple/app/tx/tests/Regression_test.cpp new file mode 100644 index 0000000000..6764b4943d --- /dev/null +++ b/src/ripple/app/tx/tests/Regression_test.cpp @@ -0,0 +1,47 @@ +//------------------------------------------------------------------------------ +/* + 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 { +namespace test { + +struct Regression_test : public beast::unit_test::suite +{ + // SigningPubKey: 0000000000000000000000000000000000 (34 zeroes) + void testBadSigningPubKey() + { + using namespace jtx; + Env env(*this); + env.fund(XRP(10000), "alice"); + env(noop("alice"), sig(none), json( R"raw( { + "SigningPubKey" : "0000000000000000000000000000000000", + "TxnSignature" : "3044022042D144D130A1651CBE5632196FE4E745A75445AA8DB95AC9905701DC891F9A30022012DF180ED1545B560681D475F570D9603BF663BD4C91F591DBA0A8C43876C563" + } )raw"), ter(temINVALID)); + } + + void run() override + { + testBadSigningPubKey(); + } +}; + +BEAST_DEFINE_TESTSUITE(Regression,app,ripple); + +} // test +} // ripple diff --git a/src/ripple/unity/app_tx.cpp b/src/ripple/unity/app_tx.cpp index 63a1d88ef7..b8b36060cb 100644 --- a/src/ripple/unity/app_tx.cpp +++ b/src/ripple/unity/app_tx.cpp @@ -46,4 +46,5 @@ #include #include #include +#include #include From 749f31f69dd3780718191b3778d4b8fa464e153f Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 17 Jun 2015 13:31:02 -0700 Subject: [PATCH 04/12] Tidy up SHAMap node handling of invalid wire formats --- src/ripple/app/ledger/impl/InboundLedgers.cpp | 3 +++ src/ripple/shamap/impl/SHAMap.cpp | 14 +++++++++----- src/ripple/shamap/impl/SHAMapSync.cpp | 18 +++++++++--------- src/ripple/shamap/impl/SHAMapTreeNode.cpp | 18 ++---------------- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index c676dd62f7..cb94df19a1 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -239,6 +239,9 @@ public: Blob (node.nodedata().begin(), node.nodedata().end()), 0, snfWIRE, uZero, false); + if (!newNode) + return; + s.erase(); newNode->addRaw(s, snfPREFIX); diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index b0ce1a72dd..8e0a46916a 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -181,7 +181,8 @@ SHAMap::fetchNodeFromDB (uint256 const& hash) const { node = SHAMapAbstractNode::make(obj->getData(), 0, snfPREFIX, hash, true); - canonicalize (hash, node); + if (node) + canonicalize (hash, node); } catch (...) { @@ -210,9 +211,12 @@ SHAMap::checkFilter(uint256 const& hash, SHAMapNodeID const& id, if (filter->haveNode (id, hash, nodeData)) { node = SHAMapAbstractNode::make(nodeData, 0, snfPREFIX, hash, true); - filter->gotNode (true, id, hash, nodeData, node->getType ()); - if (backed_) - canonicalize (hash, node); + if (node) + { + filter->gotNode (true, id, hash, nodeData, node->getType ()); + if (backed_) + canonicalize (hash, node); + } } return node; } @@ -383,7 +387,7 @@ SHAMap::descendAsync (SHAMapInnerNode* parent, int branch, ptr = SHAMapAbstractNode::make(obj->getData(), 0, snfPREFIX, hash, true); - if (backed_) + if (ptr && backed_) canonicalize (hash, ptr); } } diff --git a/src/ripple/shamap/impl/SHAMapSync.cpp b/src/ripple/shamap/impl/SHAMapSync.cpp index 8965cadf74..bc9db1977a 100644 --- a/src/ripple/shamap/impl/SHAMapSync.cpp +++ b/src/ripple/shamap/impl/SHAMapSync.cpp @@ -419,7 +419,7 @@ SHAMapAddNode SHAMap::addRootNode (Blob const& rootNode, assert (seq_ >= 1); auto node = SHAMapAbstractNode::make(rootNode, 0, format, uZero, false); - if (!node) + if (!node || !node->isValid ()) return SHAMapAddNode::invalid (); #ifdef BEAST_DEBUG @@ -459,7 +459,7 @@ SHAMapAddNode SHAMap::addRootNode (uint256 const& hash, Blob const& rootNode, SH assert (seq_ >= 1); auto node = SHAMapAbstractNode::make(rootNode, 0, format, uZero, false); - if (!node || node->getNodeHash () != hash) + if (!node || !node->isValid() || node->getNodeHash () != hash) return SHAMapAddNode::invalid (); if (backed_) @@ -537,6 +537,13 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode, auto newNode = SHAMapAbstractNode::make(rawNode, 0, snfWIRE, uZero, false); + if (!newNode || !newNode->isValid() || childHash != newNode->getNodeHash ()) + { + if (journal_.warning) journal_.warning << + "Corrupt node received"; + return SHAMapAddNode::invalid (); + } + if (!newNode->isInBounds (iNodeID)) { // Map is provably invalid @@ -544,13 +551,6 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode, return SHAMapAddNode::useful (); } - if (childHash != newNode->getNodeHash ()) - { - if (journal_.warning) journal_.warning << - "Corrupt node received"; - return SHAMapAddNode::invalid (); - } - if (backed_) canonicalize (childHash, newNode); diff --git a/src/ripple/shamap/impl/SHAMapTreeNode.cpp b/src/ripple/shamap/impl/SHAMapTreeNode.cpp index 2f0dca846a..9178876743 100644 --- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp +++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp @@ -79,28 +79,14 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f if (format == snfWIRE) { if (rawNode.empty ()) - { -#ifdef BEAST_DEBUG - deprecatedLogs().journal("SHAMapTreeNode").fatal << - "Wire format node is empty"; - assert (false); -#endif - throw std::runtime_error ("invalid node AW type"); - } + return {}; Serializer s (rawNode.data(), rawNode.size() - 1); int type = rawNode.back (); int len = s.getLength (); if ((type < 0) || (type > 4)) - { -#ifdef BEAST_DEBUG - deprecatedLogs().journal("SHAMapTreeNode").fatal << - "Invalid wire format node" << strHex (rawNode); - assert (false); -#endif - throw std::runtime_error ("invalid node AW type"); - } + return {}; if (type == 0) { From e7eb3aa63d4e93382d2102b4e79547a5b7e743d9 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 19 Jun 2015 16:02:24 -0700 Subject: [PATCH 05/12] Set version to 0.28.2-rc1 --- Builds/rpm/rippled.spec | 2 +- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index ed6fc82f81..9db98713b9 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.28.2-b10 +Version: 0.28.2-rc1 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index c84b95154a..564d230c9a 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -35,7 +35,7 @@ char const* getRawVersionString () // // The build version number (edit this for each release) // - "0.28.2-b10" + "0.28.2-rc1" // // Must follow the format described here: // From c3d34aaf4dd64754ceae45762d4698800cae477c Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 25 Jun 2015 14:45:32 -0700 Subject: [PATCH 06/12] Fix shadowing --- src/ripple/rpc/handlers/LedgerRequest.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ripple/rpc/handlers/LedgerRequest.cpp b/src/ripple/rpc/handlers/LedgerRequest.cpp index 6a2491f1e3..4dc3518933 100644 --- a/src/ripple/rpc/handlers/LedgerRequest.cpp +++ b/src/ripple/rpc/handlers/LedgerRequest.cpp @@ -47,7 +47,9 @@ Json::Value doLedgerRequest (RPC::Context& context) auto const& jsonHash = context.params[jss::ledger_hash]; if (!jsonHash.isString() || !ledgerHash.SetHex (jsonHash.asString ())) return RPC::invalid_field_message (jss::ledger_hash); - } else { + } + else + { auto const& jsonIndex = context.params[jss::ledger_index]; if (!jsonIndex.isNumeric ()) return RPC::invalid_field_message (jss::ledger_index); @@ -63,12 +65,11 @@ Json::Value doLedgerRequest (RPC::Context& context) if (ledgerIndex >= ledger->getLedgerSeq()) return RPC::make_param_error("Ledger index too large"); - auto const j = - deprecatedLogs().journal("RPCHandler"); + auto const j = deprecatedLogs().journal("RPCHandler"); // Try to get the hash of the desired ledger from the validated ledger - auto ledgerHash = hashOfSeq(*ledger, ledgerIndex, + auto neededHash = hashOfSeq(*ledger, ledgerIndex, getApp().getSLECache(), j); - if (! ledgerHash) + if (! neededHash) { // Find a ledger more likely to have the hash of the desired ledger auto const refIndex = getCandidateLedger(ledgerIndex); @@ -98,12 +99,11 @@ Json::Value doLedgerRequest (RPC::Context& context) return Json::Value(); } - ledgerHash = hashOfSeq(*ledger, ledgerIndex, + neededHash = hashOfSeq(*ledger, ledgerIndex, getApp().getSLECache(), j); - assert (ledgerHash); - if (! ledgerHash) - ledgerHash = zero; // kludge } + assert (neededHash); + ledgerHash = neededHash ? *neededHash : zero; // kludge } auto ledger = ledgerMaster.getLedgerByHash (ledgerHash); From 3fcf4ae8b70f204344f2bced85ee14fcd0688834 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 25 Jun 2015 14:46:43 -0700 Subject: [PATCH 07/12] Set version to 0.28.2-rc2 --- Builds/rpm/rippled.spec | 2 +- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index 9db98713b9..e01bdeea19 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.28.2-rc1 +Version: 0.28.2-rc2 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 564d230c9a..ad16a42ffc 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -35,7 +35,7 @@ char const* getRawVersionString () // // The build version number (edit this for each release) // - "0.28.2-rc1" + "0.28.2-rc2" // // Must follow the format described here: // From e2ef423624df2049d29455fff7aabbb3d9a4c97e Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 30 Jun 2015 10:55:20 -0700 Subject: [PATCH 08/12] Disable Websocket ping timer --- src/ripple/websocket/WebSocket02.cpp | 2 ++ src/ripple/websocket/WebSocket04.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ripple/websocket/WebSocket02.cpp b/src/ripple/websocket/WebSocket02.cpp index b194a467bd..d91f91bcad 100644 --- a/src/ripple/websocket/WebSocket02.cpp +++ b/src/ripple/websocket/WebSocket02.cpp @@ -74,6 +74,8 @@ template <> void ConnectionImpl ::setPingTimer () { auto freq = getConfig ().WEBSOCKET_PING_FREQ; + // VFALCO Disabled since it might cause hangs + freq = 0; if (freq <= 0) return; connection_ptr ptr = m_connection.lock (); diff --git a/src/ripple/websocket/WebSocket04.cpp b/src/ripple/websocket/WebSocket04.cpp index 287f5a9f5a..fa1fde5d9d 100644 --- a/src/ripple/websocket/WebSocket04.cpp +++ b/src/ripple/websocket/WebSocket04.cpp @@ -116,6 +116,8 @@ template <> void ConnectionImpl ::setPingTimer () { auto freq = getConfig ().WEBSOCKET_PING_FREQ; + // VFALCO Disabled since it might cause hangs + freq = 0; if (freq <= 0) return; if (auto con = m_connection.lock ()) From a338d9efe0f96c302ec2f2a598e33167c385b7ff Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 30 Jun 2015 10:59:39 -0700 Subject: [PATCH 09/12] Return tefEXCEPTION in transaction engine --- src/ripple/app/tx/impl/TransactionEngine.cpp | 316 ++++++++++--------- 1 file changed, 167 insertions(+), 149 deletions(-) diff --git a/src/ripple/app/tx/impl/TransactionEngine.cpp b/src/ripple/app/tx/impl/TransactionEngine.cpp index ae16c348b0..496cf9e91b 100644 --- a/src/ripple/app/tx/impl/TransactionEngine.cpp +++ b/src/ripple/app/tx/impl/TransactionEngine.cpp @@ -36,184 +36,202 @@ TransactionEngine::applyTransaction ( STTx const& txn, TransactionEngineParams params) { - assert (mLedger); + TER terResult; + bool didApply = false; - WriteLog (lsTRACE, TransactionEngine) << "applyTransaction>"; - - uint256 const& txID = txn.getTransactionID (); - - if (!txID) + try { - WriteLog (lsWARNING, TransactionEngine) << - "applyTransaction: invalid transaction id"; - return std::make_pair(temINVALID_FLAG, false); - } + assert (mLedger); - mNodes.emplace(mLedger, txID, - mLedger->getLedgerSeq(), params); + WriteLog (lsTRACE, TransactionEngine) << "applyTransaction>"; -#ifdef BEAST_DEBUG - if (1) - { - Serializer ser; - txn.add (ser); - SerialIter sit(ser.slice()); - STTx s2 (sit); + uint256 const& txID = txn.getTransactionID (); - if (!s2.isEquivalent (txn)) + if (!txID) { - WriteLog (lsFATAL, TransactionEngine) << - "Transaction serdes mismatch"; - WriteLog (lsINFO, TransactionEngine) << txn.getJson (0); - WriteLog (lsFATAL, TransactionEngine) << s2.getJson (0); - assert (false); + WriteLog (lsWARNING, TransactionEngine) << + "applyTransaction: invalid transaction id"; + return std::make_pair(temINVALID_FLAG, false); } - } -#endif - TER terResult = Transactor::transact (txn, params, this); - - if (terResult == temUNKNOWN) - { - WriteLog (lsWARNING, TransactionEngine) << - "applyTransaction: Invalid transaction: unknown transaction type"; - return std::make_pair(temUNKNOWN, false); - } - - if (ShouldLog (lsDEBUG, TransactionEngine)) - { - std::string strToken; - std::string strHuman; - - transResultInfo (terResult, strToken, strHuman); - - WriteLog (lsDEBUG, TransactionEngine) << - "applyTransaction: terResult=" << strToken << - " : " << terResult << - " : " << strHuman; - } - - bool didApply = isTesSuccess (terResult); - - if (isTecClaim (terResult) && !(params & tapRETRY)) - { - // only claim the transaction fee - WriteLog (lsDEBUG, TransactionEngine) << - "Reprocessing tx " << txID << " to only claim fee"; mNodes.emplace(mLedger, txID, mLedger->getLedgerSeq(), params); - SLE::pointer txnAcct = mNodes->entryCache (ltACCOUNT_ROOT, - getAccountRootIndex (txn.getSourceAccount ())); - - if (!txnAcct) - terResult = terNO_ACCOUNT; - else + #ifdef BEAST_DEBUG + if (1) { - std::uint32_t t_seq = txn.getSequence (); - std::uint32_t a_seq = txnAcct->getFieldU32 (sfSequence); + Serializer ser; + txn.add (ser); + SerialIter sit(ser.slice()); + STTx s2 (sit); - if (a_seq < t_seq) - terResult = terPRE_SEQ; - else if (a_seq > t_seq) - terResult = tefPAST_SEQ; + if (!s2.isEquivalent (txn)) + { + WriteLog (lsFATAL, TransactionEngine) << + "Transaction serdes mismatch"; + WriteLog (lsINFO, TransactionEngine) << txn.getJson (0); + WriteLog (lsFATAL, TransactionEngine) << s2.getJson (0); + assert (false); + } + } + #endif + + terResult = Transactor::transact (txn, params, this); + + if (terResult == temUNKNOWN) + { + WriteLog (lsWARNING, TransactionEngine) << + "applyTransaction: Invalid transaction: unknown transaction type"; + return std::make_pair(temUNKNOWN, false); + } + + if (ShouldLog (lsDEBUG, TransactionEngine)) + { + std::string strToken; + std::string strHuman; + + transResultInfo (terResult, strToken, strHuman); + + WriteLog (lsDEBUG, TransactionEngine) << + "applyTransaction: terResult=" << strToken << + " : " << terResult << + " : " << strHuman; + } + + didApply = isTesSuccess (terResult); + + if (isTecClaim (terResult) && !(params & tapRETRY)) + { + // only claim the transaction fee + WriteLog (lsDEBUG, TransactionEngine) << + "Reprocessing tx " << txID << " to only claim fee"; + mNodes.emplace(mLedger, txID, + mLedger->getLedgerSeq(), params); + + SLE::pointer txnAcct = mNodes->entryCache (ltACCOUNT_ROOT, + getAccountRootIndex (txn.getSourceAccount ())); + + if (!txnAcct) + terResult = terNO_ACCOUNT; else { - STAmount fee = txn.getTransactionFee (); - STAmount balance = txnAcct->getFieldAmount (sfBalance); + std::uint32_t t_seq = txn.getSequence (); + std::uint32_t a_seq = txnAcct->getFieldU32 (sfSequence); - // 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; - } + if (a_seq < t_seq) + terResult = terPRE_SEQ; + else if (a_seq > t_seq) + terResult = tefPAST_SEQ; else { - if (fee > balance) - fee = balance; - txnAcct->setFieldAmount (sfBalance, balance - fee); - txnAcct->setFieldU32 (sfSequence, t_seq + 1); - mNodes->entryModify (txnAcct); - didApply = true; + STAmount fee = txn.getTransactionFee (); + STAmount balance = txnAcct->getFieldAmount (sfBalance); + + // 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); + mNodes->entryModify (txnAcct); + didApply = true; + } } } } - } - 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) - { - // Transaction succeeded fully or (retries are not allowed and the - // transaction could claim a fee) - Serializer m; - mNodes->calcRawMeta (m, terResult, mTxnSeq++); - - assert(mLedger == mNodes->getLedger()); - mNodes->apply(); - - Serializer s; - txn.add (s); - - if (params & tapOPEN_LEDGER) + else if (!didApply) { - if (!mLedger->addTransaction (txID, s)) + 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) + { + // Transaction succeeded fully or (retries are not allowed and the + // transaction could claim a fee) + Serializer m; + mNodes->calcRawMeta (m, terResult, mTxnSeq++); + + assert(mLedger == mNodes->getLedger()); + mNodes->apply(); + + Serializer s; + txn.add (s); + + if (params & tapOPEN_LEDGER) { - WriteLog (lsFATAL, TransactionEngine) << - "Duplicate transaction applied"; - assert (false); - throw std::runtime_error ("Duplicate transaction applied"); + if (!mLedger->addTransaction (txID, s)) + { + WriteLog (lsFATAL, TransactionEngine) << + "Duplicate transaction applied"; + assert (false); + throw std::runtime_error ("Duplicate transaction applied"); + } + } + else + { + if (!mLedger->addTransaction (txID, s, m)) + { + WriteLog (lsFATAL, TransactionEngine) << + "Duplicate transaction applied to closed ledger"; + assert (false); + throw std::runtime_error ("Duplicate transaction applied to closed ledger"); + } + + // Charge whatever fee they specified. We break the encapsulation of + // STAmount here and use "special knowledge" - namely that a native + // amount is stored fully in the mantissa: + auto const fee = txn.getTransactionFee (); + + // The transactor guarantees these will never trigger + if (!fee.native () || fee.negative ()) + throw std::runtime_error ("amount is negative!"); + + if (fee != zero) + mLedger->destroyCoins (fee.mantissa ()); } } - else + + mNodes = boost::none; + + if (!(params & tapOPEN_LEDGER) && isTemMalformed (terResult)) { - if (!mLedger->addTransaction (txID, s, m)) - { - WriteLog (lsFATAL, TransactionEngine) << - "Duplicate transaction applied to closed ledger"; - assert (false); - throw std::runtime_error ("Duplicate transaction applied to closed ledger"); - } - - // Charge whatever fee they specified. We break the encapsulation of - // STAmount here and use "special knowledge" - namely that a native - // amount is stored fully in the mantissa: - auto const fee = txn.getTransactionFee (); - - // The transactor guarantees these will never trigger - if (!fee.native () || fee.negative ()) - throw std::runtime_error ("amount is negative!"); - - if (fee != zero) - mLedger->destroyCoins (fee.mantissa ()); + // XXX Malformed or failed transaction in closed ledger must bow out. } } - - mNodes = boost::none; - - if (!(params & tapOPEN_LEDGER) && isTemMalformed (terResult)) + catch(std::exception const& e) { - // XXX Malformed or failed transaction in closed ledger must bow out. + WriteLog (lsFATAL, TransactionEngine) << + "Caught exception: " << e.what(); + return { tefEXCEPTION, false }; + } + catch(...) + { + WriteLog (lsFATAL, TransactionEngine) << + "Caught unknown exception"; + return { tefEXCEPTION, false }; } return { terResult, didApply }; From 9d3b3f7a017320d06b8c210ac872f635004a034b Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Wed, 1 Jul 2015 12:26:40 -0700 Subject: [PATCH 10/12] Fix canonicalization race in batch apply --- src/ripple/app/misc/NetworkOPs.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index d7dccde6ef..95c71a2442 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1020,6 +1020,9 @@ void NetworkOPsImp::processTransaction (Transaction::pointer transaction, getApp().getHashRouter ().setFlag (transaction->getID (), SF_SIGGOOD); + // canonicalize can change our pointer + getApp().getMasterTransaction ().canonicalize (&transaction); + if (bLocal) doTransactionSync (transaction, bAdmin, failType); else @@ -1147,9 +1150,6 @@ void NetworkOPsImp::apply (std::unique_lock& lock) { m_journal.debug << "Transaction is now included in open ledger"; e.transaction->setStatus (INCLUDED); - - // VFALCO NOTE The value of trans can be changed here! - getApp().getMasterTransaction ().canonicalize (&e.transaction); } else if (e.result == tefPAST_SEQ) { From adebba94dc7af47c910fb635eaee9fb752161552 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 30 Jun 2015 11:00:50 -0700 Subject: [PATCH 11/12] Set version to 0.28.2-rc3 --- Builds/rpm/rippled.spec | 2 +- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index e01bdeea19..ed313b1213 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.28.2-rc2 +Version: 0.28.2-rc3 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index ad16a42ffc..d0275276fa 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -35,7 +35,7 @@ char const* getRawVersionString () // // The build version number (edit this for each release) // - "0.28.2-rc2" + "0.28.2-rc3" // // Must follow the format described here: // From 6374aad9bc94595e051a04e23580617bc1aaf300 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 7 Jul 2015 09:21:44 -0700 Subject: [PATCH 12/12] Set version to 0.28.2 --- Builds/rpm/rippled.spec | 2 +- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index ed313b1213..c81f9eda15 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.28.2-rc3 +Version: 0.28.2 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index d0275276fa..a9363bc617 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -35,7 +35,7 @@ char const* getRawVersionString () // // The build version number (edit this for each release) // - "0.28.2-rc3" + "0.28.2" // // Must follow the format described here: //