bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.65k stars 2.09k forks source link

Adding signature data to transaction with external signature #1142

Closed patwhite closed 5 years ago

patwhite commented 6 years ago

I'm trying to use bitcoin.js with a hardware wallet, where the wallet would provide the signature, but bitcoin.js would produce the final transaction hex (so I don't have to manually construct it, though that is an option). This is a 1 of 2 multi-sig transaction. The transaction ids are on testnet if you'd like to examine them.

Excuse the somewhat poor code here:

const sig = getHardwareSig()
const redeem = "512103bb9dad581f8942409140bb6082c39e08d286b8d841064ad2535a3e9743ff192e21031eb52a32cfa8da5dc1d66caab04037c57caaf2d8fc36796c7dfd485af0b1ad4e52ae"
var buf = new Buffer(redeem)
txb.addInput('89141edf593b88908f14192b6b6d782b5a7b128530f5ad49085f0851b604b757', 0, null, buf)
txb.addOutput('2N6GwVhv9G4rq8VpiCbQQ6Vged5hxZVDXRy', 9000)
txb.inputs[0].signatures = []
txb.inputs[0].signatures[0] = sig
const tx = txb.build()
var fullTx = tx.toHex()

This results in an error:

Uncaught Error: nonstandard not supported
at eval (transaction_builder.js?c914:638)
at Array.forEach (<anonymous>)
at TransactionBuilder.__build (transaction_builder.js?c914:630)
at TransactionBuilder.build (transaction_builder.js?c914:616)

Is this a supported scenario? Is it possible to get this code to work?

patwhite commented 6 years ago

I saw this in the docs: image

Failing to provide the redeem script caused a separate issue around not being able to build the transaction.

patwhite commented 6 years ago

Realized I was misusing the Buffer(rs) - I fixed that issue and was actually able to get a transaction out. I'm verifying it now, but I'd love some feedback if this is the best way to handle this problem.

Thanks in advance!

Marxpark commented 6 years ago

I'm having a similar problem when trying to add a nano ledger signature for a transaction. I prepare the transaction online and sign it with my ledger offline. Then I take the signature back online, and try to add it to the transaction. I've managed to get this process working with signature generated with seed, but not for signatures generated with nano ledger.

