Skip to content

Commit c96395b

Browse files
knstknst
committed
refactor: drop spork21 as always active
Downside of having spork21: - llmq code run in 2 different modes depending it is active or not active. It complicates implementation while we not actually need a case of "inactive" - it increase scope of test coverage; functional test to test it takes 2 minutes to run and it increase cost of CI, making CI slower, making local run of functional tests noticeable slower See previuos commit: functional test feature_llmq_is_retroactive.py actually had been broken for awhile, but disabled spork21 masked it
1 parent 7c39004 commit c96395b

File tree

11 files changed

+57
-92
lines changed

11 files changed

+57
-92
lines changed

‎src/llmq/options.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,7 @@ static bool EvalSpork(const Consensus::LLMQType llmqType, const int64_t spork_va
3131
return false;
3232
}
3333

34-
bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman)
35-
{
36-
return EvalSpork(llmqType, sporkman.GetSporkValue(SPORK_21_QUORUM_ALL_CONNECTED));
37-
}
34+
bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { return true; }
3835

3936
bool IsQuorumPoseEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman)
4037
{

‎src/llmq/signing_shares.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms
238238
// non-masternodes are not interested in sigshares
239239
if (m_mn_activeman.GetProTxHash().IsNull()) return;
240240

241-
if (m_sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
241+
if (msg_type == NetMsgType::QSIGSHARE) {
242242
std::vector<CSigShare> receivedSigShares;
243243
vRecv >> receivedSigShares;
244244

‎src/spork.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,7 @@ SporkValue CSporkManager::GetSporkValue(SporkId nSporkID) const
262262
{
263263
// Harden all sporks on Mainnet
264264
if (!Params().IsTestChain()) {
265-
switch (nSporkID) {
266-
case SPORK_21_QUORUM_ALL_CONNECTED:
267-
return 1;
268-
default:
269-
return 0;
270-
}
265+
return 0;
271266
}
272267

273268
LOCK(cs);

‎src/spork.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ enum SporkId : int32_t {
4141
SPORK_9_SUPERBLOCKS_ENABLED = 10008,
4242
SPORK_17_QUORUM_DKG_ENABLED = 10016,
4343
SPORK_19_CHAINLOCKS_ENABLED = 10018,
44-
SPORK_21_QUORUM_ALL_CONNECTED = 10020,
44+
SPORK_21_DEPRECATED = 10020,
4545
SPORK_23_QUORUM_POSE = 10022,
4646
// SPORK_24_DEPRECATED = 10023,
4747

@@ -69,13 +69,12 @@ struct CSporkDef
6969
};
7070

7171
#define MAKE_SPORK_DEF(name, defaultValue) CSporkDef{name, defaultValue, #name}
72-
[[maybe_unused]] static constexpr std::array<CSporkDef, 7> sporkDefs = {
72+
[[maybe_unused]] static constexpr std::array<CSporkDef, 6> sporkDefs = {
7373
MAKE_SPORK_DEF(SPORK_2_INSTANTSEND_ENABLED, 4070908800ULL), // OFF
7474
MAKE_SPORK_DEF(SPORK_3_INSTANTSEND_BLOCK_FILTERING, 4070908800ULL), // OFF
7575
MAKE_SPORK_DEF(SPORK_9_SUPERBLOCKS_ENABLED, 4070908800ULL), // OFF
7676
MAKE_SPORK_DEF(SPORK_17_QUORUM_DKG_ENABLED, 4070908800ULL), // OFF
7777
MAKE_SPORK_DEF(SPORK_19_CHAINLOCKS_ENABLED, 4070908800ULL), // OFF
78-
MAKE_SPORK_DEF(SPORK_21_QUORUM_ALL_CONNECTED, 4070908800ULL), // OFF
7978
MAKE_SPORK_DEF(SPORK_23_QUORUM_POSE, 4070908800ULL), // OFF
8079
};
8180
#undef MAKE_SPORK_DEF

‎test/functional/feature_llmq_connections.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ def run_test(self):
6868
for mn in self.get_quorum_masternodes(q):
6969
self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, True) == 4)
7070

71-
self.log.info("Activating SPORK_21_QUORUM_ALL_CONNECTED")
72-
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
73-
self.wait_for_sporks_same()
74-
7571
self.check_reconnects(4)
7672

7773
self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 4070908800)

‎test/functional/feature_llmq_data_recovery.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ def run_test(self):
158158

159159
node = self.nodes[0]
160160
node.sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
161-
node.sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
162161
self.wait_for_sporks_same()
163162

164163
logger.info("Test automated DGK data recovery")

‎test/functional/feature_llmq_is_retroactive.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ def create_fund_sign_tx(self):
5252

5353
def run_test(self):
5454
self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
55-
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
5655
# Turn mempool IS signing off
5756
self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 1)
5857
self.wait_for_sporks_same()

‎test/functional/feature_llmq_signing.py‎

Lines changed: 50 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,14 @@ def set_test_params(self):
2525
self.set_dash_test_params(6, 5)
2626
self.set_dash_llmq_test_params(5, 3)
2727

28-
def add_options(self, parser):
29-
parser.add_argument("--spork21", dest="spork21", default=False, action="store_true",
30-
help="Test with spork21 enabled")
31-
3228
def run_test(self):
3329

3430
self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
35-
if self.options.spork21:
36-
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
3731
self.wait_for_sporks_same()
3832

3933
self.mine_quorum()
4034

41-
if self.options.spork21:
42-
assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size
35+
assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size
4336

4437
id = "0000000000000000000000000000000000000000000000000000000000000001"
4538
msgHash = "0000000000000000000000000000000000000000000000000000000000000002"
@@ -75,42 +68,37 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout):
7568
quorumHash = self.mninfo[1].get_node(self).quorum("selectquorum", q_type, id)["quorumHash"]
7669
assert self.mninfo[1].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash)
7770
assert_sigs_nochange(False, False, False, 3)
78-
# Sign third share and test optional submit parameter if spork21 is enabled, should result in recovered sig
71+
# Sign third share and test optional submit parameter should result in recovered sig
7972
# and conflict for msgHashConflict
80-
if self.options.spork21:
81-
# 1. Providing an invalid quorum hash and set submit=false, should throw an error
82-
assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False)
83-
# 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object
84-
sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False)
85-
sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False)
86-
assert_equal(sig_share_rpc_1, sig_share_rpc_2)
87-
assert_sigs_nochange(False, False, False, 3)
88-
# 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result
89-
# in a recovered sig
90-
sig_share = CSigShare()
91-
sig_share.llmqType = int(sig_share_rpc_1["llmqType"])
92-
sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16)
93-
sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"])
94-
sig_share.id = int(sig_share_rpc_1["id"], 16)
95-
sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16)
96-
sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"])
97-
for mn in self.mninfo: # type: MasternodeInfo
98-
assert mn.get_node(self).getconnectioncount() == self.llmq_size
99-
# Get the current recovery member of the quorum
100-
q = self.nodes[0].quorum('selectquorum', q_type, id)
101-
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
102-
# Open a P2P connection to it
103-
p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface())
104-
# Send the last required QSIGSHARE message to the recovery member
105-
p2p_interface.send_message(msg_qsigshare([sig_share]))
106-
else:
107-
# If spork21 is not enabled just sign regularly
108-
self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash)
73+
# 1. Providing an invalid quorum hash and set submit=false, should throw an error
74+
assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False)
75+
# 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object
76+
sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False)
77+
sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False)
78+
assert_equal(sig_share_rpc_1, sig_share_rpc_2)
79+
assert_sigs_nochange(False, False, False, 3)
80+
# 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result
81+
# in a recovered sig
82+
sig_share = CSigShare()
83+
sig_share.llmqType = int(sig_share_rpc_1["llmqType"])
84+
sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16)
85+
sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"])
86+
sig_share.id = int(sig_share_rpc_1["id"], 16)
87+
sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16)
88+
sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"])
89+
for mn in self.mninfo: # type: MasternodeInfo
90+
assert mn.get_node(self).getconnectioncount() == self.llmq_size
91+
# Get the current recovery member of the quorum
92+
q = self.nodes[0].quorum('selectquorum', q_type, id)
93+
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
94+
# Open a P2P connection to it
95+
p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface())
96+
# Send the last required QSIGSHARE message to the recovery member
97+
p2p_interface.send_message(msg_qsigshare([sig_share]))
10998

