Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
5 changes: 1 addition & 4 deletions src/llmq/options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ static bool EvalSpork(const Consensus::LLMQType llmqType, const int64_t spork_va
return false;
}

bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman)
{
return EvalSpork(llmqType, sporkman.GetSporkValue(SPORK_21_QUORUM_ALL_CONNECTED));
}
bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { return true; }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

c96395b: You can't do that. Mainnet is running with SPORK_21_QUORUM_ALL_CONNECTED: 1 meaning that it's true for small quorums only. For large quorums it's false for a reason - connecting each node of a large quorum is too resource hungry. Also, simply dropping the old quorum connection logic means that nodes that update to this version will try to use "wrong" quorum connection logic (from the point of view of non-updated nodes) and new nodes could be pose-banned or quorum creation could fail, or both.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All you can do here is replace sporkman.GetSporkValue(SPORK_21_QUORUM_ALL_CONNECTED) with 1 and drop const CSporkManager& sporkman from params. And that's pretty much it imo.

Copy link
Copy Markdown
Collaborator Author

@knst knst Nov 13, 2025

Choose a reason for hiding this comment

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

ah, that's terrible! 😡 I just though I found a short-cut to make 6984 much simpler for signing-shares.
Anyway, nothing to do then.

I miss the fact that EvalSpork actually do something magic

    if (spork_value == 1 && llmqType != Consensus::LLMQType::LLMQ_100_67 && llmqType != Consensus::LLMQType::LLMQ_400_60 && llmqType != Consensus::LLMQType::LLMQ_400_85) {


bool IsQuorumPoseEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman)
{
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing_shares.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms
// non-masternodes are not interested in sigshares
if (m_mn_activeman.GetProTxHash().IsNull()) return;

if (m_sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
if (msg_type == NetMsgType::QSIGSHARE) {
std::vector<CSigShare> receivedSigShares;
vRecv >> receivedSigShares;

Expand Down
7 changes: 1 addition & 6 deletions src/spork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,7 @@ SporkValue CSporkManager::GetSporkValue(SporkId nSporkID) const
{
// Harden all sporks on Mainnet
if (!Params().IsTestChain()) {
switch (nSporkID) {
case SPORK_21_QUORUM_ALL_CONNECTED:
return 1;
default:
return 0;
}
return 0;
}

LOCK(cs);
Expand Down
5 changes: 2 additions & 3 deletions src/spork.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ enum SporkId : int32_t {
SPORK_9_SUPERBLOCKS_ENABLED = 10008,
SPORK_17_QUORUM_DKG_ENABLED = 10016,
SPORK_19_CHAINLOCKS_ENABLED = 10018,
SPORK_21_QUORUM_ALL_CONNECTED = 10020,
SPORK_21_DEPRECATED = 10020,
SPORK_23_QUORUM_POSE = 10022,
// SPORK_24_DEPRECATED = 10023,

Expand Down Expand Up @@ -69,13 +69,12 @@ struct CSporkDef
};

#define MAKE_SPORK_DEF(name, defaultValue) CSporkDef{name, defaultValue, #name}
[[maybe_unused]] static constexpr std::array<CSporkDef, 7> sporkDefs = {
[[maybe_unused]] static constexpr std::array<CSporkDef, 6> sporkDefs = {
MAKE_SPORK_DEF(SPORK_2_INSTANTSEND_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_3_INSTANTSEND_BLOCK_FILTERING, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_9_SUPERBLOCKS_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_17_QUORUM_DKG_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_19_CHAINLOCKS_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_21_QUORUM_ALL_CONNECTED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_23_QUORUM_POSE, 4070908800ULL), // OFF
};
#undef MAKE_SPORK_DEF
Expand Down
4 changes: 0 additions & 4 deletions test/functional/feature_llmq_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ def run_test(self):
for mn in self.get_quorum_masternodes(q):
self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, True) == 4)

self.log.info("Activating SPORK_21_QUORUM_ALL_CONNECTED")
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()

self.check_reconnects(4)

self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 4070908800)
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_llmq_data_recovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ def run_test(self):

node = self.nodes[0]
node.sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
node.sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()

logger.info("Test automated DGK data recovery")
Expand Down
25 changes: 14 additions & 11 deletions test/functional/feature_llmq_is_retroactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ def assert_no_instantlock(self, txid, node):
assert not node.getrawtransaction(txid, True)["instantlock"]

def sleep_and_assert_no_instantlock(self, txid, node, sleep=5):
time.sleep(sleep)
for _ in range(sleep):
time.sleep(1)
self.bump_mocktime(1)
self.assert_no_instantlock(txid, node)

# random delay before tx is actually send by network could take up to 30 seconds
Expand Down Expand Up @@ -63,7 +65,7 @@ def run_test(self):
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
# 3 nodes should be enough to create an IS lock even if nodes 4 and 5 (which have no tx itself)
# are the only "neighbours" in intra-quorum connections for one of them.
self.bump_mocktime(30)
self.bump_mocktime(10)
self.sleep_and_assert_no_instantlock(txid, self.nodes[0])
# Have to disable ChainLocks to avoid signing a block with a "safe" tx too early
self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4000000000)
Expand All @@ -84,7 +86,7 @@ def run_test(self):
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
# 3 nodes should be enough to create an IS lock even if nodes 4 and 5 (which have no tx itself)
# are the only "neighbours" in intra-quorum connections for one of them.
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_instantlock(txid, self.nodes[0])
block = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]
self.wait_for_chainlocked_block_all_nodes(block)
Expand All @@ -94,17 +96,18 @@ def run_test(self):
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
# Make sure nodes 1 and 2 received the TX before we continue,
# otherwise it might announce the TX to node 3 when reconnecting
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_tx(txid, self.nodes[1])
self.wait_for_tx(txid, self.nodes[2])
self.reconnect_isolated_node(3, 0)
# Make sure nodes actually try re-connecting quorum connections
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_mnauth(self.nodes[3], 2)
# node 3 fully reconnected but the TX wasn't relayed to it, so there should be no IS lock
self.sleep_and_assert_no_instantlock(txid, self.nodes[0])
# push the tx directly via rpc
self.nodes[3].sendrawtransaction(self.nodes[0].getrawtransaction(txid))
self.bump_mocktime(10)
# node 3 should vote on a tx now since it became aware of it via sendrawtransaction
# and this should be enough to complete an IS lock
self.wait_for_instantlock(txid, self.nodes[0])
Expand All @@ -125,12 +128,12 @@ def run_test(self):
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
# Make sure nodes 1 and 2 received the TX before we continue,
# otherwise it might announce the TX to node 3 when reconnecting
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_tx(txid, self.nodes[1])
self.wait_for_tx(txid, self.nodes[2])
self.reconnect_isolated_node(3, 0)
# Make sure nodes actually try re-connecting quorum connections
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_mnauth(self.nodes[3], 2)
# node 3 fully reconnected but the TX wasn't relayed to it, so there should be no IS lock
self.sleep_and_assert_no_instantlock(txid, self.nodes[0])
Expand All @@ -157,7 +160,7 @@ def test_session_timeout(self, do_cycle_llmqs):
txid_single_node = self.nodes[3].sendrawtransaction(rawtx_1)

