transaction: avoid quadratic scaling when growing arrays

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
Signed-off-by: Matt Whitlock <c-lightning@mattwhitlock.name>

Author: whitslack

src/internal.c

@@ -619,8 +619,30 @@ int replace_bytes(const unsigned char *bytes, size_t bytes_len,
 }
 
 
+static size_t ceil2(size_t s)
+{
+    --s;
+    s |= s >> 1;
+    s |= s >> 2;
+    s |= s >> 4;
+#ifndef SIZE_MAX
+# error SIZE_MAX not defined
+#endif
+#if SIZE_MAX > UINT8_MAX
+    s |= s >> 8;
+#endif
+#if SIZE_MAX > UINT16_MAX
+    s |= s >> 16;
+#endif
+#if SIZE_MAX > UINT32_MAX
+    s |= s >> 32;
+#endif
+    ++s;
+    return s + !s;
+}
 
-void *array_realloc(const void *src, size_t old_n, size_t new_n, size_t size)
+/* Don't use this to build up an array iteratively! Use array_grow. */
+static void *array_realloc(const void *src, size_t old_n, size_t new_n, size_t size)
 {
     unsigned char *p = wally_malloc(new_n * size);
     if (!p)
@@ -631,17 +653,17 @@ void *array_realloc(const void *src, size_t old_n, size_t new_n, size_t size)
     return p;
 }
 
