rpc: split spork manipulation logic to distinct "sporkupdate" call

doc/release-notes-4885.md

@@ -0,0 +1,4 @@
+Updated RPCs
+------------
+
+* The `spork` RPC call will no longer offer both get (labelled as "basic mode") and set (labelled as "advanced mode") functionality. `spork` will now only offer "basic" functionality. "Advanced" functionality is now exposed through the `sporkupdate` RPC call.

src/rpc/client.cpp

@@ -164,7 +164,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "getmempooldescendants", 1, "verbose" },
     { "logging", 0, "include" },
     { "logging", 1, "exclude" },
-    { "spork", 1, "value" },
+    { "sporkupdate", 1, "value" },
     { "voteraw", 1, "tx_index" },
     { "voteraw", 5, "time" },
     { "getblockhashes", 0, "high"},

src/rpc/misc.cpp

@@ -120,80 +120,85 @@ static UniValue mnsync(const JSONRPCRequest& request)
 */
 static UniValue spork(const JSONRPCRequest& request)
 {
-    if (request.params.size() == 1) {
-        // basic mode, show info
-        std:: string strCommand = request.params[0].get_str();
-        if (strCommand == "show") {
-            UniValue ret(UniValue::VOBJ);
-            for (const auto& sporkDef : sporkDefs) {
-                ret.pushKV(std::string(sporkDef.name), sporkManager.GetSporkValue(sporkDef.sporkId));
-            }
-            return ret;
-        } else if(strCommand == "active"){
-            UniValue ret(UniValue::VOBJ);
-            for (const auto& sporkDef : sporkDefs) {
-                ret.pushKV(std::string(sporkDef.name), sporkManager.IsSporkActive(sporkDef.sporkId));
-            }
-            return ret;
+    // default help, for basic mode
+    RPCHelpMan{"spork",
+        "\nShows information about current state of sporks\n",
+        {
+            {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'show' to show all current spork values, 'active' to show which sporks are active"},
+        },
+        {
+            RPCResult{"For 'show'",
+                RPCResult::Type::OBJ_DYN, "", "keys are the sporks, and values indicates its value",
+                {
+                    {RPCResult::Type::NUM, "SPORK_NAME", "The value of the specific spork with the name SPORK_NAME"},
+                }},
+            RPCResult{"For 'active'",
+                RPCResult::Type::OBJ_DYN, "", "keys are the sporks, and values indicates its status",
+                {
+                    {RPCResult::Type::BOOL, "SPORK_NAME", "'true' for time-based sporks if spork is active and 'false' otherwise"},
+                }},
+        },
+        RPCExamples {
+            HelpExampleCli("spork", "show")
+            + HelpExampleRpc("spork", "\"show\"")
         }
+    }.Check(request);
+
+    // basic mode, show info
+    std:: string strCommand = request.params[0].get_str();
+    if (strCommand == "show") {
+        UniValue ret(UniValue::VOBJ);
+        for (const auto& sporkDef : sporkDefs) {
+            ret.pushKV(std::string(sporkDef.name), sporkManager.GetSporkValue(sporkDef.sporkId));
+        }
+        return ret;
+    } else if(strCommand == "active"){
+        UniValue ret(UniValue::VOBJ);
+        for (const auto& sporkDef : sporkDefs) {
+            ret.pushKV(std::string(sporkDef.name), sporkManager.IsSporkActive(sporkDef.sporkId));
+        }
+        return ret;
     }
 
-    if (request.fHelp || request.params.size() != 2) {
-        // default help, for basic mode
-            RPCHelpMan{"spork",
-                "\nShows information about current state of sporks\n",
-                {
-                    {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'show' to show all current spork values, 'active' to show which sporks are active"},
-                },
-                {
-                    RPCResult{"For 'show'",
-                        RPCResult::Type::OBJ_DYN, "", "keys are the sporks, and values indicates its value",
-                        {
-                            {RPCResult::Type::NUM, "SPORK_NAME", "The value of the specific spork with the name SPORK_NAME"},
-                        }},
-                    RPCResult{"For 'active'",
-                        RPCResult::Type::OBJ_DYN, "", "keys are the sporks, and values indicates its status",
-                        {
-                            {RPCResult::Type::BOOL, "SPORK_NAME", "'true' for time-based sporks if spork is active and 'false' otherwise"},
-                        }},
-                },
-                RPCExamples {
-                    HelpExampleCli("spork", "show")
-                    + HelpExampleRpc("spork", "\"show\"")
-            }}.Check(request);
-    } else {
-        // advanced mode, update spork values
-        SporkId nSporkID = CSporkManager::GetSporkIDByName(request.params[0].get_str());
-        if(nSporkID == SPORK_INVALID)
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid spork name");
+    return NullUniValue;
+}
 
-        NodeContext& node = EnsureNodeContext(request.context);
-        if (!node.connman)
-            throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
+static UniValue sporkupdate(const JSONRPCRequest& request)
+{
+    RPCHelpMan{"sporkupdate",
+        "\nUpdate the value of the specific spork. Requires \"-sporkkey\" to be set to sign the message.\n",
+        {
+            {"name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name of the spork to update"},
+            {"value", RPCArg::Type::NUM, RPCArg::Optional::NO, "The new desired value of the spork"},
+        },
+        RPCResult{
+            RPCResult::Type::STR, "result", "\"success\" if spork value was updated or this help otherwise"
+        },
+        RPCExamples{
+            HelpExampleCli("sporkupdate", "SPORK_2_INSTANTSEND_ENABLED 4070908800")
+            + HelpExampleRpc("sporkupdate", "\"SPORK_2_INSTANTSEND_ENABLED\", 4070908800")
+        },
+    }.Check(request);
 
-        // SPORK VALUE
-        int64_t nValue = request.params[1].get_int64();
+    // advanced mode, update spork values
+    SporkId nSporkID = CSporkManager::GetSporkIDByName(request.params[0].get_str());
+    if (nSporkID == SPORK_INVALID) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid spork name");
+    }
 
-        //broadcast new spork
-        if(sporkManager.UpdateSpork(nSporkID, nValue, *node.connman)){
-            return "success";
-        } else {
-                RPCHelpMan{"spork",
-                    "\nUpdate the value of the specific spork. Requires \"-sporkkey\" to be set to sign the message.\n",
-                    {
-                        {"name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name of the spork to update"},
-                        {"value", RPCArg::Type::NUM, RPCArg::Optional::NO, "The new desired value of the spork"},
-                    },
-                    RPCResult{
-                        RPCResult::Type::STR, "result", "\"success\" if spork value was updated or this help otherwise"
-                    },
-                    RPCExamples{
-                        HelpExampleCli("spork", "SPORK_2_INSTANTSEND_ENABLED 4070908800")
-                + HelpExampleRpc("spork", "\"SPORK_2_INSTANTSEND_ENABLED\", 4070908800")
-                    },
-                }.Check(request);
-        }
+    NodeContext& node = EnsureNodeContext(request.context);
+    if (!node.connman) {
+        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
     }
+
+    // SPORK VALUE
+    int64_t nValue = request.params[1].get_int64();
+
+    // broadcast new spork
+    if (sporkManager.UpdateSpork(nSporkID, nValue, *node.connman)) {
+        return "success";
+    }
+
     return NullUniValue;
 }
 
@@ -1317,7 +1322,8 @@ static const CRPCCommand commands[] =
 
     /* Dash features */
     { "dash",               "mnsync",                 &mnsync,                 {} },
-    { "dash",               "spork",                  &spork,                  {"arg0","value"} },
+    { "dash",               "spork",                  &spork,                  {"command"} },
+    { "dash",               "sporkupdate",            &sporkupdate,            {"name","value"} },
 
     /* Not shown in help */
     { "hidden",             "setmocktime",            &setmocktime,            {"timestamp"}},

test/functional/feature_dip4_coinbasemerkleroots.py

@@ -87,7 +87,7 @@ def run_test(self):
         self.nodes[0].generate(1)
         oldhash = self.nodes[0].getbestblockhash()
         # Have to disable ChainLocks here because they won't let you to invalidate already locked blocks
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
         self.wait_for_sporks_same()
         # Test DIP8 activation once with a pre-existing quorum and once without (we don't know in which order it will activate on mainnet)
         self.test_dip8_quorum_merkle_root_activation(True)
@@ -97,8 +97,8 @@ def run_test(self):
         first_quorum = self.test_dip8_quorum_merkle_root_activation(False, True)
 
         # Re-enable ChainLocks again
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 0)
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         # Verify that the first quorum appears in MNLISTDIFF
@@ -243,13 +243,13 @@ def test_getmnlistdiff_base(self, baseBlockHash, blockHash):
 
     def test_dip8_quorum_merkle_root_activation(self, with_initial_quorum, slow_mode=False):
         if with_initial_quorum:
-            self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+            self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
             self.wait_for_sporks_same()
 
             # Mine one quorum before dip8 is activated
             self.mine_quorum()
 
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 4070908800)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 4070908800)
         self.wait_for_sporks_same()
 
         cbtx = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 2)["tx"][0]
