Swap: use correct fee on spending cap screen #21532

vkjr · 2024-10-30T11:22:15Z

fixes #21428

Summary

Incorrect fee was used before, that is fixed.
Also minor refactoring done

Review notes

get-standard-fiat-format was refactored, explanation in comment to the code

Testing notes

Platforms

Android
iOS

Areas that maybe impacted

Functional

wallet / transactions

status: ready

vkjr · 2024-10-30T11:25:24Z

src/status_im/contexts/wallet/common/utils.cljs

-(defn get-standard-fiat-format
-  [crypto-value currency-symbol fiat-value]
-  (if (string/includes? crypto-value "<")
+(defn fiat-formatted-for-ui


This function was renamed because we don't have any "standard" fiat format) But naming it as "ui"-related makes it clear that result shouldnt be used in calculations.
Also for some reason we were passing here crypto value and checking if it includes "<" sign. We actually don't need this because we have a fiat value. So extra argument removed.

status-im-auto · 2024-10-30T11:25:37Z

Jenkins Builds

Click to see older builds (8)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`806a70b`	#1	2024-10-30 11:25:37	~3 min	`tests`	📄`log`
✔️	`806a70b`	#1	2024-10-30 11:31:38	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`806a70b`	#1	2024-10-30 11:32:10	~9 min	`android`	🤖`apk` 📲
✔️	`806a70b`	#1	2024-10-30 11:33:08	~10 min	`ios`	📱`ipa` 📲

✔️	`94cbe69`	#2	2024-10-30 11:54:58	~5 min	`tests`	📄`log`
✔️	`94cbe69`	#2	2024-10-30 11:58:00	~8 min	`android-e2e`	🤖`apk` 📲
✔️	`94cbe69`	#2	2024-10-30 11:58:30	~8 min	`android`	🤖`apk` 📲
✔️	`94cbe69`	#2	2024-10-30 11:59:44	~9 min	`ios`	📱`ipa` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`a7f4f92`	#3	2024-10-31 10:07:18	~4 min	`tests`	📄`log`
✔️	`a7f4f92`	#3	2024-10-31 10:10:56	~8 min	`android-e2e`	🤖`apk` 📲
✔️	`a7f4f92`	#3	2024-10-31 10:11:23	~8 min	`android`	🤖`apk` 📲
✔️	`a7f4f92`	#3	2024-10-31 10:14:02	~11 min	`ios`	📱`ipa` 📲
❌	`a7f4f92`	#4	2024-11-08 13:22:36	~2 min	`ios`	📄`log`

❌	`11e15be`	#5	2024-11-08 13:32:22	~2 min	`ios`	📄`log`
✔️	`11e15be`	#4	2024-11-08 13:34:11	~4 min	`tests`	📄`log`
✔️	`11e15be`	#4	2024-11-08 13:37:32	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`11e15be`	#4	2024-11-08 13:37:59	~8 min	`android`	🤖`apk` 📲

vkjr · 2024-10-30T11:26:34Z

src/status_im/contexts/wallet/swap/set_spending_cap/view.cljs

@@ -198,7 +197,7 @@
        :network-image (:source network)}]
      [data-item
       {:title    (i18n/label :t/max-fees)
-        :subtitle (if (and estimated-time max-fees) max-fees (i18n/label :t/unknown))
+        :subtitle (if (and estimated-time approval-fees) approval-fees (i18n/label :t/unknown))


This is an actual bugfix.

briansztamfater

LGTM

ulisesmac · 2024-11-01T15:37:19Z

src/status_im/subs/wallet/swap.cljs

+         token-for-fees (first (filter #(= (string/lower-case (:symbol %))
+                                           (string/lower-case constants/token-for-fees-symbol))
+                                       tokens))


nit: Consider using some instead of first + filter

ulisesmac

Thank you so much for you PR @vkjr !!

status-im-auto · 2024-11-07T14:38:38Z

50% of end-end tests have passed

Total executed tests: 8
Failed tests: 4
Expected to fail tests: 0
Passed tests: 4

IDs of failed tests: 727230,703133,727229,702843

Failed tests (4)

Click to expand

Rerun failed tests

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
`Test setup failed: critical/test_wallet.py:22: in prepare_devices self.drivers, self.loop = create_shared_drivers(2) base_test_case.py:330: in create_shared_drivers raise e base_test_case.py:320: in create_shared_drivers test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1 '_asyncio.Future' object has no attribute 'session_id'`
2. test_wallet_send_eth, id: 727229
`Test setup failed: critical/test_wallet.py:22: in prepare_devices self.drivers, self.loop = create_shared_drivers(2) base_test_case.py:330: in create_shared_drivers raise e base_test_case.py:320: in create_shared_drivers test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1 '_asyncio.Future' object has no attribute 'session_id'`

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

# STEP: Check that removed user is not shown in the list anymore
Device 1: Wait for element Button for max 30s and click when it is available

critical/chats/test_public_chat_browsing.py:240: in test_restore_multiaccount_with_waku_backup_remove_switch
    self.sign_in.show_profiles_button.wait_and_click()
../views/base_element.py:100: in wait_and_click
    self.wait_for_visibility_of_element(sec)
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 1: Button by accessibility id:`show-profiles` is not found on the screen after wait_for_visibility_of_element

Device sessions

Device 1:

Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843

Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='chat-item']//android.widget.TextView[contains(@text,'https://status.app/c/')]
Device 2: Wait for element Button for max 120s and click when it is available

Test setup failed: critical/chats/test_public_chat_browsing.py:350: in prepare_devices
    self.community_2.join_community()
../views/chat_view.py:420: in join_community
    self.join_button.wait_and_click(120)
../views/base_element.py:100: in wait_and_click
    self.wait_for_visibility_of_element(sec)
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 2: Button by accessibility id:`show-request-to-join-screen-button` is not found on the screen after wait_for_visibility_of_element

Device sessions

Device 1:

Steps, video, logs

Device 2:

Steps, video, logs

Passed tests (4)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231

2. test_wallet_balance_mainnet, id: 740490

Class TestCommunityOneDeviceMerged:

1. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

clauxx · 2024-10-31T14:30:22Z

src/status_im/subs/wallet/swap.cljs

@@ -160,6 +159,7 @@
                                                                           display-decimals))]
     (or receive-amount 0))))

