revise destroyNamespace logic

src/ripple/app/tx/impl/SetHook.cpp

@@ -695,17 +695,6 @@ SetHook::destroyNamespace(
         << "HookSet(" << hook::log::NSDELETE << ")[" << HS_ACC() << "]: DeleteState "
         << "Destroying Hook Namespace for " << account << " namespace " << ns;
     
-    Keylet dirKeylet = keylet::hookStateDir(account, ns);
-
-    std::shared_ptr<SLE const> sleDirNode{};
-    unsigned int uDirEntry{0};
-    uint256 dirEntry{beast::zero};
-
-    auto sleDir = view.peek(dirKeylet);
-    
-    if (!sleDir || dirIsEmpty(view, dirKeylet))
-        return tesSUCCESS;
-
     auto sleAccount = view.peek(keylet::account(account));
     if (!sleAccount)
     {
@@ -716,6 +705,83 @@ SetHook::destroyNamespace(
     }
 
 
+    bool sleAccChanged = false;
+
+    if (!sleAccount->isFieldPresent(sfHookNamespaces))
+    {
+        // NSDELETE is an opportunistic deleter, following "delete if exists" logic
+        // this way the flag can't block the SetHook transaction just because, for example, the namespace was
+        // deleted in the previous transaction but the hsFlags have not changed.
+        // Thus this is not an error.
+        // Allow fall through to below in case, for some reason, the namespace directory *does* exist
+        // but does not appear in the vector.
+    }
+    else
+    {
+        STVector256 const& vec = sleAccount->getFieldV256(sfHookNamespaces);
+        if (vec.size() == 0)
+        {
+            // clean up structure if it's present but empty
+            sleAccount->makeFieldAbsent(sfHookNamespaces);
+            sleAccChanged = true;
+        }
+        else
+        {
+            // defensively ensure the uniqueness of the namespace array
+            std::set<uint256> spaces;
+
+            for (auto u : vec.value())
+                if (u != ns)
+                    spaces.emplace(u);
+
+            // drop through if it wasn't present (see comment block 20 lines above)
+            if (spaces.size() != vec.size())
+            {
+                sleAccChanged = true;
+
+                if (spaces.size() == 0)
+                    sleAccount->makeFieldAbsent(sfHookNamespaces);
+                else
+                {
+                    std::vector<uint256> nv;
+                    nv.reserve(spaces.size());
+
+                    for (auto u : spaces)
+                        nv.push_back(u);
+
+                    sleAccount->setFieldV256(sfHookNamespaces, STVector256 { std::move(nv) } );
+                }
+            }
+        }
+    }
+
+    Keylet dirKeylet = keylet::hookStateDir(account, ns);
+
+    std::shared_ptr<SLE const> sleDirNode{};
+    unsigned int uDirEntry{0};
+    uint256 dirEntry{beast::zero};
+
+    auto sleDir = view.peek(dirKeylet);
+
+    if (!sleDir)
+    {
+        // directory doesn't exist, this is a success condition
+        if (sleAccChanged)
+            view.update(sleAccount);
+        return tesSUCCESS;
+    }
+
+    if (dirIsEmpty(view, dirKeylet))
+    {
+        // directory exists but is empty, so remove the root page
+        if (sleAccChanged)
+            view.update(sleAccount);
+
+        view.erase(sleDir);
+        return tesSUCCESS;
+    }
+
+    // fall through to here means we must prune the entries from the directory
     if (!cdirFirst(
             view,
             dirKeylet.key,
@@ -761,7 +827,6 @@ SetHook::destroyNamespace(
             return tefBAD_LEDGER;
         }
 
-
         toDelete.push_back(uint256::fromVoid(itemKeylet.key.data()));
 
     } while (cdirNext(view, dirKeylet.key, sleDirNode, uDirEntry, dirEntry));
@@ -769,7 +834,6 @@ SetHook::destroyNamespace(
     // delete it!
     for (auto const& itemKey: toDelete)
     {
-
         auto const& sleItem = view.peek({ltHOOK_STATE, itemKey});
 
         if (!sleItem)
@@ -807,22 +871,6 @@ SetHook::destroyNamespace(
 
     sleAccount->setFieldU32(sfHookStateCount, stateCount);
 
-    STVector256 const& vec = sleAccount->getFieldV256(sfHookNamespaces);
-    if (vec.size() - 1 == 0)
-    {
-        sleAccount->makeFieldAbsent(sfHookNamespaces);
-    }
-    else
-    {
-        std::vector<uint256> nv { vec.size() - 1 };
-    
-        for (uint256 u : vec.value())
-            if (u != ns)
-                nv.push_back(u);
-
-        sleAccount->setFieldV256(sfHookNamespaces, STVector256 { std::move(nv) } );
-    }
-
     view.update(sleAccount);
 
     return tesSUCCESS;