From 7c595bf23bc26100237248fdfa636ec18a47f307 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Sat, 3 Nov 2012 04:16:48 -0700 Subject: [PATCH] Fix ripple bugs. - Default send max inherits source account as issuer. - Add end point implications for node expansion. - Fix rippling through accounts. --- src/RippleCalc.cpp | 146 +++++++++++++++++++++++++------------- src/SerializedTypes.h | 2 +- src/TransactionAction.cpp | 6 +- 3 files changed, 101 insertions(+), 53 deletions(-) diff --git a/src/RippleCalc.cpp b/src/RippleCalc.cpp index a4f3774179..2e78779a3e 100644 --- a/src/RippleCalc.cpp +++ b/src/RippleCalc.cpp @@ -799,7 +799,11 @@ void RippleCalc::calcNodeRipple( } // Calculate saPrvRedeemReq, saPrvIssueReq, saPrvDeliver from saCur... -// No account adjustments in reverse as we don't know how much is going to actually be pushed through yet. +// Based on required deliverable, propagate redeem, issue, and deliver requests to the previous node. +// Inflate amount requested by required fees. +// Reedems are limited based on IOUs previous has on hand. +// Issues are limited based on credit limits and amount owed. +// No account balance adjustments as we don't know how much is going to actually be pushed through yet. // <-- tesSUCCESS or tepPATH_DRY TER RippleCalc::calcNodeAccountRev(const unsigned int uIndex, PathState::ref pspCur, const bool bMultiQuality) { @@ -1135,7 +1139,9 @@ TER RippleCalc::calcNodeAccountRev(const unsigned int uIndex, PathState::ref psp return terResult; } -// When moving forward, we know the actual amount to push through so adjust balances. +// The reverse pass has been narrowing by credit available and inflating by fees as it worked backwards. +// Now, push through the actual amount to each node and adjust balances. +// // Perform balance adjustments between previous and current node. // - The previous node: specifies what to push through to current. // - All of previous output is consumed. @@ -1204,46 +1210,39 @@ TER RippleCalc::calcNodeAccountFwd( if (bPrvAccount && bNxtAccount) { + // Next is an account, must be rippling. + if (!uIndex) { // ^ --> ACCOUNT --> account - // First node, calculate amount to send. - // XXX Limit by stamp/ripple balance + // First node, calculate amount to ripple based on what is available. - const STAmount& saCurSendMaxReq = pspCur->saInReq.isNegative() - ? pspCur->saInReq // Negative for no limit, doing a calculation. + // Limit by sendmax. + const STAmount saCurSendMaxReq = pspCur->saInReq.isNegative() + ? pspCur->saInReq // Negative for no limit, doing a calculation. : pspCur->saInReq-pspCur->saInAct; // request - done. STAmount& saCurSendMaxPass = pspCur->saInPass; // Report how much pass sends. - if (saCurRedeemReq) - { - // Redeem requested. - saCurRedeemAct = saCurRedeemReq.isNegative() - ? saCurRedeemReq - : std::min(saCurRedeemReq, saCurSendMaxReq); - } - else - { - // No redeeming. - - saCurRedeemAct = saCurRedeemReq; - } + saCurRedeemAct = saCurRedeemReq + // Redeem requested. + ? saCurRedeemReq.isNegative() + ? saCurRedeemReq + : std::min(saCurRedeemReq, saCurSendMaxReq) + // No redeeming. + : saCurRedeemReq; saCurSendMaxPass = saCurRedeemAct; - if (saCurIssueReq && (saCurSendMaxReq.isNegative() || saCurSendMaxPass != saCurSendMaxReq)) - { - // Issue requested and pass does not meet max. - saCurIssueAct = saCurSendMaxReq.isNegative() - ? saCurIssueReq - : std::min(saCurSendMaxReq-saCurRedeemAct, saCurIssueReq); - } - else - { - // No issuing. + saCurIssueAct = (saCurIssueReq // Issue wanted. + && (saCurSendMaxReq.isNegative() // No limit. + || saCurSendMaxPass != saCurSendMaxReq)) // Not yet satisfied. + // Issue requested and pass does not meet max. + ? saCurSendMaxReq.isNegative() + ? saCurIssueReq + : std::min(saCurSendMaxReq-saCurRedeemAct, saCurIssueReq) + // No issuing. + : STAmount(saCurIssueReq); - saCurIssueAct = STAmount(saCurIssueReq); - } saCurSendMaxPass += saCurIssueAct; cLog(lsINFO) << boost::str(boost::format("calcNodeAccountFwd: ^ --> ACCOUNT --> account : saInReq=%s saInAct=%s saCurSendMaxReq=%s saCurRedeemAct=%s saCurIssueReq=%s saCurIssueAct=%s saCurSendMaxPass=%s") @@ -1287,7 +1286,7 @@ TER RippleCalc::calcNodeAccountFwd( saCurIssueAct.zero(saCurIssueReq); // Previous redeem part 1: redeem -> redeem - if (saPrvRedeemReq != saPrvRedeemAct) // Previous wants to redeem. To next must be ok. + if (saPrvRedeemReq && saCurRedeemReq) // Previous wants to redeem. { // Rate : 1.0 : quality out calcNodeRipple(QUALITY_ONE, uQualityOut, saPrvRedeemReq, saCurRedeemReq, saPrvRedeemAct, saCurRedeemAct, uRateMax); @@ -1302,25 +1301,23 @@ TER RippleCalc::calcNodeAccountFwd( } // Previous redeem part 2: redeem -> issue. - // wants to redeem and current would and can issue. - // If redeeming cur to next is done, this implies can issue. if (saPrvRedeemReq != saPrvRedeemAct // Previous still wants to redeem. - && saCurRedeemReq == saCurRedeemAct // Current has no more to redeem to next. - && saCurIssueReq) + && saCurRedeemReq == saCurRedeemAct // Current redeeming is done can issue. + && saCurIssueReq) // Current wants to issue. { // Rate : 1.0 : transfer_rate calcNodeRipple(QUALITY_ONE, lesActive.rippleTransferRate(uCurAccountID), saPrvRedeemReq, saCurIssueReq, saPrvRedeemAct, saCurIssueAct, uRateMax); } // Previous issue part 2 : issue -> issue - if (saPrvIssueReq != saPrvIssueAct) // Previous wants to issue. To next must be ok. + if (saPrvIssueReq != saPrvIssueAct // Previous wants to issue. + && saCurRedeemReq == saCurRedeemAct) // Current redeeming is done can issue. { // Rate: quality in : 1.0 calcNodeRipple(uQualityIn, QUALITY_ONE, saPrvIssueReq, saCurIssueReq, saPrvIssueAct, saCurIssueAct, uRateMax); } // Adjust prv --> cur balance : take all inbound - // XXX Currency must be in amount. lesActive.rippleCredit(uPrvAccountID, uCurAccountID, saPrvRedeemReq + saPrvIssueReq, false); } } @@ -1429,7 +1426,7 @@ bool PathState::lessPriority(PathState::ref lhs, PathState::ref rhs) return lhs->mIndex > rhs->mIndex; // Bigger is worse. } -// Make sure the path delivers to uAccountID: uCurrencyID from uIssuerID. +// Make sure last path node delivers to uAccountID: uCurrencyID from uIssuerID. // // If the unadded next node as specified by arguments would not work as is, then add the necessary nodes so it would work. // @@ -1651,7 +1648,40 @@ PathState::PathState( | STPathElement::typeIssuer, uSenderID, uInCurrencyID, - uInIssuerID); + uSenderID); + + if (tesSUCCESS == terStatus + && !!uInCurrencyID // First was not XRC + && uInIssuerID != uSenderID) { // Issuer was not same as sender + // May have an implied node. + + // Figure out next node properties for implied node. + const uint160 uNxtCurrencyID = spSourcePath.getElementCount() + ? spSourcePath.getElement(0).getCurrency() + : uOutCurrencyID; + const uint160 uNxtAccountID = spSourcePath.getElementCount() + ? spSourcePath.getElement(0).getAccountID() + : !!uOutCurrencyID + ? uOutIssuerID == uReceiverID + ? uReceiverID + : uOutIssuerID + : ACCOUNT_XNS; + + // Can't just use push implied, because it can't compensate for next account. + if (!uNxtCurrencyID // Next is XRC - will have offer next + || uInCurrencyID != uNxtCurrencyID // Next is different current - will have offer next + || uInIssuerID != uNxtAccountID) // Next is not implied issuer + { + // Add implied account. + terStatus = pushNode( + STPathElement::typeAccount + | STPathElement::typeCurrency + | STPathElement::typeIssuer, + uInIssuerID, + uInCurrencyID, + uInIssuerID); + } + } BOOST_FOREACH(const STPathElement& speElement, spSourcePath) { @@ -1659,21 +1689,35 @@ PathState::PathState( terStatus = pushNode(speElement.getNodeType(), speElement.getAccountID(), speElement.getCurrency(), speElement.getIssuerID()); } + const PaymentNode& pnPrv = vpnNodes.back(); + + if (tesSUCCESS == terStatus + && !!uOutCurrencyID // Next is not XRC + && uOutIssuerID != uReceiverID // Out issuer is not reciever + && (pnPrv.uCurrencyID != uOutCurrencyID // Previous will be an offer. + || pnPrv.uAccountID != uOutIssuerID)) // Need the implied issuer. + { + // Add implied account. + terStatus = pushNode( + STPathElement::typeAccount + | STPathElement::typeCurrency + | STPathElement::typeIssuer, + uOutIssuerID, + uInCurrencyID, + uOutIssuerID); + } + if (tesSUCCESS == terStatus) { // Create receiver node. - terStatus = pushImply(uReceiverID, uOutCurrencyID, uOutIssuerID); - if (tesSUCCESS == terStatus) - { - terStatus = pushNode( - STPathElement::typeAccount // Last node is always an account. - | STPathElement::typeCurrency - | STPathElement::typeIssuer, - uReceiverID, // Receive to output - uOutCurrencyID, // Desired currency - uOutIssuerID); - } + terStatus = pushNode( + STPathElement::typeAccount // Last node is always an account. + | STPathElement::typeCurrency + | STPathElement::typeIssuer, + uReceiverID, // Receive to output + uOutCurrencyID, // Desired currency + !!uOutCurrencyID ? uReceiverID : ACCOUNT_XNS); } if (tesSUCCESS == terStatus) diff --git a/src/SerializedTypes.h b/src/SerializedTypes.h index 4832b4621e..b86df1ddb5 100644 --- a/src/SerializedTypes.h +++ b/src/SerializedTypes.h @@ -603,7 +603,7 @@ public: int getElementCount() const { return mPath.size(); } bool isEmpty() const { return mPath.empty(); } const STPathElement& getElement(int offset) const { return mPath[offset]; } - const STPathElement& getElemet(int offset) { return mPath[offset]; } + const STPathElement& getElement(int offset) { return mPath[offset]; } void addElement(const STPathElement &e) { mPath.push_back(e); } void clear() { mPath.clear(); } bool hasSeen(const uint160 &acct); diff --git a/src/TransactionAction.cpp b/src/TransactionAction.cpp index a793957f22..8ff5c1f341 100644 --- a/src/TransactionAction.cpp +++ b/src/TransactionAction.cpp @@ -462,7 +462,11 @@ TER TransactionEngine::doPayment(const SerializedTransaction& txn, const Transac const bool bMax = txn.isFieldPresent(sfSendMax); const uint160 uDstAccountID = txn.getFieldAccount160(sfDestination); const STAmount saDstAmount = txn.getFieldAmount(sfAmount); - const STAmount saMaxAmount = bMax ? txn.getFieldAmount(sfSendMax) : saDstAmount; + const STAmount saMaxAmount = bMax + ? txn.getFieldAmount(sfSendMax) + : saDstAmount.isNative() + ? saDstAmount + : STAmount(saDstAmount.getCurrency(), mTxnAccountID, saDstAmount.getMantissa(), saDstAmount.getExponent(), saDstAmount.isNegative()); const uint160 uSrcCurrency = saMaxAmount.getCurrency(); const uint160 uDstCurrency = saDstAmount.getCurrency();