Remove redundant ApplyCustomTx (#2689)

* Remove redundant ApplyCustomTx * Format Python * Remove unused import
DeFiCh · Jan 5, 2024 · af30b58 · af30b58
1 parent a204b91
commit af30b58
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 38 deletions.
diff --git 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<unsigned char> 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<CUpdateMasterNodeMessage>(txMessage);
+        for (const auto &item : obj.updates) {
+            if (item.first == static_cast<uint8_t>(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
@@ -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
@@ -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<CUpdateMasterNodeMessage>(txMessage);
-                for (const auto &item : obj.updates) {
-                    if (item.first == static_cast<uint8_t>(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
@@ -232,7 +232,7 @@ Res CustomMetadataParse(uint32_t height,
                         const std::vector<unsigned char> &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
@@ -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
@@ -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
@@ -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"],
         )