From 4e0b36e5fd8a5be0e60539631aafb4ca2554e351 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 2 May 2026 02:12:31 +0700 Subject: [PATCH 1/4] refactor: simplify implementation of regression tests evo_trivialvalidation This commit replaces usage of helpers GetPayload+IsTrivialValid to GetValidatedPayload It unifies validation between regression tests and production code and useful for the next commits in PR --- src/evo/specialtxman.cpp | 4 ++-- src/evo/specialtxman.h | 9 +++++++++ src/test/data/trivially_invalid.json | 2 +- src/test/evo_trivialvalidation.cpp | 17 ++++++++--------- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 515c1dd06dec..c4e5dec5b4b0 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -859,8 +859,8 @@ static bool CheckHashSig(const ProTx& proTx, const CBLSPublicKey& pubKey, TxVali } template -static std::optional GetValidatedPayload(const CTransaction& tx, gsl::not_null pindexPrev, - const ChainstateManager& chainman, TxValidationState& state) +std::optional GetValidatedPayload(const CTransaction& tx, gsl::not_null pindexPrev, + const ChainstateManager& chainman, TxValidationState& state) { if (tx.nType != ProTx::SPECIALTX_TYPE) { state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type"); diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 2d5ab32096bb..ee16bad08893 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -93,6 +93,15 @@ class CSpecialTxProcessor }; +/** + * This helper does some trivial validations that doesn't depends on collateral and + * masternode list. Use CheckPro*Tx for the full validation of transaction, + * including bls signatures, list of masternodes and collateral + */ +template +std::optional GetValidatedPayload(const CTransaction& tx, gsl::not_null pindexPrev, + const ChainstateManager& chainman, TxValidationState& state); + bool CheckProRegTx(const CTransaction& tx, gsl::not_null pindexPrev, CDeterministicMNManager& dmnman, const CCoinsViewCache& view, const ChainstateManager& chainman, TxValidationState& state, bool check_sigs); diff --git a/src/test/data/trivially_invalid.json b/src/test/data/trivially_invalid.json index cfc789cbee37..88489086c809 100644 --- a/src/test/data/trivially_invalid.json +++ b/src/test/data/trivially_invalid.json @@ -85,7 +85,7 @@ "proupregtx", "legacy", "03000300010000000000000000000000000000000000000000000000000000000000000000ffffffff016affffffff0100f2052a01000000016a00000000e40000bfd280d22a21574ea9f568f7a729b370f4051b89eab6df3d0d74b65fde1bdb7900000a6a7fb814db71eb6ef12341749d81b5fd326eb4b9445a3be65668da9856910b378ec6427fe18fc81c39499e7c7060647618f8ec94404051e4a7ab68f1b017fd9e7f94761976a914c18e0b18e0d992e7471d8a4d4da428e8e191328788ac959ca83b61eeb3f5543973253da6da8e9ac23f9e7844453ac5543b51d0177799416666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666", - "gettxpayload-fail", + "bad-protx-payload", "Transaction with version set to null" ], [ diff --git a/src/test/evo_trivialvalidation.cpp b/src/test/evo_trivialvalidation.cpp index 74da999581b2..26b6bddbb778 100644 --- a/src/test/evo_trivialvalidation.cpp +++ b/src/test/evo_trivialvalidation.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -29,22 +30,20 @@ template void TestTxHelper(const CMutableTransaction& tx, gsl::not_null pindexPrev, const std::optional& expected_error, const ChainstateManager& chainman) { - const bool payload_to_fail = expected_error.has_value() && expected_error.value() == "gettxpayload-fail"; - const auto opt_payload = GetTxPayload(tx, false); - BOOST_CHECK_EQUAL(opt_payload.has_value(), !payload_to_fail); + TxValidationState dummy_state; + auto opt_payload = GetValidatedPayload(CTransaction{tx}, pindexPrev, chainman, dummy_state); - // No need to check anything else if GetTxPayload() expected to fail - if (payload_to_fail) return; + BOOST_CHECK_EQUAL(opt_payload.has_value(), !expected_error.has_value()); - TxValidationState dummy_state; - BOOST_CHECK_EQUAL(opt_payload->IsTriviallyValid(pindexPrev, chainman, dummy_state), !expected_error.has_value()); if (expected_error.has_value()) { BOOST_CHECK_EQUAL(dummy_state.GetRejectReason(), expected_error.value()); } } -void trivialvalidation_runner(const ChainstateManager& chainman, const std::string& json) +void trivialvalidation_runner(ChainstateManager& chainman, const std::string& json) { + LOCK(::cs_main); + const UniValue vectors = read_json(json); for (size_t idx = 1; idx < vectors.size(); idx++) { @@ -72,7 +71,7 @@ void trivialvalidation_runner(const ChainstateManager& chainman, const std::stri BOOST_CHECK_EQUAL(tx.nVersion, 3); BOOST_CHECK_EQUAL(tx.GetHash(), txHash); // Deserialization based on transaction nType - TxValidationState dummy_state; + switch (tx.nType) { case TRANSACTION_PROVIDER_REGISTER: { BOOST_CHECK_EQUAL(txType, "proregtx"); From d86d5716a126b19adec84daf6bc600b502ae045a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 1 May 2026 02:24:52 +0700 Subject: [PATCH 2/4] fix!: use the same version along all protx special transactions CProRegTx and CProUpServTx used to be the only type of protx that have a different version. It is theoretically acceptable in assumption that there is no new features or version will ever be introduced for protx special transaction. Though, for better compatibility for futher version, unification, simplicity of documentation and to reduce user's confusions for after-v24 version of CProUpRegTx and CProUpRevTx are allowed to be "ext addresses" even they don't have any network related fields _at the moment_ So, since now: - version 1: legacy BLS, extended addresses disallowed (pre v19 fork) - version 2: basic BLS, extended addresses disallowed (since v19 fork) - version 3: basic BLS, extended addresses allowed (since v24 fork) NOTE: there are also classes CSimplifiedMNListEntry and CDeterministicMNState use the same enum for its version; moreover CDeterministicMNState inherits version directly from CProRegTx. This refactoring doesn't contradict or conflict this behavior --- src/evo/providertx.cpp | 9 ++++----- src/evo/specialtxman.cpp | 17 +++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index b2c3691aa4f5..21be463e00ac 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -20,11 +20,10 @@ template [[nodiscard]] uint16_t GetMaxFromDeployment(gsl::not_null pindexPrev, const ChainstateManager& chainman, std::optional is_basic_override) { - constexpr bool is_extaddr_eligible{std::is_same_v, CProRegTx> || std::is_same_v, CProUpServTx>}; - return ProTxVersion::GetMax( - is_basic_override ? *is_basic_override - : DeploymentActiveAfter(pindexPrev, chainman.GetConsensus(), Consensus::DEPLOYMENT_V19), - is_extaddr_eligible ? DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24) : false); + return ProTxVersion::GetMax(is_basic_override ? *is_basic_override + : DeploymentActiveAfter(pindexPrev, chainman.GetConsensus(), + Consensus::DEPLOYMENT_V19), + DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24)); } template uint16_t GetMaxFromDeployment(gsl::not_null pindexPrev, const ChainstateManager& chainman, diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index c4e5dec5b4b0..a23472e0e9ea 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -882,15 +882,13 @@ std::optional GetValidatedPayload(const CTransaction& tx, gsl::not_null pindexPrev, const uint16_t tx_type, - const uint16_t state_version, const uint16_t tx_version, - const ChainstateManager& chainman, TxValidationState& state) +static bool IsVersionChangeValid(gsl::not_null pindexPrev, const uint16_t state_version, + const uint16_t tx_version, const ChainstateManager& chainman, TxValidationState& state) { if (!DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24)) { // New restrictions only apply after v24 deployment @@ -907,11 +905,6 @@ static bool IsVersionChangeValid(gsl::not_null pindexPrev, c return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-upgrade"); } - if (tx_type != TRANSACTION_PROVIDER_UPDATE_SERVICE && tx_version == ProTxVersion::ExtAddr) { - // Only new entries (ProRegTx) and service updates (ProUpServTx) can use ExtAddr versioning - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-tx-type"); - } - return true; } @@ -1077,7 +1070,7 @@ bool CheckProUpServTx(const CTransaction& tx, gsl::not_null return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } - if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { + if (!IsVersionChangeValid(pindexPrev, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { // pass the state returned by the function above return false; } @@ -1152,7 +1145,7 @@ bool CheckProUpRegTx(const CTransaction& tx, gsl::not_null p return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } - if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { + if (!IsVersionChangeValid(pindexPrev, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { // pass the state returned by the function above return false; } @@ -1219,7 +1212,7 @@ bool CheckProUpRevTx(const CTransaction& tx, gsl::not_null p return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } - if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { + if (!IsVersionChangeValid(pindexPrev, dmn->pdmnState->nVersion, opt_ptx->nVersion, chainman, state)) { // pass the state returned by the function above return false; } From 5393e1a4735a364d90f38a3f06c1021c0a5ceb27 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 30 Apr 2026 19:35:14 +0700 Subject: [PATCH 3/4] refactor: simplify implementation of providertx versioning It helps to drop multiple circular dependencies for providertx <-> validation.h as a side effect --- src/evo/netinfo.cpp | 2 +- src/evo/providertx.cpp | 45 +++-------------- src/evo/providertx.h | 65 +++++++++---------------- src/evo/specialtxman.cpp | 6 ++- src/evo/types.h | 24 +++++++++ src/rpc/evo.cpp | 15 +++--- src/validation.cpp | 9 ++++ src/validation.h | 7 +++ test/lint/lint-circular-dependencies.py | 9 ++-- 9 files changed, 85 insertions(+), 97 deletions(-) diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index a3f53693ccaf..eab2e93a9b16 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -5,7 +5,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index 21be463e00ac..05803bbb3305 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -13,31 +13,6 @@ #include #include