From 5050b366d907730c6e78502bf9edb5232ea6964d Mon Sep 17 00:00:00 2001
From: Edward Hennis <ed@ripple.com>
Date: Mon, 22 Nov 2021 18:28:17 -0500
Subject: [PATCH] Return correct error code for node_to_shard command errors

---
 src/ripple/net/RPCErr.h                 |  2 +-
 src/ripple/net/impl/RPCErr.cpp          |  3 +-
 src/ripple/rpc/handlers/NodeToShard.cpp |  2 +-
 src/test/rpc/NodeToShardRPC_test.cpp    | 72 +++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/src/ripple/net/RPCErr.h b/src/ripple/net/RPCErr.h
index f2bce8639..e49e96b3d 100644
--- a/src/ripple/net/RPCErr.h
+++ b/src/ripple/net/RPCErr.h
@@ -28,7 +28,7 @@ namespace ripple {
 bool
 isRpcError(Json::Value jvResult);
 Json::Value
-rpcError(int iError, Json::Value jvResult = Json::Value(Json::objectValue));
+rpcError(int iError);
 
 }  // namespace ripple
 
diff --git a/src/ripple/net/impl/RPCErr.cpp b/src/ripple/net/impl/RPCErr.cpp
index 1c28eda00..8af2a248c 100644
--- a/src/ripple/net/impl/RPCErr.cpp
+++ b/src/ripple/net/impl/RPCErr.cpp
@@ -26,8 +26,9 @@ struct RPCErr;
 
 // VFALCO NOTE Deprecated function
 Json::Value
-rpcError(int iError, Json::Value jvResult)
+rpcError(int iError)
 {
+    Json::Value jvResult(Json::objectValue);
     RPC::inject_error(iError, jvResult);
     return jvResult;
 }
diff --git a/src/ripple/rpc/handlers/NodeToShard.cpp b/src/ripple/rpc/handlers/NodeToShard.cpp
index ac250db18..fd48797cb 100644
--- a/src/ripple/rpc/handlers/NodeToShard.cpp
+++ b/src/ripple/rpc/handlers/NodeToShard.cpp
@@ -38,7 +38,7 @@ doNodeToShard(RPC::JsonContext& context)
     // Shard store must be enabled
     auto const shardStore = context.app.getShardStore();
     if (!shardStore)
-        return rpcError(rpcINTERNAL, "No shard store");
+        return RPC::make_error(rpcNOT_ENABLED);
 
     if (!context.params.isMember(jss::action))
         return RPC::missing_field_error(jss::action);
diff --git a/src/test/rpc/NodeToShardRPC_test.cpp b/src/test/rpc/NodeToShardRPC_test.cpp
index 64e089b0b..edfaf6c20 100644
--- a/src/test/rpc/NodeToShardRPC_test.cpp
+++ b/src/test/rpc/NodeToShardRPC_test.cpp
@@ -21,6 +21,7 @@
 #include <ripple/beast/utility/temp_dir.h>
 #include <ripple/core/ConfigSections.h>
 #include <ripple/nodestore/DatabaseShard.h>
+#include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/jss.h>
 #include <test/jtx/Env.h>
 
@@ -52,6 +53,76 @@ class NodeToShardRPC_test : public beast::unit_test::suite
     }
 
 public:
+    void
+    testDisabled()
+    {
+        testcase("Disabled");
+
+        beast::temp_dir tempDir;
+
+        jtx::Env env = [&] {
+            auto c = jtx::envconfig();
+            auto& sectionNode = c->section(ConfigSection::nodeDatabase());
+            sectionNode.set("earliest_seq", "257");
+            sectionNode.set("ledgers_per_shard", "256");
+            c->setupControl(true, true, true);
+
+            return jtx::Env(*this, std::move(c));
+        }();
+
+        std::uint8_t const numberOfShards = 10;
+
+        // Create some ledgers so that we can initiate a
+        // shard store database import.
+        for (int i = 0; i < 256 * (numberOfShards + 1); ++i)
+        {
+            env.close();
+        }
+
+        {
+            auto shardStore = env.app().getShardStore();
+            if (!BEAST_EXPECT(!shardStore))
+                return;
+        }
+
+        {
+            // Try the node_to_shard status RPC command. Should fail.
+
+            Json::Value jvParams;
+            jvParams[jss::action] = "status";
+
+            auto const result = env.rpc(
+                "json", "node_to_shard", to_string(jvParams))[jss::result];
+
+            BEAST_EXPECT(result[jss::error_code] == rpcNOT_ENABLED);
+        }
+
+        {
+            // Try to start a shard store import via the RPC
+            // interface. Should fail.
+
+            Json::Value jvParams;
+            jvParams[jss::action] = "start";
+
+            auto const result = env.rpc(
+                "json", "node_to_shard", to_string(jvParams))[jss::result];
+
+            BEAST_EXPECT(result[jss::error_code] == rpcNOT_ENABLED);
+        }
+
+        {
+            // Try the node_to_shard status RPC command. Should fail.
+
+            Json::Value jvParams;
+            jvParams[jss::action] = "status";
+
+            auto const result = env.rpc(
+                "json", "node_to_shard", to_string(jvParams))[jss::result];
+
+            BEAST_EXPECT(result[jss::error_code] == rpcNOT_ENABLED);
+        }
+    }
+
     void
     testStart()
     {
@@ -321,6 +392,7 @@ public:
     void
     run() override
     {
+        testDisabled();
         testStart();
         testStop();
     }