fix(wallet)_: fixing swap and bridge release issues #6409
Conversation
Recently, based on what's said here https://developers.paraswap.network/api/get-tokens-list ``` ParaSwap's API is not limited to the tokens that are listed on the tokens endpoint. Tokens endpoint is merely a list of tokens curated by ParaSwap. This list is not actively maintained and will soon be deprecated. To use a more up-to-date list of tokens it is suggested to use token-lists. ``` We've decided to rely on tokens' addresses which are available in the Status app, but it turned out that it works well for all but the ETH (native) token, cause the address for the native token was hardcodded to `0x0000000000000000000000000000000000000000`. Changes from this commit fix the issue in resolving route for swap. This should be handled as mentioned in the `token.go` file.
… to avoid failed bridge txs This is a temporary fix for the release, but a proper handling of this issue will be addressed via status-im/status-desktop#17551
saledjenic
requested review from
alaibe,
shivekkhurana,
vkjr,
dlipicar and
briansztamfater
Jenkins Builds
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (40.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6409 +/- ##
===========================================
- Coverage 61.11% 61.00% -0.11%
===========================================
Files 874 874
Lines 112418 112422 +4
===========================================
- Hits 68704 68588 -116
- Misses 35728 35859 +131
+ Partials 7986 7975 -11
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -695,6 +695,9 @@ func (tm *Manager) GetCustoms(onlyCommunityCustoms bool) ([]*Token, error) { | |||
|
|
|||
| func (tm *Manager) ToToken(network *params.Network) *Token { | |||
| return &Token{ | |||
| // TODO: we need to change the address for the native token to the correct one, we cannot to that right now cause will affect other parts of the code | |||
| // The following line is the right fix for `{"error":"Validation failed: \"srcToken\" contains an invalid value"}` error for Swap | |||
| // Address: common.HexToAddress("0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"), // for all the chains we support this is the address of the native (ETH) token | |||
There was a problem hiding this comment.
just a note, ETH doesn't really have an address since it's not a contract. 0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee is just a convention Paraswap uses to identify ETH without having to treat it specially, but it's not anything standard (other platforms might use other addresses like 0x0), so any references to ETH having that address should be limited to the paraswap processor.
There was a problem hiding this comment.
ETH doesn't really have an address since it's not a contract.
That's 100% correct, but I thought that the commonly used "address" is 0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee, if that's not adopted, then we will always need to map the ETH token's address to the address that the service expects for the ETH token.
There was a problem hiding this comment.
Yeah I don't think this is widely adopted or anything, that's why I feel it's best to handle it in a more specific spot rather than a more generic one. At the token.Token level we should probably explicitly avoid using it to prevent confusion.
There was a problem hiding this comment.
Oh wait! there is a proposal, but I'm not sure if it's accepted yet: https://eips.ethereum.org/EIPS/eip-7528
There was a problem hiding this comment.
https://github.com/ethereum/ercs/blob/master/ERCS/erc-7528.md
I think this means it is. Sorry about that, I guess we can go ahead with using that address.
I'm not sure we can expect EVERY provider to follow that convention though, so let's be careful with that. Hopefully this will be the rule and not the exception.
|
Hi @saledjenic, Thank you for the PR! I just checked mobile PR #22281, which includes the current Status Go commits. No issues from my side. However, I noticed that the Status Go PR hasn’t been reviewed yet. Please let me know if any additional commits are added after the review, and I’ll retest the PR. Otherwise, it’s good to merge. Thanks! 😊 |
| } | ||
| if params.ToToken.IsNative() { | ||
| params.ToToken.Address = common.HexToAddress("0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee") // ETH address across all chains that we support | ||
| } |
There was a problem hiding this comment.
don't we use 0x0 for native?
This PR contains 2 commits, both are temporary fix for the release.
Swap issue
fix(wallet)_: swap route calculation fixed
Recently, based on what's said here https://developers.paraswap.network/api/get-tokens-list
We've decided to rely on tokens' addresses which are available in the Status app, but it turned out
that it works well for all but the ETH (native) token, cause the address for the native token was
hardcodded to
0x0000000000000000000000000000000000000000.Changes from this commit fix the issue in resolving route for swap. This should be handled as mentioned in the
token.gofile.Bridge issue
fix(wallet)_: ensuring higher estimated gas for erc20 tokens in order to avoid failed bridge txs
This is a temporary fix for the release, but a proper handling of this issue will be addressed via status-im/status-desktop#17551