Skip to content

Commit 86105da

Browse files
PastaPastaPastaknst
PastaPastaPasta
authored and
knst
committed
Merge #6408: refactor: removed pre-MN_RR logic of validation of CL
3f2e064 refactor: set `const auto& cbTx` to avoid using optional throughout method (pasta) 0c0d91e fix: functional test feature_llmq_chainlocks.py should activate MN_RR instead v20 (Konstantin Akimov) af93e87 refactor: removed pre-MN_RR logic of validation of CL (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented The fork MN_RR is active awhile on testnet and mainnet and no more need legacy check ## What was done? Removes legacy version of checks for CL and related functional tests ## How Has This Been Tested? Run unit / functional test - DONE Reindex testnet - DONE Reindex mainnet - DONE ## Breaking Changes Removed pre-mn_rr version of checks for CL. It's no more relevant on mainnet and testnet. It affects behavior on new devnets and regtest for pre-mn_rr activation blocks. ## Checklist: - [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 - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 3f2e064 Tree-SHA512: 8b4b3a20a54602f4df9d98e17c79004214493b15c0bce9c08c68d667a5cba86b817947f008d646c48ef9f2f86676c02085c7d0ed36e83548ef5425b64faffb89
1 parent a1f7e96 commit 86105da

File tree

4 files changed

+32
-44
lines changed

4 files changed

+32
-44
lines changed

‎src/evo/cbtx.cpp‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
332332
}
333333

334334
bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
335-
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const bool check_clhdiff)
335+
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state)
336336
{
337337
if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
338338
return true;
@@ -342,44 +342,43 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
342342
if (!opt_cbTx) {
343343
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload");
344344
}
345+
const auto& cbTx = *opt_cbTx;
345346

346-
if (opt_cbTx->nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
347+
if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
347348
return true;
348349
}
349350

350351
auto best_clsig = chainlock_handler.GetBestChainLock();
351-
if (best_clsig.getHeight() == pindex->nHeight - 1 && opt_cbTx->bestCLHeightDiff == 0 && opt_cbTx->bestCLSignature == best_clsig.getSig()) {
352+
if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbTx.bestCLSignature == best_clsig.getSig()) {
352353
// matches our best clsig which still hold values for the previous block
353354
return true;
354355
}
355356

356-
if (check_clhdiff) {
357-
auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev);
358-
// If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null.
359-
if (prevBlockCoinbaseChainlock.has_value()) {
360-
// Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one
361-
if (!opt_cbTx->bestCLSignature.IsValid()) {
362-
// IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null
363-
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig");
364-
}
365-
if (opt_cbTx->bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) {
366-
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
367-
}
357+
const auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev);
358+
// If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null.
359+
if (prevBlockCoinbaseChainlock.has_value()) {
360+
// Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one
361+
if (!cbTx.bestCLSignature.IsValid()) {
362+
// IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null
363+
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig");
364+
}
365+
if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) {
366+
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
368367
}
369368
}
370369

371370
// IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null
372-
if (opt_cbTx->bestCLSignature.IsValid()) {
373-
int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast<int>(opt_cbTx->bestCLHeightDiff) - 1;
374-
if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == opt_cbTx->bestCLSignature) {
371+
if (cbTx.bestCLSignature.IsValid()) {
372+
int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast<int>(cbTx.bestCLHeightDiff) - 1;
373+
if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) {
375374
// matches our best (but outdated) clsig, no need to verify it again
376375
return true;
377376
}
378377
uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
379-
if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) {
378+
if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) {
380379
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig");
381380
}
382-
} else if (opt_cbTx->bestCLHeightDiff != 0) {
381+
} else if (cbTx.bestCLHeightDiff != 0) {
383382
// Null bestCLSignature is allowed only with bestCLHeightDiff = 0
384383
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff");
385384
}

‎src/evo/cbtx.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
9696
BlockValidationState& state);
9797

9898
bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev,
99-
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const bool check_clhdiff);
99+
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state);
100100
bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
101101
uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature);
102102

‎src/evo/specialtxman.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB
188188
nTimeMerkle += nTime5 - nTime4;
189189
LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001);
190190

