Fix OpenView::sles

src/ripple/ledger/ReadView.h

@@ -366,10 +366,8 @@ public:
 
     /** Iterable range of ledger state items.
 
-        Visiting each state entry in the ledger can be
+        @note Visiting each state entry in the ledger can
-        quite expensive. This will only visit entries in
+              become quite expensive as the ledger grows.
-        the base ledger. Entries in open views, apply
-        views, or sandboxes will not be visited.
     */
     sles_type sles;
 

src/ripple/ledger/detail/RawStateTable.h

@@ -77,6 +77,12 @@ public:
     void
     destroyXRP (std::uint64_t feeDrops);
 
+    std::unique_ptr<ReadView::sles_type::iter_base>
+    slesBegin (ReadView const& base) const;
+
+    std::unique_ptr<ReadView::sles_type::iter_base>
+    slesEnd (ReadView const& base) const;
+
 private:
     enum class Action
     {
@@ -85,6 +91,8 @@ private:
         replace,
     };
 
+    class sles_iter_impl;
+
     using items_t = std::map<key_type,
         std::pair<Action, std::shared_ptr<SLE>>>;
 

src/ripple/ledger/impl/OpenView.cpp

@@ -25,8 +25,6 @@ namespace ripple {
 
 open_ledger_t const open_ledger {};
 
-//------------------------------------------------------------------------------
-
 class OpenView::txs_iter_impl
     : public txs_type::iter_base
 {
@@ -170,14 +168,14 @@ auto
 OpenView::slesBegin() const ->
     std::unique_ptr<sles_type::iter_base>
 {
-    return base_->slesBegin();
+    return items_.slesBegin(*base_);
 }
 
 auto
 OpenView::slesEnd() const ->
     std::unique_ptr<sles_type::iter_base>
 {
-    return base_->slesEnd();
+    return items_.slesEnd(*base_);
 }
 
 auto

src/ripple/ledger/impl/RawStateTable.cpp

@@ -24,6 +24,134 @@
 namespace ripple {
 namespace detail {
 
+class RawStateTable::sles_iter_impl
+    : public ReadView::sles_type::iter_base
+{
+private:
+    std::shared_ptr<SLE const> sle0_;
+    ReadView::sles_type::iterator iter0_;
+    ReadView::sles_type::iterator end0_;
+    std::shared_ptr<SLE const> sle1_;
+    items_t::const_iterator iter1_;
+    items_t::const_iterator end1_;
+
+public:
+    sles_iter_impl (sles_iter_impl const&) = default;
+
+    sles_iter_impl (items_t::const_iterator iter1,
+        items_t::const_iterator end1,
+            ReadView::sles_type::iterator iter0,
+                ReadView::sles_type::iterator base0)
+        : iter0_ (iter0)
+        , end0_ (base0)
+        , iter1_ (iter1)
+        , end1_ (end1)
+    {
+        if (iter0_ != end0_)
+            sle0_ = *iter0_;
+        if (iter1_ != end1)
+        {
+            sle1_ = iter1_->second.second;
+            skip ();
+        }
+    }
+
+    std::unique_ptr<base_type>
+    copy() const override
+    {
+        return std::make_unique<sles_iter_impl>(*this);
+    }
+
+    bool
+    equal (base_type const& impl) const override
+    {
+        auto const& other = dynamic_cast<
+            sles_iter_impl const&>(impl);
+        assert(end1_ == other.end1_ &&
+            end0_ == other.end0_);
+        return iter1_ == other.iter1_ &&
+            iter0_ == other.iter0_;
+    }
+
+    void
+    increment() override
+    {
+        assert (sle1_ || sle0_);
+
+        if (sle1_ && !sle0_)
+        {
+            inc1();
+            return;
+        }
+
+        if (sle0_ && !sle1_)
+        {
+            inc0();
+            return;
+        }
+
+        if (sle1_->key () == sle0_->key())
+        {
+            inc1();
+            inc0();
+        }
+        else if (sle1_->key () < sle0_->key())
+        {
+            inc1();
+        }
+        else
+        {
+            inc0();
+        }
+        skip();
+    }
+
+    value_type
+    dereference() const override
+    {
+        if (! sle1_)
+            return sle0_;
+        else if (! sle0_)
+            return sle1_;
+        if (sle1_->key() <= sle0_->key())
+            return sle1_;
+        return sle0_;
+    }
+private:
+    void inc0()
+    {
+        ++iter0_;
+        if (iter0_ == end0_)
+            sle0_ = nullptr;
+        else
+            sle0_ = *iter0_;
+    }
+    
+    void inc1()
+    {
+        ++iter1_;
+        if (iter1_ == end1_)
+            sle1_ = nullptr;
+        else
+            sle1_ = iter1_->second.second;
+    }
+    
+    void skip()
+    {
+        while (iter1_ != end1_ &&
+            iter1_->second.first == Action::erase &&
+               sle0_->key() == sle1_->key())
+        {
+            inc1();
+            inc0();
+            if (! sle0_)
+                return;
+        }
+    }
+};
+
+//------------------------------------------------------------------------------
+
 // Base invariants are checked by the base during apply()
 
 void
@@ -210,5 +338,21 @@ RawStateTable::destroyXRP(std::uint64_t feeDrops)
     dropsDestroyed_ += feeDrops;
 }
 
+std::unique_ptr<ReadView::sles_type::iter_base>
+RawStateTable::slesBegin (ReadView const& base) const
+{
+    return std::make_unique<sles_iter_impl>(
+        items_.begin(), items_.end(),
+            base.sles.begin(), base.sles.end());
+}
+
+std::unique_ptr<ReadView::sles_type::iter_base>
+RawStateTable::slesEnd (ReadView const& base) const
+{
+    return std::make_unique<sles_iter_impl>(
+        items_.end(), items_.end(),
+            base.sles.end(), base.sles.end());
+}
+
 } // detail
 } // ripple

src/ripple/ledger/tests/View_test.cpp

@@ -146,8 +146,8 @@ class View_test
     void
     testLedger()
     {
-        Config const config;
         using namespace jtx;
+        Config const config;
         std::shared_ptr<Ledger const> const genesis =
             std::make_shared<Ledger>(
                 create_genesis, config);
@@ -385,6 +385,101 @@ class View_test
         }
     }
 