I.E. take this transaction: "raw" : "0100000002bbd7d20a465df3273acc601974d0b63b9ca01c4627b91373dc43a3fdacc55e1600000000fd0c010001ff01ff01ff4d0201514c53ff043587cf02a7ae837c00000000fc3b710814d1c580d7c5e4f25d04a69d67e3d3fdeb334307dfb2b2b3ec619daa0289bf3997905d0ac81769d11d47514982d13a97b2cef0dac11adc851071e47d0b000002004c53ff043587cf027c2a4a2500000000eec33819d9486e08fa05ef0b8fb847c9ba5334d6a64406652ff11fe514b22054024a03b0ce0ad406d855e89abbda68c8395acab7ec8b7f79a07ce579e68086382a000002004c53ff043587cf02c6c9796700000000b14f71c46bc20db6b3a225df2d4dcfa513f6beb82c81c24d22cae930c1880220020c5e399d6fadff34a2d652874f345d8de872aee6fbb04388047a6a3b9440052d0000020053aefdffffff66b4b2213e1452f97015f7c86c566d976c3706263b41af695cf14073d052586700000000fd0c010001ff01ff01ff4d0201514c53ff043587cf02a7ae837c00000000fc3b710814d1c580d7c5e4f25d04a69d67e3d3fdeb334307dfb2b2b3ec619daa0289bf3997905d0ac81769d11d47514982d13a97b2cef0dac11adc851071e47d0b000002004c53ff043587cf027c2a4a2500000000eec33819d9486e08fa05ef0b8fb847c9ba5334d6a64406652ff11fe514b22054024a03b0ce0ad406d855e89abbda68c8395acab7ec8b7f79a07ce579e68086382a000002004c53ff043587cf02c6c9796700000000b14f71c46bc20db6b3a225df2d4dcfa513f6beb82c81c24d22cae930c1880220020c5e399d6fadff34a2d652874f345d8de872aee6fbb04388047a6a3b9440052d0000020053aefdffffff0277822d010000000017a9140e480e9f1ffd4bccafdbf389cf103917b5e8dc508777822d010000000017a91474bb730439d6d06d888f87ba8bfa63e63253c8798712b21400", "inputs" : [ { "address" : "2MtYjmavgKb6scxxnTCXFybWXhqDFQWVx13", "value" : 29759735, "txid" : "165ec5acfda343dc7313b927461ca09c3bb6d0741960cc3a27f35d460ad2d7bb", "vin" : 0, "raw" : "0100000001ef24f8ed17c5b4d534b9bb132d5c8bdb05f023c49b212e1198c2a8646643041901000000b40047304402201e6df191ee1b910c359a6624f2926fd1d298a612dd5c1ef0771f4bfc503f7a270220544e4ead9b285e964fa75a70b69014f92997d063a61b262852fc9954af92088f014c69512102c716305cc0333df917826f103b60af9147b79dbf3feff533f5798d40470ca61421039c16a8e32dbc78392dd132ca3eecf154e4dfbcf1e40e79832fa315c10d3947582103f078f1e789df8a57a6bd9c17f8823be3924b91744ee1178df69564373828364753aeffffffff02f718c6010000000017a9140e480e9f1ffd4bccafdbf389cf103917b5e8dc5087dcc85f040000000017a914f2a8fb4d8f461a20f38ff080269bb3d720c4241b8700000000", "is_change" : 0, "address_index" : 2 }, { "address" : "2MtYjmavgKb6scxxnTCXFybWXhqDFQWVx13", "value" : 9761807, "txid" : "675852d07340f15c69af413b2606376c976d566cc8f71570f952143e21b2b466", "vin" : 0, "raw" : "0100000001ef24f8ed17c5b4d534b9bb132d5c8bdb05f023c49b212e1198c2a8646643041900000000b500483045022100daeb6e8649c5e80ae9da30fb9a5852d41eb50245bff987a1a328b6a83b8c321a022046b70787f3a6fceba07336e5d0d8f64b4a3b4fe5d84396792db77cd785ef01cf014c69512102f3f60b8011536bd40befb5f3407597b05f4612417f51922baac7241e015b43a92103414bdbfc18cc31e900fa68ecd3620d8c8b66e5f078c7e94f9148456a924b018b2103ce752be2a04df05b760ea0b244389294d9f4964a01d857e1af88e0772a60bc4253aeffffffff020ff494000000000017a9140e480e9f1ffd4bccafdbf389cf103917b5e8dc50878d6e97000000000017a914359f468d55bc05645e243f5aacb4f29693e5e93c8700000000", "is_change" : 0, "address_index" : 2 } ], "outputs" : [ { "address" : "2MtYjmavgKb6scxxnTCXFybWXhqDFQWVx13", "value" : 19759735 }, { "address" : "2N3tSvjvLWSkwJ2rAioGobwuUkgba3zQ7hr", "value" : 19759735 } ]

Signing with the proper seed produces these two signatures: { "input_index" : 1, "owner_index" : 0, "signature" : "304402205619a5c49d4aecea597211301b46abb736b1cc5e73e449cfd575327b97d7b06b022069aa58c87886bf46bb1d97bebd347415a92136b84219f1b6badba673a610208301" }, { "input_index" : 0, "owner_index" : 0, "signature" : "30440220426f02fd86ac480a1bb7ee9f44bc64d7fc30bb54b5d5de85698c5eeb2f7ac84202206522cc6aac260f0d5c563095c42cee755bdd3b4d5bf4a8034a0e889fd0c40d7201" }

Rebuilding the transaction and adding these two signatures works fine and i can broadcast the transaction.

Signing it with ledger produces these two signature: { "input_index" : 1, "owner_index" : 2, "signature" : "304402204d382486bc39afc8e75efcfeff97b7987a66c5544ec4b069889abe1669fde0b502206d84a92dba3e49626b29090e4d06f5698fa299ef563276afdb3b23b6d77d7a05" }, { "input_index" : 0, "owner_index" : 2, "signature" : "30440220073be815ddb6cb05ce55acba49ad9b5d1bcb5ac707d3239161168500914ef8ad022045b6a5a7b221bc105afdef4a335fd1afe8a9d4f0c95362c0f85fc4ceee398f95" }

Which can not be added to the transaction with the TransactionBuilder class. At first the issue was with the last byte of the signature - the TransactionBuilder wants the signature to end with 0x01, or throws an invalid hashtype error. If add the 0x01 at the end, the Builder adds the signature to the transaction, but when i try to broadcast it I get the following error:

Error: {“result”:null,“error”:{“code”:-26,“message”:“16: mandatory-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)“},“id”:“ebfe453b-924e-4d31-bb19-e70a93a11436"}

I haven't been able to get past this. I've tested the signature with another library and the broadcast was successful, so i'm certain that the signature is valid. It must be something more in the bscript.signature.decode / encode functions that doesn't support ledger signatures.

TL;DR: how to add a nano ledger signature to a bitcoin transaction via TransaciontBuilder class?

dcousens commented 6 years ago

Is this a supported scenario? Is it possible to get this code to work?

Yes.

If you could let us know how the examples didn't clarify this problem, I'd be happy to update them.

Though it does look like they helped you out in the end :).

A good pointer is to avoid manipulating Transaction directly unless you know exactly what you are doing. Use TransactionBuilder.

dcousens commented 6 years ago

@Marxpark, sorry for not seeing this earlier. Could you show us the code you are using for bitcoin.js?

ZebPayDhruvil commented 5 years ago

Hi can you help with how did you achieve it finally , I am trying to do something similar and currently cannot find a way to attach a hex signature received from a HW device to transaction

Marxpark commented 5 years ago

Hello again,

I've managed to get the adding of HW signatures working as such (at least for ledger)

builder.sign(input_index, { publicKey: signersPublicKey, getPublicKey: () => signersPublicKey, sign: () => bitcoin.script.signature.decode(ledgerSignatureHex + "01").signature }, redeemScript); You have to override the sign() method that's called within builder.sign() to return the hash that you already have. Because Bitcoin.js builder uses a few custom bytes in encoding and decoding things, you have to add "01" to the HW signature. This is working for me for now.

Hope it helps. GlHf

ning-tan commented 5 years ago

Is this a supported scenario? Is it possible to get this code to work?

Yes.

If you could let us know how the examples didn't clarify this problem, I'd be happy to update them. ... Use TransactionBuilder.

Hi @dcousens, I'm enjoying using bitcoinjs so far. Thanks so much for such a useful and beautifully written library.

I have a very similar need as the original poster as well as @Marxpark. Namely, adding an external signature (e.g. signed by an HSM) to a Transaction(Builder) managed by bitcoinjs. Would you mind pointing me to the right examples that show the TransactionBuilder code that allows one to add such a signature? For this use case, we can assume that the HSM can only produce the right signature for a transaction hash, but cannot itself add the signature to the transaction.

I understand @Marxpark's implementation in his latest reply. He provided a custom keypair object that simply returned the external signature when asked to sign(). I consider it rather "advanced" usage of the TransactionBuilder. :)

To respond to your comments quoted above, the TransactionBuilder-related examples did fail to clarify the problem for me, as none of them is close to supporting such a use case. Would you mind providing a better example, if one exists that's better than @Marxpark's solution?

dakk commented 5 years ago

I'm using @Marxpark solution but on version 3.3.2:

for (let j = 0; j < txb.tx.ins.length; j++) {
    txb.sign(j, {
        network: config.network,
        publicKey: common.toByteArray(publickey),
        sign: (hash) => {
            let s = bitcoinjs.ECSignature.parseScriptSignature(common.toByteArray(signatures[j]));
            return new bitcoinjs.ECSignature(s.signature.r, s.signature.s);
        }
    }, redeemScript, null, parseInt(options.utxos[j].value), witnessScript);
}

I confirm it's working (at least it inject the signature provided by the ledger without errors)

ning-tan commented 5 years ago

Yes, I can confirm that the @Marxpark solution worked for us. To elaborate on his explanation a bit further, the signature returned by a hardware wallet or HSM may often just be a standard DER signature. This makes sense as they have no idea about the signature hash type that was used to generate the transaction hash for signing. Bitcoin, however, requires the byte representing the sig hash type to be appended to the DER signature. The hex string "01" represents SIGHASH_ALL, the most common sig hash type.

dabura667 commented 5 years ago

any HSM that signs 32 bytes without the preimage is vulnerable.

patwhite commented 5 years ago

It would be great to add this both to documentation (specifically doing the ECPair overrides to handle signatures), but also to add a tx builder function to apply a signature. I'm happy to do the work if someone from the team can confirm the right direction for what the method on the tx builder should look like.

dcousens commented 5 years ago

@patwhite what ECPair overrides do you refer to?

patwhite commented 5 years ago

If you read the comments above, from marxpark and dakk you’ll see what I’m referring to. I don’t particularly like that method, it seems pretty hacky, but looks like the only way to apply a hardware wallet signature right now. It involves overriding the sign function on an ecpair.

