sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 19:42:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #6408: refactor: removed pre-MN_RR logic of validation of CL

Browse Source 
3f2e064b18 refactor: set `const auto& cbTx` to avoid using optional throughout method (pasta)
0c0d91e491 fix: functional test feature_llmq_chainlocks.py should activate MN_RR instead v20 (Konstantin Akimov)
af93e877f2 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 3f2e064b18

Tree-SHA512: 8b4b3a20a54602f4df9d98e17c79004214493b15c0bce9c08c68d667a5cba86b817947f008d646c48ef9f2f86676c02085c7d0ed36e83548ef5425b64faffb89
This commit is contained in:
pasta 2024-11-20 11:28:35 -06:00 committed by Konstantin Akimov
parent a1f7e96025
commit 86105daab3
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
4 changed files with 32 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/evo/cbtx.cpp
View File

@ -332,7 +332,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
} }
bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const bool check_clhdiff) const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state)
{ {
if (block.vtx[0]->nType != TRANSACTION_COINBASE) { if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
return true; return true;
@ -342,44 +342,43 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
if (!opt_cbTx) { if (!opt_cbTx) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload");
} }
const auto& cbTx = *opt_cbTx;
if (opt_cbTx->nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
return true; return true;
} }
auto best_clsig = chainlock_handler.GetBestChainLock(); auto best_clsig = chainlock_handler.GetBestChainLock();
if (best_clsig.getHeight() == pindex->nHeight - 1 && opt_cbTx->bestCLHeightDiff == 0 && opt_cbTx->bestCLSignature == best_clsig.getSig()) { if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbTx.bestCLSignature == best_clsig.getSig()) {
// matches our best clsig which still hold values for the previous block // matches our best clsig which still hold values for the previous block
return true; return true;
} }
if (check_clhdiff) { const auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev);
auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev); // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null.
// If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. if (prevBlockCoinbaseChainlock.has_value()) {
if (prevBlockCoinbaseChainlock.has_value()) { // 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
// 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 if (!cbTx.bestCLSignature.IsValid()) {
if (!opt_cbTx->bestCLSignature.IsValid()) { // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null
// IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig");
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); }
} if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) {
if (opt_cbTx->bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
}
} }
} }
// IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null
if (opt_cbTx->bestCLSignature.IsValid()) { if (cbTx.bestCLSignature.IsValid()) {
int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast<int>(opt_cbTx->bestCLHeightDiff) - 1; int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast<int>(cbTx.bestCLHeightDiff) - 1;
if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == opt_cbTx->bestCLSignature) { if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) {
// matches our best (but outdated) clsig, no need to verify it again // matches our best (but outdated) clsig, no need to verify it again
return true; return true;
} }
uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) { if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig");
} }
} else if (opt_cbTx->bestCLHeightDiff != 0) { } else if (cbTx.bestCLHeightDiff != 0) {
// Null bestCLSignature is allowed only with bestCLHeightDiff = 0 // Null bestCLSignature is allowed only with bestCLHeightDiff = 0
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff");
} }

2
src/evo/cbtx.h
View File

@ -96,7 +96,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
BlockValidationState& state); BlockValidationState& state);
bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev,
const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const bool check_clhdiff); const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state);
bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev, bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature);

2
src/evo/specialtxman.cpp
View File

@ -188,7 +188,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB
nTimeMerkle += nTime5 - nTime4; nTimeMerkle += nTime5 - nTime4;
LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001);
if (fCheckCbTxMerkleRoots && !CheckCbTxBestChainlock(block, pindex, m_clhandler, state, DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_MN_RR))) { if (fCheckCbTxMerkleRoots && !CheckCbTxBestChainlock(block, pindex, m_clhandler, state)) {
// pass the state returned by the function above // pass the state returned by the function above
return false; return false;
} }

33
test/functional/feature_llmq_chainlocks.py
View File

