diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index b0bcc3f7f2b5..9ccf89abfc61 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -190,6 +190,9 @@ bool CProUpServTx::IsTriviallyValid(TxValidationState& state) const if (nVersion < ProTxVersion::BasicBLS && nType == MnType::Evo) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-evo-version"); } + if (!IsValidMnType(nType)) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type"); + } if (netInfo->CanStorePlatform() != (nVersion >= ProTxVersion::ExtAddr)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-netinfo-version"); } diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index bb3578524d81..24fdd1082ea4 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -395,6 +395,9 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul if (!opt_proTx) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } + if (!IsValidMnType(opt_proTx->nType)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); + } for (const auto& entry : opt_proTx->netInfo->GetEntries()) { if (const auto service_opt{entry.GetAddrPort()}) { @@ -419,9 +422,6 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul if (opt_proTx->nType != dmn->nType) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); } - if (!IsValidMnType(opt_proTx->nType)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); - } auto newState = std::make_shared(*dmn->pdmnState); const uint16_t current_version{static_cast(newState->nVersion)}; @@ -1187,6 +1187,14 @@ bool CheckProUpServTx(const CTransaction& tx, gsl::not_null return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } + // Mirror BuildNewListFromBlock: nType must match the registered MN. Without this check a + // mempool-accepted ProUpServTx can make CreateNewBlock fail. An out-of-range nType is already + // rejected by IsTriviallyValid, and dmn->nType is always in range, so no separate + // IsValidMnType check is reachable here. + if (opt_ptx->nType != dmn->nType) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type-mismatch"); + } + if (!IsVersionChangeValid(pindexPrev, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { // pass the state returned by the function above return false; diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 24ace3bc7469..8b10817938a6 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,7 @@ #include #include #include +#include #include #include @@ -1190,6 +1192,151 @@ void FuncDIP3Protx(TestChainSetup& setup) const_cast(Params().GetConsensus()).DIP0003EnforcementHeight = DIP0003EnforcementHeightBackup; } +// ProUpServTx with an out-of-range or mismatched nType must be rejected before +// mempool acceptance. Historically IsTriviallyValid / CheckProUpServTx ignored nType while +// BuildNewListFromBlock rejected it, so a single relayed tx stalled CreateNewBlock/getblocktemplate. +void FuncProUpServInvalidNType(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; + + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + + CKey ownerKey; + CBLSSecretKey operatorKey; + auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, GenerateRandomAddress(), setup.coinbaseKey, ownerKey, + operatorKey); + const uint256 proTxHash = tx_reg.GetHash(); + setup.CreateAndProcessBlock({tx_reg}, coinbase_pk); + sync_dmn_tip(); + BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(proTxHash)); + BOOST_REQUIRE_EQUAL(std23::to_underlying(dmnman.GetListAtChainTip().GetMN(proTxHash)->nType), + std23::to_underlying(MnType::Regular)); + + auto make_proupserv = [&](MnType nType) { + CProUpServTx proTx; + proTx.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + proTx.nType = nType; + proTx.netInfo = NetInfoInterface::MakeNetInfo(proTx.nVersion); + proTx.proTxHash = proTxHash; + BOOST_REQUIRE_EQUAL(proTx.netInfo->AddEntry(NetInfoPurpose::CORE_P2P, "1.1.1.1:42"), NetInfoStatus::Success); + if (nType == MnType::Evo) { + // Platform fields required by CheckProUpServTx when nType claims Evo. + proTx.platformNodeID.SetHex("00112233445566778899aabbccddeeff00112233"); + proTx.platformP2PPort = 20001; + proTx.platformHTTPPort = 20002; + } + + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE; + const auto spent = + FundTransaction(chainman, tx, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), 1 * COIN); + // FundTransaction constructs a zero-fee tx; leave a dust-sized fee so ProcessTransaction + // reaches special-tx validation instead of failing on min-relay fee first. + BOOST_REQUIRE(!tx.vout.empty()); + BOOST_REQUIRE(tx.vout[0].nValue > 10000); + tx.vout[0].nValue -= 10000; + proTx.inputsHash = CalcTxInputsHash(CTransaction(tx)); + proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme); + SetTxPayload(tx, proTx); + SignTransaction(tx, spent, setup.coinbaseKey); + return tx; + }; + + // Case 1: out-of-range nType. Must fail trivial validation and never enter the mempool. + { + auto tx = make_proupserv(static_cast(42)); + auto opt_payload = GetTxPayload(tx, /*assert_type=*/false); + BOOST_REQUIRE(opt_payload.has_value()); + + TxValidationState trivial_state; + BOOST_CHECK(!opt_payload->IsTriviallyValid(trivial_state)); + BOOST_CHECK_EQUAL(trivial_state.GetRejectReason(), "bad-protx-type"); + + TxValidationState check_state; + { + LOCK(cs_main); + BOOST_CHECK(!CheckProUpServTx(CTransaction(tx), tip_index(), dmnman, chainman, check_state, /*check_sigs=*/true)); + } + BOOST_CHECK_EQUAL(check_state.GetRejectReason(), "bad-protx-type"); + + { + LOCK(cs_main); + const MempoolAcceptResult result = chainman.ProcessTransaction(MakeTransactionRef(tx)); + BOOST_CHECK(result.m_result_type == MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "bad-protx-type"); + } + } + + // Case 2: valid-range but mismatched nType (claim Evo for a Regular MN). Must fail contextual checks. + { + auto tx = make_proupserv(MnType::Evo); + auto opt_payload = GetTxPayload(tx, /*assert_type=*/false); + BOOST_REQUIRE(opt_payload.has_value()); + + // Trivial validation accepts Evo as a valid type; contextual validation must still reject. + TxValidationState trivial_state; + BOOST_CHECK(opt_payload->IsTriviallyValid(trivial_state)); + + TxValidationState check_state; + { + LOCK(cs_main); + BOOST_CHECK(!CheckProUpServTx(CTransaction(tx), tip_index(), dmnman, chainman, check_state, /*check_sigs=*/true)); + } + BOOST_CHECK_EQUAL(check_state.GetRejectReason(), "bad-protx-type-mismatch"); + BOOST_CHECK(check_state.GetResult() == TxValidationResult::TX_CONSENSUS); + + { + LOCK(cs_main); + const MempoolAcceptResult result = chainman.ProcessTransaction(MakeTransactionRef(tx)); + BOOST_CHECK(result.m_result_type == MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "bad-protx-type-mismatch"); + BOOST_CHECK(result.m_state.GetResult() == TxValidationResult::TX_CONSENSUS); + } + } + + // Case 3: the consensus path rejects a hand-built block for both reasons (defense in depth). + // Out-of-range nType is caught by the range check, a valid-but-wrong nType by the mismatch check. + for (const auto& [nType, expected_reason] : {std::pair{static_cast(42), "bad-protx-type"}, + std::pair{MnType::Evo, "bad-protx-type-mismatch"}}) { + auto tx = make_proupserv(nType); + CMutableTransaction coinbase; + coinbase.vin.emplace_back(COutPoint(), CScript() << OP_0 << OP_0); + coinbase.vout.emplace_back(50 * COIN, coinbase_pk); + + CBlock block; + block.vtx.emplace_back(MakeTransactionRef(std::move(coinbase))); + block.vtx.emplace_back(MakeTransactionRef(tx)); + BlockValidationState state; + CDeterministicMNList mn_list; + LOCK(cs_main); + BOOST_CHECK(!chainman.ActiveChainstate().ChainHelper().special_tx->BuildNewListFromBlock( + block, tip_index(), chainman.ActiveChainstate().CoinsTip(), /*debugLogs=*/false, state, mn_list)); + BOOST_CHECK_EQUAL(state.GetRejectReason(), expected_reason); + } + + // Case 4: the matching nType is still accepted, i.e. the new checks reject nothing legitimate. + { + auto tx = make_proupserv(MnType::Regular); + TxValidationState check_state; + { + LOCK(cs_main); + BOOST_CHECK_MESSAGE(CheckProUpServTx(CTransaction(tx), tip_index(), dmnman, chainman, check_state, + /*check_sigs=*/true), + "unexpected rejection: " << check_state.GetRejectReason()); + } + + LOCK(cs_main); + const MempoolAcceptResult result = chainman.ProcessTransaction(MakeTransactionRef(tx)); + BOOST_CHECK_MESSAGE(result.m_result_type == MempoolAcceptResult::ResultType::VALID, + "unexpected rejection: " << result.m_state.GetRejectReason()); + } +} + void FuncTestMempoolReorg(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); @@ -1566,6 +1713,14 @@ BOOST_AUTO_TEST_CASE(dip3_protx_basic) FuncDIP3Protx(setup); } +// Requires BasicBLS so ProUpServTx serialises nType. Pre-fix this test fails because +// IsTriviallyValid / CheckProUpServTx accept out-of-range nType. +BOOST_AUTO_TEST_CASE(proupserv_invalid_ntype_basic) +{ + TestChainV19Setup setup; + FuncProUpServInvalidNType(setup); +} + BOOST_AUTO_TEST_CASE(test_mempool_reorg_legacy) { TestChainDIP3Setup setup; diff --git a/src/test/evo_trivialvalidation.cpp b/src/test/evo_trivialvalidation.cpp index 2185e0eca3fc..16184e0a584a 100644 --- a/src/test/evo_trivialvalidation.cpp +++ b/src/test/evo_trivialvalidation.cpp @@ -7,15 +7,19 @@ #include #include +#include #include #include #include +#include +#include #include #include #include #include #include