Forward api_version 1 requests to rippled (#716)

Fixes #698

CMakeLists.txt

@@ -128,6 +128,7 @@ if(BUILD_TESTS)
     unittests/rpc/CountersTest.cpp
     unittests/rpc/AdminVerificationTest.cpp
     unittests/rpc/APIVersionTests.cpp
+    unittests/rpc/ForwardingProxyTests.cpp
     ## RPC handlers
     unittests/rpc/handlers/DefaultProcessorTests.cpp
     unittests/rpc/handlers/TestHandlerTests.cpp

src/rpc/RPCEngine.h

@@ -25,11 +25,11 @@
 #include <log/Logger.h>
 #include <rpc/Counters.h>
 #include <rpc/Errors.h>
-#include <rpc/HandlerTable.h>
 #include <rpc/RPCHelpers.h>
 #include <rpc/common/AnyHandler.h>
 #include <rpc/common/Types.h>
 #include <rpc/common/impl/AdminVerificationStrategy.h>
+#include <rpc/common/impl/ForwardingProxy.h>
 #include <util/Taggable.h>
 #include <webserver/Context.h>
 #include <webserver/DOSGuard.h>
@@ -66,7 +66,9 @@ class RPCEngineBase
     std::reference_wrapper<WorkQueue> workQueue_;
     std::reference_wrapper<Counters> counters_;
 
-    HandlerTable handlerTable_;
+    std::shared_ptr<HandlerProvider const> handlerProvider_;
+
+    detail::ForwardingProxy<LoadBalancer, Counters, HandlerProvider> forwardingProxy_;
     AdminVerificationStrategyType adminVerifier_;
 
 public:
@@ -85,7 +87,8 @@ public:
         , dosGuard_{std::cref(dosGuard)}
         , workQueue_{std::ref(workQueue)}
         , counters_{std::ref(counters)}
-        , handlerTable_{handlerProvider}
+        , handlerProvider_{handlerProvider}
+        , forwardingProxy_{balancer, counters, handlerProvider}
     {
     }
 
@@ -112,22 +115,8 @@ public:
     Result
     buildResponse(Web::Context const& ctx)
     {
-        if (shouldForwardToRippled(ctx))
+        if (forwardingProxy_.shouldForward(ctx))
-        {
+            return forwardingProxy_.forward(ctx);
-            auto toForward = ctx.params;
-            toForward["command"] = ctx.method;
-
-            if (auto const res = balancer_->forwardToRippled(toForward, ctx.clientIp, ctx.yield); not res)
-            {
-                notifyFailedToForward(ctx.method);
-                return Status{RippledError::rpcFAILED_TO_FORWARD};
-            }
-            else
-            {
-                notifyForwarded(ctx.method);
-                return *res;
-            }
-        }
 
         if (backend_->isTooBusy())
         {
@@ -136,7 +125,7 @@ public:
             return Status{RippledError::rpcTOO_BUSY};
         }
 
-        auto const method = handlerTable_.getHandler(ctx.method);
+        auto const method = handlerProvider_->getHandler(ctx.method);
         if (!method)
         {
             notifyUnknownCommand();
@@ -212,6 +201,7 @@ public:
     void
     notifyFailed(std::string const& method)
     {
+        // FIXME: seems like this is not used?
         if (validHandler(method))
             counters_.get().rpcFailed(method);
     }
@@ -230,28 +220,6 @@ public:
             counters_.get().rpcErrored(method);
     }
 
-    /**
-     * @brief Notify the system that specified method execution was forwarded to rippled
-     * @param method
-     */
-    void
-    notifyForwarded(std::string const& method)
-    {
-        if (validHandler(method))
-            counters_.get().rpcForwarded(method);
-    }
-
-    /**
-     * @brief Notify the system that specified method failed to be forwarded to rippled
-     * @param method
-     */
-    void
-    notifyFailedToForward(std::string const& method)
-    {
-        if (validHandler(method))
-            counters_.get().rpcFailedToForward(method);
-    }
-
     /**
      * @brief Notify the system that the RPC system is too busy to handle an incoming request
      */
@@ -300,55 +268,10 @@ public:
     }
 
 private:
-    bool
-    shouldForwardToRippled(Web::Context const& ctx) const
-    {
-        auto const& request = ctx.params;
-
-        if (isClioOnly(ctx.method))
-            return false;
-
-        if (isForwardCommand(ctx.method))
-            return true;
-
-        if (specifiesCurrentOrClosedLedger(request))
-            return true;
-
-        if (ctx.method == "account_info" && request.contains("queue") && request.at("queue").is_bool() &&
-            request.at("queue").as_bool())
-            return true;
-
-        return false;
-    }
-
-    bool
-    isForwardCommand(std::string const& method) const
-    {
-        static std::unordered_set<std::string> const FORWARD_COMMANDS{
-            "submit",
-            "submit_multisigned",
-            "fee",
-            "ledger_closed",
-            "ledger_current",
-            "ripple_path_find",
-            "manifest",
-            "channel_authorize",
-            "channel_verify",
-        };
-
-        return FORWARD_COMMANDS.contains(method);
-    }
-
-    bool
-    isClioOnly(std::string const& method) const
-    {
-        return handlerTable_.isClioOnly(method);
-    }
-
     bool
     validHandler(std::string const& method) const
     {
-        return handlerTable_.contains(method) || isForwardCommand(method);
+        return handlerProvider_->contains(method) || forwardingProxy_.isProxied(method);
     }
 };
 

