diff --git a/src/ripple/app/ledger/AcceptedLedgerTx.cpp b/src/ripple/app/ledger/AcceptedLedgerTx.cpp index 61dfc30632..4a8a7a4826 100644 --- a/src/ripple/app/ledger/AcceptedLedgerTx.cpp +++ b/src/ripple/app/ledger/AcceptedLedgerTx.cpp @@ -54,7 +54,7 @@ AcceptedLedgerTx::AcceptedLedgerTx ( AcceptedLedgerTx::AcceptedLedgerTx ( std::shared_ptr const& ledger, - STTx::ref txn, + std::shared_ptr const& txn, TER result, AccountIDCache const& accountCache, Logs& logs) diff --git a/src/ripple/app/ledger/AcceptedLedgerTx.h b/src/ripple/app/ledger/AcceptedLedgerTx.h index e4552b791d..b3e4c2f9a4 100644 --- a/src/ripple/app/ledger/AcceptedLedgerTx.h +++ b/src/ripple/app/ledger/AcceptedLedgerTx.h @@ -62,7 +62,7 @@ public: Logs&); AcceptedLedgerTx ( std::shared_ptr const&, - STTx::ref, + std::shared_ptr const&, TER, AccountIDCache const&, Logs&); diff --git a/src/ripple/app/ledger/ConsensusTransSetSF.cpp b/src/ripple/app/ledger/ConsensusTransSetSF.cpp index 87c8f9fd66..8382fce8fe 100644 --- a/src/ripple/app/ledger/ConsensusTransSetSF.cpp +++ b/src/ripple/app/ledger/ConsensusTransSetSF.cpp @@ -57,7 +57,7 @@ void ConsensusTransSetSF::gotNode ( // skip prefix Serializer s (nodeData.data() + 4, nodeData.size() - 4); SerialIter sit (s.slice()); - STTx::pointer stx = std::make_shared (std::ref (sit)); + auto stx = std::make_shared (std::ref (sit)); assert (stx->getTransactionID () == nodeHash); auto const pap = &app_; app_.getJobQueue ().addJob ( diff --git a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp index 8f156b9680..65ed57e10a 100644 --- a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp +++ b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp @@ -1164,7 +1164,7 @@ void LedgerConsensusImp::accept (std::shared_ptr set) << " not get in"; SerialIter sit (it.second->peekTransaction().slice()); - auto txn = std::make_shared(sit); + auto txn = std::make_shared(sit); retriableTxs.insert (txn); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 482799efc4..706297a00f 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -155,7 +155,7 @@ public: // // Must complete immediately. - void submitTransaction (STTx::pointer) override; + void submitTransaction (std::shared_ptr const&) override; void processTransaction ( Transaction::pointer& transaction, @@ -339,7 +339,7 @@ public: void pubLedger (Ledger::ref lpAccepted) override; void pubProposedTransaction ( std::shared_ptr const& lpCurrent, - STTx::ref stTxn, TER terResult) override; + std::shared_ptr const& stTxn, TER terResult) override; //-------------------------------------------------------------------------- // @@ -639,7 +639,7 @@ std::string NetworkOPsImp::strOperatingMode () const return paStatusToken[mMode]; } -void NetworkOPsImp::submitTransaction (STTx::pointer iTrans) +void NetworkOPsImp::submitTransaction (std::shared_ptr const& iTrans) { if (isNeedNetworkLedger ()) { @@ -648,11 +648,7 @@ void NetworkOPsImp::submitTransaction (STTx::pointer iTrans) } // this is an asynchronous interface - Serializer s; - iTrans->add (s); - - SerialIter sit (s.slice()); - auto trans = std::make_shared (std::ref (sit)); + auto const trans = sterilize(*iTrans); auto const txid = trans->getTransactionID (); auto const flags = app_.getHashRouter().getFlags(txid); @@ -2005,7 +2001,7 @@ Json::Value NetworkOPsImp::getLedgerFetchInfo () void NetworkOPsImp::pubProposedTransaction ( std::shared_ptr const& lpCurrent, - STTx::ref stTxn, TER terResult) + std::shared_ptr const& stTxn, TER terResult) { Json::Value jvObj = transJson (*stTxn, terResult, false, lpCurrent); diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index 27a422cb8e..ec141bde81 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -110,7 +110,7 @@ public: // // must complete immediately - virtual void submitTransaction (STTx::pointer) = 0; + virtual void submitTransaction (std::shared_ptr const&) = 0; /** * Process transactions as they arrive from the network or which are @@ -229,7 +229,7 @@ public: virtual void pubLedger (Ledger::ref lpAccepted) = 0; virtual void pubProposedTransaction ( std::shared_ptr const& lpCurrent, - STTx::ref stTxn, TER terResult) = 0; + std::shared_ptr const& stTxn, TER terResult) = 0; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/app/misc/impl/AccountTxPaging.cpp b/src/ripple/app/misc/impl/AccountTxPaging.cpp index ad0d276f88..82fadaff40 100644 --- a/src/ripple/app/misc/impl/AccountTxPaging.cpp +++ b/src/ripple/app/misc/impl/AccountTxPaging.cpp @@ -40,7 +40,7 @@ convertBlobsToTxResult ( Application& app) { SerialIter it (makeSlice(rawTxn)); - STTx::pointer txn = std::make_shared (it); + auto txn = std::make_shared (it); std::string reason; auto tr = std::make_shared (txn, reason, app); diff --git a/src/ripple/app/tx/LocalTxs.h b/src/ripple/app/tx/LocalTxs.h index b8366b7008..4985f8467c 100644 --- a/src/ripple/app/tx/LocalTxs.h +++ b/src/ripple/app/tx/LocalTxs.h @@ -36,7 +36,7 @@ public: virtual ~LocalTxs () = default; // Add a new local transaction - virtual void push_back (LedgerIndex index, STTx::ref txn) = 0; + virtual void push_back (LedgerIndex index, std::shared_ptr const& txn) = 0; // Return the set of local transactions to a new open ledger virtual CanonicalTXSet getTxSet () = 0; diff --git a/src/ripple/app/tx/Transaction.h b/src/ripple/app/tx/Transaction.h index 6bba017856..f7ee528048 100644 --- a/src/ripple/app/tx/Transaction.h +++ b/src/ripple/app/tx/Transaction.h @@ -64,7 +64,7 @@ public: public: Transaction ( - STTx::ref, std::string&, Application&) noexcept; + std::shared_ptr const&, std::string&, Application&) noexcept; static Transaction::pointer @@ -90,7 +90,7 @@ public: TransStatus sqlTransactionStatus(boost::optional const& status); - STTx::ref getSTransaction () + std::shared_ptr const& getSTransaction () { return mTransaction; } @@ -170,7 +170,7 @@ private: TER mResult = temUNCERTAIN; bool mApplying = false; - STTx::pointer mTransaction; + std::shared_ptr mTransaction; Application& mApp; beast::Journal j_; }; diff --git a/src/ripple/app/tx/TransactionMaster.h b/src/ripple/app/tx/TransactionMaster.h index 8ddb567b80..b257e4e2b7 100644 --- a/src/ripple/app/tx/TransactionMaster.h +++ b/src/ripple/app/tx/TransactionMaster.h @@ -37,15 +37,23 @@ public: TransactionMaster (TransactionMaster const&) = delete; TransactionMaster& operator= (TransactionMaster const&) = delete; - Transaction::pointer fetch (uint256 const& , bool checkDisk); - STTx::pointer fetch (std::shared_ptr const& item, SHAMapTreeNode:: TNType type, - bool checkDisk, std::uint32_t uCommitLedger); + Transaction::pointer + fetch (uint256 const& , bool checkDisk); + + std::shared_ptr + fetch (std::shared_ptr const& item, + SHAMapTreeNode:: TNType type, + bool checkDisk, std::uint32_t uCommitLedger); // return value: true = we had the transaction already bool inLedger (uint256 const& hash, std::uint32_t ledger); + void canonicalize (Transaction::pointer* pTransaction); + void sweep (void); - TaggedCache & getCache(); + + TaggedCache & + getCache(); private: Application& mApp; diff --git a/src/ripple/app/tx/impl/LocalTxs.cpp b/src/ripple/app/tx/impl/LocalTxs.cpp index 2044345846..a3f1bd343a 100644 --- a/src/ripple/app/tx/impl/LocalTxs.cpp +++ b/src/ripple/app/tx/impl/LocalTxs.cpp @@ -59,7 +59,7 @@ public: // get into a fully-validated ledger. static int const holdLedgers = 5; - LocalTx (LedgerIndex index, STTx::ref txn) + LocalTx (LedgerIndex index, std::shared_ptr const& txn) : m_txn (txn) , m_expire (index + holdLedgers) , m_id (txn->getTransactionID ()) @@ -85,7 +85,7 @@ public: return i > m_expire; } - STTx::ref getTX () const + std::shared_ptr const& getTX () const { return m_txn; } @@ -97,7 +97,7 @@ public: private: - STTx::pointer m_txn; + std::shared_ptr m_txn; LedgerIndex m_expire; uint256 m_id; AccountID m_account; @@ -113,7 +113,7 @@ public: LocalTxsImp() = default; // Add a new transaction to the set of local transactions - void push_back (LedgerIndex index, STTx::ref txn) override + void push_back (LedgerIndex index, std::shared_ptr const& txn) override { std::lock_guard lock (m_lock); diff --git a/src/ripple/app/tx/impl/Transaction.cpp b/src/ripple/app/tx/impl/Transaction.cpp index 83c79338d4..2d61cad9f8 100644 --- a/src/ripple/app/tx/impl/Transaction.cpp +++ b/src/ripple/app/tx/impl/Transaction.cpp @@ -31,7 +31,7 @@ namespace ripple { -Transaction::Transaction (STTx::ref stx, +Transaction::Transaction (std::shared_ptr const& stx, std::string& reason, Application& app) noexcept : mTransaction (stx) @@ -62,7 +62,7 @@ Transaction::pointer Transaction::sharedTransaction ( try { SerialIter sit (makeSlice(vucTransaction)); - auto txn = std::make_shared(sit); + auto txn = std::make_shared(sit); std::string reason; auto result = std::make_shared ( txn, reason, app); @@ -125,7 +125,7 @@ Transaction::pointer Transaction::transactionFromSQL ( rangeCheckedCast(ledgerSeq.value_or (0)); SerialIter it (makeSlice(rawTxn)); - auto txn = std::make_shared (it); + auto txn = std::make_shared (it); std::string reason; auto tr = std::make_shared ( txn, reason, app); diff --git a/src/ripple/app/tx/impl/TransactionMaster.cpp b/src/ripple/app/tx/impl/TransactionMaster.cpp index 98bd047e02..88b1c9ddac 100644 --- a/src/ripple/app/tx/impl/TransactionMaster.cpp +++ b/src/ripple/app/tx/impl/TransactionMaster.cpp @@ -60,11 +60,12 @@ Transaction::pointer TransactionMaster::fetch (uint256 const& txnID, bool checkD return txn; } -STTx::pointer TransactionMaster::fetch (std::shared_ptr const& item, +std::shared_ptr +TransactionMaster::fetch (std::shared_ptr const& item, SHAMapTreeNode::TNType type, bool checkDisk, std::uint32_t uCommitLedger) { - STTx::pointer txn; + std::shared_ptr txn; auto iTx = fetch (item->key(), false); if (!iTx) @@ -73,12 +74,12 @@ STTx::pointer TransactionMaster::fetch (std::shared_ptr const& item, if (type == SHAMapTreeNode::tnTRANSACTION_NM) { SerialIter sit (item->slice()); - txn = std::make_shared (std::ref (sit)); + txn = std::make_shared (std::ref (sit)); } else if (type == SHAMapTreeNode::tnTRANSACTION_MD) { auto blob = SerialIter{item->data(), item->size()}.getVL(); - txn = std::make_shared(SerialIter{blob.data(), blob.size()}); + txn = std::make_shared(SerialIter{blob.data(), blob.size()}); } } else diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index bf341a8907..3e8849ee90 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1031,7 +1031,7 @@ PeerImp::onMessage (std::shared_ptr const& m) try { - auto stx = std::make_shared(sit); + auto stx = std::make_shared(sit); uint256 txID = stx->getTransactionID (); int flags; @@ -1716,7 +1716,7 @@ PeerImp::doFetchPack (const std::shared_ptr& packet void PeerImp::checkTransaction (int flags, - bool checkSignature, STTx::pointer stx) + bool checkSignature, std::shared_ptr const& stx) { // VFALCO TODO Rewrite to not use exceptions try diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index 884868e65c..a4d59c89bf 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -449,7 +449,8 @@ private: doFetchPack (const std::shared_ptr& packet); void - checkTransaction (int flags, bool checkSignature, STTx::pointer stx); + checkTransaction (int flags, bool checkSignature, + std::shared_ptr const& stx); void checkPropose (Job& job, diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index c8360c62b6..d2fe8b85ec 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -24,6 +24,7 @@ #include #include #include +#include namespace ripple { @@ -42,9 +43,6 @@ class STTx final public: static char const* getCountedObjectName () { return "STTx"; } - using pointer = std::shared_ptr; - using ref = const std::shared_ptr&; - static std::size_t const minMultiSigners = 1; static std::size_t const maxMultiSigners = 8; @@ -134,11 +132,22 @@ private: bool checkSingleSign () const; bool checkMultiSign () const; + boost::optional tid_; TxType tx_type_; }; bool passesLocalChecks (STObject const& st, std::string&); +/** Sterilize a transaction. + + The transaction is serialized and then deserialized, + ensuring that all equivalent transactions are in canonical + form. This also ensures that program metadata such as + the transaction's digest, are all computed. +*/ +std::shared_ptr +sterilize (STTx const& stx); + } // ripple #endif diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 42835f755e..f2e0a1c1c7 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,7 @@ #include // #include #include +#include namespace ripple { @@ -74,6 +76,8 @@ STTx::STTx (STObject&& object) "Transaction not legal for format"; throw std::runtime_error ("transaction not valid"); } + + tid_ = getHash(HashPrefix::transactionID); } STTx::STTx (SerialIter& sit) @@ -106,6 +110,8 @@ STTx::STTx (SerialIter& sit) "Transaction not legal for format"; throw std::runtime_error ("transaction not valid"); } + + tid_ = getHash(HashPrefix::transactionID); } std::string @@ -161,7 +167,10 @@ STTx::getSigningHash () const uint256 STTx::getTransactionID () const { - return getHash (HashPrefix::transactionID); + assert(tid_); + if (! tid_) + LogicError("digest is undefined"); + return *tid_; } Blob STTx::getSignature () const @@ -180,6 +189,7 @@ void STTx::sign (RippleAddress const& private_key) { Blob const signature = private_key.accountPrivateSign (getSigningData (*this)); setFieldVL (sfTxnSignature, signature); + tid_ = getHash(HashPrefix::transactionID); } bool STTx::checkSign(bool allowMultiSign) const @@ -499,4 +509,13 @@ bool passesLocalChecks (STObject const& st, std::string& reason) return true; } +std::shared_ptr +sterilize (STTx const& stx) +{ + Serializer s; + stx.add(s); + SerialIter sit(s.slice()); + return std::make_shared(std::ref(sit)); +} + } // ripple diff --git a/src/ripple/rpc/handlers/Submit.cpp b/src/ripple/rpc/handlers/Submit.cpp index c39ebe6c51..18962cbc47 100644 --- a/src/ripple/rpc/handlers/Submit.cpp +++ b/src/ripple/rpc/handlers/Submit.cpp @@ -67,11 +67,11 @@ Json::Value doSubmit (RPC::Context& context) SerialIter sitTrans (makeSlice(ret.first)); - STTx::pointer stpTrans; + std::shared_ptr stpTrans; try { - stpTrans = std::make_shared (std::ref (sitTrans)); + stpTrans = std::make_shared (std::ref (sitTrans)); } catch (std::exception& e) { diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 4e33d8628b..8d07065fe2 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -295,11 +295,11 @@ checkTxJsonFields ( //------------------------------------------------------------------------------ // A move-only struct that makes it easy to return either a Json::Value or a -// STTx::pointer from transactionPreProcessImpl (). +// std::shared_ptr from transactionPreProcessImpl (). struct transactionPreProcessResult { Json::Value const first; - STTx::pointer const second; + std::shared_ptr const second; transactionPreProcessResult () = delete; transactionPreProcessResult (transactionPreProcessResult const&) = delete; @@ -318,7 +318,7 @@ struct transactionPreProcessResult , second () { } - transactionPreProcessResult (STTx::pointer&& st) + transactionPreProcessResult (std::shared_ptr&& st) : first () , second (std::move (st)) { } @@ -463,7 +463,7 @@ transactionPreProcessImpl ( return std::move (err); } - STTx::pointer stpTrans; + std::shared_ptr stpTrans; try { // If we're generating a multi-signature the SigningPubKey must be @@ -501,12 +501,12 @@ transactionPreProcessImpl ( stpTrans->sign (keypair.secretKey); } - return std::move (stpTrans); + return transactionPreProcessResult {std::move (stpTrans)}; } static std::pair -transactionConstructImpl (STTx::pointer stpTrans, +transactionConstructImpl (std::shared_ptr const& stpTrans, Rules const& rules, Application& app) { std::pair ret; @@ -931,7 +931,7 @@ Json::Value transactionSubmitMultiSigned ( } // Grind through the JSON in tx_json to produce a STTx - STTx::pointer stpTrans; + std::shared_ptr stpTrans; { STParsedJSONObject parsedTx_json ("tx_json", tx_json); if (!parsedTx_json.object) @@ -944,8 +944,8 @@ Json::Value transactionSubmitMultiSigned ( } try { - stpTrans = - std::make_shared (std::move(parsedTx_json.object.get())); + stpTrans = std::make_shared( + std::move(parsedTx_json.object.get())); } catch (std::exception& ex) { diff --git a/src/ripple/test/jtx/impl/Env.cpp b/src/ripple/test/jtx/impl/Env.cpp index c4be36dea0..284c60fbdf 100644 --- a/src/ripple/test/jtx/impl/Env.cpp +++ b/src/ripple/test/jtx/impl/Env.cpp @@ -270,15 +270,14 @@ void Env::submit (JTx const& jt) { bool didApply; - auto const& stx = jt.stx; - if (stx) + if (jt.stx) { - txid_ = stx->getTransactionID(); + txid_ = jt.stx->getTransactionID(); openLedger.modify( [&](OpenView& view, beast::Journal j) { std::tie(ter_, didApply) = ripple::apply( - app(), view, *stx, applyFlags(), + app(), view, *jt.stx, applyFlags(), beast::Journal{}); return didApply; }); @@ -385,8 +384,7 @@ Env::st (JTx const& jt) try { - return std::make_shared( - std::move(*obj)); + return sterilize(STTx{std::move(*obj)}); } catch(...) {