-
Notifications
You must be signed in to change notification settings - Fork 148
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
Support multisig with Ledger #4203
Comments
@kyranjamie mind providing some guidance here that @jbencin could possibly use to submit a PR? |
@jbencin maybe we can schedule some time to discuss what's needed? https://fantastical.app/kyranjamie/30mins-call |
@kyranjamie Yeah I just got back to working on this, I'll set something up |
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 |
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 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 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 @kyranjamie I will prepare a PR when this ledger PR will be released or even earlier to be ready ahead of time |
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 Status of SIP-027 support in other projects |
Thanks for the update. We'll take a look at this Stacks.js upgrade and parameters addition. Which are the "correct" parameters needed here? |
Set 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 |
BTW you can easily test if a signed transaction is valid with 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,
),
} |
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
The text was updated successfully, but these errors were encountered: