TransactionBuilder.sign signature ordering for multisig

[dcousens]

Pull request for #320.

Still WIP.

Signature ordering is complete, but still not sure on the semantics behind what will be necessary for TransactionBuilder.merge.
@ianpurton I managed to get signature ordering done while trying to do the TransactionBuilder.merge implementation, but it definitely needs more tests.

Highly appreciated if you could add some to this branch.

[dcousens]

This section could be clearer, IMHO

[dcousens]

Good to merge.
Still a lot of tests required, I want to re-work the internals a bit such that there isn't so much repetition.

[dcousens]

Rebased on HEAD.

[dcousens]

No longer a decrease in coverage.
Added further test cases for different pathways.
Still could use re-factoring, but that will only come with time and experience.

Overall, good to merge.

[dcousens]

This PR adds/fixes existing functionality but doesn't change the API at all. @weilu, do we bump minor or patch?

[weilu]

sign has grown very big. I see this function half the time is responsible for preparing input, the other half doing validations along the way. Can we extract the validation code into a separate function and call validate before start prep data & signing?

[dcousens]

Agreed, will see what I can look into

[dcousens]

This PR also adds support for non-standard inputs in transactions.

[dcousens]

Rebased on master. Massively simplified the implementation and removed the initialized state.

[dcousens]

@bpdavenport @dizda and any others, I'm going to merge this tonight, would appreciate any feedback before it lands, mostly in testing.
Test cases are massively appreciated.

[coveralls]

Coverage decreased (-0.3%) to 97.96% when pulling 73bf8a4 on txbmerge into 75ca355 on master.

[dizda]

Sorry I didn't saw the mention.
Thanks I will test it ASAP, good job!

[dizda]

Ok when i'm trying to sign inputs:

// Sign the input
txb.sign(i, privKey, bitcoin.Script.fromHex(wInput.address.redeem_script));

Then I have this error:

 TypeError: Cannot read property 'hashType' of undefined
    at TransactionBuilder.sign (link to this line https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/transaction_builder.js#L271)

Obsiously all values are filled.
I tried with the master branch version.

[dcousens]

Can you give us a test or any more information so I can reproduce it?
On 17 Feb 2015 10:54 pm, "Jonathan Dizdarevic" notifications@github.com
wrote:

Ok when i'm trying to sign inputs:

// Sign the input
txb.sign(i, privKey, bitcoin.Script.fromHex(wInput.address.redeem_script));

Then I have this error:

TypeError: Cannot read property 'hashType' of undefined
at TransactionBuilder.sign (link to this line https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/transaction_builder.js#L271)

Obsiously all values are filled.
I tried with the master branch version.

—
Reply to this email directly or view it on GitHub
#329 (comment)
.

[dizda]

Ok so this is a sample script who throws this error:

var bitcoin = require('bitcoinjs-lib');

var pubKeys = [
    '0227fbadd0c4a2c8c89ab7996e5f5489dfcc3dfe7924fedd0167046166f1613eaa', // #1
    '034162edc518f6fc83ab826db81d50a8002b814b9f0b739a69560e14d3773296ee', // #2
    '0296a44a6f2ed29bec93d4a6afac0fbc243c9ea1599caad6f515a140e7f4ee61e7'  // #3
];

var privKeys = [
    'L2rVPg54JmbpUYBXbMCFHFFdZByXKT1UdaAN9zvYbgFGWoHTM4A3', // #1
    'L5h7uvxNbUwht99ewKxRedEZq7wii4RDwnVSVP327UCwWLYprysM', // #2
    'KzgkfPwEHKJzoQc8CyrdVLAB8dec861zj54BEYNaP6vALGum4K8C'  // #3
];

var redeemScript = '52210227fbadd0c4a2c8c89ab7996e5f5489dfcc3dfe7924fedd0167046166f1613eaa210296a44a6f2ed29bec93d4a6afac0fbc243c9ea1599caad6f515a140e7f4ee61e721034162edc518f6fc83ab826db81d50a8002b814b9f0b739a69560e14d3773296ee53ae';


// Recover transaction from raw_transaction created by bitcoind || or raw_signed_transaction
var tx  = bitcoin.Transaction.fromHex('0100000001a9c345321fa0866212a1852a48e32876816a1a059e03e8bfd34a2acdb8ac38170100000000ffffffff02204e0000000000001976a9140b167a9a9040e46e9c87d8c8291bbefae77c6bfe88ac503403000000000017a9147a82193d183b15125285cd88aac0b8fa9e322ac18700000000');

// Build it into Transaction Builder
var txb = bitcoin.TransactionBuilder.fromTransaction(tx);

var rawSignedTransaction;

// Loop on each inputs
txb.tx.ins.forEach(function(input, i) {

    // get the txid of the input
    var txid   = bitcoin.bufferutils.reverse(input.hash).toString('hex');

    // [...]

    // Sign the input
    txb.sign(i, bitcoin.ECKey.fromWIF(privKeys[0]), bitcoin.Script.fromHex(redeemScript));
//    txb.sign(i, bitcoin.ECKey.fromWIF(privKeys[1]), bitcoin.Script.fromHex(redeemScript));

    try {
        rawSignedTransaction = txb.build().toHex();

        console.log('Successfully signed.');
    } catch (e) {
        if ('Transaction is missing signatures' === e.message) {
            // Normal, because every inputs are not signed yet.

            rawSignedTransaction = txb.buildIncomplete().toHex();
        } else if ('Not enough signatures provided' === e.message) {
            console.log('Not enough signatures provided');

            rawSignedTransaction = txb.buildIncomplete().toHex();
        } else {
            console.log(e);
        }
    }
});

console.log(rawSignedTransaction);

Got this

/PATH/node_modules/bitcoinjs-lib/src/transaction_builder.js:271
  var canSign = input.hashType &&
                     ^
TypeError: Cannot read property 'hashType' of undefined
    at TransactionBuilder.sign (/PATH/node_modules/bitcoinjs-lib/src/transaction_builder.js:271:22)

With the version 1.4.3, I do not have any problem.

[dizda]

So did you reproduce the error?

[dizda]

Ok I've found the problem.

When I comment this line

bitcoinjs-lib/src/transaction_builder.js

Line 116 in 09d8e44

if (txIn.script.buffer.length === 0) return

the signature works again.

Dunno what is the utility of this line?

[dcousens]

Can you make a new issue for this? I lost track and wasn't aware of it
until now.
On 4 Mar 2015 12:19 am, "Jonathan Dizdarevic" notifications@github.com
wrote:

Ok I've found the problem.

When I comment this line

bitcoinjs-lib/src/transaction_builder.js

Line 116 in 09d8e44

if (txIn.script.buffer.length === 0) return

the signature works again.

Dunno what is the utility of this line?

—
Reply to this email directly or view it on GitHub
#329 (comment)
.

[dizda]

Sure!

[dizda]

There you go #374

[dcousens]

Thanks :)
On 4 Mar 2015 12:33 am, "Jonathan Dizdarevic" notifications@github.com
wrote:

There you go #374 #374

—
Reply to this email directly or view it on GitHub
#329 (comment)
.