Abstract valid offer testing in OfferCreate.

src/cpp/ripple/OfferCreateTransactor.cpp

@@ -7,6 +7,80 @@
 
 SETUP_LOG();
 
+// Make sure an offer is still valid. If not, mark it unfunded.
+bool OfferCreateTransactor::bValidOffer(
+	SLE::ref			sleOfferDir,
+	const uint256&		uOfferIndex,
+	const uint160&		uOfferOwnerID,
+	const STAmount&		saOfferPays,
+	const STAmount&		saOfferGets,
+	const uint160&		uTakerAccountID,
+	boost::unordered_set<uint256>&	usOfferUnfundedFound,
+	boost::unordered_set<uint256>&	usOfferUnfundedBecame,
+	boost::unordered_set<uint160>&	usAccountTouched,
+	STAmount&			saOfferFunds) {						// <--
+	bool	bValid;
+
+	if (sleOfferDir->isFieldPresent(sfExpiration) && sleOfferDir->getFieldU32(sfExpiration) <= mEngine->getLedger()->getParentCloseTimeNC())
+	{
+		// Offer is expired. Expired offers are considered unfunded. Delete it.
+		cLog(lsINFO) << "bValidOffer: encountered expired offer";
+
+		usOfferUnfundedFound.insert(uOfferIndex);
+
+		bValid	= false;
+	}
+	else if (uOfferOwnerID == uTakerAccountID)
+	{
+		// Would take own offer. Consider old offer expired. Delete it.
+		cLog(lsINFO) << "bValidOffer: encountered taker's own old offer";
+
+		usOfferUnfundedFound.insert(uOfferIndex);
+
+		bValid	= false;
+	}
+	else if (!saOfferGets.isPositive() || !saOfferPays.isPositive())
+	{
+		// Offer has bad amounts. Consider offer expired. Delete it.
+		cLog(lsWARNING) << boost::str(boost::format("bValidOffer: BAD OFFER: saOfferPays=%s saOfferGets=%s")
+			% saOfferPays % saOfferGets);
+
+		usOfferUnfundedFound.insert(uOfferIndex);
+	}
+	else
+	{
+		cLog(lsINFO) << "bValidOffer: saOfferPays=" << saOfferPays.getFullText();
+
+		saOfferFunds	= mEngine->getNodes().accountFunds(uOfferOwnerID, saOfferPays);
+
+		if (!saOfferFunds.isPositive())
+		{
+			// Offer is unfunded, possibly due to previous balance action.
+			cLog(lsINFO) << "bValidOffer: offer unfunded: delete";
+
+			boost::unordered_set<uint160>::iterator	account	= usAccountTouched.find(uOfferOwnerID);
+			if (account != usAccountTouched.end())
+			{
+				// Previously touched account.
+				usOfferUnfundedBecame.insert(uOfferIndex);	// Delete unfunded offer on success.
+			}
+			else
+			{
+				// Never touched source account.
+				usOfferUnfundedFound.insert(uOfferIndex);	// Delete found unfunded offer when possible.
+			}
+
+			bValid	= false;
+		}
+		else
+		{
+			bValid	= true;
+		}
+	}
+
+	return bValid;
+}
+
 // 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 offers (w/ issuer)
@@ -28,7 +102,7 @@ TER OfferCreateTransactor::takeOffers(
 	bool&				bUnfunded)
 {
 	// The book has the most elements. Take the perspective of the book.
-	// Book is ordered for taker: taker pays / taker gets, < is better
+	// Book is ordered for taker: taker pays / taker gets (smaller is better)
 
 	// The order is for the other books currencys for get and pays are opposites.
 	// We want the same ratio for the respective currencies.
@@ -141,56 +215,16 @@ TER OfferCreateTransactor::takeOffers(
 			STAmount		saOfferPays		= sleOffer->getFieldAmount(sfTakerGets);
 			STAmount		saOfferGets		= sleOffer->getFieldAmount(sfTakerPays);
 
-			if (sleOffer->isFieldPresent(sfExpiration) && sleOffer->getFieldU32(sfExpiration) <= mEngine->getLedger()->getParentCloseTimeNC())
+			STAmount		saOfferFunds;	// Funds of offer owner to payout.
-			{
+			bool			bValid;
-				// Offer is expired. Expired offers are considered unfunded. Delete it.
-				cLog(lsINFO) << "takeOffers: encountered expired offer";
 
-				usOfferUnfundedFound.insert(uOfferIndex);
+			bValid	=  bValidOffer(
-			}
+				sleOfferDir, uOfferIndex, uOfferOwnerID, saOfferPays, saOfferGets,
-			else if (uOfferOwnerID == uTakerAccountID)
+				uTakerAccountID,
-			{
+				usOfferUnfundedFound, usOfferUnfundedBecame, usAccountTouched,
-				// Would take own offer. Consider old offer expired. Delete it.
+				saOfferFunds);
-				cLog(lsINFO) << "takeOffers: encountered taker's own old offer";
 
-				usOfferUnfundedBecame.insert(uOfferIndex);
+			if (bValid) {
-			}
-			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.
-
-				cLog(lsINFO) << "takeOffers: saOfferPays=" << saOfferPays.getFullText();
-
-				STAmount		saOfferFunds	= lesActive.accountFunds(uOfferOwnerID, saOfferPays);
-				SLE::pointer	sleOfferAccount;	// Owner of offer.
-
-				if (!saOfferFunds.isPositive())		// Includes zero.
-				{
-					// Offer is unfunded, possibly due to previous balance action.
-					cLog(lsINFO) << "takeOffers: offer unfunded: delete";
-
-					boost::unordered_set<uint160>::iterator	account	= usAccountTouched.find(uOfferOwnerID);
-					if (account != usAccountTouched.end())
-					{
-						// Previously touched account.
-						usOfferUnfundedBecame.insert(uOfferIndex);	// Delete unfunded offer on success.
-					}
-					else
-					{
-						// Never touched source account.
-						usOfferUnfundedFound.insert(uOfferIndex);	// Delete found unfunded offer when possible.
-					}
-				}
-				else
-				{
 				STAmount	saSubTakerPaid;
 				STAmount	saSubTakerGot;
 				STAmount	saTakerIssuerFee;
@@ -300,7 +334,6 @@ TER OfferCreateTransactor::takeOffers(
 			}
 		}
 	}
-	}
 
 	cLog(lsINFO) << "takeOffers: " << transToken(terResult);
 

src/cpp/ripple/OfferCreateTransactor.h

@@ -6,6 +6,18 @@
 class OfferCreateTransactor : public Transactor
 {
 protected:
+	bool bValidOffer(
+		SLE::ref			sleOfferDir,
+		const uint256&		uOffer,
+		const uint160&		uOfferOwnerID,
+		const STAmount&		saOfferPays,
+		const STAmount&		saOfferGets,
+		const uint160&		uTakerAccountID,
+		boost::unordered_set<uint256>&	usOfferUnfundedFound,
+		boost::unordered_set<uint256>&	usOfferUnfundedBecame,
+		boost::unordered_set<uint160>&	usAccountTouched,
+		STAmount&			saOfferFunds);
+
 	TER takeOffers(
 		const bool			bOpenLedger,
 		const bool			bPassive,