leather-io / extension

Leather browser extension
https://leather.io
MIT License
305 stars 144 forks source link

Support multisig with Ledger #4203

Open jbencin opened 1 year ago

jbencin commented 1 year ago

I've made a PR to the Ledger Stacks app to add support for multisig transactions where the number of signers is greater than 2: Zondax/ledger-stacks#152

In order to work with this PR, the wallet may require some minor changes. Currently, I believe the only change needed will be the following:

It might even work like this already, I'm not sure, but it seems like the Ledger app previously did not make this assumption

markmhendrickson commented 1 year ago

@kyranjamie mind providing some guidance here that @jbencin could possibly use to submit a PR?

kyranjamie commented 1 year ago

@jbencin maybe we can schedule some time to discuss what's needed? https://fantastical.app/kyranjamie/30mins-call

jbencin commented 1 year ago

@kyranjamie Yeah I just got back to working on this, I'll set something up

jbencin commented 1 year ago

Just an update on this: I ended up writing a CLI tool to perform the transfers. See this project.

I'll update this issue again when the PR to the Ledger app gets merged upstream, at which point it will be necessary to add support in the wallet

fess-v commented 1 year ago

Hey @kyranjamie @markmhendrickson ! For old multisig transactions (not the order-independent ones) some changes need to be made.

First of all, you need to add a similar function but for multisig transactions Your function:

export function signTransactionWithSignature(transaction: string, signatureVRS: Buffer) {
  const deserialzedTx = deserializeTransaction(transaction);
  const spendingCondition = createMessageSignature(signatureVRS.toString('hex'));
  (deserialzedTx.auth.spendingCondition as SingleSigSpendingCondition).signature =
    spendingCondition;
  return deserialzedTx;
}

Need to add the one for multisig, for example:

export function signTransactionWithSignature(transaction: string, signatureVRS: Buffer) {
  const deserialzedTx = deserializeTransaction(transaction);
  const spendingCondition = createMessageSignature(signatureVRS.toString('hex'));
  const fields = (deserialzedTx.auth.spendingCondition as MultiSigSpendingCondition).fields;
  (fields[fields.length - 1].contents as MessageSignature) = spendingCondition;

  return deserialzedTx;
}

Before sending a transaction to ledger for signing, you need to append the signer public key like this:

tx.appendPubkey(createStacksPublicKey(o.publicKey));

After signature, you will use the signTransactionWithSignature function to exchange the appended public key with a signature, similar to a single sig transaction. That's why it has fields.length - 1 because the signer should be the last owner in the fields array anyway.

The second change will be later to consider the Stacks app version since the older versions cannot work properly with multisig transactions - once the new version with this change is released we will add this verification step on our side as well.

Later on, when the order-independent multisig will be closer to release I'll drop a few integration changes for that too.

Btw, to test it you can just connect the wallet to Asigna and make any multisig transaction through the RPC request that we added before. There you'll be able to verify the Ledger integration easily.

markmhendrickson commented 1 year ago

First of all, you need to add a similar function but for multisig transactions

Is this an enhancement that either @fess-v or @jbencin you'd be interested in submitting for review as a PR?

fess-v commented 1 year ago

@markmhendrickson @kyranjamie I will prepare a PR when this ledger PR will be released or even earlier to be ready ahead of time

jbencin commented 3 months ago

I'm not sure if any work has been done to support this on Ledger yet, but I wanted to update this issue since SIP-027: non-sequential multisig transactions just passed (vote results) and everything is now official

I think it should be pretty simple to add SIP-027 support to Leather. To support signing SIP-027 transactions, I believe this will only require updating Stacks.js, which can detect the transaction type and apply the appropriate signature. To allow a user to initiate a SIP-027 transaction, it will also require a UI option to pass the correct parameters to makeUnsignedSTXTokenTransfer(). That should be it

Status of SIP-027 support in other projects

markmhendrickson commented 3 months ago

Thanks for the update. We'll take a look at this Stacks.js upgrade and parameters addition.

Which are the "correct" parameters needed here?

jbencin commented 3 months ago

Which are the "correct" parameters needed here?

Set useNonSequentialMultiSig to true when calling makeUnsignedSTXTokenTransfer() (docs)

That should be all that's needed to create a SIP-027 multisig tx. The process for signing the txs doesn't change at all

Don't hesitate to reach out to me if you have any more questions or issues

jbencin commented 3 months ago

BTW you can easily test if a signed transaction is valid with stacks-inspect. Hex-encode the transaction and run this in the stacks-core repository:

cargo build --release --bin=stacks-inspect
target/release/stacks-inspect decode-tx <hex-string>

Example:

❯ ./target/release/stacks-inspect decode-tx 8080000000040556da933238491425e460d335d3af8e04fd3e5997000000000000000000000000000000000000000202018ff6ba68b32f664fd2f3393efd9755c3f134380d337c43f244becb601a56469c1d9a110d67ec30d651ac05245b5841df61bb20336b070fb867db7efadf83038002012af60efd380bfcca29298304ea4d96fc46f4022901bc59389895d4bc7c07269a58949f1e2bbf6dc118818fd6b404766d7f62985c9e1859620480a683177350ea000203020000000000051abaa6de6c1badf30afa816e2c66db3125034facab00000000002625a06d756c74697369672074780000000000000000000000000000000000000000000000
Verified: Ok(
    (),
)
Address: SM1BDN4SJ714H89F4C39KBMXFHR2FTFJSJZ8Z711B
StacksTransaction {
    version: Testnet,
    chain_id: 2147483648,
    auth: Standard(
        OrderIndependentMultisig(
            OrderIndependentMultisigSpendingCondition {
                hash_mode: P2SH,
                signer: 56da933238491425e460d335d3af8e04fd3e5997,
                nonce: 0,
                tx_fee: 0,
                fields: [
                    Signature(
                        Compressed,
                        018ff6ba68b32f664fd2f3393efd9755c3f134380d337c43f244becb601a56469c1d9a110d67ec30d651ac05245b5841df61bb20336b070fb867db7efadf830380,
                    ),
                    Signature(
                        Compressed,
                        012af60efd380bfcca29298304ea4d96fc46f4022901bc59389895d4bc7c07269a58949f1e2bbf6dc118818fd6b404766d7f62985c9e1859620480a683177350ea,
                    ),
                ],
                signatures_required: 2,
            },
        ),
    ),
    anchor_mode: Any,
    post_condition_mode: Deny,
    post_conditions: [],
    payload: TokenTransfer(
        Standard(
            StandardPrincipalData(ST2XADQKC3EPZ62QTG5Q2RSPV64JG6KXCND0PHT7F),
        ),
        2500000,
        6d756c74697369672074780000000000000000000000000000000000000000000000,
    ),
}