KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
103 stars 94 forks source link

Hardware Wallet storage should be shared between Iguana and all HD accounts #1621

Open sergeyboyko0791 opened 1 year ago

sergeyboyko0791 commented 1 year ago

Currently, Hardware Wallet storage is in the same directory as Iguana/HD account: https://github.com/KomodoPlatform/atomicDEX-API/blob/eb8792ada38dd7171ede1dd243b6a05b69bcbd32/mm2src/coins/hd_wallet_storage/sqlite_storage.rs#L112-L119

This is incorrect since Hardware Wallet should be shared between the accounts. This can be fixed by adding a Wallet storage that is not dependent on either Iguana or HD account ID, but depends on the mnemonic passphrase only.

sergeyboyko0791 commented 1 year ago

This problem is also related to GUI storage as it should be shared between Iguana and all HD accounts as well, so I believe it's important to design such a shared DB correctly. I have multiple suggestions:

  1. Use an Iguana DB for data shared between Iguana and HD accounts. This will avoid confusion with the storage names, and . For example, we'll have two properties in the MmCtx:
    pub struct MmCtx {
    pub conf: Json,
    // ...
    /// RIPEMD160(SHA256(x)) where x is a secp256k1 pubkey derived from passphrase using **either** Iguana or HD derivation method.
    /// This hash is **unique** among Iguana and each HD accounts derived from the same passphrase.
    pub rmd160: Constructible<H160>,
    /// RIPEMD160(SHA256(x)) where x is a secp256k1 pubkey derived from passphrase using **exactly** Iguana derivation method.
    /// This hash is **common** for Iguana and each HD accounts derived from the same passphrase.
    pub iguana_rmd160: Constructible<H160>,
    }
  2. Use a different DB for such shared data. This can be also done in the following ways:
    • Add a hardcoded magic string to the input mnemonic phrase, derive a secp256k1 pubkey from it using Iguana derivation method, and then calculate RIPEMD160(SHA256(x)). If we choose this one, we should give such the shared storage a name that won't be confusing. For example, Shared and shared_rmd160: Constructible<H160> correspondingly.
    • Use the same iguana_rmd160: Constructible<H160> as in the 1. point, but use an additional suffix/prefix in the DB file path (in native <RMD160>_WALLET, in WASM WALLET::<RMD160>::<DB_NAME>).

@artemii235 what do you think?

artemii235 commented 1 year ago

@sergeyboyko0791 I think approach 2 is more preferred: iguana_rmd160 name will require additional documentation and explanation that this is used as shared DB ID. Also, after multiple accounts are used, it will be not clear where to look for shared DB files in directories with names like 05aab5342166f8594baf17a7d9bef5d567443327 (not a big problem, but will still require a bit of effort).

Add a hardcoded magic string to the input mnemonic phrase, derive a secp256k1 pubkey from it using Iguana derivation method, and then calculate RIPEMD160(SHA256(x)).

It's good. This way, you can even print the shared DB ID to log, as it can't be linked to any of the user's addresses (unlike iguana_rmd160). Please also rename shared_rmd160 to shared_db_id to define the purpose clearly.

sergeyboyko0791 commented 1 year ago

Awesome, thank you @artemii235