Skip to content

Commit 3c6ab9f

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6661: feat: remove pre-'withdrawals' logic for enforcing 2 quorums only and related test changes
56711d9 test: flip request-id for asset-unlock-tx instead generation new quorums to get a favour situation (Konstantin Akimov) 4a7937d test: re-order logic related to IS in feature_asset_locks.py (Konstantin Akimov) 825dfdd test: activate v23 earlier (block 750 instead 1050) (Konstantin Akimov) a039345 test: activate mn_rr in feature_asset_locks.py for better performance (Konstantin Akimov) 75de576 feat: remove pre-withdrawals fork logic for quorum expiration (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Fork `withdrawals` is activated long time ago. No more need to test functionality for pre-'withdrawals' fork when only 2 quorum could be used for signing messages. ## What was done? Removed pre-'withdrawals' logic to enforce using only 2 quorums for signing of withdrawals from code and from functional test `feature_asset_locks.py` Functional test `feature_asset_locks.py` generate less quorums and less blocks which significantly shortened runtime of this test, 166->139seconds on median; up to 40 seconds if quorum has been generated non-favourable way. ## How Has This Been Tested? Run in parallel 20 jobs - PR: ```TEST | STATUS | DURATION feature_asset_locks.py | ✓ Passed | 133 s feature_asset_locks.py | ✓ Passed | 134 s feature_asset_locks.py | ✓ Passed | 134 s feature_asset_locks.py | ✓ Passed | 134 s feature_asset_locks.py | ✓ Passed | 135 s feature_asset_locks.py | ✓ Passed | 135 s feature_asset_locks.py | ✓ Passed | 137 s feature_asset_locks.py | ✓ Passed | 138 s feature_asset_locks.py | ✓ Passed | 138 s feature_asset_locks.py | ✓ Passed | 139 s feature_asset_locks.py | ✓ Passed | 139 s feature_asset_locks.py | ✓ Passed | 140 s feature_asset_locks.py | ✓ Passed | 140 s feature_asset_locks.py | ✓ Passed | 140 s feature_asset_locks.py | ✓ Passed | 141 s feature_asset_locks.py | ✓ Passed | 141 s feature_asset_locks.py | ✓ Passed | 142 s feature_asset_locks.py | ✓ Passed | 142 s feature_asset_locks.py | ✓ Passed | 143 s feature_asset_locks.py | ✓ Passed | 143 s ALL | ✓ Passed | 2768 s (accumulated) Runtime: 143 s ``` - develop ``` TEST | STATUS | DURATION feature_asset_locks.py | ✓ Passed | 151 s feature_asset_locks.py | ✓ Passed | 157 s feature_asset_locks.py | ✓ Passed | 160 s feature_asset_locks.py | ✓ Passed | 161 s feature_asset_locks.py | ✓ Passed | 161 s feature_asset_locks.py | ✓ Passed | 163 s feature_asset_locks.py | ✓ Passed | 164 s feature_asset_locks.py | ✓ Passed | 165 s feature_asset_locks.py | ✓ Passed | 166 s feature_asset_locks.py | ✓ Passed | 166 s feature_asset_locks.py | ✓ Passed | 167 s feature_asset_locks.py | ✓ Passed | 167 s feature_asset_locks.py | ✓ Passed | 170 s feature_asset_locks.py | ✓ Passed | 171 s feature_asset_locks.py | ✓ Passed | 172 s feature_asset_locks.py | ✓ Passed | 174 s feature_asset_locks.py | ✓ Passed | 181 s feature_asset_locks.py | ✓ Passed | 185 s feature_asset_locks.py | ✓ Passed | 197 s feature_asset_locks.py | ✖ Failed | 49 s ALL | ✖ Failed | 3247 s (accumulated) Runtime: 197 s ``` ## Breaking Changes Removed pre-'withdrawals' requirement of using only 2 last quorums ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: light ACK 56711d9 Tree-SHA512: 779194997cff3a00626b0aba8f7026c8c4528f9327f49aa1c8cfb1cad8fa285c881517545951a2cd36d01fdcf928e2d5be2fc65fa8a915678c06f2b9f1fed2c3
2 parents 8fc24bd + 56711d9 commit 3c6ab9f

File tree

3 files changed

+48
-66
lines changed

3 files changed

+48
-66
lines changed

‎src/chainparams.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -820,9 +820,9 @@ class CRegTestParams : public CChainParams {
820820
consensus.vDeployments[Consensus::DEPLOYMENT_V23].bit = 12;
821821
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nStartTime = 0;
822822
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
823-
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nWindowSize = 350;
824-
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nThresholdStart = 350 / 5 * 4; // 80% of window size
825-
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nThresholdMin = 350 / 5 * 3; // 60% of window size
823+
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nWindowSize = 250;
824+
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nThresholdStart = 250 / 5 * 4; // 80% of window size
825+
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nThresholdMin = 250 / 5 * 3; // 60% of window size
826826
consensus.vDeployments[Consensus::DEPLOYMENT_V23].nFalloffCoeff = 5; // this corresponds to 10 periods
827827
consensus.vDeployments[Consensus::DEPLOYMENT_V23].useEHF = true;
828828

‎src/evo/assetlocktx.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,8 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const uint
122122
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
123123
assert(llmq_params_opt.has_value());
124124

125-
// We check two quorums before DEPLOYMENT_WITHDRAWALS activation
126-
// and "all active quorums + 1 the latest inactive" after activation.
127-
const int quorums_to_scan = DeploymentActiveAt(*pindexTip, Params().GetConsensus(), Consensus::DEPLOYMENT_WITHDRAWALS)
128-
? (llmq_params_opt->signingActiveQuorumCount + 1)
129-
: 2;
125+
// We check all active quorums + 1 the latest inactive
126+
const int quorums_to_scan = llmq_params_opt->signingActiveQuorumCount + 1;
130127
const auto quorums = qman.ScanQuorums(llmqType, pindexTip, quorums_to_scan);
131128

132129
if (bool isActive = std::any_of(quorums.begin(), quorums.end(), [&](const auto &q) { return q->qc->quorumHash == quorumHash; }); !isActive) {

‎test/functional/feature_asset_locks.py‎

Lines changed: 43 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def set_test_params(self):
5555
"-whitelist=127.0.0.1",
5656
"-llmqtestinstantsenddip0024=llmq_test_instantsend",
5757
]] * 2, evo_count=2)
58-
self.mn_rr_height = 620
58+
self.mn_rr_height = 560
5959

