Replace more require usages (#1822)

* Replace more require usages

* lint: replace circular dep

src/amount.h

@@ -135,7 +135,9 @@ struct CTokenAmount { // simple std::pair is less informative
 
         // add
         auto sumRes = SafeAdd(nValue, amount);
-        Require(sumRes);
+        if (!sumRes) {
+            return sumRes;
+        }
 
         nValue = *sumRes;
         return Res::Ok();

src/masternodes/accounts.cpp

@@ -3,6 +3,7 @@
 // file LICENSE or http://www.opensource.org/licenses/mit-license.php.
 
 #include <masternodes/accounts.h>
+#include <masternodes/errors.h>
 
 void CAccountsView::ForEachBalance(std::function<bool(const CScript &, const CTokenAmount &)> callback,
                                    const BalanceKey &start) {
@@ -54,15 +55,23 @@ Res CAccountsView::SubBalance(const CScript &owner, CTokenAmount amount) {
 }
 
 Res CAccountsView::AddBalances(const CScript &owner, const CBalances &balances) {
-    for (const auto &kv : balances.balances)
-        Require(AddBalance(owner, CTokenAmount{kv.first, kv.second}));
+    for (const auto &kv : balances.balances) {
+        auto res = AddBalance(owner, CTokenAmount{kv.first, kv.second});
+        if (!res) {
+            return res;
+        }
+    }
 
     return Res::Ok();
 }
 
 Res CAccountsView::SubBalances(const CScript &owner, const CBalances &balances) {
-    for (const auto &kv : balances.balances)
-        Require(SubBalance(owner, CTokenAmount{kv.first, kv.second}));
+    for (const auto &kv : balances.balances) {
+        auto res = SubBalance(owner, CTokenAmount{kv.first, kv.second});
+        if (!res) {
+            return res;
+        }
+    }
 
     return Res::Ok();
 }
@@ -84,7 +93,10 @@ uint32_t CAccountsView::GetBalancesHeight(const CScript &owner) {
 }
 
 Res CAccountsView::StoreFuturesUserValues(const CFuturesUserKey &key, const CFuturesUserValue &futures) {
-    Require(WriteBy<ByFuturesSwapKey>(key, futures), []{ return "Failed to store futures"; });
+    auto res = WriteBy<ByFuturesSwapKey>(key, futures);
+    if (!res) {
+        return DeFiErrors::AccountsFuturesStore();
+    }
     return Res::Ok();
 }
 
@@ -95,12 +107,18 @@ void CAccountsView::ForEachFuturesUserValues(
 }
 
 Res CAccountsView::EraseFuturesUserValues(const CFuturesUserKey &key) {
-    Require(EraseBy<ByFuturesSwapKey>(key), []{ return "Failed to erase futures"; });
+    auto res = EraseBy<ByFuturesSwapKey>(key);
+    if (!res) {
+        return DeFiErrors::AccountsFuturesErase();
+    }
     return Res::Ok();
 }
 
 Res CAccountsView::StoreFuturesDUSD(const CFuturesUserKey &key, const CAmount &amount) {
-    Require(WriteBy<ByFuturesDUSDKey>(key, amount), []{ return "Failed to store futures"; });
+    auto res = WriteBy<ByFuturesDUSDKey>(key, amount);
+    if (!res) {
+        return DeFiErrors::AccountsFuturesStore();
+    }
     return Res::Ok();
 }
 
@@ -110,6 +128,9 @@ void CAccountsView::ForEachFuturesDUSD(std::function<bool(const CFuturesUserKey
 }
 
 Res CAccountsView::EraseFuturesDUSD(const CFuturesUserKey &key) {
-    Require(EraseBy<ByFuturesDUSDKey>(key), []{ return "Failed to erase futures"; });
+    auto res = EraseBy<ByFuturesDUSDKey>(key);
+    if (!res) {
+        return DeFiErrors::AccountsFuturesErase();
+    }
     return Res::Ok();
 }

src/masternodes/errors.h

@@ -7,6 +7,7 @@
 
 #include <amount.h>
 #include <masternodes/res.h>
+#include <masternodes/vault.h>
 
 class DeFiErrors {
 public:
@@ -27,6 +28,58 @@ class DeFiErrors {
         return Res::Err("Below minimum swapable amount, must be at least %s BTC", GetDecimalString(minSwap));
     }
 
+    static Res ICXExists(const std::string &str, const uint256 &tx) {
+        return Res::Err("%s with creation tx %s already exists!", str, tx.GetHex());
+    }
+
+    static Res ICXOrderNotExist(const uint256 &tx) {
+        return Res::Err("order with creation tx %s doesn't exists!", tx.GetHex());
+    }
+
+    static Res ICXOrderInvalidType() {
+        return Res::Err("invalid order type!");
+    }
+
+    static Res ICXOrderAmountFromZero() {
+        return Res::Err("order amountFrom must be greater than 0!");
+    }
+
+    static Res ICXOrderOrderPriceZero() {
+        return Res::Err("order price must be greater than 0!");
+    }
+
+    static Res ICXOrderAmountsNotEqual() {
+        return Res::Err("order amountToFill does not equal to amountFrom!");
+    }
+
+    static Res ICXOrderExiryTooSmall(const uint32_t amount) {
+        return Res::Err("order expiry must be greater than %d!", amount);
+    }
+
+    static Res ICXOfferAmountZero() {
+        return Res::Err("offer amount must be greater than 0!");
+    }
+
+    static Res ICXSubmitAmountZero() {
+        return Res::Err("Invalid amount, must be greater than 0!");
+    }
+
+    static Res ICXSubmitInvalidHash() {
+        return Res::Err("Invalid hash, htlc hash is empty and it must be set!");
+    }
+
+    static Res ICXSubmitTimeout() {
+        return Res::Err("Invalid timeout, must be greater than 0!");
+    }
+
+    static Res ICXSubmitEXTEmpty() {
+        return Res::Err("Invalid htlcscriptAddress, htlcscriptAddress is empty and it must be set!");
+    }
+
+    static Res ICXSubmitEXTInvalid() {
+        return Res::Err("Invalid refundPubkey is not a valid pubkey!");
+    }
+
     static Res MNInvalidAttribute() {
         return Res::Err("Attributes unavailable");
     }
@@ -328,6 +381,18 @@ class DeFiErrors {
     static Res GovVarEraseNonExist(const uint32_t type) {
         return Res::Err("Attribute {%d} not exists", type);
     }
+
+    static Res GovVarFailedWrite() {
+        return Res::Err("Cannot write to DB");
+    }
+
+    static Res AccountsFuturesStore() {
+        return Res::Err("Failed to store futures");
+    }
+
+    static Res AccountsFuturesErase() {
+        return Res::Err("Failed to erase futures");
+    }
 };
 
 #endif  // DEFI_MASTERNODES_ERRORS_H

src/masternodes/gv.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file LICENSE or http://www.opensource.org/licenses/mit-license.php.
 
+#include <masternodes/errors.h>
 #include <masternodes/govvariables/attributes.h>
 #include <masternodes/govvariables/icx_takerfee_per_btc.h>
 #include <masternodes/govvariables/loan_daily_reward.h>
@@ -55,8 +56,12 @@ std::shared_ptr<GovVariable> CGovView::GetVariable(const std::string &name) cons
 }
 
 Res CGovView::SetStoredVariables(const std::set<std::shared_ptr<GovVariable>> &govVars, const uint32_t height) {
-    for (auto &item : govVars)
-        Require(WriteBy<ByHeightVars>(GovVarKey{height, item->GetName()}, *item), []{ return "Cannot write to DB"; });
+    for (auto &item : govVars) {
+        auto res = WriteBy<ByHeightVars>(GovVarKey{height, item->GetName()}, *item);
+        if (!res) {
+            return DeFiErrors::GovVarFailedWrite();
+        }
+    }
 
     return Res::Ok();
 }

src/masternodes/icxorder.cpp

@@ -1,4 +1,5 @@
 #include <core_io.h>  /// ValueFromAmount
+#include <masternodes/errors.h>
 #include <masternodes/icxorder.h>
 #include <rpc/util.h>  /// AmountFromValue
 
@@ -64,17 +65,24 @@ uint8_t CICXOrderView::GetICXOrderStatus(const OrderKey &key) const {
 
 Res CICXOrderView::ICXCreateOrder(const CICXOrderImpl &order) {
     // this should not happen, but for sure
-    Require(!GetICXOrderByCreationTx(order.creationTx),
-            "order with creation tx %s already exists!",
-            order.creationTx.GetHex());
-    Require(order.orderType == CICXOrder::TYPE_INTERNAL || order.orderType == CICXOrder::TYPE_EXTERNAL,
-            "invalid order type!");
-    Require(order.amountFrom != 0, "order amountFrom must be greater than 0!");
-    Require(order.amountToFill == order.amountFrom, "order amountToFill does not equal to amountFrom!");
-    Require(order.orderPrice != 0, "order price must be greater than 0!");
-    Require(order.expiry >= CICXOrder::DEFAULT_EXPIRY,
-            "order expiry must be greater than %d!",
-            CICXOrder::DEFAULT_EXPIRY - 1);
+    if (GetICXOrderByCreationTx(order.creationTx)) {
+        return DeFiErrors::ICXExists("order", order.creationTx);
+    }
+    if (order.orderType != CICXOrder::TYPE_INTERNAL && order.orderType != CICXOrder::TYPE_EXTERNAL) {
+        return DeFiErrors::ICXOrderInvalidType();
+    }
+    if (order.amountFrom == 0) {
+        return DeFiErrors::ICXOrderAmountFromZero();
+    }
+    if (order.amountToFill != order.amountFrom) {
+        return DeFiErrors::ICXOrderAmountsNotEqual();
+    }
+    if (order.orderPrice == 0) {
+        return DeFiErrors::ICXOrderOrderPriceZero();
+    }
+    if (order.expiry < CICXOrder::DEFAULT_EXPIRY) {
+        return DeFiErrors::ICXOrderExiryTooSmall(CICXOrder::DEFAULT_EXPIRY - 1);
+    }
 
     OrderKey key(order.idToken, order.creationTx);
     WriteBy<ICXOrderCreationTx>(order.creationTx, order);
@@ -87,9 +95,9 @@ Res CICXOrderView::ICXCreateOrder(const CICXOrderImpl &order) {
 
 Res CICXOrderView::ICXUpdateOrder(const CICXOrderImpl &order) {
     // this should not happen, but for sure
-    Require(GetICXOrderByCreationTx(order.creationTx),
-            "order with creation tx %s doesn't exists!",
-            order.creationTx.GetHex());
+    if (!GetICXOrderByCreationTx(order.creationTx)) {
+        return DeFiErrors::ICXOrderNotExist(order.creationTx);
+    }
 
     OrderKey key(order.idToken, order.creationTx);
     WriteBy<ICXOrderCreationTx>(order.creationTx, order);
@@ -150,10 +158,12 @@ uint8_t CICXOrderView::GetICXMakeOfferStatus(const TxidPairKey &key) const {
 
 Res CICXOrderView::ICXMakeOffer(const CICXMakeOfferImpl &makeoffer) {
     // this should not happen, but for sure
-    Require(!GetICXMakeOfferByCreationTx(makeoffer.creationTx),
-            "makeoffer with creation tx %s already exists!",
-            makeoffer.creationTx.GetHex());
-    Require(makeoffer.amount != 0, "offer amount must be greater than 0!");
+    if (GetICXMakeOfferByCreationTx(makeoffer.creationTx)) {
+        return DeFiErrors::ICXExists("makeoffer",makeoffer.creationTx);
+    }
+    if (makeoffer.amount == 0) {
+        return DeFiErrors::ICXOfferAmountZero();
+    }
 
     WriteBy<ICXMakeOfferCreationTx>(makeoffer.creationTx, makeoffer);
     WriteBy<ICXMakeOfferOpenKey>(TxidPairKey(makeoffer.orderTx, makeoffer.creationTx), CICXMakeOffer::STATUS_OPEN);
@@ -211,12 +221,18 @@ std::unique_ptr<CICXOrderView::CICXSubmitDFCHTLCImpl> CICXOrderView::GetICXSubmi
 
 Res CICXOrderView::ICXSubmitDFCHTLC(const CICXSubmitDFCHTLCImpl &submitdfchtlc) {
     // this should not happen, but for sure
-    Require(!GetICXSubmitDFCHTLCByCreationTx(submitdfchtlc.creationTx),
-            "submitdfchtlc with creation tx %s already exists!",
-            submitdfchtlc.creationTx.GetHex());
-    Require(submitdfchtlc.amount != 0, "Invalid amount, must be greater than 0!");
-    Require(!submitdfchtlc.hash.IsNull(), "Invalid hash, htlc hash is empty and it must be set!");
-    Require(submitdfchtlc.timeout != 0, "Invalid timeout, must be greater than 0!");
+    if (GetICXSubmitDFCHTLCByCreationTx(submitdfchtlc.creationTx)) {
+        return DeFiErrors::ICXExists("submitdfchtlc", submitdfchtlc.creationTx);
+    }
+    if (submitdfchtlc.amount == 0) {
+        return DeFiErrors::ICXSubmitAmountZero();
+    }
+    if (submitdfchtlc.hash.IsNull()) {
+        return DeFiErrors::ICXSubmitInvalidHash();
+    }
+    if (submitdfchtlc.timeout == 0) {
+        return DeFiErrors::ICXSubmitTimeout();
+    }
 
     WriteBy<ICXSubmitDFCHTLCCreationTx>(submitdfchtlc.creationTx, submitdfchtlc);
     WriteBy<ICXSubmitDFCHTLCOpenKey>(TxidPairKey(submitdfchtlc.offerTx, submitdfchtlc.creationTx),
@@ -290,15 +306,24 @@ std::unique_ptr<CICXOrderView::CICXSubmitEXTHTLCImpl> CICXOrderView::GetICXSubmi
 
 Res CICXOrderView::ICXSubmitEXTHTLC(const CICXSubmitEXTHTLCImpl &submitexthtlc) {
     // this should not happen, but for sure
-    Require(!GetICXSubmitEXTHTLCByCreationTx(submitexthtlc.creationTx),
-            "submitexthtlc with creation tx %s already exists!",
-            submitexthtlc.creationTx.GetHex());
-    Require(submitexthtlc.amount != 0, "Invalid amount, must be greater than 0!");
-    Require(!submitexthtlc.htlcscriptAddress.empty(),
-            "Invalid htlcscriptAddress, htlcscriptAddress is empty and it must be set!");
-    Require(!submitexthtlc.hash.IsNull(), "Invalid hash, htlc hash is empty and it must be set!");
-    Require(submitexthtlc.ownerPubkey.IsFullyValid(), "Invalid refundPubkey is not a valid pubkey!");
-    Require(submitexthtlc.timeout != 0, "Invalid timout, must be greater than 0!");
+    if (GetICXSubmitEXTHTLCByCreationTx(submitexthtlc.creationTx)) {
+        return DeFiErrors::ICXExists("submitexthtlc", submitexthtlc.creationTx);
+    }
+    if (submitexthtlc.amount == 0) {
+        return DeFiErrors::ICXSubmitAmountZero();
+    }
+    if (submitexthtlc.htlcscriptAddress.empty()) {
+        return DeFiErrors::ICXSubmitEXTEmpty();
+    }
+    if (submitexthtlc.hash.IsNull()) {
+        return DeFiErrors::ICXSubmitInvalidHash();
+    }
+    if (!submitexthtlc.ownerPubkey.IsFullyValid()) {
+        return DeFiErrors::ICXSubmitEXTInvalid();
+    }
+    if (submitexthtlc.timeout == 0) {
+        return DeFiErrors::ICXSubmitTimeout();
+    }
 
     WriteBy<ICXSubmitEXTHTLCCreationTx>(submitexthtlc.creationTx, submitexthtlc);
     WriteBy<ICXSubmitEXTHTLCOpenKey>(TxidPairKey(submitexthtlc.offerTx, submitexthtlc.creationTx),
@@ -367,9 +392,9 @@ Res CICXOrderView::ICXClaimDFCHTLC(const CICXClaimDFCHTLCImpl &claimdfchtlc,
                                    const uint256 &offertxid,
                                    const CICXOrderImpl &order) {
     // this should not happen, but for sure
-    Require(!GetICXClaimDFCHTLCByCreationTx(claimdfchtlc.creationTx),
-            "claimdfchtlc with creation tx %s already exists!",
-            claimdfchtlc.creationTx.GetHex());
+    if (GetICXClaimDFCHTLCByCreationTx(claimdfchtlc.creationTx)) {
+        return DeFiErrors::ICXExists("claimdfchtlc", claimdfchtlc.creationTx);
+    }
 
     WriteBy<ICXClaimDFCHTLCCreationTx>(claimdfchtlc.creationTx, claimdfchtlc);
     WriteBy<ICXClaimDFCHTLCKey>(TxidPairKey(offertxid, claimdfchtlc.creationTx), CICXSubmitDFCHTLC::STATUS_CLAIMED);
@@ -395,9 +420,9 @@ std::unique_ptr<CICXOrderView::CICXCloseOrderImpl> CICXOrderView::GetICXCloseOrd
 
 Res CICXOrderView::ICXCloseOrder(const CICXCloseOrderImpl &closeorder) {
     // this should not happen, but for sure
-    Require(!GetICXCloseOrderByCreationTx(closeorder.creationTx),
-            "closeorder with creation tx %s already exists!",
-            closeorder.creationTx.GetHex());
+    if (GetICXCloseOrderByCreationTx(closeorder.creationTx)) {
+        return DeFiErrors::ICXExists("closeorder", closeorder.creationTx);
+    }
 
     WriteBy<ICXCloseOrderCreationTx>(closeorder.creationTx, closeorder.orderTx);
 
@@ -414,9 +439,9 @@ std::unique_ptr<CICXOrderView::CICXCloseOfferImpl> CICXOrderView::GetICXCloseOff
 
 Res CICXOrderView::ICXCloseOffer(const CICXCloseOfferImpl &closeoffer) {
     // this should not happen, but for sure
-    Require(!GetICXCloseOrderByCreationTx(closeoffer.creationTx),
-            "closeooffer with creation tx %s already exists!",
-            closeoffer.creationTx.GetHex());
+    if (GetICXCloseOrderByCreationTx(closeoffer.creationTx)) {
+        return DeFiErrors::ICXExists("closeooffer", closeoffer.creationTx);
+    }
 
     WriteBy<ICXCloseOfferCreationTx>(closeoffer.creationTx, closeoffer.offerTx);
 

test/lint/lint-circular-dependencies.sh

@@ -17,7 +17,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "chain -> chainparams -> masternodes/mn_checks -> validation -> wallet/wallet -> chain"
     "chainparams -> key_io -> chainparams"
     "chainparams -> masternodes/mn_checks -> index/txindex -> index/base -> chainparams"
-    "chainparams -> masternodes/mn_checks -> masternodes/vaulthistory -> masternodes/vault -> chainparams"
+    "chainparams -> masternodes/mn_checks -> masternodes/errors -> masternodes/vault -> chainparams"
     "chainparams -> masternodes/mn_checks -> txmempool -> chainparams"
     "chainparams -> masternodes/mn_checks -> validation -> chainparams"
     "chainparams -> masternodes/mn_checks -> validation -> spv/spv_wrapper -> chainparams"