PSBT is finalized but throws error on broadcasting

MrUnfunny commented 1 year ago

I am trying to test a multisig setup with 2 of keys on bdk-cli and 1 in another app. I got a single signed PSBT from another app, and signed it with my wallet on bdk-cli. On signing is_finalized is set to true but on broadcasting, I'm getting the following error:

Electrum(Protocol(String("sendrawtransaction RPC error: {\"code\":-26,\"message\":\"non-mandatory-script-verify-flag (Non-canonical signature: S value is unnecessarily high)\"}")))

It's working fine if I sign PSBT with both my keys.

unsigned PSBT

```bash cHNidP8BAH0BAAAAAVStv9pYgvhqbNL4h5mYjt6PpSAAaJU4JoB51C7oYBwnAQAAAAD/////Avu4AAAAAAAAIgAg9IzqZoBdk0kthmDOwYTyOMmNHa+QVqZPFHusUkAc8FG8AgAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TAAAAAAABAP2CAQEAAAAAAQEQj+lgldQV80tEXjJjA3kdrOSJjWU8C0uAR80zDcYHswAAAAAA/////wIsAQAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUQRHMEQCICtQqFop9B05YOfqbtsbICP4ACnd9ED15RotEh83wHtXAiAimKlWmTLXypUWmXXtv6S+9pAWVuW5TVgDIMpynrMfOwEASDBFAiEAkGOVhJBOK8byoUWEjXz+1lpe+vHvAmBiOqB7/MtDNFUCIDsVBBU5Prf7qnt2nQ4kIdeZxKUOg0Zn6jE0jgcJ3ImwAW8hAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5irHwhA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUOrJN8IQMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXayTUocAAAAAAQErWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUQEFbyECwRZHKiqCmsR65ft3NkmqBXd6nk9hMI6jqlHHTsR+jmKsfCEDnQBEHja2+vHuJq10nzIgN9UnuF1KaL5x2fXGl7QRhQ6sk3whAxmkjc8+EIkMlPG/lQDSUXfVhIeGwf47vN3YhXZqf4VdrJNShyIGAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5iBCV8oa0iBgMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXQSsJ8+GIgYDnQBEHja2+vHuJq10nzIgN9UnuF1KaL5x2fXGl7QRhQ4Eolrq7gAiAgLBFkcqKoKaxHrl+3c2SaoFd3qeT2EwjqOqUcdOxH6OYgQlfKGtIgIDGaSNzz4QiQyU8b+VANJRd9WEh4bB/ju83diFdmp/hV0ErCfPhiICA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUOBKJa6u4AAA== ```

signed by other app

```bash cHNidP8BAH0BAAAAAVStv9pYgvhqbNL4h5mYjt6PpSAAaJU4JoB51C7oYBwnAQAAAAD/////Avu4AAAAAAAAIgAg9IzqZoBdk0kthmDOwYTyOMmNHa+QVqZPFHusUkAc8FG8AgAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TAAAAAAABAP2CAQEAAAAAAQEQj+lgldQV80tEXjJjA3kdrOSJjWU8C0uAR80zDcYHswAAAAAA/////wIsAQAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUQRHMEQCICtQqFop9B05YOfqbtsbICP4ACnd9ED15RotEh83wHtXAiAimKlWmTLXypUWmXXtv6S+9pAWVuW5TVgDIMpynrMfOwEASDBFAiEAkGOVhJBOK8byoUWEjXz+1lpe+vHvAmBiOqB7/MtDNFUCIDsVBBU5Prf7qnt2nQ4kIdeZxKUOg0Zn6jE0jgcJ3ImwAW8hAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5irHwhA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUOrJN8IQMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXayTUocAAAAAAQErWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUSICA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUORzBEAiAAn4X2wgAJrxa69X4I/IXS2DE58Ug93wIVpGB9SvgSJQIgK3lPTh2ciG/k0SZOF0JmyKoRwhkSa9WqkoiSNl3EjZ4BAQVvIQLBFkcqKoKaxHrl+3c2SaoFd3qeT2EwjqOqUcdOxH6OYqx8IQOdAEQeNrb68e4mrXSfMiA31Se4XUpovnHZ9caXtBGFDqyTfCEDGaSNzz4QiQyU8b+VANJRd9WEh4bB/ju83diFdmp/hV2sk1KHIgYCwRZHKiqCmsR65ft3NkmqBXd6nk9hMI6jqlHHTsR+jmIEJXyhrSIGAxmkjc8+EIkMlPG/lQDSUXfVhIeGwf47vN3YhXZqf4VdBKwnz4YiBgOdAEQeNrb68e4mrXSfMiA31Se4XUpovnHZ9caXtBGFDgSiWuruACICAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5iBCV8oa0iAgMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXQSsJ8+GIgIDnQBEHja2+vHuJq10nzIgN9UnuF1KaL5x2fXGl7QRhQ4Eolrq7gAA ```

