Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Add xDAI to ChainId #53

Closed
wants to merge 6 commits into from
Closed

Add xDAI to ChainId #53

wants to merge 6 commits into from

Conversation

anxolin
Copy link

@anxolin anxolin commented Jan 19, 2021

Closes #52

Adds xDAI support in the SDK

The constants for the factory/init code are not exported any more. They depend on the current network. A new function getFactoryParams is exported instead.

Copy link
Contributor

@moodysalem moodysalem left a comment

Choose a reason for hiding this comment

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

i think it makes more sense to just have chainId be the ChainId enum or any number, everywhere that it's used.

@anxolin anxolin mentioned this pull request Jan 19, 2021
@anxolin
Copy link
Author

anxolin commented Jan 19, 2021

i think it makes more sense to just have chainId be the ChainId enum or any number, everywhere that it's used.

Ok, that would be a different approach. I agree is more general, and it would support any other chain.
Is not one line anymore, but implemented it in #54

@anxolin anxolin requested a review from moodysalem January 22, 2021 10:40
mergify bot referenced this pull request in gnosis/cowswap Jan 25, 2021
Brings xDAI to the swap UI:
- Allows to connect
- Shows xDAI instead of ETH
- Allows to Wrap xDAI into wxDAI
- Allows to Sell xDAI (it wraps and then sells wxDAI)

![image](https://user-images.githubusercontent.com/2352112/105481766-cb989380-5ca7-11eb-84c6-e9e10da879d3.png)

![image](https://user-images.githubusercontent.com/2352112/105482349-8a54b380-5ca8-11eb-876a-2017443062c7.png)


Also includes Blockscout link:
![image](https://user-images.githubusercontent.com/2352112/105482276-71e49900-5ca8-11eb-829f-afa2757019c7.png)



## Why another xDAI PR
So here is a different approach than #94 

I was getting to some deadends with the former approaches, some criticisms 
- Some depended in a forked version of the SDK we don't control. Moreover, they are not even pushed to somewhere, they leave as a gigantic PR in Honeyswap repository that will probably never be merged. On top of this, they forked a SKD that is now sligly obsolete.
- The hacks bring a lot of typescript redefinitions to the project, and force us to change their imports, and/or do uggly hacks

## Current approach
This approach is not perfect either, it's basically a combination of:
- I forked their repo
- I PR them 2 PRS (one just adds xDAI, other adds any network). I was hopping they merge, but
- We cannot wait until Uniswap merges, so I did also a fork, and published my own package with the PR I sent to them
- In this PR I use my NPM package

Lot's of other things are happening in this PR. Sorry for making such a big PR, but I'm happy to walk through if it helps. 
I tried to at least break the commits in smaller pieces with a comment. 

## Help for the review
If it helps, let me know I can do a quick zoom session and clarify the parts. 
- My NPM package has the code from https://github.com/Uniswap/uniswap-sdk/pull/53 . Plan is to replace it once is merged.
- I tried to use our pattern of not overriding SRC, and managed in most cases, but still there was some exceptions. The exceptions where mostly because of the type, they required some objects (used as maps) to have all networks as keys. When I added xDAI they need now a value for that. I can change the type, but that would require the same change in the source file plus in every place where it's used (cause now you change the type and can return undefined)
- Most of the changes are just change on the path of the import, mostly `useContract` and `utils` because they are used a lot 
-  The most weird thing here is the updater. It was extracted from Dima's solution. He came up with a smart way to modify ETH label. Is hacky but it saves a lot of headaches (and it's contained). U'll see this in the PR in `utils/xdai/hack.ts`
- The rest, I think is easy to follow

## Not in this PR
Some labels need to be reviewed. It sometimes show ETH or WETH when is xDAI or wxDAI

Here I was wrapping xDAI:
![image](https://user-images.githubusercontent.com/2352112/105482553-d273d600-5ca8-11eb-8312-30a84f19df3e.png)

![image](https://user-images.githubusercontent.com/2352112/105482600-e3bce280-5ca8-11eb-9939-3c7320fe5eed.png)


## How to test
* Go to:
http://localhost:3000/#/swap?inputCurrency=0xb7D311E2Eb55F2f68a9440da38e7989210b9A05e&outputCurrency=0xe91D153E0b41518A2Ce8Dd3D7944Fa863463a97d&exactAmount=0.1
* Trade
@AdamREQ
Copy link

AdamREQ commented Feb 4, 2021

@moodysalem can you provide an update on this or #54? Feedback is welcome, would love to move forward with this.

@moodysalem moodysalem changed the base branch from v2 to main March 4, 2021 21:32
@stale
Copy link

stale bot commented Jul 14, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Jul 14, 2022
@stale stale bot closed this Jul 21, 2022
royalaid pushed a commit to royalaid/qidao-sdk that referenced this pull request May 27, 2023
A interested poly Wbtc, Eth, and Manhattan Metis
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xDAI (or other arbitrary chain) support
3 participants