Skip to content

Commit 6e639c7

Browse files
refactor: use more gsl::not_null in utils.h and deterministicmns.h (dashpay#5651)
## Issue being fixed or feature implemented Use not_null if the function would crash if given a nullptr ## What was done? Refactored to use gsl::not_null ## How Has This Been Tested? Compiled ## Breaking Changes Should be none ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [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)_
1 parent d5b2c26 commit 6e639c7

File tree

5 files changed

+167
-182
lines changed

5 files changed

+167
-182
lines changed

src/evo/deterministicmns.cpp

Lines changed: 92 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ static bool CompareByLastPaid(const CDeterministicMN* _a, const CDeterministicMN
178178
return CompareByLastPaid(*_a, *_b);
179179
}
180180

181-
CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(const CBlockIndex* pIndex) const
181+
CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(gsl::not_null<const CBlockIndex*> pIndex) const
182182
{
183183
if (mnMap.size() == 0) {
184184
return nullptr;
@@ -214,7 +214,7 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(const CBlockIndex* pIndex)
214214
return best;
215215
}
216216

217-
std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayees(const CBlockIndex* const pindex, int nCount) const
217+
std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayees(gsl::not_null<const CBlockIndex* const> pindex, int nCount) const
218218
{
219219
if (nCount < 0 ) {
220220
return {};
@@ -421,7 +421,7 @@ CDeterministicMNListDiff CDeterministicMNList::BuildDiff(const CDeterministicMNL
421421
return diffRet;
422422
}
423423

424-
CDeterministicMNList CDeterministicMNList::ApplyDiff(const CBlockIndex* pindex, const CDeterministicMNListDiff& diff) const
424+
CDeterministicMNList CDeterministicMNList::ApplyDiff(gsl::not_null<const CBlockIndex*> pindex, const CDeterministicMNListDiff& diff) const
425425
{
426426
CDeterministicMNList result = *this;
427427
result.blockHash = pindex->GetBlockHash();
@@ -594,7 +594,7 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash)
594594
mnInternalIdMap = mnInternalIdMap.erase(dmn->GetInternalId());
595595
}
596596

597-
bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockIndex* pindex, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck)
597+
bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck)
598598
{
599599
AssertLockHeld(cs_main);
600600

@@ -660,7 +660,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
660660
return true;
661661
}
662662

663-
bool CDeterministicMNManager::UndoBlock(const CBlockIndex* pindex)
663+
bool CDeterministicMNManager::UndoBlock(gsl::not_null<const CBlockIndex*> pindex)
664664
{
665665
int nHeight = pindex->nHeight;
666666
uint256 blockHash = pindex->GetBlockHash();
@@ -696,14 +696,14 @@ bool CDeterministicMNManager::UndoBlock(const CBlockIndex* pindex)
696696
return true;
697697
}
698698

699-
void CDeterministicMNManager::UpdatedBlockTip(const CBlockIndex* pindex)
699+
void CDeterministicMNManager::UpdatedBlockTip(gsl::not_null<const CBlockIndex*> pindex)
700700
{
701701
LOCK(cs);
702702

703703
tipIndex = pindex;
704704
}
705705

706-
bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, bool debugLogs)
706+
bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev, BlockValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, bool debugLogs)
707707
{
708708
int nHeight = pindexPrev->nHeight + 1;
709709

@@ -991,7 +991,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C
991991
return true;
992992
}
993993

994-
void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pQuorumBaseBlockIndex, CDeterministicMNList& mnList, bool debugLogs)
994+
void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitment& qc, gsl::not_null<const CBlockIndex*> pQuorumBaseBlockIndex, CDeterministicMNList& mnList, bool debugLogs)
995995
{
996996
// The commitment has already been validated at this point, so it's safe to use members of it
997997

@@ -1011,7 +1011,7 @@ void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitmen
10111011
}
10121012
}
10131013

