fix: bitcoind version breaks some recent psbt tests

[m-schmoock]

Issue and Steps to Reproduce

Running the recently added PSBT tests on a pre 0.20 bitcoind seems to fail. Whereas they work with recent versions of bitcoind (At least they work with 0.20.1 but not 0.19.1)

Affected tests:

tests/test_wallet.py::test_fundpsbt
tests/test_wallet.py::test_utxopsbt
tests/test_wallet.py::test_sign_and_send_psbt

Message:

bitcoin.rpc.JSONRPCError: {'code': -22, 'message': 'TX decode failed PSBT is not sane.: iostream error'}

[darosior]

Must be due to some fixes in https://bitcoincore.org/en/releases/0.20.0/ or https://bitcoincore.org/en/releases/0.20.1/ to RPC calls used in these tests. AFAICT this does not affect our minimum supported bitcoind version.

[niftynei]

This is working as intended. See bitcoin/bitcoin#19215

[niftynei]

tl;dr: we add both witness-utxo and non-witness-utxo data to PSBTs, which any bitcoind prior to 0.20.1* fails as invalid due to the strictness of the parser contained therein. As mentioned in the linked PR, this makes us compatible with hardware wallets which enforce providing the entire tx for verification.

* might be v0.20.0, not sure when that PR got merged in bitcoind

[niftynei]

fwiw this is currently a problem for the elementsd integration which is still on v0.18.0. I've notified the maintainers and it should be fixed/updated when they update elementsd to support the latest.

[m-schmoock]

@niftynei we could programatically disable the tests on older bitcoind versions as well...

[niftynei]

No, older versions of bitcoind are incompatible. if you’re running an older version of bitcoind, you need to upgrade to use newer versions of c-lightning. Skipping the test merely hides this fact, and makes it likely you’ll have the same problem elsewhere later.

…

On Thu, Oct 22, 2020 at 03:24 Michael Schmoock ***@***.***> wrote: @niftynei <https://github.com/niftynei> we could disable the test on older bitcoind versions as well... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4144 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIMAKMCK7QGFEUF4HLBWLDSL7T3TANCNFSM4SZZRBTQ> .

[m-schmoock]

Okay, then we should update the Readme ;) didn't know that.

https://github.com/ElementsProject/lightning/blob/master/README.md#getting-started

c-lightning only works on Linux and Mac OS, and requires a locally (or remotely) running bitcoind (version 0.16 or above) that is fully caught up with the network you're running on, and relays transactions (ie with blocksonly=0). Pruning (prune=n option in bitcoin.conf) is partially supported, see here for more details.

[m-schmoock]

@niftynei also, we should stop the daemon from running on older versions if thats true.

[darosior]

I think niftynei meant for development. Users can still run fine with 0.16 mini, we don't make PSBT calls to bitcoind in this case :) ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ Le jeudi, octobre 22, 2020 2:42 PM, Michael Schmoock <[email protected]> a écrit :

…

***@***.***(https://github.com/niftynei) also, we should stop the daemon from running on older versions if thats true. — You are receiving this because you commented. Reply to this email directly, [view it on GitHub](#4144 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AFLK3F4C4TNZ2MEAP5ECI6LSMASDZANCNFSM4SZZRBTQ).

[m-schmoock]

Isn't that PSBT stuff for dual funding and such? Thats currently experimental.. so yes, maybe its currently only developers. but that will soon change then I guess..

[darosior]

Isn't that PSBT stuff for dual funding and such?

iirc it's not required by the specs but eases management

but that will soon change then I guess..

I don't expect us to use more calls to bitcoin-cli (for something else than testing), we finalize the PSBT before broadcasting anyways

[m-schmoock]

Lets give @niftynei the final call to close this issue then :D