From d9fb724943b22b3170c5bf6c55dbe173d6a92aea Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 2 Dec 2016 01:58:55 +0400 Subject: [PATCH] Refactor IsBlockValueValid (#1177) * refactor IsBlockValueValid to return actual error string, use it in error message for bad-cb-amount * make error messages in IsBlockValueValid even more verbose --- src/main.cpp | 9 +++------ src/masternode-payments.cpp | 29 ++++++++++++++++++++++++++++- src/masternode-payments.h | 2 +- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 73f146367..bcaf22472 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2773,12 +2773,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // DASH : MODIFYED TO CHECK MASTERNODE PAYMENTS AND SUPERBLOCKS CAmount blockReward = nFees + GetBlockSubsidy(pindex->pprev->nBits, pindex->pprev->nHeight, chainparams.GetConsensus()); - if (!IsBlockValueValid(block, pindex->nHeight, blockReward)) { - LogPrintf("ConnectBlock() -- IsBlockValueValid returned false, nHeight = %d\n", pindex->nHeight); - // TODO: handle error here more accurate - this could actually fail for different reasons - return state.DoS(100, error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)", - block.vtx[0].GetValueOut(), blockReward), - REJECT_INVALID, "bad-cb-amount"); + std::string strError = ""; + if (!IsBlockValueValid(block, pindex->nHeight, blockReward, strError)) { + return state.DoS(100, error("ConnectBlock(): %s", strError), REJECT_INVALID, "bad-cb-amount"); } if (!IsBlockPayeeValid(block.vtx[0], pindex->nHeight, blockReward)) { diff --git a/src/masternode-payments.cpp b/src/masternode-payments.cpp index 453ae15f2..7a000916c 100644 --- a/src/masternode-payments.cpp +++ b/src/masternode-payments.cpp @@ -33,8 +33,10 @@ CCriticalSection cs_mapMasternodePaymentVotes; * - When non-superblocks are detected, the normal schedule should be maintained */ -bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockReward) +bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockReward, std::string &strErrorRet) { + strErrorRet = ""; + bool isNormalBlockValueMet = (block.vtx[0].GetValueOut() <= blockReward); if(fDebug) LogPrintf("block.vtx[0].GetValueOut() %lld <= blockReward %lld\n", block.vtx[0].GetValueOut(), blockReward); @@ -51,6 +53,10 @@ bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockRewar if(masternodeSync.IsSynced() && !sporkManager.IsSporkActive(SPORK_13_OLD_SUPERBLOCK_FLAG)) { // no budget blocks should be accepted here, if SPORK_13_OLD_SUPERBLOCK_FLAG is disabled LogPrint("gobject", "IsBlockValueValid -- Client synced but budget spork is disabled, checking block value against normal block reward\n"); + if(!isNormalBlockValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded normal block payment limit, budgets are disabled", + nBlockHeight, block.vtx[0].GetValueOut(), blockReward); + } return isNormalBlockValueMet; } LogPrint("gobject", "IsBlockValueValid -- WARNING: Skipping budget block value checks, accepting block\n"); @@ -58,6 +64,10 @@ bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockRewar return true; } // LogPrint("gobject", "IsBlockValueValid -- Block is not in budget cycle window, checking block value against normal block reward\n"); + if(!isNormalBlockValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded normal block payment limit, block is not in budget cycle window", + nBlockHeight, block.vtx[0].GetValueOut(), blockReward); + } return isNormalBlockValueMet; } @@ -72,8 +82,16 @@ bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockRewar // not enough data but at least it must NOT exceed superblock max value if(CSuperblock::IsValidBlockHeight(nBlockHeight)) { if(fDebug) LogPrintf("IsBlockPayeeValid -- WARNING: Client not synced, checking superblock max bounds only\n"); + if(!isSuperblockMaxValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded superblock payment limit", + nBlockHeight, block.vtx[0].GetValueOut(), nSuperblockPaymentsLimit); + } return isSuperblockMaxValueMet; } + if(!isNormalBlockValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded normal block payment limit, only normal blocks are allowed at this height", + nBlockHeight, block.vtx[0].GetValueOut(), blockReward); + } // it MUST be a regular block otherwise return isNormalBlockValueMet; } @@ -91,12 +109,21 @@ bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockRewar // triggered but invalid? that's weird LogPrintf("IsBlockValueValid -- ERROR: Invalid superblock detected at height %d: %s", nBlockHeight, block.vtx[0].ToString()); // should NOT allow invalid superblocks, when superblocks are enabled + strErrorRet = strprintf("invalid superblock detected at height %d", nBlockHeight); return false; } LogPrint("gobject", "IsBlockValueValid -- No triggered superblock detected at height %d\n", nBlockHeight); + if(!isNormalBlockValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded normal block payment limit, no triggered superblock detected", + nBlockHeight, block.vtx[0].GetValueOut(), blockReward); + } } else { // should NOT allow superblocks at all, when superblocks are disabled LogPrint("gobject", "IsBlockValueValid -- Superblocks are disabled, no superblocks allowed\n"); + if(!isNormalBlockValueMet) { + strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded normal block payment limit, superblocks are disabled", + nBlockHeight, block.vtx[0].GetValueOut(), blockReward); + } } // it MUST be a regular block diff --git a/src/masternode-payments.h b/src/masternode-payments.h index 2bc80c118..fdbbb2cc6 100644 --- a/src/masternode-payments.h +++ b/src/masternode-payments.h @@ -33,7 +33,7 @@ extern CCriticalSection cs_mapMasternodePayeeVotes; extern CMasternodePayments mnpayments; /// TODO: all 4 functions do not belong here really, they should be refactored/moved somewhere (main.cpp ?) -bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockReward); +bool IsBlockValueValid(const CBlock& block, int nBlockHeight, CAmount blockReward, std::string &strErrorRet); bool IsBlockPayeeValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward); void FillBlockPayments(CMutableTransaction& txNew, int nBlockHeight, CAmount blockReward, CTxOut& txoutMasternodeRet, std::vector& voutSuperblockRet); std::string GetRequiredPaymentsString(int nBlockHeight);