From d77e7963f946f84c1afd52bdd3ea9f86f6a5b26d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 7 Mar 2017 11:28:09 +1030 Subject: [PATCH] funding_tx: permute inputs, don't re-calculate fees built_utxos needs to calculate fees for figuring out how many utxos to use, so fix that logic and rely on it. We make build_utxos return a pointer array, so funding_tx can simply hand that to permute_inputs. Signed-off-by: Rusty Russell --- lightningd/build_utxos.c | 41 ++++++++++++++++---------------- lightningd/build_utxos.h | 12 +++++----- lightningd/funding_tx.c | 36 +++++++++++----------------- lightningd/funding_tx.h | 22 ++++++++--------- lightningd/peer_control.c | 26 ++++++++++++-------- lightningd/test/run-funding_tx.c | 29 +++++++++++----------- 6 files changed, 81 insertions(+), 85 deletions(-) diff --git a/lightningd/build_utxos.c b/lightningd/build_utxos.c index 4e6780466..a737e1175 100644 --- a/lightningd/build_utxos.c +++ b/lightningd/build_utxos.c @@ -175,39 +175,35 @@ static void unreserve_utxo(struct lightningd *ld, const struct utxo *unres) struct tracked_utxo *utxo; list_for_each(&ld->utxos, utxo, list) { - if (unres->outnum != utxo->utxo.outnum - || !structeq(&unres->txid, &utxo->utxo.txid)) + if (unres != &utxo->utxo) continue; assert(utxo->reserved); - assert(unres->amount == utxo->utxo.amount); - assert(unres->keyindex == utxo->utxo.keyindex); - assert(unres->is_p2sh == utxo->utxo.is_p2sh); utxo->reserved = false; return; } abort(); } -static void destroy_utxos(struct utxo *utxos, struct lightningd *ld) +static void destroy_utxos(const struct utxo **utxos, struct lightningd *ld) { size_t i; for (i = 0; i < tal_count(utxos); i++) - unreserve_utxo(ld, &utxos[i]); + unreserve_utxo(ld, utxos[i]); } -void confirm_utxos(struct lightningd *ld, struct utxo *utxos) +void confirm_utxos(struct lightningd *ld, const struct utxo **utxos) { tal_del_destructor2(utxos, destroy_utxos, ld); } -struct utxo *build_utxos(const tal_t *ctx, - struct lightningd *ld, u64 satoshi_out, - u32 feerate_per_kw, u64 dust_limit, - u64 *change_amount, u32 *change_keyindex) +const struct utxo **build_utxos(const tal_t *ctx, + struct lightningd *ld, u64 satoshi_out, + u32 feerate_per_kw, u64 dust_limit, + u64 *change_satoshis, u32 *change_keyindex) { size_t i = 0; - struct utxo *utxos = tal_arr(ctx, struct utxo, 0); + const struct utxo **utxos = tal_arr(ctx, const struct utxo *, 0); struct tracked_utxo *utxo; /* We assume two outputs for the weight. */ u64 satoshi_in = 0, weight = (4 + (8 + 22) * 2 + 4) * 4; @@ -221,23 +217,26 @@ struct utxo *build_utxos(const tal_t *ctx, continue; tal_resize(&utxos, i+1); - utxos[i] = utxo->utxo; + utxos[i] = &utxo->utxo; utxo->reserved = true; /* Add this input's weight. */ weight += (32 + 4 + 4) * 4; - if (utxos[i].is_p2sh) + if (utxos[i]->is_p2sh) weight += 22 * 4; - - satoshi_in += utxos[i].amount; + /* Account for witness (1 byte count + sig + key */ + weight += 1 + (1 + 73 + 1 + 33); fee = weight * feerate_per_kw / 1000; + satoshi_in += utxos[i]->amount; + if (satoshi_in >= fee + satoshi_out) { /* We simply eliminate change if it's dust. */ - *change_amount = satoshi_in - (fee + satoshi_out); - if (*change_amount < dust_limit) - *change_amount = 0; - else + *change_satoshis = satoshi_in - (fee + satoshi_out); + if (*change_satoshis < dust_limit) { + *change_satoshis = 0; + *change_keyindex = 0; + } else *change_keyindex = ld->bip32_max_index++; return utxos; diff --git a/lightningd/build_utxos.h b/lightningd/build_utxos.h index cfa76c1d8..9fa0fde91 100644 --- a/lightningd/build_utxos.h +++ b/lightningd/build_utxos.h @@ -5,12 +5,12 @@ #include /* Reserves UTXOs to build tx which pays this amount; returns NULL if - * impossible. *change_amount 0 if no change needed. */ -struct utxo *build_utxos(const tal_t *ctx, - struct lightningd *ld, u64 satoshi_out, - u32 feerate_per_kw, u64 dust_limit, - u64 *change_amount, u32 *change_keyindex); + * impossible. *change_satoshis 0 if no change needed. */ +const struct utxo **build_utxos(const tal_t *ctx, + struct lightningd *ld, u64 satoshi_out, + u32 feerate_per_kw, u64 dust_limit, + u64 *change_satoshis, u32 *change_keyindex); /* Once we've spent them, mark them confirmed. */ -void confirm_utxos(struct lightningd *ld, struct utxo *utxos); +void confirm_utxos(struct lightningd *ld, const struct utxo **utxos); #endif /* LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H */ diff --git a/lightningd/funding_tx.c b/lightningd/funding_tx.c index 43b23cb21..e9d51194d 100644 --- a/lightningd/funding_tx.c +++ b/lightningd/funding_tx.c @@ -12,24 +12,22 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx, u32 *outnum, - const struct utxo *utxos, + const struct utxo **utxomap, u64 funding_satoshis, const struct pubkey *local_fundingkey, const struct pubkey *remote_fundingkey, - const struct pubkey *changekey, - u64 feerate_per_kw, - u64 dust_limit_satoshis) + u64 change_satoshis, + const struct pubkey *changekey) { - struct bitcoin_tx *tx = bitcoin_tx(ctx, tal_count(utxos), 2); + struct bitcoin_tx *tx = bitcoin_tx(ctx, tal_count(utxomap), + change_satoshis ? 2 : 1); u8 *wscript; - u64 fee, weight, input_satoshis = 0; size_t i; - for (i = 0; i < tal_count(utxos); i++) { - tx->input[i].txid = utxos[i].txid; - tx->input[i].index = utxos[i].outnum; - tx->input[i].amount = tal_dup(tx, u64, &utxos[i].amount); - input_satoshis += utxos[i].amount; + for (i = 0; i < tal_count(utxomap); i++) { + tx->input[i].txid = utxomap[i]->txid; + tx->input[i].index = utxomap[i]->outnum; + tx->input[i].amount = tal_dup(tx, u64, &utxomap[i]->amount); } tx->output[0].amount = funding_satoshis; @@ -39,25 +37,19 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx, tx->output[0].script = scriptpubkey_p2wsh(tx, wscript); tal_free(wscript); - assert(input_satoshis >= funding_satoshis); tx->output[1].script = scriptpubkey_p2wpkh(tx, changekey); - /* Calculate what weight will be once we've signed. */ - weight = measure_tx_cost(tx) + 4 * (73 + 34); - fee = weight * feerate_per_kw / 1000; - - /* Too small an output after fee? Drop it. */ - if (input_satoshis - funding_satoshis < dust_limit_satoshis + fee) { - tal_resize(&tx->output, 1); - *outnum = 0; - } else { + if (change_satoshis != 0) { const void *map[2]; map[0] = int2ptr(0); map[1] = int2ptr(1); - tx->output[1].amount = input_satoshis - funding_satoshis - fee; + tx->output[1].amount = change_satoshis; permute_outputs(tx->output, tal_count(tx->output), map); *outnum = (map[0] == int2ptr(0) ? 0 : 1); + } else { + *outnum = 0; } + permute_inputs(tx->input, tal_count(tx->input), (const void **)utxomap); return tx; } diff --git a/lightningd/funding_tx.h b/lightningd/funding_tx.h index e2c6bb2e3..acf6f60ea 100644 --- a/lightningd/funding_tx.h +++ b/lightningd/funding_tx.h @@ -13,22 +13,20 @@ struct utxo; /** * funding_tx: create a P2WSH funding transaction for a channel. * @ctx: context to tal from. - * @outnum: txout (0 or 1) which is the funding output. - * @utxo: tal_arr of UTXO to spend as inputs. - * @funding_satoshis: satoshis to output. - * @local_fundingkey: local key for 2of2 funding output. - * @remote_fundingkey: remote key for 2of2 funding output. - * @changekey: key to send change to (if any). - * @feerate_per_kw: feerate for transaction. - * @dust_limit_satoshis: dust limit to trim change output by. + * @outnum: (out) txout (0 or 1) which is the funding output. + * @utxomap: (in/out) tal_arr of UTXO pointers to spend (permuted to match) + * @funding_satoshis: (in) satoshis to output. + * @local_fundingkey: (in) local key for 2of2 funding output. + * @remote_fundingkey: (in) remote key for 2of2 funding output. + * @change_satoshis: (in) amount to send as change. + * @changekey: (in) key to send change to (only used if change_satoshis != 0). */ struct bitcoin_tx *funding_tx(const tal_t *ctx, u32 *outnum, - const struct utxo *utxos, + const struct utxo **utxomap, u64 funding_satoshis, const struct pubkey *local_fundingkey, const struct pubkey *remote_fundingkey, - const struct pubkey *changekey, - u64 feerate_per_kw, - u64 dust_limit_satoshis); + u64 change_satoshis, + const struct pubkey *changekey); #endif /* LIGHTNING_LIGHTNINGD_FUNDING_TX_H */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index b53ca765a..b7786f3c3 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -516,7 +516,7 @@ struct funding_channel { struct peer *peer; struct command *cmd; u64 satoshi; - struct utxo *utxos; + const struct utxo **utxomap; u64 change; u32 change_keyindex; struct crypto_state *cs; @@ -553,15 +553,22 @@ static void opening_release_tx(struct subdaemon *opening, const u8 *resp, struct funding_channel *fc) { u8 *msg; + size_t i; + /* FIXME: marshal code wants array, not array of pointers. */ + struct utxo *utxos = tal_arr(fc, struct utxo, tal_count(fc->utxomap)); peer_set_condition(fc->peer, "Getting HSM to sign funding tx"); /* Get HSM to sign the funding tx. */ + for (i = 0; i < tal_count(fc->utxomap); i++) + utxos[i] = *fc->utxomap[i]; + msg = towire_hsmctl_sign_funding(fc, fc->satoshi, fc->change, fc->change_keyindex, &fc->local_fundingkey, &fc->remote_fundingkey, - fc->utxos); + utxos); + tal_free(utxos); subdaemon_req(fc->peer->ld->hsm, take(msg), -1, NULL, opening_got_hsm_funding_sig, fc); } @@ -584,14 +591,13 @@ static void opening_gen_funding(struct subdaemon *opening, const u8 *resp, return; } - bitcoin_pubkey(fc->peer->ld, &changekey, fc->change_keyindex); + if (fc->change) + bitcoin_pubkey(fc->peer->ld, &changekey, fc->change_keyindex); - /* FIXME: Real feerate! */ - /* FIXME: Real dust limit! */ - fc->funding_tx = funding_tx(fc, &outnum, fc->utxos, fc->satoshi, + fc->funding_tx = funding_tx(fc, &outnum, fc->utxomap, fc->satoshi, &fc->local_fundingkey, &fc->remote_fundingkey, - &changekey, 15000, 546); + fc->change, &changekey); bitcoin_txid(fc->funding_tx, &txid); msg = towire_opening_open_funding(fc, &txid, outnum); @@ -809,9 +815,9 @@ static void json_fund_channel(struct command *cmd, /* Try to do this now, so we know if insufficient funds. */ /* FIXME: Feerate & dustlimit */ - fc->utxos = build_utxos(fc, ld, fc->satoshi, 15000, 600, - &fc->change, &fc->change_keyindex); - if (!fc->utxos) { + fc->utxomap = build_utxos(fc, ld, fc->satoshi, 15000, 600, + &fc->change, &fc->change_keyindex); + if (!fc->utxomap) { command_fail(cmd, "Cannot afford funding transaction"); return; } diff --git a/lightningd/test/run-funding_tx.c b/lightningd/test/run-funding_tx.c index 1dd78578c..6a059a535 100644 --- a/lightningd/test/run-funding_tx.c +++ b/lightningd/test/run-funding_tx.c @@ -39,12 +39,13 @@ int main(void) { tal_t *tmpctx = tal_tmpctx(NULL); struct bitcoin_tx *input, *funding; - u64 feerate_per_kw; + u64 fee; struct pubkey local_funding_pubkey, remote_funding_pubkey; struct privkey input_privkey; struct pubkey inputkey; bool testnet; - struct utxo *utxo = tal_arr(tmpctx, struct utxo, 1); + struct utxo utxo; + const struct utxo **utxomap = tal_arr(tmpctx, const struct utxo *, 1); u64 funding_satoshis; u32 funding_outnum; u8 *subscript; @@ -89,26 +90,26 @@ int main(void) &remote_funding_pubkey)) abort(); - bitcoin_txid(input, &utxo->txid); - utxo->outnum = 0; - utxo->amount = 5000000000; + bitcoin_txid(input, &utxo.txid); + utxo.outnum = 0; + utxo.amount = 5000000000; funding_satoshis = 10000000; - feerate_per_kw = 15000; + fee = 13920; printf("input[0] txid: %s\n", - tal_hexstr(tmpctx, &utxo->txid, sizeof(utxo->txid))); - printf("input[0] input: %u\n", utxo->outnum); - printf("input[0] satoshis: %"PRIu64"\n", utxo->amount); + tal_hexstr(tmpctx, &utxo.txid, sizeof(utxo.txid))); + printf("input[0] input: %u\n", utxo.outnum); + printf("input[0] satoshis: %"PRIu64"\n", utxo.amount); printf("funding satoshis: %"PRIu64"\n", funding_satoshis); - funding = funding_tx(tmpctx, &funding_outnum, utxo, + utxomap[0] = &utxo; + funding = funding_tx(tmpctx, &funding_outnum, utxomap, funding_satoshis, &local_funding_pubkey, &remote_funding_pubkey, - &inputkey, - feerate_per_kw, - 0); - printf("# feerate_per_kw: %"PRIu64"\n", feerate_per_kw); + utxo.amount - fee - funding_satoshis, + &inputkey); + printf("# fee: %"PRIu64"\n", fee); printf("change satoshis: %"PRIu64"\n", funding->output[!funding_outnum].amount);