11099
wait_for_sigs(True, False, True, 15)
111100

112-
if self.options.spork21:
113-
mn.get_node(self).disconnect_p2ps()
101+
mn.get_node(self).disconnect_p2ps()
114102

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

178-
if self.options.spork21:
179-
id = uint256_to_string(request_id + 1)
180-
181-
# Isolate the node that is responsible for the recovery of a signature and assert that recovery fails
182-
q = self.nodes[0].quorum('selectquorum', q_type, id)
183-
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
184-
mn.get_node(self).setnetworkactive(False)
185-
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0)
186-
for i in range(4):
187-
self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash)
188-
assert_sigs_nochange(False, False, False, 3)
189-
# Need to re-connect so that it later gets the recovered sig
190-
mn.get_node(self).setnetworkactive(True)
191-
self.connect_nodes(mn.nodeIdx, 0)
192-
force_finish_mnsync(mn.get_node(self))
193-
# Make sure intra-quorum connections were also restored
194-
self.bump_mocktime(1) # need this to bypass quorum connection retry timeout
195-
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10)
196-
mn.get_node(self).ping()
197-
self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo()))
198-
# Let 2 seconds pass so that the next node is used for recovery, which should succeed
199-
self.bump_mocktime(2)
200-
wait_for_sigs(True, False, True, 2)
166+
id = uint256_to_string(request_id + 1)
167+
168+
# Isolate the node that is responsible for the recovery of a signature and assert that recovery fails
169+
q = self.nodes[0].quorum('selectquorum', q_type, id)
170+
mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0])
171+
mn.get_node(self).setnetworkactive(False)
172+
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0)
173+
for i in range(4):
174+
self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash)
175+
assert_sigs_nochange(False, False, False, 3)
176+
# Need to re-connect so that it later gets the recovered sig
177+
mn.get_node(self).setnetworkactive(True)
178+
self.connect_nodes(mn.nodeIdx, 0)
179+
force_finish_mnsync(mn.get_node(self))
180+
# Make sure intra-quorum connections were also restored
181+
self.bump_mocktime(1) # need this to bypass quorum connection retry timeout
182+
self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10)
183+
mn.get_node(self).ping()
184+
self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo()))
185+
# Let 2 seconds pass so that the next node is used for recovery, which should succeed
186+
self.bump_mocktime(2)
187+
wait_for_sigs(True, False, True, 2)
201188

