From deebd07814bf628d152a17b2e7dfbd2ba747a96d Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Mon, 4 Mar 2013 15:14:17 -0800 Subject: [PATCH] Add checking for invalid offers in ledger. --- src/cpp/ripple/OfferCreateTransactor.cpp | 8 ++++ src/cpp/ripple/PaymentTransactor.cpp | 3 +- src/cpp/ripple/RippleCalc.cpp | 60 +++++++++++++++++++----- src/cpp/ripple/RippleCalc.h | 7 ++- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/cpp/ripple/OfferCreateTransactor.cpp b/src/cpp/ripple/OfferCreateTransactor.cpp index 8155956f2..225d376c7 100644 --- a/src/cpp/ripple/OfferCreateTransactor.cpp +++ b/src/cpp/ripple/OfferCreateTransactor.cpp @@ -152,6 +152,14 @@ TER OfferCreateTransactor::takeOffers( usOfferUnfundedFound.insert(uOfferIndex); } + else if (!saOfferGets.isPositive() || !saOfferPays.isPositive()) + { + // Offer has bad amounts. Consider offer expired. Delete it. + cLog(lsWARNING) << boost::str(boost::format("takeOffers: BAD OFFER: saOfferPays=%s saOfferGets=%s") + % saOfferPays % saOfferGets); + + usOfferUnfundedFound.insert(uOfferIndex); + } else { // Get offer funds available. diff --git a/src/cpp/ripple/PaymentTransactor.cpp b/src/cpp/ripple/PaymentTransactor.cpp index 9af301117..ed46b8253 100644 --- a/src/cpp/ripple/PaymentTransactor.cpp +++ b/src/cpp/ripple/PaymentTransactor.cpp @@ -177,7 +177,8 @@ TER PaymentTransactor::doApply() bPartialPayment, bLimitQuality, bNoRippleDirect, // Always compute for finalizing ledger. - false); // Not standalone, delete unfundeds. + false, // Not standalone, delete unfundeds. + isSetBit(mParams, tapOPEN_LEDGER)); } catch (const std::exception& e) { diff --git a/src/cpp/ripple/RippleCalc.cpp b/src/cpp/ripple/RippleCalc.cpp index 99a003ccf..a6fa7d744 100644 --- a/src/cpp/ripple/RippleCalc.cpp +++ b/src/cpp/ripple/RippleCalc.cpp @@ -909,6 +909,8 @@ TER RippleCalc::calcNodeAdvance( // Got a new offer. sleOffer = lesActive.entryCache(ltOFFER, uOfferIndex); uOfrOwnerID = sleOffer->getFieldAccount(sfAccount).getAccountID(); + saTakerPays = sleOffer->getFieldAmount(sfTakerPays); + saTakerGets = sleOffer->getFieldAmount(sfTakerGets); const aciSource asLine = boost::make_tuple(uOfrOwnerID, uCurCurrencyID, uCurIssuerID); @@ -923,6 +925,16 @@ TER RippleCalc::calcNodeAdvance( bEntryAdvance = true; continue; } + else if (!saTakerPays.isPositive() || !saTakerGets.isPositive()) + { + // Offer is has bad amounts. + cLog(lsWARNING) << boost::str(boost::format("calcNodeAdvance: NON-POSITIVE: saTakerPays=%s saTakerGets=%s") + % saTakerPays % saTakerGets); + + assert(musUnfundedFound.find(uOfferIndex) != musUnfundedFound.end()); // Verify reverse found it too. + bEntryAdvance = true; + continue; + } // 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. @@ -964,9 +976,6 @@ TER RippleCalc::calcNodeAdvance( continue; } - saTakerPays = sleOffer->getFieldAmount(sfTakerPays); - saTakerGets = sleOffer->getFieldAmount(sfTakerGets); - saOfferFunds = lesActive.accountFunds(uOfrOwnerID, saTakerGets); // Funds left. if (!saOfferFunds.isPositive()) @@ -1205,8 +1214,22 @@ TER RippleCalc::calcNodeDeliverRev( break; // Adjust offer - sleOffer->setFieldAmount(sfTakerGets, saTakerGets - saOutPass); - sleOffer->setFieldAmount(sfTakerPays, saTakerPays - saInPassAct); + STAmount saTakerGetsNew = saTakerGets - saOutPass; + STAmount saTakerPaysNew = saTakerPays - saInPassAct; + + if (saTakerPaysNew.isNegative() || saTakerGetsNew.isNegative()) + { + cLog(lsWARNING) << boost::str(boost::format("calcNodeDeliverRev: NEGATIVE: saTakerPaysNew=%s saTakerGetsNew=%s") + % saTakerPaysNew % saTakerGetsNew); + + terResult = mOpenLedger + ? telFAILED_PROCESSING // Ledger is not final, can vote no. + : tecFAILED_PROCESSING; + break; + } + + sleOffer->setFieldAmount(sfTakerGets, saTakerGetsNew); + sleOffer->setFieldAmount(sfTakerPays, saTakerPaysNew); lesActive.entryModify(sleOffer); @@ -1399,8 +1422,22 @@ TER RippleCalc::calcNodeDeliverFwd( // Adjust offer // Fees are considered paid from a seperate budget and are not named in the offer. - sleOffer->setFieldAmount(sfTakerGets, saTakerGets - saOutPassAct); - sleOffer->setFieldAmount(sfTakerPays, saTakerPays - saInPassAct); + STAmount saTakerGetsNew = saTakerGets - saOutPassAct; + STAmount saTakerPaysNew = saTakerPays - saInPassAct; + + if (saTakerPaysNew.isNegative() || saTakerGetsNew.isNegative()) + { + cLog(lsWARNING) << boost::str(boost::format("calcNodeDeliverFwd: NEGATIVE: saTakerPaysNew=%s saTakerGetsNew=%s") + % saTakerPaysNew % saTakerGetsNew); + + terResult = mOpenLedger + ? telFAILED_PROCESSING // Ledger is not final, can vote no. + : tecFAILED_PROCESSING; + break; + } + + sleOffer->setFieldAmount(sfTakerGets, saTakerGetsNew); + sleOffer->setFieldAmount(sfTakerPays, saTakerPaysNew); lesActive.entryModify(sleOffer); @@ -2418,7 +2455,7 @@ TER RippleCalc::rippleCalc( // Issuer: // XRP: ACCOUNT_XRP // non-XRP: uSrcAccountID (for any issuer) or another account with trust node. - const STAmount& saMaxAmountReq, // --> -1 = no limit. + const STAmount& saMaxAmountReq, // --> -1 = no limit. // Issuer: // XRP: ACCOUNT_XRP @@ -2431,12 +2468,13 @@ TER RippleCalc::rippleCalc( const bool bPartialPayment, const bool bLimitQuality, const bool bNoRippleDirect, - const bool bStandAlone // True, not to delete unfundeds. + const bool bStandAlone, // True, not to delete unfundeds. + const bool bOpenLedger ) { - RippleCalc rc(lesActive); + RippleCalc rc(lesActive, bOpenLedger); - TER terResult = temUNCERTAIN; + TER terResult = temUNCERTAIN; // YYY Might do basic checks on src and dst validity as per doPayment. diff --git a/src/cpp/ripple/RippleCalc.h b/src/cpp/ripple/RippleCalc.h index 3f4e7e31c..eacf62a42 100644 --- a/src/cpp/ripple/RippleCalc.h +++ b/src/cpp/ripple/RippleCalc.h @@ -153,6 +153,7 @@ class RippleCalc { protected: LedgerEntrySet& lesActive; + bool mOpenLedger; public: // First time working in reverse a funding source was mentioned. Source may only be used there. @@ -192,7 +193,8 @@ public: STAmount& saPrvAct, STAmount& saCurAct, uint64& uRateMax); - RippleCalc(LedgerEntrySet& lesNodes) : lesActive(lesNodes) { ; } + RippleCalc(LedgerEntrySet& lesNodes, const bool bOpenLedger) + : lesActive(lesNodes), mOpenLedger(bOpenLedger) { ; } static TER rippleCalc( LedgerEntrySet& lesActive, @@ -207,7 +209,8 @@ public: const bool bPartialPayment, const bool bLimitQuality, const bool bNoRippleDirect, - const bool bStandAlone + const bool bStandAlone, + const bool bOpenLedger = true ); static void setCanonical(STPathSet& spsDst, const std::vector& vpsExpanded, bool bKeepDefault);