-
Notifications
You must be signed in to change notification settings - Fork 570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for *default fee paying asset* in ApplicationApi #3540
Conversation
User had reported that *feeAsset* is turning into *liquidity pool asset* while attempting to perform withdraw from a liquidity pool, seems bug is effecting objects.feeAsset.get("id") default object at liquidity pools withdraw *modal* which should show feeAsset as core asset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually an old issue. I don't think the fix is appropriate though. See #2969
I know what you mean hence for now mostly people are paying liquidity pools transaction fees in BTS. I am considering this: #3001 (comment) I believe its ready but will need to check the bug @sschiessl-bcp is referring to. |
@sschiessl-bcp please pass this for now as some users are not able to pay fees with BTS. Note: this change is effecting only liquidity removal operation of liquidity pool transaction fees. |
Actually I think we should extend the fix in #3492 to all operations: call |
Hello @sschiessl-bcp; I'd just checked and such a change would require complex work, for now please merge this fix as it's preventing some liquidity pool users to pay with BTS. Perhaps another feature implementation like this open case be created later #3088 |
@ioBanker see my comment #3540 (comment)
Don't merge this. Please fix it. |
Fixed and was tested. Credits to @thontron for tests |
accountUtils.getFinalFeeAsset(account,"liquidity_pool_withdraw") is successfully working It can be merged now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the actual code change, I see the following:
In ApplicationAPI you define
bitshares-ui/app/api/ApplicationApi.js
Lines 989 to 996 in 9e4d1d8
async liquidityPoolWithdraw( | |
account, | |
liquidityPoolId, | |
shareAsset, | |
shareAmount, | |
feeAsset = "1.3.0", | |
broadcast = true | |
) { |
but when calling it you are using
bitshares-ui/app/components/Modal/PoolStakeModal.jsx
Lines 110 to 120 in 9e4d1d8
ApplicationApi.liquidityPoolWithdraw( | |
account, | |
pool.get("id"), | |
pool.getIn(['share_asset', "symbol"]), | |
Math.floor(Number(shareAssetAmount) * Number(sharedAssetPrecision)), | |
pool.getIn(['asset_a', "symbol"]), | |
pool.getIn(['asset_b', "symbol"]), | |
Math.floor(Number(assetAAmount) * Number(assetAPrecision)), | |
Math.floor(Number(assetBAmount) * Number(assetBPrecision)) | |
) |
Please fix the function arguments and implement the fix according to
fee_asset_id = accountUtils.getFinalFeeAsset( |
Good catch; liquidityPoolWithdraw arguments are wrong as assetA and assetB shouldn't be mentioned for this operation unlike deposit, will fix this soon later. |
Below operations execution should be tested while setting a different asset "that has BTS in fee pool" prior merging this: account_create |
transfer - Tested I'd merged this change into dex.iobanker.com , please help with testing the other operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
I've changed the behavior to "if fee asset given, then use this, if not, use getFinalFeeAsset" |
if (!feeAsset) { | ||
// use default fee asset selection if none given | ||
feeAsset = accountUtils.getFinalFeeAsset( | ||
from, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be to
.
User had reported that feeAsset is turning into liquidity pool asset while attempting to perform withdraw from a liquidity pool, seems bug is effecting objects.feeAsset.get("id") default object at liquidity pools withdraw modal which should show feeAsset as core asset.
Before fixing the bug:
After fixing the bug: