From eb271ef6fdf79e841839201dcccd707c0390a972 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Thu, 29 Apr 2021 06:05:09 +0200 Subject: [PATCH 1/4] [Refactoring] Remove mocked BudgetManagerTest::IsBlockValueValid Since we can directly update the sporkManager, we can check the actual function, decoupling the maps update in AddFinalizedBudget --- src/budget/budgetmanager.cpp | 22 ++++---- src/budget/budgetmanager.h | 1 + src/test/budget_tests.cpp | 101 +++++++++++++++++++++-------------- 3 files changed, 75 insertions(+), 49 deletions(-) diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp index 383fc210a637..8520533f0d8a 100644 --- a/src/budget/budgetmanager.cpp +++ b/src/budget/budgetmanager.cpp @@ -217,20 +217,24 @@ bool CBudgetManager::AddFinalizedBudget(CFinalizedBudget& finalizedBudget) } SetBudgetProposalsStr(finalizedBudget); - { - LOCK(cs_budgets); - mapFinalizedBudgets.emplace(nHash, finalizedBudget); - // Add to feeTx index - mapFeeTxToBudget.emplace(feeTxId, nHash); - // Remove the budget from the unconfirmed map, if it was there - if (mapUnconfirmedFeeTx.count(nHash)) - mapUnconfirmedFeeTx.erase(nHash); - } + ForceAddFinalizedBudget(nHash, feeTxId, finalizedBudget); + LogPrint(BCLog::MNBUDGET,"%s: finalized budget %s [%s (%s)] added\n", __func__, nHash.ToString(), finalizedBudget.GetName(), finalizedBudget.GetProposalsStr()); return true; } +void CBudgetManager::ForceAddFinalizedBudget(const uint256& nHash, const uint256& feeTxId, const CFinalizedBudget& finalizedBudget) +{ + LOCK(cs_budgets); + mapFinalizedBudgets.emplace(nHash, finalizedBudget); + // Add to feeTx index + mapFeeTxToBudget.emplace(feeTxId, nHash); + // Remove the budget from the unconfirmed map, if it was there + if (mapUnconfirmedFeeTx.count(nHash)) + mapUnconfirmedFeeTx.erase(nHash); +} + bool CBudgetManager::AddProposal(CBudgetProposal& budgetProposal) { AssertLockNotHeld(cs_proposals); // need to lock cs_main here (CheckCollateral) diff --git a/src/budget/budgetmanager.h b/src/budget/budgetmanager.h index 8245962250f3..add1de973cee 100644 --- a/src/budget/budgetmanager.h +++ b/src/budget/budgetmanager.h @@ -119,6 +119,7 @@ class CBudgetManager bool IsBudgetPaymentBlock(int nBlockHeight, int& nCountThreshold) const; bool AddProposal(CBudgetProposal& budgetProposal); bool AddFinalizedBudget(CFinalizedBudget& finalizedBudget); + void ForceAddFinalizedBudget(const uint256& nHash, const uint256& feeTxId, const CFinalizedBudget& finalizedBudget); uint256 SubmitFinalBudget(); bool UpdateProposal(const CBudgetVote& vote, CNode* pfrom, std::string& strError); diff --git a/src/test/budget_tests.cpp b/src/test/budget_tests.cpp index 2ab42dcf9a51..6145b04881a3 100644 --- a/src/test/budget_tests.cpp +++ b/src/test/budget_tests.cpp @@ -14,30 +14,6 @@ #include -class BudgetManagerTest : CBudgetManager -{ -public: - void ForceAddFinalizedBudget(CFinalizedBudget& finalizedBudget) - { - LOCK(cs_budgets); - const uint256& nHash = finalizedBudget.GetHash(); - mapFinalizedBudgets.emplace(nHash, finalizedBudget); - mapFeeTxToBudget.emplace(finalizedBudget.GetFeeTXHash(), nHash); - } - - bool IsBlockValueValid(int nHeight, CAmount nExpectedValue, CAmount nMinted, bool fSporkActive = true) - { - // suppose masternodeSync is complete - if (fSporkActive) { - // add current payee amount to the expected block value - CAmount expectedPayAmount; - if (GetExpectedPayeeAmount(nHeight, expectedPayAmount)) { - nExpectedValue += expectedPayAmount; - } - } - return nMinted <= nExpectedValue; - } -}; BOOST_FIXTURE_TEST_SUITE(budget_tests, TestingSetup) @@ -63,20 +39,35 @@ BOOST_AUTO_TEST_CASE(budget_value) BOOST_AUTO_TEST_CASE(block_value) { - BudgetManagerTest t_budgetman; SelectParams(CBaseChainParams::TESTNET); std::string strError; + // force mnsync complete + masternodeSync.RequestedMasternodeAssets = MASTERNODE_SYNC_FINISHED; + + // enable SPORK_13 + int64_t nTime = GetTime() - 10; + const CSporkMessage& spork = CSporkMessage(SPORK_13_ENABLE_SUPERBLOCKS, nTime + 1, nTime); + sporkManager.AddSporkMessage(spork); + BOOST_CHECK(sporkManager.IsSporkActive(SPORK_13_ENABLE_SUPERBLOCKS)); + // regular block int nHeight = 100; - volatile CAmount nExpected = GetBlockValue(nHeight); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected-1)); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected)); - BOOST_CHECK(!t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+1)); + const CAmount nBlockReward = GetBlockValue(nHeight); + CAmount nExpectedRet = nBlockReward; + CAmount nBudgetAmtRet = 0; + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward-1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); + BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); // superblock - create the finalized budget with a proposal, and vote on it nHeight = 144; - nExpected = GetBlockValue(nHeight); const CTxIn mnVin(GetRandHash(), 0); const CScript payee = GetScriptForDestination(CKeyID(uint160(ParseHex("816115944e077fe7c803cfa57f29b36bf87c1d35")))); const CAmount propAmt = 100 * COIN; @@ -85,20 +76,50 @@ BOOST_AUTO_TEST_CASE(block_value) CFinalizedBudget fin("main (test)", 144, {txBudgetPayment}, finTxId); const CFinalizedBudgetVote fvote(mnVin, fin.GetHash()); BOOST_CHECK(fin.AddOrUpdateVote(fvote, strError)); - t_budgetman.ForceAddFinalizedBudget(fin); + g_budgetman.ForceAddFinalizedBudget(fin.GetHash(), fin.GetFeeTXHash(), fin); // check superblock's block-value - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected)); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+propAmt-1)); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+propAmt)); - BOOST_CHECK(!t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+propAmt+1)); + nExpectedRet = nBlockReward; + nBudgetAmtRet = 0; + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward + propAmt); + BOOST_CHECK_EQUAL(nBudgetAmtRet, propAmt); + nExpectedRet = nBlockReward; + nBudgetAmtRet = 0; + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt-1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward + propAmt); + BOOST_CHECK_EQUAL(nBudgetAmtRet, propAmt); + nExpectedRet = nBlockReward; + nBudgetAmtRet = 0; + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward + propAmt); + BOOST_CHECK_EQUAL(nBudgetAmtRet, propAmt); + nExpectedRet = nBlockReward; + nBudgetAmtRet = 0; + BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt+1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward + propAmt); + BOOST_CHECK_EQUAL(nBudgetAmtRet, propAmt); + + // disable SPORK_13 + const CSporkMessage& spork2 = CSporkMessage(SPORK_13_ENABLE_SUPERBLOCKS, 4070908800ULL, nTime + 1); + sporkManager.AddSporkMessage(spork2); + BOOST_CHECK(!sporkManager.IsSporkActive(SPORK_13_ENABLE_SUPERBLOCKS)); // check with spork disabled - nExpected = GetBlockValue(nHeight); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected-1, false)); - BOOST_CHECK(t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected, false)); - BOOST_CHECK(!t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+1, false)); - BOOST_CHECK(!t_budgetman.IsBlockValueValid(nHeight, nExpected, nExpected+propAmt, false)); + nExpectedRet = nBlockReward; + nBudgetAmtRet = 0; + BOOST_CHECK(IsBlockValueValid(nHeight, nExpectedRet, nBlockReward, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); + BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt-1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); + BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); + BOOST_CHECK(!IsBlockValueValid(nHeight, nExpectedRet, nBlockReward+propAmt+1, nBudgetAmtRet)); + BOOST_CHECK_EQUAL(nExpectedRet, nBlockReward); + BOOST_CHECK_EQUAL(nBudgetAmtRet, 0); } static CScript GetRandomP2PKH() From 7c687506271f223df5ab7ec333291dbb96a70697 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Thu, 29 Apr 2021 06:34:05 +0200 Subject: [PATCH 2/4] [Cleanup][Trivial] Remove un-implemented function ExecuteSpork --- src/spork.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/spork.h b/src/spork.h index 97b6ecbbc336..d2ef2cdeec52 100644 --- a/src/spork.h +++ b/src/spork.h @@ -108,7 +108,6 @@ class CSporkManager void ProcessSpork(CNode* pfrom, std::string& strCommand, CDataStream& vRecv); int64_t GetSporkValue(SporkId nSporkID); - void ExecuteSpork(SporkId nSporkID, int nValue); // Create/Sign/Relay the spork message, and update the maps bool UpdateSpork(SporkId nSporkID, int64_t nValue); // Add spork message to mapSporks and mapSporksActive From 76fbe44b5e70349b97da03bf99dc20683fa71a9a Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 30 Apr 2021 14:32:57 +0200 Subject: [PATCH 3/4] [Refactoring] Use AddSporkMessage globally for maps update --- src/spork.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 62939f1964e3..e942a25ffa89 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -74,6 +74,7 @@ void CSporkManager::LoadSporksFromDB() } // add spork to memory + AddSporkMessage(spork); mapSporks[spork.GetHash()] = spork; mapSporksActive[spork.nSporkID] = spork; std::time_t result = spork.nValue; @@ -131,7 +132,6 @@ int CSporkManager::ProcessSporkMsg(CSporkMessage& spork) return 0; } - uint256 hash = spork.GetHash(); std::string sporkName = sporkManager.GetSporkNameByID(spork.nSporkID); std::string strStatus; { @@ -173,11 +173,7 @@ int CSporkManager::ProcessSporkMsg(CSporkMessage& spork) LogPrintf("%s : got %s spork %d (%s) with value %d (signed at %d)\n", __func__, strStatus, spork.nSporkID, sporkName, spork.nValue, spork.nTimeSigned); - { - LOCK(cs); - mapSporks[hash] = spork; - mapSporksActive[spork.nSporkID] = spork; - } + AddSporkMessage(spork); spork.Relay(); // PIVX: add to spork database. From 42be32c9b755bdd50342a0e705f240e3556cdd83 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 30 Apr 2021 14:42:32 +0200 Subject: [PATCH 4/4] [Trivial] Rename AddSporkMessage --> AddOrUpdateSporkMessage --- src/spork.cpp | 11 +++++------ src/spork.h | 2 +- src/test/budget_tests.cpp | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index e942a25ffa89..ba286c28fc73 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -74,9 +74,8 @@ void CSporkManager::LoadSporksFromDB() } // add spork to memory - AddSporkMessage(spork); - mapSporks[spork.GetHash()] = spork; - mapSporksActive[spork.nSporkID] = spork; + AddOrUpdateSporkMessage(spork); + std::time_t result = spork.nValue; // If SPORK Value is greater than 1,000,000 assume it's actually a Date and then convert to a more readable format std::string sporkName = sporkManager.GetSporkNameByID(spork.nSporkID); @@ -173,7 +172,7 @@ int CSporkManager::ProcessSporkMsg(CSporkMessage& spork) LogPrintf("%s : got %s spork %d (%s) with value %d (signed at %d)\n", __func__, strStatus, spork.nSporkID, sporkName, spork.nValue, spork.nTimeSigned); - AddSporkMessage(spork); + AddOrUpdateSporkMessage(spork); spork.Relay(); // PIVX: add to spork database. @@ -207,14 +206,14 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue) if(spork.Sign(strMasterPrivKey)){ spork.Relay(); - AddSporkMessage(spork); + AddOrUpdateSporkMessage(spork); return true; } return false; } -void CSporkManager::AddSporkMessage(const CSporkMessage& spork) +void CSporkManager::AddOrUpdateSporkMessage(const CSporkMessage& spork) { LOCK(cs); mapSporks[spork.GetHash()] = spork; diff --git a/src/spork.h b/src/spork.h index d2ef2cdeec52..89d18955e3f6 100644 --- a/src/spork.h +++ b/src/spork.h @@ -111,7 +111,7 @@ class CSporkManager // Create/Sign/Relay the spork message, and update the maps bool UpdateSpork(SporkId nSporkID, int64_t nValue); // Add spork message to mapSporks and mapSporksActive - void AddSporkMessage(const CSporkMessage& spork); + void AddOrUpdateSporkMessage(const CSporkMessage& spork); bool IsSporkActive(SporkId nSporkID); std::string GetSporkNameByID(SporkId id); diff --git a/src/test/budget_tests.cpp b/src/test/budget_tests.cpp index 6145b04881a3..ba518cccd013 100644 --- a/src/test/budget_tests.cpp +++ b/src/test/budget_tests.cpp @@ -48,7 +48,7 @@ BOOST_AUTO_TEST_CASE(block_value) // enable SPORK_13 int64_t nTime = GetTime() - 10; const CSporkMessage& spork = CSporkMessage(SPORK_13_ENABLE_SUPERBLOCKS, nTime + 1, nTime); - sporkManager.AddSporkMessage(spork); + sporkManager.AddOrUpdateSporkMessage(spork); BOOST_CHECK(sporkManager.IsSporkActive(SPORK_13_ENABLE_SUPERBLOCKS)); // regular block @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(block_value) // disable SPORK_13 const CSporkMessage& spork2 = CSporkMessage(SPORK_13_ENABLE_SUPERBLOCKS, 4070908800ULL, nTime + 1); - sporkManager.AddSporkMessage(spork2); + sporkManager.AddOrUpdateSporkMessage(spork2); BOOST_CHECK(!sporkManager.IsSporkActive(SPORK_13_ENABLE_SUPERBLOCKS)); // check with spork disabled @@ -181,7 +181,7 @@ BOOST_AUTO_TEST_CASE(IsCoinbaseValueValid_test) // enable SPORK_8 int64_t nTime = GetTime() - 10; const CSporkMessage& spork = CSporkMessage(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT, nTime + 1, nTime); - sporkManager.AddSporkMessage(spork); + sporkManager.AddOrUpdateSporkMessage(spork); BOOST_CHECK(sporkManager.IsSporkActive(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT)); // Underpaying with SPORK_8 enabled