Reduce MagicEnum usage in server definitions

Use existing type, ledger, transaction, and transaction-result registries to build server definitions. This removes custom enum ranges and duplicated name translation logic.

build-core.sh

@@ -95,8 +95,16 @@ if [[ "$4" == "" ]]; then
   echo "Non GH, local building, no Action runner magic"
 else
   # GH Action, runner
-  cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
-  cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
+    if [[ "$(git rev-parse --abbrev-ref HEAD)" == "release" ]]; then
+        echo "building on the release branch... placing it in builds/candidate"
+        mkdir /data/builds/candidate
+        cp /io/release-build/xahaud /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
+        cp /io/release-build/release.info /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
+    else
+        echo "building non-release branch, placing it in builds root"
+        cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
+        cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
+    fi  
   echo "Published build to: http://build.xahau.tech/"
   echo $(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
 fi

include/xrpl/protocol/detail/features.macro

@@ -34,6 +34,7 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
+XRPL_FIX    (HookMap,                    Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (GuardDepth32,               Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(NamedHooks,                 Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(IOURewardClaim,             Supported::yes, VoteBehavior::DefaultNo)

src/test/app/HookAPI_test.cpp

@@ -3632,8 +3632,9 @@ public:
 
         auto const alice = Account{"alice"};
         auto const bob = Account{"bob"};
+        auto const claire = Account{"claire"};
         Env env{*this, features};
-        env.fund(XRP(100000), alice, bob);
+        env.fund(XRP(100000), alice, bob, claire);
         env.close();
 
         // Compute hook hash for the accept hook
@@ -3773,6 +3774,84 @@ public:
             BEAST_EXPECT(result2.has_value());
             BEAST_EXPECT(result2.value() == newData.size());
         }
+
+        {
+            // fixHookMap: foreign state set without grant after state previous
+            // modified
+
+            HookStateMap stateMap;
+            auto hookCtx = makeStubHookContext(
+                applyCtx, alice.id(), bob.id(), {}, stateMap);
+
+            AccountID const aliceid = alice.id();
+
+            // Pre-populate stateMap
+            stateMap[alice.id()] = {
+                100,  // availableForReserves
+                1,    // namespaceCount
+                1,    // hookStateScale
+                {}};
+
+            auto& api = hookCtx.api();
+
+            // setup a hook on alice, and on claire, no grants
+            env(hook(alice, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1)));
+            env(hook(claire, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1)));
+            env.close();
+
+            // First modification
+            auto result1 =
+                api.state_foreign_set(testKey, testNs, aliceid, testData);
+            BEAST_EXPECT(result1.has_value());
+
+            // Second modification this time using bob as hookacc (should hit
+            // cache)
+            auto hookCtx2 = makeStubHookContext(
+                applyCtx, claire.id(), bob.id(), {}, stateMap);
+
+            // check the state entry is carried into the second context
+
+            // does the map contain the account?
+            BEAST_EXPECT(
+                hookCtx2.result.stateMap.find(aliceid) !=
+                hookCtx2.result.stateMap.end());
+
+            // the name space?
+            BEAST_EXPECT(
+                std::get<3>(hookCtx2.result.stateMap[aliceid]).find(testNs) !=
+                std::get<3>(hookCtx2.result.stateMap[aliceid]).end());
+
+            // the key entry?
+            BEAST_EXPECT(
+                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs].find(
+                    testKey) !=
+                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs].end());
+
+            // is the entry marked as modified?
+            BEAST_EXPECT(
+                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs][testKey]
+                    .first);
+
+            auto& api2 = hookCtx2.api();
+            Bytes newData{0x04, 0x05};
+            auto result2 =
+                api2.state_foreign_set(testKey, testNs, aliceid, newData);
+
+            if (features[fixHookMap])
+            {
+                // new behaviour: grant is missing, cannot write
+                BEAST_EXPECT(!result2.has_value());
+                BEAST_EXPECT(result2.error() == NOT_AUTHORIZED);
+                BEAST_EXPECT(hookCtx2.result.foreignStateSetDisabled);
+            }
+            else
+            {
+                // old behaviour: allow this illegal write due to the entry
+                // being modified previously in the map
+                BEAST_EXPECT(result2.has_value());
+                BEAST_EXPECT(result2.value() == newData.size());
+            }
+        }
     }
 
     void
@@ -4832,6 +4911,7 @@ public:
 
         test_state(features);
         test_state_foreign(features);
