Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rpc)!: remove spork active and its test, spork=spork show, update spork/sporkupdate help text #6207

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/release-notes-6207.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
RPC changes
-----------

- `spork` no longer have `active` and `show` commands, it always shows actual raw values now. `command` argument is deprecated and ignored.
36 changes: 10 additions & 26 deletions src/rpc/misc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,56 +149,40 @@ static RPCHelpMan spork()
{
// default help, for basic mode
return RPCHelpMan{"spork",
"\nShows information about current state of sporks\n",
"\nShows information about current state of sporks for non-mainnet networks. Mainnet values are hardcoded.\n",
{
{"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'show' to show all current spork values, 'active' to show which sporks are active"},
{"command", RPCArg::Type::STR, RPCArg::Default{""}, "Deprecated and ignored"},
},
{
RPCResult{"For 'show'",
RPCResult{
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"},
}},
Comment on lines -162 to -166
Copy link
Member

@PastaPastaPasta PastaPastaPasta Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dropping this param is a breaking change

},
RPCExamples {
HelpExampleCli("spork", "show")
+ HelpExampleRpc("spork", "\"show\"")
HelpExampleCli("spork", "")
+ HelpExampleRpc("spork", "")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{

// basic mode, show info
std:: string strCommand = request.params[0].get_str();
const NodeContext& node = EnsureAnyNodeContext(request.context);
CHECK_NONFATAL(node.sporkman);
if (strCommand == "show") {
UniValue ret(UniValue::VOBJ);
for (const auto& sporkDef : sporkDefs) {
ret.pushKV(std::string(sporkDef.name), node.sporkman->GetSporkValue(sporkDef.sporkId));
}
return ret;
} else if(strCommand == "active"){
UniValue ret(UniValue::VOBJ);
for (const auto& sporkDef : sporkDefs) {
ret.pushKV(std::string(sporkDef.name), node.sporkman->IsSporkActive(sporkDef.sporkId));
}
return ret;
UniValue ret(UniValue::VOBJ);
for (const auto& sporkDef : sporkDefs) {
ret.pushKV(std::string(sporkDef.name), node.sporkman->GetSporkValue(sporkDef.sporkId));
}

return NullUniValue;
return ret;
},
};
}

static RPCHelpMan sporkupdate()
{
return RPCHelpMan{"sporkupdate",
"\nUpdate the value of the specific spork. Requires \"-sporkkey\" to be set to sign the message.\n",
"\nUpdate the value of the specific spork on non-mainnet networks. 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"},
Expand Down
2 changes: 1 addition & 1 deletion test/functional/feature_multikeysporks.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def setup_network(self):

def get_test_spork_value(self, node, spork_name):
self.bump_mocktime(5) # advance ProcessTick
info = node.spork('show')
info = node.spork()
# use InstantSend spork for tests
return info[spork_name]

Expand Down
29 changes: 12 additions & 17 deletions test/functional/feature_sporks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,44 +20,39 @@ def setup_network(self):
# connect only 2 first nodes at start
self.connect_nodes(0, 1)

def get_test_spork_state(self, node):
info = node.spork('active')
def get_test_spork_value(self, node):
# use InstantSend spork for tests
return info['SPORK_2_INSTANTSEND_ENABLED']
return node.spork()['SPORK_2_INSTANTSEND_ENABLED']

def set_test_spork_state(self, node, state):
if state:
value = 0
else:
value = 4070908800
def set_test_spork_value(self, node, value):
# use InstantSend spork for tests
node.sporkupdate("SPORK_2_INSTANTSEND_ENABLED", value)

def run_test(self):
spork_default_state = self.get_test_spork_state(self.nodes[0])
spork_default_value = self.get_test_spork_value(self.nodes[0])
# check test spork default state matches on all nodes
assert self.get_test_spork_state(self.nodes[1]) == spork_default_state
assert self.get_test_spork_state(self.nodes[2]) == spork_default_state
assert self.get_test_spork_value(self.nodes[1]) == spork_default_value
assert self.get_test_spork_value(self.nodes[2]) == spork_default_value

# check spork propagation for connected nodes
spork_new_state = not spork_default_state
self.set_test_spork_state(self.nodes[0], spork_new_state)
self.wait_until(lambda: self.get_test_spork_state(self.nodes[1]), timeout=10)
spork_new_value = spork_default_value + 1
self.set_test_spork_value(self.nodes[0], spork_new_value)
self.wait_until(lambda: self.get_test_spork_value(self.nodes[1]) == spork_new_value, timeout=10)

# restart nodes to check spork persistence
self.stop_node(0)
self.stop_node(1)
self.start_node(0)
self.start_node(1)
assert self.get_test_spork_state(self.nodes[0]) == spork_new_state
assert self.get_test_spork_state(self.nodes[1]) == spork_new_state
assert self.get_test_spork_value(self.nodes[0]) == spork_new_value
assert self.get_test_spork_value(self.nodes[1]) == spork_new_value

# Generate one block to kick off masternode sync, which also starts sporks syncing for node2
self.generate(self.nodes[1], 1, sync_fun=self.no_op)

# connect new node and check spork propagation after restoring from cache
self.connect_nodes(1, 2)
self.wait_until(lambda: self.get_test_spork_state(self.nodes[2]), timeout=10)
self.wait_until(lambda: self.get_test_spork_value(self.nodes[2]) == spork_new_value, timeout=10)

if __name__ == '__main__':
SporkTest().main()
14 changes: 7 additions & 7 deletions test/functional/test_framework/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,7 @@ def activate_by_name(self, name, expected_activation_height=None):
self.log.info("Wait for " + name + " activation")

# disable spork17 while mining blocks to activate "name" to prevent accidental quorum formation
spork17_value = self.nodes[0].spork('show')['SPORK_17_QUORUM_DKG_ENABLED']
spork17_value = self.nodes[0].spork()['SPORK_17_QUORUM_DKG_ENABLED']
self.bump_mocktime(1)
self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 4070908800)
self.wait_for_sporks_same()
Expand Down Expand Up @@ -1674,8 +1674,8 @@ def wait_for_best_chainlock(self, node, block_hash, timeout=15):
def wait_for_sporks_same(self, timeout=30):
def check_sporks_same():
self.bump_mocktime(1)
sporks = self.nodes[0].spork('show')
return all(node.spork('show') == sporks for node in self.nodes[1:])
sporks = self.nodes[0].spork()
return all(node.spork() == sporks for node in self.nodes[1:])
wait_until_helper(check_sporks_same, timeout=timeout, sleep=0.5)

def wait_for_quorum_connections(self, quorum_hash, expected_connections, mninfos, llmq_type_name="llmq_test", timeout = 60, wait_proc=None):
Expand Down Expand Up @@ -1824,8 +1824,8 @@ def move_blocks(self, nodes, num_blocks):
self.generate(self.nodes[0], num_blocks, sync_fun=lambda: self.sync_blocks(nodes))

def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None):
spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1
spork21_active = self.nodes[0].spork()['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
spork23_active = self.nodes[0].spork()['SPORK_23_QUORUM_POSE'] <= 1

if expected_connections is None:
expected_connections = (self.llmq_size - 1) if spork21_active else 2
Expand Down Expand Up @@ -1909,8 +1909,8 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec
return new_quorum

def mine_cycle_quorum(self, llmq_type_name="llmq_test_dip0024", llmq_type=103, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None):
spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1
spork21_active = self.nodes[0].spork()['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
spork23_active = self.nodes[0].spork()['SPORK_23_QUORUM_POSE'] <= 1

if expected_connections is None:
expected_connections = (self.llmq_size_dip0024 - 1) if spork21_active else 2
Expand Down
Loading