191-
if (fCheckCbTxMerkleRoots && !CheckCbTxBestChainlock(block, pindex, m_clhandler, state, DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_MN_RR))) {
191+
if (fCheckCbTxMerkleRoots && !CheckCbTxBestChainlock(block, pindex, m_clhandler, state)) {
192192
// pass the state returned by the function above
193193
return false;
194194
}

‎test/functional/feature_llmq_chainlocks.py‎

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515

1616
from test_framework.messages import CBlock, CCbTx
1717
from test_framework.test_framework import DashTestFramework
18-
from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, softfork_active
18+
from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync
1919

2020

2121
class LLMQChainLocksTest(DashTestFramework):
2222
def set_test_params(self):
23-
self.set_dash_test_params(5, 4, [["-testactivationheight=mn_rr@1100"]] * 5)
23+
self.set_dash_test_params(5, 4)
2424

2525
def run_test(self):
2626
# Connect all nodes to node1 so that we always have the whole network connected
@@ -31,8 +31,8 @@ def run_test(self):
3131

3232
self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False)
3333

34-
self.activate_v20(expected_activation_height=900)
35-
self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount()))
34+
self.activate_mn_rr(expected_activation_height=900)
35+
self.log.info("Activated MN_RR at height:" + str(self.nodes[0].getblockcount()))
3636

3737
# v20 is active for the next block, not for the tip
3838
self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False)
@@ -243,14 +243,8 @@ def run_test(self):
243243
assert_equal(tip_1['cbTx']['bestCLSignature'], tip_0['cbTx']['bestCLSignature'])
244244
assert_equal(tip_1['cbTx']['bestCLHeightDiff'], tip_0['cbTx']['bestCLHeightDiff'] + 1)
245245

246-
self.log.info("Test that bestCLHeightDiff conditions are relaxed before mn_rr")
247-
self.test_bestCLHeightDiff(False)
248-
249-
self.activate_mn_rr(expected_activation_height=1100)
250-
self.log.info("Activated mn_rr at height:" + str(self.nodes[0].getblockcount()))
251-
252-
self.log.info("Test that bestCLHeightDiff conditions are stricter after mn_rr")
253-
self.test_bestCLHeightDiff(True)
246+
self.log.info("Test bestCLHeightDiff restrictions")
247+
self.test_bestCLHeightDiff()
254248

255249
def create_chained_txs(self, node, amount):
256250
txid = node.sendtoaddress(node.getnewaddress(), amount)
@@ -293,11 +287,10 @@ def test_coinbase_best_cl(self, node, expected_cl_in_cb=True, expected_null_cl=F
293287
else:
294288
assert "bestCLHeightDiff" not in cbtx and "bestCLSignature" not in cbtx
295289

296-
def test_bestCLHeightDiff(self, mn_rr_active):
290+
def test_bestCLHeightDiff(self):
297291
# We need 2 blocks we can grab clsigs from
298292
for _ in range(2):
299293
self.wait_for_chainlocked_block_all_nodes(self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0])
300-
assert_equal(softfork_active(self.nodes[1], "mn_rr"), mn_rr_active)
301294
tip1_hash = self.nodes[1].getbestblockhash()
302295

303296
self.isolate_node(1)
@@ -339,23 +332,19 @@ def test_bestCLHeightDiff(self, mn_rr_active):
339332
mal_block.hashMerkleRoot = mal_block.calc_merkle_root()
340333
mal_block.solve()
341334
result = self.nodes[1].submitblock(mal_block.serialize().hex())
342-
assert_equal(result, "bad-cbtx-older-clsig" if mn_rr_active else "bad-cbtx-invalid-clsig")
335+
assert_equal(result, "bad-cbtx-older-clsig")
343336
assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
344337

345-
# Update the sig too and it should pass now when mn_rr is not active and fail otherwise
338+
# Update the sig too and it should fail
346339
old_blockhash = self.nodes[1].getblockhash(self.nodes[1].getblockcount() - 1)
347340
cbtx.bestCLSignature = bytes.fromhex(self.nodes[1].getblock(old_blockhash, 2)["tx"][0]["cbTx"]["bestCLSignature"])
348341
mal_block.vtx[0].vExtraPayload = cbtx.serialize()
349342
mal_block.vtx[0].rehash()
350343
mal_block.hashMerkleRoot = mal_block.calc_merkle_root()
351344
mal_block.solve()
352345
result = self.nodes[1].submitblock(mal_block.serialize().hex())
353-
if mn_rr_active:
354-
assert_equal(result, "bad-cbtx-older-clsig")
355-
assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
356-
else:
357-
assert_equal(result, None)
358-
assert not self.nodes[1].getbestblockhash() == tip1_hash
346+
assert_equal(result, "bad-cbtx-older-clsig")
347+
assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
359348

360349
self.reconnect_isolated_node(1, 0)
361350
self.sync_all()

0 commit comments

Comments
 (0)