202189
if __name__ == '__main__':
203190
LLMQSigningTest().main()

‎test/functional/feature_llmq_simplepose.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,6 @@ def run_test(self):
4848
self.test_banning(self.isolate_mn, 2)
4949

5050
self.repair_masternodes(False)
51-
52-
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
53-
self.wait_for_sporks_same()
54-
5551
self.reset_probe_timeouts()
5652

5753
if not self.options.disable_spork23:

‎test/functional/test_framework/test_framework.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,11 +2104,10 @@ def move_blocks(self, nodes, num_blocks):
21042104
self.generate(self.nodes[0], num_blocks, sync_fun=lambda: self.sync_blocks(nodes))
21052105

21062106
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, skip_maturity=False):
2107-
spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
21082107
spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1
21092108

21102109
if expected_connections is None:
2111-
expected_connections = (self.llmq_size - 1) if spork21_active else 2
2110+
expected_connections = self.llmq_size - 1
21122111
if expected_members is None:
21132112
expected_members = self.llmq_size
21142113
if expected_contributions is None:
@@ -2194,13 +2193,12 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec
21942193
return new_quorum
21952194

21962195
def mine_cycle_quorum(self):
2197-
spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
21982196
spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1
21992197

22002198
llmq_type_name="llmq_test_dip0024"
22012199
llmq_type=103
22022200
llmq_cycle_len = 24
2203-
expected_connections = (self.llmq_size_dip0024 - 1) if spork21_active else 2
2201+
expected_connections = self.llmq_size_dip0024 - 1
22042202
expected_members = self.llmq_size_dip0024
22052203
expected_contributions = self.llmq_size_dip0024
22062204
expected_commitments = self.llmq_size_dip0024

0 commit comments

Comments
 (0)