From d62c6a974e54c040bb2fb0427797ea3d0e146581 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Wed, 18 Jul 2012 17:40:21 -0700 Subject: [PATCH 1/4] Add logging for offer create. --- src/Amount.cpp | 22 ++++++++++++-- src/RPCServer.cpp | 23 +++++++------- src/SerializedTypes.h | 1 + src/TransactionEngine.cpp | 64 +++++++++++++++++++++++++++------------ 4 files changed, 78 insertions(+), 32 deletions(-) diff --git a/src/Amount.cpp b/src/Amount.cpp index ed38f7b3d..1fb63fea4 100644 --- a/src/Amount.cpp +++ b/src/Amount.cpp @@ -743,7 +743,7 @@ uint64 STAmount::getRate(const STAmount& offerOut, const STAmount& offerIn) // Taker gets all taker can pay for with saTakerFunds, limited by saOfferPays and saOfferFunds. // --> saOfferFunds: Limit for saOfferPays -// --> saTakerFunds: Limit for saOfferGets +// --> saTakerFunds: Limit for saOfferGets : How much taker really wants. : Driver // --> saOfferPays: Request : this should be reduced as the offer is fullfilled. // --> saOfferGets: Request : this should be reduced as the offer is fullfilled. // --> saTakerPays: Total : Used to know the approximate ratio of the exchange. @@ -837,6 +837,23 @@ STAmount STAmount::deserialize(SerializerIterator& it) return ret; } +std::string STAmount::getFullText() const +{ + if (mIsNative) + { + return str(boost::format("%s " SYSTEM_CURRENCY_CODE) % getText()); + } + else + { + return str(boost::format("%s %s/%s %dE%d" ) + % getText() + % getHumanCurrency() + % NewcoinAddress::createHumanAccountID(mIssuer) + % getMantissa() + % getExponent()); + } +} + Json::Value STAmount::getJson(int) const { Json::Value elem(Json::objectValue); @@ -850,7 +867,8 @@ Json::Value STAmount::getJson(int) const if (!mIssuer.isZero()) elem["issuer"] = NewcoinAddress::createHumanAccountID(mIssuer); - }else + } + else { elem=getText(); } diff --git a/src/RPCServer.cpp b/src/RPCServer.cpp index 1c410b672..90476e212 100644 --- a/src/RPCServer.cpp +++ b/src/RPCServer.cpp @@ -830,7 +830,8 @@ Json::Value RPCServer::doNicknameSet(const Json::Value& params) return obj; } -// offer_create [passive] +// offer_create [passive] +// *offering* for *wants* Json::Value RPCServer::doOfferCreate(const Json::Value ¶ms) { NewcoinAddress naSeed; @@ -848,22 +849,22 @@ Json::Value RPCServer::doOfferCreate(const Json::Value ¶ms) { return RPCError(rpcSRC_ACT_MALFORMED); } - else if (!saTakerPays.setValue(params[2u].asString(), params[3u].asString())) - { - return RPCError(rpcPAYS_AMT_MALFORMED); - } - else if (!naTakerPaysID.setAccountID(params[4u].asString())) - { - return RPCError(rpcPAYS_ACT_MALFORMED); - } - else if (!saTakerGets.setValue(params[5u].asString(), params[6u].asString())) + else if (!saTakerGets.setValue(params[2u].asString(), params[3u].asString())) { return RPCError(rpcGETS_AMT_MALFORMED); } - else if (!naTakerGetsID.setAccountID(params[7u].asString())) + else if (!naTakerGetsID.setAccountID(params[4u].asString())) { return RPCError(rpcGETS_ACT_MALFORMED); } + else if (!saTakerPays.setValue(params[5u].asString(), params[6u].asString())) + { + return RPCError(rpcPAYS_AMT_MALFORMED); + } + else if (!naTakerPaysID.setAccountID(params[7u].asString())) + { + return RPCError(rpcPAYS_ACT_MALFORMED); + } else if (params.size() == 10 && params[9u].asString() != "passive") { return RPCError(rpcINVALID_PARAMS); diff --git a/src/SerializedTypes.h b/src/SerializedTypes.h index 0cd2eedeb..ee4153291 100644 --- a/src/SerializedTypes.h +++ b/src/SerializedTypes.h @@ -261,6 +261,7 @@ public: SerializedTypeID getSType() const { return STI_AMOUNT; } std::string getText() const; std::string getRaw() const; + std::string getFullText() const; void add(Serializer& s) const; int getExponent() const { return mOffset; } diff --git a/src/TransactionEngine.cpp b/src/TransactionEngine.cpp index e73333bad..2a08807e4 100644 --- a/src/TransactionEngine.cpp +++ b/src/TransactionEngine.cpp @@ -128,7 +128,7 @@ STAmount TransactionEngine::accountHolds(const uint160& uAccountID, const uint16 saAmount = rippleHolds(uAccountID, uCurrency, uIssuerID); Log(lsINFO) << "accountHolds: " - << saAmount.getText() + << saAmount.getFullText() << " : " << STAmount::createHumanCurrency(uCurrency) << "/" @@ -1930,11 +1930,10 @@ TransactionEngineResult TransactionEngine::doInvoice(const SerializedTransaction return tenUNKNOWN; } -// Take as much as possible. Adjusts account balances. +// Take as much as possible. Adjusts account balances. Charges fees on top to taker. // --> uBookBase: The order book to take against. -// --> saTakerPays: What the taker wanted (w/ issuer) +// --> saTakerPays: What the taker offers (w/ issuer) // --> saTakerGets: What the taker wanted (w/ issuer) -// --> saTakerFund: What taker can afford // <-- saTakerPaid: What taker paid not including fees. To reduce an offer. // <-- saTakerGot: What taker got not including fees. To reduce an offer. // <-- terResult: terSUCCESS or terNO_ACCOUNT @@ -2017,6 +2016,12 @@ TransactionEngineResult TransactionEngine::takeOffers( STAmount saOfferPays = sleOffer->getIValueFieldAmount(sfTakerGets); STAmount saOfferGets = sleOffer->getIValueFieldAmount(sfTakerPays); + if (sleOffer->getIFieldPresent(sfGetsIssuer)) + saOfferPays.setIssuer(sleOffer->getIValueFieldAccount(sfGetsIssuer).getAccountID()); + + if (sleOffer->getIFieldPresent(sfPaysIssuer)) + saOfferGets.setIssuer(sleOffer->getIValueFieldAccount(sfPaysIssuer).getAccountID()); + if (sleOffer->getIFieldPresent(sfExpiration) && sleOffer->getIFieldU32(sfExpiration) <= mLedger->getParentCloseTimeNC()) { // Offer is expired. Delete it. @@ -2037,8 +2042,7 @@ TransactionEngineResult TransactionEngine::takeOffers( { // Get offer funds available. - if (sleOffer->getIFieldPresent(sfPaysIssuer)) - saOfferPays.setIssuer(sleOffer->getIValueFieldAccount(sfPaysIssuer).getAccountID()); + Log(lsINFO) << "takeOffers: saOfferPays=" << saOfferPays.getJson(0); STAmount saOfferFunds = accountFunds(uOfferOwnerID, saOfferPays); STAmount saTakerFunds = accountFunds(uTakerAccountID, saTakerPays); @@ -2055,14 +2059,34 @@ TransactionEngineResult TransactionEngine::takeOffers( } else { + STAmount saPay = saTakerPays - saTakerPaid; + if (saTakerFunds < saPay) + saPay = saTakerFunds; STAmount saSubTakerPaid; STAmount saSubTakerGot; + Log(lsINFO) << "takeOffers: applyOffer: saTakerPays: " << saTakerPays.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saTakerPaid: " << saTakerPaid.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saTakerFunds: " << saTakerFunds.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saOfferFunds: " << saOfferFunds.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saPay: " << saPay.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saOfferPays: " << saOfferPays.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saOfferGets: " << saOfferGets.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saTakerPays: " << saTakerPays.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saTakerGets: " << saTakerGets.getFullText(); + bool bOfferDelete = STAmount::applyOffer( - saOfferFunds, saTakerFunds, - saOfferPays, saOfferGets, - saTakerPays, saTakerGets, - saSubTakerPaid, saSubTakerGot); + saOfferFunds, + saPay, // Driver XXX need to account for fees. + saOfferPays, + saOfferGets, + saTakerPays, + saTakerGets, + saSubTakerPaid, + saSubTakerGot); + + Log(lsINFO) << "takeOffers: applyOffer: saSubTakerPaid: " << saSubTakerPaid.getFullText(); + Log(lsINFO) << "takeOffers: applyOffer: saSubTakerGot: " << saSubTakerGot.getFullText(); // Adjust offer if (bOfferDelete) @@ -2115,10 +2139,10 @@ Log(lsWARNING) << "doOfferCreate> " << txn.getJson(0); uint160 uGetsIssuerID = txn.getITFieldAccount(sfGetsIssuer); STAmount saTakerPays = txn.getITFieldAmount(sfTakerPays); saTakerPays.setIssuer(uPaysIssuerID); -Log(lsWARNING) << "doOfferCreate: saTakerPays=" << saTakerPays.getJson(0); +Log(lsWARNING) << "doOfferCreate: saTakerPays=" << saTakerPays.getFullText(); STAmount saTakerGets = txn.getITFieldAmount(sfTakerGets); saTakerGets.setIssuer(uGetsIssuerID); -Log(lsWARNING) << "doOfferCreate: saTakerGets=" << saTakerGets.getJson(0); +Log(lsWARNING) << "doOfferCreate: saTakerGets=" << saTakerGets.getFullText(); uint32 uExpiration = txn.getITFieldU32(sfExpiration); bool bHaveExpiration = txn.getITFieldPresent(sfExpiration); uint32 uSequence = txn.getSequence(); @@ -2218,21 +2242,23 @@ Log(lsWARNING) << "doOfferCreate: saTakerGets=" << saTakerGets.getJson(0); ); Log(lsWARNING) << "doOfferCreate: takeOffers=" << terResult; - Log(lsWARNING) << "doOfferCreate: takeOffers: saOfferPaid=" << saOfferPaid.getText(); - Log(lsWARNING) << "doOfferCreate: takeOffers: saOfferGot=" << saOfferGot.getText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saOfferPaid=" << saOfferPaid.getFullText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saOfferGot=" << saOfferGot.getFullText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerPays=" << saTakerPays.getFullText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerGets=" << saTakerGets.getFullText(); if (terSUCCESS == terResult) { - saTakerGets -= saOfferPaid; // Reduce payout to takers by what srcAccount just paid. saTakerPays -= saOfferGot; // Reduce payin from takers by what offer just got. + saTakerGets -= saOfferPaid; // Reduce payout to takers by what srcAccount just paid. } } - Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerPays=" << saTakerPays.getText(); - Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerGets=" << saTakerGets.getJson(0); - Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerGets=" << NewcoinAddress::createHumanAccountID(saTakerGets.getIssuer()); + Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerPays=" << saTakerPays.getFullText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerGets=" << saTakerGets.getFullText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: saTakerGets=" << NewcoinAddress::createHumanAccountID(saTakerGets.getIssuer()); Log(lsWARNING) << "doOfferCreate: takeOffers: mTxnAccountID=" << NewcoinAddress::createHumanAccountID(mTxnAccountID); - Log(lsWARNING) << "doOfferCreate: takeOffers: funds=" << accountFunds(mTxnAccountID, saTakerGets).getText(); + Log(lsWARNING) << "doOfferCreate: takeOffers: funds=" << accountFunds(mTxnAccountID, saTakerGets).getFullText(); // Log(lsWARNING) << "doOfferCreate: takeOffers: uPaysIssuerID=" << NewcoinAddress::createHumanAccountID(uPaysIssuerID); // Log(lsWARNING) << "doOfferCreate: takeOffers: uGetsIssuerID=" << NewcoinAddress::createHumanAccountID(uGetsIssuerID); From ab61b7ca467a1eceb5a30e7e91581876d072c0cb Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 18 Jul 2012 17:47:15 -0700 Subject: [PATCH 2/4] Let's test divide too, while we're at it. --- src/Amount.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Amount.cpp b/src/Amount.cpp index a141dc7ca..66c91a44e 100644 --- a/src/Amount.cpp +++ b/src/Amount.cpp @@ -1037,10 +1037,22 @@ BOOST_AUTO_TEST_CASE( CustomCurrency_test ) if (STAmount(currency,31,-1).getText() != "3.1") BOOST_FAIL("STAmount fail"); if (STAmount(currency,31,-2).getText() != "0.31") BOOST_FAIL("STAmount fail"); - if (STAmount::multiply(STAmount(currency, 20) , STAmount(3), currency).getText() != "60") + if (STAmount::multiply(STAmount(currency, 20), STAmount(3), currency).getText() != "60") BOOST_FAIL("STAmount multiply fail"); - if (STAmount::multiply(STAmount(currency, 20) , STAmount(3), uint160()).getText() != "60") + if (STAmount::multiply(STAmount(currency, 20), STAmount(3), uint160()).getText() != "60") BOOST_FAIL("STAmount multiply fail"); + if (STAmount::multiply(STAmount(20), STAmount(3), currency).getText() != "60") + BOOST_FAIL("STAmount multiply fail"); + if (STAmount::multiply(STAmount(20), STAmount(3), uint160()).getText() != "60") + BOOST_FAIL("STAmount multiply fail"); + if (STAmount::divide(STAmount(currency, 60) , STAmount(3), currency).getText() != "20") + BOOST_FAIL("STAmount divide fail"); + if (STAmount::divide(STAmount(currency, 60) , STAmount(3), uint160()).getText() != "20") + BOOST_FAIL("STAmount divide fail"); + if (STAmount::divide(STAmount(currency, 60) , STAmount(currency, 3), currency).getText() != "20") + BOOST_FAIL("STAmount divide fail"); + if (STAmount::divide(STAmount(currency, 60) , STAmount(currency, 3), uint160()).getText() != "20") + BOOST_FAIL("STAmount divide fail"); BOOST_TEST_MESSAGE("Amount CC Complete"); } From d1939cc69d76a7e2d713abdc869a73b749d0c1b3 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Wed, 18 Jul 2012 19:11:57 -0700 Subject: [PATCH 3/4] Save changes when sending stamps. --- src/TransactionEngine.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/TransactionEngine.cpp b/src/TransactionEngine.cpp index 2a08807e4..7952a08c6 100644 --- a/src/TransactionEngine.cpp +++ b/src/TransactionEngine.cpp @@ -12,9 +12,9 @@ #include "../json/writer.h" #include "Config.h" +#include "Log.h" #include "TransactionFormats.h" #include "utils.h" -#include "Log.h" // Small for testing, should likely be 32 or 64. #define DIR_NODE_MAX 2 @@ -273,9 +273,26 @@ STAmount TransactionEngine::accountSend(const uint160& uSenderID, const uint160& SLE::pointer sleSender = entryCache(ltACCOUNT_ROOT, Ledger::getAccountRootIndex(uSenderID)); SLE::pointer sleReceiver = entryCache(ltACCOUNT_ROOT, Ledger::getAccountRootIndex(uReceiverID)); + Log(lsINFO) << str(boost::format("accountSend> %s (%s) -> %s (%s) : %s") + % NewcoinAddress::createHumanAccountID(uSenderID) + % (sleSender->getIValueFieldAmount(sfBalance)).getFullText() + % NewcoinAddress::createHumanAccountID(uReceiverID) + % (sleReceiver->getIValueFieldAmount(sfBalance)).getFullText() + % saAmount.getFullText()); + sleSender->setIFieldAmount(sfBalance, sleSender->getIValueFieldAmount(sfBalance) - saAmount); sleReceiver->setIFieldAmount(sfBalance, sleReceiver->getIValueFieldAmount(sfBalance) + saAmount); + Log(lsINFO) << str(boost::format("accountSend< %s (%s) -> %s (%s) : %s") + % NewcoinAddress::createHumanAccountID(uSenderID) + % (sleSender->getIValueFieldAmount(sfBalance)).getFullText() + % NewcoinAddress::createHumanAccountID(uReceiverID) + % (sleReceiver->getIValueFieldAmount(sfBalance)).getFullText() + % saAmount.getFullText()); + + entryModify(sleSender); + entryModify(sleReceiver); + saActualCost = saAmount; } else From 7236eed852325cbdbd050d99df8192c3ed510da7 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Wed, 18 Jul 2012 19:12:27 -0700 Subject: [PATCH 4/4] Fixes for applyOffer. --- src/Amount.cpp | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Amount.cpp b/src/Amount.cpp index e58d71db5..ae5f53506 100644 --- a/src/Amount.cpp +++ b/src/Amount.cpp @@ -6,6 +6,7 @@ #include #include "Config.h" +#include "Log.h" #include "SerializedTypes.h" #include "utils.h" @@ -766,26 +767,39 @@ bool STAmount::applyOffer( // Amount offer can pay out, limited by offer and funds. STAmount saOfferPaysAvailable = saOfferFunds < saOfferPays ? saOfferFunds : saOfferPays; - // Amount offer needs to get to be complete, limited by offer funds. + // Amount offer can get in proportion, limited by offer funds. STAmount saOfferGetsAvailable = saOfferFunds == saOfferPays ? saOfferGets // Offer was fully funded, avoid shenanigans. : divide(multiply(saTakerPays, saOfferPaysAvailable, uint160(1)), saTakerGets, saOfferGets.getCurrency()); - if (saTakerFunds >= saOfferGetsAvailable) + if (saOfferGets == saOfferGetsAvailable && saTakerFunds >= saOfferGets) + { + // Taker gets all of offer available. + saTakerPaid = saOfferGets; // Taker paid what offer could get. + saTakerGot = saOfferPays; // Taker got what offer could pay. + + Log(lsINFO) << "applyOffer: took all outright"; + } + else if (saTakerFunds >= saOfferGetsAvailable) { // Taker gets all of offer available. saTakerPaid = saOfferGetsAvailable; // Taker paid what offer could get. saTakerGot = saOfferPaysAvailable; // Taker got what offer could pay. - return true; // No left over offer. + Log(lsINFO) << "applyOffer: took all available"; + } + else + { + // Taker only get's a portion of offer. + saTakerPaid = saTakerFunds; // Taker paid all he had. + saTakerGot = divide(multiply(saTakerFunds, saOfferPaysAvailable, uint160(1)), saOfferGetsAvailable, saOfferPays.getCurrency()); + + Log(lsINFO) << "applyOffer: saTakerGot=" << saTakerGot.getFullText(); + Log(lsINFO) << "applyOffer: saOfferPaysAvailable=" << saOfferPaysAvailable.getFullText(); } - // Taker only get's a portion of offer. - saTakerPaid = saTakerFunds; // Taker paid all he had. - saTakerGot = divide(multiply(saTakerFunds, saOfferPaysAvailable, uint160(1)), saOfferGetsAvailable, saOfferPays.getCurrency()); - - return saTakerGot >= saOfferPaysAvailable; + return saTakerGot >= saOfferPays; } STAmount STAmount::getPay(const STAmount& offerOut, const STAmount& offerIn, const STAmount& needed)