# Make sure nodes 1 and 2 received the TX before we continue
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_tx(txid_all_nodes, self.nodes[1])
self.wait_for_tx(txid_all_nodes, self.nodes[2])
# Make sure signing is done on nodes 1 and 2 (it's async)
Expand All @@ -167,15 +170,15 @@ def test_session_timeout(self, do_cycle_llmqs):
time.sleep(2) # make sure Cleanup() is called
self.reconnect_isolated_node(3, 0)
# Make sure nodes actually try re-connecting quorum connections
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_mnauth(self.nodes[3], 2)

self.nodes[0].sendrawtransaction(rawtx_1)
# Make sure nodes 1 and 2 received the TX
self.bump_mocktime(30)
self.bump_mocktime(10)
self.wait_for_tx(txid_single_node, self.nodes[1])
self.wait_for_tx(txid_single_node, self.nodes[2])
self.bump_mocktime(30)
self.bump_mocktime(10)

# Make sure signing is done on nodes 1 and 2 (it's async)
time.sleep(5)
Expand Down
113 changes: 50 additions & 63 deletions test/functional/feature_llmq_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,14 @@ def set_test_params(self):
self.set_dash_test_params(6, 5)
self.set_dash_llmq_test_params(5, 3)

def add_options(self, parser):
parser.add_argument("--spork21", dest="spork21", default=False, action="store_true",
help="Test with spork21 enabled")

def run_test(self):

self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
if self.options.spork21:
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()

self.mine_quorum()

if self.options.spork21:
assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size
assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size

