Skip to content

TransactionBuilder P2SH Multisig order of signatures (alternative implementation) #349

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

TransactionBuilder P2SH Multisig order of signatures (alternative implementation) #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c0d2cc
add failing test case for when P2SH multisig is signed in a different…
rubensayshi Feb 3, 2015
4f9d87b
now ordering signatures based on pubKeys for P2SH multisig
rubensayshi Feb 3, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions src/transaction_builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ TransactionBuilder.prototype.__build = function(allowIncomplete) {
var scriptSig
var scriptType = input.scriptType

var signatures = input.signatures.map(function(signature) {
var signatures = input.signatures.filter(function(signature) { return !!signature }).map(function(signature) {
return signature.toScriptSignature(input.hashType)
})

Expand Down Expand Up @@ -223,7 +223,7 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
var prevOutScript = this.prevOutScripts[index]
var prevOutType = this.prevOutTypes[index]

var scriptType, hash
var scriptType, hash, pubKeys
if (redeemScript) {
prevOutScript = prevOutScript || scripts.scriptHashOutput(redeemScript.getHash())
prevOutType = prevOutType || 'scripthash'
Expand All @@ -235,6 +235,13 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
assert.notEqual(scriptType, 'scripthash', 'RedeemScript can\'t be P2SH')
assert.notEqual(scriptType, 'nonstandard', 'RedeemScript not supported (nonstandard)')

// for multisig we need to get the pubKeys from the redeemScript since their order is important
if (scriptType === 'multisig') {
pubKeys = redeemScript.chunks.slice(1, -2).map(function(pubKey) {
return ECPubKey.fromBuffer(pubKey)
})
}

hash = this.tx.hashForSignature(index, redeemScript, hashType)

} else {
Expand Down Expand Up @@ -268,8 +275,25 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
assert.deepEqual(input.redeemScript, redeemScript, 'Inconsistent redeemScript')

var signature = privKey.sign(hash)
input.pubKeys.push(privKey.pub)
input.signatures.push(signature)

if (pubKeys) {
var multisigSignatures = {};
input.signatures.filter(function(signature) { return !!signature }).forEach(function(signature) {
pubKeys.forEach(function(pubKey, i) {
if (!multisigSignatures[i] && pubKey.verify(hash, signature)) {
multisigSignatures[i] = signature
}
})
})
input.signatures = pubKeys.map(function(pubKey, i) { return multisigSignatures[i] || null })
var signatureIndex = pubKeys.map(function(pubKey) { return pubKey.toBuffer().toString('hex') }).indexOf(privKey.pub.toBuffer().toString('hex'))
Copy link
Contributor Author

@rubensayshi rubensayshi Feb 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a better way to do .indexOf for the pubKey?

assert.notEqual(signatureIndex, -1)
Copy link
Contributor Author

@rubensayshi rubensayshi Feb 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's 'correct' to fail here if we're esentially signing with a privKey for which there is no pubKey?
And I guess I should add a testcase for it!

Copy link
Contributor

@dcousens dcousens Feb 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/bitcoinjs/bitcoinjs-lib/pull/329/files#diff-f18685c2b076ef917fa091a6f60a0dd4R345

Copy link
Contributor

@dcousens dcousens Feb 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a tonne of fun to deal with pubKey inputs properly as well.

input.signatures[signatureIndex] = signature;
input.pubKeys = pubKeys
} else {
input.signatures.push(signature)
input.pubKeys.push(privKey.pub)
}
}

module.exports = TransactionBuilder
27 changes: 27 additions & 0 deletions test/transaction_builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,5 +290,32 @@ describe('TransactionBuilder', function() {

assert.equal(tx2.toHex(), '0100000001cff58855426469d0ef16442ee9c644c4fb13832467bcbc3173168a7916f0714900000000fd1c01004830450221009c92c1ae1767ac04e424da7f6db045d979b08cde86b1ddba48621d59a109d818022004f5bb21ad72255177270abaeb2d7940ac18f1e5ca1f53db4f3fd1045647a8a8014830450221009418caa5bc18da87b188a180125c0cf06dce6092f75b2d3c01a29493466800fd02206ead65e7ca6e0f17eefe6f78457c084eab59af7c9882be1437de2e7116358eb9014c8752410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b84104c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a52aeffffffff0110270000000000001976a914751e76e8199196d454941c45d1b3a323f1433bd688ac00000000')
})

it('works for the P2SH multisig case with signing out of order', function() {
// same test case as above but privKeys are in different order
var privKeys = [
"91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgww7vXtT",
"91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgwmaKkrx"
].map(ECKey.fromWIF)
// 2of2
var redeemScript = Script.fromASM("OP_2 0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 04c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a OP_2 OP_CHECKMULTISIG")

txb.addInput("4971f016798a167331bcbc67248313fbc444c6e92e4416efd06964425588f5cf", 0)
txb.addOutput("1BgGZ9tcN4rm9KBzDn7KprQz87SZ26SAMH", 10000)
txb.sign(0, privKeys[0], redeemScript)

var tx = txb.buildIncomplete()

// in another galaxy...
// ... far, far away
var txb2 = TransactionBuilder.fromTransaction(tx)

// [you should] verify that Transaction is what you want...
// ... then sign it
txb2.sign(0, privKeys[1], redeemScript)
var tx2 = txb2.build()

assert.equal(tx2.toHex(), '0100000001cff58855426469d0ef16442ee9c644c4fb13832467bcbc3173168a7916f0714900000000fd1c01004830450221009c92c1ae1767ac04e424da7f6db045d979b08cde86b1ddba48621d59a109d818022004f5bb21ad72255177270abaeb2d7940ac18f1e5ca1f53db4f3fd1045647a8a8014830450221009418caa5bc18da87b188a180125c0cf06dce6092f75b2d3c01a29493466800fd02206ead65e7ca6e0f17eefe6f78457c084eab59af7c9882be1437de2e7116358eb9014c8752410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b84104c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a52aeffffffff0110270000000000001976a914751e76e8199196d454941c45d1b3a323f1433bd688ac00000000')
})
})
})