+    // Return a list of keys found via sles
+    static
+    std::vector<uint256>
+    sles (ReadView const& ledger)
+    {
+        std::vector<uint256> v;
+        v.reserve (32);
+        for(auto const& sle : ledger.sles)
+            v.push_back(sle->key());
+        return v;
+    }
+
+    template <class... Args>
+    static
+    std::vector<uint256>
+    list (Args... args)
+    {
+        return std::vector<uint256> ({uint256(args)...});
+    }
+
+    void
+    testSles()
+    {
+        using namespace jtx;
+        Config const config;
+        std::shared_ptr<Ledger const> const genesis =
+            std::make_shared<Ledger> (
+                create_genesis, config);
+        auto const ledger =
+            std::make_shared<Ledger> (
+                open_ledger, *genesis);
+        auto setup123 = [&ledger, this]()
+        {
+            // erase middle element
+            wipe (*ledger);
+            ledger->rawInsert (sle (1));
+            ledger->rawInsert (sle (2));
+            ledger->rawInsert (sle (3));
+            expect (sles (*ledger) == list (1, 2, 3));
+        };
+        {
+            setup123 ();
+            OpenView view (ledger.get ());
+            view.rawErase (sle (1));
+            view.rawInsert (sle (4));
+            view.rawInsert (sle (5));
+            expect (sles (view) == list (2, 3, 4, 5));
+        }
+        {
+            setup123 ();
+            OpenView view (ledger.get ());
+            view.rawErase (sle (1));
+            view.rawErase (sle (2));
+            view.rawInsert (sle (4));
+            view.rawInsert (sle (5));
+            expect (sles (view) == list (3, 4, 5));
+        }
+        {
+            setup123 ();
+            OpenView view (ledger.get ());
+            view.rawErase (sle (1));
+            view.rawErase (sle (2));
+            view.rawErase (sle (3));
+            view.rawInsert (sle (4));
+            view.rawInsert (sle (5));
+            expect (sles (view) == list (4, 5));
+        }
+        {
+            setup123 ();
+            OpenView view (ledger.get ());
+            view.rawErase (sle (3));
+            view.rawInsert (sle (4));
+            view.rawInsert (sle (5));
+            expect (sles (view) == list (1, 2, 4, 5));
+        }
+        {
+            setup123 ();
+            OpenView view (ledger.get ());
+            view.rawReplace (sle (1, 10));
+            view.rawReplace (sle (3, 30));
+            expect (sles (view) == list (1, 2, 3));
+            expect (seq (view.read(k (1))) == 10);
+            expect (seq (view.read(k (2))) == 1);
+            expect (seq (view.read(k (3))) == 30);
+
+            view.rawErase (sle (3));
+            expect (sles (view) == list (1, 2));
+
+            view.rawInsert (sle (5));
+            view.rawInsert (sle (4));
+            view.rawInsert (sle (3));
+            expect (sles (view) == list (1, 2, 3, 4, 5));
+        }
+    }
+
     void
     testRegressions()
     {
@@ -424,7 +519,7 @@ class View_test
         testMetaSucc();
         testStacked();
         testContext();
-
+        testSles();
         testRegressions();
     }
 };