From 3543530172ea1ee5bfb7c89db5caedfbe7d60f76 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Dec 2017 16:35:01 +1030 Subject: [PATCH] build_utxos: fix weight calculation, and make more accurate. Accuracy improvements: 1. We assumed the output was a p2wpkh, but it can be user-supplied now. 2. We assumed we always had change; remove this for wallet_select_all. Calculation out-by-one fixes: 1. We need to add 1 byte (4 sipa) for the input count. 2. We need to add 1 byte (4 sipa) for the output count. 3. We need to add 1 byte (4 sipa) for the output script length for each output. 4. We need to add 1 byte (4 sipa) for the input script length for each input. 5. We need to add 1 byte (4 sipa) for the PUSH optcode for each P2SH input. The results are now a slight overestimate (due to guessing 73 bytes for signature, whereas they're 71 or 72 in practice). Fixes: #458 Reported-by: Jonas Nick @jonasnick Signed-off-by: Rusty Russell --- lightningd/build_utxos.c | 2 ++ lightningd/build_utxos.h | 1 + lightningd/peer_control.c | 3 ++- wallet/wallet.c | 44 +++++++++++++++++++++++++++++++-------- wallet/wallet.h | 2 ++ wallet/walletrpc.c | 2 ++ 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/lightningd/build_utxos.c b/lightningd/build_utxos.c index 5b2cb2931..85ba22946 100644 --- a/lightningd/build_utxos.c +++ b/lightningd/build_utxos.c @@ -11,12 +11,14 @@ const struct utxo **build_utxos(const tal_t *ctx, struct lightningd *ld, u64 satoshi_out, u32 feerate_per_kw, u64 dust_limit, + size_t outputscriptlen, u64 *change_satoshis, u32 *change_keyindex) { u64 fee_estimate = 0; u64 bip32_max_index = db_get_intvar(ld->wallet->db, "bip32_max_index", 0); const struct utxo **utxos = wallet_select_coins(ctx, ld->wallet, satoshi_out, feerate_per_kw, + outputscriptlen, &fee_estimate, change_satoshis); /* Oops, didn't have enough coins available */ diff --git a/lightningd/build_utxos.h b/lightningd/build_utxos.h index ac59c69b9..305d89067 100644 --- a/lightningd/build_utxos.h +++ b/lightningd/build_utxos.h @@ -9,6 +9,7 @@ const struct utxo **build_utxos(const tal_t *ctx, struct lightningd *ld, u64 satoshi_out, u32 feerate_per_kw, u64 dust_limit, + size_t outputscriptlen, u64 *change_satoshis, u32 *change_keyindex); #endif /* LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index a927a65f4..a552024c0 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2631,7 +2631,8 @@ static void json_fund_channel(struct command *cmd, /* FIXME: dustlimit */ fc->utxomap = build_utxos(fc, cmd->ld, fc->funding_satoshi, get_feerate(cmd->ld->topology, FEERATE_NORMAL), - 600, &fc->change, &fc->change_keyindex); + 600, BITCOIN_SCRIPTPUBKEY_P2WSH_LEN, + &fc->change, &fc->change_keyindex); if (!fc->utxomap) { command_fail(cmd, "Cannot afford funding transaction"); return; diff --git a/wallet/wallet.c b/wallet/wallet.c index a7d3ac31a..2f6c1894a 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -134,22 +134,35 @@ void wallet_confirm_utxos(struct wallet *w, const struct utxo **utxos) static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w, const u64 value, const u32 feerate_per_kw, + size_t outscriptlen, + bool may_have_change, u64 *satoshi_in, u64 *fee_estimate) { size_t i = 0; struct utxo **available; + u64 weight; const struct utxo **utxos = tal_arr(ctx, const struct utxo *, 0); - /* We assume two outputs for the weight. */ - u64 weight = (4 + (8 + 22) * 2 + 4) * 4; tal_add_destructor2(utxos, destroy_utxos, w); + /* version, input count, output count, locktime */ + weight = (4 + 1 + 1 + 4) * 4; + + /* The main output: amount, len, scriptpubkey */ + weight += (8 + 1 + outscriptlen) * 4; + + /* Change output will be P2WPKH */ + if (may_have_change) + weight += (8 + 1 + BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN) * 4; + *fee_estimate = 0; *satoshi_in = 0; available = wallet_get_utxos(ctx, w, output_state_available); for (i = 0; i < tal_count(available); i++) { + size_t input_weight; + tal_resize(&utxos, i + 1); utxos[i] = tal_steal(utxos, available[i]); @@ -157,13 +170,22 @@ static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w, w, &available[i]->txid, available[i]->outnum, output_state_available, output_state_reserved)) fatal("Unable to reserve output"); + + /* Input weight: txid + index + sequence */ + input_weight = (32 + 4 + 4) * 4; - weight += (32 + 4 + 4) * 4; + /* We always encode the length of the script, even if empty */ + input_weight += 1 * 4; + + /* P2SH variants include push of <0 <20-byte-key-hash>> */ if (utxos[i]->is_p2sh) - weight += 22 * 4; + input_weight += 23 * 4; + + /* Account for witness (1 byte count + sig + key) */ + input_weight += 1 + (1 + 73 + 1 + 33); + + weight += input_weight; - /* Account for witness (1 byte count + sig + key */ - weight += 1 + (1 + 73 + 1 + 33); *fee_estimate = weight * feerate_per_kw / 1000; *satoshi_in += utxos[i]->amount; if (*satoshi_in >= *fee_estimate + value) @@ -177,12 +199,14 @@ static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w, const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, const u64 value, const u32 feerate_per_kw, + size_t outscriptlen, u64 *fee_estimate, u64 *changesatoshi) { u64 satoshi_in; const struct utxo **utxo; utxo = wallet_select(ctx, w, value, feerate_per_kw, + outscriptlen, true, &satoshi_in, fee_estimate); /* Couldn't afford it? */ @@ -194,15 +218,17 @@ const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, } const struct utxo **wallet_select_all(const tal_t *ctx, struct wallet *w, - const u32 feerate_per_kw, - u64 *value, - u64 *fee_estimate) + const u32 feerate_per_kw, + size_t outscriptlen, + u64 *value, + u64 *fee_estimate) { u64 satoshi_in; const struct utxo **utxo; /* Huge value, but won't overflow on addition */ utxo = wallet_select(ctx, w, (1ULL << 56), feerate_per_kw, + outscriptlen, false, &satoshi_in, fee_estimate); /* Can't afford fees? */ diff --git a/wallet/wallet.h b/wallet/wallet.h index c2f35ef97..74d66a6c6 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -134,11 +134,13 @@ struct utxo **wallet_get_utxos(const tal_t *ctx, struct wallet *w, const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, const u64 value, const u32 feerate_per_kw, + size_t outscriptlen, u64 *fee_estimate, u64 *change_satoshi); const struct utxo **wallet_select_all(const tal_t *ctx, struct wallet *w, const u32 feerate_per_kw, + size_t outscriptlen, u64 *value, u64 *fee_estimate); diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index e9731b158..707aa438d 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -246,6 +246,7 @@ static void json_withdraw(struct command *cmd, if (withdraw_all) { withdraw->utxos = wallet_select_all(cmd, cmd->ld->wallet, feerate_per_kw, + tal_len(withdraw->destination), &withdraw->amount, &fee_estimate); /* FIXME Pull dust amount from the daemon config */ @@ -259,6 +260,7 @@ static void json_withdraw(struct command *cmd, withdraw->utxos = wallet_select_coins(cmd, cmd->ld->wallet, withdraw->amount, feerate_per_kw, + tal_len(withdraw->destination), &fee_estimate, &withdraw->changesatoshi); if (!withdraw->utxos) {