spacemeshos / smapp

Spacemesh App (Smesher + Wallet) 🏦📊
https://spacemesh.io
Apache License 2.0
132 stars 41 forks source link

Multisig Support #888

Open avive opened 2 years ago

avive commented 2 years ago

Rationale

We need to support multi-sig transactions for sm 0.3 (Milestone 7)

Prerequisites

go-sm transactions api service

User story

TBR

Acceptance criteria

Out of scope

How to implement

If you have an idea of how to implement it, provide a short description.

How to test

oriya commented 2 years ago

I am assigning this to myself to follow up.

brusherru commented 2 years ago

Rationale

We want to support multi-signature accounts (aka 2-of-3 signatures to spend money). This feature requires:

Prerequisites:

Overview

To be on the same page I will collect all information related to the topic there.

New transactions structure

We already have a new transaction structure for it:

image

Following the scheme we can conclude that:

Signing transactions

We have a mockups how Smappers should interact with vaults: https://www.figma.com/file/6bbFkIAzVu36bIpUNnMqoy/Smapp-Designs?node-id=4818%3A18503

The basic scenario:

  1. Alice, Bob and Charlie decided to make a 2/3-sig account
  2. They used their favorite messenger to communicate with each other, so we don't handle any messaging between parties
  3. Then they decided that Alice will spawn this account and pay for gas. She clicks on "add new vault": image
  4. Bob and Charlie sends to Alice public keys of their single-key accounts.
  5. Alice chooses one of her single-key accounts in a dropdown and put others two in inputs: image
  6. Then Alice checks the summary and clicks on "Create Vault" button: image
  7. And then some ✨magic✨ starts...

We have this mockup:

image

It seems to me like the further signing of one of other participants was planned to be on-chain, but as far as I understood we don't want to put a partially signed transactions on chain. So we need to have some mechanism that help Smappers to sign such transactions and publish them to the network.

In addition to it, I've found this repo: https://github.com/spacemeshos/multisig-server It does not seems working and actual, but I can assume that we had a plan to host a standalone service that will help Smappers to sign multi-sig transactions off-chain. Is it correct? Do we have a plan (and team) to actualise this repo?

🙏 If we have the exact plan or you know any other details — feel free to share it.

Importing vaults

We already have mockups for this as well: https://www.figma.com/file/6bbFkIAzVu36bIpUNnMqoy/Smapp-Designs?node-id=4870%3A539 It covers the case of importing vault for further interaction with it. But in case if your wallet don't have any account that is listed as Master accounts in the imported vault — it should be "view only" mode. So User can watch for transactions and etc, but can't send tx and etc.

Notes from @noamnelke

When we has been working with @oriya over product genesis roadmap we discussed multi-sig account a bit, and then Noam provided this detailed notes:

ok, as I see it there are two things that need to be addressed about multisig wallets: creating them and using them.

I’m hoping that the using part will be near identical to using a normal wallet, with small changes. E.g. when authoring a transaction, we need to mostly do some explaining, that sending / confirming / signing a transaction (however we word if for a normal transaction) doesn’t mean it’s finalized and it still needs to get further confirmations. We should also provide the number of additional confirmations / signatures the transaction will need before it can be sent to the network.

When someone else authored a transaction, we need some notification that says a transaction is waiting to be signed / confirmed and some list of pending transactions, with dismiss / review actions. Clicking the notification, or clicking review in the list of pending transactions should open a screen almost identical to a normal transaction authoring screen, only the transaction details can’t be edited and there should be a small section that says which other co-signers the transaction already has, and how many signatures are still missing.

Other then this, with regards to using a multisig account, there are only some minor details that need to be worked out, like notifications when someone else signed a transaction you authored, or if a transaction is pending for a long time and isn’t getting signed (at some point the coordination server will prune transactions that are pending for too long), or if a transaction in a multisig account was approved without your involvement (e.g. in a 2-of-3 multisig, the other two may have approved a transaction, and you just need to be notified that it happened).

The harder part, imo, is setting up a multisig account. Unlike a single-sig account, setting up a multisig account requires some coordination and can’t be a local, atomic operation. Mostly you need the public keys of all participants. In theory you could reuse an existing key for this, but I think that for privacy reasons the default behavior should be to generate a dedicated keypair. I There should be some “pending wallet” screen where the user can see/copy their public key and also paste others’ keys when they get them. This screen should also allow setting the multisig config (i.e. how many signatures are needed to authorize a spend transaction).

When all the signatures are collected and the config is chosen, an account address can be calculated. After that point it’s like any other account. However, there are some edge cases: if a participant didn’t include the same set of public keys or the same config, then the resulting account will be different. Perhaps we should implement a step where we show the user the resulting address and give them an opportunity to make sure that everyone got the same address before confirming. Even then, some people will make mistakes so we should allow these to be easily fixed. I believe that allowing importing wallets, if done correctly, is a good solution.

