Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Validation] Remove temporary guard for 5.3 rules pre-enforcement #2549

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/masternode-payments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ std::string CMasternodePaymentWinner::GetStrMessage() const
bool CMasternodePaymentWinner::IsValid(CNode* pnode, std::string& strError, int chainHeight)
{
int n = mnodeman.GetMasternodeRank(vinMasternode, nBlockHeight - 100);
bool guard = Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3);
if ((guard && n < 1) || n > MNPAYMENTS_SIGNATURES_TOTAL) {
if (n < 1 || n > MNPAYMENTS_SIGNATURES_TOTAL) {
//It's common to have masternodes mistakenly think they are in the top 10
// We don't want to print all of these messages, or punish them unless they're way off
if (n > MNPAYMENTS_SIGNATURES_TOTAL * 2) {
Expand All @@ -177,7 +176,7 @@ bool CMasternodePaymentWinner::IsValid(CNode* pnode, std::string& strError, int
}

// Must be a P2PKH
if (guard && !payee.IsPayToPublicKeyHash()) {
if (!payee.IsPayToPublicKeyHash()) {
LogPrint(BCLog::MASTERNODE, "%s - payee must be a P2PKH\n", __func__);
return false;
}
Expand Down Expand Up @@ -225,9 +224,7 @@ bool IsBlockValueValid(int nHeight, CAmount& nExpectedValue, CAmount nMinted, CA
}
}

// !todo: remove after V5.3 enforcement and default it to true
const bool isUpgradeEnforced = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_3);
return (!isUpgradeEnforced || nMinted >= 0) && nMinted <= nExpectedValue;
return nMinted >= 0 && nMinted <= nExpectedValue;
}

bool IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pindexPrev)
Expand Down Expand Up @@ -332,8 +329,7 @@ bool CMasternodePayments::GetLegacyMasternodeTxOut(int nHeight, std::vector<CTxO
if (!GetBlockPayee(nHeight, payee)) {
//no masternode detected
const Consensus::Params& consensus = Params().GetConsensus();
const uint256& hash = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_3) ?
mnodeman.GetHashAtHeight(nHeight - 1) : consensus.hashGenesisBlock;
const uint256& hash = mnodeman.GetHashAtHeight(nHeight - 1);
MasternodeRef winningNode = mnodeman.GetCurrentMasterNode(hash);
if (winningNode) {
payee = winningNode->GetPayeeScript();
Expand Down
20 changes: 5 additions & 15 deletions src/masternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ int MasternodeRemovalSeconds()
// Used for sigTime < maxTimeWindow
int64_t GetMaxTimeWindow(int chainHeight)
{
bool isV5_3 = Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3);
return GetAdjustedTime() + (isV5_3 ? (60 * 2) : (60 * 60));
return GetAdjustedTime() + 60 * 2;
}


Expand Down Expand Up @@ -301,13 +300,6 @@ bool CMasternodeBroadcast::Create(const std::string& strService,
if (!CheckDefaultPort(_service, strErrorRet, "CMasternodeBroadcast::Create"))
return false;

// Check if the MN has a ADDRv2 and reject it if the new NU wasn't enforced.
if (!_service.IsAddrV1Compatible() &&
!Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3)) {
strErrorRet = "Cannot start MN with a v2 address before the v5.3 enforcement";
return false;
}

return Create(txin, _service, keyCollateralAddressNew, pubKeyCollateralAddressNew, keyMasternodeNew, pubKeyMasternodeNew, strErrorRet, mnbRet);
}

Expand Down Expand Up @@ -412,9 +404,8 @@ bool CMasternodeBroadcast::CheckAndUpdate(int& nDos, int nChainHeight)
return false;
}

