feat: approve token transactions on swap #21076
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (39)
|
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
from
5b2f7df to
ad1fa0b
Compare
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
2 times, most recently
from
94723fb to
4da2d50
Compare
briansztamfater
commented
Aug 22, 2024
| @@ -8,9 +8,17 @@ | |||
| (rf/reg-event-fx | |||
| :wallet/pending-transaction-status-changed-received | |||
| (fn [{:keys [db]} [{:keys [message]}]] | |||
| (let [details (transforms/json->clj message) | |||
| tx-hash (:hash details)] | |||
| {:db (update-in db [:wallet :transactions tx-hash] assoc :status :confirmed :blocks 1)}))) | |||
There was a problem hiding this comment.
:blocks param was not used, and also it didn't make much sense to hardcode it to 1. Ideally, if we need it in the future, we should listen to transaction events and update the value accordingly, but for now it is not needed.
briansztamfater
commented
Aug 22, 2024
Comment on lines
+329
to
+470
| (money/->wei | ||
| :gwei | ||
| max-fee-per-gas-medium)) | ||
| :MaxPriorityFeePerGas | ||
| (money/to-hex | ||
| (money/->wei | ||
| :gwei | ||
| max-priority-fee-per-gas))) | ||
| (not eip-1559-enabled?) (assoc :TxType "0x00" | ||
| :GasPrice | ||
| (money/to-hex | ||
| (money/->wei | ||
| :gwei | ||
| gas-price)))))) | ||
|
|
||
| (defn approval-path | ||
| [{:keys [route from-address to-address token-address]}] | ||
| (let [{:keys [from]} route | ||
| from-chain-id (:chain-id from) | ||
| approval-amount-required (:approval-amount-required route) | ||
| approval-amount-required-sanitized (-> approval-amount-required | ||
| (utils.hex/normalize-hex) | ||
| (native-module/hex-to-number)) | ||
| approval-contract-address (:approval-contract-address route) | ||
| data (native-module/encode-function-call | ||
| constants/contract-function-signature-erc20-approve | ||
| [approval-contract-address | ||
| approval-amount-required-sanitized]) | ||
| tx-data (transaction-data {:from-address from-address | ||
| :to-address to-address | ||
| :token-address token-address | ||
| :route route | ||
| :data data | ||
| :eth-transfer? false})] | ||
| {:BridgeName constants/bridge-name-transfer | ||
| :ChainID from-chain-id | ||
| :TransferTx tx-data})) | ||
|
|
||
| (defn transaction-path | ||
| [{:keys [from-address to-address token-id token-address route data eth-transfer?]}] | ||
| (let [{:keys [bridge-name amount-in bonder-fees from | ||
| to]} route | ||
| tx-data (transaction-data {:from-address from-address | ||
| :to-address to-address | ||
| :token-address token-address | ||
| :route route | ||
| :data data | ||
| :eth-transfer? eth-transfer?}) | ||
| to-chain-id (:chain-id to) | ||
| from-chain-id (:chain-id from)] | ||
| (cond-> {:BridgeName bridge-name | ||
| :ChainID from-chain-id} | ||
|
|
||
| (= bridge-name constants/bridge-name-erc-721-transfer) | ||
| (assoc :ERC721TransferTx | ||
| (assoc tx-data | ||
| :Recipient to-address | ||
| :TokenID token-id | ||
| :ChainID to-chain-id)) | ||
|
|
||
| (= bridge-name constants/bridge-name-erc-1155-transfer) | ||
| (assoc :ERC1155TransferTx | ||
| (assoc tx-data | ||
| :Recipient to-address | ||
| :TokenID token-id | ||
| :ChainID to-chain-id | ||
| :Amount amount-in)) | ||
|
|
||
| (= bridge-name constants/bridge-name-transfer) | ||
| (assoc :TransferTx tx-data) | ||
|
|
||
| (= bridge-name constants/bridge-name-hop) | ||
| (assoc :HopTx | ||
| (assoc tx-data | ||
| :ChainID from-chain-id | ||
| :ChainIDTo to-chain-id | ||
| :Symbol token-id | ||
| :Recipient to-address | ||
| :Amount amount-in | ||
| :BonderFee bonder-fees)) | ||
|
|
||
| (not (or (= bridge-name constants/bridge-name-erc-721-transfer) | ||
| (= bridge-name constants/bridge-name-transfer) | ||
| (= bridge-name constants/bridge-name-hop))) | ||
| (assoc :CbridgeTx | ||
| (assoc tx-data | ||
| :ChainID to-chain-id | ||
| :Symbol token-id | ||
| :Recipient to-address | ||
| :Amount amount-in))))) | ||
|
|
||
| (defn multi-transaction-command | ||
| [{:keys [from-address to-address from-asset to-asset amount-out multi-transaction-type] | ||
| :or {multi-transaction-type constants/multi-transaction-type-unknown}}] | ||
| {:fromAddress from-address | ||
| :toAddress to-address | ||
| :fromAsset from-asset | ||
| :toAsset to-asset | ||
| :fromAmount amount-out | ||
| :type multi-transaction-type}) |
There was a problem hiding this comment.
Moved functions to utils file
briansztamfater
changed the title
briansztamfater
requested review from
shivekkhurana,
smohamedjavid,
clauxx,
J-Son89,
alwx and
mohsen-ghafouri
alwx
reviewed
Aug 26, 2024
| (money/to-hex | ||
| (money/->wei | ||
| :gwei | ||
| gas-price)))))) |
There was a problem hiding this comment.
maybe it makes sense to replace cond with a simple if here for better readability?
alwx
approved these changes
Aug 26, 2024
smohamedjavid
approved these changes
Aug 27, 2024
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
from
ad1fa0b to
23eeb62
Compare
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
from
49888c6 to
37c70c8
Compare
pavloburykh
force-pushed
the
feat/fetch-swap-proposal
branch
from
23eeb62 to
ad7f1ce
Compare
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
from
ad7f1ce to
e95962f
Compare
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
from
37c70c8 to
11156eb
Compare
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
from
e95962f to
46bec86
Compare
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
from
11156eb to
4c7487f
Compare
mohsen-ghafouri
approved these changes
Sep 2, 2024
|
Already tested by @VolodLytvynenko on #21134 |
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
2 times, most recently
from
703b6aa to
a2b1a3c
Compare
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
2 times, most recently
from
4321942 to
51487f6
Compare
100% of end-end tests have passedPassed tests (7)Click to expandClass TestWalletMultipleDevice:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
|
|
Thank you @briansztamfater! E2E have passed. PR is ready for merge. |
briansztamfater
force-pushed
the
feat/fetch-swap-proposal
branch
from
a2b1a3c to
59a257d
Compare
Signed-off-by: Brian Sztamfater <[email protected]>
briansztamfater
force-pushed
the
feat/swap-approve-token
branch
from
51487f6 to
fbe2bfc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #20343
Summary
This PR implements Approve transctions for ERC20 tokens when getting a Swap Proposal that needs the pay token to be approved before proceeding.
In the video below, an example of Approving 8 DAI on Mainnet
approvetxswap.mp4
Platforms
Functional
Non-functional
Steps to test
status: ready