From cc94abf891c28c259ffea739b721d0e787860966 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 20 Oct 2012 12:20:47 -0700 Subject: [PATCH 1/3] Some cleanups. --- src/SerializedTypes.h | 2 -- src/TransactionMeta.cpp | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/SerializedTypes.h b/src/SerializedTypes.h index d7df1d1b7..fadc452e0 100644 --- a/src/SerializedTypes.h +++ b/src/SerializedTypes.h @@ -36,13 +36,11 @@ protected: SField::ptr fName; virtual SerializedType* duplicate() const { return new SerializedType(*fName); } - SerializedType(SField::ptr n) : fName(n) { assert(fName); } public: SerializedType() : fName(&sfGeneric) { ; } SerializedType(SField::ref n) : fName(&n) { assert(fName); } - SerializedType(const SerializedType& n) : fName(n.fName) { ; } virtual ~SerializedType() { ; } static std::auto_ptr deserialize(SField::ref name) diff --git a/src/TransactionMeta.cpp b/src/TransactionMeta.cpp index 30117e1e7..068329b8e 100644 --- a/src/TransactionMeta.cpp +++ b/src/TransactionMeta.cpp @@ -31,6 +31,7 @@ bool TransactionMetaSet::isNodeAffected(const uint256& node) const STObject& TransactionMetaSet::getAffectedNode(const uint256& node, SField::ref type, bool overrideType) { + assert(&type); for (STArray::iterator it = mNodes.begin(); it != mNodes.end(); ++it) { if (it->getFieldH256(sfLedgerIndex) == node) @@ -47,7 +48,7 @@ STObject& TransactionMetaSet::getAffectedNode(const uint256& node, SField::ref t assert(obj.getFName() == type); obj.setFieldH256(sfLedgerIndex, node); - return mNodes.back(); + return obj; } const STObject& TransactionMetaSet::peekAffectedNode(const uint256& node) const From 7b8e445456c010d81173877ae9fd5f640c36514e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 20 Oct 2012 12:52:20 -0700 Subject: [PATCH 2/3] Some extra constructors. --- src/SerializedObject.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SerializedObject.h b/src/SerializedObject.h index 69683ed49..0c9425917 100644 --- a/src/SerializedObject.h +++ b/src/SerializedObject.h @@ -170,7 +170,9 @@ protected: public: STArray() { ; } + STArray(int n) { value.reserve(n); } STArray(SField::ref f) : SerializedType(f) { ; } + STArray(SField::ref f, int n) : SerializedType(f) { value.reserve(n); } STArray(SField::ref f, const vector& v) : SerializedType(f), value(v) { ; } STArray(vector& v) : value(v) { ; } From 4b2b75b3678984cbfdbdb684d58e3192190d56dc Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 20 Oct 2012 12:52:25 -0700 Subject: [PATCH 3/3] Fix the bug Andrey reported. A reference into an array can become invalid if the array changes size. This happened in the txn metadata code when we had to thread while handling a node. --- src/LedgerEntrySet.cpp | 22 ++++++++++----------- src/TransactionMeta.cpp | 43 +++++++++++++++++++++++++++++++++-------- src/TransactionMeta.h | 4 +++- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/LedgerEntrySet.cpp b/src/LedgerEntrySet.cpp index d074f902d..e42be001d 100644 --- a/src/LedgerEntrySet.cpp +++ b/src/LedgerEntrySet.cpp @@ -318,7 +318,7 @@ bool LedgerEntrySet::threadTx(SLE::ref threadTo, Ledger::ref ledger, uint32 prevLgrID; if (!threadTo->thread(mSet.getTxID(), mSet.getLgrSeq(), prevTxID, prevLgrID)) return false; - if (TransactionMetaSet::thread(mSet.getAffectedNode(threadTo->getIndex(), sfModifiedNode, false), + if (TransactionMetaSet::thread(mSet.getAffectedNode(threadTo->getIndex(), sfModifiedNode), prevTxID, prevLgrID)) return true; assert(false); @@ -395,7 +395,7 @@ void LedgerEntrySet::calcRawMeta(Serializer& s, TER result) continue; SLE::pointer curNode = it->second.mEntry; - STObject &metaNode = mSet.getAffectedNode(it->first, *type, true); + mSet.setAffectedNode(it->first, *type); if (type == &sfDeletedNode) { @@ -406,16 +406,16 @@ void LedgerEntrySet::calcRawMeta(Serializer& s, TER result) { // node has an amount, covers ripple state nodes STAmount amount = origNode->getFieldAmount(sfAmount); if (amount.isNonZero()) - metaNode.setFieldAmount(sfPreviousBalance, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfPreviousBalance, amount); amount = curNode->getFieldAmount(sfAmount); if (amount.isNonZero()) - metaNode.setFieldAmount(sfFinalBalance, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfFinalBalance, amount); if (origNode->getType() == ltRIPPLE_STATE) { - metaNode.setFieldAccount(sfLowID, + mSet.getAffectedNode(it->first).setFieldAccount(sfLowID, NewcoinAddress::createAccountID(origNode->getFieldAmount(sfLowLimit).getIssuer())); - metaNode.setFieldAccount(sfHighID, + mSet.getAffectedNode(it->first).setFieldAccount(sfHighID, NewcoinAddress::createAccountID(origNode->getFieldAmount(sfHighLimit).getIssuer())); } } @@ -424,10 +424,10 @@ void LedgerEntrySet::calcRawMeta(Serializer& s, TER result) { // check for non-zero balances STAmount amount = origNode->getFieldAmount(sfTakerPays); if (amount.isNonZero()) - metaNode.setFieldAmount(sfFinalTakerPays, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfFinalTakerPays, amount); amount = origNode->getFieldAmount(sfTakerGets); if (amount.isNonZero()) - metaNode.setFieldAmount(sfFinalTakerGets, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfFinalTakerGets, amount); } } @@ -451,17 +451,17 @@ void LedgerEntrySet::calcRawMeta(Serializer& s, TER result) { // node has an amount, covers account root nodes and ripple nodes STAmount amount = origNode->getFieldAmount(sfAmount); if (amount != curNode->getFieldAmount(sfAmount)) - metaNode.setFieldAmount(sfPreviousBalance, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfPreviousBalance, amount); } if (origNode->getType() == ltOFFER) { STAmount amount = origNode->getFieldAmount(sfTakerPays); if (amount != curNode->getFieldAmount(sfTakerPays)) - metaNode.setFieldAmount(sfPreviousTakerPays, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfPreviousTakerPays, amount); amount = origNode->getFieldAmount(sfTakerGets); if (amount != curNode->getFieldAmount(sfTakerGets)) - metaNode.setFieldAmount(sfPreviousTakerGets, amount); + mSet.getAffectedNode(it->first).setFieldAmount(sfPreviousTakerGets, amount); } } diff --git a/src/TransactionMeta.cpp b/src/TransactionMeta.cpp index 068329b8e..b6b33a98f 100644 --- a/src/TransactionMeta.cpp +++ b/src/TransactionMeta.cpp @@ -6,8 +6,10 @@ #include #include +#include "Log.h" + TransactionMetaSet::TransactionMetaSet(const uint256& txid, uint32 ledger, const std::vector& vec) : - mTransactionID(txid), mLedger(ledger), mNodes(sfAffectedNodes) + mTransactionID(txid), mLedger(ledger), mNodes(sfAffectedNodes, 32) { Serializer s(vec); SerializerIterator sit(s); @@ -29,16 +31,14 @@ bool TransactionMetaSet::isNodeAffected(const uint256& node) const return false; } -STObject& TransactionMetaSet::getAffectedNode(const uint256& node, SField::ref type, bool overrideType) -{ - assert(&type); +void TransactionMetaSet::setAffectedNode(const uint256& node, SField::ref type) +{ // make sure the node exists and force its type for (STArray::iterator it = mNodes.begin(); it != mNodes.end(); ++it) { if (it->getFieldH256(sfLedgerIndex) == node) { - if (overrideType) - it->setFName(type); - return *it; + it->setFName(type); + return; } } @@ -47,10 +47,37 @@ STObject& TransactionMetaSet::getAffectedNode(const uint256& node, SField::ref t assert(obj.getFName() == type); obj.setFieldH256(sfLedgerIndex, node); +} + +STObject& TransactionMetaSet::getAffectedNode(const uint256& node, SField::ref type) +{ + assert(&type); + for (STArray::iterator it = mNodes.begin(); it != mNodes.end(); ++it) + { + if (it->getFieldH256(sfLedgerIndex) == node) + return *it; + } + + mNodes.push_back(STObject(sfModifiedNode)); + STObject& obj = mNodes.back(); + + assert(obj.getFName() == type); + obj.setFieldH256(sfLedgerIndex, node); return obj; } +STObject& TransactionMetaSet::getAffectedNode(const uint256& node) +{ + for (STArray::iterator it = mNodes.begin(); it != mNodes.end(); ++it) + { + if (it->getFieldH256(sfLedgerIndex) == node) + return *it; + } + assert(false); + throw std::runtime_error("Affected node not found"); +} + const STObject& TransactionMetaSet::peekAffectedNode(const uint256& node) const { for (STArray::const_iterator it = mNodes.begin(); it != mNodes.end(); ++it) @@ -63,7 +90,7 @@ void TransactionMetaSet::init(const uint256& id, uint32 ledger) { mTransactionID = id; mLedger = ledger; - mNodes = STArray(sfAffectedNodes); + mNodes = STArray(sfAffectedNodes, 32); } void TransactionMetaSet::swap(TransactionMetaSet& s) diff --git a/src/TransactionMeta.h b/src/TransactionMeta.h index 81b8c2a44..100b09900 100644 --- a/src/TransactionMeta.h +++ b/src/TransactionMeta.h @@ -39,7 +39,9 @@ public: uint32 getLgrSeq() { return mLedger; } bool isNodeAffected(const uint256&) const; - STObject& getAffectedNode(const uint256&, SField::ref type, bool overrideType); + void setAffectedNode(const uint256&, SField::ref type); + STObject& getAffectedNode(const uint256&, SField::ref type); + STObject& getAffectedNode(const uint256&); const STObject& peekAffectedNode(const uint256&) const; Json::Value getJson(int p) const { return getAsObject().getJson(p); }