Add and use support for pulling tx without PSBT wrappers

[jgriffiths]

Includes #6882 which is rebased and I reckon is ready to merge.

Supersedes #6975, @vincenzopalazzo @grubles your review/testing would be appreciated.

Fixes #6975
Fixes #6955

[cdecker]

Thanks @jgriffiths this looks great, and I'm glad to see some of the utilities we open-coded in CLN making their way back into libwally 👍

I'll wait for @grubles to confirm that this works, and then merge it. In the meantime I think your script change in the last commit could give us a speed boost and so I'm benchmarking a blockchain sync with and without this change to quantify that.

[vincenzopalazzo]

ACK fe96d3a

This looks good to me and my node is syncing with the liquid chain. It is not difficult to get a pegin transaction while syncing, so currently there is no crash.

Thanks for push this work on the final line

[grubles]

ACK fe96d3a

Builds and syncs with --network=liquid successfully.

[jgriffiths]

I'll wait for @grubles to confirm that this works, and then merge it.

Awesome, thanks @cdecker

I'm glad to see some of the utilities we open-coded in CLN making their way back into libwally 👍

Worth noting that nothing has moved from cln to wally. The removed code was either added to wally for other purposes or was already in wally (but not exposed through its api,) and was exposed for 1.0.0.

I say this is worth mentioning because there is code in cln that should move to wally, I'm thinking particularly of the taproot code outside of the keypath spending support gsanders and I worked to merge last year. Also the previous iteration of the wally update PR removed a large amount of code relating to witness serialization but I had to pull it out as no-one helped me figure out how it was upsetting tal. Unfortunately I don't have time to work on this exclusively, but I would be happy to work with someone to get code into wally where it can help others who need e.g. taproot support.

In the meantime I think your script change in the last commit could give us a speed boost and so I'm benchmarking a blockchain sync with and without this change to quantify that.

That change and your alloc removal will help, but note that your changes will be lost in the noise of the rest of that PR. In order of effect, the changes in this PR that will improve speed should be:

1. Building wally and libsecp in release mode, which enables asm hash impls amongst other optimizations
2. Not building PSBTs for every tx in each block and elsewhere (multiple dozens of allocations per tx)
3. Not re-serializing the tx to redundantly re-compute its txid (allocates, serializes and multi-hashes every tx redundantly)

If block processing in particular is a concern, one easy win I noticed but didn't make is to remove the top level tal_count calls from the block tx iteration for loops by computing the count outside the loop first, particularly in topology.c. I don't believe the compiler will optimize out these calls, and they perform redundant verification each time. The double nested loops for fees in particular are probably dominated by wasted tal overhead.

edit: I've made the above change to avoid the tal overhead, and also added early-exit for pegin, fee and cant-possibly-be-p2wsh outputs.

[cdecker]

Worth noting that nothing has moved from cln to wally. The removed code was either added to wally for other purposes or was already in wally (but not exposed through its api,) and was exposed for 1.0.0.

Oh yeah, I meant more as in the use-cases we needed in CLN seem to be shared, rather than the code. Thanks for the detailed answer too, it'd certainly be useful to consolidate on wally for anything that accesses or manipulates TXs.

I also finally managed to get benchmarks working, which was strange since the bitcoind backend was slower than CLN could process them :-)

Awesome find on that redundant tal_count and fee computation issue 👍

[jgriffiths]

bitcoind backend was slower than CLN could process them :-)

Probably no point optimising further for now then :)

I suspect the rpc overhead is what make this true. The absolute fastest way to do initial sync if the core block files are available would be to use an extremely cut-down block parser based on e.g. https://github.com/rustyrussell/bitcoin-iterate. With an interface that just returns each tx in the block as a pointer + length, the block txs could be constructed directly from the memmapped bytes without rpc overhead and without bin<>hex conversion. At the expense of having more code to maintain you could leave some of the parsing code intact and pass the filter criteria into the call so that filtered txs are skipped completely without even creating them.