wallet-core icon indicating copy to clipboard operation
wallet-core copied to clipboard

Support custom EVM chain without code change, but runtime data

Open optout21 opened this issue 1 year ago • 3 comments

Is your feature request related to a problem? Please describe. Support for a new EVM-clone chain has been simplified (see #2214), but still requires code change and new release in wallet-core. It would be nice if new custom chain can be added to a wallet dynamically at runtime, but still some support from wallet core.

Describe the solution you'd like There are several possible solutions in the wallet app, with various degree of flexibility:

  1. A custom EVM-clone chain can be added by the user (runtime), similar how it is possible in MetaMask. Wallet core is not involved, app uses coin=Ethereum for address derivation, and coin=Ethereum and custom chainId for signing.
  2. A list of alternative chains is maintained in a curated config file. Adding a new chain to this config will add it to the app (w/o new releases). Internal handling is like above.
  3. A custom EVM-clone chain can be added by the user, or from config to the App. App 'registers' the new chain properties with wallet code. Wallet core extends its relevant coin-specific methods to also support the dynamically registered chains (additionally to the ones statically defined in registry.json).

This issue is about number 3.

Limitation Since dynamically registered types are not known at build time, generated sources will not contain these coins.

Describe alternatives you've considered See above number 1.

Checklist

Resources

optout21 avatar Oct 04 '22 10:10 optout21

Is your feature request related to a problem? Please describe. Support for a new EVM-clone chain has been simplified (see #2214), but still requires code change and new release in wallet-core. It would be nice if new custom chain can be added to a wallet dynamically at runtime, but still some support from wallet core.

Describe the solution you'd like There are several possible solutions in the wallet app, with various degree of flexibility:

  1. A custom EVM-clone chain can be added by the user (runtime), similar how it is possible in MetaMask. Wallet core is not involved, app uses coin=Ethereum for address derivation, and coin=Ethereum and custom chainId for signing.
  2. A list of alternative chains is maintained in a curated config file. Adding a new chain to this config will add it to the app (w/o new releases). Internal handling is like above.
  3. A custom EVM-clone chain can be added by the user, or from config to the App. App 'registers' the new chain properties with wallet code. Wallet core extends its relevant coin-specific methods to also support the dynamically registered chains (additionally to the ones statically defined in registry.json).

This issue is about number 3.

Limitation Since dynamically registered types are not known at build time, generated sources will not contain these coins.

Describe alternatives you've considered See above number 1.

Checklist

Resources

The number 1 and number 2 solutions make more sense to me. Checking the way it works for cosmos, it requires only a custom chainId for signing. Also checked the keplr code, and it's similar. For some chains, the point number 2 is required (evmos use a different derivation method).

If we really need number 3, let's try to find code changes that will allow us to rely only on 1 and 2.

Milerius avatar Oct 06 '22 11:10 Milerius

I think we might only need to change KeyStore, allow adding unknown CoinType

hewigovens avatar Oct 07 '22 05:10 hewigovens

I think we might only need to change KeyStore, allow adding unknown CoinType

Yes, but then we can't use the enum TWCoinType since it's scoped and undefined behaviour to have enum values that are not scoped:

After CWG 1766 (C++17) See CWG defect 1766. The [expr.static.cast]p10 paragraph has been strengthened, so you now can invoke UB if you cast a value that is outside the representable range of an enum to the enum type. This still doesn't apply to the scenario in the question, since data[0] is of the underlying type of the enumeration (see above).

So we may want to add some function to query CoinType by a string in this case or any other runtime values.

My suggested approach

Many function will need to be written using runtime prefix runtimeAccount runtimeAddAccount, etc but that should just work.

/// Account for a particular coin within a wallet.
class Account {
  public:
    /// Coin this account is for
    TWCoinType coin;
    nlohmann::json mRuntimeCoin;

    /// other fields
    Account(nlohmann::json runtimeCoin, /*other account fields*/);
};

I'm going to draft a runtime registry that use a std::unordered_map<std::string, CoinConfig> - in an another pr we will extend the KeyStore to use this registry.

Milerius avatar Oct 07 '22 14:10 Milerius