src/rpc/common/APIVersion.h

@@ -36,9 +36,10 @@ static constexpr uint32_t API_VERSION_DEFAULT = 2u;
 /**
  * @brief Minimum API version supported by this build
  *
- * Note: Clio does not support v1 and only supports v2 and newer.
+ * Note: Clio does not natively support v1 and only supports v2 or newer.
+ * However, Clio will forward all v1 requests to rippled for backward compatibility.
  */
-static constexpr uint32_t API_VERSION_MIN = 2u;
+static constexpr uint32_t API_VERSION_MIN = 1u;
 
 /**
  * @brief Maximum API version supported by this build

src/rpc/common/Types.h

@@ -30,10 +30,14 @@
 namespace Server {
 struct ConnectionBase;
 }
+
+class LoadBalancer;
 class SubscriptionManager;
 
 namespace RPC {
 
+class Counters;
+
 /**
  * @brief Return type used for Validators that can return error but don't have
  * specific value to return

src/rpc/common/impl/ForwardingProxy.h

@@ -0,0 +1,139 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of clio: https://github.com/XRPLF/clio
+    Copyright (c) 2023, the clio developers.
+
+    Permission to use, copy, modify, and distribute this software for any
+    purpose with or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL,  DIRECT,  INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#pragma once
+
+#include <etl/LoadBalancer.h>
+#include <etl/Source.h>
+#include <log/Logger.h>
+#include <rpc/Counters.h>
+#include <rpc/RPCHelpers.h>
+#include <rpc/common/Types.h>
+#include <webserver/Context.h>
+
+#include <memory>
+#include <string>
+
+namespace RPC::detail {
+
+template <typename LoadBalancerType, typename CountersType, typename HandlerProviderType>
+class ForwardingProxy
+{
+    clio::Logger log_{"RPC"};
+
+    std::shared_ptr<LoadBalancerType> balancer_;
+    std::reference_wrapper<CountersType> counters_;
+    std::shared_ptr<HandlerProviderType const> handlerProvider_;
+
+public:
+    ForwardingProxy(
+        std::shared_ptr<LoadBalancerType> const& balancer,
+        CountersType& counters,
+        std::shared_ptr<HandlerProviderType const> const& handlerProvider)
+        : balancer_{balancer}, counters_{std::ref(counters)}, handlerProvider_{handlerProvider}
+    {
+    }
+
+    bool
+    shouldForward(Web::Context const& ctx) const
+    {
+        if (ctx.method == "subscribe" || ctx.method == "unsubscribe")
+            return false;
+
+        // TODO: if needed, make configurable with json config option
+        if (ctx.apiVersion == 1)
+            return true;
+
+        if (handlerProvider_->isClioOnly(ctx.method))
+            return false;
+
+        if (isProxied(ctx.method))
+            return true;
+
+        auto const& request = ctx.params;
+
+        if (specifiesCurrentOrClosedLedger(request))
+            return true;
+
+        if (ctx.method == "account_info" && request.contains("queue") && request.at("queue").is_bool() &&
+            request.at("queue").as_bool())
+            return true;
+
+        return false;
+    }
+
+    Result
+    forward(Web::Context const& ctx)
+    {
+        auto toForward = ctx.params;
+        toForward["command"] = ctx.method;
+
+        if (auto const res = balancer_->forwardToRippled(toForward, ctx.clientIp, ctx.yield); not res)
+        {
+            notifyFailedToForward(ctx.method);
+            return Status{RippledError::rpcFAILED_TO_FORWARD};
+        }
+        else
+        {
+            notifyForwarded(ctx.method);
+            return *res;
+        }
+    }
+
+    bool
+    isProxied(std::string const& method) const
+    {
+        static std::unordered_set<std::string> const proxiedCommands{
+            "submit",
+            "submit_multisigned",
+            "fee",
+            "ledger_closed",
+            "ledger_current",
+            "ripple_path_find",
+            "manifest",
+            "channel_authorize",
+            "channel_verify",
+        };
+
+        return proxiedCommands.contains(method);
+    }
+
+private:
+    void
+    notifyForwarded(std::string const& method)
+    {
+        if (validHandler(method))
+            counters_.get().rpcForwarded(method);
+    }
+
+    void
+    notifyFailedToForward(std::string const& method)
+    {
+        if (validHandler(method))
+            counters_.get().rpcFailedToForward(method);
+    }
+
+    bool
+    validHandler(std::string const& method) const
+    {
+        return handlerProvider_->contains(method) || isProxied(method);
+    }
+};
+
+}  // namespace RPC::detail

src/webserver/RPCExecutor.h

@@ -155,21 +155,25 @@ private:
                     boost::beast::http::status::ok);
             }
 
-            auto context = connection->upgraded ? RPC::make_WsContext(
+            auto const context = [&] {
-                                                      yc,
+                if (connection->upgraded)
-                                                      request,
+                    return RPC::make_WsContext(
-                                                      connection,
+                        yc,
-                                                      tagFactory_.with(connection->tag()),
+                        request,
-                                                      *range,
+                        connection,
-                                                      connection->clientIp,
+                        tagFactory_.with(connection->tag()),
-                                                      std::cref(apiVersionParser_))
+                        *range,
-                                                : RPC::make_HttpContext(
+                        connection->clientIp,
-                                                      yc,
+                        std::cref(apiVersionParser_));
-                                                      request,
+                else
-                                                      tagFactory_.with(connection->tag()),
+                    return RPC::make_HttpContext(
-                                                      *range,
+                        yc,
-                                                      connection->clientIp,
+                        request,
-                                                      std::cref(apiVersionParser_));
+                        tagFactory_.with(connection->tag()),
+                        *range,
+                        connection->clientIp,
+                        std::cref(apiVersionParser_));
+            }();
 
             if (!context)
             {
@@ -217,8 +221,8 @@ private:
                     response["result"] = result;
                 }
 
-                // for ws , there is additional field "status" in response
+                // for ws there is an additional field "status" in the response,
-                // otherwise , the "status" is in the "result" field
+                // otherwise the "status" is in the "result" field
                 if (connection->upgraded)
                 {
                     if (!id.is_null())

unittests/rpc/ForwardingProxyTests.cpp

@@ -0,0 +1,300 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of clio: https://github.com/XRPLF/clio
+    Copyright (c) 2023, the clio developers.
+
+    Permission to use, copy, modify, and distribute this software for any
+    purpose with or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL,  DIRECT,  INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <util/Fixtures.h>
+#include <util/MockCounters.h>
+#include <util/MockHandlerProvider.h>
+#include <util/MockLoadBalancer.h>
+
+#include <config/Config.h>
+#include <rpc/common/impl/ForwardingProxy.h>
+
+#include <boost/json.hpp>
+#include <gtest/gtest.h>
+
+using namespace clio;
+using namespace RPC;
+using namespace testing;
+
+constexpr static auto CLIENT_IP = "127.0.0.1";
+
+class RPCForwardingProxyTest : public HandlerBaseTest
+{
+protected:
+    std::shared_ptr<MockLoadBalancer> loadBalancer = std::make_shared<MockLoadBalancer>();
+    std::shared_ptr<MockHandlerProvider> handlerProvider = std::make_shared<MockHandlerProvider>();
+    MockCounters counters;
+
+    clio::Config config;
+    util::TagDecoratorFactory tagFactory{config};
+
+    RPC::detail::ForwardingProxy<MockLoadBalancer, MockCounters, MockHandlerProvider> proxy{
+        loadBalancer,
+        counters,
+        handlerProvider};
+};
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfClioOnly)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "test";
+    auto const params = boost::json::parse("{}");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(true));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_FALSE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfProxied)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "submit";
+    auto const params = boost::json::parse("{}");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_TRUE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfCurrentLedgerSpecified)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "anymethod";
+    auto const params = boost::json::parse(R"({"ledger_index": "current"})");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_TRUE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfClosedLedgerSpecified)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "anymethod";
+    auto const params = boost::json::parse(R"({"ledger_index": "closed"})");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_TRUE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfAccountInfoWithQueueSpecified)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "account_info";
+    auto const params = boost::json::parse(R"({"queue": true})");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_TRUE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfAccountInfoQueueIsFalse)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "account_info";
+    auto const params = boost::json::parse(R"({"queue": false})");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_FALSE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfAPIVersionIsV1)
+{
+    auto const apiVersion = 1u;
+    auto const method = "api_version_check";
+    auto const params = boost::json::parse("{}");
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_TRUE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfAPIVersionIsV2)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const apiVersion = 2u;
+    auto const method = "api_version_check";
+    auto const params = boost::json::parse("{}");
+
+    ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_FALSE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldNeverForwardSubscribe)
+{
+    auto const apiVersion = 1u;
+    auto const method = "subscribe";
+    auto const params = boost::json::parse("{}");
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_FALSE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ShouldNeverForwardUnsubscribe)
+{
+    auto const apiVersion = 1u;
+    auto const method = "unsubscribe";
+    auto const params = boost::json::parse("{}");
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.shouldForward(ctx);
+        ASSERT_FALSE(res);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ForwardCallsBalancerWithCorrectParams)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const rawBalancerPtr = static_cast<MockLoadBalancer*>(loadBalancer.get());
+    auto const apiVersion = 2u;
+    auto const method = "submit";
+    auto const params = boost::json::parse(R"({"test": true})");
+    auto const forwarded = boost::json::parse(R"({"test": true, "command": "submit"})");
+
+    ON_CALL(*rawBalancerPtr, forwardToRippled).WillByDefault(Return(std::make_optional<boost::json::object>()));
+    EXPECT_CALL(*rawBalancerPtr, forwardToRippled(forwarded.as_object(), CLIENT_IP, _)).Times(1);
+
+    ON_CALL(*rawHandlerProviderPtr, contains).WillByDefault(Return(true));
+    EXPECT_CALL(*rawHandlerProviderPtr, contains(method)).Times(1);
+
+    ON_CALL(counters, rpcForwarded).WillByDefault(Return());
+    EXPECT_CALL(counters, rpcForwarded(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.forward(ctx);
+
+        auto const data = std::get_if<boost::json::object>(&res);
+        EXPECT_TRUE(data != nullptr);
+    });
+}
+
+TEST_F(RPCForwardingProxyTest, ForwardingFailYieldsErrorStatus)
+{
+    auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
+    auto const rawBalancerPtr = static_cast<MockLoadBalancer*>(loadBalancer.get());
+    auto const apiVersion = 2u;
+    auto const method = "submit";
+    auto const params = boost::json::parse(R"({"test": true})");
+    auto const forwarded = boost::json::parse(R"({"test": true, "command": "submit"})");
+
+    ON_CALL(*rawBalancerPtr, forwardToRippled).WillByDefault(Return(std::nullopt));
+    EXPECT_CALL(*rawBalancerPtr, forwardToRippled(forwarded.as_object(), CLIENT_IP, _)).Times(1);
+
+    ON_CALL(*rawHandlerProviderPtr, contains).WillByDefault(Return(true));
+    EXPECT_CALL(*rawHandlerProviderPtr, contains(method)).Times(1);
+
+    ON_CALL(counters, rpcFailedToForward).WillByDefault(Return());
+    EXPECT_CALL(counters, rpcFailedToForward(method)).Times(1);
+
+    runSpawn([&](auto yield) {
+        auto const range = mockBackendPtr->fetchLedgerRange();
+        auto const ctx =
+            Web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
+
+        auto const res = proxy.forward(ctx);
+
+        auto const status = std::get_if<Status>(&res);
+        EXPECT_TRUE(status != nullptr);
+        EXPECT_EQ(*status, ripple::rpcFAILED_TO_FORWARD);
+    });
+}

unittests/util/MockCounters.h

@@ -26,8 +26,15 @@
 
 struct MockCounters
 {
+    MOCK_METHOD(void, rpcFailed, (std::string const&), ());
     MOCK_METHOD(void, rpcErrored, (std::string const&), ());
     MOCK_METHOD(void, rpcComplete, (std::string const&, std::chrono::microseconds const&), ());
     MOCK_METHOD(void, rpcForwarded, (std::string const&), ());
+    MOCK_METHOD(void, rpcFailedToForward, (std::string const&), ());
+    MOCK_METHOD(void, onTooBusy, (), ());
+    MOCK_METHOD(void, onNotReady, (), ());
+    MOCK_METHOD(void, onBadSyntax, (), ());
+    MOCK_METHOD(void, onUnknownCommand, (), ());
+    MOCK_METHOD(void, onInternalError, (), ());
     MOCK_METHOD(boost::json::object, report, (), (const));
 };

unittests/util/MockHandlerProvider.h

@@ -22,38 +22,12 @@
 #include <rpc/common/AnyHandler.h>
 #include <rpc/common/Types.h>
 
-#include <memory>
+#include <gmock/gmock.h>
-#include <optional>
-#include <string>
 
-namespace RPC {
+struct MockHandlerProvider : public RPC::HandlerProvider
-
-class HandlerTable
 {
-    std::shared_ptr<HandlerProvider const> provider_;
-
 public:
-    HandlerTable(std::shared_ptr<HandlerProvider const> const& provider) : provider_{provider}
+    MOCK_METHOD(bool, contains, (std::string const&), (const, override));
-    {
+    MOCK_METHOD(std::optional<RPC::AnyHandler>, getHandler, (std::string const&), (const, override));
-    }
+    MOCK_METHOD(bool, isClioOnly, (std::string const&), (const, override));
-
-    bool
-    contains(std::string const& method) const
-    {
-        return provider_->contains(method);
-    }
-
-    std::optional<AnyHandler>
-    getHandler(std::string const& command) const
-    {
-        return provider_->getHandler(command);
-    }
-
-    bool
-    isClioOnly(std::string const& command) const
-    {
-        return provider_->isClioOnly(command);
-    }
 };
-
-}  // namespace RPC