From 049f92e8829be6deacd9d6b113a4429ce12f741a Mon Sep 17 00:00:00 2001 From: Stefan Thomas Date: Tue, 12 Mar 2013 12:35:38 +0100 Subject: [PATCH] Clean up orderbook-related variable naming. Also adds the ability to unsubscribe from both sides of an orderbook simultaneously. Adds better error handling. --- src/cpp/ripple/NetworkOPs.cpp | 10 +- src/cpp/ripple/NetworkOPs.h | 8 +- src/cpp/ripple/OrderBookDB.cpp | 30 +++--- src/cpp/ripple/OrderBookDB.h | 14 +-- src/cpp/ripple/RPCHandler.cpp | 166 +++++++++++++++++++++++++++------ 5 files changed, 169 insertions(+), 59 deletions(-) diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index 44bee343e..55d65dc96 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -1598,21 +1598,21 @@ void NetworkOPs::unsubAccount(uint64 uSeq, const boost::unordered_setgetOrderBookDB().makeBookListeners(currencyIn, currencyOut, issuerIn, issuerOut); + theApp->getOrderBookDB().makeBookListeners(currencyPays, currencyGets, issuerPays, issuerGets); if (listeners) listeners->addSubscriber(isrListener); return true; } bool NetworkOPs::unsubBook(uint64 uSeq, - const uint160& currencyIn, const uint160& currencyOut, const uint160& issuerIn, const uint160& issuerOut) + const uint160& currencyPays, const uint160& currencyGets, const uint160& issuerPays, const uint160& issuerGets) { BookListeners::pointer listeners = - theApp->getOrderBookDB().getBookListeners(currencyIn, currencyOut, issuerIn, issuerOut); + theApp->getOrderBookDB().getBookListeners(currencyPays, currencyGets, issuerPays, issuerGets); if (listeners) listeners->removeSubscriber(uSeq); return true; diff --git a/src/cpp/ripple/NetworkOPs.h b/src/cpp/ripple/NetworkOPs.h index af7a30218..1fa99064b 100644 --- a/src/cpp/ripple/NetworkOPs.h +++ b/src/cpp/ripple/NetworkOPs.h @@ -322,10 +322,10 @@ public: bool subServer(InfoSub::ref ispListener, Json::Value& jvResult); bool unsubServer(uint64 uListener); - bool subBook(InfoSub::ref ispListener, const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut); - bool unsubBook(uint64 uListener, const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut); + bool subBook(InfoSub::ref ispListener, const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets); + bool unsubBook(uint64 uListener, const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets); bool subTransactions(InfoSub::ref ispListener); bool unsubTransactions(uint64 uListener); diff --git a/src/cpp/ripple/OrderBookDB.cpp b/src/cpp/ripple/OrderBookDB.cpp index 8cdaa42bd..8e76b7579 100644 --- a/src/cpp/ripple/OrderBookDB.cpp +++ b/src/cpp/ripple/OrderBookDB.cpp @@ -93,40 +93,40 @@ void OrderBookDB::getBooks(const uint160& issuerID, const uint160& currencyID, s } } -BookListeners::pointer OrderBookDB::makeBookListeners(const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut) +BookListeners::pointer OrderBookDB::makeBookListeners(const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets) { boost::recursive_mutex::scoped_lock sl(mLock); - BookListeners::pointer ret = getBookListeners(currencyIn, currencyOut, issuerIn, issuerOut); + BookListeners::pointer ret = getBookListeners(currencyPays, currencyGets, issuerPays, issuerGets); if (!ret) { ret = boost::make_shared(); - mListeners[issuerIn][issuerOut][currencyIn][currencyOut] = ret; + mListeners[issuerPays][issuerGets][currencyPays][currencyGets] = ret; } return ret; } -BookListeners::pointer OrderBookDB::getBookListeners(const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut) +BookListeners::pointer OrderBookDB::getBookListeners(const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets) { BookListeners::pointer ret; boost::recursive_mutex::scoped_lock sl(mLock); std::map > > >::iterator - it0 = mListeners.find(issuerIn); + it0 = mListeners.find(issuerPays); if(it0 == mListeners.end()) return ret; std::map > >::iterator - it1 = (*it0).second.find(issuerOut); + it1 = (*it0).second.find(issuerGets); if(it1 == (*it0).second.end()) return ret; - std::map >::iterator it2 = (*it1).second.find(currencyIn); + std::map >::iterator it2 = (*it1).second.find(currencyPays); if(it2 == (*it1).second.end()) return ret; - std::map::iterator it3 = (*it2).second.find(currencyOut); + std::map::iterator it3 = (*it2).second.find(currencyGets); if(it3 == (*it2).second.end()) return ret; @@ -216,16 +216,16 @@ void OrderBookDB::processTxn(Ledger::ref ledger, const ALTransaction& alTx, Json if (data) { STAmount takerGets = data->getFieldAmount(sfTakerGets); - uint160 currencyOut = takerGets.getCurrency(); - uint160 issuerOut = takerGets.getIssuer(); + uint160 currencyGets = takerGets.getCurrency(); + uint160 issuerGets = takerGets.getIssuer(); STAmount takerPays = data->getFieldAmount(sfTakerPays); - uint160 currencyIn = takerPays.getCurrency(); - uint160 issuerIn = takerPays.getIssuer(); + uint160 currencyPays = takerPays.getCurrency(); + uint160 issuerPays = takerPays.getIssuer(); // determine the OrderBook BookListeners::pointer book = - getBookListeners(currencyIn, currencyOut, issuerIn, issuerOut); + getBookListeners(currencyPays, currencyGets, issuerPays, issuerGets); if (book) book->publish(jvObj); } diff --git a/src/cpp/ripple/OrderBookDB.h b/src/cpp/ripple/OrderBookDB.h index b5a95e43b..15ebd24d0 100644 --- a/src/cpp/ripple/OrderBookDB.h +++ b/src/cpp/ripple/OrderBookDB.h @@ -35,8 +35,8 @@ class OrderBookDB boost::unordered_map > mIssuerMap; //std::vector mAllOrderBooks; - // issuerIn, issuerOut, currencyIn, currencyOut - std::map > > > mListeners; + // issuerPays, issuerGets, currencyPays, currencyGets + std::map > > > mListeners; uint32 mSeq; boost::recursive_mutex mLock; @@ -56,13 +56,13 @@ public: void getBooks(const uint160& issuerID, const uint160& currencyID, std::vector& bookRet); // returns the best rate we can find - float getPrice(uint160& currencyIn,uint160& currencyOut); + float getPrice(uint160& currencyPays,uint160& currencyGets); - BookListeners::pointer getBookListeners(const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut); - BookListeners::pointer makeBookListeners(const uint160& currencyIn, const uint160& currencyOut, - const uint160& issuerIn, const uint160& issuerOut); + BookListeners::pointer getBookListeners(const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets); + BookListeners::pointer makeBookListeners(const uint160& currencyPays, const uint160& currencyGets, + const uint160& issuerPays, const uint160& issuerGets); // see if this txn effects any orderbook void processTxn(Ledger::ref ledger, const ALTransaction& alTx, Json::Value& jvObj); diff --git a/src/cpp/ripple/RPCHandler.cpp b/src/cpp/ripple/RPCHandler.cpp index 870423541..78a39dc14 100644 --- a/src/cpp/ripple/RPCHandler.cpp +++ b/src/cpp/ripple/RPCHandler.cpp @@ -2754,34 +2754,90 @@ Json::Value RPCHandler::doSubscribe(Json::Value jvRequest, int& cost) { for (Json::Value::iterator it = jvRequest["books"].begin(); it != jvRequest["books"].end(); it++) { - uint160 currencyOut; - STAmount::currencyFromString(currencyOut,(*it)["CurrencyOut"].asString()); - uint160 issuerOut,issuerIn; - if(currencyOut.isNonZero()) - STAmount::issuerFromString(issuerOut,(*it)["IssuerOut"].asString()); - uint160 currencyIn; - STAmount::currencyFromString(currencyIn,(*it)["CurrencyIn"].asString()); - if(currencyIn.isNonZero()) - STAmount::issuerFromString(issuerIn,(*it)["IssuerIn"].asString()); + uint160 uTakerPaysCurrencyID; + uint160 uTakerPaysIssuerID; + Json::Value jvTakerPays = (*it)["taker_pays"]; - bool bothSides=false; - if((*it).isMember("BothSides") && (*it)["BothSides"].asBool()) bothSides=true; + // Parse mandatory currency. + if (!jvTakerPays.isMember("currency") + || !STAmount::currencyFromString(uTakerPaysCurrencyID, jvTakerPays["currency"].asString())) + { + cLog(lsINFO) << "Bad taker_pays currency."; - mNetOps->subBook(ispSub, currencyIn, currencyOut, issuerIn, issuerOut); - if(bothSides) mNetOps->subBook(ispSub, currencyOut, currencyIn, issuerOut, issuerIn); - if((*it)["StateNow"].asBool()) + return rpcError(rpcSRC_CUR_MALFORMED); + } + // Parse optional issuer. + else if (((jvTakerPays.isMember("issuer")) + && (!jvTakerPays["issuer"].isString() + || !STAmount::issuerFromString(uTakerPaysIssuerID, jvTakerPays["issuer"].asString()))) + // Don't allow illegal issuers. + || (!uTakerPaysCurrencyID != !uTakerPaysIssuerID) + || ACCOUNT_ONE == uTakerPaysIssuerID) + { + cLog(lsINFO) << "Bad taker_pays issuer."; + + return rpcError(rpcSRC_ISR_MALFORMED); + } + + uint160 uTakerGetsCurrencyID; + uint160 uTakerGetsIssuerID; + Json::Value jvTakerGets = (*it)["taker_gets"]; + + // Parse mandatory currency. + if (!jvTakerGets.isMember("currency") + || !STAmount::currencyFromString(uTakerGetsCurrencyID, jvTakerGets["currency"].asString())) + { + cLog(lsINFO) << "Bad taker_pays currency."; + + return rpcError(rpcSRC_CUR_MALFORMED); + } + // Parse optional issuer. + else if (((jvTakerGets.isMember("issuer")) + && (!jvTakerGets["issuer"].isString() + || !STAmount::issuerFromString(uTakerGetsIssuerID, jvTakerGets["issuer"].asString()))) + // Don't allow illegal issuers. + || (!uTakerGetsCurrencyID != !uTakerGetsIssuerID) + || ACCOUNT_ONE == uTakerGetsIssuerID) + { + cLog(lsINFO) << "Bad taker_gets issuer."; + + return rpcError(rpcDST_ISR_MALFORMED); + } + + if (uTakerPaysCurrencyID == uTakerGetsCurrencyID + && uTakerPaysIssuerID == uTakerGetsIssuerID) + { + cLog(lsINFO) << "taker_gets same as taker_pays."; + + return rpcError(rpcBAD_MARKET); + } + + RippleAddress raTakerID; + + if (!(*it).isMember("taker")) + { + raTakerID.setAccountID(ACCOUNT_ONE); + } + else if (!raTakerID.setAccountID((*it)["taker"].asString())) + { + return rpcError(rpcBAD_ISSUER); + } + + bool bothSides = (*it)["both_sides"].asBool(); + + mNetOps->subBook(ispSub, uTakerPaysCurrencyID, uTakerGetsCurrencyID, uTakerPaysIssuerID, uTakerGetsIssuerID); + if (bothSides) mNetOps->subBook(ispSub, uTakerGetsCurrencyID, uTakerPaysCurrencyID, uTakerGetsIssuerID, uTakerPaysIssuerID); + if ((*it)["state_now"].asBool()) { Ledger::pointer ledger= theApp->getLedgerMaster().getClosedLedger(); - RippleAddress raTakerID; - raTakerID.setAccountID(ACCOUNT_ONE); const Json::Value jvMarker = Json::Value(Json::nullValue); - mNetOps->getBookPage(ledger, currencyOut, issuerOut, currencyIn, issuerIn, raTakerID.getAccountID(), false, 0, jvMarker, jvResult); - if(bothSides) + mNetOps->getBookPage(ledger, uTakerPaysCurrencyID, uTakerPaysIssuerID, uTakerGetsCurrencyID, uTakerGetsIssuerID, raTakerID.getAccountID(), false, 0, jvMarker, jvResult); + if (bothSides) { Json::Value tempJson(Json::objectValue); - if(jvResult.isMember("offers")) jvResult["bids"]=jvResult["offers"]; - mNetOps->getBookPage(ledger, currencyIn, issuerIn, currencyOut, issuerOut, raTakerID.getAccountID(), false, 0, jvMarker, tempJson); - if(tempJson.isMember("offers")) jvResult["asks"]=tempJson["offers"]; + if (jvResult.isMember("offers")) jvResult["bids"]=jvResult["offers"]; + mNetOps->getBookPage(ledger, uTakerGetsCurrencyID, uTakerGetsIssuerID, uTakerPaysCurrencyID, uTakerPaysIssuerID, raTakerID.getAccountID(), false, 0, jvMarker, tempJson); + if (tempJson.isMember("offers")) jvResult["asks"]=tempJson["offers"]; } } } @@ -2886,14 +2942,68 @@ Json::Value RPCHandler::doUnsubscribe(Json::Value jvRequest, int& cost) { for (Json::Value::iterator it = jvRequest["books"].begin(); it != jvRequest["books"].end(); it++) { - uint160 currencyOut; - STAmount::issuerFromString(currencyOut,(*it)["CurrencyOut"].asString()); - uint160 issuerOut = RippleAddress::createNodePublic( (*it)["IssuerOut"].asString() ).getAccountID(); - uint160 currencyIn; - STAmount::issuerFromString(currencyOut,(*it)["CurrencyIn"].asString()); - uint160 issuerIn = RippleAddress::createNodePublic( (*it)["IssuerIn"].asString() ).getAccountID(); + uint160 uTakerPaysCurrencyID; + uint160 uTakerPaysIssuerID; + Json::Value jvTakerPays = (*it)["taker_pays"]; - mNetOps->unsubBook(ispSub->getSeq(), currencyIn, currencyOut, issuerIn, issuerOut); + // Parse mandatory currency. + if (!jvTakerPays.isMember("currency") + || !STAmount::currencyFromString(uTakerPaysCurrencyID, jvTakerPays["currency"].asString())) + { + cLog(lsINFO) << "Bad taker_pays currency."; + + return rpcError(rpcSRC_CUR_MALFORMED); + } + // Parse optional issuer. + else if (((jvTakerPays.isMember("issuer")) + && (!jvTakerPays["issuer"].isString() + || !STAmount::issuerFromString(uTakerPaysIssuerID, jvTakerPays["issuer"].asString()))) + // Don't allow illegal issuers. + || (!uTakerPaysCurrencyID != !uTakerPaysIssuerID) + || ACCOUNT_ONE == uTakerPaysIssuerID) + { + cLog(lsINFO) << "Bad taker_pays issuer."; + + return rpcError(rpcSRC_ISR_MALFORMED); + } + + uint160 uTakerGetsCurrencyID; + uint160 uTakerGetsIssuerID; + Json::Value jvTakerGets = (*it)["taker_gets"]; + + // Parse mandatory currency. + if (!jvTakerGets.isMember("currency") + || !STAmount::currencyFromString(uTakerGetsCurrencyID, jvTakerGets["currency"].asString())) + { + cLog(lsINFO) << "Bad taker_pays currency."; + + return rpcError(rpcSRC_CUR_MALFORMED); + } + // Parse optional issuer. + else if (((jvTakerGets.isMember("issuer")) + && (!jvTakerGets["issuer"].isString() + || !STAmount::issuerFromString(uTakerGetsIssuerID, jvTakerGets["issuer"].asString()))) + // Don't allow illegal issuers. + || (!uTakerGetsCurrencyID != !uTakerGetsIssuerID) + || ACCOUNT_ONE == uTakerGetsIssuerID) + { + cLog(lsINFO) << "Bad taker_gets issuer."; + + return rpcError(rpcDST_ISR_MALFORMED); + } + + if (uTakerPaysCurrencyID == uTakerGetsCurrencyID + && uTakerPaysIssuerID == uTakerGetsIssuerID) + { + cLog(lsINFO) << "taker_gets same as taker_pays."; + + return rpcError(rpcBAD_MARKET); + } + + bool bothSides = (*it)["both_sides"].asBool(); + + mNetOps->unsubBook(ispSub->getSeq(), uTakerPaysCurrencyID, uTakerGetsCurrencyID, uTakerPaysIssuerID, uTakerGetsIssuerID); + if (bothSides) mNetOps->unsubBook(ispSub->getSeq(), uTakerGetsCurrencyID, uTakerPaysCurrencyID, uTakerGetsIssuerID, uTakerPaysIssuerID); } }