// reject old signature version after v5.3
if (nMessVersion != MessageVersion::MESS_VER_HASH &&
Params().GetConsensus().NetworkUpgradeActive(nChainHeight, Consensus::UPGRADE_V5_3)) {
// reject old signature version
if (nMessVersion != MessageVersion::MESS_VER_HASH) {
LogPrint(BCLog::MASTERNODE, "mnb - rejecting old message version for mn %s\n", vin.prevout.hash.ToString());
nDos = 30;
return false;
Expand Down Expand Up @@ -619,9 +610,8 @@ bool CMasternodePing::CheckAndUpdate(int& nDos, int nChainHeight, bool fRequireA
return false;
}

// reject old signature version after v5.3
if (nMessVersion != MessageVersion::MESS_VER_HASH &&
Params().GetConsensus().NetworkUpgradeActive(nChainHeight, Consensus::UPGRADE_V5_3)) {
// reject old signature version
if (nMessVersion != MessageVersion::MESS_VER_HASH) {
LogPrint(BCLog::MNPING, "mnp - rejecting old message version for mn %s\n", vin.prevout.hash.ToString());
nDos = 30;
return false;
Expand Down
20 changes: 2 additions & 18 deletions src/masternodeman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,6 @@ int CMasternodeMan::CheckAndRemove(bool forceExpiredRemoval)
return 0;
}

// !TODO: can be removed after enforcement
bool reject_v0 = Params().GetConsensus().NetworkUpgradeActive(GetBestHeight(), Consensus::UPGRADE_V5_3);

LOCK(cs);

//remove inactive and outdated (or replaced by DMN)
Expand All @@ -256,8 +253,7 @@ int CMasternodeMan::CheckAndRemove(bool forceExpiredRemoval)
if (activeState == CMasternode::MASTERNODE_REMOVE ||
activeState == CMasternode::MASTERNODE_VIN_SPENT ||
(forceExpiredRemoval && activeState == CMasternode::MASTERNODE_EXPIRED) ||
mn->protocolVersion < ActiveProtocol() ||
(reject_v0 && mn->nMessVersion != MessageVersion::MESS_VER_HASH)) {
mn->protocolVersion < ActiveProtocol()) {
LogPrint(BCLog::MASTERNODE, "Removing inactive (legacy) Masternode %s\n", it->first.ToString());
//erase all of the broadcasts we've seen from this vin
// -- if we missed a few pings and the node was removed, this will allow is to get it back without them
Expand Down Expand Up @@ -728,16 +724,8 @@ int CMasternodeMan::ProcessMNBroadcast(CNode* pfrom, CMasternodeBroadcast& mnb)
}

int chainHeight = GetBestHeight();
const auto& consensus = Params().GetConsensus();
// Check if mnb contains a ADDRv2 and reject it if the new NU wasn't enforced.
if (!mnb.addr.IsAddrV1Compatible() &&
!consensus.NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3)) {
LogPrint(BCLog::MASTERNODE, "mnb - received a ADDRv2 before enforcement\n");
return 33;
}

int nDoS = 0;
if (!mnb.CheckAndUpdate(nDoS, GetBestHeight())) {
if (!mnb.CheckAndUpdate(nDoS, chainHeight)) {
return nDoS;
}

Expand Down Expand Up @@ -880,10 +868,6 @@ int CMasternodeMan::ProcessMessageInner(CNode* pfrom, std::string& strCommand, C
return ProcessMNBroadcast(pfrom, mnb);

} else if (strCommand == NetMsgType::MNBROADCAST2) {
if (!Params().GetConsensus().NetworkUpgradeActive(GetBestHeight(), Consensus::UPGRADE_V5_3)) {
LogPrint(BCLog::MASTERNODE, "%s: mnb2 not enabled pre-V5.3 enforcement\n", __func__);
return 30;
}
CMasternodeBroadcast mnb;
OverrideStream<CDataStream> s(&vRecv, vRecv.GetType(), vRecv.GetVersion() | ADDRV2_FORMAT);
s >> mnb;
Expand Down
5 changes: 0 additions & 5 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -910,11 +910,6 @@ bool static PushTierTwoGetDataRequest(const CInv& inv,
if (it != mnodeman.mapSeenMasternodeBroadcast.end()) {
const auto& mnb = it->second;

// Just to be double sure, do not broadcast BIP155 addresses pre-v5.3 enforcement
if (mnb.isBIP155Addr && !Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3)) {
return false;
}

int version = mnb.isBIP155Addr ? PROTOCOL_VERSION | ADDRV2_FORMAT : PROTOCOL_VERSION;
CDataStream ss(SER_NETWORK, version);
ss.reserve(1000);
Expand Down
14 changes: 3 additions & 11 deletions src/test/budget_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ BOOST_FIXTURE_TEST_CASE(block_value, TestnetSetup)
CAmount nExpectedRet = nBlockReward;
CAmount nBudgetAmtRet = 0;

// under-minting
BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, -1, nBudgetAmtRet));

// regular block
BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, 0, nBudgetAmtRet));
BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward-1, nBudgetAmtRet));
Expand Down Expand Up @@ -129,17 +132,6 @@ BOOST_FIXTURE_TEST_CASE(block_value, TestnetSetup)
BOOST_CHECK_EQUAL(nBudgetAmtRet, 0);
}

BOOST_FIXTURE_TEST_CASE(block_value_undermint, RegTestingSetup)
{
int nHeight = 100;
CAmount nExpectedRet = GetBlockValue(nHeight);
CAmount nBudgetAmtRet = 0;
// under-minting blocks are invalid after v6
BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, -1, nBudgetAmtRet));
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_3, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, -1, nBudgetAmtRet));
}

/**
* 1) Create two proposals and two budget finalizations with a different proposal payment order:
BudA pays propA and propB, BudB pays propB and propA.
Expand Down
16 changes: 2 additions & 14 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2076,9 +2076,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st
// Update chainActive & related variables.
UpdateTip(pindexNew);
// Update TierTwo managers
// !TODO: remove isV5_3 guard after v5.3 enforcement
bool isV5_3 = Params().GetConsensus().NetworkUpgradeActive(pindexNew->nHeight, Consensus::UPGRADE_V5_3);
if (!fLiteMode && isV5_3) {
if (!fLiteMode) {
mnodeman.SetBestHeight(pindexNew->nHeight);
g_budgetman.SetBestHeight(pindexNew->nHeight);
}
Expand Down Expand Up @@ -2942,8 +2940,7 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
}
}

bool fActiveV5_3 = chainparams.GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_3);
if (fActiveV5_3 && block.IsProofOfStake()) {
if (block.IsProofOfStake()) {
CTransactionRef csTx = block.vtx[1];
if (csTx->vin.size() > 1) {
return state.DoS(100, false, REJECT_INVALID, "bad-cs-multi-inputs", false,
Expand Down Expand Up @@ -3369,15 +3366,6 @@ bool ProcessNewBlock(const std::shared_ptr<const CBlock>& pblock, const FlatFile
if (!ActivateBestChain(state, pblock))
return error("%s : ActivateBestChain failed", __func__);

// !TODO: remove after v5.3 enforcement
bool isV5_3 = Params().GetConsensus().NetworkUpgradeActive(
WITH_LOCK(cs_main, return chainActive.Height(); ),
Consensus::UPGRADE_V5_3);
if (!fLiteMode && !isV5_3) {
mnodeman.SetBestHeight(newHeight);
g_budgetman.SetBestHeight(newHeight);
}

LogPrintf("%s : ACCEPTED Block %ld in %ld milliseconds with size=%d\n", __func__, newHeight, GetTimeMillis() - nStartTime,
GetSerializeSize(*pblock, CLIENT_VERSION));

Expand Down