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

feat(addAmountOutInSentTransactions)_: Add amount out in NewRouterSentTransaction function #5995

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

Khushboo-dev-cpp
Copy link
Contributor

A short summary which serves as a squashed-commit message.

A description to understand introduced changes without reading the code.

Important changes:

  • Something worth noting for reviewers.

Closes #

@status-im-auto
Copy link
Member

status-im-auto commented Oct 28, 2024

Jenkins Builds

Click to see older builds (23)
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ bc67541 #1 2024-10-28 12:35:30 ~3 min tests 📄log
✔️ bc67541 #1 2024-10-28 12:36:57 ~5 min macos 📦zip
✔️ bc67541 #1 2024-10-28 12:37:11 ~5 min linux 📦zip
✔️ bc67541 #1 2024-10-28 12:37:49 ~6 min ios 📦zip
✔️ bc67541 #1 2024-10-28 12:37:55 ~6 min android 📦aar
✔️ bc67541 #1 2024-10-28 12:38:06 ~6 min tests-rpc 📄log
✖️ bc67541 #1 2024-10-28 12:42:53 ~11 min windows 📦zip
✔️ 4670752 #2 2024-10-30 16:50:18 ~5 min linux 📦zip
✔️ 4670752 #2 2024-10-30 16:50:24 ~5 min macos 📦zip
✔️ 4670752 #2 2024-10-30 16:50:32 ~5 min android 📦aar
✔️ 4670752 #2 2024-10-30 16:51:08 ~6 min ios 📦zip
✔️ 4670752 #2 2024-10-30 16:51:45 ~6 min tests-rpc 📄log
✔️ 4670752 #2 2024-10-30 16:55:15 ~10 min macos 📦zip
✖️ 4670752 #2 2024-10-30 16:56:09 ~11 min windows 📦zip
✔️ 4670752 #2 2024-10-30 17:19:28 ~34 min tests 📄log
✔️ 266303f #3 2024-10-31 08:45:27 ~5 min macos 📦zip
✔️ 266303f #3 2024-10-31 08:45:38 ~5 min linux 📦zip
✔️ 266303f #3 2024-10-31 08:45:51 ~5 min android 📦aar
✔️ 266303f #3 2024-10-31 08:46:07 ~6 min ios 📦zip
✔️ 266303f #3 2024-10-31 08:46:18 ~6 min tests-rpc 📄log
✔️ 266303f #3 2024-10-31 08:48:17 ~8 min macos 📦zip
✖️ 266303f #3 2024-10-31 08:51:29 ~11 min windows 📦zip
✔️ 266303f #3 2024-10-31 09:14:33 ~34 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ e4dc175 #4 2024-10-31 09:08:11 ~5 min linux 📦zip
✔️ e4dc175 #4 2024-10-31 09:08:20 ~5 min macos 📦zip
✔️ e4dc175 #4 2024-10-31 09:08:35 ~5 min android 📦aar
✖️ e4dc175 #4 2024-10-31 09:08:56 ~5 min tests-rpc 📄log
✔️ e4dc175 #4 2024-10-31 09:09:11 ~6 min ios 📦zip
✔️ e4dc175 #4 2024-10-31 09:12:43 ~9 min macos 📦zip
✖️ e4dc175 #4 2024-10-31 09:14:05 ~10 min windows 📦zip
✔️ e4dc175 #4 2024-10-31 09:48:54 ~34 min tests 📄log
✔️ bb9d78e #5 2024-10-31 15:19:26 ~4 min tests-rpc 📄log
✔️ bb9d78e #5 2024-10-31 15:20:43 ~5 min macos 📦zip
✔️ bb9d78e #5 2024-10-31 15:21:03 ~5 min windows 📦zip
✔️ bb9d78e #5 2024-10-31 15:21:06 ~5 min android 📦aar
✔️ bb9d78e #5 2024-10-31 15:22:56 ~7 min macos 📦zip
✔️ bb9d78e #5 2024-10-31 15:48:45 ~33 min tests 📄log

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 47.49%. Comparing base (097d19a) to head (e4dc175).
Report is 14 commits behind head on develop.

Files with missing lines Patch % Lines
...vices/wallet/transfer/transaction_manager_route.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5995      +/-   ##
===========================================
+ Coverage    47.46%   47.49%   +0.02%     
===========================================
  Files          849      849              
  Lines       138611   138613       +2     
===========================================
+ Hits         65794    65830      +36     
+ Misses       65017    64997      -20     
+ Partials      7800     7786      -14     
Flag Coverage Δ
functional 10.24% <50.00%> (+<0.01%) ⬆️
unit 46.98% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
services/wallet/responses/router_transactions.go 100.00% <100.00%> (ø)
...vices/wallet/transfer/transaction_manager_route.go 43.00% <0.00%> (-0.22%) ⬇️

... and 27 files with indirect coverage changes

@@ -43,7 +43,8 @@ type RouterSentTransaction struct {
ToChain uint64 `json:"toChain"`
FromToken string `json:"fromToken"`
ToToken string `json:"toToken"`
Amount string `json:"amount"` // amount of the transaction
Amount string `json:"amount"` // amount of the transaction
AmountOut string `json:"amountOut"` // amount of the transaction sent
Copy link
Contributor

Choose a reason for hiding this comment

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

@Khushboo-dev-cpp why do we need the amount out? It is supposed to be used internally only on the statusgo side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also RouterSentTransaction refers to a single tx sent, but SendDetails contains info about all general data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saledjenic the reason why I added this is because i case of Swaps we need a from token amount to swap and the actual to token amount that will be received by the swap.

I am not sure I understand the difference between individual tx's and the SendDetails and when what should be used.

I tried to add the amountOut to the SendDetails but it seems like the SendDetails are filled =with the input values from the client where this amount out is not available. Please correct me if I am wrong. but thats the reason I added this new param in the RouterSentTransaction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically SendDetails contains global details, what was the intention of the user, and corresponds to the router input params. A single send is composed of one or more transactions, so each RouterSentTransaction corresponds to each transaction that was sent. Worth noticing that an approval tx is also a single RouterSentTransaction, it means if you want to swap erc20 token you will have 2 RouterSentTransaction, one for approval, one for swap.

services/wallet/responses/router_transactions.go Outdated Show resolved Hide resolved
services/wallet/responses/router_transactions.go Outdated Show resolved Hide resolved
@Khushboo-dev-cpp Khushboo-dev-cpp marked this pull request as ready for review October 31, 2024 08:42
@status-im-auto
Copy link
Member

✔️ status-go/prs/linux/x86_64/main/PR-5995#5 🔹 ~5 min 45 sec 🔹 bb9d78e 🔹 📦 linux package

@status-im-auto
Copy link
Member

✔️ status-go/prs/ios/PR-5995#5 🔹 ~8 min 34 sec 🔹 bb9d78e 🔹 📦 ios package

@alaibe alaibe merged commit c4bb706 into develop Nov 1, 2024
14 of 15 checks passed
@alaibe alaibe deleted the feat/addAmountOut branch November 1, 2024 11:29
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.

4 participants