Add directory and order book iterators

This should fix the crossed order book bug.
* Change OfferCreate::takeOffers to use new iterators
* Change NetworkOps::getBookPage to use new iterators
* If we find an offer in the book but not the ledger, deindex it

Builds/VisualStudio2012/RippleD.vcxproj

@@ -844,6 +844,12 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\src\ripple_app\ledger\DirectoryEntryIterator.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\src\ripple_app\ledger\Ledger.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
@@ -916,6 +922,12 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\src\ripple_app\ledger\OrderBookIterator.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|x64'">true</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\src\ripple_app\ledger\SerializedValidation.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
@@ -2426,6 +2438,7 @@
     <ClInclude Include="..\..\src\ripple_app\data\DatabaseCon.h" />
     <ClInclude Include="..\..\src\ripple_app\data\DBInit.h" />
     <ClInclude Include="..\..\src\ripple_app\data\SqliteDatabase.h" />
+    <ClInclude Include="..\..\src\ripple_app\ledger\DirectoryEntryIterator.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\Ledger.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\LedgerCleaner.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\LedgerMaster.h" />
@@ -2439,6 +2452,7 @@
     <ClInclude Include="..\..\src\ripple_app\ledger\InboundLedgers.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\LedgerEntrySet.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\LedgerHistory.h" />
+    <ClInclude Include="..\..\src\ripple_app\ledger\OrderBookIterator.h" />
     <ClInclude Include="..\..\src\ripple_app\ledger\SerializedValidation.h" />
     <ClInclude Include="..\..\src\ripple_app\main\CollectorManager.h" />
     <ClInclude Include="..\..\src\ripple_app\main\IoServicePool.h" />

Builds/VisualStudio2012/RippleD.vcxproj.filters

@@ -2922,6 +2922,18 @@
     <ClInclude Include="..\..\src\ripple_core\nodestore\impl\Tuning.h">
       <Filter>[2] Old Ripple\ripple_core\nodestore\impl</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\src\ripple_app\ledger\DirectoryEntryIterator.h">
+      <Filter>[2] Old Ripple\ripple_app\ledger</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\ripple_app\ledger\OrderBookIterator.h">
+      <Filter>[2] Old Ripple\ripple_app\ledger</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\ripple_app\ledger\DirectoryEntryIterator.h">
+      <Filter>[2] Old Ripple\ripple_app\ledger</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\ripple_app\ledger\OrderBookIterator.h">
+      <Filter>[2] Old Ripple\ripple_app\ledger</Filter>
+    </ClInclude>
     <ClInclude Include="..\..\src\ripple_app\peers\PeerSet.h">
       <Filter>[2] Old Ripple\ripple_app\peers</Filter>
     </ClInclude>

src/ripple_app/ledger/DirectoryEntryIterator.cpp

@@ -0,0 +1,107 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012, 2013 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+/** Get the current ledger entry
+*/
+SLE::pointer DirectoryEntryIterator::getEntry (LedgerEntrySet& les, LedgerEntryType type)
+{
+    return les.entryCache (type, mEntryIndex);
+}
+
+/** Position the iterator at the first entry
+*/
+bool DirectoryEntryIterator::firstEntry (LedgerEntrySet& les)
+{
+    WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::firstEntry(" << mRootIndex.GetHex() << ")";
+    mEntry = 0;
+    mDirIndex = mRootIndex;
+
+    return nextEntry (les);
+}
+
+/** Advance the iterator to the next entry
+*/
+bool DirectoryEntryIterator::nextEntry (LedgerEntrySet& les)
+{
+
+    if (!mDirNode || mDirNode->getIndex() != mDirIndex)
+    {
+        WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::nextEntry(" << mRootIndex.GetHex() << ") need dir node";
+        // Are we already at the end
+        if (mDirIndex.isZero())
+        {
+            WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::nextEntry(" << mRootIndex.GetHex() << ") at end";
+            return false;
+        }
+
+        // Fetch the current directory
+        mDirNode = les.entryCache (ltDIR_NODE, mRootIndex);
+        if (!mDirNode)
+        {
+            WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::nextEntry(" << mRootIndex.GetHex() << ") no dir node";
+            mEntryIndex.zero();
+            return false;
+        }
+    }
+
+   if (!les.dirNext (mRootIndex, mDirNode, mEntry, mEntryIndex))
+   {
+       mDirIndex.zero();
+       mDirNode.reset();
+       WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::nextEntry(" << mRootIndex.GetHex() << ") now at end";
+       return false;
+   }
+
+   WriteLog (lsTRACE, Ledger) << "DirectoryEntryIterator::nextEntry(" << mRootIndex.GetHex() << ") now at " << mEntry;
+   return true;
+}
+
+bool DirectoryEntryIterator::addJson (Json::Value& j) const
+{
+    if (mDirNode && (mEntry != 0))
+    {
+        j["dir_root"] = mRootIndex.GetHex();
+        j["dir_index"] = mDirIndex.GetHex();
+        j["dir_entry"] = static_cast<Json::UInt> (mEntry);
+        return true;
+    }
+    return false;
+}
+
+bool DirectoryEntryIterator::setJson (Json::Value const& j, LedgerEntrySet& les)
+{
+    if (!j.isMember("dir_root") || !j.isMember("dir_index") || !j.isMember("dir_entry"))
+        return false;
+#if 0 // WRITEME
+    Json::Value const& dirRoot = j["dir_root"];
+    Json::Value const& dirIndex = j["dir_index"];
+    Json::Value const& dirEntry = j["dir_entry"];
+
+    assert(false); // CAUTION: This function is incomplete
+
+    mEntry = j["dir_entry"].asUInt ();
+
+    if (!mDirIndex.SetHex(j["dir_index"].asString()))
+        return false;
+#endif
+
+    return true;
+}
+
+// vim:ts=4

src/ripple_app/ledger/DirectoryEntryIterator.h

