stellar-deprecated / account-viewer

DEPRECATED. Go to https://github.com/stellar/account-viewer-v2
Apache License 2.0
62 stars 65 forks source link

Add support for StellarGuard. #68

Closed pselden closed 5 years ago

pselden commented 6 years ago

Accounts that are protected by StellarGuard multisig will have their transactions submitted directly to StellarGuard.

Go to https://test.stellarguard.me to sign up for a test account and link a Testnet Stellar Account.

Successful Transaction with StellarGuard: screen shot 2018-05-01 at 2 14 45 am

pselden commented 6 years ago

Obviously this will need a fair amount of discussion about whether this belongs here or not, but in my mind it's similar to the Ledger integration except without requiring people to pay for one.

A couple things to note:

1) It will only affect users of the account viewer who had multisig enabled (this is going to be a very small percent right now since multisig wasn't even supported at all until last week). 2) Due to the way that StellarGuard accounts are configured, the "hasStellarGuard" check doesn't need to go to a server at all, so there is no performance penalty for non-StellarGuard users. 3) The StellarGuard JS SDK is very small, and I'd be fine rewriting it here using angular's $http service if you didn't want to take on another third party dependency. See https://github.com/stellarguard/stellarguard-js-sdk for details.

pselden commented 5 years ago

Hi @bartekn -- is this something that would be considered in the future?

I added support to the Foxlet wallet (formerly Stellar Desktop Client) here, and where I created a brand new angular service instead of using the SDK - https://github.com/stellarchat/desktop-client/pull/258. If you don't want the dependency on StellarGuard SDK i can do the same here.

bartekn commented 5 years ago

I think we don't want to deploy a single vendor solution to the official wallet. Have you seen SEP-0007. Is it hard to switch to this scheme?

pselden commented 5 years ago

Understood.

The multisig story in SEP-0007 was/is not very clear and I'm not quite sure where StellarGuard would fit into it. I don't think I would register URI handler myself for web+stellar links because StellarGuard is not a primary wallet -- you only use it as the last leg to your multisig transaction.

The section on multisig says the following:

For multisig accounts the wallet is responsible for coordinating the collection of signatures and submitting to the network/callback. Supporting multisig accounts would require the wallet to have a backend service to support this coordination or to use a third-party service.

This implies to me that it's the wallet (Account Viewer in this case) itself that decides how to coordinate multisig (like submitting to StellarGuard, or submitting to its own service), and not a user choice that is triggered by a URI/QR code.

@bartekn or @nikhilsaraf, would you be up for a short discussion discussing how multisig transactions should function in practice in the context of SEP-0007 so I can be sure to get it right? I'd definitely be willing to create a published JavaScript library along the way for parsing/verifying these URIs according to the spec.

nikhilsaraf commented 5 years ago

Hi @pselden, I think there is a way to achieve what you are looking for, while also keeping the account viewer agnostic to any specific multi-sig vendor. This solution can be used by all wallets too. Would you like to get on a call to discuss this early next week?

pselden commented 5 years ago

https://github.com/stellar/account-viewer/pull/68#issuecomment-411564375 -- that'd be great. I'll message you on Slack to set something up.