sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

refactor: prefer using TxValidationState directly for unification with bitcoin implementation (#5335)

Browse Source 
## Issue being fixed or feature implemented
The benefits of using custom struct `maybe_error` is less significant
since the interface `TxValidationState` became simpler after
refactorings from bitcoin#15921 and bitcoin#17004

The refactoring of `maybe_error` as a class result from PR #5109 is
still useful but not for case of TxValidationState.



## What was done?
Unified using TxValidationState in dash's related code.

## How Has This Been Tested?
Run unit/functional tests


## Breaking Changes
No breaking changes, logic are same.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] 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
This commit is contained in:
Konstantin Akimov 2023-04-26 03:17:30 +07:00 committed by GitHub
parent f402423dd4
commit e3c8dcc340
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 64 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/evo/deterministicmns.cpp
View File

@ -1451,8 +1451,9 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVali
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-payload");
}
if (auto maybe_err = ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev)); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev), state)) {
// pass the state returned by the function above
return false;
}
// 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
@ -1540,8 +1541,9 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVali
}
}
if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!CheckInputsHash(tx, ptx, state)) {
// pass the state returned by the function above
return false;
}
if (keyForPayloadSig) {
@ -1571,8 +1573,9 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxV
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-payload");
}
if (auto maybe_err = ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev)); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev), state)) {
// pass the state returned by the function above
return false;
}
if (!CheckService(ptx, state)) {
@ -1616,8 +1619,9 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxV
}
// we can only check the signature if pindexPrev != nullptr and the MN is known
if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!CheckInputsHash(tx, ptx, state)) {
// pass the state returned by the function above
return false;
}
if (check_sigs && !CheckHashSig(ptx, mn->pdmnState->pubKeyOperator.Get(), state)) {
// pass the state returned by the function above
@ -1639,8 +1643,9 @@ bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-payload");
}
if (auto maybe_err = ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev)); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev), state)) {
// pass the state returned by the function above
return false;
}
CTxDestination payoutDest;
@ -1689,8 +1694,9 @@ bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
}
}
if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!CheckInputsHash(tx, ptx, state)) {
// pass the state returned by the function above
return false;
}
if (check_sigs && !CheckHashSig(ptx, PKHash(dmn->pdmnState->keyIDOwner), state)) {
// pass the state returned by the function above
@ -1712,8 +1718,9 @@ bool CheckProUpRevTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-payload");
}
if (auto maybe_err = ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev)); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!ptx.IsTriviallyValid(llmq::utils::IsV19Active(pindexPrev), state)) {
// pass the state returned by the function above
return false;
}
if (pindexPrev) {
@ -1722,8 +1729,9 @@ bool CheckProUpRevTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxVa
if (!dmn)
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) {
return state.Invalid(maybe_err.reason, std::string(maybe_err.error_str));
if (!CheckInputsHash(tx, ptx, state)) {
// pass the state returned by the function above
return false;
}
if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) {
// pass the state returned by the function above

46
src/evo/providertx.cpp
View File

@ -11,40 +11,40 @@
#include <script/standard.h>
#include <util/underlying.h>
maybe_error CProRegTx::IsTriviallyValid(bool is_bls_legacy_scheme) const
bool CProRegTx::IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const
{
if (nVersion == 0 || nVersion > GetVersion(is_bls_legacy_scheme)) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-version"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
}
if (!IsValidMnType(nType)) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-type"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-type");
}
if (nMode != 0) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-mode"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-mode");
}
if (keyIDOwner.IsNull() || !pubKeyOperator.IsValid() || keyIDVoting.IsNull()) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null");
}
if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee");
}
CTxDestination payoutDest;
if (!ExtractDestination(scriptPayout, payoutDest)) {
// should not happen as we checked script types before
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-dest"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-dest");
}
// don't allow reuse of payout key for other keys (don't allow people to put the payee key onto an online server)
if (payoutDest == CTxDestination(PKHash(keyIDOwner)) || payoutDest == CTxDestination(PKHash(keyIDVoting))) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse");
}
if (nOperatorReward > 10000) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-reward"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-reward");
}
return {};
return true;
}
std::string CProRegTx::MakeSignString() const
@ -84,13 +84,13 @@ std::string CProRegTx::ToString() const
nVersion, ToUnderlying(nType), collateralOutpoint.ToStringShort(), addr.ToString(), (double)nOperatorReward / 100, EncodeDestination(PKHash(keyIDOwner)), pubKeyOperator.ToString(nVersion == LEGACY_BLS_VERSION), EncodeDestination(PKHash(keyIDVoting)), payee, platformNodeID.ToString(), platformP2PPort, platformHTTPPort);
}
maybe_error CProUpServTx::IsTriviallyValid(bool is_bls_legacy_scheme) const
bool CProUpServTx::IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const
{
if (nVersion == 0 || nVersion > GetVersion(is_bls_legacy_scheme)) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-version"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
}
return {};
return true;
}
std::string CProUpServTx::ToString() const
@ -105,22 +105,22 @@ std::string CProUpServTx::ToString() const
nVersion, ToUnderlying(nType), proTxHash.ToString(), addr.ToString(), payee, platformNodeID.ToString(), platformP2PPort, platformHTTPPort);
}
maybe_error CProUpRegTx::IsTriviallyValid(bool is_bls_legacy_scheme) const
bool CProUpRegTx::IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const
{
if (nVersion == 0 || nVersion > GetVersion(is_bls_legacy_scheme)) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-version"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
}
if (nMode != 0) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-mode"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-mode");
}
if (!pubKeyOperator.IsValid() || keyIDVoting.IsNull()) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null");
}
if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) {
return {TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee"};
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee");
}
return {};
return true;
}
std::string CProUpRegTx::ToString() const
@ -135,18 +135,18 @@ std::string CProUpRegTx::ToString() const
nVersion, proTxHash.ToString(), pubKeyOperator.ToString(nVersion == LEGACY_BLS_VERSION), EncodeDestination(PKHash(keyIDVoting)), payee);
}
maybe_error CProUpRevTx::IsTriviallyValid(bool is_bls_legacy_scheme) const
bool CProUpRevTx::IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const
{
if (nVersion == 0 || nVersion > GetVersion(is_bls_legacy_scheme)) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-version"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
}
// nReason < CProUpRevTx::REASON_NOT_SPECIFIED is always `false` since
// nReason is unsigned and CProUpRevTx::REASON_NOT_SPECIFIED == 0
if (nReason > CProUpRevTx::REASON_LAST) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-reason"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-reason");
}
return {};
return true;
}
std::string CProUpRevTx::ToString() const