finalized psbt

```bash cHNidP8BAH0BAAAAAVStv9pYgvhqbNL4h5mYjt6PpSAAaJU4JoB51C7oYBwnAQAAAAD/////Avu4AAAAAAAAIgAg9IzqZoBdk0kthmDOwYTyOMmNHa+QVqZPFHusUkAc8FG8AgAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TAAAAAAABAP2CAQEAAAAAAQEQj+lgldQV80tEXjJjA3kdrOSJjWU8C0uAR80zDcYHswAAAAAA/////wIsAQAAAAAAABYAFP+dpWfmLzDqhlT6HV+9R774474TWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUQRHMEQCICtQqFop9B05YOfqbtsbICP4ACnd9ED15RotEh83wHtXAiAimKlWmTLXypUWmXXtv6S+9pAWVuW5TVgDIMpynrMfOwEASDBFAiEAkGOVhJBOK8byoUWEjXz+1lpe+vHvAmBiOqB7/MtDNFUCIDsVBBU5Prf7qnt2nQ4kIdeZxKUOg0Zn6jE0jgcJ3ImwAW8hAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5irHwhA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUOrJN8IQMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXayTUocAAAAAAQErWb0AAAAAAAAiACD0jOpmgF2TSS2GYM7BhPI4yY0dr5BWpk8Ue6xSQBzwUSICAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5iSDBFAiEAvlRcWoZet7QWxJZWjMibWXU3F88FJEytGJjQpQoDKugCIB4l+mOyscAmXdTWaCjWuTHt6YKX3BBAyWJob+R66xzCASICA50ARB42tvrx7iatdJ8yIDfVJ7hdSmi+cdn1xpe0EYUORzBEAiAAn4X2wgAJrxa69X4I/IXS2DE58Ug93wIVpGB9SvgSJQIgK3lPTh2ciG/k0SZOF0JmyKoRwhkSa9WqkoiSNl3EjZ4BAQVvIQLBFkcqKoKaxHrl+3c2SaoFd3qeT2EwjqOqUcdOxH6OYqx8IQOdAEQeNrb68e4mrXSfMiA31Se4XUpovnHZ9caXtBGFDqyTfCEDGaSNzz4QiQyU8b+VANJRd9WEh4bB/ju83diFdmp/hV2sk1KHIgYCwRZHKiqCmsR65ft3NkmqBXd6nk9hMI6jqlHHTsR+jmIEJXyhrSIGAxmkjc8+EIkMlPG/lQDSUXfVhIeGwf47vN3YhXZqf4VdBKwnz4YiBgOdAEQeNrb68e4mrXSfMiA31Se4XUpovnHZ9caXtBGFDgSiWuruAQcAAQj9AwEEAEcwRAIgAJ+F9sIACa8WuvV+CPyF0tgxOfFIPd8CFaRgfUr4EiUCICt5T04dnIhv5NEmThdCZsiqEcIZEmvVqpKIkjZdxI2eAUgwRQIhAL5UXFqGXre0FsSWVozIm1l1NxfPBSRMrRiY0KUKAyroAiAeJfpjsrHAJl3U1mgo1rkx7emCl9wQQMliaG/keuscwgFvIQLBFkcqKoKaxHrl+3c2SaoFd3qeT2EwjqOqUcdOxH6OYqx8IQOdAEQeNrb68e4mrXSfMiA31Se4XUpovnHZ9caXtBGFDqyTfCEDGaSNzz4QiQyU8b+VANJRd9WEh4bB/ju83diFdmp/hV2sk1KHACICAsEWRyoqgprEeuX7dzZJqgV3ep5PYTCOo6pRx07Efo5iBCV8oa0iAgMZpI3PPhCJDJTxv5UA0lF31YSHhsH+O7zd2IV2an+FXQSsJ8+GIgIDnQBEHja2+vHuJq10nzIgN9UnuF1KaL5x2fXGl7QRhQ4Eolrq7gAA ```

