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

feat(wallets): add evm to tron link #350

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

nikaaru
Copy link
Member

@nikaaru nikaaru commented Sep 2, 2023

No description provided.

@@ -21,8 +21,8 @@
"parserOptions": {
"ecmaVersion": "latest",
"sourceType": "module",
// "project": ["tsconfig.base.json"]
Copy link
Member Author

Choose a reason for hiding this comment

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

@yeager-eren
please check eslint parser issue

const { tronLink, TronLinkEVM } = window;

if (!tronLink) {
return tronLink;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return tronLink;
return null;

Make it more explicit.

@@ -23,27 +30,63 @@ export const config = {
defaultNetwork: Networks.TRON,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Default network is tron, When you are on Ethereum network on TronLink, it goes through an error. How we can fix it?

if (!r) {
const results: ProviderConnectResult[] = [];

if (ethInstance) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It tries to connect to Ethereum and Tron (instance) at the same time but the wallet seems doesn't support that and one network should be connected at the same time.


ethInstance?.on('accountsChanged', (addresses: string[]) => {
const eth_chainId = meta
.filter(isEvmBlockchain)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line need as long as you have a find?

ethInstance?.on('accountsChanged', (addresses: string[]) => {
const eth_chainId = meta
.filter(isEvmBlockchain)
.find((blockchain) => blockchain.name === Networks.ETHEREUM)?.chainId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to support for BSC as well?

.filter(isEvmBlockchain)
.find((blockchain) => blockchain.name === Networks.ETHEREUM)?.chainId;
if (state.connected) {
if (state.network != Networks.ETHEREUM && eth_chainId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why we need this condition here.


export const getSigners: (provider: any) => SignerFactory = signer;

export const getWalletInfo: (allBlockChains: BlockchainMeta[]) => WalletInfo = (
allBlockChains
) => {
const evms = evmBlockchains(allBlockChains);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It returns the whole list evm compatible networks, but we only need BSC and ETH here as the wallet only has support for this.

@nikaaru nikaaru requested a review from RanGojo as a code owner May 14, 2024 13:14
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.

3 participants