@@ -269,7 +269,7 @@ def test_dip8_quorum_merkle_root_activation(self, with_initial_quorum, slow_mode
             assert_equal(merkleRootQuorums, 0)
 
         self.bump_mocktime(1)
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         # Mine quorum and verify that merkleRootQuorums has changed

test/functional/feature_llmq_chainlocks.py

@@ -31,7 +31,7 @@ def run_test(self):
 
         self.activate_dip8()
 
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         self.log.info("Mining 4 quorums")
@@ -144,10 +144,10 @@ def run_test(self):
         assert not node0_tip_block["chainlock"]
         assert node0_tip_block["previousblockhash"] == good_tip
         self.log.info("Disable LLMQ based InstantSend for a very short time (this never gets propagated to other nodes)")
-        self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 4070908800)
+        self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 4070908800)
         self.log.info("Now the TXs should be included")
         self.nodes[0].generate(1)
-        self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
         self.log.info("Assert that TXs got included now")
         for txid in txs:
             tx = self.nodes[0].getrawtransaction(txid, 1)

test/functional/feature_llmq_connections.py

@@ -22,7 +22,7 @@ def set_test_params(self):
         self.set_dash_llmq_test_params(5, 3)
 
     def run_test(self):
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         q = self.mine_quorum()
@@ -38,7 +38,7 @@ def run_test(self):
         self.check_reconnects(2)
 
         self.log.info("Activating SPORK_23_QUORUM_POSE")