14
src/evo/providertx.h
View File

@ -114,7 +114,7 @@ public:
obj.pushKV("inputsHash", inputsHash.ToString());
}
maybe_error IsTriviallyValid(bool is_bls_legacy_scheme) const;
bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const;
};
class CProUpServTx
@ -194,7 +194,7 @@ public:
obj.pushKV("inputsHash", inputsHash.ToString());
}
maybe_error IsTriviallyValid(bool is_bls_legacy_scheme) const;
bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const;
};
class CProUpRegTx
@ -259,7 +259,7 @@ public:
obj.pushKV("inputsHash", inputsHash.ToString());
}
maybe_error IsTriviallyValid(bool is_bls_legacy_scheme) const;
bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const;
};
class CProUpRevTx
@ -322,17 +322,17 @@ public:
obj.pushKV("inputsHash", inputsHash.ToString());
}
maybe_error IsTriviallyValid(bool is_bls_legacy_scheme) const;
bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const;
};
template <typename ProTx>
static maybe_error CheckInputsHash(const CTransaction& tx, const ProTx& proTx)
static bool CheckInputsHash(const CTransaction& tx, const ProTx& proTx, TxValidationState& state)
{
if (uint256 inputsHash = CalcTxInputsHash(tx); inputsHash != proTx.inputsHash) {
return {TxValidationResult::TX_CONSENSUS, "bad-protx-inputs-hash"};
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-inputs-hash");
}
return {};
return true;
}

9
src/evo/specialtx.h
View File

@ -15,15 +15,6 @@
#include <string_view>
#include <vector>
struct maybe_error {
bool did_err{false};
TxValidationResult reason{TxValidationResult::TX_CONSENSUS};
std::string_view error_str;
constexpr maybe_error() = default;
constexpr maybe_error(TxValidationResult reasonIn, std::string_view err): did_err(true), reason(reasonIn), error_str(err) {};
};
template <typename T>
inline bool GetTxPayload(const std::vector<unsigned char>& payload, T& obj)
{

18
src/test/evo_trivialvalidation.cpp
View File

@ -43,33 +43,34 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_valid)
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");
CProRegTx ptx;
BOOST_CHECK(GetTxPayload(tx, ptx, false));
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
break;
}
case TRANSACTION_PROVIDER_UPDATE_SERVICE: {
BOOST_CHECK_EQUAL(txType, "proupservtx");
CProUpServTx ptx;
BOOST_CHECK(GetTxPayload(tx, ptx, false));
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
break;
}
case TRANSACTION_PROVIDER_UPDATE_REGISTRAR: {
BOOST_CHECK_EQUAL(txType, "proupregtx");
CProUpRegTx ptx;
BOOST_CHECK(GetTxPayload(tx, ptx, false));
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
break;
}
case TRANSACTION_PROVIDER_UPDATE_REVOKE: {
BOOST_CHECK_EQUAL(txType, "prouprevtx");
CProUpRevTx ptx;
BOOST_CHECK(GetTxPayload(tx, ptx, false));
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
break;
}
default:
@ -108,10 +109,11 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_invalid)
BOOST_CHECK_EQUAL(tx.nVersion, 3);
BOOST_CHECK_EQUAL(tx.GetHash(), txHash);
// Deserialization based on transaction nType
TxValidationState dummy_state;
if (txType == "proregtx") {
CProRegTx ptx;
if (GetTxPayload(tx, ptx, false)) {
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
} else {
BOOST_CHECK(tx.nType != TRANSACTION_PROVIDER_REGISTER || ptx.nVersion == 0);
}
@ -119,7 +121,7 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_invalid)
else if (txType == "proupservtx") {
CProUpServTx ptx;
if (GetTxPayload(tx, ptx, false)) {
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
} else {
BOOST_CHECK(tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE || ptx.nVersion == 0);
}
@ -127,7 +129,7 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_invalid)
else if (txType == "proupregtx") {
CProUpRegTx ptx;
if (GetTxPayload(tx, ptx, false)) {
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
}
else {
BOOST_CHECK(tx.nType != TRANSACTION_PROVIDER_UPDATE_REGISTRAR || ptx.nVersion == 0);
@ -136,7 +138,7 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_invalid)
else if (txType == "prouprevtx") {
CProUpRevTx ptx;
if (GetTxPayload(tx, ptx, false)) {
BOOST_CHECK(ptx.IsTriviallyValid(bls::bls_legacy_scheme.load()).did_err);
BOOST_CHECK(!ptx.IsTriviallyValid(bls::bls_legacy_scheme.load(), dummy_state));
} else {
BOOST_CHECK(tx.nType != TRANSACTION_PROVIDER_UPDATE_REVOKE || ptx.nVersion == 0);
}