diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 11c580392f..2cf9a45066 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -179,22 +179,25 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c std::vector dummy; const auto txType = GuessCustomTxType(tx, dummy); - if (IsBelowDF6MintTokenOrAccountToUtxos(txType, nSpendHeight) || (nSpendHeight >= chainparams.GetConsensus().DF20GrandCentralHeight && txType == CustomTxType::UpdateMasternode)) { - CCustomCSView discardCache(mnview, nullptr, nullptr, nullptr); - // Note: TXs are already filtered. So we pass isEVMEnabled to false, but for future proof, refactor this enough, - // that it's propagated. - // - // Note: We set time to 0 here. Take care not to use time - // in Apply for MintToken or AccountToUtxos path. + if (txType == CustomTxType::UpdateMasternode) { BlockContext blockCtx(nSpendHeight, {}, chainparams.GetConsensus(), &discardCache); auto txCtx = TransactionContext{ inputs, tx, blockCtx, }; - auto res = ApplyCustomTx(blockCtx, txCtx, &canSpend); - if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-customtx", res.msg); + auto &[res, txMessage] = txCtx.GetTxMessage(); + if (!res) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-updatemasternode-message", strprintf("%sTx: %s", ToString(txType), res.msg)); + } + auto obj = std::get(txMessage); + for (const auto &item : obj.updates) { + if (item.first == static_cast(UpdateMasternodeType::OwnerAddress)) { + if (const auto node = mnview.GetMasternode(obj.mnId)) { + canSpend = node->collateralTx.IsNull() ? obj.mnId : node->collateralTx; + } + break; + } } } diff --git a/src/dfi/consensus/masternodes.cpp b/src/dfi/consensus/masternodes.cpp index 982e9cf114..2c1b16741d 100644 --- a/src/dfi/consensus/masternodes.cpp +++ b/src/dfi/consensus/masternodes.cpp @@ -198,7 +198,9 @@ Res CMasternodesConsensus::operator()(const CUpdateMasterNodeMessage &obj) const } if (tx.vout[1].nValue != GetMnCollateralAmount(height)) { - return Res::Err("Incorrect collateral amount"); + return Res::Err("Incorrect collateral amount. Found: %s Expected: %s", + GetDecimalString(tx.vout[1].nValue), + GetDecimalString(GetMnCollateralAmount(height))); } if (mnview.GetMasternodeIdByOwner(keyID) || mnview.GetMasternodeIdByOperator(keyID)) { diff --git a/src/dfi/mn_checks.cpp b/src/dfi/mn_checks.cpp index 1a5f64ae8b..baa61653f3 100644 --- a/src/dfi/mn_checks.cpp +++ b/src/dfi/mn_checks.cpp @@ -511,7 +511,7 @@ void PopulateVaultHistoryData(CHistoryWriters &writers, } } -Res ApplyCustomTx(BlockContext &blockCtx, TransactionContext &txCtx, uint256 *canSpend) { +Res ApplyCustomTx(BlockContext &blockCtx, TransactionContext &txCtx) { auto &mnview = blockCtx.GetView(); const auto isEvmEnabledForBlock = blockCtx.GetEVMEnabledForBlock(); const auto &consensus = txCtx.GetConsensus(); @@ -566,18 +566,6 @@ Res ApplyCustomTx(BlockContext &blockCtx, TransactionContext &txCtx, uint256 *ca res = CustomTxVisit(txMessage, blockCtxTxView, txCtx); if (res) { - if (canSpend && txType == CustomTxType::UpdateMasternode) { - auto obj = std::get(txMessage); - for (const auto &item : obj.updates) { - if (item.first == static_cast(UpdateMasternodeType::OwnerAddress)) { - if (const auto node = mnview.GetMasternode(obj.mnId)) { - *canSpend = node->collateralTx.IsNull() ? obj.mnId : node->collateralTx; - } - break; - } - } - } - // Track burn fee if (txType == CustomTxType::CreateToken || txType == CustomTxType::CreateMasternode) { mnview.GetHistoryWriters().AddFeeBurn(tx.vout[0].scriptPubKey, tx.vout[0].nValue); diff --git a/src/dfi/mn_checks.h b/src/dfi/mn_checks.h index 52d391e874..0904794fbd 100644 --- a/src/dfi/mn_checks.h +++ b/src/dfi/mn_checks.h @@ -232,7 +232,7 @@ Res CustomMetadataParse(uint32_t height, const std::vector &metadata, CCustomTxMessage &txMessage); -Res ApplyCustomTx(BlockContext &blockCtx, TransactionContext &txCtx, uint256 *canSpend = nullptr); +Res ApplyCustomTx(BlockContext &blockCtx, TransactionContext &txCtx); Res CustomTxVisit(const CCustomTxMessage &txMessage, BlockContext &blockCtx, const TransactionContext &txCtx); diff --git a/src/dfi/mn_rpc.cpp b/src/dfi/mn_rpc.cpp index 1f99ec0dcd..47457e5e8b 100644 --- a/src/dfi/mn_rpc.cpp +++ b/src/dfi/mn_rpc.cpp @@ -1097,8 +1097,8 @@ UniValue isappliedcustomtx(const JSONRPCRequest &request) { return result; } - // post Dakota it's not allowed tx to be skipped - // so tx that can be found in a block is applyed + // Post Dakota TXs are not allowed to be skipped, + // so TXs found in a block are applied. if (blockHeight >= Params().GetConsensus().DF6DakotaHeight) { result.setBool(true); } else { diff --git a/test/functional/feature_prevent_bad_tx_propagation.py b/test/functional/feature_prevent_bad_tx_propagation.py index 1bf69d9759..41e4ebb261 100755 --- a/test/functional/feature_prevent_bad_tx_propagation.py +++ b/test/functional/feature_prevent_bad_tx_propagation.py @@ -5,9 +5,8 @@ # file LICENSE or http://www.opensource.org/licenses/mit-license.php. """Test account mining behaviour""" -from test_framework.authproxy import JSONRPCException from test_framework.test_framework import DefiTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_raises_rpc_error class AccountMiningTest(DefiTestFramework): @@ -34,13 +33,14 @@ def run_test(self): # Corrent account to utxo tx - entering mempool node.accounttoutxos(account, {destination: "4@DFI"}) - try: - # Not enough amount - rejected - node.accounttoutxos(account, {destination: "2@DFI"}) - except JSONRPCException as e: - errorString = e.error["message"] - - assert "bad-txns-customtx" in errorString + # Not enough amount - rejected + assert_raises_rpc_error( + -26, + "AccountToUtxosTx: amount 1.00000000 is less than 2.00000000", + node.accounttoutxos, + account, + {destination: "2@DFI"}, + ) # Store block height blockcount = node.getblockcount() diff --git a/test/functional/rpc_updatemasternode.py b/test/functional/rpc_updatemasternode.py index 9f43a86787..285f360d46 100755 --- a/test/functional/rpc_updatemasternode.py +++ b/test/functional/rpc_updatemasternode.py @@ -472,7 +472,6 @@ def run_test(self): rawtx = self.nodes[0].createrawtransaction( [ {"txid": mn_id, "vout": 1}, - {"txid": missing_auth_tx, "vout": missing_input_vout}, {"txid": owner_auth_tx, "vout": owner_auth_vout}, ], [ @@ -483,7 +482,7 @@ def run_test(self): signed_rawtx = self.nodes[0].signrawtransactionwithwallet(rawtx) assert_raises_rpc_error( -26, - "bad-txns-collateral-locked, tried to spend locked collateral for", + "Incorrect collateral amount. Found: 10.10000000 Expected: 10.00000000", self.nodes[0].sendrawtransaction, signed_rawtx["hex"], )