@ -15,12 +15,12 @@ from io import BytesIO
from test_framework.messages import CBlock, CCbTx from test_framework.messages import CBlock, CCbTx
from test_framework.test_framework import DashTestFramework from test_framework.test_framework import DashTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, softfork_active from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync
class LLMQChainLocksTest(DashTestFramework): class LLMQChainLocksTest(DashTestFramework):
def set_test_params(self): def set_test_params(self):
self.set_dash_test_params(5, 4, [["-testactivationheight=mn_rr@1100"]] * 5) self.set_dash_test_params(5, 4)
def run_test(self): def run_test(self):
# Connect all nodes to node1 so that we always have the whole network connected # Connect all nodes to node1 so that we always have the whole network connected
@ -31,8 +31,8 @@ class LLMQChainLocksTest(DashTestFramework):
self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False) self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False)
self.activate_v20(expected_activation_height=900) self.activate_mn_rr(expected_activation_height=900)
self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount())) self.log.info("Activated MN_RR at height:" + str(self.nodes[0].getblockcount()))
# v20 is active for the next block, not for the tip # v20 is active for the next block, not for the tip
self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False) self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False)
@ -243,14 +243,8 @@ class LLMQChainLocksTest(DashTestFramework):
assert_equal(tip_1['cbTx']['bestCLSignature'], tip_0['cbTx']['bestCLSignature']) assert_equal(tip_1['cbTx']['bestCLSignature'], tip_0['cbTx']['bestCLSignature'])
assert_equal(tip_1['cbTx']['bestCLHeightDiff'], tip_0['cbTx']['bestCLHeightDiff'] + 1) assert_equal(tip_1['cbTx']['bestCLHeightDiff'], tip_0['cbTx']['bestCLHeightDiff'] + 1)
self.log.info("Test that bestCLHeightDiff conditions are relaxed before mn_rr") self.log.info("Test bestCLHeightDiff restrictions")
self.test_bestCLHeightDiff(False) self.test_bestCLHeightDiff()
self.activate_mn_rr(expected_activation_height=1100)
self.log.info("Activated mn_rr at height:" + str(self.nodes[0].getblockcount()))
self.log.info("Test that bestCLHeightDiff conditions are stricter after mn_rr")
self.test_bestCLHeightDiff(True)
def create_chained_txs(self, node, amount): def create_chained_txs(self, node, amount):
txid = node.sendtoaddress(node.getnewaddress(), amount) txid = node.sendtoaddress(node.getnewaddress(), amount)
@ -293,11 +287,10 @@ class LLMQChainLocksTest(DashTestFramework):
else: else:
assert "bestCLHeightDiff" not in cbtx and "bestCLSignature" not in cbtx assert "bestCLHeightDiff" not in cbtx and "bestCLSignature" not in cbtx
def test_bestCLHeightDiff(self, mn_rr_active): def test_bestCLHeightDiff(self):
# We need 2 blocks we can grab clsigs from # We need 2 blocks we can grab clsigs from
for _ in range(2): for _ in range(2):
self.wait_for_chainlocked_block_all_nodes(self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]) self.wait_for_chainlocked_block_all_nodes(self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0])
assert_equal(softfork_active(self.nodes[1], "mn_rr"), mn_rr_active)
tip1_hash = self.nodes[1].getbestblockhash() tip1_hash = self.nodes[1].getbestblockhash()
self.isolate_node(1) self.isolate_node(1)
@ -339,10 +332,10 @@ class LLMQChainLocksTest(DashTestFramework):
mal_block.hashMerkleRoot = mal_block.calc_merkle_root() mal_block.hashMerkleRoot = mal_block.calc_merkle_root()
mal_block.solve() mal_block.solve()
result = self.nodes[1].submitblock(mal_block.serialize().hex()) result = self.nodes[1].submitblock(mal_block.serialize().hex())
assert_equal(result, "bad-cbtx-older-clsig" if mn_rr_active else "bad-cbtx-invalid-clsig") assert_equal(result, "bad-cbtx-older-clsig")
assert_equal(self.nodes[1].getbestblockhash(), tip1_hash) assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
# Update the sig too and it should pass now when mn_rr is not active and fail otherwise # Update the sig too and it should fail
old_blockhash = self.nodes[1].getblockhash(self.nodes[1].getblockcount() - 1) old_blockhash = self.nodes[1].getblockhash(self.nodes[1].getblockcount() - 1)
cbtx.bestCLSignature = bytes.fromhex(self.nodes[1].getblock(old_blockhash, 2)["tx"][0]["cbTx"]["bestCLSignature"]) cbtx.bestCLSignature = bytes.fromhex(self.nodes[1].getblock(old_blockhash, 2)["tx"][0]["cbTx"]["bestCLSignature"])
mal_block.vtx[0].vExtraPayload = cbtx.serialize() mal_block.vtx[0].vExtraPayload = cbtx.serialize()
@ -350,12 +343,8 @@ class LLMQChainLocksTest(DashTestFramework):
mal_block.hashMerkleRoot = mal_block.calc_merkle_root() mal_block.hashMerkleRoot = mal_block.calc_merkle_root()
mal_block.solve() mal_block.solve()
result = self.nodes[1].submitblock(mal_block.serialize().hex()) result = self.nodes[1].submitblock(mal_block.serialize().hex())
if mn_rr_active: assert_equal(result, "bad-cbtx-older-clsig")
assert_equal(result, "bad-cbtx-older-clsig") assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
assert_equal(self.nodes[1].getbestblockhash(), tip1_hash)
else:
assert_equal(result, None)
assert not self.nodes[1].getbestblockhash() == tip1_hash
self.reconnect_isolated_node(1, 0) self.reconnect_isolated_node(1, 0)
self.sync_all() self.sync_all()