Fix #3486 "Order cancellation fee error"

[xiangxn]

Handle issues mentioned in #3486

Closes #3486

[sschiessl-bcp]

I have the feeling that getPossibleFees2 and getFinalFeeAsset2 are really almost the same. What is the actual difference? It seems to create lots of duplicate code

[sschiessl-bcp]

@xiangxn I have introduced a parameter to getFinalFeeAsset that allows us to not create duplicate code, please have a look at it.
Generally, your getPossibleFees was declared a getter method, but did manipulate the array or balance that the user fed into it. This should be avoided to not confuse the caller

[sschiessl-bcp]

@xiangxn please test my change again and if it still does what you wanted it to do, let's merge

[xiangxn]

@xiangxn I have introduced a parameter to getFinalFeeAsset that allows us to not create duplicate code, please have a look at it. Generally, your getPossibleFees was declared a getter method, but did manipulate the array or balance that the user fed into it. This should be avoided to not confuse the caller

A reference parameter is required here, which will simplify the operation logic, so it is also an independent method to distinguish it from getPossibleFees.

Moreover, the logic here is that a front-end accumulated fee is required, rather than a separate judgment on whether the account balance is sufficient.

Therefore, your code must not meet the requirements of this logic.

[abitmore]

a front-end accumulated fee is required

As I understood, for a transaction which contains multiple operations, we need to sum up fees of all the operations and check if the account balance and the fee pools are sufficient to pay all the fees.

In addition, if an operation would cause a balance change, the change should be counted in when calculating fees.

[xiangxn]

This line of code also causes subscript overflow: https://github.com/xiangxn/bitshares-ui/blob/fix-3486/app/lib/common/account_utils.js#L136

@sschiessl-bcp

[xiangxn]

I reverted the branch and removed the logic of quoting parameters, and also dealt with the problem of not compiling.

@sschiessl-bcp @abitmore

[xiangxn]

getFinalFeeAsset2 is not compatible with getFinalFeeAsset, because one is for batch processing and the other is for single fee checking. If you want to force one, you may only leave getFinalFeeAsset2, and modify a lot of code used by the getFinalFeeAsset version.

My English is terrible, don't know if you can understand what I'm saying?😄

[sschiessl-bcp]

I don't exactly understand yet why we need this complicated logic and duplicate code.

I have restored my change to a new branch so allow comparison, I have also fixed the subscript overflow.
https://github.com/bitshares/bitshares-ui/tree/bugfix/3486_cancel_default_fee_selection

I probably don't understand it correctly at the moment, can you please tell me
a) what behavior it is that you want to have
b) and which case I can test to see how my code does not fulfill it?

In any case, we need to avoid duplicate code if it is at the end just a very small change to the business logic.

[xiangxn]

In this problem, we need to take out the user's balance at one time and accumulate the required fees at the front end. The original method is to take the data on the chain every time, so it cannot achieve the purpose.

I have used the code you provided, modified. @sschiessl-bcp

[sschiessl-bcp]

Ah, now I understand. My way was doing the check only for one op, but we want it for ALL ops in the loop.