search

vulpemventures / ldk

Liquid Development Kit provides abstractions to speed-up implementation of elements-based wallets for browsers and NodeJS
MIT License
9 stars 11 forks source link

Drop IdentityType (?) #78

Open louisinger opened 2 years ago

louisinger commented 2 years ago

It looks like IdentityType is never used outside of LDK. We only use it for additional checks inside the Identity's constructors. If the idea is to find a way to discriminate Identities by their types, I think instanceof is enough. cc @tiero

tiero commented 2 years ago

Not sure about that. IdentityType enum is just a way to categorize various type of wallet structure etc..

we can have many identities of Mnemonic type, maybe with different derivation type etc..This comes handy when extending/overriding

louisinger commented 2 years ago

it could be a constant setup in each Identity's implementation, not a constructor argument. With the current implementation we need to put type: Mnemonic in all the new Mnemonic() statements.

tiero commented 2 years ago

Just t give you a bit of rationale about it:

the original idea of that IdentityType is to mostly support Hardware Wallets/Keyless kind of signers:

we could have a Mnemonic class that support many different identity types, such as "I want HD wallet and I have keys, so will pass as options" (what we call now IdentityType.Mnemonic, since we explicitly checks for seed being passed) or "I want an HD wallet but I have my Trezor, so please connect over the transport I pass here in the options to get the xpub and pair with it"

But I see how we are evolving the architecture, moving in favor of on-way binding Class to their own Options, which is more a design and semantic change, we could eventually discuss and agree on. So just to say, it's not simply as "drop a field in the interface"