mirror of
https://github.com/dashpay/dash.git
synced 2024-12-25 20:12:57 +01:00
fix: Move CreditPoolDiff checks out of ProcessSpecialTxsInBlock, use correct block reward (#5594)
## Issue being fixed or feature implemented The block reward calculation logic in `SetTarget` doesn't work on superblocks. ## What was done? Move `CreditPoolDiff` checks out of `ProcessSpecialTxsInBlock` to use correct block reward. ## How Has This Been Tested? run tests ## Breaking Changes n/a, sb blocks should now be processed correctly, non-sb blocks shouldn't be affected ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] 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)_
This commit is contained in:
UdjinM6
GitHub
parent
1dd4b09990
commit
c814dcaaea
@ -228,7 +228,7 @@ void CCreditPoolDiff::AddRewardRealloced(const CAmount reward) {
|
||||
platformReward += reward;
|
||||
}
|
||||
|
||||
bool CCreditPoolDiff::SetTarget(const CTransaction& tx, TxValidationState& state)
|
||||
bool CCreditPoolDiff::SetTarget(const CTransaction& tx, const CAmount blockReward, TxValidationState& state)
|
||||
{
|
||||
CCbTx cbTx;
|
||||
if (!GetTxPayload(tx, cbTx)) {
|
||||
@ -239,13 +239,8 @@ bool CCreditPoolDiff::SetTarget(const CTransaction& tx, TxValidationState& state
|
||||
|
||||
targetBalance = cbTx.creditPoolBalance;
|
||||
|
||||
|
||||
if (!llmq::utils::IsMNRewardReallocationActive(pindex)) return true;
|
||||
|
||||
CAmount blockReward = 0;
|
||||
for (const CTxOut& txout : tx.vout) {
|
||||
blockReward += txout.nValue;
|
||||
}
|
||||
platformReward = MasternodePayments::PlatformShare(GetMasternodePayment(cbTx.nHeight, blockReward, params.BRRHeight, true));
|
||||
LogPrintf("CreditPool: set target to %lld with MN reward %lld\n", *targetBalance, platformReward);
|
||||
|
||||
@ -292,11 +287,18 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state)
|
||||
return true;
|
||||
}
|
||||
|
||||
bool CCreditPoolDiff::ProcessTransaction(const CTransaction& tx, TxValidationState& state)
|
||||
bool CCreditPoolDiff::ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockReward, TxValidationState& state)
|
||||
{
|
||||
if (tx.nVersion != 3) return true;
|
||||
if (tx.nType == TRANSACTION_COINBASE) return SetTarget(tx, state);
|
||||
|
||||
assert(tx.nType == TRANSACTION_COINBASE);
|
||||
|
||||
return SetTarget(tx, blockReward, state);
|
||||
}
|
||||
|
||||
bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state)
|
||||
{
|
||||
if (tx.nVersion != 3) return true;
|
||||
if (tx.nType != TRANSACTION_ASSET_LOCK && tx.nType != TRANSACTION_ASSET_UNLOCK) return true;
|
||||
|
||||
if (!CheckAssetLockUnlockTx(tx, pindex, pool.indexes, state)) {
|
||||
|
||||
@ -77,12 +77,19 @@ private:
|
||||
public:
|
||||
explicit CCreditPoolDiff(CCreditPool starter, const CBlockIndex *pindex, const Consensus::Params& consensusParams);
|
||||
|
||||
/**
|
||||
* This function should be called for each block's coinbase transaction
|
||||
* to change amount of credit pool
|
||||
* coinbase transaction's Payload must be valid if nVersion of coinbase transaction equals 3
|
||||
*/
|
||||
bool ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockReward, TxValidationState& state);
|
||||
|
||||
/**
|
||||
* This function should be called for each Asset Lock/Unlock tx
|
||||
* to change amount of credit pool
|
||||
* @return true if transaction can be included in this block
|
||||
*/
|
||||
bool ProcessTransaction(const CTransaction& tx, TxValidationState& state);
|
||||
bool ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state);
|
||||
|
||||
/**
|
||||
* This function should be called by miner for initialization of MasterNode reward
|
||||
@ -102,7 +109,7 @@ public:
|
||||
}
|
||||
|
||||
private:
|
||||
bool SetTarget(const CTransaction& tx, TxValidationState& state);
|
||||
bool SetTarget(const CTransaction& tx, const CAmount blockReward, TxValidationState& state);
|
||||
bool Lock(const CTransaction& tx, TxValidationState& state);
|
||||
bool Unlock(const CTransaction& tx, TxValidationState& state);
|
||||
};
|
||||
|
||||
@ -138,10 +138,8 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
|
||||
int64_t nTime1 = GetTimeMicros();
|
||||
|
||||
const CCreditPool creditPool = creditPoolManager->GetCreditPool(pindex->pprev, consensusParams);
|
||||
std::optional<CCreditPoolDiff> creditPoolDiff;
|
||||
if (bool fV20Active_context = llmq::utils::IsV20Active(pindex->pprev); fV20Active_context) {
|
||||
if (llmq::utils::IsV20Active(pindex->pprev)) {
|
||||
LogPrintf("%s: CCreditPool is %s\n", __func__, creditPool.ToString());
|
||||
creditPoolDiff.emplace(creditPool, pindex->pprev, consensusParams);
|
||||
}
|
||||
|
||||
for (const auto& ptr_tx : block.vtx) {
|
||||
@ -158,21 +156,6 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
|
||||
strprintf("Process Special Transaction failed (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));
|
||||
}
|
||||
if (creditPoolDiff != std::nullopt && !creditPoolDiff->ProcessTransaction(*ptr_tx, tx_state)) {
|
||||
assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS || tx_state.GetResult() == TxValidationResult::TX_BAD_SPECIAL);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
|
||||
strprintf("Process Special Transaction failed at Credit Pool (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));
|
||||
}
|
||||
}
|
||||
if (creditPoolDiff != std::nullopt) {
|
||||
CAmount locked_proposed{0};
|
||||
if(creditPoolDiff->GetTargetBalance()) locked_proposed = *creditPoolDiff->GetTargetBalance();
|
||||
|
||||
CAmount locked_calculated = creditPoolDiff->GetTotalLocked();
|
||||
if (locked_proposed != locked_calculated) {
|
||||
LogPrintf("%s: mismatched locked amount in CbTx: %lld against re-calculated: %lld\n", __func__, locked_proposed, locked_calculated);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-assetlocked-amount");
|
||||
}
|
||||
}
|
||||
|
||||
int64_t nTime2 = GetTimeMicros();
|
||||
@ -265,3 +248,49 @@ bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq:
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams,
|
||||
const CAmount blockReward, BlockValidationState& state)
|
||||
{
|
||||
AssertLockHeld(cs_main);
|
||||
|
||||
try {
|
||||
|
||||
if (!llmq::utils::IsV20Active(pindex->pprev)) return true;
|
||||
|
||||
const CCreditPool creditPool = creditPoolManager->GetCreditPool(pindex->pprev, consensusParams);
|
||||
LogPrintf("%s: CCreditPool is %s\n", __func__, creditPool.ToString());
|
||||
CCreditPoolDiff creditPoolDiff(creditPool, pindex->pprev, consensusParams);
|
||||
|
||||
for (const auto& ptr_tx : block.vtx) {
|
||||
TxValidationState tx_state;
|
||||
if (ptr_tx->IsCoinBase()) {
|
||||
if (!creditPoolDiff.ProcessCoinbaseTransaction(*ptr_tx, blockReward, tx_state)) {
|
||||
assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
|
||||
strprintf("Process Coinbase Transaction failed at Credit Pool (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));
|
||||
}
|
||||
} else {
|
||||
if (!creditPoolDiff.ProcessLockUnlockTransaction(*ptr_tx, tx_state)) {
|
||||
assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
|
||||
strprintf("Process Lock/Unlock Transaction failed at Credit Pool (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));
|
||||
}
|
||||
}
|
||||
}
|
||||
CAmount locked_proposed{0};
|
||||
if (creditPoolDiff.GetTargetBalance()) locked_proposed = *creditPoolDiff.GetTargetBalance();
|
||||
|
||||
CAmount locked_calculated = creditPoolDiff.GetTotalLocked();
|
||||
if (locked_proposed != locked_calculated) {
|
||||
LogPrintf("%s: mismatched locked amount in CbTx: %lld against re-calculated: %lld\n", __func__, locked_proposed, locked_calculated);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-assetlocked-amount");
|
||||
}
|
||||
|
||||
} catch (const std::exception& e) {
|
||||
LogPrintf("%s -- failed: %s\n", __func__, e.what());
|
||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-checkcreditpooldiff");
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@ -30,5 +30,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
|
||||
const Consensus::Params& consensusParams, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots,
|
||||
BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
|
||||
bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
|
||||
bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams,
|
||||
const CAmount blockReward, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
|
||||
|
||||
#endif // BITCOIN_EVO_SPECIALTXMAN_H
|
||||
|
||||
@ -463,7 +463,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
|
||||
// `state` is local here because used to log info about this specific tx
|
||||
TxValidationState state;
|
||||
|
||||
if (!creditPoolDiff->ProcessTransaction(iter->GetTx(), state)) {
|
||||
if (!creditPoolDiff->ProcessLockUnlockTransaction(iter->GetTx(), state)) {
|
||||
if (fUsingModified) {
|
||||
mapModifiedTx.get<ancestor_score>().erase(modit);
|
||||
failedTx.insert(iter);
|
||||
|
||||
@ -1985,6 +1985,7 @@ static int64_t nTimeForks = 0;
|
||||
static int64_t nTimeVerify = 0;
|
||||
static int64_t nTimeISFilter = 0;
|
||||
static int64_t nTimeSubsidy = 0;
|
||||
static int64_t nTimeCreditPool = 0;
|
||||
static int64_t nTimeValueValid = 0;
|
||||
static int64_t nTimePayeeValid = 0;
|
||||
static int64_t nTimeProcessSpecial = 0;
|
||||
@ -2368,14 +2369,22 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
|
||||
int64_t nTime5_2 = GetTimeMicros(); nTimeSubsidy += nTime5_2 - nTime5_1;
|
||||
LogPrint(BCLog::BENCHMARK, " - GetBlockSubsidy: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_2 - nTime5_1), nTimeSubsidy * MICRO, nTimeSubsidy * MILLI / nBlocksTotal);
|
||||
|
||||
if (!CheckCreditPoolDiffForBlock(block, pindex, m_params.GetConsensus(), blockReward, state)) {
|
||||
return error("ConnectBlock(DASH): CheckCreditPoolDiffForBlock for block %s failed with %s",
|
||||
pindex->GetBlockHash().ToString(), state.ToString());
|
||||
}
|
||||
|
||||
int64_t nTime5_3 = GetTimeMicros(); nTimeCreditPool += nTime5_3 - nTime5_2;
|
||||
LogPrint(BCLog::BENCHMARK, " - CheckCreditPoolDiffForBlock: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeCreditPool * MICRO, nTimeCreditPool * MILLI / nBlocksTotal);
|
||||
|
||||
if (!MasternodePayments::IsBlockValueValid(*sporkManager, *governance, *::masternodeSync, block, pindex->nHeight, blockReward, strError)) {
|
||||
// NOTE: Do not punish, the node might be missing governance data
|
||||
LogPrintf("ERROR: ConnectBlock(DASH): %s\n", strError);
|
||||
return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-amount");
|
||||
}
|
||||
|
||||
int64_t nTime5_3 = GetTimeMicros(); nTimeValueValid += nTime5_3 - nTime5_2;
|
||||
LogPrint(BCLog::BENCHMARK, " - IsBlockValueValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeValueValid * MICRO, nTimeValueValid * MILLI / nBlocksTotal);
|
||||
int64_t nTime5_4 = GetTimeMicros(); nTimeValueValid += nTime5_4 - nTime5_3;
|
||||
LogPrint(BCLog::BENCHMARK, " - IsBlockValueValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_4 - nTime5_3), nTimeValueValid * MICRO, nTimeValueValid * MILLI / nBlocksTotal);
|
||||
|
||||
if (!MasternodePayments::IsBlockPayeeValid(*sporkManager, *governance, *block.vtx[0], pindex->nHeight, blockReward)) {
|
||||
// NOTE: Do not punish, the node might be missing governance data
|
||||
@ -2383,8 +2392,8 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
|
||||
return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-payee");
|
||||
}
|
||||
|
||||
int64_t nTime5_4 = GetTimeMicros(); nTimePayeeValid += nTime5_4 - nTime5_3;
|
||||
LogPrint(BCLog::BENCHMARK, " - IsBlockPayeeValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_4 - nTime5_3), nTimePayeeValid * MICRO, nTimePayeeValid * MILLI / nBlocksTotal);
|
||||
int64_t nTime5_5 = GetTimeMicros(); nTimePayeeValid += nTime5_5 - nTime5_4;
|
||||
LogPrint(BCLog::BENCHMARK, " - IsBlockPayeeValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_5 - nTime5_4), nTimePayeeValid * MICRO, nTimePayeeValid * MILLI / nBlocksTotal);
|
||||
|
||||
int64_t nTime5 = GetTimeMicros(); nTimeDashSpecific += nTime5 - nTime4;
|
||||
LogPrint(BCLog::BENCHMARK, " - Dash specific: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5 - nTime4), nTimeDashSpecific * MICRO, nTimeDashSpecific * MILLI / nBlocksTotal);
|
||||
|
||||
@ -41,6 +41,7 @@ from test_framework.util import (
|
||||
assert_equal,
|
||||
assert_greater_than,
|
||||
assert_greater_than_or_equal,
|
||||
hex_str_to_bytes,
|
||||
)
|
||||
|
||||
llmq_type_test = 100
|
||||
@ -164,8 +165,15 @@ class AssetLocksTest(DashTestFramework):
|
||||
block_time = best_block["time"] + 1
|
||||
|
||||
cbb = create_coinbase(height, dip4_activated=True, v20_activated=True)
|
||||
cbb.calc_sha256()
|
||||
gbt = node_wallet.getblocktemplate()
|
||||
cbb.vExtraPayload = hex_str_to_bytes(gbt["coinbase_payload"])
|
||||
cbb.rehash()
|
||||
block = create_block(tip, cbb, block_time, version=4)
|
||||
# Add quorum commitments from block template
|
||||
for tx_obj in gbt["transactions"]:
|
||||
tx = FromHex(CTransaction(), tx_obj["data"])
|
||||
if tx.nType == 6:
|
||||
block.vtx.append(tx)
|
||||
for tx in txes:
|
||||
block.vtx.append(tx)
|
||||
block.hashMerkleRoot = block.calc_merkle_root()
|
||||
@ -281,6 +289,7 @@ class AssetLocksTest(DashTestFramework):
|
||||
self.sync_all()
|
||||
|
||||
self.log.info('Mine block with incorrect credit-pool value...')
|
||||
coin = coins.pop()
|
||||
extra_lock_tx = self.create_assetlock(coin, COIN, pubkey)
|
||||
self.create_and_check_block([extra_lock_tx], expected_error = 'bad-cbtx-assetlocked-amount')
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user