6060
def skip_test_if_missing_module(self):
6161
self.skip_if_no_wallet()
@@ -361,35 +361,37 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
361361
result_expected={'allowed': False, 'reject-reason' : 'max-fee-exceeded'})
362362
self.check_mempool_result(tx=asset_unlock_tx_zero_fee,
363363
result_expected={'allowed': False, 'reject-reason' : 'bad-txns-assetunlock-fee-outofrange'})
364-
# not-verified is a correct faiure from mempool. Mempool knows nothing about CreditPool indexes and he just report that signature is not validated
364+
# not-verified is a correct error message when adding to mempool because Mempool knows nothing about CreditPool and indexes.
365+
# but the signature is invalid so far as we changed index without re-signing it by quorum
365366
self.check_mempool_result(tx=asset_unlock_tx_duplicate_index,
366367
result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-not-verified'})
367368

368-
self.log.info("Validating that we calculate payload hash correctly: ask quorum forcely by message hash...")
369+
self.log.info("Validating payload hash calculation by using hard-coded message hash")
369370
asset_unlock_tx_payload = CAssetUnlockTx()
370371
asset_unlock_tx_payload.deserialize(BytesIO(asset_unlock_tx.vExtraPayload))
371372

372373
assert_equal(asset_unlock_tx_payload.quorumHash, int(self.mninfo[0].get_node(self).quorum("selectquorum", llmq_type_test, 'e6c7a809d79f78ea85b72d5df7e9bd592aecf151e679d6e976b74f053a7f9056')["quorumHash"], 16))
373374

375+
txid = self.send_tx(asset_unlock_tx)
376+
377+
self.log.info("Test RPC getassetunlockstatuses part I")
378+
tip = self.nodes[0].getblockcount()
379+
indexes_statuses_no_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"])
380+
assert_equal([{'index': 101, 'status': 'mempooled'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_no_height)
381+
indexes_statuses_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"], tip)
382+
assert_equal([{'index': 101, 'status': 'unknown'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_height)
383+
384+
374385
self.log.info("Test no IS for asset unlock...")
375386
self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
376387
self.wait_for_sporks_same()
377388

378-
txid = self.send_tx(asset_unlock_tx)
379389
assert_equal(node.getmempoolentry(txid)['fees']['base'], Decimal("0.0007"))
380390
is_id = node_wallet.sendtoaddress(node_wallet.getnewaddress(), 1)
381391
self.bump_mocktime(30)
382392
for node in self.nodes:
383393
self.wait_for_instantlock(is_id, node)
384394

385-
386-
tip = self.nodes[0].getblockcount()
387-
indexes_statuses_no_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"])
388-
assert_equal([{'index': 101, 'status': 'mempooled'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_no_height)
389-
indexes_statuses_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"], tip)
390-
assert_equal([{'index': 101, 'status': 'unknown'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_height)
391-
392-
393395
rawtx = node.getrawtransaction(txid, 1)
394396
rawtx_is = node.getrawtransaction(is_id, 1)
395397
assert_equal(rawtx["instantlock"], False)
@@ -398,7 +400,7 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
398400
assert_equal(rawtx_is["chainlock"], False)
399401
assert not "confirmations" in rawtx
400402
assert not "confirmations" in rawtx_is
401-
self.log.info("Disable back IS")
403+
self.log.info("Reset IS spork")
402404
self.set_sporks()
403405

404406
assert "assetUnlockTx" in node.getrawtransaction(txid, 1)
@@ -441,28 +443,17 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
441443
self.check_mempool_result(tx=asset_unlock_tx_too_late,
442444
result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-late'})
443445

444-
self.log.info("Checking that two quorums later it is too late because quorum is not active...")
445-
self.mine_quorum_2_nodes()
446-
self.log.info("Expecting new reject-reason...")
447-
assert not softfork_active(self.nodes[0], 'withdrawals')
448-
self.check_mempool_result(tx=asset_unlock_tx_too_late,
449-
result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-old-quorum'})
450-
451446
block_to_reconsider = node.getbestblockhash()
452447
self.log.info("Test block invalidation with asset unlock tx...")
453448
for inode in self.nodes:
454449
inode.invalidateblock(block_asset_unlock)
455450
self.validate_credit_pool_balance(locked)
456-
self.generate_batch(50)
451+
self.generate_batch(25)
457452
self.validate_credit_pool_balance(locked)
458453
for inode in self.nodes:
459454
inode.reconsiderblock(block_to_reconsider)
460455
self.validate_credit_pool_balance(locked - 2 * COIN)
461456

462-
self.log.info("Forcibly mining asset_unlock_tx_too_late and ensure block is invalid")
463-
assert not softfork_active(self.nodes[0], 'withdrawals')
464-
self.create_and_check_block([asset_unlock_tx_too_late], expected_error = "bad-assetunlock-too-old-quorum")
465-
466457
self.generate(node, 1)
467458

468459
self.validate_credit_pool_balance(locked - 2 * COIN)
@@ -633,7 +624,7 @@ def test_withdrawal_limits(self, node_wallet, node, pubkey):
633624
def test_mn_rr(self, node_wallet, node, pubkey):
634625
self.log.info("Activate mn_rr...")
635626
locked = self.get_credit_pool_balance()
636-
self.activate_mn_rr(expected_activation_height=620)
627+
self.activate_mn_rr(expected_activation_height=560)
637628
self.log.info(f'mn-rr height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}')
638629
assert_equal(locked, self.get_credit_pool_balance())
639630

@@ -645,7 +636,7 @@ def test_mn_rr(self, node_wallet, node, pubkey):
645636
all_mn_rewards = platform_reward + owner_reward + operator_reward
646637
assert_equal(all_mn_rewards, bt['coinbasevalue'] * 3 // 4) # 75/25 mn/miner reward split
647638
assert_equal(platform_reward, all_mn_rewards * 375 // 1000) # 0.375 platform share
648-
assert_equal(platform_reward, 104549943)
639+
assert_equal(platform_reward, 112592247)
649640
assert_equal(locked, self.get_credit_pool_balance())
650641
self.generate(node, 1)
651642
locked += platform_reward
@@ -660,6 +651,7 @@ def test_mn_rr(self, node_wallet, node, pubkey):
660651

661652
def test_withdrawals_fork(self, node_wallet, node, pubkey):
662653
self.log.info("Testing asset unlock after 'withdrawals' activation...")
654+
self.activate_by_name('withdrawals', 600)
663655
assert softfork_active(node_wallet, 'withdrawals')
664656
self.log.info(f'post-withdrawals height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}')
665657

@@ -676,23 +668,19 @@ def test_withdrawals_fork(self, node_wallet, node, pubkey):
676668
quorumHash_str = format(asset_unlock_tx_payload.quorumHash, '064x')
677669
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
678670

679-
while quorumHash_str != node_wallet.quorum('list')['llmq_test_platform'][-1]:
680-
self.log.info("Generate one more quorum until signing quorum becomes the last one in the list")
681-
self.mine_quorum_2_nodes()
682-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
671+
if quorumHash_str != node_wallet.quorum('list')['llmq_test_platform'][-1]:
672+
self.log.info("The quorum for this msg-hash is not the last one in the list of active quorums. Try again!")
673+
index += 1
674+
else:
675+
break
683676

684-
self.log.info("Generate one more quorum after which signing quorum is gone but Asset Unlock tx is still valid")
685-
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
686-
self.mine_quorum_2_nodes()
687-
assert quorumHash_str not in node_wallet.quorum('list')['llmq_test_platform']
677+
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
678+
self.log.info("Generate one more quorum to make signing quorum inactive but still valid")
679+
self.mine_quorum_2_nodes()
680+
assert quorumHash_str not in node_wallet.quorum('list')['llmq_test_platform']
688681

689-
if asset_unlock_tx_payload.requestedHeight + HEIGHT_DIFF_EXPIRING > node_wallet.getblockcount():
690-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
691-
break
692-
else:
693-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-late'})
694-
self.log.info("Asset Unlock tx expired, let's try again...")
695-
index += 1
682+
assert asset_unlock_tx_payload.requestedHeight + HEIGHT_DIFF_EXPIRING > node_wallet.getblockcount()
683+
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
696684

697685
self.log.info("Generate one more quorum after which signing quorum becomes too old")
698686
self.mine_quorum_2_nodes()
@@ -716,7 +704,7 @@ def test_withdrawals_fork(self, node_wallet, node, pubkey):
716704

717705
def test_v23_fork(self, node_wallet, node, pubkey):
718706
self.log.info("Testing asset unlock after 'v23' activation...")
719-
self.activate_by_name('v23', 1050)
707+
self.activate_by_name('v23', 750)
720708
self.log.info(f'post-v23 height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}')
721709

722710
index = 601
@@ -732,23 +720,20 @@ def test_v23_fork(self, node_wallet, node, pubkey):
732720
quorumHash_str = format(asset_unlock_tx_payload.quorumHash, '064x')
733721
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
734722

735-
while quorumHash_str != node_wallet.quorum('list')['llmq_test_platform'][-1]:
736-
self.log.info("Generate one more quorum until signing quorum becomes the last one in the list")
737-
self.mine_quorum_2_nodes()
738-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
723+
if quorumHash_str != node_wallet.quorum('list')['llmq_test_platform'][-1]:
724+
self.log.info("The quorum for this msg-hash is not the last one in the list of active quorums. Try again!")
725+
index += 1
726+
else:
727+
break
739728

740-
self.log.info("Generate one more quorum after which signing quorum is gone but Asset Unlock tx is still valid")
741-
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
742-
self.mine_quorum_2_nodes()
743-
assert quorumHash_str not in node_wallet.quorum('list')['llmq_test_platform']
729+
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
730+
self.log.info("Generate one more quorum to make signing quorum inactive but still valid")
731+
self.mine_quorum_2_nodes()
732+
assert quorumHash_str not in node_wallet.quorum('list')['llmq_test_platform']
744733

745-
if asset_unlock_tx_payload.requestedHeight + HEIGHT_DIFF_EXPIRING > node_wallet.getblockcount():
746-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
747-
break
748-
else:
749-
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-late'})
750-
self.log.info("Asset Unlock tx expired, let's try again...")
751-
index += 1
734+
735+
assert asset_unlock_tx_payload.requestedHeight + HEIGHT_DIFF_EXPIRING > node_wallet.getblockcount()
736+
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
752737

753738
self.log.info("Generate one more quorum after which signing quorum becomes too old")
754739
self.mine_quorum_2_nodes()

0 commit comments

Comments
 (0)