-int array_grow(void **src, size_t num_items, size_t *allocation_len,
+int array_grow(void **src, size_t num_items_needed, size_t *allocation_len,
                size_t item_size)
 {
-    if (num_items == *allocation_len) {
-        /* Array is full, allocate more space */
-        const size_t n = (*allocation_len == 0 ? 1 : *allocation_len) * 2;
+    if (num_items_needed > *allocation_len) {
+        /* Array needs grown; allocate more space */
+        const size_t n = ceil2(num_items_needed);
         void *p = array_realloc(*src, *allocation_len, n, item_size);
         if (!p)
             return WALLY_ENOMEM;
         /* Free and replace the old array with the new enlarged copy */
-        clear_and_free(*src, num_items * item_size);
+        clear_and_free(*src, *allocation_len * item_size);
         *src = p;
         *allocation_len = n;
     }

src/internal.h

@@ -101,9 +101,7 @@ bool clone_data(void **dst, const void *src, size_t len);
 bool clone_bytes(unsigned char **dst, const unsigned char *src, size_t len);
 int replace_bytes(const unsigned char *bytes, size_t bytes_len,
                   unsigned char **bytes_out, size_t *bytes_len_out);
-void *array_realloc(const void *src, size_t old_n, size_t new_n, size_t size);
-
-int array_grow(void **src, size_t num_items, size_t *allocation_len,
+int array_grow(void **src, size_t num_items_needed, size_t *allocation_len,
                size_t item_size);
 
 struct ext_key;

src/map.c

@@ -246,7 +246,7 @@ int map_add(struct wally_map *map_in,
         return ignore_dups ? WALLY_OK : WALLY_EINVAL;
     }
 
-    ret = array_grow((void *)&map_in->items, map_in->num_items,
+    ret = array_grow((void *)&map_in->items, map_in->num_items + 1,
                      &map_in->items_allocation_len, sizeof(struct wally_map_item));
     if (ret == WALLY_OK) {
         struct wally_map_item *new_item = map_in->items + map_in->num_items;

src/psbt.c

@@ -1785,7 +1785,7 @@ int wally_psbt_add_tx_input_at(struct wally_psbt *psbt,
         return ret;
 
     if (psbt->num_inputs >= psbt->inputs_allocation_len &&
-        (ret = array_grow((void *)&psbt->inputs, psbt->num_inputs,
+        (ret = array_grow((void *)&psbt->inputs, psbt->num_inputs + 1,
                           &psbt->inputs_allocation_len,
                           sizeof(*psbt->inputs))) != WALLY_OK)
         return ret;
@@ -1950,7 +1950,7 @@ int wally_psbt_add_tx_output_at(struct wally_psbt *psbt,
         return ret;
 
     if (psbt->num_outputs >= psbt->outputs_allocation_len &&
-        (ret = array_grow((void *)&psbt->outputs, psbt->num_outputs,
+        (ret = array_grow((void *)&psbt->outputs, psbt->num_outputs + 1,
                           &psbt->outputs_allocation_len,
                           sizeof(*psbt->outputs))) != WALLY_OK)
         return ret;

src/transaction.c

@@ -272,6 +272,7 @@ int wally_tx_witness_stack_set(struct wally_tx_witness_stack *stack, size_t inde
                                const unsigned char *witness, size_t witness_len)
 {
     unsigned char *new_witness = NULL;
+    int ret;
 
     if (!is_valid_witness_stack(stack) || (!witness && witness_len))
         return WALLY_EINVAL;
@@ -280,18 +281,12 @@ int wally_tx_witness_stack_set(struct wally_tx_witness_stack *stack, size_t inde
         return WALLY_ENOMEM;
 
     if (index >= stack->num_items) {
-        if (index >= stack->items_allocation_len) {
-            /* Expand the witness array */
-            struct wally_tx_witness_item *p;
-            p = array_realloc(stack->items, stack->items_allocation_len,
-                              index + 1, sizeof(*stack->items));
-            if (!p) {
-                clear_and_free(new_witness, witness_len);
-                return WALLY_ENOMEM;
-            }
-            clear_and_free(stack->items, stack->num_items * sizeof(*stack->items));
-            stack->items = p;
-            stack->items_allocation_len = index + 1;
+        /* Expand the witness array if needed */
+        ret = array_grow((void *)&stack->items, index + 1,
+                         &stack->items_allocation_len, sizeof(*stack->items));
+        if (ret != WALLY_OK) {
+            clear_and_free(new_witness, witness_len);
+            return ret;
         }
         stack->num_items = index + 1;
     }
@@ -1198,18 +1193,11 @@ int wally_tx_add_input_at(struct wally_tx *tx, uint32_t index,
     if (!is_valid_tx(tx) || index > tx->num_inputs || !is_valid_tx_input(input))
         return WALLY_EINVAL;
 
-    if (tx->num_inputs >= tx->inputs_allocation_len) {
-        /* Expand the inputs array */
-        struct wally_tx_input *p;
-        p = array_realloc(tx->inputs, tx->inputs_allocation_len,
-                          tx->num_inputs + 1, sizeof(*tx->inputs));
-        if (!p)
-            return WALLY_ENOMEM;
-
-        clear_and_free(tx->inputs, tx->num_inputs * sizeof(*tx->inputs));
-        tx->inputs = p;
-        tx->inputs_allocation_len += 1;
-    }
+    /* Expand the inputs array if needed */
+    ret = array_grow((void *)&tx->inputs, tx->num_inputs + 1,
+                     &tx->inputs_allocation_len, sizeof(*tx->inputs));
+    if (ret != WALLY_OK)
+        return ret;
 
     memmove(tx->inputs + index + 1, tx->inputs + index,
             (tx->num_inputs - index) * sizeof(*input));
@@ -1427,18 +1415,11 @@ int wally_tx_add_output_at(struct wally_tx *tx, uint32_t index,
     } else if (!is_valid_elements_tx_output(output))
         return WALLY_EINVAL;
 
-    if (tx->num_outputs >= tx->outputs_allocation_len) {
-        /* Expand the outputs array */
-        struct wally_tx_output *p;
-        p = array_realloc(tx->outputs, tx->outputs_allocation_len,
-                          tx->num_outputs + 1, sizeof(*tx->outputs));
-        if (!p)
-            return WALLY_ENOMEM;
-
-        clear_and_free(tx->outputs, tx->num_outputs * sizeof(*tx->outputs));
-        tx->outputs = p;
-        tx->outputs_allocation_len += 1;
-    }
+    /* Expand the outputs array if needed */
+    ret = array_grow((void *)&tx->outputs, tx->num_outputs + 1,
+                     &tx->outputs_allocation_len, sizeof(*tx->outputs));
+    if (ret != WALLY_OK)
+        return ret;
 
     memmove(tx->outputs + index + 1, tx->outputs + index,
             (tx->num_outputs - index) * sizeof(*output));