From 1795b1376888f1873db30b3b87d0d561ab6fbb09 Mon Sep 17 00:00:00 2001 From: xiangxn Date: Sun, 24 Apr 2022 05:19:34 +0800 Subject: [PATCH] Fix #3486 "Order cancellation fee error" (#3492) * fix 3486 * introduce raiseIfInsufficient to reduce duplicate code * default to given preference if nothing else has balance * forgot to adjust myopenorders catch * fix subscript overflow * fix 3486 Co-authored-by: Stefan --- app/actions/MarketsActions.js | 40 +++++++---- app/components/Exchange/MyOpenOrders.jsx | 8 ++- app/lib/common/account_utils.js | 92 ++++++++++++++++++------ 3 files changed, 105 insertions(+), 35 deletions(-) diff --git a/app/actions/MarketsActions.js b/app/actions/MarketsActions.js index 52cfeceed8..ce2016385c 100644 --- a/app/actions/MarketsActions.js +++ b/app/actions/MarketsActions.js @@ -143,10 +143,10 @@ class MarketsActions { subscribeMarket(base, quote, bucketSize, groupedOrderLimit) { /* - * DataFeed will call subscribeMarket with undefined groupedOrderLimit, - * so we keep track of the last value used and use that instead in that - * case - */ + * DataFeed will call subscribeMarket with undefined groupedOrderLimit, + * so we keep track of the last value used and use that instead in that + * case + */ if (typeof groupedOrderLimit === "undefined") groupedOrderLimit = currentGroupedOrderLimit; else currentGroupedOrderLimit = groupedOrderLimit; @@ -164,16 +164,16 @@ class MarketsActions { return dispatch => { let subscription = (marketId, subResult) => { /* - ** When switching markets rapidly we might receive sub notifications - ** from the previous markets, in that case disregard them - */ + ** When switching markets rapidly we might receive sub notifications + ** from the previous markets, in that case disregard them + */ if (marketId !== currentMarket) { return; } /* In the case of many market notifications arriving at the same time, - * we queue them in a batch here and dispatch them all at once at a frequency - * defined by "subBatchTime" - */ + * we queue them in a batch here and dispatch them all at once at a frequency + * defined by "subBatchTime" + */ if (!dispatchSubTimeout) { subBatchResults = subBatchResults.concat(subResult); @@ -703,7 +703,9 @@ class MarketsActions { } cancelLimitOrder(accountID, orderID) { - // Set the fee asset to use + // FIXME we need a global approach how gee asset id is selected, + // this is only doing it for the cancel action, but ideally, + // all fee selection in the UI have the same logic let fee_asset_id = accountUtils.getFinalFeeAsset( accountID, "limit_order_cancel" @@ -728,17 +730,27 @@ class MarketsActions { console.log("cancelLimitOrders", accountID, orderIDs); } let tr = WalletApi.new_transaction(); + let balances = accountUtils.getAccountBalances(accountID); for (let i = 0; i < orderIDs.length; i++) { let id = orderIDs[i]; let fallbackFeeAsset = typeof fallbackFeeAssets === "string" ? fallbackFeeAssets : fallbackFeeAssets[i]; + let {fees} = accountUtils.getPossibleFees( + accountID, + "limit_order_cancel" + ); let fee_asset_id = accountUtils.getFinalFeeAsset( accountID, "limit_order_cancel", fallbackFeeAsset ); + balances[fee_asset_id] -= fees[fee_asset_id]; + // check balance + Object.keys(balances).forEach(key => { + if (balances[key] < 0) throw "Insufficient balance: " + key; + }); tr.add_type_operation("limit_order_cancel", { fee: { amount: 0, @@ -756,9 +768,9 @@ class MarketsActions { cancelLimitOrderSuccess(ids) { return dispatch => { /* In the case of many cancel orders being issued at the same time, - * we batch them here and dispatch them all at once at a frequency - * defined by "dispatchCancelTimeout" - */ + * we batch them here and dispatch them all at once at a frequency + * defined by "dispatchCancelTimeout" + */ if (!dispatchCancelTimeout) { cancelBatchIDs = cancelBatchIDs.concat(ids); dispatchCancelTimeout = setTimeout(() => { diff --git a/app/components/Exchange/MyOpenOrders.jsx b/app/components/Exchange/MyOpenOrders.jsx index e80c864e95..54d5ba2505 100644 --- a/app/components/Exchange/MyOpenOrders.jsx +++ b/app/components/Exchange/MyOpenOrders.jsx @@ -11,6 +11,7 @@ import {LimitOrder, CallOrder} from "common/MarketClasses"; import ReactTooltip from "react-tooltip"; import {Button} from "bitshares-ui-style-guide"; import {MarketsOrderView, MarketOrdersRowView} from "./View/MarketOrdersView"; +import NotificationActions from "actions/NotificationActions"; class MarketOrdersRow extends React.Component { shouldComponentUpdate(nextProps) { @@ -229,7 +230,12 @@ class MarketOrders extends React.Component { this.resetSelected(); }) .catch(err => { - console.log("cancel orders error:", err); + if ( + typeof err === "string" && + err.startsWith("Insufficient balance") + ) + NotificationActions.error(err); + else console.log("cancel orders error:", err); }); }); } diff --git a/app/lib/common/account_utils.js b/app/lib/common/account_utils.js index 62aafb49f4..3ffd979b63 100644 --- a/app/lib/common/account_utils.js +++ b/app/lib/common/account_utils.js @@ -7,6 +7,7 @@ import { scamAccountsBittrex, scamAccountsOther } from "./scamAccounts"; +import SettingsStore from "stores/SettingsStore"; export default class AccountUtils { /** @@ -29,8 +30,14 @@ export default class AccountUtils { return feePool >= fee; } - static getPossibleFees(account, operation) { + /** + * Returns all assets that the user can actually use to pay the fee given by the operation. + * This estimates the fee in core asset, checks user balance and fee pool of all balances + * and decides which one may be used. + */ + static getPossibleFees(account, operation, raiseIfInsufficient = false) { let core = ChainStore.getAsset("1.3.0"); + account = !account || account.toJS ? account : ChainStore.getAccount(account); @@ -46,18 +53,15 @@ export default class AccountUtils { let fee = estimateFee(operation, null, globalObject); let accountBalances = account.get("balances"); - if (!accountBalances) { - return {assets: ["1.3.0"], fees: {"1.3.0": 0}}; + if (!accountBalances || Object.keys(accountBalances).length == 0) { + return {assets: [], fees: {}}; } - accountBalances.forEach((balanceID, assetID) => { - let balanceObject = ChainStore.getObject(balanceID); - let balance = balanceObject - ? parseInt(balanceObject.get("balance"), 10) - : 0; + for (const [assetID, balance] of Object.entries( + this.getAccountBalances(account) + )) { let hasBalance = false, eqFee; - if (assetID === "1.3.0" && balance >= fee) { hasBalance = true; } else if (balance && ChainStore.getAsset(assetID)) { @@ -84,23 +88,71 @@ export default class AccountUtils { assets.push(assetID); fees[assetID] = eqFee ? eqFee : fee; } - }); + } + + // if requested, raise exception if no fees found + if (raiseIfInsufficient && assets.length == 0) { + throw "Insufficient balance for fee"; + } return {assets, fees}; } - static getFinalFeeAsset(account, operation, fee_asset_id = "1.3.0") { - let {assets: feeAssets} = this.getPossibleFees(account, operation); - if (feeAssets.length === 1) { - fee_asset_id = feeAssets[0]; - } else if ( - feeAssets.length > 0 && - feeAssets.indexOf(fee_asset_id) === -1 - ) { - fee_asset_id = feeAssets[0]; + /** + * Returns the fee asset id that can be used to pay for the fee. + * It will try to use the globally set preference, and if not possible the given + * preference (usually defaults to core asset), and if also not possible, + * just any asset of the user that has sufficient balance and has a funded feepool. + * If nothing is available, it returns the globally set default, or raises an error. + */ + static getFinalFeeAsset( + account, + operation, + feeAssetId = "1.3.0", + raiseIfInsufficient = false + ) { + // user can set a default in the settings + let default_fee_asset_symbol = SettingsStore.getSetting("fee_asset"); + let default_fee_asset = ChainStore.getAsset( + default_fee_asset_symbol + ).toJS(); + let {assets: feeAssets} = this.getPossibleFees( + account, + operation, + raiseIfInsufficient + ); + if (feeAssets.length > 0) { + if ( + feeAssets.indexOf(default_fee_asset.id) !== -1 + ) { + return default_fee_asset.id; + } else if ( + feeAssets.indexOf(feeAssetId) !== -1 + ) { + return feeAssetId; + } else { + // take any that allows to pay the fee + return feeAssets[0]; + } + } else { + // can't pay fee, show user his chosen default + return default_fee_asset.id; } + } - return fee_asset_id; + static getAccountBalances(account) { + account = + !account || account.toJS ? account : ChainStore.getAccount(account); + let accountBalances = account.get("balances"); + let balances = {}; + accountBalances.forEach((balanceID, assetID) => { + let balanceObject = ChainStore.getObject(balanceID); + let balance = balanceObject + ? parseInt(balanceObject.get("balance"), 10) + : 0; + balances[assetID] = balance; + }); + return balances; } static isKnownScammer(account) {