Adding support for *default fee paying asset* in ApplicationApi by ioBanker · Pull Request #3540 · bitshares/bitshares-ui

ioBanker · 2022-08-30T14:46:54Z

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:

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.

abitmore

This is actually an old issue. I don't think the fix is appropriate though. See #2969

ioBanker · 2022-09-03T18:19:28Z

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.

ioBanker · 2022-09-03T18:20:26Z

@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.

abitmore · 2022-09-03T19:27:25Z

Actually I think we should extend the fix in #3492 to all operations: call accountUtils.getFinalFeeAsset() to get an available asset (maybe globally in some way).

ioBanker · 2022-09-03T20:40:48Z

Seems already implemented at loan modal; will check the code there if it can be easily implemented here for all

sschiessl-bcp · 2022-09-14T06:21:50Z

Seems already implemented at loan modal; will check the code there if it can be easily implemented here for all

Had time to check?

ioBanker · 2022-09-14T15:22:40Z

Seems already implemented at loan modal; will check the code there if it can be easily implemented here for all

Had time to check?

will be checking this weekend and then update here.

ioBanker · 2022-09-16T07:15:39Z

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

abitmore · 2022-09-16T11:24:14Z

@ioBanker see my comment #3540 (comment)

Actually I think we should extend the fix in #3492 to all operations: call accountUtils.getFinalFeeAsset() to get an available asset (maybe globally in some way).

Don't merge this. Please fix it.

ioBanker · 2022-09-17T10:25:58Z

@ioBanker see my comment #3540 (comment)

Actually I think we should extend the fix in #3492 to all operations: call accountUtils.getFinalFeeAsset() to get an available asset (maybe globally in some way).

Don't merge this. Please fix it.

Fixed and was tested.

Credits to @thontron for tests

app/api/ApplicationApi.js

ioBanker · 2022-09-26T16:19:12Z

accountUtils.getFinalFeeAsset(account,"liquidity_pool_withdraw") is successfully working

It can be merged now.

sschiessl-bcp

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

bitshares-ui/app/actions/AccountActions.js

Line 73 in 9e4d1d8

fee_asset_id = accountUtils.getFinalFeeAsset(

, which has the proper fallback options. Is this maybe the same for deposit?

ioBanker · 2022-10-03T13:45:02Z

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

bitshares-ui/app/actions/AccountActions.js

Line 73 in 9e4d1d8

fee_asset_id = accountUtils.getFinalFeeAsset(

, which has the proper fallback options. Is this maybe the same for deposit?

Good catch; liquidityPoolWithdraw arguments are wrong as assetA and assetB shouldn't be mentioned for this operation unlike deposit, will fix this soon later.

ioBanker · 2022-10-24T19:00:34Z

Below operations execution should be tested while setting a different asset "that has BTS in fee pool" prior merging this:

account_create
transfer
proposal_create
asset_issue
worker_create
withdraw_permission_create
withdraw_permission_update
withdraw_permission_claim
withdraw_permission_delete
vesting_balance_create
ticket_create
liquidity_pool_create
liquidity_pool_delete
liquidity_pool_deposit
liquidity_pool_withdraw
liquidity_pool_exchange

ioBanker · 2022-10-24T19:34:30Z

transfer - Tested
asset_issue - Tested
worker_create - Tested
ticket_create - Tested
liquidity_pool_create - Tested
liquidity_pool_delete - Tested
liquidity_pool_deposit - Tested
liquidity_pool_withdraw - Tested
liquidity_pool_exchange - Tested

I'd merged this change into dex.iobanker.com , please help with testing the other operations.

abitmore

Thanks.

app/api/ApplicationApi.js

sschiessl-bcp · 2022-10-31T14:06:46Z

I've changed the behavior to "if fee asset given, then use this, if not, use getFinalFeeAsset"

abitmore · 2022-10-31T16:17:51Z

app/api/ApplicationApi.js

+        if (!feeAsset) {
+            // use default fee asset selection if none given
+            feeAsset = accountUtils.getFinalFeeAsset(
+                from,


This should be to.

…hares#3540) * Fixing bug in liquidity pools withdraw operation 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. * Use getFinalFeeAsset to determine FeeAsset in LP * Update ApplicationApi.js * Update ApplicationApi.js * Update ApplicationApi.js * Update ApplicationApi.js * fix: honor given fee asset choices with getFinalFeeAsset as fallback Co-authored-by: Stefan <[email protected]>

abitmore reviewed Aug 30, 2022

View reviewed changes

Use getFinalFeeAsset to determine FeeAsset in LP

6fdc5ce

abitmore reviewed Sep 17, 2022

View reviewed changes

app/api/ApplicationApi.js Outdated Show resolved Hide resolved

Update ApplicationApi.js

4c2a2fc

sschiessl-bcp requested changes Oct 3, 2022

View reviewed changes

ioBanker closed this Oct 20, 2022

abitmore reopened this Oct 20, 2022

ioBanker changed the title ~~Fixing bug in liquidity pools withdraw operation~~ Adding support to *default fee paying asset* in ApplicationApi Oct 24, 2022

ioBanker changed the title ~~Adding support to *default fee paying asset* in ApplicationApi~~ Adding support for *default fee paying asset* in ApplicationApi Oct 24, 2022

ioBanker added 2 commits October 24, 2022 21:08

Update ApplicationApi.js

2011a91

Update ApplicationApi.js

3ff510b

ioBanker requested review from abitmore and sschiessl-bcp and removed request for sschiessl-bcp October 29, 2022 16:37

abitmore reviewed Oct 29, 2022

View reviewed changes

app/api/ApplicationApi.js Outdated Show resolved Hide resolved

Update ApplicationApi.js

747c821

fix: honor given fee asset choices with getFinalFeeAsset as fallback

9a8abc5

sschiessl-bcp merged commit 172f015 into bitshares:develop Oct 31, 2022

abitmore reviewed Oct 31, 2022

View reviewed changes

	async liquidityPoolWithdraw(
	account,
	liquidityPoolId,
	shareAsset,
	shareAmount,
	feeAsset = "1.3.0",
	broadcast = true
	) {

	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))

	)

Conversation

ioBanker commented Aug 30, 2022

Uh oh!

abitmore left a comment

Choose a reason for hiding this comment

Uh oh!

ioBanker commented Sep 3, 2022

Uh oh!

ioBanker commented Sep 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abitmore commented Sep 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ioBanker commented Sep 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sschiessl-bcp commented Sep 14, 2022

Uh oh!

ioBanker commented Sep 14, 2022

Uh oh!

ioBanker commented Sep 16, 2022

Uh oh!

abitmore commented Sep 16, 2022

Uh oh!

ioBanker commented Sep 17, 2022

Uh oh!

Uh oh!

ioBanker commented Sep 26, 2022

Uh oh!

sschiessl-bcp left a comment

Choose a reason for hiding this comment

Uh oh!

ioBanker commented Oct 3, 2022

Uh oh!

ioBanker commented Oct 24, 2022

Uh oh!

ioBanker commented Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abitmore left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sschiessl-bcp commented Oct 31, 2022

Uh oh!

abitmore Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ioBanker commented Sep 3, 2022 •

edited

Loading

abitmore commented Sep 3, 2022 •

edited

Loading

ioBanker commented Sep 3, 2022 •

edited

Loading

ioBanker commented Oct 24, 2022 •

edited

Loading