@@ -0,0 +1,88 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012, 2013 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#ifndef RIPPLE_DIRECTORYENTRYITERATOR_H_INCLUDED
+#define RIPPLE_DIRECTORYENTRYITERATOR_H_INCLUDED
+
+/** An iterator that walks the ledger entries in a single directory
+*/
+class DirectoryEntryIterator
+{
+
+public:
+
+    DirectoryEntryIterator () : mEntry(0)
+    { ; }
+
+    DirectoryEntryIterator (uint256 const& index) : mRootIndex(index), mEntry(0)
+    { ; }
+
+    /** Construct from a reference to the root directory
+    */
+    DirectoryEntryIterator (SLE::ref directory) : mEntry (0), mDirNode (directory)
+    {
+        if (mDirNode)
+        {
+            mDirIndex = mDirNode->getIndex();
+            mRootIndex = mDirNode->getIndex();
+        }
+    }
+
+    /** Get the SLE this iterator currently references
+    */
+    SLE::pointer getEntry (LedgerEntrySet& les, LedgerEntryType type);
+
+    /** Make this iterator point to the first offer
+    */
+    bool firstEntry (LedgerEntrySet&);
+
+    /** Make this iterator point to the next offer
+    */
+    bool nextEntry (LedgerEntrySet&);
+
+    /** Add this iterator's position to a JSON object
+    */
+    bool addJson (Json::Value&) const;
+
+    /** Set this iterator's position from a JSON object
+    */
+    bool setJson (Json::Value const&, LedgerEntrySet& les);
+
+    uint256 const& getEntryLedgerIndex () const
+    {
+        return mEntryIndex;
+    }
+
+    uint256 const& getDirectory () const
+    {
+        return mDirIndex;
+    }
+
+private:
+
+    uint256      mRootIndex;    // ledger index of the root directory
+    uint256      mDirIndex;     // ledger index of the current directory
+    unsigned int mEntry;        // entry index we are on (0 means first is next)
+    uint256      mEntryIndex;   // ledger index of the current entry
+    SLE::pointer mDirNode;      // SLE for the entry we are on
+};
+
+#endif
+
+// vim:ts=4

src/ripple_app/ledger/LedgerEntrySet.cpp

@@ -278,11 +278,6 @@ bool LedgerEntrySet::hasChanges ()
     return false;
 }
 