I believe the SMApp should maintain a collection of known keys that is append only. We should support a “view only” wallet mode, where we can import wallets into the SMApp based on their address (if they’re already spawned) or based on their spawn arguments otherwise. When a wallet is imported it becomes view-only if we don’t control the keys, or a regular wallet if we do.

If you made a mistake when setting up a multisig wallet and ended up with a different address, the easiest fix would be to ask someone else who did get the right address for the spawn arguments and import them. If SMApp recognizes that it has access to one of the keys it will import it as a regular (not view-only) wallet, as if setup was done correctly.

This is also useful if someone generates a multisig wallet using one of the keys you control without coordinating with you and you want to retroactively import the wallet.

We need to more work on this, including some wireframes, to make sure we’re not missing anything, but I think this is a good start.

Open questions

  1. [x] How we transfer partially signed messages between parties / sign spawn tx of 2/3-sig account?
  2. [x] What keys are used in 2/3-sig account? Single-sig?
  3. [x] Signing multi-sig transactions is on-chain / off-chain with our service / off-chain without our service?
  4. [ ] Do we have any way (and optimized enough) to automatically find out all vaults, that uses Users public key? Since they are encoded in MethodArguments, I assume that the answer is no, please tell me that I'm wrong.

I may miss something, so feel free to fix me, add details, etc.

cc: @dshulyak @countvonzero @oriya @noamnelke @lrettig

avive commented 2 years ago

My 2 cents: I'd like to note regarding the last round of notes form @noamnelke that we have already had a design iteration of vaults and safe which resulted in the current visual designs that is minimally usable and doesn't need to address some of the issues that were brought up. I recommend that it will be implemented w/o going back to the drawing board as there's really no need for that and the last visual design iteration only included an extensive round of comments from Noam and other team members. The features should be developed from that iteration in future iterations. Noam mentions many advanced use cases that we shouldn't worry about for the first iteration if we want to have it implemented quickly although of course it is good to have these in mind.

avive commented 2 years ago

Looks like the product site is down - there were several vaults and multi-sig related journeys and interactive prototypes which describe the last product iteration over these features - I hope you guys considering them before continuing to work on these features - otherwise you need to do the use cases and flows from scratch. This of course, can be done, but plz be conscious about it if you really want to do this now in the dev cycle. I wanted to share some links but it is no longer possible.

brusherru commented 2 years ago

@avive thanks for these notes. 🙏 Regarding the product website and last product iteration, I found this: https://github.com/spacemeshos/product/blob/master/multisig_vaults_interaction_design.md

It is pretty clear and I understand how the web service may work (using vault address and public keys), but it will work for SpendTransaction, meanwhile, it is not clear to me how we can use the same web service to sign spawn transactions — we still don't have the vault address at this stage... so perhaps this web service should implement such endpoints: GetSpawnTransaction :: Request(TemplateAddress + Template Arguments) -> Response(PartiallySignedTx) SignSpawnTransaction :: Request(TemplateAddress + Template Arguments + Sign) -> Response(?)

brusherru commented 2 years ago

Btw, what is the status of this web service, and who is taking care of it? :)

brusherru commented 2 years ago

We had a conversation with @noamnelke and he clarified a few points for me:

  1. Principal address — is the address of the multi-sig account itself. So multi-sig account pays for gas, and we don't stick to any single-sig accounts at all.
  2. Keypairs (public key) used there are any random keypairs, which are not required to be spawned.
  3. Meanwhile, for security reasons, it will be much better to create a new keypair for each vault.
  4. To sign transactions we may support two different ways:
    1. "Censorship-resistant mode" — to sign a transaction Alice should send a partially signed transaction to Bob/Charlie via messenger, and Bob/Charlie should sign it by pasting this partially signed transaction into Smapp.
    2. "Easy mode" — to deploy a centralised webservice, that will take care of exchanging partially signed transactions. So Smappers can subscribe for new transactions of vault addresses, and get notified when someone initiates a transaction and requests the signature of other parties via this webservice.
  5. We need an additional way to import vault — by template arguments.

From my point of view, we should focus on "censorship-resistant mode" in the first iteration.

lrettig commented 2 years ago
  1. Principal address — is the address of the multi-sig account itself. So multi-sig account pays for gas, and we don't stick to any single-sig accounts at all.

one quick point: while the multisig (principal) account can pay for gas once it's been spawned, prior to this, another principal account needs to pay the gas to spawn it - unless the account-to-be-created already contains funds, in which case it could be spawned using self-spawn (but I think this would be quite rare and doesn't necessarily need to be considered right now).

  1. Meanwhile, for security reasons, it will be much better to create a new keypair for each vault.

does smapp already support hierarchical deterministic wallet generation?

lrettig commented 2 years ago

