From ca0ab6c01de1b7dccd1cd0173c12dfab06e8cd79 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Thu, 8 Nov 2012 15:17:46 -0800 Subject: [PATCH 1/3] Fixes for rippling through offers. --- src/cpp/ripple/RippleCalc.cpp | 68 ++++++++++++++++++++------------ src/cpp/ripple/SerializedTypes.h | 2 + 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/cpp/ripple/RippleCalc.cpp b/src/cpp/ripple/RippleCalc.cpp index efd7cf0202..f5255a66da 100644 --- a/src/cpp/ripple/RippleCalc.cpp +++ b/src/cpp/ripple/RippleCalc.cpp @@ -42,7 +42,7 @@ TER RippleCalc::calcNodeAdvance( const uint160& uCurIssuerID = pnCur.uIssuerID; uint256& uDirectTip = pnCur.uDirectTip; - uint256 uDirectEnd = pnCur.uDirectEnd; + uint256& uDirectEnd = pnCur.uDirectEnd; bool& bDirectAdvance = pnCur.bDirectAdvance; SLE::pointer& sleDirectDir = pnCur.sleDirectDir; STAmount& saOfrRate = pnCur.saOfrRate; @@ -63,12 +63,13 @@ TER RippleCalc::calcNodeAdvance( { bool bDirectDirDirty = false; - if (!uDirectEnd) + if (!uDirectTip) { // Need to initialize current node. uDirectTip = Ledger::getBookBase(uPrvCurrencyID, uPrvIssuerID, uCurCurrencyID, uCurIssuerID); uDirectEnd = Ledger::getQualityNext(uDirectTip); + sleDirectDir = lesActive.entryCache(ltDIR_NODE, uDirectTip); bDirectAdvance = !sleDirectDir; bDirectDirDirty = true; @@ -100,7 +101,7 @@ TER RippleCalc::calcNodeAdvance( else { // No more offers. Should be done rather than fall off end of book. - cLog(lsINFO) << "calcNodeAdvance: Unreachable: Fell off end of order book."; + cLog(lsWARNING) << "calcNodeAdvance: Unreachable: Fell off end of order book."; assert(false); terResult = tefEXCEPTION; @@ -148,7 +149,7 @@ TER RippleCalc::calcNodeAdvance( } else if (!bReverse) { - cLog(lsINFO) << boost::str(boost::format("calcNodeAdvance: unreachable: ran out of offers")); + cLog(lsWARNING) << boost::str(boost::format("calcNodeAdvance: unreachable: ran out of offers")); assert(false); // Can't run out of offers in forward direction. terResult = tefEXCEPTION; } @@ -175,6 +176,8 @@ TER RippleCalc::calcNodeAdvance( // Allowed to access source from this node? // XXX This can get called multiple times for same source in a row, caching result would be nice. + // XXX Going forward could we fund something with a worse quality which was previously skipped? Might need to check + // quality. curIssuerNodeConstIterator itForward = pspCur->umForward.find(asLine); const bool bFoundForward = itForward != pspCur->umForward.end(); @@ -263,10 +266,9 @@ TER RippleCalc::calcNodeAdvance( return terResult; } -// Between offer nodes, the fee charged may vary. Therefore, process one inbound offer at a time. -// Propagate the inbound offer's requirements to the previous node. The previous node adjusts the amount output and the -// amount spent on fees. -// Continue process till request is satisified while we the rate does not increase past the initial rate. +// Between offer nodes, the fee charged may vary. Therefore, process one inbound offer at a time. Propagate the inbound offer's +// requirements to the previous node. The previous node adjusts the amount output and the amount spent on fees. Continue process +// till request is satisified while we the rate does not increase past the initial rate. TER RippleCalc::calcNodeDeliverRev( const unsigned int uIndex, // 0 < uIndex < uLast PathState::ref pspCur, @@ -284,8 +286,13 @@ TER RippleCalc::calcNodeDeliverRev( const uint160& uPrvAccountID = pnPrv.uAccountID; const STAmount& saTransferRate = pnCur.saTransferRate; - STAmount& saPrvDlvReq = pnPrv.saRevDeliver; // To be adjusted. + STAmount& saPrvDlvReq = pnPrv.saRevDeliver; // To be set. + uint256& uDirectTip = pnCur.uDirectTip; + + uDirectTip = 0; // Restart book searching. + + saPrvDlvReq.zero(pnPrv.uCurrencyID, pnPrv.uIssuerID); saOutAct.zero(saOutReq); while (saOutAct != saOutReq) // Did not deliver limit. @@ -310,8 +317,8 @@ TER RippleCalc::calcNodeDeliverRev( } const STAmount saOutFeeRate = uOfrOwnerID == uCurIssuerID || uOutAccountID == uCurIssuerID // Issuer receiving or sending. - ? saOne // No fee. - : saTransferRate; // Transfer rate of issuer. + ? saOne // No fee. + : saTransferRate; // Transfer rate of issuer. cLog(lsINFO) << boost::str(boost::format("calcNodeDeliverRev: uOfrOwnerID=%s uOutAccountID=%s uCurIssuerID=%s saTransferRate=%s saOutFeeRate=%s") % RippleAddress::createHumanAccountID(uOfrOwnerID) % RippleAddress::createHumanAccountID(uOutAccountID) @@ -328,19 +335,18 @@ TER RippleCalc::calcNodeDeliverRev( % saRateMax % saOutFeeRate); } - else if (saRateMax < saOutFeeRate) + else if (saOutFeeRate > saRateMax) { // Offer exceeds initial rate. cLog(lsINFO) << boost::str(boost::format("calcNodeDeliverRev: Offer exceeds initial rate: saRateMax=%s saOutFeeRate=%s") % saRateMax % saOutFeeRate); - nothing(); - break; + break; // Done. Don't bother looking for smaller saTransferRates. } else if (saOutFeeRate < saRateMax) { - // Reducing rate. + // Reducing rate. Additional offers will only considered for this increment if they are at least this good. saRateMax = saOutFeeRate; @@ -348,7 +354,10 @@ TER RippleCalc::calcNodeDeliverRev( % saRateMax); } + // Amount that goes to the taker. STAmount saOutPass = std::min(std::min(saOfferFunds, saTakerGets), saOutReq-saOutAct); // Offer maximum out - assuming no out fees. + // Amount charged to the offer owner. + // The fee goes to issuer. The fee is paid by offer owner and not passed as a cost to taker. STAmount saOutPlusFees = STAmount::multiply(saOutPass, saOutFeeRate); // Offer out with fees. cLog(lsINFO) << boost::str(boost::format("calcNodeDeliverRev: saOutReq=%s saOutAct=%s saTakerGets=%s saOutPass=%s saOutPlusFees=%s saOfferFunds=%s") @@ -372,7 +381,7 @@ TER RippleCalc::calcNodeDeliverRev( % saOfferFunds); } - // Compute portion of input needed to cover output. + // Compute portion of input needed to cover actual output. STAmount saInPassReq = STAmount::multiply(saOutPass, saOfrRate, saTakerPays); STAmount saInPassAct; @@ -387,6 +396,7 @@ TER RippleCalc::calcNodeDeliverRev( if (!!uPrvAccountID) { // account --> OFFER --> ? + // Due to node expansion, previous is guaranteed to be the issuer. // Previous is the issuer and receiver is an offer, so no fee or quality. // Previous is the issuer and has unlimited funds. // Offer owner is obtaining IOUs via an offer, so credit line limits are ignored. @@ -400,6 +410,7 @@ TER RippleCalc::calcNodeDeliverRev( else { // offer --> OFFER --> ? + // Chain and compute the previous offer now. terResult = calcNodeDeliverRev( uIndex-1, @@ -430,7 +441,10 @@ TER RippleCalc::calcNodeDeliverRev( // Funds were spent. bFundsDirty = true; - // Deduct output, don't actually need to send. + // Want to deduct output to limit calculations while computing reverse. Don't actually need to send. + // Sending could be complicated: could fund a previous offer not yet visited. + // However, these deductions and adjustments are tenative. + // Must reset balances when going forward to perform actual transfers. lesActive.accountSend(uOfrOwnerID, uCurIssuerID, saOutPass); // Adjust offer @@ -482,8 +496,12 @@ TER RippleCalc::calcNodeDeliverFwd( STAmount& saCurDeliverAct = pnCur.saFwdDeliver; - saInAct = 0; - saInFees = 0; + uint256& uDirectTip = pnCur.uDirectTip; + + uDirectTip = 0; // Restart book searching. + + saInAct.zero(saInFunds); + saInFees.zero(saInFunds); while (tesSUCCESS == terResult && saInAct != saInReq // Did not deliver limit. @@ -503,10 +521,9 @@ TER RippleCalc::calcNodeDeliverFwd( STAmount& saTakerPays = pnCur.saTakerPays; STAmount& saTakerGets = pnCur.saTakerGets; - - const STAmount saInFeeRate = uInAccountID == uPrvIssuerID || uOfrOwnerID == uPrvIssuerID // Issuer receiving or sending. - ? saOne // No fee. - : saTransferRate; // Transfer rate of issuer. + const STAmount saInFeeRate = uInAccountID == uPrvIssuerID || uOfrOwnerID == uPrvIssuerID // Issuer receiving or sending. + ? saOne // No fee. + : saTransferRate; // Transfer rate of issuer. // // First calculate assuming no output fees. @@ -519,8 +536,8 @@ TER RippleCalc::calcNodeDeliverFwd( STAmount saInPassAct = STAmount::divide(saInSum, saInFeeRate); // In without fees. STAmount saOutPassMax = STAmount::divide(saInPassAct, saOfrRate, saOutFunded); // Out. - STAmount saInPassFees; - STAmount saOutPassAct; + STAmount saInPassFees(saInFunds.getCurrency(), saInFunds.getIssuer()); + STAmount saOutPassAct(saOfferFunds.getCurrency(), saOfferFunds.getIssuer()); cLog(lsINFO) << boost::str(boost::format("calcNodeDeliverFwd: saOutFunded=%s saInFunded=%s saInTotal=%s saInSum=%s saInPassAct=%s saOutPassMax=%s") % saOutFunded @@ -1483,6 +1500,7 @@ TER PathState::pushImply( } // Append a node and insert before it any implied nodes. +// Offers may go back to back. // <-- terResult: tesSUCCESS, temBAD_PATH, terNO_LINE, tepPATH_DRY TER PathState::pushNode( const int iType, diff --git a/src/cpp/ripple/SerializedTypes.h b/src/cpp/ripple/SerializedTypes.h index 4151379c52..7b1e8c0cf5 100644 --- a/src/cpp/ripple/SerializedTypes.h +++ b/src/cpp/ripple/SerializedTypes.h @@ -296,6 +296,8 @@ public: // Zero while copying currency and issuer. STAmount* zero(const STAmount& saTmpl) { mCurrency = saTmpl.mCurrency; mIssuer = saTmpl.mIssuer; mIsNative = saTmpl.mIsNative; return zero(); } + STAmount* zero(const uint160& uCurrencyID, const uint160& uIssuerID) + { mCurrency = uCurrencyID; mIssuer = uIssuerID; mIsNative = !uCurrencyID; return zero(); } int compare(const STAmount&) const; From 054a16b1055942a32162999656a1c5ef37237717 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Thu, 8 Nov 2012 15:18:05 -0800 Subject: [PATCH 2/3] UT: Add test for currency conversion. --- test/offer-test.js | 69 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/offer-test.js b/test/offer-test.js index 10ec4444bf..eb9e9dec56 100644 --- a/test/offer-test.js +++ b/test/offer-test.js @@ -333,5 +333,74 @@ buster.testCase("Offer tests", { done(); }); }, + + "ripple currency conversion" : + function (done) { + var self = this; + + // self.remote.set_trace(); + + async.waterfall([ + function (callback) { + self.what = "Create accounts."; + + testutils.create_accounts(self.remote, "root", "10000", ["alice", "bob", "mtgox"], callback); + }, + function (callback) { + self.what = "Set limits."; + + testutils.credit_limits(self.remote, + { + "alice" : "100/USD/mtgox", + "bob" : "1000/USD/mtgox" + }, + callback); + }, + function (callback) { + self.what = "Distribute funds."; + + testutils.payments(self.remote, + { + "mtgox" : "100/USD/alice" + }, + callback); + }, + function (callback) { + self.remote.transaction() + .offer_create("bob", "100/USD/mtgox", "500") + .on('proposed', function (m) { + // console.log("PROPOSED: offer_create: %s", JSON.stringify(m)); + callback(m.result != 'tesSUCCESS'); + }) + .submit(); + }, + function (callback) { + self.what = "Alice converts USD to XRC."; + + self.remote.transaction() + .payment("alice", "alice", "500") + .send_max("100/USD/mtgox") + .on('proposed', function (m) { + // console.log("proposed: %s", JSON.stringify(m)); + + callback(m.result != 'tesSUCCESS'); + }) + .submit(); + }, + function (callback) { + self.what = "Verify balances."; + + testutils.verify_balances(self.remote, + { + "alice" : [ "0/USD/mtgox", "500" ], + "bob" : "100/USD/mtgox", + }, + callback); + }, + ], function (error) { + buster.refute(error, self.what); + done(); + }); + }, }); // vim:sw=2:sts=2:ts=8 From 8f83fb9d672179e4229120220f8dd91f7a90576f Mon Sep 17 00:00:00 2001 From: Stefan Thomas Date: Thu, 8 Nov 2012 15:27:53 -0800 Subject: [PATCH 3/3] webpack.js: Added -o parameter to specify output directory. --- webpack.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/webpack.js b/webpack.js index fe21a57d83..27fa4f085f 100644 --- a/webpack.js +++ b/webpack.js @@ -5,12 +5,18 @@ var extend = require("extend"); var programPath = __dirname + "/src/js/remote.js"; -var watch = false; -process.argv.forEach(function (arg) { +var cfg = { + watch: false, + outputDir: __dirname + "/build" +}; +for (var i = 0, l = process.argv.length; i < l; i++) { + var arg = process.argv[i]; if (arg === '-w' || arg === '--watch') { - watch = true; + cfg.watch = true; + } else if (arg === '-o') { + cfg.outputDir = process.argv[++i]; } -}); +}; var builds = [{ filename: 'ripple-'+pkg.version+'.js', @@ -27,12 +33,12 @@ var defaultOpts = { libary: 'ripple', // However, it's fixed in webpack 0.8, so we include the correct spelling too: library: 'ripple', - watch: watch + watch: cfg.watch }; function build(opts) { var opts = extend({}, defaultOpts, opts); - opts.output = __dirname + "/build/"+opts.filename; + opts.output = cfg.outputDir + "/"+opts.filename; return function (callback) { var filename = opts.filename; webpack(programPath, opts, function (err, result) { @@ -44,7 +50,7 @@ function build(opts) { } } -if (!watch) { +if (!cfg.watch) { console.log('Compiling Ripple JavaScript...'); async.series(builds.map(build), function (err) { if (err) {