+


extra space

status-im-auto · 2024-11-08T13:25:29Z

0% of end-end tests have passed

Total executed tests: 4
Failed tests: 4
Expected to fail tests: 0
Passed tests: 0

IDs of failed tests: 727230,703133,702843,727229

Failed tests (4)

Click to expand

Rerun failed tests

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
Test setup failed: critical/test_wallet.py:28: in prepare_devices self.loop.run_until_complete( /usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete return future.result() __init__.py:52: in run_in_parallel returns.append(await k) /usr/lib/python3.10/concurrent/futures/thread.py:58: in run result = self.fn(self.args, self.kwargs) ../views/sign_in_view.py:297: in recover_access self.terms_and_privacy_checkbox.click() ../views/base_element.py:90: in click element = self.find_element() ../views/base_element.py:79: in find_element raise NoSuchElementException( Device 1: Button by xpath: `//[@content-desc='terms-privacy-checkbox-container']/*[@content-desc='checkbox-off']` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
2. test_wallet_send_eth, id: 727229
## Recover access(password:qwerty1234, keycard:False) Device 2: Find `Button` by `xpath`: `//[@content-desc='terms-privacy-checkbox-container']/[@content-desc='checkbox-off']` Test setup failed: critical/test_wallet.py:28: in prepare_devices self.loop.run_until_complete( /usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete return future.result() __init__.py:52: in run_in_parallel returns.append(await k) /usr/lib/python3.10/concurrent/futures/thread.py:58: in run result = self.fn(self.args, self.kwargs) ../views/sign_in_view.py:297: in recover_access self.terms_and_privacy_checkbox.click() ../views/base_element.py:90: in click element = self.find_element() ../views/base_element.py:79: in find_element raise NoSuchElementException( Device 1: Button by xpath: `//[@content-desc='terms-privacy-checkbox-container']/*[@content-desc='checkbox-off']` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

# STEP: Check that removed user is not shown in the list anymore
Device 1: Wait for element Button for max 30s and click when it is available

critical/chats/test_public_chat_browsing.py:240: in test_restore_multiaccount_with_waku_backup_remove_switch
    self.sign_in.show_profiles_button.wait_and_click()
../views/base_element.py:100: in wait_and_click
    self.wait_for_visibility_of_element(sec)
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 1: Button by accessibility id:`show-profiles` is not found on the screen after wait_for_visibility_of_element

Device sessions

Device 1:

Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843

## Creating new multiaccount (password:'qwerty1234', keycard:'False', enable_notification: 'False')
Device 2: Find Button by xpath: //*[@content-desc='terms-privacy-checkbox-container']/*[@content-desc='checkbox-off']

Test setup failed: critical/chats/test_public_chat_browsing.py:314: in prepare_devices
    self.loop.run_until_complete(run_in_parallel(((self.device_1.create_user, {'enable_notifications': True,
/usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete
    return future.result()
__init__.py:52: in run_in_parallel
    returns.append(await k)
/usr/lib/python3.10/concurrent/futures/thread.py:58: in run
    result = self.fn(*self.args, **self.kwargs)
../views/sign_in_view.py:246: in create_user
    self.terms_and_privacy_checkbox.click()
../views/base_element.py:90: in click
    element = self.find_element()
../views/base_element.py:79: in find_element
    raise NoSuchElementException(
 Device 1: Button by xpath: `//*[@content-desc='terms-privacy-checkbox-container']/*[@content-desc='checkbox-off']` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Device sessions

Device 1:

Steps, video, logs

Device 2:

Steps, video, logs

VolodLytvynenko · 2024-11-08T15:22:45Z

hi @vkjr thank you for PR.
On the swap page, it seems the fee is not being calculated as 'approval fee + swap fee.'

In the table below

When I enter a non-approved value (5.989 DAI) on mobile, the difference between non-approved and approved ERC-20 (5.98 DAI) is only about 90 cents . However the approval fee for this transaction is approximately $4.

I perfomed four tests on both Desktop and mobile to minimize the impact of fee amount changes.

On Desktop, where it appears to work correctly, the non-approved value is around $4 less than the approved value. Could you take a look at this?

Steps:

Go to swap
Enter non-approved value
Enter appoved value
Check fee between non-approved and approved ERC20

Actual result:

seems on the swap fee for non-approved in calculated incorrectly or calculated including only swap fee

mobile.mp4

Expected result:

On the swap fee for non-approved includes approval fee + swap fee

vkjr · 2024-11-11T14:35:19Z

@VolodLytvynenko, thanks for checking, will take a look

vkjr self-assigned this Oct 30, 2024

vkjr commented Oct 30, 2024

View reviewed changes

vkjr requested review from briansztamfater, shivekkhurana, clauxx, ulisesmac and smohamedjavid October 30, 2024 11:26

briansztamfater approved these changes Oct 30, 2024

View reviewed changes

vkjr added the request-manual-qa label Oct 30, 2024

vkjr force-pushed the approval-fees-not-shown branch from 94cbe69 to a7f4f92 Compare October 31, 2024 10:02

smohamedjavid approved these changes Nov 1, 2024

View reviewed changes

ulisesmac reviewed Nov 1, 2024

View reviewed changes

ulisesmac approved these changes Nov 1, 2024

View reviewed changes

clauxx approved these changes Nov 8, 2024

View reviewed changes

src/status_im/subs/wallet/swap.cljs

@@ -160,6 +159,7 @@

display-decimals))]

(or receive-amount 0))))

Copy link

Member

clauxx Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space

VolodLytvynenko self-assigned this Nov 8, 2024

vkjr added 3 commits November 8, 2024 14:29

Approval fees shown instead of swap fees

2a92047

Rename function and get rid of unnecessary argument

b73cc44

lint fix

11e15be

VolodLytvynenko force-pushed the approval-fees-not-shown branch from a7f4f92 to 11e15be Compare November 8, 2024 13:29

VolodLytvynenko added the Tested - Issues label Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swap: use correct fee on spending cap screen #21532

Swap: use correct fee on spending cap screen #21532

vkjr commented Oct 30, 2024 •

edited

Loading

vkjr Oct 30, 2024

status-im-auto commented Oct 30, 2024 •

edited

Loading

vkjr Oct 30, 2024

briansztamfater left a comment

ulisesmac Nov 1, 2024

ulisesmac left a comment

status-im-auto commented Nov 7, 2024

Class TestWalletMultipleDevice:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

Class TestWalletOneDevice:

Class TestCommunityOneDeviceMerged:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

clauxx Oct 31, 2024

status-im-auto commented Nov 8, 2024

Class TestWalletMultipleDevice:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

VolodLytvynenko commented Nov 8, 2024 •

edited

Loading

vkjr commented Nov 11, 2024

		@@ -160,6 +159,7 @@
		display-decimals))]
		(or receive-amount 0))))

