From ca77d06a250e738cf6934a5835b2dabe451cd73a Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Thu, 11 Jan 2024 21:43:01 -0600 Subject: [PATCH] refactor: make GetTxPayload return an Optional T instead of taking in a T& return (#5733) ## Issue being fixed or feature implemented We should avoid return by reference; especially return by reference with a bool flag indicating validity. ## What was done? Instead we use a std::optional ## How Has This Been Tested? Unit tests pass ## Breaking Changes Should be none ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] 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)_ --------- Co-authored-by: Konstantin Akimov Co-authored-by: UdjinM6 --- src/bloom.cpp | 41 +++-- src/core_write.cpp | 36 ++--- src/evo/assetlocktx.cpp | 21 +-- src/evo/cbtx.cpp | 53 ++++--- src/evo/creditpool.cpp | 23 ++- src/evo/deterministicmns.cpp | 190 ++++++++++++------------ src/evo/mnhftx.cpp | 18 +-- src/evo/simplifiedmns.cpp | 9 +- src/evo/specialtx.h | 23 ++- src/evo/specialtxman.cpp | 6 +- src/llmq/blockprocessor.cpp | 5 +- src/llmq/commitment.cpp | 5 +- src/rpc/blockchain.cpp | 5 +- src/rpc/evo.cpp | 8 +- src/rpc/rawtransaction.cpp | 4 +- src/test/evo_assetlocks_tests.cpp | 36 ++--- src/test/evo_deterministicmns_tests.cpp | 9 +- src/test/evo_mnhf_tests.cpp | 8 +- src/test/evo_trivialvalidation.cpp | 7 +- src/test/util/setup_common.cpp | 12 +- src/txmempool.cpp | 95 +++++------- 21 files changed, 288 insertions(+), 326 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 54c9204a2d..77eb2f3600 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -126,12 +126,11 @@ bool CBloomFilter::CheckSpecialTransactionMatchesAndUpdate(const CTransaction &t } switch(tx.nType) { case(TRANSACTION_PROVIDER_REGISTER): { - CProRegTx proTx; - if (GetTxPayload(tx, proTx)) { - if(contains(proTx.collateralOutpoint) || - contains(proTx.keyIDOwner) || - contains(proTx.keyIDVoting) || - CheckScript(proTx.scriptPayout)) { + if (const auto opt_proTx = GetTxPayload(tx)) { + if(contains(opt_proTx->collateralOutpoint) || + contains(opt_proTx->keyIDOwner) || + contains(opt_proTx->keyIDVoting) || + CheckScript(opt_proTx->scriptPayout)) { if ((nFlags & BLOOM_UPDATE_MASK) == BLOOM_UPDATE_ALL) insert(tx.GetHash()); return true; @@ -140,47 +139,43 @@ bool CBloomFilter::CheckSpecialTransactionMatchesAndUpdate(const CTransaction &t return false; } case(TRANSACTION_PROVIDER_UPDATE_SERVICE): { - CProUpServTx proTx; - if (GetTxPayload(tx, proTx)) { - if(contains(proTx.proTxHash)) { + if (const auto opt_proTx = GetTxPayload(tx)) { + if(contains(opt_proTx->proTxHash)) { return true; } - if(CheckScript(proTx.scriptOperatorPayout)) { + if(CheckScript(opt_proTx->scriptOperatorPayout)) { if ((nFlags & BLOOM_UPDATE_MASK) == BLOOM_UPDATE_ALL) - insert(proTx.proTxHash); + insert(opt_proTx->proTxHash); return true; } } return false; } case(TRANSACTION_PROVIDER_UPDATE_REGISTRAR): { - CProUpRegTx proTx; - if (GetTxPayload(tx, proTx)) { - if(contains(proTx.proTxHash)) + if (const auto opt_proTx = GetTxPayload(tx)) { + if(contains(opt_proTx->proTxHash)) return true; - if(contains(proTx.keyIDVoting) || - CheckScript(proTx.scriptPayout)) { + if(contains(opt_proTx->keyIDVoting) || + CheckScript(opt_proTx->scriptPayout)) { if ((nFlags & BLOOM_UPDATE_MASK) == BLOOM_UPDATE_ALL) - insert(proTx.proTxHash); + insert(opt_proTx->proTxHash); return true; } } return false; } case(TRANSACTION_PROVIDER_UPDATE_REVOKE): { - CProUpRevTx proTx; - if (GetTxPayload(tx, proTx)) { - if(contains(proTx.proTxHash)) + if (const auto opt_proTx = GetTxPayload(tx)) { + if(contains(opt_proTx->proTxHash)) return true; } return false; } case(TRANSACTION_ASSET_LOCK): { // inputs of Asset Lock transactions are standard. But some outputs are special - CAssetLockPayload assetLockTx; - if (GetTxPayload(tx, assetLockTx)) { + if (const auto opt_assetlockTx = GetTxPayload(tx)) { bool fFound = false; - const auto& extraOuts = assetLockTx.getCreditOutputs(); + const auto& extraOuts = opt_assetlockTx->getCreditOutputs(); for (unsigned int i = 0; i < extraOuts.size(); ++i) { fFound = ProcessTxOut(extraOuts[i], tx.GetHash(), i) || fFound; diff --git a/src/core_write.cpp b/src/core_write.cpp index f78578246c..f5c42631f0 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -266,40 +266,40 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, } if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - if (CProRegTx proTx; GetTxPayload(tx, proTx)) { - entry.pushKV("proRegTx", proTx.ToJson()); + if (const auto opt_proTx = GetTxPayload(tx)) { + entry.pushKV("proRegTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - if (CProUpServTx proTx; GetTxPayload(tx, proTx)) { - entry.pushKV("proUpServTx", proTx.ToJson()); + if (const auto opt_proTx = GetTxPayload(tx)) { + entry.pushKV("proUpServTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - if (CProUpRegTx proTx; GetTxPayload(tx, proTx)) { - entry.pushKV("proUpRegTx", proTx.ToJson()); + if (const auto opt_proTx = GetTxPayload(tx)) { + entry.pushKV("proUpRegTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - if (CProUpRevTx proTx; GetTxPayload(tx, proTx)) { - entry.pushKV("proUpRevTx", proTx.ToJson()); + if (const auto opt_proTx = GetTxPayload(tx)) { + entry.pushKV("proUpRevTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_COINBASE) { - if (CCbTx cbTx; GetTxPayload(tx, cbTx)) { - entry.pushKV("cbTx", cbTx.ToJson()); + if (const auto opt_cbTx = GetTxPayload(tx)) { + entry.pushKV("cbTx", opt_cbTx->ToJson()); } } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { - if (llmq::CFinalCommitmentTxPayload qcTx; GetTxPayload(tx, qcTx)) { - entry.pushKV("qcTx", qcTx.ToJson()); + if (const auto opt_qcTx = GetTxPayload(tx)) { + entry.pushKV("qcTx", opt_qcTx->ToJson()); } } else if (tx.nType == TRANSACTION_MNHF_SIGNAL) { - if (MNHFTxPayload mnhfTx; GetTxPayload(tx, mnhfTx)) { - entry.pushKV("mnhfTx", mnhfTx.ToJson()); + if (const auto opt_mnhfTx = GetTxPayload(tx)) { + entry.pushKV("mnhfTx", opt_mnhfTx->ToJson()); } } else if (tx.nType == TRANSACTION_ASSET_LOCK) { - if (CAssetLockPayload assetLockTx; GetTxPayload(tx, assetLockTx)) { - entry.pushKV("assetLockTx", assetLockTx.ToJson()); + if (const auto opt_assetLockTx = GetTxPayload(tx)) { + entry.pushKV("assetLockTx", opt_assetLockTx->ToJson()); } } else if (tx.nType == TRANSACTION_ASSET_UNLOCK) { - if (CAssetUnlockPayload assetUnlockTx; GetTxPayload(tx, assetUnlockTx)) { - entry.pushKV("assetUnlockTx", assetUnlockTx.ToJson()); + if (const auto opt_assetUnlockTx = GetTxPayload(tx)) { + entry.pushKV("assetUnlockTx", opt_assetUnlockTx->ToJson()); } } diff --git a/src/evo/assetlocktx.cpp b/src/evo/assetlocktx.cpp index 2908432f66..d96c97e38e 100644 --- a/src/evo/assetlocktx.cpp +++ b/src/evo/assetlocktx.cpp @@ -60,21 +60,21 @@ bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state) if (returnAmount == 0) return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-no-return"); - CAssetLockPayload assetLockTx; - if (!GetTxPayload(tx, assetLockTx)) { + const auto opt_assetLockTx = GetTxPayload(tx); + if (!opt_assetLockTx.has_value()) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-payload"); } - if (assetLockTx.getVersion() == 0 || assetLockTx.getVersion() > CAssetLockPayload::CURRENT_VERSION) { + if (opt_assetLockTx->getVersion() == 0 || opt_assetLockTx->getVersion() > CAssetLockPayload::CURRENT_VERSION) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-version"); } - if (assetLockTx.getCreditOutputs().empty()) { + if (opt_assetLockTx->getCreditOutputs().empty()) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-emptycreditoutputs"); } CAmount creditOutputsAmount = 0; - for (const CTxOut& out : assetLockTx.getCreditOutputs()) { + for (const CTxOut& out : opt_assetLockTx->getCreditOutputs()) { if (out.nValue == 0 || !MoneyRange(out.nValue) || !MoneyRange(creditOutputsAmount + out.nValue)) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-credit-outofrange"); } @@ -158,10 +158,11 @@ bool CheckAssetUnlockTx(const CTransaction& tx, gsl::not_null(tx); + if (!opt_assetUnlockTx) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-payload"); } + auto& assetUnlockTx = *opt_assetUnlockTx; if (assetUnlockTx.getVersion() == 0 || assetUnlockTx.getVersion() > CAssetUnlockPayload::CURRENT_VERSION) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version"); @@ -187,11 +188,11 @@ bool CheckAssetUnlockTx(const CTransaction& tx, gsl::not_null(tx); + if (!opt_assetUnlockTx) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-payload"); } - const CAmount txfee_aux = assetUnlockTx.getFee(); + const CAmount txfee_aux = opt_assetUnlockTx->getFee(); if (txfee_aux == 0 || !MoneyRange(txfee_aux)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-assetunlock-fee-outofrange"); } diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 8f96d5f2b0..8f81e5f69f 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -29,10 +29,11 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-invalid"); } - CCbTx cbTx; - if (!GetTxPayload(tx, cbTx)) { + const auto opt_cbTx = GetTxPayload(tx); + if (!opt_cbTx) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload"); } + const auto& cbTx = *opt_cbTx; if (cbTx.nVersion == CCbTx::Version::INVALID || cbTx.nVersion >= CCbTx::Version::UNKNOWN) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); @@ -68,10 +69,11 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, const int64_t nTime1 = GetTimeMicros(); - CCbTx cbTx; - if (!GetTxPayload(*block.vtx[0], cbTx)) { + const auto opt_cbTx = GetTxPayload(*block.vtx[0]); + if (!opt_cbTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } + auto cbTx = *opt_cbTx; int64_t nTime2 = GetTimeMicros(); nTimePayload += nTime2 - nTime1; LogPrint(BCLog::BENCHMARK, " - GetTxPayload: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), nTimePayload * 0.000001); @@ -255,23 +257,23 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre const auto& tx = block.vtx[i]; if (tx->nVersion == 3 && tx->nType == TRANSACTION_QUORUM_COMMITMENT) { - llmq::CFinalCommitmentTxPayload qc; - if (!GetTxPayload(*tx, qc)) { + const auto opt_qc = GetTxPayload(*tx); + if (!opt_qc) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload-calc-cbtx-quorummerkleroot"); } - if (qc.commitment.IsNull()) { + if (opt_qc->commitment.IsNull()) { // having null commitments is ok but we don't use them here, move to the next tx continue; } - const auto& llmq_params_opt = llmq::GetLLMQParams(qc.commitment.llmqType); + const auto& llmq_params_opt = llmq::GetLLMQParams(opt_qc->commitment.llmqType); if (!llmq_params_opt.has_value()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type-calc-cbtx-quorummerkleroot"); } const auto& llmq_params = llmq_params_opt.value(); - auto qcHash = ::SerializeHash(qc.commitment); + auto qcHash = ::SerializeHash(opt_qc->commitment); if (llmq::utils::IsQuorumRotationEnabled(llmq_params, pindexPrev)) { - auto& map_indexed_hashes = qcIndexedHashes[qc.commitment.llmqType]; - map_indexed_hashes[qc.commitment.quorumIndex] = qcHash; + auto& map_indexed_hashes = qcIndexedHashes[opt_qc->commitment.llmqType]; + map_indexed_hashes[opt_qc->commitment.quorumIndex] = qcHash; } else { auto& vec_hashes = qcHashes[llmq_params.type]; if (vec_hashes.size() == size_t(llmq_params.signingActiveQuorumCount)) { @@ -328,17 +330,17 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons return true; } - CCbTx cbTx; - if (!GetTxPayload(*block.vtx[0], cbTx)) { + const auto opt_cbTx = GetTxPayload(*block.vtx[0]); + if (!opt_cbTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { + if (opt_cbTx->nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { return true; } auto best_clsig = chainlock_handler.GetBestChainLock(); - if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbTx.bestCLSignature == best_clsig.getSig()) { + if (best_clsig.getHeight() == pindex->nHeight - 1 && opt_cbTx->bestCLHeightDiff == 0 && opt_cbTx->bestCLSignature == best_clsig.getSig()) { // matches our best clsig which still hold values for the previous block return true; } @@ -347,29 +349,29 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. if (prevBlockCoinbaseChainlock.has_value()) { // Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one - if (!cbTx.bestCLSignature.IsValid()) { + if (!opt_cbTx->bestCLSignature.IsValid()) { // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); } int prevBlockCoinbaseCLHeight = pindex->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; - int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(opt_cbTx->bestCLHeightDiff) - 1; if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig"); } } // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null - if (cbTx.bestCLSignature.IsValid()) { - int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; - if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) { + if (opt_cbTx->bestCLSignature.IsValid()) { + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(opt_cbTx->bestCLHeightDiff) - 1; + if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == opt_cbTx->bestCLSignature) { // matches our best (but outdated) clsig, no need to verify it again return true; } uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature))) { + if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature))) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); } - } else if (cbTx.bestCLHeightDiff != 0) { + } else if (opt_cbTx->bestCLHeightDiff != 0) { // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); } @@ -450,12 +452,7 @@ std::optional GetCoinbaseTx(const CBlockIndex* pindex) } CTransactionRef cbTx = block.vtx[0]; - CCbTx cbTxPayload; - if (!GetTxPayload(*cbTx, cbTxPayload)) { - return std::nullopt; - } - - return cbTxPayload; + return GetTxPayload(*cbTx); } std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index d0fc398813..75ffc2cd79 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -32,13 +32,13 @@ std::unique_ptr creditPoolManager; static bool GetDataFromUnlockTx(const CTransaction& tx, CAmount& toUnlock, uint64_t& index, TxValidationState& state) { - CAssetUnlockPayload assetUnlockTx; - if (!GetTxPayload(tx, assetUnlockTx)) { + const auto opt_assetUnlockTx = GetTxPayload(tx); + if (!opt_assetUnlockTx.has_value()) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-payload"); } - index = assetUnlockTx.getIndex(); - toUnlock = assetUnlockTx.getFee(); + index = opt_assetUnlockTx->getIndex(); + toUnlock = opt_assetUnlockTx->getFee(); for (const CTxOut& txout : tx.vout) { if (!MoneyRange(txout.nValue)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-txout-outofrange"); @@ -143,14 +143,13 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo AddToCache(block_index->GetBlockHash(), block_index->nHeight, emptyPool); return emptyPool; } - CAmount locked{0}; - { - CCbTx cbTx; - if (!GetTxPayload(block->vtx[0]->vExtraPayload, cbTx)) { - throw std::runtime_error(strprintf("%s: failed-getcreditpool-cbtx-payload", __func__)); + CAmount locked = [&, func=__func__]() { + const auto opt_cbTx = GetTxPayload(block->vtx[0]->vExtraPayload); + if (!opt_cbTx) { + throw std::runtime_error(strprintf("%s: failed-getcreditpool-cbtx-payload", func)); } - locked = cbTx.creditPoolBalance; - } + return opt_cbTx->creditPoolBalance; + }(); // We use here sliding window with LimitBlocksToTrace to determine // current limits for asset unlock transactions. @@ -235,7 +234,7 @@ CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex *pindexP bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) { - if (CAssetLockPayload assetLockTx; !GetTxPayload(tx, assetLockTx)) { + if (const auto opt_assetLockTx = GetTxPayload(tx); !opt_assetLockTx) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-lock-payload"); } diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 184a0a888a..4c74975ed0 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -745,10 +745,11 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no } if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - CProRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } + auto& proTx = *opt_proTx; if (proTx.nType == MnType::Evo && !isV19Active) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); @@ -808,37 +809,37 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no __func__, tx.GetHash().ToString(), nHeight, proTx.ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - CProUpServTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - if (proTx.nType == MnType::Evo && !DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)) { + if (opt_proTx->nType == MnType::Evo && !DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - if (newList.HasUniqueProperty(proTx.addr) && newList.GetUniquePropertyMN(proTx.addr)->proTxHash != proTx.proTxHash) { + if (newList.HasUniqueProperty(opt_proTx->addr) && newList.GetUniquePropertyMN(opt_proTx->addr)->proTxHash != opt_proTx->proTxHash) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-addr"); } - CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); + CDeterministicMNCPtr dmn = newList.GetMN(opt_proTx->proTxHash); if (!dmn) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } - if (proTx.nType != dmn->nType) { + if (opt_proTx->nType != dmn->nType) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); } - if (!IsValidMnType(proTx.nType)) { + if (!IsValidMnType(opt_proTx->nType)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); } auto newState = std::make_shared(*dmn->pdmnState); - newState->addr = proTx.addr; - newState->scriptOperatorPayout = proTx.scriptOperatorPayout; - if (proTx.nType == MnType::Evo) { - newState->platformNodeID = proTx.platformNodeID; - newState->platformP2PPort = proTx.platformP2PPort; - newState->platformHTTPPort = proTx.platformHTTPPort; + newState->addr = opt_proTx->addr; + newState->scriptOperatorPayout = opt_proTx->scriptOperatorPayout; + if (opt_proTx->nType == MnType::Evo) { + newState->platformNodeID = opt_proTx->platformNodeID; + newState->platformP2PPort = opt_proTx->platformP2PPort; + newState->platformHTTPPort = opt_proTx->platformHTTPPort; } if (newState->IsBanned()) { // only revive when all keys are set @@ -846,84 +847,84 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no newState->Revive(nHeight); if (debugLogs) { LogPrintf("CDeterministicMNManager::%s -- MN %s revived at height %d\n", - __func__, proTx.proTxHash.ToString(), nHeight); + __func__, opt_proTx->proTxHash.ToString(), nHeight); } } } - newList.UpdateMN(proTx.proTxHash, newState); + newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, proTx.proTxHash.ToString(), nHeight, proTx.ToString()); + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - CProUpRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); + CDeterministicMNCPtr dmn = newList.GetMN(opt_proTx->proTxHash); if (!dmn) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } auto newState = std::make_shared(*dmn->pdmnState); - if (newState->pubKeyOperator != proTx.pubKeyOperator) { + if (newState->pubKeyOperator != opt_proTx->pubKeyOperator) { // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes newState->ResetOperatorFields(); newState->BanIfNotBanned(nHeight); // we update pubKeyOperator here, make sure state version matches - newState->nVersion = proTx.nVersion; - newState->pubKeyOperator = proTx.pubKeyOperator; + newState->nVersion = opt_proTx->nVersion; + newState->pubKeyOperator = opt_proTx->pubKeyOperator; } - newState->keyIDVoting = proTx.keyIDVoting; - newState->scriptPayout = proTx.scriptPayout; + newState->keyIDVoting = opt_proTx->keyIDVoting; + newState->scriptPayout = opt_proTx->scriptPayout; - newList.UpdateMN(proTx.proTxHash, newState); + newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, proTx.proTxHash.ToString(), nHeight, proTx.ToString()); + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - CProUpRevTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); + CDeterministicMNCPtr dmn = newList.GetMN(opt_proTx->proTxHash); if (!dmn) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } auto newState = std::make_shared(*dmn->pdmnState); newState->ResetOperatorFields(); newState->BanIfNotBanned(nHeight); - newState->nRevocationReason = proTx.nReason; + newState->nRevocationReason = opt_proTx->nReason; - newList.UpdateMN(proTx.proTxHash, newState); + newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { LogPrintf("CDeterministicMNManager::%s -- MN %s revoked operator key at height %d: %s\n", - __func__, proTx.proTxHash.ToString(), nHeight, proTx.ToString()); + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { - llmq::CFinalCommitmentTxPayload qc; - if (!GetTxPayload(tx, qc)) { + const auto opt_qc = GetTxPayload(tx); + if (!opt_qc) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); } - if (!qc.commitment.IsNull()) { - const auto& llmq_params_opt = llmq::GetLLMQParams(qc.commitment.llmqType); + if (!opt_qc->commitment.IsNull()) { + const auto& llmq_params_opt = llmq::GetLLMQParams(opt_qc->commitment.llmqType); if (!llmq_params_opt.has_value()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); } - int qcnHeight = int(qc.nHeight); - int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + int(qc.commitment.quorumIndex); + int qcnHeight = int(opt_qc->nHeight); + int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + int(opt_qc->commitment.quorumIndex); auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); - if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != qc.commitment.quorumHash) { + if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { // we should actually never get into this case as validation should have caught it...but let's be sure return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-quorum-hash"); } - HandleQuorumCommitment(qc.commitment, pQuorumBaseBlockIndex, newList, debugLogs); + HandleQuorumCommitment(opt_qc->commitment, pQuorumBaseBlockIndex, newList, debugLogs); } } } @@ -1098,10 +1099,11 @@ bool CDeterministicMNManager::IsProTxWithCollateral(const CTransactionRef& tx, u if (tx->nVersion != 3 || tx->nType != TRANSACTION_PROVIDER_REGISTER) { return false; } - CProRegTx proTx; - if (!GetTxPayload(*tx, proTx)) { + const auto opt_proTx = GetTxPayload(*tx); + if (!opt_proTx) { return false; } + auto& proTx = *opt_proTx; if (!proTx.collateralOutpoint.hash.IsNull()) { return false; @@ -1506,17 +1508,17 @@ static std::optional GetValidatedPayload(const CTransaction& tx, gsl::not return std::nullopt; } - ProTx ptx; - if (!GetTxPayload(tx, ptx)) { + auto opt_ptx = GetTxPayload(tx); + if (!opt_ptx) { state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-payload"); return std::nullopt; } const bool is_basic_scheme_active{DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; - if (!ptx.IsTriviallyValid(is_basic_scheme_active, state)) { + if (!opt_ptx->IsTriviallyValid(is_basic_scheme_active, state)) { // pass the state returned by the function above return std::nullopt; } - return ptx; + return opt_ptx; } bool CheckProRegTx(const CTransaction& tx, gsl::not_null pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs) @@ -1526,17 +1528,16 @@ bool CheckProRegTx(const CTransaction& tx, gsl::not_null pin // pass the state returned by the function above return false; } - const auto& ptx{*opt_ptx}; // It's allowed to set addr to 0, which will put the MN into PoSe-banned state and require a ProUpServTx to be issues later // If any of both is set, it must be valid however - if (ptx.addr != CService() && !CheckService(ptx, state)) { + if (opt_ptx->addr != CService() && !CheckService(*opt_ptx, state)) { // pass the state returned by the function above return false; } - if (ptx.nType == MnType::Evo) { - if (!CheckPlatformFields(ptx, state)) { + if (opt_ptx->nType == MnType::Evo) { + if (!CheckPlatformFields(*opt_ptx, state)) { return false; } } @@ -1545,11 +1546,11 @@ bool CheckProRegTx(const CTransaction& tx, gsl::not_null pin const PKHash *keyForPayloadSig = nullptr; COutPoint collateralOutpoint; - CAmount expectedCollateral = GetMnType(ptx.nType).collat_amount; + CAmount expectedCollateral = GetMnType(opt_ptx->nType).collat_amount; - if (!ptx.collateralOutpoint.hash.IsNull()) { + if (!opt_ptx->collateralOutpoint.hash.IsNull()) { Coin coin; - if (!view.GetCoin(ptx.collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != expectedCollateral) { + if (!view.GetCoin(opt_ptx->collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != expectedCollateral) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral"); } @@ -1564,25 +1565,25 @@ bool CheckProRegTx(const CTransaction& tx, gsl::not_null pin return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-pkh"); } - collateralOutpoint = ptx.collateralOutpoint; + collateralOutpoint = opt_ptx->collateralOutpoint; } else { - if (ptx.collateralOutpoint.n >= tx.vout.size()) { + if (opt_ptx->collateralOutpoint.n >= tx.vout.size()) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-index"); } - if (tx.vout[ptx.collateralOutpoint.n].nValue != expectedCollateral) { + if (tx.vout[opt_ptx->collateralOutpoint.n].nValue != expectedCollateral) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral"); } - if (!ExtractDestination(tx.vout[ptx.collateralOutpoint.n].scriptPubKey, collateralTxDest)) { + if (!ExtractDestination(tx.vout[opt_ptx->collateralOutpoint.n].scriptPubKey, collateralTxDest)) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-dest"); } - collateralOutpoint = COutPoint(tx.GetHash(), ptx.collateralOutpoint.n); + collateralOutpoint = COutPoint(tx.GetHash(), opt_ptx->collateralOutpoint.n); } // don't allow reuse of collateral key for other keys (don't allow people to put the collateral key onto an online server) // this check applies to internal and external collateral, but internal collaterals are not necessarily a P2PKH - if (collateralTxDest == CTxDestination(PKHash(ptx.keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(ptx.keyIDVoting))) { + if (collateralTxDest == CTxDestination(PKHash(opt_ptx->keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(opt_ptx->keyIDVoting))) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse"); } @@ -1590,43 +1591,43 @@ bool CheckProRegTx(const CTransaction& tx, gsl::not_null pin auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); // only allow reusing of addresses when it's for the same collateral (which replaces the old MN) - if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->collateralOutpoint != collateralOutpoint) { + if (mnList.HasUniqueProperty(opt_ptx->addr) && mnList.GetUniquePropertyMN(opt_ptx->addr)->collateralOutpoint != collateralOutpoint) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr"); } // never allow duplicate keys, even if this ProTx would replace an existing MN - if (mnList.HasUniqueProperty(ptx.keyIDOwner) || mnList.HasUniqueProperty(ptx.pubKeyOperator)) { + if (mnList.HasUniqueProperty(opt_ptx->keyIDOwner) || mnList.HasUniqueProperty(opt_ptx->pubKeyOperator)) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-key"); } // never allow duplicate platformNodeIds for EvoNodes - if (ptx.nType == MnType::Evo) { - if (mnList.HasUniqueProperty(ptx.platformNodeID)) { + if (opt_ptx->nType == MnType::Evo) { + if (mnList.HasUniqueProperty(opt_ptx->platformNodeID)) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-platformnodeid"); } } if (!DeploymentDIP0003Enforced(pindexPrev->nHeight, Params().GetConsensus())) { - if (ptx.keyIDOwner != ptx.keyIDVoting) { + if (opt_ptx->keyIDOwner != opt_ptx->keyIDVoting) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-not-same"); } } } - if (!CheckInputsHash(tx, ptx, state)) { + if (!CheckInputsHash(tx, *opt_ptx, state)) { // pass the state returned by the function above return false; } if (keyForPayloadSig) { // collateral is not part of this ProRegTx, so we must verify ownership of the collateral - if (check_sigs && !CheckStringSig(ptx, *keyForPayloadSig, state)) { + if (check_sigs && !CheckStringSig(*opt_ptx, *keyForPayloadSig, state)) { // pass the state returned by the function above return false; } } else { // collateral is part of this ProRegTx, so we know the collateral is owned by the issuer - if (!ptx.vchSig.empty()) { + if (!opt_ptx->vchSig.empty()) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-sig"); } } @@ -1641,53 +1642,52 @@ bool CheckProUpServTx(const CTransaction& tx, gsl::not_null // pass the state returned by the function above return false; } - const auto& ptx{*opt_ptx}; - if (!CheckService(ptx, state)) { + if (!CheckService(*opt_ptx, state)) { // pass the state returned by the function above return false; } - if (ptx.nType == MnType::Evo) { - if (!CheckPlatformFields(ptx, state)) { + if (opt_ptx->nType == MnType::Evo) { + if (!CheckPlatformFields(*opt_ptx, state)) { return false; } } auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); - auto mn = mnList.GetMN(ptx.proTxHash); + auto mn = mnList.GetMN(opt_ptx->proTxHash); if (!mn) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } // don't allow updating to addresses already used by other MNs - if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->proTxHash != ptx.proTxHash) { + if (mnList.HasUniqueProperty(opt_ptx->addr) && mnList.GetUniquePropertyMN(opt_ptx->addr)->proTxHash != opt_ptx->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr"); } // don't allow updating to platformNodeIds already used by other EvoNodes - if (ptx.nType == MnType::Evo) { - if (mnList.HasUniqueProperty(ptx.platformNodeID) && mnList.GetUniquePropertyMN(ptx.platformNodeID)->proTxHash != ptx.proTxHash) { + if (opt_ptx->nType == MnType::Evo) { + if (mnList.HasUniqueProperty(opt_ptx->platformNodeID) && mnList.GetUniquePropertyMN(opt_ptx->platformNodeID)->proTxHash != opt_ptx->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-platformnodeid"); } } - if (ptx.scriptOperatorPayout != CScript()) { + if (opt_ptx->scriptOperatorPayout != CScript()) { if (mn->nOperatorReward == 0) { // don't allow setting operator reward payee in case no operatorReward was set return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee"); } - if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash()) { + if (!opt_ptx->scriptOperatorPayout.IsPayToPublicKeyHash() && !opt_ptx->scriptOperatorPayout.IsPayToScriptHash()) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee"); } } // we can only check the signature if pindexPrev != nullptr and the MN is known - if (!CheckInputsHash(tx, ptx, state)) { + if (!CheckInputsHash(tx, *opt_ptx, state)) { // pass the state returned by the function above return false; } - if (check_sigs && !CheckHashSig(ptx, mn->pdmnState->pubKeyOperator.Get(), state)) { + if (check_sigs && !CheckHashSig(*opt_ptx, mn->pdmnState->pubKeyOperator.Get(), state)) { // pass the state returned by the function above return false; } @@ -1702,22 +1702,21 @@ bool CheckProUpRegTx(const CTransaction& tx, gsl::not_null p // pass the state returned by the function above return false; } - const auto& ptx{*opt_ptx}; CTxDestination payoutDest; - if (!ExtractDestination(ptx.scriptPayout, payoutDest)) { + if (!ExtractDestination(opt_ptx->scriptPayout, payoutDest)) { // should not happen as we checked script types before return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-dest"); } auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); - auto dmn = mnList.GetMN(ptx.proTxHash); + auto dmn = mnList.GetMN(opt_ptx->proTxHash); if (!dmn) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } // don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server) - if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(ptx.keyIDVoting))) { + if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(opt_ptx->keyIDVoting))) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse"); } @@ -1732,28 +1731,28 @@ bool CheckProUpRegTx(const CTransaction& tx, gsl::not_null p if (!ExtractDestination(coin.out.scriptPubKey, collateralTxDest)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-collateral-dest"); } - if (collateralTxDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(ptx.keyIDVoting))) { + if (collateralTxDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || collateralTxDest == CTxDestination(PKHash(opt_ptx->keyIDVoting))) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse"); } - if (mnList.HasUniqueProperty(ptx.pubKeyOperator)) { - auto otherDmn = mnList.GetUniquePropertyMN(ptx.pubKeyOperator); - if (ptx.proTxHash != otherDmn->proTxHash) { + if (mnList.HasUniqueProperty(opt_ptx->pubKeyOperator)) { + auto otherDmn = mnList.GetUniquePropertyMN(opt_ptx->pubKeyOperator); + if (opt_ptx->proTxHash != otherDmn->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-key"); } } if (!DeploymentDIP0003Enforced(pindexPrev->nHeight, Params().GetConsensus())) { - if (dmn->pdmnState->keyIDOwner != ptx.keyIDVoting) { + if (dmn->pdmnState->keyIDOwner != opt_ptx->keyIDVoting) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-not-same"); } } - if (!CheckInputsHash(tx, ptx, state)) { + if (!CheckInputsHash(tx, *opt_ptx, state)) { // pass the state returned by the function above return false; } - if (check_sigs && !CheckHashSig(ptx, PKHash(dmn->pdmnState->keyIDOwner), state)) { + if (check_sigs && !CheckHashSig(*opt_ptx, PKHash(dmn->pdmnState->keyIDOwner), state)) { // pass the state returned by the function above return false; } @@ -1768,18 +1767,17 @@ bool CheckProUpRevTx(const CTransaction& tx, gsl::not_null p // pass the state returned by the function above return false; } - const auto& ptx{*opt_ptx}; auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); - auto dmn = mnList.GetMN(ptx.proTxHash); + auto dmn = mnList.GetMN(opt_ptx->proTxHash); if (!dmn) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); - if (!CheckInputsHash(tx, ptx, state)) { + if (!CheckInputsHash(tx, *opt_ptx, state)) { // pass the state returned by the function above return false; } - if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) { + if (check_sigs && !CheckHashSig(*opt_ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) { // pass the state returned by the function above return false; } diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 0ecaf5cff0..9c44e2a0d9 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -107,11 +107,11 @@ bool CheckMNHFTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValida return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-type"); } - MNHFTxPayload mnhfTx; - if (!GetTxPayload(tx, mnhfTx)) { + const auto opt_mnhfTx = GetTxPayload(tx); + if (!opt_mnhfTx) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-payload"); } - + auto& mnhfTx = *opt_mnhfTx; if (mnhfTx.nVersion == 0 || mnhfTx.nVersion > MNHFTxPayload::CURRENT_VERSION) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-version"); } @@ -153,11 +153,11 @@ std::optional extractEHFSignal(const CTransaction& tx) return std::nullopt; } - MNHFTxPayload mnhfTx; - if (!GetTxPayload(tx, mnhfTx)) { + const auto opt_mnhfTx = GetTxPayload(tx); + if (!opt_mnhfTx) { return std::nullopt; } - return mnhfTx.signal.versionBit; + return opt_mnhfTx->signal.versionBit; } static bool extractSignals(const CBlock& block, const CBlockIndex* const pindex, std::vector& new_signals, BlockValidationState& state) @@ -176,11 +176,11 @@ static bool extractSignals(const CBlock& block, const CBlockIndex* const pindex, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); } - MNHFTxPayload mnhfTx; - if (!GetTxPayload(tx, mnhfTx)) { + const auto opt_mnhfTx = GetTxPayload(tx); + if (!opt_mnhfTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-tx-payload"); } - const uint8_t bit = mnhfTx.signal.versionBit; + const uint8_t bit = opt_mnhfTx->signal.versionBit; if (std::find(new_signals.begin(), new_signals.end(), bit) != new_signals.end()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-duplicates-in-block"); } diff --git a/src/evo/simplifiedmns.cpp b/src/evo/simplifiedmns.cpp index 9ced1bb0d2..4d8c1b3852 100644 --- a/src/evo/simplifiedmns.cpp +++ b/src/evo/simplifiedmns.cpp @@ -266,11 +266,10 @@ UniValue CSimplifiedMNListDiff::ToJson(bool extended) const } obj.pushKV("newQuorums", newQuorumsArr); - CCbTx cbTxPayload; - if (GetTxPayload(*cbTx, cbTxPayload)) { - obj.pushKV("merkleRootMNList", cbTxPayload.merkleRootMNList.ToString()); - if (cbTxPayload.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) { - obj.pushKV("merkleRootQuorums", cbTxPayload.merkleRootQuorums.ToString()); + if (const auto opt_cbTxPayload = GetTxPayload(*cbTx)) { + obj.pushKV("merkleRootMNList", opt_cbTxPayload->merkleRootMNList.ToString()); + if (opt_cbTxPayload->nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) { + obj.pushKV("merkleRootQuorums", opt_cbTxPayload->merkleRootQuorums.ToString()); } } diff --git a/src/evo/specialtx.h b/src/evo/specialtx.h index 8123e65366..ab407aafd4 100644 --- a/src/evo/specialtx.h +++ b/src/evo/specialtx.h @@ -12,30 +12,27 @@ #include #include +#include #include template -inline bool GetTxPayload(const std::vector& payload, T& obj) +std::optional GetTxPayload(const std::vector& payload) { CDataStream ds(payload, SER_NETWORK, PROTOCOL_VERSION); try { + T obj; ds >> obj; + return ds.empty() ? std::make_optional(std::move(obj)) : std::nullopt; } catch (const std::exception& e) { - return false; + return std::nullopt; } - return ds.empty(); } -template -inline bool GetTxPayload(const CMutableTransaction& tx, T& obj, bool assert_type = true) +template +std::optional GetTxPayload(const TxType& tx, bool assert_type = true) { - if (assert_type) { ASSERT_IF_DEBUG(tx.nType == obj.SPECIALTX_TYPE); } - return tx.nType == obj.SPECIALTX_TYPE && GetTxPayload(tx.vExtraPayload, obj); -} -template -inline bool GetTxPayload(const CTransaction& tx, T& obj, bool assert_type = true) -{ - if (assert_type) { ASSERT_IF_DEBUG(tx.nType == obj.SPECIALTX_TYPE); } - return tx.nType == obj.SPECIALTX_TYPE && GetTxPayload(tx.vExtraPayload, obj); + if (assert_type) { ASSERT_IF_DEBUG(tx.nType == T::SPECIALTX_TYPE); } + if (tx.nType != T::SPECIALTX_TYPE) return std::nullopt; + return GetTxPayload(tx.vExtraPayload); } template diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 2e1f694f7e..d626544135 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -288,11 +288,11 @@ bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, assert(tx.nVersion == 3); assert(tx.nType == TRANSACTION_COINBASE); - CCbTx cbTx; - if (!GetTxPayload(tx, cbTx)) { + const auto opt_cbTx = GetTxPayload(tx); + if (!opt_cbTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } - CAmount target_balance{cbTx.creditPoolBalance}; + CAmount target_balance{opt_cbTx->creditPoolBalance}; // But it maybe not included yet in previous block yet; in this case value must be 0 CAmount locked_calculated{creditPoolDiff->GetTotalLocked()}; if (target_balance != locked_calculated) { diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 8aaba1038e..d6e123fd35 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -359,12 +359,13 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, gsl::no for (const auto& tx : block.vtx) { if (tx->nType == TRANSACTION_QUORUM_COMMITMENT) { - CFinalCommitmentTxPayload qc; - if (!GetTxPayload(*tx, qc)) { + const auto opt_qc = GetTxPayload(*tx); + if (!opt_qc) { // should not happen as it was verified before processing the block LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d GetTxPayload fails\n", __func__, pindex->nHeight); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); } + auto& qc = *opt_qc; const auto& llmq_params_opt = GetLLMQParams(qc.commitment.llmqType); if (!llmq_params_opt.has_value()) { diff --git a/src/llmq/commitment.cpp b/src/llmq/commitment.cpp index f9a90a0532..2184f6ac58 100644 --- a/src/llmq/commitment.cpp +++ b/src/llmq/commitment.cpp @@ -174,11 +174,12 @@ bool CFinalCommitment::VerifySizes(const Consensus::LLMQParams& params) const bool CheckLLMQCommitment(const CTransaction& tx, gsl::not_null pindexPrev, TxValidationState& state) { - CFinalCommitmentTxPayload qcTx; - if (!GetTxPayload(tx, qcTx)) { + const auto opt_qcTx = GetTxPayload(tx); + if (!opt_qcTx) { LogPrintfFinalCommitment("h[%d] GetTxPayload failed\n", pindexPrev->nHeight); return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-qc-payload"); } + auto& qcTx = *opt_qcTx; const auto& llmq_params_opt = GetLLMQParams(qcTx.commitment.llmqType); if (!llmq_params_opt.has_value()) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 10ec0f3045..f55958cf4f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -243,9 +243,8 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn } result.pushKV("tx", txs); if (!block.vtx[0]->vExtraPayload.empty()) { - CCbTx cbTx; - if (GetTxPayload(block.vtx[0]->vExtraPayload, cbTx)) { - result.pushKV("cbTx", cbTx.ToJson()); + if (const auto opt_cbTx = GetTxPayload(block.vtx[0]->vExtraPayload)) { + result.pushKV("cbTx", opt_cbTx->ToJson()); } } result.pushKV("time", block.GetBlockTime()); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 87c59e5c02..3e95c441db 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -833,10 +833,12 @@ static UniValue protx_register_submit(const JSONRPCRequest& request, const Chain if (tx.nType != TRANSACTION_PROVIDER_REGISTER) { throw JSONRPCError(RPC_INVALID_PARAMETER, "transaction not a ProRegTx"); } - CProRegTx ptx; - if (!GetTxPayload(tx, ptx)) { + auto ptx = [&tx]() { + if (const auto opt_ptx = GetTxPayload(tx); opt_ptx.has_value()) { + return *opt_ptx; + } throw JSONRPCError(RPC_INVALID_PARAMETER, "transaction payload not deserializable"); - } + }(); if (!ptx.vchSig.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "payload signature not empty"); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index cf1f755e99..72586fcb7d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -444,8 +444,8 @@ static UniValue getassetunlockstatuses(const JSONRPCRequest& request) LOCK(mempool.cs); return std::any_of(mempool.mapTx.begin(), mempool.mapTx.end(), [index](const CTxMemPoolEntry &e) { if (e.GetTx().nType == CAssetUnlockPayload::SPECIALTX_TYPE) { - if (CAssetUnlockPayload assetUnlockTx; GetTxPayload(e.GetTx(), assetUnlockTx)) { - return index == assetUnlockTx.getIndex(); + if (auto opt_assetUnlockTx = GetTxPayload(e.GetTx())) { + return index == opt_assetUnlockTx->getIndex(); } else { throw JSONRPCError(RPC_TRANSACTION_ERROR, "bad-assetunlocktx-payload"); } diff --git a/src/test/evo_assetlocks_tests.cpp b/src/test/evo_assetlocks_tests.cpp index e8fba5e00e..855d16ed0a 100644 --- a/src/test/evo_assetlocks_tests.cpp +++ b/src/test/evo_assetlocks_tests.cpp @@ -151,10 +151,10 @@ BOOST_FIXTURE_TEST_CASE(evo_assetlock, TestChain100Setup) { BOOST_CHECK(tx.nVersion == 3); - CAssetLockPayload lockPayload; - GetTxPayload(tx, lockPayload); + const auto opt_payload = GetTxPayload(tx); - BOOST_CHECK(lockPayload.getVersion() == 1); + BOOST_CHECK(opt_payload.has_value()); + BOOST_CHECK(opt_payload->getVersion() == 1); } { @@ -186,12 +186,8 @@ BOOST_FIXTURE_TEST_CASE(evo_assetlock, TestChain100Setup) BOOST_CHECK(CheckAssetLockTx(CTransaction(txSmallOutput), tx_state)); } - const CAssetLockPayload assetLockPayload = [tx]() -> CAssetLockPayload { - CAssetLockPayload payload; - GetTxPayload(tx, payload); - return payload; - }(); - const std::vector creditOutputs = assetLockPayload.getCreditOutputs(); + const auto assetLockPayload = GetTxPayload(tx); + const std::vector creditOutputs = assetLockPayload->getCreditOutputs(); { // Sum of credit output greater than OP_RETURN @@ -340,12 +336,12 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup) } { - CAssetUnlockPayload unlockPayload; - GetTxPayload(tx, unlockPayload); - BOOST_CHECK(unlockPayload.getVersion() == 1); - BOOST_CHECK(unlockPayload.getRequestedHeight() == 1000'000); - BOOST_CHECK(unlockPayload.getFee() == 2000'000'000u); - BOOST_CHECK(unlockPayload.getIndex() == 0x001122334455667788L); + const auto unlockPayload = GetTxPayload(tx); + BOOST_CHECK(unlockPayload.has_value()); + BOOST_CHECK(unlockPayload->getVersion() == 1); + BOOST_CHECK(unlockPayload->getRequestedHeight() == 1000'000); + BOOST_CHECK(unlockPayload->getFee() == 2000'000'000u); + BOOST_CHECK(unlockPayload->getIndex() == 0x001122334455667788L); // Wrong type "Asset Lock TX" instead "Asset Unlock TX" CMutableTransaction txWrongType = tx; @@ -357,11 +353,11 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup) BOOST_CHECK(tx.nVersion == 3); for (uint8_t payload_version : {0, 1, 2, 255}) { CAssetUnlockPayload unlockPayload_tmp{payload_version, - unlockPayload.getIndex(), - unlockPayload.getFee(), - unlockPayload.getRequestedHeight(), - unlockPayload.getQuorumHash(), - unlockPayload.getQuorumSig()}; + unlockPayload->getIndex(), + unlockPayload->getFee(), + unlockPayload->getRequestedHeight(), + unlockPayload->getQuorumHash(), + unlockPayload->getQuorumSig()}; CMutableTransaction txWrongVersion = tx; SetTxPayload(txWrongVersion, unlockPayload_tmp); if (payload_version != 1) { diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index fb7adcaff8..9c8fc9dc88 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -181,15 +181,16 @@ static CMutableTransaction CreateProUpRevTx(const CTxMemPool& mempool, SimpleUTX template static CMutableTransaction MalleateProTxPayout(const CMutableTransaction& tx) { - ProTx proTx; - GetTxPayload(tx, proTx); + auto opt_protx = GetTxPayload(tx); + BOOST_ASSERT(opt_protx.has_value()); + auto& protx = *opt_protx; CKey key; key.MakeNewKey(false); - proTx.scriptPayout = GetScriptForDestination(PKHash(key.GetPubKey())); + protx.scriptPayout = GetScriptForDestination(PKHash(key.GetPubKey())); CMutableTransaction tx2 = tx; - SetTxPayload(tx2, proTx); + SetTxPayload(tx2, protx); return tx2; } diff --git a/src/test/evo_mnhf_tests.cpp b/src/test/evo_mnhf_tests.cpp index 4dbcfa04cc..031f207814 100644 --- a/src/test/evo_mnhf_tests.cpp +++ b/src/test/evo_mnhf_tests.cpp @@ -20,12 +20,10 @@ bool VerifyMNHFTx(const CTransaction& tx, TxValidationState& state) { - MNHFTxPayload mnhfTx; - if (!GetTxPayload(tx, mnhfTx)) { + if (const auto opt_mnhfTx_payload = GetTxPayload(tx); !opt_mnhfTx_payload) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-payload"); - } - - if (mnhfTx.nVersion == 0 || mnhfTx.nVersion > MNHFTxPayload::CURRENT_VERSION) { + } else if (opt_mnhfTx_payload->nVersion == 0 || + opt_mnhfTx_payload->nVersion > MNHFTxPayload::CURRENT_VERSION) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-version"); } diff --git a/src/test/evo_trivialvalidation.cpp b/src/test/evo_trivialvalidation.cpp index 1adaa99654..7cb2bde84e 100644 --- a/src/test/evo_trivialvalidation.cpp +++ b/src/test/evo_trivialvalidation.cpp @@ -20,16 +20,15 @@ BOOST_FIXTURE_TEST_SUITE(evo_trivialvalidation, BasicTestingSetup) template void TestTxHelper(const CMutableTransaction& tx, bool is_basic_bls, bool expected_failure, const std::string& expected_error) { - T payload; - const bool payload_to_fail = expected_failure && expected_error == "gettxpayload-fail"; - BOOST_CHECK_EQUAL(GetTxPayload(tx, payload, false), !payload_to_fail); + const auto opt_payload = GetTxPayload(tx, false); + BOOST_CHECK_EQUAL(opt_payload.has_value(), !payload_to_fail); // No need to check anything else if GetTxPayload() expected to fail if (payload_to_fail) return; TxValidationState dummy_state; - BOOST_CHECK_EQUAL(payload.IsTriviallyValid(is_basic_bls, dummy_state), !expected_failure); + BOOST_CHECK_EQUAL(opt_payload->IsTriviallyValid(is_basic_bls, dummy_state), !expected_failure); if (expected_failure) { BOOST_CHECK_EQUAL(dummy_state.GetRejectReason(), expected_error); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index fdda05357a..fdd726cbd6 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -405,19 +405,17 @@ CBlock TestChainSetup::CreateBlock(const std::vector& txns, // Manually update CbTx as we modified the block here if (block.vtx[0]->nType == TRANSACTION_COINBASE) { LOCK(cs_main); - CCbTx cbTx; - if (!GetTxPayload(*block.vtx[0], cbTx)) { - BOOST_ASSERT(false); - } + auto cbTx = GetTxPayload(*block.vtx[0]); + BOOST_ASSERT(cbTx.has_value()); BlockValidationState state; - if (!CalcCbTxMerkleRootMNList(block, ::ChainActive().Tip(), cbTx.merkleRootMNList, state, ::ChainstateActive().CoinsTip())) { + if (!CalcCbTxMerkleRootMNList(block, ::ChainActive().Tip(), cbTx->merkleRootMNList, state, ::ChainstateActive().CoinsTip())) { BOOST_ASSERT(false); } - if (!CalcCbTxMerkleRootQuorums(block, ::ChainActive().Tip(), *m_node.llmq_ctx->quorum_block_processor, cbTx.merkleRootQuorums, state)) { + if (!CalcCbTxMerkleRootQuorums(block, ::ChainActive().Tip(), *m_node.llmq_ctx->quorum_block_processor, cbTx->merkleRootQuorums, state)) { BOOST_ASSERT(false); } CMutableTransaction tmpTx{*block.vtx[0]}; - SetTxPayload(tmpTx, cbTx); + SetTxPayload(tmpTx, *cbTx); block.vtx[0] = MakeTransactionRef(tmpTx); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4c030fb623..6ceda55144 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -408,9 +409,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces // fully checked by AcceptToMemoryPool() at this point, so we just assume that // everything is fine here. if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - CProRegTx proTx; - bool ok = GetTxPayload(tx, proTx); - assert(ok); + auto proTx = *Assert(GetTxPayload(tx)); if (!proTx.collateralOutpoint.hash.IsNull()) { mapProTxRefs.emplace(tx.GetHash(), proTx.collateralOutpoint.hash); } @@ -423,27 +422,20 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces mapProTxCollaterals.emplace(COutPoint(tx.GetHash(), proTx.collateralOutpoint.n), tx.GetHash()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - CProUpServTx proTx; - bool ok = GetTxPayload(tx, proTx); - assert(ok); + auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash()); mapProTxAddresses.emplace(proTx.addr, tx.GetHash()); } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - CProUpRegTx proTx; - bool ok = GetTxPayload(tx, proTx); - assert(ok); + auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash()); mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash()); - auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); - assert(dmn); + auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash)); newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator); if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) { newit->isKeyChangeProTx = true; } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - CProUpRevTx proTx; - bool ok = GetTxPayload(tx, proTx); - assert(ok); + auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash()); auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); assert(dmn); @@ -452,9 +444,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces newit->isKeyChangeProTx = true; } } else if (tx.nType == TRANSACTION_ASSET_UNLOCK) { - CAssetUnlockPayload assetUnlockTx; - bool ok = GetTxPayload(tx, assetUnlockTx); - assert(ok); + auto assetUnlockTx = *Assert(GetTxPayload(tx)); mapAssetUnlockExpiry.insert({tx.GetHash(), assetUnlockTx.getHeightToExpiry()}); } else if (tx.nType == TRANSACTION_MNHF_SIGNAL) { PrioritiseTransaction(tx.GetHash(), 0.1 * COIN); @@ -630,10 +620,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) }; if (it->GetTx().nType == TRANSACTION_PROVIDER_REGISTER) { - CProRegTx proTx; - if (!GetTxPayload(it->GetTx(), proTx)) { - assert(false); - } + auto proTx = *Assert(GetTxPayload(it->GetTx())); if (!proTx.collateralOutpoint.IsNull()) { eraseProTxRef(it->GetTx().GetHash(), proTx.collateralOutpoint.hash); } @@ -643,24 +630,15 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) mapProTxCollaterals.erase(proTx.collateralOutpoint); mapProTxCollaterals.erase(COutPoint(it->GetTx().GetHash(), proTx.collateralOutpoint.n)); } else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - CProUpServTx proTx; - if (!GetTxPayload(it->GetTx(), proTx)) { - assert(false); - } + auto proTx = *Assert(GetTxPayload(it->GetTx())); eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash()); mapProTxAddresses.erase(proTx.addr); } else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - CProUpRegTx proTx; - if (!GetTxPayload(it->GetTx(), proTx)) { - assert(false); - } + auto proTx = *Assert(GetTxPayload(it->GetTx())); eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash()); mapProTxBlsPubKeyHashes.erase(proTx.pubKeyOperator.GetHash()); } else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - CProUpRevTx proTx; - if (!GetTxPayload(it->GetTx(), proTx)) { - assert(false); - } + auto proTx = *Assert(GetTxPayload(it->GetTx())); eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash()); } else if (it->GetTx().nType == TRANSACTION_ASSET_UNLOCK) { mapAssetUnlockExpiry.erase(it->GetTx().GetHash()); @@ -899,11 +877,12 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) removeProTxSpentCollateralConflicts(tx); if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - CProRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return; } + auto& proTx = *opt_proTx; if (mapProTxAddresses.count(proTx.addr)) { uint256 conflictHash = mapProTxAddresses[proTx.addr]; @@ -919,35 +898,35 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) removeProTxCollateralConflicts(tx, COutPoint(tx.GetHash(), proTx.collateralOutpoint.n)); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - CProUpServTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return; } - if (mapProTxAddresses.count(proTx.addr)) { - uint256 conflictHash = mapProTxAddresses[proTx.addr]; + if (mapProTxAddresses.count(opt_proTx->addr)) { + uint256 conflictHash = mapProTxAddresses[opt_proTx->addr]; if (conflictHash != tx.GetHash() && mapTx.count(conflictHash)) { removeRecursive(mapTx.find(conflictHash)->GetTx(), MemPoolRemovalReason::CONFLICT); } } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - CProUpRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return; } - removeProTxPubKeyConflicts(tx, proTx.pubKeyOperator); - removeProTxKeyChangedConflicts(tx, proTx.proTxHash, ::SerializeHash(proTx.pubKeyOperator)); + removeProTxPubKeyConflicts(tx, opt_proTx->pubKeyOperator); + removeProTxKeyChangedConflicts(tx, opt_proTx->proTxHash, ::SerializeHash(opt_proTx->pubKeyOperator)); } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - CProUpRevTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return; } - removeProTxKeyChangedConflicts(tx, proTx.proTxHash, ::SerializeHash(CBLSPublicKey())); + removeProTxKeyChangedConflicts(tx, opt_proTx->proTxHash, ::SerializeHash(CBLSPublicKey())); } } @@ -1263,11 +1242,12 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { }; if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - CProRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return true; // i.e. can't decode payload == conflict } + auto& proTx = *opt_proTx; if (mapProTxAddresses.count(proTx.addr) || mapProTxPubKeyIDs.count(proTx.keyIDOwner) || mapProTxBlsPubKeyHashes.count(proTx.pubKeyOperator.GetHash())) return true; if (!proTx.collateralOutpoint.hash.IsNull()) { @@ -1282,19 +1262,20 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { } return false; } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - CProUpServTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return true; // i.e. can't decode payload == conflict } - auto it = mapProTxAddresses.find(proTx.addr); - return it != mapProTxAddresses.end() && it->second != proTx.proTxHash; + auto it = mapProTxAddresses.find(opt_proTx->addr); + return it != mapProTxAddresses.end() && it->second != opt_proTx->proTxHash; } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - CProUpRegTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return true; // i.e. can't decode payload == conflict } + auto& proTx = *opt_proTx; // this method should only be called with validated ProTxs auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); @@ -1312,12 +1293,12 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { auto it = mapProTxBlsPubKeyHashes.find(proTx.pubKeyOperator.GetHash()); return it != mapProTxBlsPubKeyHashes.end() && it->second != proTx.proTxHash; } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - CProUpRevTx proTx; - if (!GetTxPayload(tx, proTx)) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx.GetHash().ToString()); return true; // i.e. can't decode payload == conflict } - + auto& proTx = *opt_proTx; // this method should only be called with validated ProTxs auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); if (!dmn) {