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

Derivable EVM Coin Names #398

Merged
merged 1 commit into from
Feb 15, 2024
Merged

Conversation

adraffy
Copy link
Contributor

@adraffy adraffy commented Feb 11, 2024

evmCoinTypeToNameMap is frozen and in ensjs: bypassFormat cannot be supplied to getRecords().

This might require additional changes elsewhere, but EVM coinTypes shouldn't require a name.

@TateB
Copy link
Collaborator

TateB commented Feb 12, 2024

this concept seems useful but it'll need a few minor API additions probably + some type updates. i'm not sure that the expected default behaviour should be to return the name of a coin as effectively "Unknown", so maybe it works better to either:
a. have an "allowUnknownEvm" param or similar, or
b. allow a custom name mapping for evms to be passed into the coin type getter

is there a specific use-case you have in mind for this?

@TateB
Copy link
Collaborator

TateB commented Feb 12, 2024

alternatively, maybe its better to just have ensjs changes here?

@adraffy
Copy link
Contributor Author

adraffy commented Feb 12, 2024

All EVM coins are synthetic though according to ENSIP-11, why do they need to be added into that frozen map to be supported by this library?

@TateB
Copy link
Collaborator

TateB commented Feb 14, 2024

they don't, i was just considering the best api for this

what about formatted as:
Unknown Chain (${chainId}) and an unknownEvmChain property

does that work for your use-case?

@adraffy
Copy link
Contributor Author

adraffy commented Feb 14, 2024

Ah sorry, I think I misread your initial response.

Any name format would work for me, and being able to know that the name is "unnamed" is great too!


Related: say I have a coinType is there an isEVMCoinType(type): boolean?

With the above change, you can now do:

function isEVMCoinType(x) {
    try {
        return getCoderByCoinType(x).evmChainId > 0;
     } catch (ignored) {}
}

Or currently, you need to import from utils/evm:

function isEVMCoinType(x) {
    try {
        return coinTypeToEvmChainId(x);
     } catch (ignored) {}
}

Alternatively, instead of the large API surface, you could use a single function:

// chain > type > name
let coder = getCoin({chain: 5});
let coder = getCoin({type: 60});
let coder = getCoin({name: 'ETH'});

getCoin({type: 0x80000000+5}).isEVM // true
getCoin({chain: 5}).isEVM // true
getCoin({chain: 69}).isUnnamed // true
getCoin({name: 'ETH'}).type // 60
getCoin({chain: 100}).legacyType // 700

@TateB
Copy link
Collaborator

TateB commented Feb 15, 2024

i think isEvmCoinType(coinType) is reasonable

@TateB TateB merged commit 8a72eb8 into ensdomains:master Feb 15, 2024
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.

2 participants