Skip to content

feat: integrate celo - #5052

Merged
guanbinrui merged 1 commit into
DimensionDev:developfrom
brymut:feat/celo
Dec 26, 2021
Merged

feat: integrate celo#5052
guanbinrui merged 1 commit into
DimensionDev:developfrom
brymut:feat/celo

Conversation

@brymut

@brymut brymut commented Dec 3, 2021

Copy link
Copy Markdown
Contributor

closes #4866

To integrate Celo as one of the supported blockchains

@guanbinrui

guanbinrui commented Dec 5, 2021

Copy link
Copy Markdown
Member

Hi, @brymut. Thanks for this pull request. FYI, there are some updates about EVM chains on the develop branch. So please merge that into this. Also, we need this CELO integration to pass the below test cases before we accept this request.

  • Check if the asset list on the dashboard page is working when you choose CELO as the network.
    • Setup CELO for DEBANK API.
  • Check if the trending view is working. Try to hover a CELO token in any tweets.
    • Setup CELO for Coingeck API.
    • Setup CELO for CMC API.

image

  • Check if the gas estimate dialog is working.
    • Setup CELO for Coingeck API.

Goto chrome-extension://jkoeaghipilijlahjplgbfiocjhldnap/dashboard.html#/wallets/transfer and check the estimated USD value is working.

image

  • Trade with the DEX on CELO.

    • Integrate a DEX for CELO.
  • Send a transaction and check if the explorer links are working.

@brymut

brymut commented Dec 6, 2021

Copy link
Copy Markdown
Contributor Author

Checklist(complete): @guanbinrui can confirm that everything on the checklist works

  • Check if the asset list on the dashboard page is working when you choose CELO as the network.

Screen Shot 2021-12-08 at 11 42 00 AM

  • Check if the trending view is working. Try to hover a CELO token in any tweets.

Screen Shot 2021-12-08 at 11 44 06 AM

  • Check if the gas estimate dialogue is working.

Screen Shot 2021-12-08 at 11 49 27 AM

  • Goto chrome-extension://jkoeaghipilijlahjplgbfiocjhldnap/dashboard.html#/wallets/transfer and check the estimated USD value is working.

  • Trade with the DEX on CELO. (sushiswap)

Screen Shot 2021-12-08 at 11 38 23 AM

  • Send a transaction and check if the explorer links are working.

@brymut

brymut commented Dec 7, 2021

Copy link
Copy Markdown
Contributor Author

Hi again @guanbinrui, I think DimensionDev/Mask-Token-List#35 needs to be merged for the celo token list to be used for this PR. Tested locally using https://raw.githubusercontent.com/brymut/Mask-Token-List/master/dist/v0.0.23/42220/tokens.json however I've added https://raw.githubusercontent.com/DimensionDev/Mask-Token-List/gh-pages/latest/42220/tokens.json to the PR for when the token list is updated

@brymut
brymut force-pushed the feat/celo branch 2 times, most recently from b363f01 to fc429f0 Compare December 8, 2021 00:55
@brymut
brymut force-pushed the feat/celo branch 2 times, most recently from 77e7f14 to cacf813 Compare December 8, 2021 08:55
ChainId.Matic,
ChainId.Arbitrum,
ChainId.xDai,
ChainId.Celo,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the ITO plugin now is not working on CELO.

@brymut brymut Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@guanbinrui would the relevant contracts need to be deployed on Celo by a Mask team owned address or how else can I handle this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi again @guanbinrui, I will try deploying with my own address to see how that goes. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It appears that it would be best to open a PR at https://github.com/DimensionDev/InitialTwitterOffering for Celo. What do you think? @guanbinrui

@guanbinrui

Copy link
Copy Markdown
Member

Hi again @guanbinrui, I think DimensionDev/Mask-Token-List#35 needs to be merged for the celo token list to be used for this PR. Tested locally using https://raw.githubusercontent.com/brymut/Mask-Token-List/master/dist/v0.0.23/42220/tokens.json however I've added https://raw.githubusercontent.com/DimensionDev/Mask-Token-List/gh-pages/latest/42220/tokens.json to the PR for when the token list is updated

Hi @brymut, We just released the v0.0.23. Would you please bump the version to v0.0.24?

@brymut

brymut commented Dec 9, 2021

Copy link
Copy Markdown
Contributor Author

Hi @Jack-Works could you please have a look too :)

@AinuX

AinuX commented Dec 15, 2021

Copy link
Copy Markdown
Collaborator

@brymut

  1. Duplicate $CELO in token list
    image

  2. Duplicate $CELO asset in dashboard asset list
    image

  3. Miss GWEI info in gas setting dialog
    image
    image

  4. Estimate gas error when swap
    image

@brymut

brymut commented Dec 15, 2021

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @AinuX, will address asap. :)

@brymut

brymut commented Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Hi @AinuX, @guanbinrui, there seems to be a bug that is affecting the token list on Celo chain. When getting the token list from the DEBANK open api, most mainnet native tokens of a chain are returned with an id of the name of the chain for example ETH or MATIC tokens are returned as the following:

{
  "id": "eth",
  "chain": "eth",
  "name": "ETH",
  "symbol": "ETH",
  "display_symbol": null,
  "optimized_symbol": "ETH",
  "decimals": 18,
...

or

{
  "id": "matic",
  "chain": "matic",
  "name": "MATIC",
  "symbol": "MATIC",
  "display_symbol": null,
  "optimized_symbol": "MATIC",
  "decimals": 18,
...

so the way this is handled on the Mask plugin is by assigning the id to the address of 0x0000000000000000000000000000000000000000 in the data represention after the the token has been fetched by the API.
As over here:

const { NATIVE_TOKEN_ADDRESS } = getTokenConstants()

and the constant is set in the json file here:
"NATIVE_TOKEN_ADDRESS": {

However for the Celo chain, the Native token CELO, when fetched from the DEBANK open API, the id of the native token has an etherereum address unlike the rest which are given an id of the chain name instead like the two examples above:

{
  "id": "0x471ece3750da237f93b8e339c536989b8978a438",
  "chain": "celo",
  "name": "Celo native asset",
  "symbol": "CELO",
  "display_symbol": null,
  "optimized_symbol": "CELO",
  "decimals": 18,
  ...

And based on the code handling the data coming from the Debank open api, the CELO Native token is not managed as a native token because the id returned from the api is an actual address on the chain instead of the chain name:

function formatAssetsFromDebank(data: WalletTokenRecord[], network?: NetworkType) {

And when it comes to the assets being populated on the Assets list, the Celo native token is duplicated since in the code no Native token was found with an id of a native token (chain name), even though it was returned (with an eth address) if that makes sense?

const nativeTokens: Web3Plugin.Asset<Web3Plugin.FungibleToken>[] = networks

And for some reason, when I add the proper NATIVE_TOKEN_ADDRESS for Celo in the tokens.json file like so:

Screen Shot 2021-12-20 at 5 42 45 AM

on this line of code:

const { NATIVE_TOKEN_ADDRESS } = getTokenConstants()

in my debugger, the address 0x0000000000000000000000000000000000000000 is still returned.

Screen Shot 2021-12-20 at 5 44 46 AM

Could you please help me understand that?

ps: I've pushed a change to include the Native token address for the Celo token instead of the 0x0 address

@brymut

brymut commented Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Also @AinuX, I can't seem to reproduce the issue you're mentioning about the missing gwei info as it works fine for me.

Screen Shot 2021-12-20 at 6 15 32 AM

Screen Shot 2021-12-20 at 6 16 39 AM

Screen Shot 2021-12-20 at 6 16 52 AM

Comment thread packages/dashboard/src/pages/Labs/components/SwapSettingDialog.tsx Outdated
Jack-Works
Jack-Works previously approved these changes Dec 20, 2021
@brymut
brymut requested a review from Jack-Works December 21, 2021 07:43
Comment thread packages/dashboard/src/pages/Labs/components/SwapSettingDialog.tsx Outdated
@brymut
brymut force-pushed the feat/celo branch 2 times, most recently from 2114b3b to 83aff76 Compare December 21, 2021 08:19
@brymut
brymut requested review from UncleBill and septs December 21, 2021 08:21
@AinuX

AinuX commented Dec 22, 2021

Copy link
Copy Markdown
Collaborator

@brymut

Current build is broken, please sync latest dev branch.

@brymut

brymut commented Dec 22, 2021

Copy link
Copy Markdown
Contributor Author

@AinuX, updated.

@brymut

brymut commented Dec 23, 2021

Copy link
Copy Markdown
Contributor Author

Any update @AinuX?

@AinuX

AinuX commented Dec 23, 2021

Copy link
Copy Markdown
Collaborator

@brymut

Problem 1,2,3 still exist.

iShot2021-12-23.22.24.01.mp4

Additional,
5. Can't show other token in CELO chain even the debank api responded.
image

@brymut

brymut commented Dec 23, 2021

Copy link
Copy Markdown
Contributor Author

@AinuX @guanbinrui @septs

For 1 & 2, I already explained in this comment #5052 (comment) that Debank returns the id for the Celo Native token in the form of an eth address compared to the other native tokens on other chains which return short names and the way the ids are used to create native token representations on token lists in the code results in a duplicate for Celo:

const chainIdFromId = getChainIdFromName(y.id)

For 3, I can't reproduce at all, the Gwei info is populated properly, please refer to the following screen recording:

testtransfer.mov

Update @AinuX: its because the wei value is shifted 9 decimals back and the integer value is used which can result in a '0' value if the shifted value is less than 0.5 based on the formatToGwei function here:

return new BigNumber(value).shiftedBy(-9).integerValue()

Screen Shot 2021-12-25 at 1 36 56 PM
Screen Shot 2021-12-25 at 1 41 31 PM

Screen Shot 2021-12-25 at 12 46 42 PM

For 5, I think it's because of the is_verified property being null, its filtered out in this function formatting assets from Debank:

.filter((x) => x.is_verified)

However, other assets that are verified on Debank API do show up on the list:
Screen Shot 2021-12-25 at 12 52 59 PM

@gitcoinbot gitcoinbot mentioned this pull request Dec 25, 2021
13 tasks
@guanbinrui
guanbinrui merged commit c5cfdf3 into DimensionDev:develop Dec 26, 2021
@guanbinrui

Copy link
Copy Markdown
Member

Hi @brymut, Thanks for your work. We will handle the rest part.

@brymut

brymut commented Dec 27, 2021

Copy link
Copy Markdown
Contributor Author

Thank you for the help @guanbinrui

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.

[Bounty] Integrate CELO into Mask

6 participants