+        test_state_foreign_set(features - fixHookMap);
         test_state_foreign_set(features);
         test_state_foreign_set_max(features);
         test_state_set(features);

src/xrpld/app/hook/applyHook.h

@@ -15,6 +15,7 @@
 #include <memory>
 #include <optional>
 #include <queue>
+#include <utility>
 #include <vector>
 #include <wasmedge/wasmedge.h>
 
@@ -174,6 +175,8 @@ struct HookResult
         false;  // hook_again allows strong pre-apply to nominate
                 // additional weak post-apply execution
     std::shared_ptr<STObject const> provisionalMeta;
+    std::set<std::pair<AccountID, uint256 /* namespace */>>
+        foreignStateGrantCache;  // add found grants here to avoid rechecking
 };
 
 class HookExecutor;

src/xrpld/app/hook/detail/HookAPI.cpp

@@ -1920,88 +1920,114 @@ HookAPI::state_foreign_set(
     if (hookCtx.result.foreignStateSetDisabled)
         return Unexpected(PREVIOUS_FAILURE_PREVENTS_RETRY);
 
-    // first check if we've already modified this state
-    auto cacheEntry = lookup_state_cache(account, ns, key);
-    if (cacheEntry && cacheEntry->get().first)
-    {
-        // if a cache entry already exists and it has already been modified
-        // don't check grants again
-        if (auto ret = set_state_cache(account, ns, key, data, true);
-            !ret.has_value())
-            return Unexpected(ret.error());
+    bool const hasFix = hookCtx.applyCtx.view().rules().enabled(fixHookMap);
 
-        return data.size();
+    if (!hasFix)
+    {
+        // first check if we've already modified this state
+        auto cacheEntry = lookup_state_cache(account, ns, key);
+        if (cacheEntry && cacheEntry->get().first)
+        {
+            // if a cache entry already exists and it has already been modified
+            // don't check grants again
+            if (auto ret = set_state_cache(account, ns, key, data, true);
+                !ret.has_value())
+                return Unexpected(ret.error());
+
+            return data.size();
+        }
     }
-
-    // cache miss or cache was present but entry was not marked as previously
-    // modified therefore before continuing we need to check grants
-    auto const sle =
-        hookCtx.applyCtx.view().read(ripple::keylet::hook(account));
-    if (!sle)
-        return Unexpected(INTERNAL_ERROR);
-
-    bool found_auth = false;
-
-    // we do this by iterating the hooks installed on the foreign account and in
-    // turn their grants and namespaces
-    auto const& hooks = sle->getFieldArray(sfHooks);
-    for (auto const& hookObj : hooks)
+    // check if we've used a grant to modify this state entry before, if not
+    // look up possible grants
+    if (!hasFix ||
+        hookCtx.result.foreignStateGrantCache.find({account, ns}) ==
+            hookCtx.result.foreignStateGrantCache.end())
     {
-        // skip blank entries
-        if (!hookObj.isFieldPresent(sfHookHash))
-            continue;
+        auto const sle =
+            hookCtx.applyCtx.view().read(ripple::keylet::hook(account));
 
-        if (!hookObj.isFieldPresent(sfHookGrants))
-            continue;
-
-        auto const& hookGrants = hookObj.getFieldArray(sfHookGrants);
-
-        if (hookGrants.size() < 1)
-            continue;
-
-        // the grant allows the hook to modify the granter's namespace only
-        if (hookObj.isFieldPresent(sfHookNamespace))
+        if (!sle)
         {
-            if (hookObj.getFieldH256(sfHookNamespace) != ns)
-                continue;
-        }
-        else
-        {
-            // fetch the hook definition
-            auto const def =
-                hookCtx.applyCtx.view().read(ripple::keylet::hookDefinition(
-                    hookObj.getFieldH256(sfHookHash)));
-            if (!def)  // should never happen except in a rare race condition
-                continue;
-            if (def->getFieldH256(sfHookNamespace) != ns)
-                continue;
-        }
-
-        // this is expensive search so we'll disallow after one failed attempt
-        for (auto const& hookGrantObj : hookGrants)
-        {
-            bool hasAuthorizedField = hookGrantObj.isFieldPresent(sfAuthorize);
-
-            if (hookGrantObj.getFieldH256(sfHookHash) ==
-                    hookCtx.result.hookHash &&
-                (!hasAuthorizedField ||
-                 hookGrantObj.getAccountID(sfAuthorize) ==
-                     hookCtx.result.account))
+            if (hasFix)
             {
-                found_auth = true;
-                break;
+                hookCtx.result.foreignStateSetDisabled = true;
+                return Unexpected(NOT_AUTHORIZED);
             }
+
+            return Unexpected(INTERNAL_ERROR);
         }
 
-        if (found_auth)
-            break;
-    }
+        // RH TODO: test this code path more completely
 
-    if (!found_auth)
-    {
-        // hook only gets one attempt
-        hookCtx.result.foreignStateSetDisabled = true;
-        return Unexpected(NOT_AUTHORIZED);
+        bool found_auth = false;
+
+        // we do this by iterating the hooks installed on the foreign account
+        // and in turn their grants and namespaces
+        auto const& hooks = sle->getFieldArray(sfHooks);
+        for (auto const& hookObj : hooks)
+        {
+            // skip blank entries
+            if (!hookObj.isFieldPresent(sfHookHash))
+                continue;
+
+            if (!hookObj.isFieldPresent(sfHookGrants))
+                continue;
+
+            auto const& hookGrants = hookObj.getFieldArray(sfHookGrants);
+
+            if (hookGrants.size() < 1)
+                continue;
+
+            // the grant allows the hook to modify the granter's namespace only
+            if (hookObj.isFieldPresent(sfHookNamespace))
+            {
+                if (hookObj.getFieldH256(sfHookNamespace) != ns)
+                    continue;
+            }
+            else
+            {
+                // fetch the hook definition
+                auto const def =
+                    hookCtx.applyCtx.view().read(ripple::keylet::hookDefinition(
+                        hookObj.getFieldH256(sfHookHash)));
+                if (!def)  // should never happen except in a rare race
+                           // condition
+                    continue;
+                if (def->getFieldH256(sfHookNamespace) != ns)
+                    continue;
+            }
+
+            // this is expensive search so we'll disallow after one failed
+            // attempt
+            for (auto const& hookGrantObj : hookGrants)
+            {
+                bool hasAuthorizedField =
+                    hookGrantObj.isFieldPresent(sfAuthorize);
+
+                if (hookGrantObj.getFieldH256(sfHookHash) ==
+                        hookCtx.result.hookHash &&
+                    (!hasAuthorizedField ||
+                     hookGrantObj.getAccountID(sfAuthorize) ==
+                         hookCtx.result.account))
+                {
+                    found_auth = true;
+                    break;
+                }
+            }
+
+            if (found_auth)
+                break;
+        }
+
+        if (!found_auth)
+        {
+            // hook only gets one attempt
+            hookCtx.result.foreignStateSetDisabled = true;
+            return Unexpected(NOT_AUTHORIZED);
+        }
+
+        // add the grant to the cache
+        hookCtx.result.foreignStateGrantCache.emplace(account, ns);
     }
 
     if (auto ret = set_state_cache(account, ns, key, data, true);

src/xrpld/rpc/handlers/ServerDefinitions.cpp

@@ -22,12 +22,8 @@
 #include <xrpld/app/main/Application.h>
 #include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/NetworkOPs.h>
-#include <xrpld/rpc/detail/TransactionSign.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/json_writer.h>
-#include <xrpl/protocol/LedgerFormats.h>
-#include <xrpl/protocol/RPCErr.h>
-#include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/digest.h>
 #include <xrpl/protocol/jss.h>
@@ -35,14 +31,6 @@
 #include <magic_enum.hpp>
 #include <sstream>
 
-#define MAGIC_ENUM(x, _min, _max)               \
-    template <>                                 \
-    struct magic_enum::customize::enum_range<x> \
-    {                                           \
-        static constexpr int min = _min;        \
-        static constexpr int max = _max;        \
-    };
-
 #define MAGIC_ENUM_16(x)                        \
     template <>                                 \
     struct magic_enum::customize::enum_range<x> \
@@ -58,15 +46,6 @@
         static constexpr bool is_flags = true;  \
     };
 
-MAGIC_ENUM(ripple::SerializedTypeID, -2, 10004);
-MAGIC_ENUM(ripple::LedgerEntryType, 0, 255);
-MAGIC_ENUM(ripple::TELcodes, -399, 300);
-MAGIC_ENUM(ripple::TEMcodes, -299, -200);
-MAGIC_ENUM(ripple::TEFcodes, -199, -100);
-MAGIC_ENUM(ripple::TERcodes, -99, -1);
-MAGIC_ENUM(ripple::TEScodes, 0, 1);
-MAGIC_ENUM(ripple::TECcodes, 100, 255);
-MAGIC_ENUM_16(ripple::TxType);
 MAGIC_ENUM_FLAG(ripple::UniversalFlags);
 MAGIC_ENUM_FLAG(ripple::AccountSetFlags);
 MAGIC_ENUM_FLAG(ripple::OfferCreateFlags);
@@ -192,24 +171,19 @@ private:
 
         ret[jss::TYPES]["Done"] = -1;
         std::map<int32_t, std::string> type_map{{-1, "Done"}};
-        for (auto const& entry : magic_enum::enum_entries<SerializedTypeID>())
+        for (auto const& [rawName, typeValue] : sTypeMap)
         {
-            const auto name = entry.second;
-            std::string type_name =
-                translate(name.data() + 4 /* remove STI_ */);
-            int32_t type_value = static_cast<int32_t>(entry.first);
-            ret[jss::TYPES][type_name] = type_value;
-            type_map[type_value] = type_name;
+            std::string typeName =
+                translate(std::string(rawName).substr(4) /* remove STI_ */);
+            ret[jss::TYPES][typeName] = typeValue;
+            type_map[typeValue] = typeName;
         }
 
         ret[jss::LEDGER_ENTRY_TYPES] = Json::objectValue;
         ret[jss::LEDGER_ENTRY_TYPES][jss::Invalid] = -1;
-        for (auto const& entry : magic_enum::enum_entries<LedgerEntryType>())
+        for (auto const& f : LedgerFormats::getInstance())
         {
-            const auto name = entry.second;
-            std::string type_name = translate(name.data() + 2 /* remove lt_ */);
-            int32_t type_value = static_cast<int32_t>(entry.first);
-            ret[jss::LEDGER_ENTRY_TYPES][type_name] = type_value;
+            ret[jss::LEDGER_ENTRY_TYPES][f.getName()] = f.getType();
         }
 
         ret[jss::FIELDS] = Json::arrayValue;
@@ -326,71 +300,16 @@ private:
         }
 
         ret[jss::TRANSACTION_RESULTS] = Json::objectValue;
-        for (auto const& entry : magic_enum::enum_entries<TELcodes>())
+        for (auto const& [code, terInfo] : transResults())
         {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
+            ret[jss::TRANSACTION_RESULTS][terInfo.first] = code;
         }
-        for (auto const& entry : magic_enum::enum_entries<TEMcodes>())
-        {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
-        }
-        for (auto const& entry : magic_enum::enum_entries<TEFcodes>())
-        {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
-        }
-        for (auto const& entry : magic_enum::enum_entries<TERcodes>())
-        {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
-        }
-        for (auto const& entry : magic_enum::enum_entries<TEScodes>())
-        {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
-        }
-        for (auto const& entry : magic_enum::enum_entries<TECcodes>())
-        {
-            const auto name = entry.second;
-            ret[jss::TRANSACTION_RESULTS][STR(name)] =
-                static_cast<int32_t>(entry.first);
-        }
-
-        auto const translate_tt = [](std::string inp) -> std::string {
-            if (inp == "Amendment")
-                return "EnableAmendment";
-            if (inp == "Fee")
-                return "SetFee";
-            if (inp == "PaychanClaim")
-                return "PaymentChannelClaim";
-            if (inp == "PaychanCreate")
-                return "PaymentChannelCreate";
-            if (inp == "PaychanFund")
-                return "PaymentChannelFund";
-            if (inp == "RegularKeySet")
-                return "SetRegularKey";
-            if (inp == "HookSet")
-                return "SetHook";
-            if (inp == "RemarksSet")
-                return "SetRemarks";
-            return inp;
-        };
 
         ret[jss::TRANSACTION_TYPES] = Json::objectValue;
         ret[jss::TRANSACTION_TYPES][jss::Invalid] = -1;
-        for (auto const& entry : magic_enum::enum_entries<TxType>())
+        for (auto const& f : TxFormats::getInstance())
         {
-            const auto name = entry.second;
-            std::string type_name = translate_tt(translate(name.data() + 2));
-            int32_t type_value = static_cast<int32_t>(entry.first);
-            ret[jss::TRANSACTION_TYPES][type_name] = type_value;
+            ret[jss::TRANSACTION_TYPES][f.getName()] = f.getType();
         }
 
         // Transaction Flags: