rust-bitcoin Non-canonical DER signature

Question

I am trying to create a raw transaction spending a P2PKH output using rust-bitcoin, but whenever I am pushing the transaction to the testnet I receive the following error mandatory-script-verify-flag-failed (Non-canonical DER signature) My signing code looks like this:

/// Sign a bitcoin transaction spending P2PKH outputs
/// 
/// # Arguments
/// 
/// * `tx` the transaction to be signed
/// * `script_pubkeys` list of script pubkeys of the inputs (has to be indexed in the same order as the inputs in the transaction)
/// * `skeys` list of secret keys with which to sign the inputs (has to be indexed in the same order as the inputs in the transaction)
/// * `pkeys` list of public keys for the inputs which are spent (has to be indexed in the same order as the inputs in the transaction)
/// * `curve` reference to the elliptic curve object used for signing
pub fn sign_transaction(tx : Transaction, script_pubkeys : Vec<Script>, skeys : Vec<PrivateKey>, pkeys: Vec<PublicKey>, curve : &Secp256k1<All>) -> Transaction {
    let mut signed_inp : Vec<TxIn> = Vec::new();

    for (i, unsigned_inp) in tx.input.iter().enumerate() {
        let script_pubkey = script_pubkeys.get(i).unwrap();
        let signing_key = skeys.get(i).unwrap().key;
        let pub_key = pkeys.get(i).unwrap();
        let sighash = tx.signature_hash(i, &script_pubkey, SIGHASH_ALL);
        let msg = Message::from_slice(&sighash.as_ref()).unwrap();
        let sig = curve.sign(&msg, &signing_key);
        // Standard P2PKH redeem script:
        // <sig> <pubkey>
        let redeem_script = Builder::new()
            .push_slice(&sig.serialize_der())
            .push_int(1)
            .push_key(&pub_key)
            .into_script();
        signed_inp.push(TxIn {
            previous_output : unsigned_inp.previous_output,
            script_sig: redeem_script,
            sequence : unsigned_inp.sequence,
            witness : unsigned_inp.witness.clone()
        });
        
    }
    
    Transaction {
        version : tx.version,
        lock_time : tx.lock_time,
        input : signed_inp,
        output: tx.output
    }
}

Here is an example transaction 01000000010b41a0a10f36c39b057053281d1d4d9f67410c40a093c351024f57af4e133c21010000006a46304402202d99eb85a14f483f4679b93eb1cf0f67b64d788dc4cd5ad782e75540508922ce0220752a9237e320497ba289a91edd7135dd6c53f9332dc33c71a3020bf0555607fc512103202430a99091407e6c724c5c88504e69ef6917f042a65cb990537922f823d9dfffffffff02e80300000000000017a9141831af16119be20b532668d5995bd05ac955153b872a091e00000000001976a9146b538e889cf0df7b69112b50db8faadb4e457b9288ac00000000

and in decoded form

{
   "result":{
      "txid":"b36457604e0ccd491e97c30c06a011fd906bb83d6fe81c676efb9fc60f17c5a5",
      "hash":"b36457604e0ccd491e97c30c06a011fd906bb83d6fe81c676efb9fc60f17c5a5",
      "version":1,
      "size":223,
      "vsize":223,
      "weight":892,
      "locktime":0,
      "vin":[
         {
            "txid":"213c134eaf574f0251c393a0400c41679f4d1d1d285370059bc3360fa1a0410b",
            "vout":1,
            "scriptSig":{
               "asm":"304402202d99eb85a14f483f4679b93eb1cf0f67b64d788dc4cd5ad782e75540508922ce0220752a9237e320497ba289a91edd7135dd6c53f9332dc33c71a3020bf0555607fc 1 03202430a99091407e6c724c5c88504e69ef6917f042a65cb990537922f823d9df",
               "hex":"46304402202d99eb85a14f483f4679b93eb1cf0f67b64d788dc4cd5ad782e75540508922ce0220752a9237e320497ba289a91edd7135dd6c53f9332dc33c71a3020bf0555607fc512103202430a99091407e6c724c5c88504e69ef6917f042a65cb990537922f823d9df"
            },
            "sequence":4294967295
         }
      ],
      "vout":[
         {
            "value":0.00001000,
            "n":0,
            "scriptPubKey":{
               "asm":"OP_HASH160 1831af16119be20b532668d5995bd05ac955153b OP_EQUAL",
               "hex":"a9141831af16119be20b532668d5995bd05ac955153b87",
               "reqSigs":1,
               "type":"scripthash",
               "addresses":[
                  "2MuT9izFXKft5umHqr1gviyhus6JGLCxGd6"
               ]
            }
         },
         {
            "value":0.01968426,
            "n":1,
            "scriptPubKey":{
               "asm":"OP_DUP OP_HASH160 6b538e889cf0df7b69112b50db8faadb4e457b92 OP_EQUALVERIFY OP_CHECKSIG",
               "hex":"76a9146b538e889cf0df7b69112b50db8faadb4e457b9288ac",
               "reqSigs":1,
               "type":"pubkeyhash",
               "addresses":[
                  "mqJShCuqM7FK5zPa7Z1PNJySMHTVPrK9bb"
               ]
            }
         }
      ]
   },
   "error":null,
   "id":"curltest"
}

Any help on what could be wrong is much appreciated

Answer 1

I believe the intention of your push_int(1) is to add SIGHASH_ALL to the transaction.

However, one of Script's many needless inflexibilities is that the sighash flag actually needs to be part of the signature itself, rather than as a separate stack element in the script.

This means that you need to take the Vec<u8> returned from Signature::serialize_der, mutate this with sig.push(1), and then add it onto the script stack. By manually editing your transaction to change the 51 (OP_1) to 01 (the 1 byte), and increasing the length in the signature opcode to consume this, I was able to make your transaction pass validation on testnet. (I didn't submit it though, to save you the annoyance of dealing with "input already spent" errors!)

I'm not thrilled with this API -- I suppose at the very least that Signature should have a a method serialize_der_with_sighashflag or something to eliminate the need for the caller to use a temporary variable and mut, but it's not super common to be manually constructing scripts like this and I guess it hasn't come up.

You might consider looking at rust-miniscript which does a lot of this stuff for you.