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

Sip 02x non sequential multisig transactions #152

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

jbencin
Copy link

@jbencin jbencin commented Aug 31, 2023

While implementing Stacks multisig in the Ledger app (Zondax/ledger-stacks#152), I found the current multisig format confusing and hard to work with. Other developers that have tried to work with multisig seem feel the same way (example: PR #139), and as far as I know there is currently no Stacks wallet with full multisig support. So wrote up this SIP which makes slight modifications to SIP-005 to add a new multisig transaction type which is a bit simpler and allows participants to sign in any order.

@AcrossfireX
Copy link

@jbencin - can you comment on how this relates to stacks-network/stacks-core#3710 and its associated SIP draft? Is it possible to fold these into one shared SIP effort? Let me know if it makes more sense for them to remain seperate.

@jbencin
Copy link
Author

jbencin commented Sep 21, 2023

@AcrossfireX That PR is now implementing this draft SIP. I know there is another draft SIP mentioning order-independent multisig (#139), but that one lacks implementation details

@fess-v
Copy link

fess-v commented Sep 21, 2023

I will close the previous SIP PR so we can move all discussions to this one instead @jbencin @AcrossfireX

Copy link
Collaborator

@MarvinJanssen MarvinJanssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I welcome this SIP as it will make multisig much easier to use on Stacks. Excited to see more work in this direction. Some initial comments.

signature_1(tx), signature_2(tx), ..., signature_n(tx)
```

This would address all of the concerns listed above, and would not increase transaction size or make it easier to forge a signature
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

providing a small real-world use-case or example demonstrating these drawbacks might make the introduction even more vivid for readers who might not be deeply familiar with the subject of multisig wallets

Copy link

@fess-v fess-v Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add something like:
Imagine a DAO that has 5 management team members. They decide to make a 3 out of 5 multisig on Stacks.
With the current version, signing a transaction in the creation order is required, i.e., after the 3rd owner only the 4th and 5th can sign, after the 5th owner signed - no one can add another signature.
It makes usage of the multisig painful for a lot of scenarios, for example:

  • It is impossible to start signing a transaction from the 4th and 5th owners since they won't have enough owners after them to finalize the transaction
  • If the 1st owner initiated a transaction, the 4th owner signed, the 2nd and 3rd owners can't sign this transaction anymore, so only the 5th owner can finalize it, the case of 2nd or 3rd owners being online and 5th not - they need to wait until the last owner makes this signature.
  • If the 3rd owner initiated and signed a transaction, he needs the 4th and 5th owners' signatures in order to finalize it. 1st and 2nd owners can't participate until they make a new signing flow for that transaction.

In short, for small multisig wallets it seems to be not a blocking problem, but for multisig wallets with 4 or more owners, it becomes really hard to initiate transactions and get signatures in place.
@jo-tm @jbencin

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fess-v This is a good example, it shows that when you establish signer order by creating a multisig address, not all signers are equal. Those near the top of the list have much more flexibility than those near the bottom

Would you please add this to the draft SIP? While you're at it, add your name and email to the authors list, you've done a lot of work on this

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! will do that soon, thank you @jbencin

@AcrossfireX
Copy link

This SIP should be considered Accepted from the SIP Editors and should progress to technical CAB review. (Likely already underway)

## Examples

Imagine a DAO that has a management team comprised of five members.
They create a 3 out of 5 multisig account on Stacks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine lines 60-61:

Imagine a DAO that has a management team comprised of five members, and tey create a 3 out of 5 multisig account on Stacks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it's considered a best practice in Markdown to write one sentence per line. It displays the same, but makes diffs and merges easier


The steps for signing a non-sequential multisig transaction (hash modes `0x05` and `0x07`) shall be as follows:

0. Set the spending condition address, and optionally, its signature count.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for SIPS/documents in general, we should start numbered lists with 1 rather than the technically correct 0.
Cosmetic change - I don't feel strongly if it remains starting at 0 though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep it at 0 only because this is a modification of SIP-05 and that uses 0 indexing for these steps

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the signature count should be optional. Either this algorithm needs to work without it, or it should be mandatory.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I don't the code actually treats it as optional. This line was copied directly from SIP-005, so we may want to revise that also


The steps for verifying a non-sequential multisig transaction (hash modes `0x05` and `0x07`) shall be as follows:

0. Extract the public key(s) and signature(s) from the spending condition.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for SIPS/documents in general, we should start numbered lists with 1 rather than the technically correct 0.
Cosmetic change - I don't feel strongly if it remains starting at 0 though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are the public keys and signatures encoded?

0. Set the spending condition address, and optionally, its signature count.
1. Clear the other spending condition fields, using the appropriate algorithm below.
If this is a sponsored transaction, and the signer is the origin, then set the sponsor spending condition
to the "signing sentinel" value (see below).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this (see below) point more directly? I don't see any other reference to a "signing sentinel".
example:

..(see below in point 4)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be defined in SIP-05 where I copied it from

Co-authored-by: wileyj <2847772+wileyj@users.noreply.github.com>
Co-authored-by: Brice Dobry <brice@obycode.com>
@jbencin
Copy link
Author

jbencin commented Oct 27, 2023

One more small thing I want to add here...

Even though the signing order is independent, the order of the public keys in the auth fields still determines how the address is generated. For backwards compatibility and to enable previously generated multisig accounts to use order-independent signing, I don't think we should mandate public key order, but I think we should recommend a public key ordering of least to greatest (which is equivalent to lexicographical sorting of hex-encoded values) to remove the guesswork when creating a transaction

@AcrossfireX
Copy link

@jcnelson - would we be able to get this in front of the Steering Committee for final approval and merge as the activation points to it being ready for epoch 3.0? It has been approved by the SIP editors and Technical CAB and seems ready for final discussions by the Steering Comittee.

Also would you prefer that the editors assign a formal SIP number to this as it has not yet been chosen. It appears that SIP-026 would be available.

obycode
obycode previously approved these changes Mar 27, 2024
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This SIP was approved by the Technical CAB on 27 Oct 2023

There are a few drawbacks to doing it this way:

- The order in which the signers must sign is fixed as soon as funds are sent to a multisig account, which limits flexibility when creating a spending transaction from a multisig account
- The process of signing a transaction requires each signer to validate the entire signature chain before signing, in order to make sure it matches the transaction, leading to `O(n^2)` signing times
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight correction -- it requires each signer to perform O(n) signature verifications. The O(n^2) total verifications is distributed across n signers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll clarify

to the "signing sentinel" value (see below).
2. Serialize the transaction into a byte sequence, and hash it to form an
initial `sighash`.
3. Calculate the `presign-sighash` over the `sighash` by hashing the
Copy link
Contributor

@jcnelson jcnelson Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific in what you mean by "hashing the sighash with ... here? Do you mean to say that you concatenate the sighash with these fields?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this was copied straight from SIP-005, so if we change one we should change the other

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I'm pointing out here is that this step is not well-specified when taken out of context. How is the reader supposed to know what hash algorithm to use, for example? Can't the text just say it here?

and the nonce (as an 8-byte big-endian value).
4. Calculate the ECDSA signature over the `presign-sighash` by treating this
hash as the message digest. Note that the signature must be a `libsecp256k1`
recoverable signature. Store the message signature and public key encoding byte as a signature auth field.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please further specify whether or not the recoverable signature is in vrs or rsv order

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specified in SIP-005. I was trying to keep this document minimal and not reproduce everything here. I mention on lines 75-76 that for anything not explicitly mentioned here, SIP-005 still applies

4. Calculate the ECDSA signature over the `presign-sighash` by treating this
hash as the message digest. Note that the signature must be a `libsecp256k1`
recoverable signature. Store the message signature and public key encoding byte as a signature auth field.
5. Repeat step 4 until the signer threshold is reached.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing some steps related to how to serialize the resulting public keys and signatures into the transaction.

The steps for verifying a non-sequential multisig transaction (hash modes `0x05` and `0x07`) shall be as follows:

0. Extract the public key(s) and signature(s) from the spending condition.
1. Clear the spending condition.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain what it means to "clear" the spending condition with respect to how it affects the binary representation. Someone implementing this SIP needs to know this in order to calculate the sighash in the next step.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zero it. Again, same language as SIP-005

6. Repeat step 4 for each signature, so that all of the public keys are
recovered.
7. Verify that the sequence of public keys hash to the address, using
the address's indicated public key hashing algorithm.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, SIP-005 includes a description of how each TransactionAuth struct is laid out in memory, as well as a description of what each of the bytes mean. This is missing from this SIP. Please add it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to say that everything in SIP-005 still applies. Can you think of a way to make this clearer without copying the entire text of SIP-005's "Transaction Encoding" section?

recovered.
7. Verify that the sequence of public keys hash to the address, using
the address's indicated public key hashing algorithm.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a missing requirement in this section is to mandate a particular public key representation. SIP-005 mandates a compressed representation, with an extra byte to indicate whether or not the uncompressed representation should be used when calculating the address.

Does this SIP require that all public keys be compressed, or does it allow the same flexibility as SIP-005? I think it needs to to the latter, since the design of the TransactionAuth structs is meant to support representing the same public key formats as those supported in Bitcoin. The reason for this is that it's possible to determine the Stacks address of users who hold their STX with Bitcoin keys -- because the 20-byte hash component of Stacks and Bitcoin addresses is calculated the same way, these users (and counterparties that interact with them) can determine their funds' Stacks address without knowing the public keys. I think we need to preserve that property here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as SIP-005. Either format is allowed except in P2WSH where uncompressed keys are not

@jbencin
Copy link
Author

jbencin commented Mar 29, 2024

@jcnelson I copied all the text from SIP-005's "Transaction Encoding" section and merged it with this SIP. This can now be viewed as a complete replacement for that section

@jbencin jbencin force-pushed the sip-02x-non-sequential-multisig-transactions branch 2 times, most recently from 0345acd to a82bfa0 Compare March 29, 2024 20:12
@jbencin jbencin force-pushed the sip-02x-non-sequential-multisig-transactions branch from a82bfa0 to e099ef6 Compare March 29, 2024 20:23
is used to distinguish between each chain's transactions. Transactions for the
main Stacks blockchain MUST have a chain ID of `0x00000000`.

#### Transaction Authorization
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jbencin, you only need to include the description of the new "Transaction Authorization" data this SIP presents. You don't need anything else. Specifically, the reader needs to be able to read this section, and know exactly how to construct byte-for-byte a valid, well-formed TransactionAuth that supports non-sequential signatures.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcnelson I guess I misunderstood what you were asking for and included too much this time. I've updated it again so that it this SIP only contains the "Transaction Authorization" and "Transaction Signing and Verifying" sections. Because it was easy to do, these sections also contain details for the old multisig formats, so that it's no longer necessary for the user to read the corresponding sections of SIP-005

It does not remove support for the current format, rather it is intended to co-exist with the old format and give users a choice of which format to use.

The issue with the current format is that it establishes a signer order when funds are sent to multisig account address, and requires signers to sign in the same order to spend the funds.
In practice, the current format has proven difficult to understand and implement, as evidenced by the lack of Stacks multisig implementations today.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In practice, the current format has proven difficult to understand and implement, as evidenced by the lack of Stacks multisig implementations today.
In practice, the current format has proven difficult to understand and implement, as evident by the lack of Stacks multisig implementations today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants