From 7e03db5062de3e5a0c94404c8651c503937174c9 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 14 Mar 2019 17:47:13 +0100 Subject: [PATCH] tx: Change permute_{inputs,outputs} to sort both old and new txs This is required in order for both old and new transactions to be identical. Signed-off-by: Christian Decker --- channeld/commit_tx.c | 2 +- common/close_tx.c | 2 +- common/funding_tx.c | 4 +- common/initial_commit_tx.c | 2 +- common/permute_tx.c | 84 ++++++++++++++++++++++++++++++++------ common/permute_tx.h | 10 ++--- common/withdraw_tx.c | 4 +- 7 files changed, 83 insertions(+), 25 deletions(-) diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index 7fd114149..c4a5d860d 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -272,7 +272,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, * 7. Sort the outputs into [BIP 69+CLTV * order](#transaction-input-and-output-ordering) */ - permute_outputs(tx->output, cltvs, (const void **)*htlcmap); + permute_outputs(tx, cltvs, (const void **)*htlcmap); /* BOLT #3: * diff --git a/common/close_tx.c b/common/close_tx.c index a4318059a..4760bccb5 100644 --- a/common/close_tx.c +++ b/common/close_tx.c @@ -61,6 +61,6 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx, return tal_free(tx); tal_resize(&tx->output, num_outputs); - permute_outputs(tx->output, NULL, NULL); + permute_outputs(tx, NULL, NULL); return tx; } diff --git a/common/funding_tx.c b/common/funding_tx.c index f4385aff4..d2751deda 100644 --- a/common/funding_tx.c +++ b/common/funding_tx.c @@ -39,12 +39,12 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx, map[1] = int2ptr(1); tx->output[1].script = scriptpubkey_p2wpkh(tx, changekey); tx->output[1].amount = change; - permute_outputs(tx->output, NULL, map); + permute_outputs(tx, NULL, map); *outnum = (map[0] == int2ptr(0) ? 0 : 1); } else { *outnum = 0; } - permute_inputs(tx->input, (const void **)utxomap); + permute_inputs(tx, (const void **)utxomap); return tx; } diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 78639b4b2..a3cfdbc03 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -199,7 +199,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * 7. Sort the outputs into [BIP 69+CLTV * order](#transaction-input-and-output-ordering) */ - permute_outputs(tx->output, NULL, NULL); + permute_outputs(tx, NULL, NULL); /* BOLT #3: * diff --git a/common/permute_tx.c b/common/permute_tx.c index 909ffdf18..74ff0e468 100644 --- a/common/permute_tx.c +++ b/common/permute_tx.c @@ -54,10 +54,31 @@ static void swap_inputs(struct bitcoin_tx_input *inputs, } } -void permute_inputs(struct bitcoin_tx_input *inputs, - const void **map) +static void swap_wally_inputs(struct wally_tx_input *inputs, + const void **map, + size_t i1, size_t i2) { - size_t i; + struct wally_tx_input tmpinput; + const void *tmp; + + if (i1 == i2) + return; + + tmpinput = inputs[i1]; + inputs[i1] = inputs[i2]; + inputs[i2] = tmpinput; + + if (map) { + tmp = map[i1]; + map[i1] = map[i2]; + map[i2] = tmp; + } +} + +void permute_inputs(struct bitcoin_tx *tx, const void **map) +{ + size_t i, best_pos; + struct bitcoin_tx_input *inputs = tx->input; size_t num_inputs = tal_count(inputs); /* We can't permute nothing! */ @@ -66,9 +87,14 @@ void permute_inputs(struct bitcoin_tx_input *inputs, /* Now do a dumb sort (num_inputs is small). */ for (i = 0; i < num_inputs-1; i++) { + best_pos = i + find_best_in(inputs + i, num_inputs - i); /* Swap best into first place. */ - swap_inputs(inputs, map, - i, i + find_best_in(inputs + i, num_inputs - i)); + swap_inputs(inputs, map, i, best_pos); + + /* TODO(cdecker) Remove once all transactions pass this test */ + if (tx->wtx->num_inputs == num_inputs) + /* TODO(cdecker) Set `map` once the old style is removed */ + swap_wally_inputs(tx->wtx->inputs, NULL, i, best_pos); } } @@ -99,6 +125,33 @@ static void swap_outputs(struct bitcoin_tx_output *outputs, } } +static void swap_wally_outputs(struct wally_tx_output *outputs, + const void **map, + u32 *cltvs, + size_t i1, size_t i2) +{ + struct wally_tx_output tmpoutput; + + if (i1 == i2) + return; + + tmpoutput = outputs[i1]; + outputs[i1] = outputs[i2]; + outputs[i2] = tmpoutput; + + if (map) { + const void *tmp = map[i1]; + map[i1] = map[i2]; + map[i2] = tmp; + } + + if (cltvs) { + u32 tmp = cltvs[i1]; + cltvs[i1] = cltvs[i2]; + cltvs[i2] = tmp; + } +} + static bool output_better(const struct bitcoin_tx_output *a, u32 cltv_a, const struct bitcoin_tx_output *b, @@ -149,11 +202,10 @@ static size_t find_best_out(struct bitcoin_tx_output *outputs, return best; } -void permute_outputs(struct bitcoin_tx_output *outputs, - u32 *cltvs, - const void **map) +void permute_outputs(struct bitcoin_tx *tx, u32 *cltvs, const void **map) { - size_t i; + size_t i, best_pos; + struct bitcoin_tx_output *outputs = tx->output; size_t num_outputs = tal_count(outputs); /* We can't permute nothing! */ @@ -162,10 +214,18 @@ void permute_outputs(struct bitcoin_tx_output *outputs, /* Now do a dumb sort (num_outputs is small). */ for (i = 0; i < num_outputs-1; i++) { + best_pos = + i + find_best_out(outputs + i, cltvs ? cltvs + i : NULL, + num_outputs - i); + /* Swap best into first place. */ swap_outputs(outputs, map, cltvs, - i, i + find_best_out(outputs + i, - cltvs ? cltvs + i : NULL, - num_outputs - i)); + i, best_pos); + + /* TODO(cdecker) Remove once all transactions pass this test */ + if (tx->wtx->num_outputs == num_outputs) + /* TODO(cdecker) Set `map` once the old style is removed */ + swap_wally_outputs(tx->wtx->outputs, NULL, NULL, + i, best_pos); } } diff --git a/common/permute_tx.h b/common/permute_tx.h index 6968c94fa..4107fd430 100644 --- a/common/permute_tx.h +++ b/common/permute_tx.h @@ -7,21 +7,19 @@ struct htlc; /** * permute_inputs: permute the transaction inputs into BIP69 order. - * @inputs: usually bitcoin_tx->inputs, must be tal_arr. + * @tx: the transaction whose inputs are to be sorted (inputs must be tal_arr). * @map: if non-NULL, pointers to be permuted the same as the inputs. */ -void permute_inputs(struct bitcoin_tx_input *inputs, const void **map); +void permute_inputs(struct bitcoin_tx *tx, const void **map); /** * permute_outputs: permute the transaction outputs into BIP69 + cltv order. - * @outputs: usually bitcoin_tx->outputs, must be tal_arr. + * @tx: the transaction whose outputs are to be sorted (outputs must be tal_arr). * @cltvs: CLTV delays to use as a tie-breaker, or NULL. * @map: if non-NULL, pointers to be permuted the same as the outputs. * * So the caller initiates the map with which htlcs are used, it * can easily see which htlc (if any) is in output #0 with map[0]. */ -void permute_outputs(struct bitcoin_tx_output *outputs, - u32 *cltvs, - const void **map); +void permute_outputs(struct bitcoin_tx *tx, u32 *cltvs, const void **map); #endif /* LIGHTNING_COMMON_PERMUTE_TX_H */ diff --git a/common/withdraw_tx.c b/common/withdraw_tx.c index 8cd9d52bd..092a392dd 100644 --- a/common/withdraw_tx.c +++ b/common/withdraw_tx.c @@ -29,9 +29,9 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx, map[1] = int2ptr(1); tx->output[1].script = scriptpubkey_p2wpkh(tx, changekey); tx->output[1].amount = change; - permute_outputs(tx->output, NULL, map); + permute_outputs(tx, NULL, map); } - permute_inputs(tx->input, (const void **)utxos); + permute_inputs(tx, (const void **)utxos); return tx; }