-        self.nodes[0].spork("SPORK_23_QUORUM_POSE", 0)
+        self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 0)
         self.wait_for_sporks_same()
 
         self.log.info("mining one block and waiting for all members to connect to each other")
@@ -64,7 +64,7 @@ def run_test(self):
             wait_until(lambda: self.get_mn_probe_count(mn.node, q, True) == 4)
 
         self.log.info("Activating SPORK_21_QUORUM_ALL_CONNECTED")
-        self.nodes[0].spork("SPORK_21_QUORUM_ALL_CONNECTED", 0)
+        self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
         self.wait_for_sporks_same()
 
         self.check_reconnects(4)

test/functional/feature_llmq_data_recovery.py

@@ -121,8 +121,8 @@ def test_llmq_qvvec_sync(self, llmq_sync_entries):
     def run_test(self):
 
         node = self.nodes[0]
-        node.spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
-        node.spork("SPORK_21_QUORUM_ALL_CONNECTED", 0)
+        node.sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        node.sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
         self.wait_for_sporks_same()
         self.activate_dip8()
 

test/functional/feature_llmq_dkgerrors.py

@@ -19,7 +19,7 @@ def set_test_params(self):
     def run_test(self):
         self.activate_dip8()
 
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         self.log.info("Mine one quorum without simulating any errors")
@@ -83,13 +83,13 @@ def assert_member_valid(self, quorumHash, proTxHash, expectedValid):
 
     def heal_masternodes(self, blockCount):
         # We're not testing PoSe here, so lets heal the MNs :)
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 4070908800)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 4070908800)
         self.wait_for_sporks_same()
         for i in range(blockCount):
             self.bump_mocktime(1)
             self.nodes[0].generate(1)
         self.sync_all()
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
 

test/functional/feature_llmq_is_cl_conflicts.py

@@ -60,7 +60,7 @@ def run_test(self):
 
         self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn())
 
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
         self.mine_quorum()
@@ -216,7 +216,7 @@ def test_chainlock_overrides_islock_overrides_nonchainlock(self, deterministic):
         # Ensure spork uniqueness in multiple function runs
         self.bump_mocktime(1)
         # Disable ChainLocks to avoid accidental locking
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
         self.wait_for_sporks_same()
 
         # Send tx1, which will later conflict with the ISLOCK
@@ -268,7 +268,7 @@ def test_chainlock_overrides_islock_overrides_nonchainlock(self, deterministic):
             assert_equal(node.getbestblockhash(), islock_tip)
 
         # Check that the CL-ed block overrides the one with islocks
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 0)  # Re-enable ChainLocks to accept clsig
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)  # Re-enable ChainLocks to accept clsig
         self.test_node.send_clsig(cl)  # relay clsig ASAP to prevent nodes from locking islock-ed tip
         self.wait_for_sporks_same()
         for node in self.nodes:

test/functional/feature_llmq_is_migration.py

@@ -38,8 +38,8 @@ def run_test(self):
         self.activate_dip8()
 
         node = self.nodes[0]
-        node.spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
-        node.spork("SPORK_2_INSTANTSEND_ENABLED", 0)
+        node.sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        node.sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
         self.wait_for_sporks_same()
 
         self.mine_quorum()

test/functional/feature_llmq_is_retroactive.py

@@ -28,9 +28,9 @@ def set_test_params(self):
     def run_test(self):
         self.activate_dip8()
 
-        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         # Turn mempool IS signing off
-        self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 1)
+        self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 1)
         self.wait_for_sporks_same()
 
         self.mine_quorum()
@@ -45,18 +45,18 @@ def run_test(self):
         # are the only "neighbours" in intra-quorum connections for one of them.
         self.wait_for_instantlock(txid, self.nodes[0], False, 5)
         # Have to disable ChainLocks to avoid signing a block with a "safe" tx too early
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 4000000000)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4000000000)
         self.wait_for_sporks_same()
         # We have to wait in order to include tx in block
         self.bump_mocktime(10 * 60 + 1)
         block = self.nodes[0].generate(1)[0]
         self.wait_for_instantlock(txid, self.nodes[0])
-        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)
         self.wait_for_sporks_same()
         self.wait_for_chainlocked_block_all_nodes(block)
 
         self.log.info("Enable mempool IS signing")
-        self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
         self.wait_for_sporks_same()
 
         self.log.info("trying normal IS lock")