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

DEX app seed: BIP-44 (+ BIP-39) interoperability (DEX-managed wallets) #1670

Closed
LasTshaMAN opened this issue Jun 19, 2022 · 3 comments
Closed
Labels
discussion wontfix This will not be worked on

Comments

@LasTshaMAN
Copy link
Contributor

With current implementation, DEX uses dex-app-seed to "rule them all". Seeds for dex-managed wallets (aka seeded or native wallets) are derived from dex-app-seed - see this comment for brief code overview on how it works).
It is very similar in spirit to how BIP44 seeds work, but not compatible with it (meaning, if I have BIP44 wallet with non-empty ETH, BTC, DOGE ... balances I can't import it in DEX in such a way that DEX auto-discovers those funds, I also can't just take my funds bound to dex-app-seed and use them directly with BIP44-compatible wallet say, Trezor, by simply importing dex-app-seed there; funds have to be moved on the blockchains to/from DEX).

It would be a significant UX improvement for this dex-app-seed to be interoperable with other wallets in BIP44 + BIP39 sense (BIP44+39 is well known and widely supported seed-format).
It doesn't have to be the only way to work with DEX, it might be the most simple/popular one, here are the things it will make possible:

  • many if not most crypto-users already have readily available wallet in BIP44+39 format they might want to re-use with DEX (instead of handling yet another DEX-specific seed with additional backups)
  • DEX can auto-discover already existing funds on wallets derived as per BIP44, so that these funds are readily available for trading once everything syncs after mnemonic import
  • users can export dex-app-seed in BIP44+39 format and seamlessly re-use it as-is in any other crypto-wallet with BIP44+39 support (at their own risk! its known that importing seeds into random software often leads to lost funds)
  • removed the need for transferring funds over blockchain when dex-app-seed is exported/imported, just take your seed - ALL the coins are already there
  • ^ this might be especially relevant during market bull runs (when a lot of people start experimenting with DEX and for example Ethereum fees are high due to all the demand)
For better understanding of how this will improve UX, here is a sketch of my latest DEX "getting-started" experience

I started using DEX only recently when SPV wallets got implemented, I'm lazy and waited for that (so I don't have to bother with full nodes),
then I've noticed I can export DEX seed only as hex-encoded byte sequence and these days mnemonics are used everywhere instead (e.g. because its its painful to write down random character sequence correctly on paper, in multiple copies),
so I've suggested to implement basic export/import seed import as mnemonic (which is #1660),
while working on it I've learned the basics behind dex-app-seed (its usage and implementation), and thought to myself: I already have BIP39 wallet (24 word mnemonic, properly backed up) lying around with small amount of ETH on it (that I'm kinda OK to loose if something goes wrong, using it for experiments) can't I just use this wallet as my dex-app-seed and place some orders to see how it works ? do I have to backup/store yet another seed phrase (yes) ?

Lets go on a tangent for a sec here

Consider a case where I have another wallet with significant amount of ETH on it, and I want to convert all that ETH to BTC or DCR on DEX,
now, what would I do ?
If its a significant amount of ETH, I'm probably storing it with a seed that hasn't been exposed to software other than that of hardware wallet (such as Trezor, lets say I store it on Trezor and have some paper backups).
DEX doesn't support hardware wallets (mostly because of the need for interactivity during order settlement), so I can't use my Trezor.
Because of that need for interactivity during order settlement I can't connect to DEX with external wallet (be it Metamask, geth or ...) managed by Trezor, so its either I expose my seed to software wallet or I create a new wallet(seed) which will be exposed to software (either the likes of Metamask/geth; or its gonna be DEX-managed SPV) and transfer my funds there.
I weigh security-usability trade-offs here, Metamask (and the likes of it, assuming DEX is able to connect to these) I don't trust with my seed; geth is probably better, but its a huge piece of software I need to learn how to properly manage and use its command-line interface (which average Joe won't be able to do).
DEX-SPV is using geth underneath so, IMO DEX-SPV falls roughly in the same risk-category as using geth directly, but it has UI (which guides user and saves him from doing something like copy-pasting his seed phrase instead of typing it in).
Now, I realise it would be a bit safer to limit seed access to geth only (compared to DEX managed SPV wallet), and that option should always be there for advanced users, but as long as SPV is supported it will be the most natural choice for 99.9% of all users.

And now I have to deal with a new dex-app-seed and back it up properly.
Then I have to fund DEX-SPV wallet (btw, not so cheap for Ethereum, especially during bull run when people are coming to crypto and experiment with new tech).
Then I have to transfer my BTC or DCR (after I bought some) back to my Trezor wallet.
With 0 security benefits for most users.

Instead (if it were possible), I could have imported my BIP44+39 seed and used DEX right away.

Recommended user wallet / DEX setup

I think the following setup could be striking a good balance between usability/security/cost:

  • common crypto best practices apply: expose your BIP39 mnemonic ONLY to well-trusted hardware wallet (Trezor, Ledger, ...)
  • wanna trade on DEX with this same wallet ? at your own risk (if you trust DEX), import mnemonic there (that's DEX limitation - your wallet must be online, hardware wallets aren't designed with that in mind)
  • DEX UI will be reminding you about "DEX-exclusive-access rule" (mentioned below) when trading
  • use your hardware wallet with whatever software you trust/like (Metamask, Trezor One, ...); no need to transfer funds between wallets (bought some DCR on DEX, after order has been finalized - its already in your hardware wallet ready to be used in whatever app you like)

