Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ERC20 token support #531

Merged
merged 41 commits into from Mar 10, 2023
Merged

Conversation

0a1c
Copy link
Contributor

@0a1c 0a1c commented Oct 10, 2022

Add capability to:

  • Add/manage ERC-20 tokens
  • Fetch user balances
  • Show hard-coded ERC-20 currencies

Details

This PR builds infrastructure for ERC-20 tokens, similar to CW20 tokens, with a few distinctions. ERC20 queries differ from CW20 and Secret queries in that they query a JSON-RPC endpoint, rather than an HTTP endpoint. The infrastructure is built around this difference, while maintaining principles from the CW20 and Secret implementations.

Architecture

erc20/contract-query.ts

  • The base layer of the ERC-20 module—has the ability to query an ERC-20 contract for all relevant fields. The ObservableQueryERC20ContractData class parallels the ObservableCosmwasmContractChainQuery class for CW20s, and serves as an extendable object to fulfill contract requests.
  • This file is lengthier than its counterparts because it requires additional infrastructure to fetch and parse ERC-20 contract data using the ObservableJsonRPCQuery class

erc20/erc20-balance.ts

  • Connects the ERC-20 module to the extension's BalanceRegistry by wrapping ObservableQueryERC20ContractData, as a parallel to cw20-balance.ts

erc20/erc20-contract-info.ts

  • Wraps ObservableQueryERC20ContractData to provide a tokenInfo() response, to match the behavior provided in cw20-contract-info.ts
  • This class does not fetch balances and is used primarily when adding a new ERC-20 token in the UI

erc20/queries.ts

  • Wraps ObservableQueryERC20ContractData to provide a root query object, similar to cosmwasm/queries.ts

erc20/transaction-client.ts

  • Creates an Ethereum Tx client to interface with a chain's Ethereum JSON-RPC provider
  • This class can format Ethereum Tx's, broadcast signed Tx's to the node, and wait for confirmation

erc20/types.ts

  • Contains type definitions for the module

accounts/ethereum.ts

  • This class is responsible for sending ERC-20 token transfers when requested by the UI. It uses Keplr's signEthereum and the erc20/transaction-client to do so.

Notes

Support has not yet been implemented for mobile.

Demo

Token Management
https://user-images.githubusercontent.com/11379797/194942357-f0efaefc-16cb-4107-99ab-a5fadf78dacc.mov
Token Transfer via EVM
https://user-images.githubusercontent.com/11379797/208807580-c6fedd5d-e972-443a-bdf4-b682abcb4f92.mov
IBC Transfer of ERC-20 Token
https://user-images.githubusercontent.com/11379797/209244773-860ebca7-4e4b-4e70-82ee-2c083f8d023c.mov

@Thunnini Thunnini force-pushed the develop branch 2 times, most recently from ede9055 to cb7e8fc Compare November 25, 2022 12:39
@0a1c 0a1c marked this pull request as draft November 30, 2022 17:42
@0a1c 0a1c marked this pull request as ready for review November 30, 2022 18:55
@Thunnini
Copy link
Member

Thunnini commented Dec 5, 2022

@keplr-wallet/store-etc is uploaded in npm, and also used in other projects as well. We cannot merge this PR when @keplr-wallet/store-etc is deleted

@Thunnini
Copy link
Member

Thunnini commented Dec 9, 2022

To be honest, showing only balance (without support transfer) to the user has alot of parts that we need to think about. Also, as of right now, this feature is only for evmos and if features being added start becoming domain specific, it becomes harder and harder to manage code. Current code needs to go through refactoring in the near future, so implementing a special feature is a huge burden in terms of code management. Also, I do think that current PR cycle is very slow, and that you might be unhappy about it.
The suggested way is the following: For evmos, instead of showing assets, we can try linking to your dashboard. This way I think we both can implement this faster and come to an stable agreement. I say we try this first and once this becomes stabilized, adding this feature to Keplr. What do you think of?

@0a1c 0a1c mentioned this pull request Jan 3, 2023
packages/extension/package.json Outdated Show resolved Hide resolved
packages/extension/src/config.ts Show resolved Hide resolved
packages/extension/src/pages/send/index.tsx Show resolved Hide resolved
const type = new DenomHelper(cur.coinMinimalDenom).type;
return (
type === "native" ||
(type === "erc20" && this.initialChainId.includes("evmos"))
Copy link
Member

Choose a reason for hiding this comment

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

Please take off the restriction on the chain ID, unless there is a specific reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored to use the evmos-erc20 feature flag. The purpose for this condition is that Evmos supports sending ERC-20 tokens in IBC transactions, but other ERC-20 chains may not.

Let me know if this is not a concern, and we can remove the check.

packages/stores/package.json Outdated Show resolved Hide resolved
packages/stores/src/ibc/currency-registrar.ts Outdated Show resolved Hide resolved
packages/stores/src/query/erc20/contract-query.ts Outdated Show resolved Hide resolved
packages/stores/src/query/erc20/erc20-contract-info.ts Outdated Show resolved Hide resolved
}
}

protected createSignableEthereumTx(
Copy link
Member

Choose a reason for hiding this comment

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

Move this to account store

Copy link
Contributor Author

@0a1c 0a1c Feb 28, 2023

Choose a reason for hiding this comment

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

I'm not sure what you mean. account/ethereum uses createERC20TokenTransferTx in transaction client, and createERC20TokenTransferTx uses createSignableEthereumTx.

Are you suggesting that account/ethereum calls createERC20TokenTransferTx in transaction client, which calls createSignableEthereumTx once again in account/ethereum?

packages/stores/package.json Outdated Show resolved Hide resolved
@HeesungB HeesungB merged commit 1199fa4 into chainapsis:develop Mar 10, 2023
@HeesungB
Copy link
Collaborator

@austinchandra Thank you for contribution!

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.

None yet

3 participants