Merge remote-tracking branch 'upstream/dev' into claimreward-hookname-validation

src/test/app/ClaimReward_test.cpp

@@ -563,6 +563,20 @@ public:
                 env(reward::claim(alice),
                     reward::issuer(issuer),
                     ter(tecNO_TARGET));
+                env.close();
+            }
+            // Invalid HookName
+            {
+                auto hookObj = hso(jtx::genesis::AcceptHook, overrideFlag);
+                hookObj[jss::HookOn] = to_string(~UINT256_BIT[ttCLAIM_REWARD]);
+                hookObj[jss::HookName] = "4142434445";
+                env(hook(issuer, {{hookObj}}, 0), fee(XRP(1)));
+                env.close();
+
+                env(reward::claim(alice),
+                    reward::issuer(issuer),
+                    ter(tecNO_TARGET));
+                env.close();
             }
             // Vaild HookOn
             {

src/xrpld/app/hook/applyHook.h

@@ -82,7 +82,10 @@ namespace hook_api {
 namespace hook {
 
 bool
-canHook(ripple::TxType txType, ripple::uint256 hookOn);
+canHook(
+    STTx const& tx,
+    ripple::uint256 hookOn,
+    std::optional<ripple::Slice> hookName);
 
 bool
 canEmit(ripple::TxType txType, ripple::uint256 hookCanEmit);

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

@@ -785,7 +785,7 @@ hook::removeHookNamespaceEntry(ripple::SLE& sleAccount, ripple::uint256 ns)
 // transactions. If you wish to set a hook that has control over ttHOOK_SET then
 // set bit 1U<<22.
 bool
-hook::canHook(ripple::TxType txType, ripple::uint256 hookOn)
+canHookTT(ripple::TxType txType, ripple::uint256 hookOn)
 {
     // invert ttHOOK_SET bit
     hookOn ^= UINT256_BIT[ttHOOK_SET];
@@ -799,7 +799,28 @@ hook::canHook(ripple::TxType txType, ripple::uint256 hookOn)
 bool
 hook::canEmit(ripple::TxType txType, ripple::uint256 hookCanEmit)
 {
-    return hook::canHook(txType, hookCanEmit);
+    return canHookTT(txType, hookCanEmit);
+}
+
+bool
+hook::canHook(
+    STTx const& tx,
+    ripple::uint256 hookOn,
+    std::optional<ripple::Slice> hookName)
+{
+    if (!canHookTT(tx.getTxnType(), hookOn))
+        return false;
+
+    if (!hookName || hookName->empty())
+        // no hook name specified to hook, so we can always hook
+        return true;
+
+    if (!tx.isFieldPresent(sfHookName))
+        // hook name specified hook, but no hook name specified in the
+        // transaction, so we can't hook without the hook name
+        return false;
+
+    return tx[sfHookName] == *hookName;
 }
 
 ripple::uint256

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

@@ -158,7 +158,11 @@ ClaimReward::preclaim(PreclaimContext const& ctx)
 
                 auto const& hookOn =
                     hook::getHookOn(hook, sleDef, sfHookOnIncoming);
-                if (hook::canHook(ttCLAIM_REWARD, hookOn))
+                auto const hookName =
+                    ctx.view.rules().enabled(fixHookNameValidation)
+                    ? hook[~sfHookName]
+                    : std::nullopt;
+                if (hook::canHook(ctx.tx, hookOn, hookName))
                 {
                     hasClaimRewardHook = true;
                     break;

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

@@ -287,19 +287,7 @@ Transactor::calculateHookChainFee(
             // LCOV_EXCL_STOP
         }
 
-        std::optional<Blob> requiredHookName;
-        if (hookObj.isFieldPresent(sfHookName) &&
-            hookObj.getFieldVL(sfHookName).size() > 0)
-            requiredHookName = hookObj.getFieldVL(sfHookName);
-
-        if (requiredHookName)
-        {
-            // need to specify same hook name in the transaction
-            if (!tx.isFieldPresent(sfHookName))
-                continue;
-            if (*requiredHookName != tx.getFieldVL(sfHookName))
-                continue;
-        }
+        auto const hookName = hookObj[~sfHookName];
 
         uint32_t flags = 0;
         if (hookObj.isFieldPresent(sfFlags))
@@ -311,7 +299,7 @@ Transactor::calculateHookChainFee(
         uint256 hookOn = hook::getHookOn(
             hookObj, hookDef, isOutgoing ? sfHookOnOutgoing : sfHookOnIncoming);
 
-        if (hook::canHook(tx.getTxnType(), hookOn) &&
+        if (hook::canHook(tx, hookOn, hookName) &&
             (!collectCallsOnly || (flags & hook::hsfCOLLECT)))
         {
             XRPAmount const toAdd{hookDef->getFieldAmount(sfFee).xrp().drops()};
@@ -1381,25 +1369,13 @@ Transactor::executeHookChain(
             // LCOV_EXCL_STOP
         }
 
-        std::optional<Blob> requiredHookName;
-        if (hookObj.isFieldPresent(sfHookName) &&
-            hookObj.getFieldVL(sfHookName).size() > 0)
-            requiredHookName = hookObj.getFieldVL(sfHookName);
-
-        if (requiredHookName)
-        {
-            // need to specify same hook name in the transaction
-            if (!ctx_.tx.isFieldPresent(sfHookName))
-                continue;
-            if (*requiredHookName != ctx_.tx.getFieldVL(sfHookName))
-                continue;
-        }
+        auto const hookName = hookObj[~sfHookName];
 
         // check if the hook can fire
         uint256 hookOn = hook::getHookOn(
             hookObj, hookDef, isOutgoing ? sfHookOnOutgoing : sfHookOnIncoming);
 
-        if (!hook::canHook(ctx_.tx.getTxnType(), hookOn))
+        if (!hook::canHook(ctx_.tx, hookOn, hookName))
             continue;  // skip if it can't
 
         uint256 hookCanEmit = hook::getHookCanEmit(hookObj, hookDef);