Skip to content

Commit 25f749a

Browse files
committed
[Refactor] Check confirmations once - remove nBlockFeeTx / nTime
Since we no longer add immature/unconfirmed proposal/budgets there's no reason to cache the collateral tx height.
1 parent 6b4c503 commit 25f749a

File tree

2 files changed

+31
-58
lines changed

2 files changed

+31
-58
lines changed

src/masternode-budget.cpp

Lines changed: 31 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,23 @@ std::map<uint256, int64_t> askedForSourceProposalOrBudget;
2323

2424
int nSubmittedFinalBudget;
2525

26-
bool CheckCollateral(const uint256& nTxCollateralHash, const uint256& nExpectedHash, std::string& strError, int64_t& nTime, int& nHeight, bool fBudgetFinalization)
26+
bool CheckCollateralConfs(const uint256& nTxCollateralHash, int nCurrentHeight, int nProposalHeight, std::string& strError)
27+
{
28+
//if we're syncing we won't have swiftTX information, so accept 1 confirmation
29+
const int nRequiredConfs = Params().GetConsensus().nBudgetFeeConfirmations;
30+
const int nConf = GetIXConfirmations(nTxCollateralHash) + nCurrentHeight - nProposalHeight + 1;
31+
32+
if (nConf < nRequiredConfs) {
33+
strError = strprintf("Collateral requires at least %d confirmations - %d confirmations "
34+
"(current height: %d, fee tx height: %d)", nRequiredConfs, nConf, nCurrentHeight, nProposalHeight);
35+
LogPrint(BCLog::MNBUDGET,"%s: %s\n", __func__, strError);
36+
return false;
37+
}
38+
return true;
39+
}
40+
41+
bool CheckCollateral(const uint256& nTxCollateralHash, const uint256& nExpectedHash, std::string& strError, int64_t& nTime, int nCurrentHeight, bool fBudgetFinalization)
2742
{
28-
nTime = 0; // Updated at the end.
2943
CTransaction txCollateral;
3044
uint256 nBlockHash;
3145
if (!GetTransaction(nTxCollateralHash, txCollateral, nBlockHash, true)) {
@@ -79,39 +93,26 @@ bool CheckCollateral(const uint256& nTxCollateralHash, const uint256& nExpectedH
7993
strError = strprintf("Collateral transaction %s is unconfirmed", nTxCollateralHash.ToString());
8094
return false;
8195
}
96+
nTime = 0;
97+
int nProposalHeight = 0;
8298
{
8399
LOCK(cs_main);
84100
BlockMap::iterator mi = mapBlockIndex.find(nBlockHash);
85101
if (mi != mapBlockIndex.end() && (*mi).second) {
86102
CBlockIndex* pindex = (*mi).second;
87103
if (chainActive.Contains(pindex)) {
88-
nHeight = pindex->nHeight;
104+
nProposalHeight = pindex->nHeight;
89105
nTime = pindex->nTime;
90106
}
91107
}
92108
}
93109

94-
if (!nTime) {
110+
if (!nProposalHeight) {
95111
strError = strprintf("Collateral transaction %s not in Active chain", nTxCollateralHash.ToString());
96112
return false;
97113
}
98114

99-
return true;
100-
}
101-
102-
bool CheckCollateralConfs(const uint256& nTxCollateralHash, int nCurrentHeight, int nProposalHeight, std::string& strError)
103-
{
104-
//if we're syncing we won't have swiftTX information, so accept 1 confirmation
105-
const int nRequiredConfs = Params().GetConsensus().nBudgetFeeConfirmations;
106-
const int nConf = GetIXConfirmations(nTxCollateralHash) + nCurrentHeight - nProposalHeight + 1;
107-
108-
if (nConf < nRequiredConfs) {
109-
strError = strprintf("Collateral requires at least %d confirmations - %d confirmations "
110-
"(current height: %d, fee tx height: %d)", nRequiredConfs, nConf, nCurrentHeight, nProposalHeight);
111-
LogPrint(BCLog::MNBUDGET,"%s: %s\n", __func__, strError);
112-
return false;
113-
}
114-
return true;
115+
return CheckCollateralConfs(nTxCollateralHash, nCurrentHeight, nProposalHeight, strError);
115116
}
116117

117118
void CBudgetManager::CheckOrphanVotes()
@@ -445,14 +446,15 @@ bool CBudgetManager::AddFinalizedBudget(CFinalizedBudget& finalizedBudget)
445446
}
446447

447448
std::string strError;
448-
if (!CheckCollateral(finalizedBudget.GetFeeTXHash(), nHash, strError, finalizedBudget.nTime, finalizedBudget.nBlockFeeTx, true)) {
449+
int nCurrentHeight = GetBestHeight();
450+
if (!CheckCollateral(finalizedBudget.GetFeeTXHash(), nHash, strError, finalizedBudget.nTime, nCurrentHeight, true)) {
449451
LogPrint(BCLog::MNBUDGET,"%s: invalid finalized budget (%s) collateral - %s\n",
450452
__func__, nHash.ToString(), strError);
451453
return false;
452454
}
453455

454-
// update confirms/expiration
455-
if (!finalizedBudget.UpdateValid(GetBestHeight())) {
456+
// update expiration
457+
if (!finalizedBudget.UpdateValid(nCurrentHeight)) {
456458
LogPrint(BCLog::MNBUDGET,"%s: invalid finalized budget: %s %s\n", __func__, nHash.ToString(), finalizedBudget.IsInvalidLogStr());
457459
return false;
458460
}
@@ -480,14 +482,15 @@ bool CBudgetManager::AddProposal(CBudgetProposal& budgetProposal)
480482
}
481483

482484
std::string strError;
483-
if (!CheckCollateral(budgetProposal.GetFeeTXHash(), nHash, strError, budgetProposal.nTime, budgetProposal.nBlockFeeTx, false)) {
485+
int nCurrentHeight = GetBestHeight();
486+
if (!CheckCollateral(budgetProposal.GetFeeTXHash(), nHash, strError, budgetProposal.nTime, nCurrentHeight, false)) {
484487
LogPrint(BCLog::MNBUDGET,"%s: invalid budget proposal (%s) collateral - %s\n",
485488
__func__, nHash.ToString(), strError);
486489
return false;
487490
}
488491

489-
// update confirms/expiration
490-
if (!budgetProposal.UpdateValid(GetBestHeight())) {
492+
// update expiration / heavily-downvoted
493+
if (!budgetProposal.UpdateValid(nCurrentHeight)) {
491494
LogPrint(BCLog::MNBUDGET,"%s: Invalid budget proposal %s %s\n", __func__, nHash.ToString(), budgetProposal.IsInvalidLogStr());
492495
return false;
493496
}
@@ -1286,7 +1289,6 @@ CBudgetProposal::CBudgetProposal()
12861289
nBlockStart = 0;
12871290
nBlockEnd = 0;
12881291
nAmount = 0;
1289-
nBlockFeeTx = 0;
12901292
nTime = 0;
12911293
fValid = true;
12921294
strInvalid = "";
@@ -1300,7 +1302,6 @@ CBudgetProposal::CBudgetProposal(std::string strProposalNameIn, std::string strU
13001302
nBlockEnd = nBlockEndIn;
13011303
address = addressIn;
13021304
nAmount = nAmountIn;
1303-
nBlockFeeTx = 0;
13041305
nFeeTXHash = nFeeTXHashIn;
13051306
fValid = true;
13061307
strInvalid = "";
@@ -1315,7 +1316,6 @@ CBudgetProposal::CBudgetProposal(const CBudgetProposal& other)
13151316
address = other.address;
13161317
nAmount = other.nAmount;
13171318
nTime = other.nTime;
1318-
nBlockFeeTx = other.nBlockFeeTx;
13191319
nFeeTXHash = other.nFeeTXHash;
13201320
mapVotes = other.mapVotes;
13211321
fValid = true;
@@ -1404,11 +1404,6 @@ bool CBudgetProposal::CheckAddress()
14041404
return true;
14051405
}
14061406

1407-
bool CBudgetProposal::CheckRequiredConfs(int nCurrentHeight)
1408-
{
1409-
return CheckCollateralConfs(nFeeTXHash, nCurrentHeight, nBlockFeeTx, strInvalid);
1410-
}
1411-
14121407
bool CBudgetProposal::IsWellFormed(const CAmount& nTotalBudget)
14131408
{
14141409
return CheckStartEnd() && CheckAmount(nTotalBudget) && CheckAddress();
@@ -1435,10 +1430,6 @@ bool CBudgetProposal::UpdateValid(int nCurrentHeight)
14351430
return false;
14361431
}
14371432

1438-
if (!CheckRequiredConfs(nCurrentHeight)) {
1439-
return false;
1440-
}
1441-
14421433
fValid = true;
14431434
strInvalid.clear();
14441435
return true;
@@ -1697,8 +1688,7 @@ CFinalizedBudget::CFinalizedBudget() :
16971688
vecBudgetPayments(),
16981689
nFeeTXHash(),
16991690
strProposals(""),
1700-
nTime(0),
1701-
nBlockFeeTx(0)
1691+
nTime(0)
17021692
{ }
17031693

17041694
CFinalizedBudget::CFinalizedBudget(const CFinalizedBudget& other) :
@@ -1711,8 +1701,7 @@ CFinalizedBudget::CFinalizedBudget(const CFinalizedBudget& other) :
17111701
vecBudgetPayments(other.vecBudgetPayments),
17121702
nFeeTXHash(other.nFeeTXHash),
17131703
strProposals(other.strProposals),
1714-
nTime(other.nTime),
1715-
nBlockFeeTx(other.nBlockFeeTx)
1704+
nTime(other.nTime)
17161705
{ }
17171706

17181707
bool CFinalizedBudget::AddOrUpdateVote(const CFinalizedBudgetVote& vote, std::string& strError)
@@ -1962,11 +1951,6 @@ bool CFinalizedBudget::CheckName()
19621951
return true;
19631952
}
19641953

1965-
bool CFinalizedBudget::CheckRequiredConfs(int nCurrentHeight)
1966-
{
1967-
return CheckCollateralConfs(nFeeTXHash, nCurrentHeight, nBlockFeeTx, strInvalid);
1968-
}
1969-
19701954
bool CFinalizedBudget::IsExpired(int nCurrentHeight)
19711955
{
19721956
// Remove obsolete finalized budgets after some time
@@ -1997,10 +1981,6 @@ bool CFinalizedBudget::UpdateValid(int nCurrentHeight)
19971981
return false;
19981982
}
19991983

2000-
if (!CheckRequiredConfs(nCurrentHeight)) {
2001-
return false;
2002-
}
2003-
20041984
fValid = true;
20051985
strInvalid.clear();
20061986
return true;

src/masternode-budget.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,6 @@ class CFinalizedBudget
409409
bool CheckStartEnd();
410410
bool CheckAmount(const CAmount& nTotalBudget);
411411
bool CheckName();
412-
bool CheckRequiredConfs(int nCurrentHeight);
413412

414413
protected:
415414
std::map<uint256, CFinalizedBudgetVote> mapVotes;
@@ -422,7 +421,6 @@ class CFinalizedBudget
422421
public:
423422
// Set in CBudgetManager::AddFinalizedBudget via CheckCollateral
424423
int64_t nTime;
425-
int nBlockFeeTx;
426424

427425
CFinalizedBudget();
428426
CFinalizedBudget(const CFinalizedBudget& other);
@@ -479,7 +477,6 @@ class CFinalizedBudget
479477
{
480478
READWRITE(LIMITED_STRING(strBudgetName, 20));
481479
READWRITE(nFeeTXHash);
482-
READWRITE(nBlockFeeTx);
483480
READWRITE(nTime);
484481
READWRITE(nBlockStart);
485482
READWRITE(vecBudgetPayments);
@@ -553,7 +550,6 @@ class CBudgetProposal
553550
bool CheckStartEnd();
554551
bool CheckAmount(const CAmount& nTotalBudget);
555552
bool CheckAddress();
556-
bool CheckRequiredConfs(int nCurrentHeight);
557553

558554
protected:
559555
std::map<uint256, CBudgetVote> mapVotes;
@@ -568,7 +564,6 @@ class CBudgetProposal
568564
public:
569565
// Set in CBudgetManager::AddProposal via CheckCollateral
570566
int64_t nTime;
571-
int nBlockFeeTx;
572567

573568
CBudgetProposal();
574569
CBudgetProposal(const CBudgetProposal& other);
@@ -633,14 +628,12 @@ class CBudgetProposal
633628
//for syncing with other clients
634629
READWRITE(LIMITED_STRING(strProposalName, 20));
635630
READWRITE(LIMITED_STRING(strURL, 64));
636-
READWRITE(nTime);
637631
READWRITE(nBlockStart);
638632
READWRITE(nBlockEnd);
639633
READWRITE(nAmount);
640634
READWRITE(*(CScriptBase*)(&address));
641635
READWRITE(nTime);
642636
READWRITE(nFeeTXHash);
643-
READWRITE(nBlockFeeTx);
644637

645638
//for saving to the serialized db
646639
READWRITE(mapVotes);

0 commit comments

Comments
 (0)