refactor(hooks): simplify macro names for byte validation

- Rename VALIDATE_JS_BYTES* macros to shorter JS_BYTES* variants
- Improve code readability with more concise macro names
- Apply consistent formatting to macro definitions

src/ripple/app/hook/impl/applyHook.cpp

@@ -1003,11 +1003,89 @@ GetLengthOfAlreadyValidatedJSIntArrayOrHexString(
     return (len / 2);
 }
 
-inline std::optional<std::vector<uint8_t>>
+namespace jshook {
-FromJSIntArrayOrHexString(JSContext* ctx, JSValueConst& v, int max_len)
+enum class ByteConversionError : uint8_t {
+    Success = 0,
+    InvalidType,  // Not an array or string
+    EmptyInput,   // Empty array/string (might not be an error in some cases)
+    ExceedsMaxLength,       // Input length exceeds max_len
+    InvalidArrayElement,    // Array contains non-number element
+    ByteOutOfRange,         // Byte value not in [0, 255]
+    StringParseError,       // Failed to parse JS string
+    StringLengthMismatch,   // String length doesn't match size
+    InvalidHexCharacter,    // Non-hex character in string
+    StringTerminationError  // Unexpected null terminator
+};
+
+struct ByteConversionOptions
+{
+    uint32_t max_len;
+    bool truncate = false;
+
+    ByteConversionOptions(int len)
+        : max_len(len), truncate(false)  // NOLINT(*-explicit-constructor)
+    {
+    }
+};
+
+struct ByteConversionResult
+{
+    std::optional<std::vector<uint8_t>> data;
+    ByteConversionError error;
+    size_t input_byte_count =
+        0;  // Normalized to bytes for both arrays and hex strings
+
+    // Convenience methods
+    bool
+    ok() const
+    {
+        return error == ByteConversionError::Success;
+    }
+    bool
+    has_value() const
+    {
+        return data.has_value();
+    }
+    operator bool() const
+    {
+        return ok();
+    }
+
+    // Computed properties
+    size_t
+    bytes_processed() const
+    {
+        return data ? data->size() : 0;
+    }
+    bool
+    was_truncated() const
+    {
+        return data && data->size() < input_byte_count;
+    }
+
+    const std::vector<uint8_t>&
+    value() const
+    {
+        if (!data.has_value())
+            throw std::runtime_error("No data available");
+        return *data;
+    }
+
+    std::vector<uint8_t>
+    value_or(std::vector<uint8_t> default_val) const
+    {
+        return data.value_or(std::move(default_val));
+    }
+};
+
+inline ByteConversionResult
+FromJSIntArrayOrHexStringWithError(
+    JSContext* ctx,
+    JSValueConst& v,
+    ByteConversionOptions options)
 {
     std::vector<uint8_t> out;
-    out.reserve(max_len);
+    out.reserve(options.max_len);
 
     auto const a = JS_IsArray(ctx, v);
     auto const s = JS_IsString(v);
@@ -1019,32 +1097,56 @@ FromJSIntArrayOrHexString(JSContext* ctx, JSValueConst& v, int max_len)
         int64_t n = 0;
         js_get_length64(ctx, &n, v);
 
-        if (n == 0)
+        size_t original_n = n;  // Store original array length
-            return out;
 
-        if (n > max_len)
+        if (n == 0)
-            return {};
+            return {out, ByteConversionError::Success, 0};
+
+        if (n > options.max_len)
+        {
+            if (options.truncate)
+                n = options.max_len;  // Truncate to max_len
+            else
+                return {
+                    std::nullopt,
+                    ByteConversionError::ExceedsMaxLength,
+                    original_n};
+        }
 
         for (int64_t i = 0; i < n; ++i)
         {
             JSValue x = JS_GetPropertyInt64(ctx, v, i);
             if (!JS_IsNumber(x))
-                return {};
+            {
+                JS_FreeValue(ctx, x);
+                return {
+                    std::nullopt,
+                    ByteConversionError::InvalidArrayElement,
+                    original_n};
+            }
 
             int64_t byte = 0;
             JS_ToInt64(ctx, &byte, x);
-            if (byte > 256 || byte < 0)
+            JS_FreeValue(ctx, x);
-                return {};
+
+            if (byte > 255 || byte < 0)  // Fixed: should be 255, not 256
+                return {
+                    std::nullopt,
+                    ByteConversionError::ByteOutOfRange,
+                    original_n};
 
             out.push_back((uint8_t)byte);
         }
 
-        return out;
+        return {out, ByteConversionError::Success, original_n};
     }
 
     if (JS_IsString(v))
     {
-        auto [len, str] = FromJSString(ctx, v, max_len << 1U);
+        auto [len, str] = FromJSString(ctx, v, options.max_len << 1U);
+
+        size_t original_byte_count =
+            (len + 1) / 2;  // Round up for odd-length strings
 
         // std::cout << "Debug FromJSIAOHS: len=" << len << ", str=";
 
@@ -1054,16 +1156,28 @@ FromJSIntArrayOrHexString(JSContext* ctx, JSValueConst& v, int max_len)
         //     std::cout << "<no string>\n";
 
         if (!str)
-            return {};
+            return {std::nullopt, ByteConversionError::StringParseError, 0};
 
         if (len <= 0)
-            return out;
+            return {out, ByteConversionError::Success, 0};
 
-        if (len > (max_len << 1U))
+        if (len > (options.max_len << 1U))
-            return {};
+        {
+            if (options.truncate)
+                len = options.max_len
+                    << 1U;  // Truncate to max_len * 2 (hex chars)
+            else
+                return {
+                    std::nullopt,
+                    ByteConversionError::ExceedsMaxLength,
+                    original_byte_count};
+        }
 
         if (len != str->size())
-            return {};
+            return {
+                std::nullopt,
+                ByteConversionError::StringLengthMismatch,
+                original_byte_count};
 
         auto const parseHexNibble = [](uint8_t a) -> std::optional<uint8_t> {
             if (a >= '0' && a <= '9')
@@ -1085,26 +1199,159 @@ FromJSIntArrayOrHexString(JSContext* ctx, JSValueConst& v, int max_len)
         {
             auto first = parseHexNibble(*cstr++);
             if (!first.has_value())
-                return {};
+                return {
+                    std::nullopt,
+                    ByteConversionError::InvalidHexCharacter,
+                    original_byte_count};
 
-            out[i++] = *first;
+            out.push_back(*first);  // Fixed: was using uninitialized out[i++]
+            i++;
         }
 
-        for (; i < len && *cstr != '\0'; ++i)
+        for (; i < len && *cstr != '\0';
+             i += 2)  // Fixed: increment by 2 for hex pairs
         {
+            // Check if we've reached max_len bytes
+            if (options.truncate && out.size() >= options.max_len)
+                break;
+
             auto a = parseHexNibble(*cstr++);
             auto b = parseHexNibble(*cstr++);
 
             if (!a.has_value() || !b.has_value())
-                return {};
+                return {
+                    std::nullopt,
+                    ByteConversionError::InvalidHexCharacter,
+                    original_byte_count};
 
             out.push_back((*a << 4U) | (*b));
         }
 
-        return out;
+        return {out, ByteConversionError::Success, original_byte_count};
     }
 
-    return {};
+    return {std::nullopt, ByteConversionError::InvalidType, 0};
+}
+
+// Validation result wrapper
+template <typename T>
+struct ValidationResult
+{
+    T value;
+    int64_t error_code;
+
+    bool
+    has_error() const
+    {
+        return error_code != 0;
+    }
+};
+
+// Core validation function
+template <typename Validator>
+inline ValidationResult<std::optional<std::vector<uint8_t>>>
+validate_js_bytes_with(
+    JSContext* ctx,
+    JSValueConst& v,
+    ByteConversionOptions options,
+    Validator&& validator,
+    bool allow_undefined = false)
+{
+    // Handle undefined for optional case
+    if (JS_IsUndefined(v))
+    {
+        if (allow_undefined)
+            return {std::nullopt, 0};
+        else
+            return {{}, INVALID_ARGUMENT};
+    }
+
+    // Convert with full error information
+    auto result = FromJSIntArrayOrHexStringWithError(ctx, v, options);
+
+    // Let validator process the full result with error info
+    int64_t validation_error = validator(result);
+    if (validation_error != 0)
+        return {{}, validation_error};
+
+    return {std::move(result.data), 0};
+}
+
+// Validator definitions
+inline auto validate_state_data = [](const ByteConversionResult& r) -> int64_t {
+    if (r.input_byte_count > hook::maxHookStateDataSize())
+        return TOO_BIG;
+
+    if (!r.ok())
+        return INVALID_ARGUMENT;
+
+    return 0;
+};
+
+inline auto validate_state_key = [](const ByteConversionResult& r) -> int64_t {
+    if (r.input_byte_count > 32)
+        return TOO_BIG;
+
+    if (!r.ok())
+        return INVALID_ARGUMENT;
+
+    assert(r.has_value());
+    if (r.data->empty())
+        return TOO_SMALL;
+
+    return 0;
+};
+
+inline auto validate_namespace = [](const ByteConversionResult& r) -> int64_t {
+    if (!r.ok())
+        return INVALID_ARGUMENT;
+
+    assert(r.has_value());
+    if (r.data->size() != 32)
+        return INVALID_ARGUMENT;
+
+    return 0;
+};
+
+inline auto validate_account_id = [](const ByteConversionResult& r) -> int64_t {
+    if (!r.ok())
+        return INVALID_ARGUMENT;
+
+    // If provided, must be exactly 20 bytes
+    assert(r.has_value());
+    if (r.data->size() != 20)
+        return INVALID_ARGUMENT;
+
+    return 0;
+};
+}  // namespace jshook
+
+// Single macro that works for both optional and required cases
+#define JS_BYTES(var_name, js_val, max_len, validator, ...)  \
+    auto var_name##_result = jshook::validate_js_bytes_with( \
+        ctx,                                                 \
+        js_val,                                              \
+        jshook::ByteConversionOptions(max_len),              \
+        validator,                                           \
+        ##__VA_ARGS__);                                      \
+    if (var_name##_result.has_error())                       \
+        returnJS(var_name##_result.error_code);              \
+    auto& var_name = var_name##_result.value
+
+// Convenience macros
+#define JS_BYTES_REQUIRED(var_name, js_val, max_len, validator) \
+    JS_BYTES(var_name, js_val, max_len, validator, false)
+
+#define JS_BYTES_OPTIONAL(var_name, js_val, max_len, validator) \
+    JS_BYTES(var_name, js_val, max_len, validator, true)
+
+inline std::optional<std::vector<uint8_t>>
+FromJSIntArrayOrHexString(
+    JSContext* ctx,
+    JSValueConst& v,
+    jshook::ByteConversionOptions options)
+{
+    return jshook::FromJSIntArrayOrHexStringWithError(ctx, v, options).data;
 }
 
 inline int32_t
@@ -2316,69 +2563,39 @@ DEFINE_JS_FUNCTION(
 {
     JS_HOOK_SETUP();
 
-    auto val = FromJSIntArrayOrHexString(ctx, raw_val, 0x10000);
+    // Validate all inputs using macros - much cleaner!
-    auto key_in = FromJSIntArrayOrHexString(ctx, raw_key, 0x10000);
+    JS_BYTES_OPTIONAL(
-    auto ns_in = FromJSIntArrayOrHexString(ctx, raw_ns, 0x10000);
+        val,
-    auto acc_in = FromJSIntArrayOrHexString(ctx, raw_acc, 0x10000);
+        raw_val,
+        hook::maxHookStateDataSize(),
+        jshook::validate_state_data);
+    JS_BYTES_REQUIRED(key, raw_key, 32, jshook::validate_state_key);
+    JS_BYTES_OPTIONAL(ns, raw_ns, 32, jshook::validate_namespace);
+    JS_BYTES_OPTIONAL(acc, raw_acc, 20, jshook::validate_account_id);
 
-    if (!val.has_value() && !JS_IsUndefined(raw_val))
+    // Extract values with defaults
-        returnJS(INVALID_ARGUMENT);
+    uint256 namespace_id = ns.has_value() ? uint256::fromVoid(ns->data())
+                                          : hookCtx.result.hookNamespace;
 
-    if (!ns_in.has_value() && !JS_IsUndefined(raw_ns))
+    AccountID account_id = acc.has_value() ? AccountID::fromVoid(acc->data())
-        returnJS(INVALID_ARGUMENT);
+                                           : hookCtx.result.account;
 
-    if (!acc_in.has_value() && !JS_IsUndefined(raw_acc))
+    auto state_key = make_state_key(
-        returnJS(INVALID_ARGUMENT);
+        std::string_view{(const char*)(key->data()), key->size()});
-
-    // val may be populated and empty, this is a delete operation...
-
-    if (val.has_value())
-    {
-        if (val->size() > hook::maxHookStateDataSize())
-            returnJS(TOO_BIG);
-    }
-
-    if (key_in.has_value())
-    {
-        if (key_in->size() > 32)
-            returnJS(TOO_BIG);
-
-        if (key_in->size() < 1)
-            // FromJSIntArrayOrHexString() does not return data of length 0.
-            returnJS(TOO_SMALL);
-    }
-    else
-    {
-        returnJS(INVALID_ARGUMENT);
-    }
-
-    if (ns_in.has_value() && ns_in->size() != 32)
-        returnJS(INVALID_ARGUMENT);
-
-    if (acc_in.has_value() && acc_in->size() != 20)
-        returnJS(INVALID_ARGUMENT);
-
-    uint256 ns = ns_in.has_value() ? uint256::fromVoid(ns_in->data())
-                                   : hookCtx.result.hookNamespace;
-
-    AccountID acc = acc_in.has_value() ? AccountID::fromVoid(acc_in->data())
-                                       : hookCtx.result.account;
-
-    auto key = make_state_key(
-        std::string_view{(const char*)(key_in->data()), key_in->size()});
 
     auto const sleAccount = view.peek(hookCtx.result.accountKeylet);
     if (!sleAccount)
         returnJS(tefINTERNAL);
 
-    if (!key)
+    if (!state_key)
         returnJS(INTERNAL_ERROR);
 
     ripple::Blob data;
     if (val.has_value())
         data = ripple::Blob(val->data(), val->data() + val->size());
 
-    returnJS(__state_foreign_set(hookCtx, applyCtx, j, data, *key, ns, acc));
+    returnJS(__state_foreign_set(
+        hookCtx, applyCtx, j, data, *state_key, namespace_id, account_id));
 
     JS_HOOK_TEARDOWN();
 }