Do we have any way (and optimized enough) to automatically find out all vaults, that uses Users public key? Since they are encoded in MethodArguments, I assume that the answer is no, please tell me that I'm wrong.

No, the public keys are part of the multisig account's immutable state. Blockchains are notoriously hard for indexing and finding data buried in state. We could emit an event on creation, which would be a bit easier to index/search, but we haven't designed this yet and it might not make it into genesis. This would probably have to be a third-party data service built on top of go-spacemesh. (Here is one such option: https://thegraph.com/docs/en/)

brusherru commented 2 years ago

@lrettig

another principal account needs to pay the gas to spawn it - unless the account-to-be-created already contains funds

On the picture above shown only self-spawn, so I guess that signel-sig and multi-sig accounts are self-spawn only? So in case if you want to self-spawn it — send some SMH there before. Are we going to implement both ways?

does smapp already support hierarchical deterministic wallet generation?

We have some kind of derivation, but only by index. Not by derivation path as in BIP32 standard. So probably the answer is NO.

lrettig commented 2 years ago

On the picture above shown only self-spawn, so I guess that signel-sig and multi-sig accounts are self-spawn only? Are we going to implement both ways?

It should be possible to spawn a multisig in two ways: via an ordinary spawn tx (funded by another principal account), or via self-spawn in the way you describe (send some SMH to the multisig address first, then send the self-spawn tx). Will double check this with @noamnelke.

does smapp already support hierarchical deterministic wallet generation?

We have some kind of derivation, but only by index. Not by derivation path as in BIP32 standard. So probably the answer is NO.

Will open a separate issue to discuss this

noamnelke commented 2 years ago

Sorry for the late response...

IIUC @dshulyak only implemented self-spawn for genesis. I don't fully agree with this, because I think it would be almost as easy to support both modes of spawning, but there are probably more important things to argue about and we can quite easily change this later IMO.

This means that accounts can only be self-spawned, regardless of if they're single- or multi-sig.

I disagree with you @lrettig that self-spawn for multi-sig would be the "rare". I think that when you set up an account, the point where you can say you're done is when you can generate an address to receive funds. If you don't need to use the account immediately then fine, what's the benefit of having a presence on-chain? If you do need to use it immediately then the first thing you need to do is receive funds (regardless of how you spawn) - after than you can self-spawn.

Is there a case for supporting non-self-spawn - sure! But I don't consider it a "must for genesis".

dshulyak commented 2 years ago

@noamnelke btw regardless of the decision could you clarify spawn implementation in https://github.com/spacemeshos/go-spacemesh/issues/3339 . because the way i see it the tx format for spawn and self-spawn is different

lrettig commented 2 years ago

@noamnelke I think the point is that, for a multisig, a "simple" spawn requires only a single signature, i.e., it can be an "ordinary" transaction. Whereas the self-spawn case would, by definition, require multiple signatures, and the UX for this is quite tricky (I'm not sure we've even worked it out yet, I suppose that's the purpose of this issue). My gut feeling is that people would prefer to do it the easier, faster, simpler way.

lrettig commented 2 years ago

setting up a multisig account requires... the public keys of all participants... When all the signatures are collected and the config is chosen, an account address can be calculated... if a participant didn’t include the same set of public keys or the same config, then the resulting account will be different

Dumb, obvious question: this means we won't allow changing the signatories later, right? I.e., the set of authorized pubkeys is part of immutable state? This is also a usability issue, since existing multisig solutions allow adding/removing signatories, changing the minimum number of required confirmations, etc. Rather than launch with something lacking basic features people are used to, I think it might make more sense to take the time to get the design right.

tal-m commented 2 years ago

Dumb, obvious question: this means we won't allow changing the signatories later, right? I.e., the set of authorized pubkeys is part of immutable state? This is also a usability issue, since existing multisig solutions allow adding/removing signatories, changing the minimum number of required confirmations, etc. Rather than launch with something lacking basic features people are used to, I think it might make more sense to take the time to get the design right.

It's true that you can't change the keys for the basic multisig wallet that we're planning for genesis without changing the account address --- in the same way that you can't change the keys for a single signature wallet (you can always "change the keys" by transferring the entire balance to a new account with different keys). So this doesn't cover all the use-cases for more advanced multisig wallets -- those will have to wait for after genesis.

However, I think it's critical to support some version of multisig at genesis (by support, I think it's enough to have CLI support; I'm ok with the UI taking longer). The reason is that this is a basic security measure for vault accounts --- and these won't be able to change the signers in any case (by design).

I also think that a simple multisig wallet is already very useful for "regular" miners. Personally, I would be very worried about holding large amounts in a single-sig wallet -- if it's a hardware wallet it feels to easy too lose or misplace, and if the key is duplicated it seems too easy to steal. So there's a case to be made to have UI support for it as well.