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

Multisig transaction signing order #1487

Closed
fess-v opened this issue Apr 21, 2023 · 2 comments · Fixed by #1710
Closed

Multisig transaction signing order #1487

fess-v opened this issue Apr 21, 2023 · 2 comments · Fixed by #1710
Labels
bug Unwanted or unintended logic causing harm discussion Under consideration and/or debate docs Impacts docs or requires updated documentation icebox

Comments

@fess-v
Copy link
Contributor

fess-v commented Apr 21, 2023

Describe the bug

I don't know if it's even possible, but when I try to sign a multisig transaction starting from the third address (out of 3 public keys) it gives an error "Signer hash does not equal hash of public key(s): ba3f7c941ce749ba14f2a7c24d067948858ba16d != 0f8b02bbaa17b8df1eaede6102baef275a726914"
Tried the following workarounds from this issue, but seems like it is only possible to sign a multisig transaction starting from lower index to a higher one from the initial public keys array - like (0, 1), (1, 2) (0, 2), but not (2, 1). So when the last owner, for example, wants to initiate a multisig transaction, he can't do it anyhow, as well as if the second owner initiates the transaction - the first one won't be able to confirm it after that

How to reproduce

import {
  TransactionSigner,
  getPublicKey,
  createStacksPrivateKey,
  publicKeyToString,
  standardPrincipalCV,
  broadcastTransaction,
  makeUnsignedSTXTokenTransfer,
} from '@stacks/transactions';
import { StacksTestnet } from '@stacks/network';
import BigNum from 'bn.js';

// Private key strings with compression byte appended
const privKeyStrings = [
  '6d430bb91222408e7706c9001cfaeb91b08c2be6d5ac95779ab52c6b431950e001',
  '2a584d899fed1d24e26b524f202763c8ab30260167429f157f1c119f550fa6af01',
  'd5200dee706ee53ae98a03fba6cf4fdcc5084c30cfa9e1b3462dcdeaa3e0f1d201',
];
const privKeys = privKeyStrings.map(createStacksPrivateKey);

// Compressed public keys
const pubKeys = privKeys.map(getPublicKey);
const pubKeyStrings = pubKeys.map(publicKeyToString);

const recipient = standardPrincipalCV('SN3C9PPBTKJNWJ5HZBBWERRAF4E3W6M39AN54S92');
const amount = new BigNum(74);
const fee = new BigNum(278);
const nonce = new BigNum(0);
const memo = 'test';
const network = new StacksTestnet();

(async () => {
  const transaction = await makeUnsignedSTXTokenTransfer({
    recipient,
    amount,
    network,
    fee,
    nonce,
    memo,
    numSignatures: 2,
    publicKeys: pubKeyStrings,
  });

  const signer = new TransactionSigner(transaction);
  signer.signOrigin(privKeys[2]);
  signer.appendOrigin(pubKeys[0]);
  signer.signOrigin(privKeys[1]);


  const txId = await broadcastTransaction(transaction, network);
  console.log('Transaction ID: ' + JSON.stringify(txId, null, 2).replace(/^"|"$/, ''));
})().catch((err) => {
  console.error('Error:' + err);
  process.exit(1);
});

Expected behavior

It should be possible to initiate a signature from whatever owner

@fess-v fess-v added the bug Unwanted or unintended logic causing harm label Apr 21, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in DevTools Apr 21, 2023
@janniks
Copy link
Collaborator

janniks commented Apr 24, 2023

I will double-check, but I believe this is how multi-sig txs work in Stacks, the signing needs to be in-order.

@fess-v
Copy link
Contributor Author

fess-v commented Apr 25, 2023

thank you!
in this scenario, it is not possible to start signing with the last owner, for example, it adds a lot of restrictions on signature flows and makes it pretty complicated to make a user-friendly way for multisig owners

@janniks janniks added discussion Under consideration and/or debate docs Impacts docs or requires updated documentation labels Apr 26, 2023
@hugocaillard hugocaillard moved this from 🆕 New to 🔖 To do in DevTools Apr 26, 2023
@janniks janniks moved this from 🔖 To do to ❄️ Icebox in DevTools Jun 26, 2023
@smcclellan smcclellan moved this from ❄️ Icebox to 📋 Backlog in DevTools Jul 7, 2023
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in DevTools Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unwanted or unintended logic causing harm discussion Under consideration and/or debate docs Impacts docs or requires updated documentation icebox
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants