Update libwally to 0.8.8, Support PSBTv2 and PSETv2 #5898

instagibbs · 2023-01-13T17:26:40Z

This contains support for taproot, PSBTv2, removesPSETv0 (elements support break)

For internal PSBT construction(e.g. fundpsbt), RPC still returns v0(unless PSET). If a v0/v2 is passed in RPC, the same version should be returned.

This PR also updates the elementsd binary being run to latest binary release.

instagibbs · 2023-01-18T20:45:18Z

Will rebase onto #5910 to break elements tests once merged

instagibbs · 2023-01-19T19:34:47Z

rebasing onto #5910 to see how much more breaks vs #5910

instagibbs · 2023-01-31T22:05:12Z

All tests are passing locally for me now. Had to disable a bunch of Elements tests since PSETv0 has been killed off in libwally.

A python library for v2 would be great as a replacement for these bitcoind rpc checks.

edit: actually looks like I'm failing the same number of tests as master 🥇

instagibbs · 2023-02-01T22:00:33Z

Ran the tests locally against bitcoin/bitcoin#21283 for bitcoind, and everything except the test relying on joinpsbt succeeded, which is expected since it's not supporting v2 yet. This demonstrates that the input is being handled correctly for both versions. Might be worth mulling over if this run can be made intermittently to catch regressions.

instagibbs · 2023-02-06T20:18:54Z

Sounds like people want canned tests where possible instead of importing wallycore. I'll take a look at seeing what tests I can rescue or increase coverage of PSBTv2 as rpc input.

instagibbs · 2023-02-28T19:34:28Z

I probably should expose an rpc for converting PSBT versions to allow for easier debugging for end users when they get V2 in logs?

doc/lighting-setpsbtversion.7.md

doc/lightning-setpsbtversion.7.md

rustyrussell

This is glorious!

I went through with a fine-tooth comb, and added as many nits as I could pick.

wallet/test/run-wallet.c

external/libwally-core

rustyrussell · 2023-03-19T23:19:47Z

bitcoin/psbt.c

@@ -264,7 +252,7 @@ void psbt_input_add_pubkey(struct wally_psbt *psbt, size_t in,
 	pubkey_to_der(pk_der, pubkey);

 	tal_wally_start();
-	wally_err = wally_psbt_input_add_keypath_item(&psbt->inputs[in],
+	wally_err = wally_psbt_input_keypath_add(&psbt->inputs[in],
 						      pk_der, sizeof(pk_der),


This commit is so big, I suggest an indent fixup commit at the end, if anything.

bitcoin/psbt.c

tests/test_misc.py

wallet/walletrpc.c

tests/test_wallet.py

Which gets libwally upset post-update to 0.8.8

rustyrussell · 2023-03-21T03:13:14Z

Deserves a changelog. I think two?

Changelog-Changed: JSON-RPC: elements network PSET now only supports PSETv2.
Changelog-Added: JSON-RPC: PSBTv2 supported for fundchannel_complete, openchannel_update, reserveinputs, sendpsbt, signpsbt, withdraw and unreserveinputs parameter psbt, openchannel_init and openchannel_bump parameter initialpsbt, openchannel_signed parameter signed_psbt and utxopsbt parameter utxopsbt

vincenzopalazzo

There are a couple of other extra { } inside some if statements, but overall I guess LGTM , these are just style comments 😸

vincenzopalazzo · 2023-03-21T07:31:55Z

bitcoin/tx.c

+static struct wally_tx_input *wally_tx_input_from_outpoint_sequence(const struct bitcoin_outpoint *outpoint,
+			u32 sequence)
+{
+	struct wally_tx_input *tx_in;


Suggested change

struct wally_tx_input *tx_in;

struct wally_tx_input *tx_in;

here there is a space nit

vincenzopalazzo · 2023-03-21T07:34:12Z

bitcoin/tx.c

+            abort();
+    } else {
+        if (wally_tx_input_init_alloc(outpoint->txid.shad.sha.u.u8,
+                          sizeof(outpoint->txid.shad.sha.u.u8),
+                          outpoint->n,
+                          sequence, NULL, 0, NULL,
+                          &tx_in) != WALLY_OK)
+            abort();
+    }


I am not a big expert on this part of the code but it is possible abort with some more useful message? I think this case will never be true in the same case, but do you think that it will be useful to print some more information?

vincenzopalazzo · 2023-03-21T07:35:20Z

bitcoin/tx.c

+		if (wally_psbt_extract(psbt, WALLY_PSBT_EXTRACT_NON_FINAL, &tx->wtx) != WALLY_OK) {
 			tx->wtx = NULL;
+		}


we can remove the { .. } from here?

vincenzopalazzo · 2023-03-21T07:36:14Z

common/psbt_internal.c

@@ -35,18 +35,16 @@ void psbt_finalize_input(const tal_t *ctx,
 	 * on these just .. ignores it!? Murder. Anyway, here we do a final


we should open an issue for this? so at least we do not forget

common/psbt_internal.c

common/psbt_open.c

lightningd/dual_open_control.c

instagibbs · 2023-03-21T13:39:41Z

~~@rustyrussell what's the process for noting/aggregating changelog?~~ got it

instagibbs · 2023-03-21T17:55:31Z

Pushed a couple more nit fixes and changelogs in commit messages

Libwally update breaks compatibility, so we do this in one large step. Changelog-Changed: JSON-RPC: elements network PSET now only supports PSETv2. Changelog-Added: JSON-RPC: PSBTv2 supported for fundchannel_complete, openchannel_update, reserveinputs, sendpsbt, signpsbt, withdraw and unreserveinputs parameter psbt, openchannel_init and openchannel_bump parameter initialpsbt, openchannel_signed parameter signed_psbt and utxopsbt parameter utxopsbt

PSBTv2 support is quite low in the ecosystem, so having a call to convert log messages and the like should be useful since they'll often be in v2. Changelog-Added: Added setpsbtversion RPC to aid debugging and compatibility

rustyrussell

Ack 63fbcec

instagibbs mentioned this pull request Jan 13, 2023

DRAFT: Update libwally to begin PSBTv2 support #5451

Closed

instagibbs force-pushed the libwally_update_2 branch from 2d51823 to e240b7d Compare January 13, 2023 21:24

instagibbs requested a review from cdecker as a code owner January 17, 2023 22:25

instagibbs force-pushed the libwally_update_2 branch 2 times, most recently from ba0efb0 to 846b113 Compare January 18, 2023 16:32

instagibbs force-pushed the libwally_update_2 branch from a1e2e48 to d1cc4e5 Compare January 19, 2023 19:34

instagibbs marked this pull request as draft January 23, 2023 22:26

instagibbs force-pushed the libwally_update_2 branch 4 times, most recently from 75f1bdf to 7979c9d Compare January 31, 2023 15:05

instagibbs changed the title ~~WIP Update libwally-core to 0.8.8~~ WIP Support PSBTv2 Jan 31, 2023

instagibbs force-pushed the libwally_update_2 branch 5 times, most recently from 7fd3347 to a5366b2 Compare February 1, 2023 21:46

instagibbs force-pushed the libwally_update_2 branch 3 times, most recently from 5929954 to b008fef Compare February 6, 2023 18:51

instagibbs force-pushed the libwally_update_2 branch 5 times, most recently from 77928e3 to 56aa3e7 Compare February 8, 2023 00:00

instagibbs force-pushed the libwally_update_2 branch 4 times, most recently from d6c0385 to a66db3a Compare March 2, 2023 21:39

instagibbs commented Mar 2, 2023

View reviewed changes

doc/lighting-setpsbtversion.7.md Outdated Show resolved Hide resolved

instagibbs force-pushed the libwally_update_2 branch 5 times, most recently from 7ba0b5f to 002092d Compare March 3, 2023 03:22

jgriffiths reviewed Mar 6, 2023

View reviewed changes

doc/lightning-setpsbtversion.7.md Outdated Show resolved Hide resolved

instagibbs force-pushed the libwally_update_2 branch from 002092d to cf7b5e3 Compare March 6, 2023 15:26

rustyrussell approved these changes Mar 19, 2023

View reviewed changes

test: add PSBT field that doesn't collide with PSBTv2 fields

ed51248

Which gets libwally upset post-update to 0.8.8

instagibbs force-pushed the libwally_update_2 branch from cf7b5e3 to 80962e6 Compare March 20, 2023 15:12

ShahanaFarooqui added this to the v23.05 milestone Mar 20, 2023

vincenzopalazzo reviewed Mar 21, 2023

View reviewed changes

instagibbs force-pushed the libwally_update_2 branch from 80962e6 to 3429a1e Compare March 21, 2023 17:55

instagibbs added 5 commits March 21, 2023 16:19

Re-enable PSBT tests for Liquid except test_sign_and_send_psbt

c1e4c9a

Make startup_regtest.sh more robust to bitcoind wallet state

0dbd3b7

test_closing_different_fees: b vs balance in loop

63fbcec

instagibbs force-pushed the libwally_update_2 branch from 3429a1e to 63fbcec Compare March 21, 2023 20:20

rustyrussell approved these changes Mar 23, 2023

View reviewed changes

rustyrussell merged commit e7bf529 into ElementsProject:master Mar 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update libwally to 0.8.8, Support PSBTv2 and PSETv2 #5898

Update libwally to 0.8.8, Support PSBTv2 and PSETv2 #5898

instagibbs commented Jan 13, 2023 •

edited

Loading

instagibbs commented Jan 18, 2023

instagibbs commented Jan 19, 2023

instagibbs commented Jan 31, 2023 •

edited

Loading

instagibbs commented Feb 1, 2023

instagibbs commented Feb 6, 2023

instagibbs commented Feb 28, 2023

rustyrussell left a comment

rustyrussell Mar 19, 2023

rustyrussell commented Mar 21, 2023

vincenzopalazzo left a comment

vincenzopalazzo Mar 21, 2023

vincenzopalazzo Mar 21, 2023

vincenzopalazzo Mar 21, 2023

vincenzopalazzo Mar 21, 2023

instagibbs commented Mar 21, 2023 •

edited

Loading

instagibbs commented Mar 21, 2023

rustyrussell left a comment

		@@ -35,18 +35,16 @@ void psbt_finalize_input(const tal_t *ctx,
		* on these just .. ignores it!? Murder. Anyway, here we do a final

Update libwally to 0.8.8, Support PSBTv2 and PSETv2 #5898

Update libwally to 0.8.8, Support PSBTv2 and PSETv2 #5898

Conversation

instagibbs commented Jan 13, 2023 • edited Loading

instagibbs commented Jan 18, 2023

instagibbs commented Jan 19, 2023

instagibbs commented Jan 31, 2023 • edited Loading

instagibbs commented Feb 1, 2023

instagibbs commented Feb 6, 2023

instagibbs commented Feb 28, 2023

rustyrussell left a comment

Choose a reason for hiding this comment

rustyrussell Mar 19, 2023

Choose a reason for hiding this comment

rustyrussell commented Mar 21, 2023

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo Mar 21, 2023

Choose a reason for hiding this comment

vincenzopalazzo Mar 21, 2023

Choose a reason for hiding this comment

vincenzopalazzo Mar 21, 2023

Choose a reason for hiding this comment

vincenzopalazzo Mar 21, 2023

Choose a reason for hiding this comment

instagibbs commented Mar 21, 2023 • edited Loading

instagibbs commented Mar 21, 2023

rustyrussell left a comment

Choose a reason for hiding this comment

instagibbs commented Jan 13, 2023 •

edited

Loading

instagibbs commented Jan 31, 2023 •

edited

Loading

instagibbs commented Mar 21, 2023 •

edited

Loading