1014-
CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(const CBlockIndex* pindex)
1014+
CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_null<const CBlockIndex*> pindex)
10151015
{
10161016
AssertLockHeld(cs);
10171017
CDeterministicMNList snapshot;
@@ -1512,7 +1512,7 @@ static bool CheckHashSig(const ProTx& proTx, const CBLSPublicKey& pubKey, TxVali
15121512
return true;
15131513
}
15141514

1515-
bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
1515+
bool CheckProRegTx(const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
15161516
{
15171517
if (tx.nType != TRANSACTION_PROVIDER_REGISTER) {
15181518
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type");
@@ -1634,7 +1634,7 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVali
16341634
return true;
16351635
}
16361636

1637-
bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, bool check_sigs)
1637+
bool CheckProUpServTx(const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, bool check_sigs)
16381638
{
16391639
if (tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) {
16401640
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type");
@@ -1661,50 +1661,48 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxV
16611661
}
16621662
}
16631663

1664-
if (pindexPrev) {
1665-
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1666-
auto mn = mnList.GetMN(ptx.proTxHash);
1667-
if (!mn) {
1668-
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
1669-
}
1670-
1671-
// don't allow updating to addresses already used by other MNs
1672-
if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->proTxHash != ptx.proTxHash) {
1673-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr");
1674-
}
1664+
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1665+
auto mn = mnList.GetMN(ptx.proTxHash);
1666+
if (!mn) {
1667+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
1668+
}
16751669

1676-
// don't allow updating to platformNodeIds already used by other EvoNodes
1677-
if (ptx.nType == MnType::Evo) {
1678-
if (mnList.HasUniqueProperty(ptx.platformNodeID) && mnList.GetUniquePropertyMN(ptx.platformNodeID)->proTxHash != ptx.proTxHash) {
1679-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-platformnodeid");
1680-
}
1681-
}
1670+
// don't allow updating to addresses already used by other MNs
1671+
if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->proTxHash != ptx.proTxHash) {
1672+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr");
1673+
}
16821674

1683-
if (ptx.scriptOperatorPayout != CScript()) {
1684-
if (mn->nOperatorReward == 0) {
1685-
// don't allow setting operator reward payee in case no operatorReward was set
1686-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee");
1687-
}
1688-
if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash()) {
1689-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee");
1690-
}
1675+
// don't allow updating to platformNodeIds already used by other EvoNodes
1676+
if (ptx.nType == MnType::Evo) {
1677+
if (mnList.HasUniqueProperty(ptx.platformNodeID) && mnList.GetUniquePropertyMN(ptx.platformNodeID)->proTxHash != ptx.proTxHash) {
1678+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-platformnodeid");
16911679
}
1680+
}
16921681

1693-
// we can only check the signature if pindexPrev != nullptr and the MN is known
1694-
if (!CheckInputsHash(tx, ptx, state)) {
1695-
// pass the state returned by the function above
1696-
return false;
1682+
if (ptx.scriptOperatorPayout != CScript()) {
1683+
if (mn->nOperatorReward == 0) {
1684+
// don't allow setting operator reward payee in case no operatorReward was set
1685+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee");
16971686
}
1698-
if (check_sigs && !CheckHashSig(ptx, mn->pdmnState->pubKeyOperator.Get(), state)) {
1699-
// pass the state returned by the function above
1700-
return false;
1687+
if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash()) {
1688+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee");
17011689
}
17021690
}
17031691

1692+
// we can only check the signature if pindexPrev != nullptr and the MN is known
1693+
if (!CheckInputsHash(tx, ptx, state)) {
1694+
// pass the state returned by the function above
1695+
return false;
1696+
}
1697+
if (check_sigs && !CheckHashSig(ptx, mn->pdmnState->pubKeyOperator.Get(), state)) {
1698+
// pass the state returned by the function above
1699+
return false;
1700+
}
1701+
17041702
return true;
17051703
}
17061704

1707-
bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
1705+
bool CheckProUpRegTx(const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
17081706
{
17091707
if (tx.nType != TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {
17101708
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type");
@@ -1726,60 +1724,58 @@ bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
17261724
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-dest");
17271725
}
17281726

1729-
if (pindexPrev) {
1730-
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1731-
auto dmn = mnList.GetMN(ptx.proTxHash);
1732-
if (!dmn) {
1733-
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
1734-
}
1727+
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1728+
auto dmn = mnList.GetMN(ptx.proTxHash);
1729+
if (!dmn) {
1730+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
1731+
}
17351732

1736-
// don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server)
1737-
if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(ptx.keyIDVoting))) {
1738-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse");
1739-
}
1733+
// don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server)
1734+
if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(ptx.keyIDVoting))) {
1735+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse");
1736+
}
17401737

1741-
Coin coin;
1742-
if (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent()) {
1743-
// this should never happen (there would be no dmn otherwise)
1744-
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-collateral");
1745-
}
1738+
Coin coin;
1739+
if (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent()) {
1740+
// this should never happen (there would be no dmn otherwise)
1741+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-collateral");
1742+
}
17461743

1747-
// don't allow reuse of collateral key for other keys (don't allow people to put the collateral key onto an online server)
1748-
CTxDestination collateralTxDest;
1749-
if (!ExtractDestination(coin.out.scriptPubKey, collateralTxDest)) {
1750-
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-collateral-dest");
1751-
}
1752-
if (collateralTxDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(ptx.keyIDVoting))) {
1753-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse");
1754-
}
1744+
// don't allow reuse of collateral key for other keys (don't allow people to put the collateral key onto an online server)
1745+
CTxDestination collateralTxDest;
1746+
if (!ExtractDestination(coin.out.scriptPubKey, collateralTxDest)) {
1747+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-collateral-dest");
1748+
}
1749+
if (collateralTxDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(ptx.keyIDVoting))) {
1750+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse");
1751+
}
17551752

