Add checking for invalid offers in ledger.

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.

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)
 		{

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);
+		STAmount	saTakerGetsNew	= saTakerGets - saOutPass;
-		sleOffer->setFieldAmount(sfTakerPays, saTakerPays - saInPassAct);
+		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);
+			STAmount	saTakerGetsNew	= saTakerGets - saOutPassAct;
-			sleOffer->setFieldAmount(sfTakerPays, saTakerPays - saInPassAct);
+			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.
 

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<PathState::pointer>& vpsExpanded, bool bKeepDefault);