fixHookNameValidation Amendment

include/xrpl/protocol/detail/features.macro

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

include/xrpl/protocol/detail/ledger_entries.macro

@@ -262,7 +262,6 @@ LEDGER_ENTRY(ltACCOUNT_ROOT, 0x0061, AccountRoot, account, ({
     {sfHookStateScale,       soeOPTIONAL},
     {sfCron,                 soeOPTIONAL},
     {sfAMMID,                soeOPTIONAL},
-    {sfSecureUI,             soeOPTIONAL},
 }))
 
 /** A ledger object which contains a list of object identifiers.

include/xrpl/protocol/detail/sfields.macro

@@ -293,7 +293,6 @@ TYPED_SFIELD(sfAssetClass,               VL,        29)
 TYPED_SFIELD(sfProvider,                 VL,        30)
 TYPED_SFIELD(sfMPTokenMetadata,          VL,        31)
 TYPED_SFIELD(sfCredentialType,           VL,        32)
-TYPED_SFIELD(sfSecureUI,                 VL,        96)
 TYPED_SFIELD(sfHookName,                 VL,        97)
 TYPED_SFIELD(sfRemarkValue,              VL,        98)
 TYPED_SFIELD(sfRemarkName,               VL,        99)

include/xrpl/protocol/detail/transactions.macro

@@ -74,7 +74,6 @@ TRANSACTION(ttACCOUNT_SET, 3, AccountSet, ({
     {sfTickSize, soeOPTIONAL},
     {sfNFTokenMinter, soeOPTIONAL},
     {sfHookStateScale, soeOPTIONAL},
-    {sfSecureUI, soeOPTIONAL},
 }))
 
 /** This transaction type cancels an existing escrow. */

src/test/app/SetHook_test.cpp

@@ -2002,11 +2002,35 @@ public:
             BEAST_EXPECT(!env.meta()->isFieldPresent(sfHookEmissions));
         }
 
-        // Call named hook with the wrong hook name
+        // Call named hook with the wrong hook name (size == 0)
+        for (auto const fix : {true, false})
+        {
+            auto f = features - fixHookNameValidation;
+            if (fix)
+                f = f | fixHookNameValidation;
+            Env env{*this, f};
+
+            env.fund(XRP(10000), alice);
+            // execute both named and non-named hooks
+
+            auto jv = invoke::invoke(alice);
+            jv[jss::HookName] = "";
+
+            auto const expected = fix ? ter(temMALFORMED) : ter(tesSUCCESS);
+            env(jv,
+                M("Call named hook with the wrong hook name (size == 0)"),
+                HSFEE,
+                ter(expected));
+            env.close();
+        }
+
+        // Call named hook with the wrong hook name (size > 0)
         {
             auto jv = invoke::invoke(alice);
             jv[jss::HookName] = "41424345";
-            env(jv, M("Call named hook with the wrong hook name"), HSFEE);
+            env(jv,
+                M("Call named hook with the wrong hook name (size > 0)"),
+                HSFEE);
             env.close();
             // execute only non-named hook
             BEAST_EXPECT(!env.meta()->isFieldPresent(sfHookEmissions));

src/xrpld/app/tx/detail/SetAccount.cpp

@@ -197,31 +197,6 @@ SetAccount::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    if (tx.isFieldPresent(sfSecureUI))
-    {
-        if (!ctx.rules.enabled(featureSecureUI))
-            return temMALFORMED;
-
-        Blob ui = tx.getFieldVL(sfSecureUI);
-
-        if (ui.size() == 0)
-        {
-            // this is an unset operation, pass
-        }
-        else if (ui.size() > 4096)
-        {
-            JLOG(j.trace()) << "SecureUI: Too long > 4096 bytes";
-            return temMALFORMED;
-        }
-        else if (!URIToken::validateUTF8(ui))
-        {
-            JLOG(j.trace()) << "SecureUI: Not UTF-8";
-            return temMALFORMED;
-        }
-
-        // valid
-    } 
-
     return preflight2(ctx);
 }
 
@@ -724,23 +699,6 @@ SetAccount::doApply()
             sle->setFieldU16(sfHookStateScale, newScale);
         }
     }
-
-    if (tx.isFieldPresent(sfSecureUI))
-    {
-        Blob ui = tx.getFieldVL(sfSecureUI);
-        if (ui.size() == 0)
-        {
-            // unset operation
-            if (sle->isFieldPresent(sfSecureUI))
-                sle->makeFieldAbsent(sfSecureUI);
-        }
-        else
-        {
-            // set operation
-            sle->setFieldVL(sfSecureUI, std::move(ui));
-        }
-    }
-
     ctx_.view().update(sle);
 
     return tesSUCCESS;

src/xrpld/app/tx/detail/Transactor.cpp

@@ -155,7 +155,11 @@ preflight1(PreflightContext const& ctx)
             !ctx.rules.enabled(featureNamedHooks))
             return temMALFORMED;
 
-        if (!SetHook::validateHookName(ctx.tx.getFieldVL(sfHookName), ctx.j))
+        auto const& name = ctx.tx.getFieldVL(sfHookName);
+
+        if (name.size() == 0 && ctx.rules.enabled(fixHookNameValidation))
+            return temMALFORMED;
+        if (!SetHook::validateHookName(name, ctx.j))
             return temMALFORMED;
     }