Swap: use correct fee on spending cap screen #21532

Are you sure you want to change the base?

Swap: use correct fee on spending cap screen #21532

Conversation

vkjr commented Oct 30, 2024 • edited Loading

Summary

Review notes

Testing notes

Platforms

Areas that maybe impacted

Functional

vkjr Oct 30, 2024

Choose a reason for hiding this comment

status-im-auto commented Oct 30, 2024 • edited Loading

Jenkins Builds

vkjr Oct 30, 2024

Choose a reason for hiding this comment

briansztamfater left a comment

Choose a reason for hiding this comment

ulisesmac Nov 1, 2024

Choose a reason for hiding this comment

ulisesmac left a comment

Choose a reason for hiding this comment

status-im-auto commented Nov 7, 2024

50% of end-end tests have passed

Failed tests (4)

Class TestWalletMultipleDevice:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

Passed tests (4)

Class TestWalletOneDevice:

Class TestCommunityOneDeviceMerged:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

clauxx Oct 31, 2024

Choose a reason for hiding this comment

status-im-auto commented Nov 8, 2024

0% of end-end tests have passed

Failed tests (4)

Class TestWalletMultipleDevice:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

VolodLytvynenko commented Nov 8, 2024 • edited Loading

Steps:

Actual result:

Expected result:

vkjr commented Nov 11, 2024

vkjr commented Oct 30, 2024 •

edited

Loading

status-im-auto commented Oct 30, 2024 •

edited

Loading

VolodLytvynenko commented Nov 8, 2024 •

edited

Loading