Skip to content
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

chore: improvements of the sending route generated by the router process #16330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

saledjenic
Copy link
Contributor

@saledjenic saledjenic commented Sep 13, 2024

Corresponding status-go PR:

This PR is the first step in the improvement planned for the desktop app. Next steps:

  • move buying/releasing ens name and stickers to the same flow as sending transactions (statusgo part is already ready to support that) and that change will remove a big portion of code from the desktop app
  • improve the notification mechanism in the desktop app (transaction sent, transaction confirmed/pending/failed)
  • improve swap to sing the swap a single time only regardless of whether approval is needed or not (for this some more tuning will be needed on the statusgo side)

Note: minimal changes are done in the swap modal part on the qml side just to support new sending transaction flow, further improvements are necessary there

Closes #16335

Affected areas by this change are Send, Bridge and Swap modal, would be good to test the following:

  • Send:
    • eth
    • erc20
    • erc721
    • erc1155
  • Bridge:
    • eth -> eth
    • erc20 -> erc20
  • Swap:
    • eth -> erc20
    • erc20 -> eth
    • erc20 -> erc20

@status-im-auto
Copy link
Member

status-im-auto commented Sep 13, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9b4e506 #1 2024-09-13 11:55:01 ~6 min tests/nim 📄log
✔️ 9b4e506 #1 2024-09-13 11:55:22 ~7 min macos/aarch64 🍎dmg
9b4e506 #1 2024-09-13 11:59:51 ~11 min tests/ui 📄log
✔️ 9b4e506 #1 2024-09-13 11:59:55 ~11 min macos/x86_64 🍎dmg
✔️ 9b4e506 #1 2024-09-13 12:02:50 ~14 min linux-nix/x86_64 📦tgz
✔️ 9b4e506 #1 2024-09-13 12:04:08 ~16 min linux/x86_64 📦tgz
9b4e506 #2 2024-09-13 12:16:57 ~14 min tests/ui 📄log
9b4e506 #3 2024-09-13 12:39:57 ~10 min tests/ui 📄log
✔️ a40b807 #2 2024-09-17 07:50:04 ~6 min tests/nim 📄log
✔️ a40b807 #2 2024-09-17 07:50:30 ~7 min macos/aarch64 🍎dmg
a40b807 #4 2024-09-17 07:54:54 ~11 min tests/ui 📄log
✔️ a40b807 #2 2024-09-17 07:55:17 ~11 min linux-nix/x86_64 📦tgz
✔️ a40b807 #2 2024-09-17 07:56:50 ~13 min macos/x86_64 🍎dmg
✔️ a40b807 #2 2024-09-17 07:57:31 ~14 min linux/x86_64 📦tgz

@status-im-auto
Copy link
Member

@virginiabalducci
Copy link

virginiabalducci commented Sep 13, 2024

Send:

Test user's geth.log geth.log.zip

  • - eth OK
  • - erc20 OK
  • - erc721 OK
  • - erc1155 OK

Swap:

  • - eth -> erc20 OK

  • erc20 -> eth -> I see an error message "Something went wrong. Change amount, token or try again later."
    Screenshot 2024-09-13 at 4 03 01 PM

WRN 2024-09-13 16:00:22.612-03:00 qt warning                                 topics="qt" tid=31777056 text="qrc:/app/AppLayouts/Wallet/WalletUtils.qml:125: TypeError: Cannot call method 'split' of undefined \n             const chainIdsArray = chainIds.split(\":\")" file=:0 category=default
WRN 2024-09-13 16:00:22.612-03:00 qt warning                                 topics="qt" tid=31777056 text="qrc:/app/AppLayouts/Wallet/popups/swap/SwapModalAdaptor.qml:71: TypeError: Cannot read property 'amount' of undefined \n                     expression: model.currencyBalance.amount" file=:0 category=default

app_20240913_120139.log

  • erc20 -> erc20

Screenshot 2024-09-13 at 4 06 43 PM

app_20240913_120139.log

WRN 2024-09-13 16:06:22.956-03:00 qt warning                                 topics="qt" tid=31777056 text="qrc:/app/AppLayouts/Wallet/popups/swap/SwapModalAdaptor.qml:71: TypeError: Cannot read property 'amount' of undefined \n                     expression: model.currencyBalance.amount" file=:0 category=default
DBG 2024-09-13 16:06:32.992-03:00 NewBE_callPrivateRPC                       topics="rpc" tid=31777056 file=core.nim:27 rpc_method=wallet_getSuggestedRoutesAsync

UPD (nastya): i was able to do swap erc20 to erc20 https://etherscan.io//tx//0x19a1eaefbc75ab9ace48987d4807a3addfcba19049a16437a1da7788197c3e1e

UPD (nastya): i was able to do bridge as well

Bridge:

@virginiabalducci
Copy link

testing this PR is blocked due to #16352 because I need to send some funds to bridge and get them to L2, to test the Bridge functionality in the desktop app, and to test swap on L2 networks.

@saledjenic
Copy link
Contributor Author

testing this PR is blocked due to #16352 because I need to send some funds to bridge and get them to L2, to test the Bridge functionality in the desktop app, and to test swap on L2 networks.

@virginiabalducci I added a comment to the issue you logged, also rebased this PR on top of Dario's change for the disabled send button, could you try it out again?

@status-im-auto
Copy link
Member

@anastasiyaig
Copy link
Contributor

@saledjenic i was able to do the remaining part of transactions (swap and bridge). it looks fine
cc @virginiabalducci

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify the sending process after the best route is generated by the router
5 participants