From 39cb1899d0f252bc20b8417ea29d96b56be8268d Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 3 Oct 2012 22:23:32 -0700 Subject: [PATCH 1/9] A few small bugfixes and some exra logging to track down a sync bug that Jed reported. --- src/ConnectionPool.cpp | 8 ++++- src/ConnectionPool.h | 2 +- src/LedgerAcquire.cpp | 3 +- src/LedgerConsensus.cpp | 4 +++ src/NetworkOPs.cpp | 65 +++++++++++++++++++++++++++------------ src/NetworkOPs.h | 1 + src/Peer.cpp | 3 +- src/TransactionEngine.cpp | 4 --- 8 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/ConnectionPool.cpp b/src/ConnectionPool.cpp index ed196af6c..6285d4555 100644 --- a/src/ConnectionPool.cpp +++ b/src/ConnectionPool.cpp @@ -227,8 +227,9 @@ void ConnectionPool::policyHandler(const boost::system::error_code& ecResult) // YYY: Should probably do this in the background. // YYY: Might end up sending to disconnected peer? -void ConnectionPool::relayMessage(Peer* fromPeer, const PackedMessage::pointer& msg) +int ConnectionPool::relayMessage(Peer* fromPeer, const PackedMessage::pointer& msg) { + int sentTo = 0; boost::mutex::scoped_lock sl(mPeerLock); BOOST_FOREACH(naPeer pair, mConnectedMap) @@ -237,8 +238,13 @@ void ConnectionPool::relayMessage(Peer* fromPeer, const PackedMessage::pointer& if (!peer) std::cerr << "CP::RM null peer in list" << std::endl; else if ((!fromPeer || !(peer.get() == fromPeer)) && peer->isConnected()) + { + ++sentTo; peer->sendPacket(msg); + } } + + return sentTo; } // Schedule a connection via scanning. diff --git a/src/ConnectionPool.h b/src/ConnectionPool.h index bb13d0643..4db023985 100644 --- a/src/ConnectionPool.h +++ b/src/ConnectionPool.h @@ -58,7 +58,7 @@ public: void start(); // Send message to network. - void relayMessage(Peer* fromPeer, const PackedMessage::pointer& msg); + int relayMessage(Peer* fromPeer, const PackedMessage::pointer& msg); // Manual connection request. // Queue for immediate scanning. diff --git a/src/LedgerAcquire.cpp b/src/LedgerAcquire.cpp index bbfd67063..76a43e727 100644 --- a/src/LedgerAcquire.cpp +++ b/src/LedgerAcquire.cpp @@ -267,7 +267,8 @@ void PeerSet::sendRequest(const newcoin::TMGetLedger& tmGL, Peer::ref peer) void PeerSet::sendRequest(const newcoin::TMGetLedger& tmGL) { boost::recursive_mutex::scoped_lock sl(mLock); - if (mPeers.empty()) return; + if (mPeers.empty()) + return; PackedMessage::pointer packet = boost::make_shared(tmGL, newcoin::mtGET_LEDGER); diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 2dc6672f6..65730e817 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -50,9 +50,13 @@ boost::weak_ptr TransactionAcquire::pmDowncast() void TransactionAcquire::trigger(Peer::ref peer, bool timer) { if (mComplete || mFailed) + { + Log(lsINFO) << "complete or failed"; return; + } if (!mHaveRoot) { + Log(lsINFO) << "have no root"; newcoin::TMGetLedger tmGL; tmGL.set_ledgerhash(mHash.begin(), mHash.size()); tmGL.set_itype(newcoin::liTS_CANDIDATE); diff --git a/src/NetworkOPs.cpp b/src/NetworkOPs.cpp index ce77bf69d..4f24651c9 100644 --- a/src/NetworkOPs.cpp +++ b/src/NetworkOPs.cpp @@ -25,7 +25,7 @@ NetworkOPs::NetworkOPs(boost::asio::io_service& io_service, LedgerMaster* pLedgerMaster) : mMode(omDISCONNECTED),mNetTimer(io_service), mLedgerMaster(pLedgerMaster), mCloseTimeOffset(0), - mLastCloseProposers(0), mLastCloseConvergeTime(LEDGER_IDLE_INTERVAL), mLastValidationTime(0) + mLastCloseProposers(0), mLastCloseConvergeTime(1000 * LEDGER_IDLE_INTERVAL), mLastValidationTime(0) { } @@ -114,7 +114,18 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, } TER r = mLedgerMaster->doTransaction(*trans->getSTransaction(), tapOPEN_LEDGER); - if (r == tefFAILURE) throw Fault(IO_ERROR); + +#ifdef DEBUG + if (r != tesSUCCESS) + { + std::string token, human; + if (transResultInfo(r, token, human)) + Log(lsINFO) << "TransactionResult: " << token << ": " << human; + } +#endif + + if (r == tefFAILURE) + throw Fault(IO_ERROR); if (r == terPRE_SEQ) { // transaction should be held @@ -150,7 +161,8 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, tx.set_receivetimestamp(getNetworkTimeNC()); PackedMessage::pointer packet = boost::make_shared(tx, newcoin::mtTRANSACTION); - theApp->getConnectionPool().relayMessage(source, packet); + int sentTo = theApp->getConnectionPool().relayMessage(source, packet); + Log(lsINFO) << "Transaction relayed to " << sentTo << " node(s)"; return trans; } @@ -629,6 +641,24 @@ int NetworkOPs::beginConsensus(const uint256& networkClosed, Ledger::pointer clo return mConsensus->startup(); } +bool NetworkOPs::haveConsensusObject() +{ + if (mConsensus) + return true; + if (mMode != omFULL) + return false; + + uint256 networkClosed; + std::vector peerList = theApp->getConnectionPool().getPeerVector(); + bool ledgerChange = checkLastClosedLedger(peerList, networkClosed); + if (!ledgerChange) + { + Log(lsWARNING) << "Beginning consensus due to peer action"; + beginConsensus(networkClosed, theApp->getMasterLedger().getCurrentLedger()); + } + return mConsensus; +} + // <-- bool: true to relay bool NetworkOPs::recvPropose(uint32 proposeSeq, const uint256& proposeHash, const uint256& prevLedger, uint32 closeTime, const std::string& pubKey, const std::string& signature, const NewcoinAddress& nodePublic) @@ -651,18 +681,7 @@ bool NetworkOPs::recvPropose(uint32 proposeSeq, const uint256& proposeHash, cons NewcoinAddress naPeerPublic = NewcoinAddress::createNodePublic(strCopy(pubKey)); - if ((!mConsensus) && (mMode == omFULL)) - { - uint256 networkClosed; - std::vector peerList = theApp->getConnectionPool().getPeerVector(); - bool ledgerChange = checkLastClosedLedger(peerList, networkClosed); - if (!ledgerChange) - { - Log(lsWARNING) << "Beginning consensus due to proposal from peer"; - beginConsensus(networkClosed, theApp->getMasterLedger().getCurrentLedger()); - } - } - if (!mConsensus) + if (!haveConsensusObject()) { Log(lsINFO) << "Received proposal outside consensus window"; return mMode != omFULL; @@ -704,7 +723,7 @@ bool NetworkOPs::recvPropose(uint32 proposeSeq, const uint256& proposeHash, cons SHAMap::pointer NetworkOPs::getTXMap(const uint256& hash) { - if (!mConsensus) + if (!haveConsensusObject()) return SHAMap::pointer(); return mConsensus->getTransactionTree(hash, false); } @@ -712,21 +731,24 @@ SHAMap::pointer NetworkOPs::getTXMap(const uint256& hash) bool NetworkOPs::gotTXData(const boost::shared_ptr& peer, const uint256& hash, const std::list& nodeIDs, const std::list< std::vector >& nodeData) { - if (!mConsensus) + if (!haveConsensusObject()) return false; return mConsensus->peerGaveNodes(peer, hash, nodeIDs, nodeData); } bool NetworkOPs::hasTXSet(const boost::shared_ptr& peer, const uint256& set, newcoin::TxSetStatus status) { - if (!mConsensus) + if (!haveConsensusObject()) + { + Log(lsINFO) << "Peer has TX set, not during consensus"; return false; + } return mConsensus->peerHasSet(peer, set, status); } void NetworkOPs::mapComplete(const uint256& hash, SHAMap::ref map) { - if (mConsensus) + if (!haveConsensusObject()) mConsensus->mapComplete(hash, map, true); } @@ -832,6 +854,11 @@ Json::Value NetworkOPs::getServerInfo() if (!theConfig.VALIDATION_SEED.isValid()) info["serverState"] = "none"; else info["validationPKey"] = NewcoinAddress::createNodePublic(theConfig.VALIDATION_SEED).humanNodePublic(); + Json::Value lastClose = Json::objectValue; + lastClose["proposers"] = theApp->getOPs().getPreviousProposers(); + lastClose["convergeTime"] = theApp->getOPs().getPreviousConvergeTime(); + info["lastClose"] = lastClose; + if (mConsensus) info["consensus"] = mConsensus->getJson(); diff --git a/src/NetworkOPs.h b/src/NetworkOPs.h index cc84f9f89..4c64374f1 100644 --- a/src/NetworkOPs.h +++ b/src/NetworkOPs.h @@ -84,6 +84,7 @@ protected: Json::Value transJson(const SerializedTransaction& stTxn, TER terResult, const std::string& strStatus, int iSeq, const std::string& strType); void pubTransactionAll(Ledger::ref lpCurrent, const SerializedTransaction& stTxn, TER terResult, const char* pState); void pubTransactionAccounts(Ledger::ref lpCurrent, const SerializedTransaction& stTxn, TER terResult, const char* pState); + bool haveConsensusObject(); Json::Value pubBootstrapAccountInfo(Ledger::ref lpAccepted, const NewcoinAddress& naAccountID); diff --git a/src/Peer.cpp b/src/Peer.cpp index 641a33571..db4ca281b 100644 --- a/src/Peer.cpp +++ b/src/Peer.cpp @@ -937,7 +937,6 @@ void Peer::recvGetLedger(newcoin::TMGetLedger& packet) if (packet.itype() == newcoin::liTS_CANDIDATE) { // Request is for a transaction candidate set Log(lsINFO) << "Received request for TX candidate set data " << getIP(); - Ledger::pointer ledger; if ((!packet.has_ledgerhash() || packet.ledgerhash().size() != 32)) { punishPeer(PP_INVALID_REQUEST); @@ -948,7 +947,7 @@ void Peer::recvGetLedger(newcoin::TMGetLedger& packet) map = theApp->getOPs().getTXMap(txHash); if (!map) { - Log(lsERROR) << "We do not hav the map our peer wants"; + Log(lsERROR) << "We do not have the map our peer wants"; punishPeer(PP_INVALID_REQUEST); return; } diff --git a/src/TransactionEngine.cpp b/src/TransactionEngine.cpp index a89562dd0..2994034c8 100644 --- a/src/TransactionEngine.cpp +++ b/src/TransactionEngine.cpp @@ -345,11 +345,7 @@ TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, Transa terResult = terPRE_SEQ; } else if (mLedger->hasTransaction(txID)) - { - Log(lsWARNING) << "applyTransaction: duplicate sequence number"; - terResult = tefALREADY; - } else { Log(lsWARNING) << "applyTransaction: past sequence number"; From 414a44b6b5c3fdc65f0d8e41574811cda0749af1 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 02:19:47 -0700 Subject: [PATCH 2/9] Prevent a race condition that can cause us to miss an "I have a transaction set" message if it arrives as we're in the process of generating a new last-closed ledger. --- src/LedgerConsensus.cpp | 8 ++++++++ src/Peer.cpp | 27 ++++++++++++++++++++++++--- src/Peer.h | 3 +++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 65730e817..b96589212 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -790,6 +790,14 @@ void LedgerConsensus::startAcquiring(const TransactionAcquire::pointer& acquire) } } } + + std::vector peerList = theApp->getConnectionPool().getPeerVector(); + BOOST_FOREACH(Peer::ref peer, peerList) + { + if (peer->hasTxSet(acquire->getHash()) + acquire->peerHash(peer); + } + acquire->resetTimer(); } diff --git a/src/Peer.cpp b/src/Peer.cpp index db4ca281b..2397071ae 100644 --- a/src/Peer.cpp +++ b/src/Peer.cpp @@ -744,8 +744,11 @@ void Peer::recvHaveTxSet(newcoin::TMHaveTransactionSet& packet) punishPeer(PP_INVALID_REQUEST); return; } - memcpy(hashes.begin(), packet.hash().data(), 32); - if (!theApp->getOPs().hasTXSet(shared_from_this(), hashes, packet.status())) + uint256 hash; + memcpy(hash.begin(), packet.hash().data(), 32); + if (packet.status() == newcoin::tsHAVE) + addTxSet(hash); + if (!theApp->getOPs().hasTXSet(shared_from_this(), hash, packet.status())) punishPeer(PP_UNWANTED_DATA); } @@ -1142,11 +1145,29 @@ void Peer::addLedger(const uint256& hash) BOOST_FOREACH(const uint256& ledger, mRecentLedgers) if (ledger == hash) return; - if (mRecentLedgers.size() == 16) + if (mRecentLedgers.size() == 128) mRecentLedgers.pop_front(); mRecentLedgers.push_back(hash); } +bool Peer::hasTxSet(const uint256& hash) const +{ + BOOST_FOREACH(const uint256& set, mRecentTxSets) + if (set == hash) + return true; + return false; +} + +void Peer::addTxSet(const uint256& hash) +{ + BOOST_FOREACH(const uint256& set, mRecentTxSets) + if (set == hash) + return; + if (mRecentTxSets.size() == 128) + mRecentTxSets.pop_front(); + mRecentTxSets.push_back(hash); +} + // Get session information we can sign to prevent man in the middle attack. // (both sides get the same information, neither side controls it) void Peer::getSessionCookie(std::string& strDst) diff --git a/src/Peer.h b/src/Peer.h index ae4abf730..0b28a1dc9 100644 --- a/src/Peer.h +++ b/src/Peer.h @@ -46,6 +46,7 @@ private: uint256 mClosedLedgerHash, mPreviousLedgerHash; std::list mRecentLedgers; + std::list mRecentTxSets; boost::asio::ssl::stream mSocketSsl; @@ -118,6 +119,7 @@ protected: void getSessionCookie(std::string& strDst); void addLedger(const uint256& ledger); + void addTxSet(const uint256& TxSet); public: @@ -157,6 +159,7 @@ public: uint256 getClosedLedgerHash() const { return mClosedLedgerHash; } bool hasLedger(const uint256& hash) const; + bool hasTxSet(const uint256& hash) const; NewcoinAddress getNodePublic() const { return mNodePublic; } void cycleStatus() { mPreviousLedgerHash = mClosedLedgerHash; mClosedLedgerHash.zero(); } }; From 015ba9f24d6ef0800ba53e91cc1017f14f8a9af9 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 12:53:38 -0700 Subject: [PATCH 3/9] Typo. --- src/LedgerConsensus.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index b96589212..9b12539be 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -794,8 +794,8 @@ void LedgerConsensus::startAcquiring(const TransactionAcquire::pointer& acquire) std::vector peerList = theApp->getConnectionPool().getPeerVector(); BOOST_FOREACH(Peer::ref peer, peerList) { - if (peer->hasTxSet(acquire->getHash()) - acquire->peerHash(peer); + if (peer->hasTxSet(acquire->getHash())) + acquire->peerHas(peer); } acquire->resetTimer(); From 9c637aedb99b3c63cbc39f3e2867d643853a5f03 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 12:54:20 -0700 Subject: [PATCH 4/9] Clean up some expensive logging. --- src/DeterministicKeys.cpp | 6 +++++- src/LedgerEntrySet.cpp | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/DeterministicKeys.cpp b/src/DeterministicKeys.cpp index dc050067f..08a14bb43 100644 --- a/src/DeterministicKeys.cpp +++ b/src/DeterministicKeys.cpp @@ -5,6 +5,8 @@ #include #include +// #define EC_DEBUG + // Functions to add CKey support for deterministic EC keys #include "Serializer.h" @@ -107,7 +109,9 @@ EC_KEY* CKey::GenerateRootDeterministicKey(const uint128& seed) BN_CTX_free(ctx); - assert(EC_KEY_check_key(pkey)==1); +#ifdef EC_DEBUG + assert(EC_KEY_check_key(pkey)==1); // CAUTION: This check is *very* expensive +#endif return pkey; } diff --git a/src/LedgerEntrySet.cpp b/src/LedgerEntrySet.cpp index 478290bb3..a3d4ef2ba 100644 --- a/src/LedgerEntrySet.cpp +++ b/src/LedgerEntrySet.cpp @@ -5,6 +5,8 @@ #include "Log.h" +// #define META_DEBUG + // Small for testing, should likely be 32 or 64. #define DIR_NODE_MAX 2 @@ -294,7 +296,9 @@ SLE::pointer LedgerEntrySet::getForMod(const uint256& node, Ledger::ref ledger, bool LedgerEntrySet::threadTx(const NewcoinAddress& threadTo, Ledger::ref ledger, boost::unordered_map& newMods) { +#ifdef META_DEBUG Log(lsTRACE) << "Thread to " << threadTo.getAccountID(); +#endif SLE::pointer sle = getForMod(Ledger::getAccountRootIndex(threadTo.getAccountID()), ledger, newMods); if (!sle) { @@ -321,12 +325,16 @@ bool LedgerEntrySet::threadOwners(SLE::ref node, Ledger::ref ledger, boost::unor { // thread new or modified node to owner or owners if (node->hasOneOwner()) // thread to owner's account { +#ifdef META_DEBUG Log(lsTRACE) << "Thread to single owner"; +#endif return threadTx(node->getOwner(), ledger, newMods); } else if (node->hasTwoOwners()) // thread to owner's accounts] { +#ifdef META_DEBUG Log(lsTRACE) << "Thread to two owners"; +#endif return threadTx(node->getFirstOwner(), ledger, newMods) && threadTx(node->getSecondOwner(), ledger, newMods); @@ -349,17 +357,23 @@ void LedgerEntrySet::calcRawMeta(Serializer& s) switch (it->second.mAction) { case taaMODIFY: +#ifdef META_DEBUG Log(lsTRACE) << "Modified Node " << it->first; +#endif nType = TMNModifiedNode; break; case taaDELETE: +#ifdef META_DEBUG Log(lsTRACE) << "Deleted Node " << it->first; +#endif nType = TMNDeletedNode; break; case taaCREATE: +#ifdef META_DEBUG Log(lsTRACE) << "Created Node " << it->first; +#endif nType = TMNCreatedNode; break; @@ -420,10 +434,7 @@ void LedgerEntrySet::calcRawMeta(Serializer& s) if ((nType == TMNCreatedNode) || (nType == TMNModifiedNode)) { if (curNode->isThreadedType()) // always thread to self - { - Log(lsTRACE) << "Thread to self"; threadTx(curNode, mLedger, newMod); - } } if (nType == TMNModifiedNode) @@ -454,7 +465,7 @@ void LedgerEntrySet::calcRawMeta(Serializer& s) it != end; ++it) entryModify(it->second); -#ifdef DEBUG +#ifdef META_DEBUG Log(lsINFO) << "Metadata:" << mSet.getJson(0); #endif From bf7c32daa19b9e4a08e19c86efa0255b6474bcc5 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 15:31:36 -0700 Subject: [PATCH 5/9] The five last missing bits of the JSON input code. --- src/SerializedObject.cpp | 50 +++++++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/src/SerializedObject.cpp b/src/SerializedObject.cpp index baf9ba1cb..d8b7b5dcb 100644 --- a/src/SerializedObject.cpp +++ b/src/SerializedObject.cpp @@ -761,17 +761,44 @@ Json::Value STVector256::getJson(int options) const std::string STArray::getFullText() const { - return "WRITEME"; + std::string r = "["; + + bool first = true; + BOOST_FOREACH(const STObject& o, value) + { + if (!first) + r += ","; + r += o.getFullText(); + first = false; + } + + r += "]"; + return r; } std::string STArray::getText() const { - return "WRITEME"; + std::string r = "["; + + bool first = true; + BOOST_FOREACH(const STObject& o, value) + { + if (!first) + r += ","; + r += o.getText(); + first = false; + } + + r += "]"; + return r; } -Json::Value STArray::getJson(int) const +Json::Value STArray::getJson(int p) const { - return Json::Value("WRITEME"); + Json::Value v = Json::arrayValue; + BOOST_FOREACH(const STObject& o, value) + v.append(o.getJson(p)); + return v; } void STArray::add(Serializer& s) const @@ -1004,13 +1031,24 @@ std::auto_ptr STObject::parseJson(const Json::Value& object, SField::r if (!currency.isString()) throw std::runtime_error("path element currencies must be strings"); hasCurrency = true; - // WRITEME + if (currency.asString().size() == 40) + uCurrency.SetHex(currency.asString()); + else if (!STAmount::currencyFromString(uCurrency, currency.asString())) + throw std::runtime_error("invalid currency"); } if (!issuer.isNull()) { // human account id if (!issuer.isString()) throw std::runtime_error("path element issuers must be strings"); - // WRITEME + if (issuer.asString().size() == 40) + uIssuer.SetHex(issuer.asString()); + else + { + NewcoinAddress a; + if (!a.setAccountPublic(issuer.asString())) + throw std::runtime_error("path element issuer invalid"); + uIssuer = a.getAccountID(); + } } p.addElement(STPathElement(uAccount, uCurrency, uIssuer, hasCurrency)); } From 8bddef34fcaf8183a0d8ff4e81283c8f088a9e2e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 17:01:34 -0700 Subject: [PATCH 6/9] Remove old unit test. --- src/SerializedObject.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SerializedObject.h b/src/SerializedObject.h index ed8d06560..2e28fc958 100644 --- a/src/SerializedObject.h +++ b/src/SerializedObject.h @@ -145,8 +145,6 @@ public: { return makeDefaultObject(STI_NOTPRESENT, name); } static std::auto_ptr makeDefaultObject(SField::ref name) { return makeDefaultObject(name.fieldType, name); } - - static void unitTest(); }; class STArray : public SerializedType From d4ee8720769a82edf33a4d49b2cc7dc8d30ebdce Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 17:01:42 -0700 Subject: [PATCH 7/9] Properly construct dynamic field names --- src/FieldNames.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/FieldNames.cpp b/src/FieldNames.cpp index 612d250c9..66fc52558 100644 --- a/src/FieldNames.cpp +++ b/src/FieldNames.cpp @@ -7,6 +7,7 @@ #include #include +#include "utils.h" // These must stay at the top of this file std::map SField::codeToField; @@ -52,7 +53,8 @@ SField::ref SField::getField(int code) return sfInvalid; } - return *(new SField(code, static_cast(type), field, NULL)); + std::string dynName = lexical_cast_i(type) + "/" + lexical_cast_i(field); + return *(new SField(code, static_cast(type), field, dynName.c_str())); } int SField::compare(SField::ref f1, SField::ref f2) From ab4e44b06de53a0d322c4467c0f6bd5c4c7e9338 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 17:01:57 -0700 Subject: [PATCH 8/9] Allow 'temporary' fields. --- src/FieldNames.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/FieldNames.h b/src/FieldNames.h index 784900c14..443ae5008 100644 --- a/src/FieldNames.h +++ b/src/FieldNames.h @@ -52,7 +52,11 @@ public: SField(int fc, SerializedTypeID tid, int fv, const char* fn) : fieldCode(fc), fieldType(tid), fieldValue(fv), fieldName(fn) - { codeToField[fc] = this; } + { codeToField[fieldCode] = this; } + + SField(SerializedTypeID tid, int fv, const char *fn, bool temporary) : + fieldCode(FIELD_CODE(tid, fv)), fieldType(tid), fieldValue(fv), fieldName(fn) + { if (!temporary) codeToField[fieldCode] = this; } SField(int fc) : fieldCode(fc), fieldType(STI_UNKNOWN), fieldValue(0) { ; } From 2d861a44f44a51a11bfbfbf60631e316e4f1e7bf Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 4 Oct 2012 17:02:14 -0700 Subject: [PATCH 9/9] Bugfix for makeFieldAbsent. New unit test. --- src/SerializedObject.cpp | 81 +++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/SerializedObject.cpp b/src/SerializedObject.cpp index d8b7b5dcb..6542fedd7 100644 --- a/src/SerializedObject.cpp +++ b/src/SerializedObject.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "../json/writer.h" @@ -427,7 +428,7 @@ void STObject::makeFieldAbsent(SField::ref field) if (f.getSType() == STI_NOTPRESENT) return; - mData.replace(index, makeDefaultObject(f.getFName())); + mData.replace(index, makeNonPresentObject(f.getFName())); } bool STObject::delField(SField::ref field) @@ -1107,67 +1108,63 @@ std::auto_ptr STObject::parseJson(const Json::Value& object, SField::r return std::auto_ptr(new STObject(*name, data)); } -#if 0 +BOOST_AUTO_TEST_SUITE(SerializedObject) -static SOElement testSOElements[2][16] = -{ // field, name, id, type, flags - { - { sfFlags, "Flags", STI_UINT32, SOE_FLAGS, 0 }, - { sfTest1, "Test1", STI_VL, SOE_REQUIRED, 0 }, - { sfTest2, "Test2", STI_HASH256, SOE_IFFLAG, 1 }, - { sfTest3, "Test3", STI_UINT32, SOE_REQUIRED, 0 }, - { sfInvalid, NULL, STI_DONE, SOE_NEVER, -1 } - } -}; - -void STObject::unitTest() +BOOST_AUTO_TEST_CASE( FieldManipulation_test ) { - STObject object1(testSOElements[0], "TestElement1"); + SField sfTestVL(STI_VL, 10, "TestVL", true); + SField sfTestH256(STI_HASH256, 32, "TestH256", true); + SField sfTestU32(STI_UINT32, 15, "TestU32", true); + SField sfTestObject(STI_OBJECT, 9, "TestObject", true); + + std::vector elements; + elements.push_back(new SOElement(sfFlags, SOE_REQUIRED)); + elements.push_back(new SOElement(sfTestVL, SOE_REQUIRED)); + elements.push_back(new SOElement(sfTestH256, SOE_OPTIONAL)); + elements.push_back(new SOElement(sfTestU32, SOE_REQUIRED)); + + STObject object1(elements, sfTestObject); STObject object2(object1); - if (object1.getSerializer() != object2.getSerializer()) throw std::runtime_error("STObject error"); + if (object1.getSerializer() != object2.getSerializer()) BOOST_FAIL("STObject error"); - if (object1.isFieldPresent(sfTest2) || !object1.isFieldPresent(sfTest1)) - throw std::runtime_error("STObject error"); + if (object1.isFieldPresent(sfTestH256) || !object1.isFieldPresent(sfTestVL)) + BOOST_FAIL("STObject error"); - object1.makeFieldPresent(sfTest2); - if (!object1.isFieldPresent(sfTest2)) throw std::runtime_error("STObject Error"); + object1.makeFieldPresent(sfTestH256); + if (!object1.isFieldPresent(sfTestH256)) BOOST_FAIL("STObject Error"); + if (object1.getFieldH256(sfTestH256) != uint256()) BOOST_FAIL("STObject error"); - if ((object1.getFlags() != 1) || (object2.getFlags() != 0)) throw std::runtime_error("STObject error"); - if (object1.getFieldH256(sfTest2) != uint256()) throw std::runtime_error("STObject error"); - - if (object1.getSerializer() == object2.getSerializer()) throw std::runtime_error("STObject error"); - object1.makeFieldAbsent(sfTest2); - if (object1.isFieldPresent(sfTest2)) throw std::runtime_error("STObject error"); - if (object1.getFlags() != 0) throw std::runtime_error("STObject error"); - if (object1.getSerializer() != object2.getSerializer()) throw std::runtime_error("STObject error"); + if (object1.getSerializer() == object2.getSerializer()) BOOST_FAIL("STObject error"); + object1.makeFieldAbsent(sfTestH256); + if (object1.isFieldPresent(sfTestH256)) BOOST_FAIL("STObject error"); + if (object1.getFlags() != 0) BOOST_FAIL("STObject error"); + if (object1.getSerializer() != object2.getSerializer()) BOOST_FAIL("STObject error"); STObject copy(object1); - if (object1.isFieldPresent(sfTest2)) throw std::runtime_error("STObject error"); - if (copy.isFieldPresent(sfTest2)) throw std::runtime_error("STObject error"); - if (object1.getSerializer() != copy.getSerializer()) throw std::runtime_error("STObject error"); - copy.setFieldU32(sfTest3, 1); - if (object1.getSerializer() == copy.getSerializer()) throw std::runtime_error("STObject error"); -#ifdef DEBUG - Log(lsDEBUG) << copy.getJson(0); -#endif + if (object1.isFieldPresent(sfTestH256)) BOOST_FAIL("STObject error"); + if (copy.isFieldPresent(sfTestH256)) BOOST_FAIL("STObject error"); + if (object1.getSerializer() != copy.getSerializer()) BOOST_FAIL("STObject error"); + copy.setFieldU32(sfTestU32, 1); + if (object1.getSerializer() == copy.getSerializer()) BOOST_FAIL("STObject error"); for (int i = 0; i < 1000; i++) { - std::cerr << "tol: i=" << i << std::endl; std::vector j(i, 2); - object1.setFieldVL(sfTest1, j); + + object1.setFieldVL(sfTestVL, j); Serializer s; object1.add(s); SerializerIterator it(s); - STObject object3(testSOElements[0], it, "TestElement3"); - if (object1.getFieldVL(sfTest1) != j) throw std::runtime_error("STObject error"); - if (object3.getFieldVL(sfTest1) != j) throw std::runtime_error("STObject error"); + STObject object3(elements, it, sfTestObject); + + if (object1.getFieldVL(sfTestVL) != j) BOOST_FAIL("STObject error"); + if (object3.getFieldVL(sfTestVL) != j) BOOST_FAIL("STObject error"); } } -#endif +BOOST_AUTO_TEST_SUITE_END(); // vim:ts=4