Fix : Add possibility to not sort the public keys in Transaction.from for P2SH multisig transactions

[antonin-arquey]

Hi everyone,

Quick pull request to allow the use of the options object when creating a multisig transaction. Before we have (example 3-3 P2SH).

const transaction = new Transaction()
  .from(output, pubKeys, 3)
  .to(randomAddress, 1000);

However, this was sorting the public keys passed as parameter by default, with no way to disable it, so you had the error Provided public keys don't hash to the provided output if your output script was generated with the keys in a different unsorted order.

I added the possibility to do this :

const transaction = new Transaction()
  .from(output, pubKeys, 3, false, { noSorting: true })
  .to(randomAddress, 1000);

Passing an option object as last parameter (matchin with the format expected by Script.buildMultisigOut or Script.buildP2SHMultisigIn), so you can create and validate multisig transaction with the unsorted public keys.

[antonin-arquey]

Travis build failed after npm install on node 8, not sure why as I did not do any change to package.json. It seems that I'm not the only one with problems with the CI

[DanielRHayes]

@antonin-arquey what would be the use case to not sort the keys? If the keys are in a random order you would be required to persist that order to restore a wallet from a mnemonic. Sorting the keys was intentional in BIP 45 for that reason.

[antonin-arquey]

If the multisig wallet was not created with keys in sorted order (and bitcore-lib allows you to do that), then you can't create the transaction as the transaction builder will always sort the public keys. It's a weird behaviour to allow to create and sign ps2h scripts with unsorted keys, but then always sort the keys in the transaction builder.

[patwhite]

@DanielRHayes I'm running into this issue as well - it's possible I'm doing something a little bit different than BIP 45, but basically as I read BIP 45 you're taking the public key of the purpose derivation path (m/45') and using that to create a public key, which you then sort and assign cosigner indexes to each of the cosigners. Then, when you derive your eventual key (m/45'/Cosigner'/0/0 for instance) you're no longer going to line up with the original sort order. Maybe you read that section differently?

The index of the party creating a P2SH multisig address. The indices can be determined independently by lexicographically sorting the purpose public keys of each cosigner. Each cosigner creates addresses on its own branch, even though they have independent extended master public key, as explained in the "Address generation" section.

[patwhite]

Anything I can do to help with the testing issues?

[patwhite]

I guess I should add, I then assume you stay with the cosigner ordering as dictate by the original cosigner index selection. That is to say, as long as you keep the derivation information, you can re-derive the eventual ordering, but it won't necessarily be sorted alphabetically.

[antonin-arquey]

Yup fixed it !

[patwhite]

Someone just opened another PR to do this, maybe we could get one of the two accepted?

[matiu]

fixed in https://github.com/bitpay/bitcore-lib/pull/255 thanks!