transaction: avoid quadratic scaling when growing arrays

[whitslack]

wally_tx_witness_stack_set(), wally_tx_add_input_at(), and wally_tx_add_output_at() all exhibited quadratic scaling behavior when iteratively appending elements to their respective arrays because they were calling array_realloc() to expand their arrays by one element at a time.

Change them to use array_grow(), and change the contract of array_grow() so that it accepts a minimum element count and ensures that the array is at least large enough to accommodate that many elements, enlarging it to the next greater power of two if necessary.

See: ElementsProject/lightning#8924

[jgriffiths]

For wally_tx_witness_stack_init_alloc, MAX_WITNESS_ITEMS_ALLOC is enforced for standardness purposes and to limit the potential of an adversarial tx from causing huge allocations. Generally its expected that callers will have sized their witness appropriately when creating their own txs, but in the case where we are parsing an already created tx, we should allocate the correct number of elements up front while retaining the safety of not allowing huge allocs.

I think the best fix is to provide an internal impl for wally_tx_witness_stack_init_alloc which takes the maximum allocation length to clamp to, with the exposed function maintaining its existing limit. In tx_from_bytes (and in pull_witness for consistency), we'd call the internal impl, passing the number of remaining bytes in the tx as the maximum, while wally_tx_witness_stack_init_alloc can continue to use MAX_WITNESS_ITEMS_ALLOC.

That way we have a single alloc for all valid txs, while limiting the potential allocs for a malicious tx that will fail later parsing.

[whitslack]

If you know the size of the array that you'll need (or a reasonable upper bound on it) in advance, then certainly you should preallocate it rather than relying on any kind of dynamic growth, whether linear or geometric. I suggested the least-invasive change that addresses the performance problem.

---

Snide remarks from the peanut gallery: If you people (I'm talking to the CLN folks now too) would just use C++ rather than repeatedly and crudely reinventing the wheel in C, we wouldn't constantly be running into issues like this. The STL containers are a one-stop shop — no need to roll your own dynamic allocation code every time you need a resizable container. I've never understood why anyone still codes in C; it's like the mentally challenged cousin of C++.

[jgriffiths]

rather than repeatedly and crudely reinventing the wheel in C

wally was written in C because it was initially developed (over 10 years ago) to replace python, javascript/cordova and java wallet crypto implementations with a single shared core of primitives and higher level wrapping of secp functionality that the GreenAddress wallets and server backend could use. At that time there was no comparable library that we could have used instead. The decision to write it in C was ultimately made to most easily support the platforms we needed to at the time and planned for in the future. FWIW I wanted to write it in C++ but the integration story for C++ wasn't good at that time.

I've never understood why anyone still codes in C

People have their reasons and projects have their requirements. The calculus if decided today would absolutely be different of course. But in commercial software development we often end up living with decisions from the past longer than personal projects have to.