fix: fix fees calculation and add support for L1 fees for Optimism transactions #19603
Conversation
Jenkins BuildsClick to see older builds (27)
|
| @@ -243,8 +243,9 @@ | |||
| (<= input-num-value 0) | |||
| (> input-num-value (current-limit))) | |||
| amount-text (str @input-value " " token-symbol) | |||
| first-route (first route) | |||
There was a problem hiding this comment.
We use the first route to identify the native token symbol, which is the token used to pay the fees. This should always be the same token (ETH) across all routes as long as we're only supporting Layer 2s and the mainnet, and not including sidechains or alternative chains. Therefore, selecting the first element as the basis for this information should be entirely safe.
| (let [gas-amount (money/bignumber (get data :gas-amount)) | ||
| gas-fees (get data :gas-fees) | ||
| eip1559-enabled? (get gas-fees :eip-1559-enabled) | ||
| optimal-price-gwei (money/bignumber (if eip1559-enabled? | ||
| (get gas-fees :max-fee-per-gas-medium) | ||
| (get gas-fees :gas-price))) | ||
| total-gas-fee-wei (money/mul (money/->wei :gwei optimal-price-gwei) gas-amount) | ||
| l1-fee-wei (money/->wei :gwei (get gas-fees :l-1-gas-fee))] | ||
| (money/add total-gas-fee-wei l1-fee-wei))) |
There was a problem hiding this comment.
Used the same logic from status-desktop, values are received in gwei and we transform them to wei.
|
|
||
| (defn calculate-full-route-gas-fee | ||
| [route] | ||
| (reduce money/add (map calculate-gas-fee route))) | ||
| (money/wei->ether (reduce money/add (map calculate-gas-fee route)))) |
There was a problem hiding this comment.
Sums all the routes fees in wei and then convert the total value to ether
There was a problem hiding this comment.
I think it maybe worth it adding that comment "Sums all the routes fees in wei and then convert the total value to ether" as a docstring to the function
| (get gas-fees :max-fee-per-gas-medium) | ||
| (get gas-fees :gas-price))) | ||
| total-gas-fee-wei (money/mul (money/->wei :gwei optimal-price-gwei) gas-amount) | ||
| l1-fee-wei (money/->wei :gwei (get gas-fees :l-1-gas-fee))] |
There was a problem hiding this comment.
l-1-gas-fee should be 0 unless it is an Optimism (or Optimism Sepolia) transaction.
For more context: https://docs.optimism.io/stack/transactions/fees#l1-data-fee
briansztamfater
force-pushed
the
fix/fee-calculation
branch
4 times, most recently
from
48ad7c7
to
cb56042
Compare
briansztamfater
changed the title
briansztamfater
requested review from
shivekkhurana,
vkjr,
FFFra,
J-Son89,
OmarBasem and
ulisesmac
88% of end-end tests have passedFailed tests (5)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (46)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
|
|
|
||
| (defn calculate-full-route-gas-fee | ||
| [route] | ||
| (reduce money/add (map calculate-gas-fee route))) | ||
| (money/wei->ether (reduce money/add (map calculate-gas-fee route)))) |
There was a problem hiding this comment.
I think it maybe worth it adding that comment "Sums all the routes fees in wei and then convert the total value to ether" as a docstring to the function
briansztamfater
force-pushed
the
fix/fee-calculation
branch
2 times, most recently
from
53d6a55
to
b767983
Compare
VolodLytvynenko
force-pushed
the
fix/fee-calculation
branch
from
b767983
to
b7fa37a
Compare
briansztamfater
force-pushed
the
fix/fee-calculation
branch
from
b7fa37a
to
11a0ebd
Compare
|
Hi @briansztamfater thank you for PR. I think the PR can be merged since all information related to gas fees fetches is implemented in status go. However, please address any issues if you see that the problem is on your side. I have created a comparison table with gas fees using Metamask:
In conclusion, regarding above the table:
How gas fee is estimated in Optimism in Status: |
QuestionShould the gas fee be fetched on the confirmation page which is shown when collectibles or tokens are going to be sent? Currently, it doesn't |
|
Hey @VolodLytvynenko, thanks for testing! We have an issue for that: #18408 👍 |
|
@VolodLytvynenko quick question, do you experience same fees values when trying those transactions in Status Desktop? |
@briansztamfater hey. Unfortunately can't check it on a desktop. All transactions related to L2 just don't work there, routes are not fetched. In case of L1 the fees are the same as on mobile |
briansztamfater
force-pushed
the
fix/fee-calculation
branch
from
11a0ebd
to
44ad8dc
Compare
Ok, no problem, I will merge this so we can have at least some fees displayed, if there are some things to be adjusted, probably will be adjusted on status-go, we can fill another issue if necessary for mobile in the future. |
…ansactions Signed-off-by: Brian Sztamfater <brian@status.im>
briansztamfater
force-pushed
the
fix/fee-calculation
branch
from
44ad8dc
to
ae90df0
Compare
fixes #18780
fixes #19431
Summary
This PR fixes fees calculation and adds support for Optimism L1 fees.
cc @FFFra
Testing notes
Platforms
Areas that maybe impacted
Functional
Steps to test
General fees fix testing
For Optimism specific L1 fees
status: ready