1756-
if (mnList.HasUniqueProperty(ptx.pubKeyOperator)) {
1757-
auto otherDmn = mnList.GetUniquePropertyMN(ptx.pubKeyOperator);
1758-
if (ptx.proTxHash != otherDmn->proTxHash) {
1759-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-key");
1760-
}
1753+
if (mnList.HasUniqueProperty(ptx.pubKeyOperator)) {
1754+
auto otherDmn = mnList.GetUniquePropertyMN(ptx.pubKeyOperator);
1755+
if (ptx.proTxHash != otherDmn->proTxHash) {
1756+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-key");
17611757
}
1758+
}
17621759

1763-
if (!deterministicMNManager->IsDIP3Enforced(pindexPrev->nHeight)) {
1764-
if (dmn->pdmnState->keyIDOwner != ptx.keyIDVoting) {
1765-
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-not-same");
1766-
}
1760+
if (!deterministicMNManager->IsDIP3Enforced(pindexPrev->nHeight)) {
1761+
if (dmn->pdmnState->keyIDOwner != ptx.keyIDVoting) {
1762+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-not-same");
17671763
}
1764+
}
17681765

1769-
if (!CheckInputsHash(tx, ptx, state)) {
1770-
// pass the state returned by the function above
1771-
return false;
1772-
}
1773-
if (check_sigs && !CheckHashSig(ptx, PKHash(dmn->pdmnState->keyIDOwner), state)) {
1774-
// pass the state returned by the function above
1775-
return false;
1776-
}
1766+
if (!CheckInputsHash(tx, ptx, state)) {
1767+
// pass the state returned by the function above
1768+
return false;
1769+
}
1770+
if (check_sigs && !CheckHashSig(ptx, PKHash(dmn->pdmnState->keyIDOwner), state)) {
1771+
// pass the state returned by the function above
1772+
return false;
17771773
}
17781774

17791775
return true;
17801776
}
17811777

1782-
bool CheckProUpRevTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, bool check_sigs)
1778+
bool CheckProUpRevTx(const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, bool check_sigs)
17831779
{
17841780
if (tx.nType != TRANSACTION_PROVIDER_UPDATE_REVOKE) {
17851781
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type");
@@ -1795,20 +1791,18 @@ bool CheckProUpRevTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
17951791
return false;
17961792
}
17971793

1798-
if (pindexPrev) {
1799-
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1800-
auto dmn = mnList.GetMN(ptx.proTxHash);
1801-
if (!dmn)
1802-
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
1794+
auto mnList = deterministicMNManager->GetListForBlock(pindexPrev);
1795+
auto dmn = mnList.GetMN(ptx.proTxHash);
1796+
if (!dmn)
1797+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
18031798

1804-
if (!CheckInputsHash(tx, ptx, state)) {
1805-
// pass the state returned by the function above
1806-
return false;
1807-
}
1808-
if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) {
1809-
// pass the state returned by the function above
1810-
return false;
1811-
}
1799+
if (!CheckInputsHash(tx, ptx, state)) {
1800+
// pass the state returned by the function above
1801+
return false;
1802+
}
1803+
if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) {
1804+
// pass the state returned by the function above
1805+
return false;
18121806
}
18131807

18141808
return true;

0 commit comments

Comments
 (0)