-bool LedgerEntrySet::intersect (const LedgerEntrySet& lesLeft, const LedgerEntrySet& lesRight)
-{
-    return true;    // XXX Needs implementation
-}
-
 Json::Value LedgerEntrySet::getJson (int) const
 {
     Json::Value ret (Json::objectValue);
@@ -751,7 +746,7 @@ TER LedgerEntrySet::dirDelete (
     const bool                      bSoft)          // --> True, uNodeDir is not hard and fast (pass uNodeDir=0).
 {
     uint64              uNodeCur    = uNodeDir;
-    SLE::pointer        sleNode     = entryCache (ltDIR_NODE, uNodeCur ? Ledger::getDirNodeIndex (uRootIndex, uNodeCur) : uRootIndex);
+    SLE::pointer        sleNode     = entryCache (ltDIR_NODE, Ledger::getDirNodeIndex (uRootIndex, uNodeCur));
 
     if (!sleNode)
     {
@@ -882,11 +877,11 @@ TER LedgerEntrySet::dirDelete (
         {
             // Not root and not last node. Can delete node.
 
-            SLE::pointer        slePrevious = entryCache (ltDIR_NODE, uNodePrevious ? Ledger::getDirNodeIndex (uRootIndex, uNodePrevious) : uRootIndex);
+            SLE::pointer        slePrevious = entryCache (ltDIR_NODE, Ledger::getDirNodeIndex (uRootIndex, uNodePrevious));
 
             assert (slePrevious);
 
-            SLE::pointer        sleNext     = entryCache (ltDIR_NODE, uNodeNext ? Ledger::getDirNodeIndex (uRootIndex, uNodeNext) : uRootIndex);
+            SLE::pointer        sleNext     = entryCache (ltDIR_NODE, Ledger::getDirNodeIndex (uRootIndex, uNodeNext));
 
             assert (slePrevious);
             assert (sleNext);
@@ -1069,18 +1064,21 @@ void LedgerEntrySet::ownerCountAdjust (const uint160& uOwnerID, int iAmount, SLE
     }
 }
 
-TER LedgerEntrySet::offerDelete (SLE::ref sleOffer, uint256 const& uOfferIndex, const uint160& uOwnerID)
+TER LedgerEntrySet::offerDelete (SLE::pointer sleOffer)
 {
+
+    uint256 offerIndex = sleOffer->getIndex ();
+    uint160 uOwnerID   = sleOffer->getFieldAccount160 (sfAccount);
     bool    bOwnerNode = sleOffer->isFieldPresent (sfOwnerNode);   // Detect legacy dirs.
     uint64  uOwnerNode = sleOffer->getFieldU64 (sfOwnerNode);
-    TER     terResult   = dirDelete (false, uOwnerNode, Ledger::getOwnerDirIndex (uOwnerID), uOfferIndex, false, !bOwnerNode);
-    if (tesSUCCESS == terResult)
-        ownerCountAdjust (uOwnerID, -1);
-
-    // Offer delete is always hard. Always have hints.
     uint256 uDirectory = sleOffer->getFieldH256 (sfBookDirectory);
     uint64  uBookNode  = sleOffer->getFieldU64 (sfBookNode);
-    TER terResult2 = dirDelete ( false, uBookNode, uDirectory, uOfferIndex, true, false);
+
+    TER     terResult  = dirDelete (false, uOwnerNode, Ledger::getOwnerDirIndex (uOwnerID), offerIndex, false, !bOwnerNode);
+    TER     terResult2 = dirDelete (false, uBookNode, uDirectory, offerIndex, true, false);
+
+    if (tesSUCCESS == terResult)
+        ownerCountAdjust (uOwnerID, -1);
 
     entryDelete (sleOffer);
 
@@ -1094,8 +1092,7 @@ TER LedgerEntrySet::offerDelete (uint256 const& uOfferIndex)
     if (!sleOffer)
         return tesSUCCESS;
 
-    const uint160   uOwnerID    = sleOffer->getFieldAccount160 (sfAccount);
+    return offerDelete (sleOffer);
-    return offerDelete (sleOffer, uOfferIndex, uOwnerID);
 }
 
 // Returns amount owed by uToAccountID to uFromAccountID.

src/ripple_app/ledger/LedgerEntrySet.h

@@ -182,7 +182,7 @@ public:
 
     // Offer functions.
     TER                 offerDelete (uint256 const & uOfferIndex);
-    TER                 offerDelete (SLE::ref sleOffer, uint256 const & uOfferIndex, const uint160 & uOwnerID);
+    TER                 offerDelete (SLE::pointer sleOffer);
 
     // Balance functions.
     uint32              rippleTransferRate (const uint160 & uIssuerID);
@@ -246,8 +246,6 @@ public:
         return mEntries.end ();
     }
 
-    static bool intersect (const LedgerEntrySet & lesLeft, const LedgerEntrySet & lesRight);
-
 private:
     Ledger::pointer mLedger;
     std::map<uint256, LedgerEntrySetEntry>  mEntries; // cannot be unordered!

src/ripple_app/ledger/OrderBookIterator.cpp

@@ -0,0 +1,227 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012, 2013 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+/** Iterate through the directories in an order book
+*/
+BookDirIterator::BookDirIterator(uint160 const& uInCurrency, uint160 const& uInIssuer,
+    uint160 const& uOutCurrency, uint160 const& uOutIssuer)
+{
+    mBase = Ledger::getBookBase(uInCurrency, uInIssuer, uOutCurrency, uOutIssuer);
+    mEnd = Ledger::getQualityNext(mBase);
+    mIndex = mBase;
+}
+
+bool BookDirIterator::nextDirectory (LedgerEntrySet& les)
+{
+    WriteLog (lsTRACE, Ledger) << "BookDirectoryIterator:: nextDirectory";
+
+    // Are we already at the end?
+    if (mIndex.isZero ())
+        return false;
+
+    // Get the ledger index of the next directory
+    mIndex = les.getNextLedgerIndex (mIndex, mEnd);
+
+    if (mIndex.isZero ())
+    {
+        // We ran off the end of the book
+        WriteLog (lsTRACE, Ledger) << "BookDirectoryIterator:: no next ledger index";
+        return false;
+    }
+    assert (mIndex < mEnd);
+
+    WriteLog (lsTRACE, Ledger) << "BookDirectoryIterator:: index " << mIndex.GetHex();
+
+    // Retrieve the SLE from the LES
+    mOfferDir = les.entryCache (ltDIR_NODE, mIndex);
+    assert (mOfferDir);
+
+    return !!mOfferDir;
+}
+
+bool BookDirIterator::firstDirectory (LedgerEntrySet& les)
+{
+    WriteLog (lsTRACE, Ledger) << "BookDirIterator(" << mBase.GetHex() << ") firstDirectory";
+
+    /** Jump to the beginning
+    */
+    mIndex = mBase;
+
+    return nextDirectory (les);
+}
+
+/** The LES may have changed. Repoint to the current directory if it still exists,
+    Otherwise, go to the next one.
+*/
+bool BookDirIterator::resync (LedgerEntrySet& les)
+{
+    if (mIndex.isZero ())
+        mIndex = mBase;
+    else if (mIndex != mBase)
+        --mIndex;
+
+    return nextDirectory (les);
+}
+
+DirectoryEntryIterator BookDirIterator::getOfferIterator () const
+{
+    WriteLog (lsTRACE, Ledger) << "BookDirIterator(" << mBase.GetHex() << ") get offer iterator";
+    return DirectoryEntryIterator (mOfferDir);
+}
+
+uint64 BookDirIterator::getRate () const
+{
+    return Ledger::getQuality(mIndex);
+}
+
+bool BookDirIterator::addJson (Json::Value& jv) const
+{
+    if (! (*this))
+        return false;
+
+    jv["book_index"] = mIndex.GetHex();
+    return true;
+}
+
+bool BookDirIterator::setJson(Json::Value const& jv)
+{
+    if (!jv.isMember("book_index"))
+        return false;
+    const Json::Value& bi = jv["book_index"];
+    if (!bi.isString ())
+        return false;
+    mIndex.SetHexExact(bi.asString());
+    return true;
+}
+
+bool OrderBookIterator::addJson (Json::Value& jv) const
+{
+    return mOfferIterator.addJson(jv) && mDirectoryIterator.addJson(jv);
+}
+
+bool OrderBookIterator::setJson (Json::Value const& jv)
+{
+    return mDirectoryIterator.setJson (jv) && mOfferIterator.setJson (jv, mEntrySet);
+}
+
+STAmount OrderBookIterator::getCurrentRate () const
+{
+    return mDirectoryIterator.getCurrentRate();
+}
+
+uint64 OrderBookIterator::getCurrentQuality () const
+{
+    return mDirectoryIterator.getCurrentQuality();
+}
+
+uint256 OrderBookIterator::getCurrentDirectory () const
+{
+    return mOfferIterator.getDirectory ();
+}
+
+uint256 OrderBookIterator::getCurrentIndex () const
+{
+    return mOfferIterator.getEntryLedgerIndex();
+}
+
+/** Retrieve the offer the iterator points to
+*/
+SLE::pointer OrderBookIterator::getCurrentOffer ()
+{
+    return mOfferIterator.getEntry (mEntrySet, ltOFFER);
+}
+
+/** Go to the first offer in the first directory
+*/
+bool OrderBookIterator::firstOffer ()
+{
+    WriteLog (lsTRACE, Ledger) << "OrderBookIterator: first offer";
+    // Go to first directory in order book
+    if (!mDirectoryIterator.firstDirectory (mEntrySet))
+    {
+        WriteLog (lsTRACE, Ledger) << "OrderBookIterator: no first directory";
+        return false;
+    }
+    mOfferIterator = mDirectoryIterator.getOfferIterator ();
+
+    // Take the next offer
+    return nextOffer();
+}
+
+/** Go to the next offer, possibly changing directories
+*/
+bool OrderBookIterator::nextOffer ()
+{
+    WriteLog (lsTRACE, Ledger) << "OrderBookIterator: next offer";
+    do
+    {
+
+         // Is there a next offer in the current directory
+         if (mOfferIterator.nextEntry (mEntrySet))
+         {
+             WriteLog (lsTRACE, Ledger) << "OrderBookIterator: there is a next offer in this directory";
+             return true;
+         }
+
+          // Is there a next directory
+
+          if (!mDirectoryIterator.nextDirectory (mEntrySet))
+          {
+              WriteLog (lsTRACE, Ledger) << "OrderBookIterator: there is no next directory";
+              return false;
+          }
+          WriteLog (lsTRACE, Ledger) << "OrderBookIterator: going to next directory";
+
+          // Set to before its first offer
+         mOfferIterator = mDirectoryIterator.getOfferIterator ();
+     }
+     while (1);
+}
+
+/** Rewind to the beginning of this directory, then take the next offer
+*/
+bool OrderBookIterator::rewind ()
+{
+    if (!mDirectoryIterator.resync (mEntrySet))
+        return false;
+
+    mOfferIterator = mDirectoryIterator.getOfferIterator ();
+    return nextOffer ();
+}
+
+/** Go to before the first offer in the next directory
+*/
+bool OrderBookIterator::nextDir ()
+{
+    if (!mDirectoryIterator.nextDirectory (mEntrySet))
+        return false;
+
+    mOfferIterator = mDirectoryIterator.getOfferIterator ();
+
+    return true;
+}
+
+/** Advance to the next offer in this directory
+*/
+bool OrderBookIterator::nextOfferInDir ()
+{
+    return mOfferIterator.nextEntry (mEntrySet);
+}
+
+// vim:ts=4

src/ripple_app/ledger/OrderBookIterator.h

@@ -0,0 +1,203 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012, 2013 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#ifndef RIPPLE_ORDERBOOKITERATOR_H_INCLUDED
+#define RIPPLE_ORDERBOOKITERATOR_H_INCLUDED
+
+/** An iterator that walks the directories in a book
+*/
+class BookDirIterator
+{
+
+public:
+
+    BookDirIterator ()
+    { ; }
+
+    BookDirIterator (
+        uint160 const& uInCurrency, uint160 const& uInIssuer,
+        uint160 const& uOutCurrency, uint160 const& uOutIssuer);
+
+    uint256 const& getBookBase () const
+    {
+        return mBase;
+    }
+
+    uint256 const& getBookEnd () const
+    {
+        return mEnd;
+    }
+
+    uint256 const& getCurrentIndex() const
+    {
+        return mIndex;
+    }
+
+    void setCurrentIndex(uint256 const& index)
+    {
+        mIndex = index;
+    }
+
+    /** Get the current exchange rate
+    */
+    STAmount getCurrentRate () const
+    {
+        return STAmount::setRate (getCurrentQuality());
+    }
+
+    /** Get the current quality
+    */
+    uint64 getCurrentQuality () const
+    {
+        return Ledger::getQuality(mIndex);
+    }
+
+    /** Make this iterator refer to the next book
+    */
+    bool nextDirectory (LedgerEntrySet&);
+
+    /** Make this iterator refer to the first book
+    */
+    bool firstDirectory (LedgerEntrySet&);
+
+    /** The LES may have changed
+        Resync the iterator
+    */
+    bool resync (LedgerEntrySet&);
+
+    /** Get an iterator to the offers in this directory
+    */
+    DirectoryEntryIterator getOfferIterator () const;
+
+    uint64 getRate () const;
+
+    bool addJson (Json::Value&) const;
+
+    bool setJson (Json::Value const&);
+
+    // Does this iterator currently point to a valid directory
+    operator bool () const
+    {
+        return mOfferDir &&  (mOfferDir->getIndex() == mIndex);
+    }    
+
+private:
+
+    uint256      mBase;     // The first index a directory in the book can have
+    uint256      mEnd;      // The first index a directory in the book cannot have
+    uint256      mIndex;    // The index we are currently on
+    SLE::pointer mOfferDir; // The directory page we are currently on
+};
+
+/** An iterator that walks the offers in a book
+    CAUTION: The LedgerEntrySet must remain valid for the life of the iterator
+*/
+class OrderBookIterator
+{
+public:
+
+    OrderBookIterator (
+        LedgerEntrySet& set,
+        uint160 const& uInCurrency,
+        uint160 const& uInIssuer,
+        uint160 const& uOutCurrency,
+        uint160 const& uOutIssuer) :
+            mEntrySet (set),
+            mDirectoryIterator (uInCurrency, uInIssuer, uOutCurrency, uOutIssuer)
+    { ; }
+
+    bool addJson (Json::Value&) const;
+
+    bool setJson (Json::Value const&);
+
+    STAmount getCurrentRate () const;
+
+    uint64 getCurrentQuality () const;
+
+    uint256 getCurrentIndex () const;
+
+    uint256 getCurrentDirectory () const;
+
+    SLE::pointer getCurrentOffer ();
+
+    /** Position the iterator at the first offer in the first directory.
+        Returns whether there is an offer to point to.
+    */
+    bool firstOffer ();
+
+    /** Position the iterator at the next offer, going to the next directory if needed
+        Returns whether there is a next offer.
+    */
+    bool nextOffer ();
+
+    /** Position the iterator at the first offer in the next directory.
+        Returns whether there is a next directory to point to.
+    */
+    bool nextDir ();
+
+    /** Position the iterator at the first offer in the current directory.
+        Returns whether there is an offer in the directory.
+    */
+    bool firstOfferInDir ();
+
+    /** Position the iterator at the next offer in the current directory.
+        Returns whether there is a next offer in the directory.
+    */
+    bool nextOfferInDir ();
+
+    /** Position the iterator at the first offer at the current quality.
+        If none, position the iterator at the first offer at the next quality.
+        This rather odd semantic is required by the payment engine.
+    */
+    bool rewind ();
+
+    LedgerEntrySet& peekEntrySet ()
+    {
+        return mEntrySet;
+    }
+
+    BookDirIterator const& peekDirIterator () const
+    {
+        return mDirectoryIterator;
+    }
+
+    DirectoryEntryIterator const& peekDirectoryEntryIterator () const
+    {
+        return mOfferIterator;
+    }
+
+    BookDirIterator& peekDirIterator ()
+    {
+        return mDirectoryIterator;
+    }
+
+    DirectoryEntryIterator& peekDirectoryEntryIterator ()
+    {
+        return mOfferIterator;
+    }
+
+private:
+    LedgerEntrySet&         mEntrySet;
+    BookDirIterator         mDirectoryIterator;
+    DirectoryEntryIterator  mOfferIterator;
+};
+
+#endif
+
+// vim:ts=4

src/ripple_app/misc/NetworkOPs.cpp

@@ -2782,25 +2782,9 @@ void NetworkOPsImp::getBookPage (Ledger::pointer lpLedger, const uint160& uTaker
     Json::Value&    jvOffers    = (jvResult["offers"] = Json::Value (Json::arrayValue));
 
     std::map<uint160, STAmount> umBalance;
-    const uint256   uBookBase   = Ledger::getBookBase (uTakerPaysCurrencyID, uTakerPaysIssuerID, uTakerGetsCurrencyID, uTakerGetsIssuerID);
-    const uint256   uBookEnd    = Ledger::getQualityNext (uBookBase);
-    uint256         uTipIndex   = uBookBase;
-
-    m_journal.trace << boost::str (boost::format ("getBookPage: uTakerPaysCurrencyID=%s uTakerPaysIssuerID=%s") % STAmount::createHumanCurrency (uTakerPaysCurrencyID) % RippleAddress::createHumanAccountID (uTakerPaysIssuerID));
-    m_journal.trace << boost::str (boost::format ("getBookPage: uTakerGetsCurrencyID=%s uTakerGetsIssuerID=%s") % STAmount::createHumanCurrency (uTakerGetsCurrencyID) % RippleAddress::createHumanAccountID (uTakerGetsIssuerID));
-    m_journal.trace << boost::str (boost::format ("getBookPage: uBookBase=%s") % uBookBase);
-    m_journal.trace << boost::str (boost::format ("getBookPage:  uBookEnd=%s") % uBookEnd);
-    m_journal.trace << boost::str (boost::format ("getBookPage: uTipIndex=%s") % uTipIndex);
 
     LedgerEntrySet  lesActive (lpLedger, tapNONE, true);
-
+    OrderBookIterator obIterator (lesActive, uTakerPaysCurrencyID, uTakerPaysIssuerID, uTakerGetsCurrencyID, uTakerGetsIssuerID);
-    bool            bDone           = false;
-    bool            bDirectAdvance  = true;
-
-    SLE::pointer    sleOfferDir;
-    uint256         uOfferIndex;
-    unsigned int    uBookEntry;
-    STAmount        saDirRate;
 
     unsigned int    iLeft           = iLimit;
 
@@ -2809,42 +2793,16 @@ void NetworkOPsImp::getBookPage (Ledger::pointer lpLedger, const uint160& uTaker
 
     uint32  uTransferRate   = lesActive.rippleTransferRate (uTakerGetsIssuerID);
 
-    while (!bDone && (iLeft > 0))
+    while ((iLeft > 0) && obIterator.nextOffer ())
     {
-        if (bDirectAdvance)
-        {
-            bDirectAdvance  = false;
-
-            m_journal.trace << "getBookPage: bDirectAdvance";
-
-            sleOfferDir     = lesActive.entryCache (ltDIR_NODE, lpLedger->getNextLedgerIndex (uTipIndex, uBookEnd));
-
-            if (!sleOfferDir)
-            {
-                m_journal.trace << "getBookPage: bDone";
-                bDone           = true;
-            }
-            else
-            {
-                uTipIndex       = sleOfferDir->getIndex ();
-                saDirRate       = STAmount::setRate (Ledger::getQuality (uTipIndex));
-
-                lesActive.dirFirst (uTipIndex, sleOfferDir, uBookEntry, uOfferIndex);
-
-                m_journal.trace << boost::str (boost::format ("getBookPage:   uTipIndex=%s") % uTipIndex);
-                m_journal.trace << boost::str (boost::format ("getBookPage: uOfferIndex=%s") % uOfferIndex);
-            }
-        }
-
-        if (!bDone)
-        {
-            SLE::pointer    sleOffer        = lesActive.entryCache (ltOFFER, uOfferIndex);
 
+        SLE::pointer    sleOffer        = obIterator.getCurrentOffer();
         if (sleOffer)
         {
             const uint160   uOfferOwnerID   = sleOffer->getFieldAccount160 (sfAccount);
             const STAmount& saTakerGets     = sleOffer->getFieldAmount (sfTakerGets);
             const STAmount& saTakerPays     = sleOffer->getFieldAmount (sfTakerPays);
+            STAmount        saDirRate       = obIterator.getCurrentRate ();
             STAmount        saOwnerFunds;
 
             if (uTakerGetsIssuerID == uOfferOwnerID)
@@ -2935,20 +2893,7 @@ void NetworkOPsImp::getBookPage (Ledger::pointer lpLedger, const uint160& uTaker
                 jvOf["quality"]     = saDirRate.getText ();
                 --iLeft;
             }
-            }
-            else
-            {
-                m_journal.warning << "Missing offer";
-	    }
 
-            if (!lesActive.dirNext (uTipIndex, sleOfferDir, uBookEntry, uOfferIndex))
-            {
-                bDirectAdvance  = true;
-            }
-            else
-            {
-                m_journal.trace << boost::str (boost::format ("getBookPage: uOfferIndex=%s") % uOfferIndex);
-            }
         }
     }
 

src/ripple_app/paths/RippleCalc.cpp

@@ -197,9 +197,10 @@ TER RippleCalc::calcNodeAdvance (
         {
             // Got a new offer.
             sleOffer    = lesActive.entryCache (ltOFFER, uOfferIndex);
+
             if (!sleOffer)
             {
-                WriteLog (lsWARNING, RippleCalc) << "Missing offer in directory, " << uOfferIndex;
+                WriteLog (lsWARNING, RippleCalc) << "Missing offer in directory";
                 bEntryAdvance = true;
             }
             else

src/ripple_app/ripple_app.h

@@ -111,6 +111,8 @@ namespace ripple {
 #include "ledger/AcceptedLedgerTx.h"
 #include "ledger/AcceptedLedger.h"
 #include "ledger/LedgerEntrySet.h"
+#include "ledger/DirectoryEntryIterator.h"
+#include "ledger/OrderBookIterator.h"
 #include "tx/TransactionEngine.h"
 #include "misc/CanonicalTXSet.h"
 #include "ledger/LedgerHolder.h"

src/ripple_app/ripple_app_pt6.cpp

@@ -26,6 +26,8 @@ namespace ripple
 
 #include "ledger/LedgerEntrySet.cpp"
 #include "ledger/AcceptedLedger.cpp"
+#include "ledger/DirectoryEntryIterator.cpp"
+#include "ledger/OrderBookIterator.cpp"
 #include "consensus/DisputedTx.cpp"
 #include "misc/HashRouter.cpp"
 #include "misc/Offer.cpp"

src/ripple_app/tx/OfferCancelTransactor.cpp

@@ -51,7 +51,7 @@ TER OfferCancelTransactor::doApply ()
         {
             WriteLog (lsDEBUG, OfferCancelTransactor) << "OfferCancel: uOfferSequence=" << uOfferSequence;
 
-            terResult   = mEngine->getNodes ().offerDelete (sleOffer, uOfferIndex, mTxnAccountID);
+            terResult   = mEngine->getNodes ().offerDelete (sleOffer);
         }
         else
         {

src/ripple_app/tx/OfferCreateTransactor.cpp

@@ -22,7 +22,6 @@ SETUP_LOG (OfferCreateTransactor)
 // Make sure an offer is still valid. If not, mark it unfunded.
 bool OfferCreateTransactor::bValidOffer (
     SLE::ref            sleOffer,
-    uint256 const&      uOfferIndex,
     const uint160&      uOfferOwnerID,
     const STAmount&     saOfferPays,
     const STAmount&     saOfferGets,
@@ -39,7 +38,7 @@ bool OfferCreateTransactor::bValidOffer (
         // Offer is expired. Expired offers are considered unfunded. Delete it.
         WriteLog (lsINFO, OfferCreateTransactor) << "bValidOffer: encountered expired offer";
 
-        usOfferUnfundedFound.insert (uOfferIndex);
+        usOfferUnfundedFound.insert (sleOffer->getIndex());
 
         bValid  = false;
     }
@@ -48,7 +47,7 @@ bool OfferCreateTransactor::bValidOffer (
         // Would take own offer. Consider old offer expired. Delete it.
         WriteLog (lsINFO, OfferCreateTransactor) << "bValidOffer: encountered taker's own old offer";
 
-        usOfferUnfundedFound.insert (uOfferIndex);
+        usOfferUnfundedFound.insert (sleOffer->getIndex());
 
         bValid  = false;
     }
@@ -58,7 +57,7 @@ bool OfferCreateTransactor::bValidOffer (
         WriteLog (lsWARNING, OfferCreateTransactor) << boost::str (boost::format ("bValidOffer: BAD OFFER: saOfferPays=%s saOfferGets=%s")
                 % saOfferPays % saOfferGets);
 
-        usOfferUnfundedFound.insert (uOfferIndex);
+        usOfferUnfundedFound.insert (sleOffer->getIndex());
 
         bValid = false;
     }
@@ -78,12 +77,12 @@ bool OfferCreateTransactor::bValidOffer (
             if (account != usAccountTouched.end ())
             {
                 // Previously touched account.
-                usOfferUnfundedBecame.insert (uOfferIndex); // Delete unfunded offer on success.
+                usOfferUnfundedBecame.insert (sleOffer->getIndex()); // Delete unfunded offer on success.
             }
             else
             {
                 // Never touched source account.
-                usOfferUnfundedFound.insert (uOfferIndex);  // Delete found unfunded offer when possible.
+                usOfferUnfundedFound.insert (sleOffer->getIndex());  // Delete found unfunded offer when possible.
             }
 
             bValid  = false;
@@ -130,8 +129,6 @@ TER OfferCreateTransactor::takeOffers (
     WriteLog (lsDEBUG, OfferCreateTransactor) << "takeOffers: bSell: " << bSell << ": against book: " << uBookBase.ToString ();
 
     LedgerEntrySet&         lesActive           = mEngine->getNodes ();
-    uint256                 uTipIndex           = uBookBase;
-    const uint256           uBookEnd            = Ledger::getQualityNext (uBookBase);
     const uint64            uTakeQuality        = STAmount::getRate (saTakerGets, saTakerPays);
     STAmount                saTakerRate         = STAmount::setRate (uTakeQuality);
     const uint160           uTakerPaysAccountID = saTakerPays.getIssuer ();
@@ -145,57 +142,35 @@ TER OfferCreateTransactor::takeOffers (
     saTakerGot      = STAmount (saTakerGets.getCurrency (), saTakerGets.getIssuer ());
     bUnfunded       = false;
 
-    while (temUNCERTAIN == terResult)
+    OrderBookIterator bookIterator (lesActive,
+        saTakerPays.getCurrency(), saTakerPays.getIssuer(),
+        saTakerGets.getCurrency(), saTakerGets.getIssuer());
+
+    while ((temUNCERTAIN == terResult) && bookIterator.nextOffer())
     {
-        SLE::pointer    sleOfferDir;
-        uint64          uTipQuality     = 0;
         STAmount        saTakerFunds    = lesActive.accountFunds (uTakerAccountID, saTakerPays);
         STAmount        saSubTakerPays  = saTakerPays - saTakerPaid; // How much more to spend.
         STAmount        saSubTakerGets  = saTakerGets - saTakerGot; // How much more is wanted.
+        uint64          uTipQuality = bookIterator.getCurrentQuality();
 
-        // Figure out next offer to take, if needed.
+        if (!saTakerFunds.isPositive ())
-        if (saTakerFunds.isPositive ()          // Taker has funds available.
-                && saSubTakerPays.isPositive ()
-                && saSubTakerGets.isPositive ())
         {
-            sleOfferDir     = mEngine->entryCache (ltDIR_NODE, lesActive.getNextLedgerIndex (uTipIndex, uBookEnd));
+            // Taker is out of funds. Don't create the offer.
-
+            bUnfunded = true;
-            if (sleOfferDir)
-            {
-                uTipIndex       = sleOfferDir->getIndex ();
-                uTipQuality     = Ledger::getQuality (uTipIndex);
-
-                WriteLog (lsDEBUG, OfferCreateTransactor) << boost::str (boost::format ("takeOffers: possible counter offer found: uTipQuality=%d uTipIndex=%s")
-                        % uTipQuality
-                        % uTipIndex.ToString ());
-
-            }
-            else
-            {
-                WriteLog (lsTRACE, OfferCreateTransactor) << "takeOffers: counter offer book is empty: "
-                        << uTipIndex.ToString ()
-                        << " ... "
-                        << uBookEnd.ToString ();
-            }
-        }
-
-        if (!saTakerFunds.isPositive ())                    // Taker has no funds.
-        {
-            // Done. Ran out of funds on previous round. As fees aren't calculated directly in this routine, funds are checked here.
-            WriteLog (lsDEBUG, OfferCreateTransactor) << "takeOffers: done: taker unfunded.";
-
-            bUnfunded   = true;                             // Don't create an order.
             terResult = tesSUCCESS;
         }
-        else if (!sleOfferDir                               // No offer directory to take.
+        else if (!saSubTakerPays.isPositive() || !saSubTakerGets.isPositive())
-                 || uTakeQuality < uTipQuality                   // No offers of sufficient quality available.
+        {
+            // Offer is completely consumed
+            terResult = tesSUCCESS;
+        }
+        else if ((uTakeQuality < uTipQuality)
                  || (bPassive && uTakeQuality == uTipQuality))
         {
-            // Done.
+            // Offer does not cross this offer
-            STAmount    saTipRate           = sleOfferDir ? STAmount::setRate (uTipQuality) : saTakerRate;
+            STAmount    saTipRate           = STAmount::setRate (uTipQuality);
 
-            WriteLog (lsDEBUG, OfferCreateTransactor) << boost::str (boost::format ("takeOffers: done: dir=%d uTakeQuality=%d %c uTipQuality=%d saTakerRate=%s %c saTipRate=%s bPassive=%d")
+            WriteLog (lsDEBUG, OfferCreateTransactor) << boost::str (boost::format ("takeOffers: done: uTakeQuality=%d %c uTipQuality=%d saTakerRate=%s %c saTipRate=%s bPassive=%d")
-                    % !!sleOfferDir
                     % uTakeQuality
                     % (uTakeQuality == uTipQuality
                        ? '='
@@ -216,22 +191,22 @@ TER OfferCreateTransactor::takeOffers (
         }
         else
         {
-            // Have an offer directory to consider.
+            // We have a crossing offer to consider.
-            WriteLog (lsTRACE, OfferCreateTransactor) << "takeOffers: considering dir: " << sleOfferDir->getJson (0);
 
-            SLE::pointer    sleBookNode;
+            SLE::pointer    sleOffer        = bookIterator.getCurrentOffer ();
-            unsigned int    uBookEntry;
-            uint256         uOfferIndex;
 
-            lesActive.dirFirst (uTipIndex, sleBookNode, uBookEntry, uOfferIndex);
+            if (!sleOffer)
-
+            { // offer is in directory but not in ledger
-            SLE::pointer    sleOffer        = mEngine->entryCache (ltOFFER, uOfferIndex);
+                uint256 offerIndex = bookIterator.getCurrentIndex ();
-
+                WriteLog (lsWARNING, OfferCreateTransactor) << "takeOffers: offer not found : " << offerIndex;
-            if (sleOffer)
+                usMissingOffers.insert (missingOffer_t (
+                    bookIterator.getCurrentIndex (), bookIterator.getCurrentDirectory ()));
+            }
+            else
             {
                 WriteLog (lsDEBUG, OfferCreateTransactor) << "takeOffers: considering offer : " << sleOffer->getJson (0);
 
-                const uint160   uOfferOwnerID   = sleOffer->getFieldAccount160 (sfAccount);
+                const uint160&  uOfferOwnerID   = sleOffer->getFieldAccount160 (sfAccount);
                 STAmount        saOfferPays     = sleOffer->getFieldAmount (sfTakerGets);
                 STAmount        saOfferGets     = sleOffer->getFieldAmount (sfTakerPays);
 
@@ -239,7 +214,7 @@ TER OfferCreateTransactor::takeOffers (
                 bool            bValid;
 
                 bValid  =  bValidOffer (
-                               sleOffer, uOfferIndex, uOfferOwnerID, saOfferPays, saOfferGets,
+                               sleOffer, uOfferOwnerID, saOfferPays, saOfferGets,
                                uTakerAccountID,
                                usOfferUnfundedFound, usOfferUnfundedBecame, usAccountTouched,
                                saOfferFunds);
@@ -298,7 +273,7 @@ TER OfferCreateTransactor::takeOffers (
                         // Offer now fully claimed or now unfunded.
                         WriteLog (lsDEBUG, OfferCreateTransactor) << "takeOffers: Offer claimed: Delete.";
 
-                        usOfferUnfundedBecame.insert (uOfferIndex); // Delete unfunded offer on success.
+                        usOfferUnfundedBecame.insert (sleOffer->getIndex()); // Delete unfunded offer on success.
 
                         // Offer owner's account is no longer pristine.
                         usAccountTouched.insert (uOfferOwnerID);
@@ -370,14 +345,12 @@ TER OfferCreateTransactor::takeOffers (
                     }
                 }
             }
-            else
-            {
-                WriteLog (lsWARNING, OfferCreateTransactor) << "missing offer";
-                // WRITEME: Remove the missing offer from the directory
-            }
         }
     }
 
+    if (temUNCERTAIN == terResult)
+        terResult = tesSUCCESS;
+
     WriteLog (lsDEBUG, OfferCreateTransactor) << "takeOffers: " << transToken (terResult);
 
     if (tesSUCCESS == terResult)
@@ -522,7 +495,7 @@ TER OfferCreateTransactor::doApply ()
         {
             WriteLog (lsWARNING, OfferCreateTransactor) << "OfferCreate: uCancelSequence=" << uCancelSequence;
 
-            terResult   = mEngine->getNodes ().offerDelete (sleCancel, uCancelIndex, mTxnAccountID);
+            terResult   = mEngine->getNodes ().offerDelete (sleCancel);
         }
         else
         {
@@ -627,8 +600,8 @@ TER OfferCreateTransactor::doApply ()
         lesActive.swapWith (lesCheckpoint);                                 // Restore with just fees paid.
     }
     else if (
-        !saTakerPays                                                        // Wants nothing more.
+        !saTakerPays.isPositive()                                           // Wants nothing more.
-        || !saTakerGets                                                     // Offering nothing more.
+        || !saTakerGets.isPositive()                                        // Offering nothing more.
         || bImmediateOrCancel                                               // Do not persist.
         || !lesActive.accountFunds (mTxnAccountID, saTakerGets).isPositive () // Not funded.
         || bUnfunded)                                                       // Consider unfunded.
@@ -729,6 +702,8 @@ TER OfferCreateTransactor::doApply ()
     // On storing meta data, delete offers that were found unfunded to prevent encountering them in future.
     if (tesSUCCESS == terResult)
     {
+
+        // Go through the list of unfunded offers and remove them
         BOOST_FOREACH (uint256 const & uOfferIndex, usOfferUnfundedFound)
         {
 
@@ -739,6 +714,36 @@ TER OfferCreateTransactor::doApply ()
             if (tesSUCCESS != terResult)
                 break;
         }
+
+        // Go through the list of offers not found and remove them from the order book
+        BOOST_FOREACH (missingOffer_t const& indexes, usMissingOffers)
+        {
+            SLE::pointer sleDirectory = lesActive.entryCache (ltDIR_NODE, indexes.second);
+            if (sleDirectory)
+            {
+                STVector256 svIndexes = sleDirectory->getFieldV256 (sfIndexes);
+                std::vector<uint256>& vuiIndexes = svIndexes.peekValue();
+                std::vector<uint256>::iterator it = std::find (vuiIndexes.begin(), vuiIndexes.end (), indexes.first);
+                if (it != vuiIndexes.end ())
+                {
+                    vuiIndexes.erase (it);
+                    sleDirectory->setFieldV256 (sfIndexes, svIndexes);
+                    lesActive.entryModify (sleDirectory);
+                    WriteLog (lsWARNING, OfferCreateTransactor) << "takeOffers: offer " << indexes.first <<
+                        " removed from directory " << indexes.second;
+                }
+                else
+                {
+                    WriteLog (lsINFO, OfferCreateTransactor) << "takeOffers: offer " << indexes.first <<
+                        " not found in directory " << indexes.second;
+                }
+            }
+            else
+            {
+                WriteLog (lsWARNING, OfferCreateTransactor) << "takeOffers: directory " << indexes.second <<
+                    " not found for offer " << indexes.first;
+            }
+        }
     }
 
     CondLog (tesSUCCESS != terResult, lsINFO, OfferCreateTransactor) << boost::str (boost::format ("OfferCreate: final terResult=%s") % transToken (terResult));

src/ripple_app/tx/OfferCreateTransactor.h

@@ -29,7 +29,6 @@ public:
 private:
     bool bValidOffer (
         SLE::ref            sleOfferDir,
-        uint256 const&      uOffer,
         const uint160&      uOfferOwnerID,
         const STAmount&     saOfferPays,
         const STAmount&     saOfferGets,
@@ -54,6 +53,8 @@ private:
 
     boost::unordered_set<uint256>   usOfferUnfundedFound;   // Offers found unfunded.
 
+    typedef std::pair <uint256, uint256> missingOffer_t;
+    boost::unordered_set<missingOffer_t> usMissingOffers;
 };
 
 #endif