id = "0000000000000000000000000000000000000000000000000000000000000001"
msgHash = "0000000000000000000000000000000000000000000000000000000000000002"
Expand Down Expand Up @@ -75,42 +68,37 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout):
quorumHash = self.mninfo[1].get_node(self).quorum("selectquorum", q_type, id)["quorumHash"]
assert self.mninfo[1].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash)
assert_sigs_nochange(False, False, False, 3)
# Sign third share and test optional submit parameter if spork21 is enabled, should result in recovered sig
# Sign third share and test optional submit parameter should result in recovered sig
# and conflict for msgHashConflict
if self.options.spork21:
# 1. Providing an invalid quorum hash and set submit=false, should throw an error
assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False)
# 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object
sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False)
sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False)
assert_equal(sig_share_rpc_1, sig_share_rpc_2)
assert_sigs_nochange(False, False, False, 3)
# 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result
# in a recovered sig
sig_share = CSigShare()
sig_share.llmqType = int(sig_share_rpc_1["llmqType"])
sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16)
sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"])
sig_share.id = int(sig_share_rpc_1["id"], 16)
sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16)
sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"])
for mn in self.mninfo: # type: MasternodeInfo
assert mn.get_node(self).getconnectioncount() == self.llmq_size
# Get the current recovery member of the quorum
q = self.nodes[0].quorum('selectquorum', q_type, id)
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
# Open a P2P connection to it
p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface())
# Send the last required QSIGSHARE message to the recovery member
p2p_interface.send_message(msg_qsigshare([sig_share]))
else:
# If spork21 is not enabled just sign regularly
self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash)
# 1. Providing an invalid quorum hash and set submit=false, should throw an error
assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False)
# 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object
sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False)
sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False)
assert_equal(sig_share_rpc_1, sig_share_rpc_2)
assert_sigs_nochange(False, False, False, 3)
# 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result
# in a recovered sig
sig_share = CSigShare()
sig_share.llmqType = int(sig_share_rpc_1["llmqType"])
sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16)
sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"])
sig_share.id = int(sig_share_rpc_1["id"], 16)
sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16)
sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"])
for mn in self.mninfo: # type: MasternodeInfo
assert mn.get_node(self).getconnectioncount() == self.llmq_size
# Get the current recovery member of the quorum
q = self.nodes[0].quorum('selectquorum', q_type, id)
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
# Open a P2P connection to it
p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface())
# Send the last required QSIGSHARE message to the recovery member
p2p_interface.send_message(msg_qsigshare([sig_share]))

wait_for_sigs(True, False, True, 15)

if self.options.spork21:
mn.get_node(self).disconnect_p2ps()
mn.get_node(self).disconnect_p2ps()

# Test `quorum verify` rpc
node = self.mninfo[0].get_node(self)
Expand Down Expand Up @@ -175,29 +163,28 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout):
self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash)
wait_for_sigs(True, False, True, 15)

if self.options.spork21:
id = uint256_to_string(request_id + 1)

# Isolate the node that is responsible for the recovery of a signature and assert that recovery fails
q = self.nodes[0].quorum('selectquorum', q_type, id)
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
mn.get_node(self).setnetworkactive(False)
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0)
for i in range(4):
self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash)
assert_sigs_nochange(False, False, False, 3)
# Need to re-connect so that it later gets the recovered sig
mn.get_node(self).setnetworkactive(True)
self.connect_nodes(mn.nodeIdx, 0)
force_finish_mnsync(mn.get_node(self))
# Make sure intra-quorum connections were also restored
self.bump_mocktime(1) # need this to bypass quorum connection retry timeout
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10)
mn.get_node(self).ping()
self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo()))
# Let 2 seconds pass so that the next node is used for recovery, which should succeed
self.bump_mocktime(2)
wait_for_sigs(True, False, True, 2)
id = uint256_to_string(request_id + 1)

# Isolate the node that is responsible for the recovery of a signature and assert that recovery fails
q = self.nodes[0].quorum('selectquorum', q_type, id)
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
mn.get_node(self).setnetworkactive(False)
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0)
for i in range(4):
self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash)
assert_sigs_nochange(False, False, False, 3)
# Need to re-connect so that it later gets the recovered sig
mn.get_node(self).setnetworkactive(True)
self.connect_nodes(mn.nodeIdx, 0)
force_finish_mnsync(mn.get_node(self))
# Make sure intra-quorum connections were also restored
self.bump_mocktime(1) # need this to bypass quorum connection retry timeout
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10)
mn.get_node(self).ping()
self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo()))
# Let 2 seconds pass so that the next node is used for recovery, which should succeed
self.bump_mocktime(2)
wait_for_sigs(True, False, True, 2)

if __name__ == '__main__':
LLMQSigningTest().main()
4 changes: 0 additions & 4 deletions test/functional/feature_llmq_simplepose.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ def run_test(self):
self.test_banning(self.isolate_mn, 2)

self.repair_masternodes(False)

self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()

self.reset_probe_timeouts()

if not self.options.disable_spork23:
Expand Down
Loading