rajarshimaitra commented 1 year ago

Thanks @MrUnfunny for reporting.. This is happening because probably the other app that you are using is creating signature of "High S" which is prohibited in Bitcoin protocol. That's what the S value is unnecessarily high error means and this is a security property enforced by Bitcoin consensus. If a Signature is created with "High S" then its possible to trivially create another valid signature with "Low S" from the signature data itself, without knowing the private key. That's why "High S" signature values are never allowed in the Bitcoin protocol.

BDK by default (or the underlying secp256k1 library) always creates low S signature value and that's why you don't see the error when you sign with both bdk keys.

To test this hypothesis, you can do the following.

Create a single sig PSBT with the other app, and try to broadcast the transaction. If the error happens, then something is surely wrong in the signing logic of the other app. If it doesn't, then the app probably has a bug that only triggers for complex scripts, but that's unlikely.
Try out with another app of your choice and see if the error still occurs..

To me this doesn't seems like a bdk issue, but I am anyway curious to know what app you are using for this, as you might wanna notify the developers there for this bug..

rajarshimaitra commented 1 year ago

Also from the psbt data I can see the witness script is something like this Script(OP_PUSHBYTES_33 02c116472a2a829ac47ae5fb773649aa05777a9e4f61308ea3aa51c74ec47e8e62 OP_CHECKSIG OP_SWAP OP_PUSHBYTES_33 039d00441e36b6faf1ee26ad749f322037d527b85d4a68be71d9f5c697b411850e OP_CHECKSIG OP_ADD OP_SWAP OP_PUSHBYTES_33 0319a48dcf3e10890c94f1bf9500d25177d5848786c1fe3bbcddd885766a7f855d OP_CHECKSIG OP_ADD OP_PUSHNUM_2 OP_EQUAL)

Which is a very roundabout way of doing multisig. Would you mind sharing the descriptor you are using for creating the script? I am curious..

MrUnfunny commented 1 year ago

Hey @rajarshimaitra The signature from the other app seems to be faulty. My original doubt was if finalize_psbt verifies the signatures but I forgot to add it here in this issue because it was already clarified on discord.

On the Witness script, yes that might be a little messy. I was just playing around with miniscript and didn't put much effort into it. On this webpage, I found miniscript for 3-of-3 that turns into a 2-of-3 after 90 days as:

 thresh(3,pk(key_1),s:pk(key_2),s:pk(key_3),sln:older(12960))

I just changed the threshold value and removed the timelock. It was working well so I didn't pay much attention to it. I also saw there is a multi operator and thresh(2,pk(key_1),s:pk(key_2),s:pk(key_3)) generates a descriptor with multi operator on compiling with bdk-cli. So, I assumed that bdk-cli might be compiling internally but I'm probably mistaken.

bitcoindevkit / bdk-cli

PSBT is finalized but throws error on broadcasting #127