I don't think anyone should be doing that with his cold wallet and feel safe about it, but for many every-day use-cases these are pretty good security guarantees.

Implementation notes

  • if DEX handles BIP44 derivation, and hands down hardened extended keys to all DEX-supported managed wallets(BTC SPV, ETH wallet, ...) compromise of one such wallet (e.g. due to software vulnerability) doesn't result in corruption for other DEX-managed wallets (see last sentence in this section) - that's same security guarantees current implementation has
  • a notable caveat, if proposed approach gets implemented, there will be one thing DEX user needs to be aware of, while imported wallet is being used by DEX (has outstanding/unfinished orders against it) due to order execution being a multi-step process, DEX needs "exclusive" access to the funds on the used wallet(on the funds needed to drive the order to completion), this could be mitigated like this: when importing BIP39 seed a warning would pop up where user has to type in "I will loose all my funds in this wallet if I use it outside of DEX when I have DEX orders in progress" to proceed further (this warning could be popping up on every new order placed, until user disables auto pop ups in settings - confirming once again he understands what this is about)

Backward compatibility options

Seems like any of these will do the job (this is more of a question about dev-time / user-convenience tradeoff):

  • either force the users to undergo some kind of migration routine
  • or support both old and new dex-app-seed format (which shouldn't be too hard to do given conceptual similarity between existing approach and BIP44, existing code might require some refactoring in advance though)

On BIP39

There is nothing special about this encoding scheme except that it is widely adopted (I think its not a stretch to say most 24 word mnemonics out there are BIP39),
if different(superior) seed-encoding scheme, say, SLIP39 becomes widely adopted BIP39 part can be easily replaced.

This proposal is largely about ease of use, which is almost always a prerequisite for mass adoption and is the result of recent Matrix discussion.

@LasTshaMAN
Copy link
Contributor Author

A related feature that addresses some (not all) UX improvements outlined above would be to: allow export/import for each DEX-managed (aka seeded) wallet.
For example, it would allow DEX user to re-use his existing funded external wallet (e.g. Metamask) with existing dex-app-seed without the need for transferring funds over blockchain (this exact scenario is not covered by this proposal).
However this process needs to be repeated for every imported/exported wallet (DOGE, ETH, BTC, ... all will have its own seeds, each requiring proper backup).
And, separate DEX-specific dex-app-seed is still a thing (that's another set of backups).

Some work on this can be found in #1648.

If implemented correctly, both features (one described in this comment, and the one this proposal is about) could be supported, each addressing its own set of use-cases (however UX needs to be taken into account if this route is chosen, because providing too many options might be overwhelming for users).

@chappjc
Copy link
Member

chappjc commented Jun 20, 2022

Thank you for presenting a clear case for a new DEX app seed and key derivation scheme for all built-in wallets. I am open to discussion about how to do it without compromising security or breaking critical functionality of DEX. Two considerations there, and the latter is also of great importance. Trades will fail and funds may be lost if people use unmanaged wallet software while orders and trades are active. The world doesn't need another multi-coin wallet, it needs self-custody p2p trading software that works.

But in terms of security (never mind broken DEX trades from user meddling), the requirement of a hardware wallet to use the DEX app seed securely outside of the DEX app makes for a narrow use case. Almost all users will either: (a) take a seed phrase they are already using directly with some software wallet and give it to DEX, or (b) make a new seed with DEX and import that into a other wallets that have no business with the master key, such as MetaMask, Exodus, Coinomi, etc. Both amount to the same thing: the master key in different software.

if DEX handles BIP44 derivation, and hands down hardened extended keys to all DEX-supported managed wallets(BTC SPV, ETH wallet, ...) compromise of one such wallet (e.g. due to software vulnerability) doesn't result in corruption for other DEX-managed wallets

The above is unrelated to the fundamental issue. It's irrelevant because this wonderful isolation is all thwarted by sharing the DEX master key (via the seed) with other software.

Fundamentally the choice is: we can either make a bip39+bip44 multi-coin wallet with seed words that work with so many other wallet apps (which will unfortunately encourage sharing the master key via seed with other software, and result in failed trades and lost funds), or we make a bulletproof DEX app that keeps exclusive control over all coins, except as required for recovery.

I believe there are better ways to make user funds accessible outside of the DEX app than via the master key. I do not believe that avoiding a DEX-specific seed/phrase is adequate justification for making these compromises. If the product has value and works reliably, writing down a dozen or so words is no burden.


That's the essence of what I said in the chat that preceded this issue. I don't mean to continue the discussion at this time, just to restate my position.

@LasTshaMAN LasTshaMAN changed the title DEX app seed: BIP44 (+ BIP39) interoperability (DEX-managed wallets) DEX app seed: BIP-44 (+ BIP-39) interoperability (DEX-managed wallets) Jun 20, 2022
@buck54321 buck54321 added wontfix This will not be worked on discussion labels Sep 15, 2022
@buck54321
Copy link
Member

This issue is outdated. We do need a seed scheme which encodes the dexc birthday though. Feel free to open a follow-up issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants