SigningOptions not being applied on transactionbuilder

[Jossec101]

Hey, amazing lib, couldn't be easier to work with it!

I found a potential bug, we are playing with SIGHASH_NONE PSBTs but it looks like the txBuilder is not applying it in the code below:

            var channelFundingPSBT = txBuilder
                .AddCoin(multisigUTXOs.First().Value.ToScriptCoin(multisigScriptPubKey))
                .SetChange(multisigScriptPubKey)
                .SetSigningOptions(SigHash.None)
                .SendFees(feesCoins)
                .BuildPSBT(false);

If you inspect the object, you will find that: channelFundingPSBT.SigningOptions.SigHash = SigHash.All instead of SigHash.None.

Workaround:

Set the settings by hand

            channelFundingPSBT.Settings.SigningOptions.SigHash = SigHash.None;

Hope it helps anyone with this issue until the fix is done.

BR,

[Jossec101]

The workaround did not really work, the Sighash is not serialized until you explicility set it like:

                 var builder = txBuilder;

                builder.AddCoins(multisigCoins);

                builder.SetSigningOptions(SigHash.None)
                    .SendAllRemainingToChange()
                    .SetChange(changeAddress.Address)
                    .SendEstimatedFees(feeRateResult.FeeRate);

                result.Item1 = builder.BuildPSBT(false);

                result.Item1.Settings.SigningOptions = new SigningOptions(SigHash.None);
                foreach (var input in result.Item1.Inputs)
                {
                    input.SighashType = SigHash.None;
                }

[NicolasDorier]

Hey, checking now

[NicolasDorier]

@Jossec101 so I checked, the signature have SigHash.None applied to it. But indeed, PSBT doesn't.

input.SighashType = SigHash.None; shouldn't be set imho, but yeah the SigningOptions should be. Fixing it

[Jossec101]

Yes, perhaps when a SighHash different than SigHash.All a console warning message could be printed (the message could be disabled by a env-var or any global config) so users are aware of miners attack, we need to use it for a special transaction, then we apply SigHash.All to the whole transaction. We will release a new open source software that the lightning network community will like (hope so!) and NBitcoin is core :).

Thanks for checking it

[NicolasDorier]

Awesome! :) I don't know if that help you but https://github.com/dgarage/NBXplorer/ provide a block explorer which work on pruned node. It tracks UTXOs and is quite flexible. Built on NBitcoin, so you may inspire from the code as well.

[NicolasDorier]

@Jossec101 I am wondering if it makes sense to propagate the SetSigningOptions, as you don't sign with the transaction builder at all. I opened a PR, I am not sure if it will break something.

[Jossec101]

@Jossec101 I am wondering if it makes sense to propagate the SetSigningOptions, as you don't sign with the transaction builder at all. I opened a PR, I am not sure if it will break something.

I've checked the PR and the propagation of the SetSigningOptions would be required for two use cases even if you are not signing on the transaction builder.

• If you want to sign a PSBT with a hardware wallet (.e.g. Ledger) you need to detail the SigHash type if you are not using SIGHASH_ALL.
• In terms of UX, if you detail the SigHash of the PSBT and you use a software wallet (I've tested in Sparrow), the SigHash type is automatically selected.

Example PSBT with input.SighashType set manually (check "sighash": "NONE")

{
  "tx": {
    "txid": "6ae342f73d9b023ef624306e9d3d72315290b8bb542f1da6a0aeda4185787028",
    "hash": "6ae342f73d9b023ef624306e9d3d72315290b8bb542f1da6a0aeda4185787028",
    "version": 1,
    "size": 94,
    "vsize": 94,
    "weight": 376,
    "locktime": 0,
    "vin": [
      {
        "txid": "422accecfd169e31a765eccd97bfc5d1343434c4abd60b39491db3915fa1dc36",
        "vout": 1,
        "scriptSig": {
          "asm": "",
          "hex": ""
        },
        "sequence": 4294967295
      }
    ],
    "vout": [
      {
        "value": 19.99999364,
        "n": 0,
        "scriptPubKey": {
          "asm": "0 b8d2c8369915fff20815dd6d8b68a0d79fba98f3a1356ca45749b0b2778ef457",
          "desc": "addr(bcrt1qhrfvsd5ezhllyzq4m4kck69q670m4x8n5y6kefzhfxctyauw73ts3d5exe)#rgvv3ntn",
          "hex": "0020b8d2c8369915fff20815dd6d8b68a0d79fba98f3a1356ca45749b0b2778ef457",
          "address": "bcrt1qhrfvsd5ezhllyzq4m4kck69q670m4x8n5y6kefzhfxctyauw73ts3d5exe",
          "type": "witness_v0_scripthash"
        }
      }
    ]
  },
  "global_xpubs": [
    {
      "xpub": "tpubDCNTr5eMFBvYQJKJNySFcXM3HdxjseLSpTA5crAPAbXYjBb5zgtwKrHTTdRu11vUCZVYeHcV6H2oj2reuGma9Hu3t1LSPNgL5b8F6W6hsQN",
      "master_fingerprint": "1fcce4de",
      "path": "m/48'/1'/1'"
    },
    {
      "xpub": "tpubDCfM7v7fKZ31gTGGggNMycfCr5cDGinyijveRZ44RYSgAgEARwhaBd6PPpWst8kKbhEVoqNasgjHFWZKrEQoJ9pzPVEmNZDNe92hShzEMDy",
      "master_fingerprint": "60f3a0b3",
      "path": "m/48'/1'/1'"
    },
    {
      "xpub": "tpubDCwziS3VhtLnXSR7oL9Xkft5LVbrsfEQ9h7YkCNSa1cYSi1KNuMEnsb9NeouNjpq91KSK3R87jFx8oFGvSM5g9Vax1VrvawWzD9xnGjgndB",
      "master_fingerprint": "ed0210c8",
      "path": "m/48'/1'/1'"
    }
  ],
  "psbt_version": 0,
  "proprietary": [
  ],
  "unknown": {
  },
  "inputs": [
    {
      "witness_utxo": {
        "amount": 20.00000000,
        "scriptPubKey": {
          "asm": "0 f6b5c59c805ffa7846462fbeee79b4fe7d2fbe5ec1fb2a0a633e62acc3939ef5",
          "desc": "addr(bcrt1q766ut8yqtla8s3jx97lwu7d5le7jl0j7c8aj5znr8e32esunnm6sq9dks7)#0f3fvvh3",
          "hex": "0020f6b5c59c805ffa7846462fbeee79b4fe7d2fbe5ec1fb2a0a633e62acc3939ef5",
          "address": "bcrt1q766ut8yqtla8s3jx97lwu7d5le7jl0j7c8aj5znr8e32esunnm6sq9dks7",
          "type": "witness_v0_scripthash"
        }
      },
      "sighash": "NONE",
      "witness_script": {
        "asm": "2 02ac1334b9cc21c05131347f0ab7146fcbbe51dc42a2dbfdc68b3a6da4a5296acc 02b14290f6b13649cd15b55b8646deddef2adfc2e990edd10e242512bf107d421a 0306881ff91b1f17e8b2935cbcdf5777cb8cf878c7a176edd3bbe3f2a15b9a3cff 3 OP_CHECKMULTISIG",
        "hex": "522102ac1334b9cc21c05131347f0ab7146fcbbe51dc42a2dbfdc68b3a6da4a5296acc2102b14290f6b13649cd15b55b8646deddef2adfc2e990edd10e242512bf107d421a210306881ff91b1f17e8b2935cbcdf5777cb8cf878c7a176edd3bbe3f2a15b9a3cff53ae",
        "type": "multisig"
      },
      "bip32_derivs": [
        {
          "pubkey": "02ac1334b9cc21c05131347f0ab7146fcbbe51dc42a2dbfdc68b3a6da4a5296acc",
          "master_fingerprint": "60f3a0b3",
          "path": "m/48'/1'/1'/0/5"
        },
        {
          "pubkey": "02b14290f6b13649cd15b55b8646deddef2adfc2e990edd10e242512bf107d421a",
          "master_fingerprint": "ed0210c8",
          "path": "m/48'/1'/1'/0/5"
        },
        {
          "pubkey": "0306881ff91b1f17e8b2935cbcdf5777cb8cf878c7a176edd3bbe3f2a15b9a3cff",
          "master_fingerprint": "1fcce4de",
          "path": "m/48'/1'/1'/0/5"
        }
      ]
    }
  ],
  "outputs": [
    {
    }
  ],
  "fee": 0.00000636
}

Best,

[NicolasDorier]

@Jossec101 I think in those case, the correct way is to manually set SigHash to PSBT. This is an optional field, I can't decide from SigningOptions alone if I should leave it empty or not.

[NicolasDorier]

I am closing this as any bug I saw while looking this issue has been fixed, and I think TransactionBuilder.SetSigningOptions shouldn't try to change the PSBT's SigHash, as this is an optional field. Once can just set them directly on the PSBTInput.

Feel free to reopen if you think there is still issues.