junderw commented 5 years ago

@dcousens They're hacking a fake ECPair object into the TransactionBuilder.prototype.sign method arg.

builder.sign(input_index, {
  publicKey: signersPublicKey,
  getPublicKey: () => signersPublicKey,
  sign: () => bitcoin.script.signature.decode(ledgerSignatureHex + "01").signature
}, redeemScript);

@patwhite Why not just use manual Transaction manipulation.

It's difficult to make an example when all hardware wallets have different formats of what data is being returned.

This might work. idk

Using bitcoinjs-lib v4 with payments API.

const SIGHASH_ALL = 0x01

const txb = new bitcoin.TransactionBuilder()
txb.addInput(unspent.txId, unspent.vout)
txb.addOutput(address, amount)

// TransactionBuilder is good for adding inputs and outputs
// and signing standard transactions.
// non-standard operations require direct management
const tx = txb.buildIncomplete()

// Get the signature hash for the Transaction object.
// in the case of Segwit, use hashForWitnessV0
const signatureHashInput0 = tx.hashForSignature(0, prevScriptPubkey, SIGHASH_ALL)

// assuming this sig is DER formatted with low-S encoding.
// No sighash appended. (I am guessing from other comments)
const sigFromHWW = signWithHWW(signatureHashInput0)

// create new buffer 1 byte larger. copy DER sig in and write sighash byte to last byte.
const newSig = Buffer.alloc(sigFromHWW.length + 1, 0)
sigFromHWW.copy(newSig, 0)
newSig[newSig.length-1] = SIGHASH_ALL // 0x01

const scriptSig = bitcoin.payments.p2pkh({
  output: prevScriptPubkey,
  input: bitcoin.script.compile([
    newSig,
    yourPubkeyBuffer
  ])
}).input

// then set the scriptSig manually
tx.setInputScript(0, scriptSig)
dcousens commented 5 years ago

I don't understand, TransactionBuilder expects sign to return a 64-byte R/S signature (since 4.0.0), not 65-bytes.

The hash type is encoded by TransactionBuilder, not the sign funciton.

See https://github.com/bitcoinjs/bitcoinjs-lib/blob/9a8552acda9c1881c43b3b6a9b42fb6b46ebd424/src/transaction_builder.js#L695-L697

typeforce should be telling you that: https://github.com/bitcoinjs/bitcoinjs-lib/blob/9a8552acda9c1881c43b3b6a9b42fb6b46ebd424/src/script_signature.js#L40-L44

I think this is merely a residual question that has been resolved by the changes in 4.0.0.

junderw commented 5 years ago

@dcousens they are using v3 afaik

dcousens commented 5 years ago

@junderw

https://github.com/bitcoinjs/bitcoinjs-lib/blob/f4caaf42e7b58332d74c1540f88bcda7e55b82e6/src/transaction_builder.js#L711-L714

If you return a Buffer, the same thing happens in latest version of ^3.3.0.

Please see https://github.com/bitcoinjs/bitcoinjs-lib/pull/1271

dcousens commented 5 years ago

@Marxpark @dakk @ning-tan the dance you are doing with signature encoding is probably because you are not using Buffer's. I cannot emphasize enough how dangerous it is to mess around with your data types.

ning-tan commented 5 years ago

The issue with the sig hash bit notwithstanding, since this issue has now been closed, what's the official "resolution" for the asked-for feature? Namely being able to add an external signature?

Was the official resolution:

  1. No we do not intend to support this feature, or
  2. Yes, please use the "fake" ECPair solution as the accepted solution, or
  3. we are just closing this issue with no further comments
  4. any other resolution?

Thanks.

dabura667 commented 5 years ago
  1. What feature? There's no concrete feature explained. Just a hack to get around it, and a vague insinuation of what the exact signature format is.
  2. From @dcousens comment, no, messing with types is not a good idea.
  3. @junderw made a reference to manual transaction manipulation using an example that looks like a modified example from integration tests. It was hidden as outdated by @dcousens probably for the same reason as 2.
  4. No other resolution has been given.

OP seems to have fixed his problem, so closing the issue seems warranted.

A possible future pull request would be to allow passing a callback to sign (or maybe add a new signWithHardware) that takes the sighash as input and returns an RSbuffer.

dcousens commented 5 years ago

See https://github.com/bitcoinjs/bitcoinjs-lib/pull/1271