From 05a0b8abb4cd55fc284e8d5d48f768e4dc6b871d Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Sun, 1 Mar 2026 17:54:16 -0500 Subject: [PATCH] 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: https://github.com/ElementsProject/lightning/issues/8924 Signed-off-by: Matt Whitlock --- src/internal.c | 34 ++++++++++++++++++++++++------ src/internal.h | 4 +--- src/map.c | 2 +- src/psbt.c | 4 ++-- src/transaction.c | 53 +++++++++++++++-------------------------------- 5 files changed, 49 insertions(+), 48 deletions(-) diff --git a/src/internal.c b/src/internal.c index 451fdf4d0..5bc5bff2b 100644 --- a/src/internal.c +++ b/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; } diff --git a/src/internal.h b/src/internal.h index bc07f5163..68f32b068 100644 --- a/src/internal.h +++ b/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; diff --git a/src/map.c b/src/map.c index 332cf0a75..17ab27a40 100644 --- a/src/map.c +++ b/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; diff --git a/src/psbt.c b/src/psbt.c index 74b46a549..552ffc2fc 100644 --- a/src/psbt.c +++ b/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; diff --git a/src/transaction.c b/src/transaction.c index 4d191f423..bd08f5ffc 100644 --- a/src/transaction.c +++ b/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));