From 2632cc3f340eeea10e93aad8c434932b54b84113 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 21 Feb 2019 13:08:42 +1030 Subject: [PATCH] lightningd/json: make wallet_tx functions take amount_sat. Using param_tok is generally deprecated, as it doesn't give any sanity checking for the JSON 'check' command. So make param_wtx usable directly, and also make it have a struct amount_sat. Signed-off-by: Rusty Russell --- common/wallet_tx.c | 72 +++++++++++++++++++++++++----------- common/wallet_tx.h | 13 +++++-- lightningd/json.c | 1 + lightningd/json.h | 5 --- lightningd/jsonrpc.c | 17 --------- lightningd/opening_control.c | 51 +++++++++++++------------ wallet/walletrpc.c | 16 +++----- 7 files changed, 96 insertions(+), 79 deletions(-) diff --git a/common/wallet_tx.c b/common/wallet_tx.c index 54e2b3b47..bb9ec7f72 100644 --- a/common/wallet_tx.c +++ b/common/wallet_tx.c @@ -4,26 +4,53 @@ #include #include -void wtx_init(struct command *cmd, struct wallet_tx * wtx) +void wtx_init(struct command *cmd, struct wallet_tx *wtx, struct amount_sat max) { wtx->cmd = cmd; - wtx->amount = 0; - wtx->change_key_index = 0; - wtx->utxos = NULL; - wtx->all_funds = false; + wtx->amount = max; } -static struct command_result *check_amount(const struct wallet_tx *tx, - u64 amount) +struct command_result *param_wtx(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct wallet_tx *wtx) { - if (tal_count(tx->utxos) == 0) { - return command_fail(tx->cmd, FUND_CANNOT_AFFORD, + struct amount_sat max = wtx->amount; + + if (json_tok_streq(buffer, tok, "all")) { + wtx->all_funds = true; + return NULL; + } + wtx->all_funds = false; + if (!parse_amount_sat(&wtx->amount, + buffer + tok->start, tok->end - tok->start)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be satoshis or 'all', not '%.*s'", + name, + tok->end - tok->start, + buffer + tok->start); + + if (amount_sat_greater(wtx->amount, max)) + return command_fail(wtx->cmd, FUND_MAX_EXCEEDED, + "Amount exceeded %s", + type_to_string(tmpctx, struct amount_sat, + &max)); + return NULL; +} + +static struct command_result *check_amount(const struct wallet_tx *wtx, + struct amount_sat amount) +{ + if (tal_count(wtx->utxos) == 0) { + return command_fail(wtx->cmd, FUND_CANNOT_AFFORD, "Cannot afford transaction"); } - if (amount < 546) { - return command_fail(tx->cmd, FUND_OUTPUT_IS_DUST, - "Output %"PRIu64" satoshis would be dust", - amount); + if (amount.satoshis < get_chainparams(wtx->cmd->ld)->dust_limit) { + return command_fail(wtx->cmd, FUND_OUTPUT_IS_DUST, + "Output %s would be dust", + type_to_string(tmpctx, struct amount_sat, + &amount)); } return NULL; } @@ -34,18 +61,21 @@ struct command_result *wtx_select_utxos(struct wallet_tx *tx, { struct command_result *res; u64 fee_estimate; + if (tx->all_funds) { - u64 amount; + struct amount_sat amount; tx->utxos = wallet_select_all(tx->cmd, tx->cmd->ld->wallet, fee_rate_per_kw, out_len, - &amount, + &amount.satoshis, &fee_estimate); res = check_amount(tx, amount); if (res) return res; - if (amount <= tx->amount) { - tx->change = 0; + /* tx->amount is max permissible */ + if (amount_sat_less_eq(amount, tx->amount)) { + tx->change = AMOUNT_SAT(0); + tx->change_key_index = 0; tx->amount = amount; return NULL; } @@ -56,15 +86,15 @@ struct command_result *wtx_select_utxos(struct wallet_tx *tx, } tx->utxos = wallet_select_coins(tx->cmd, tx->cmd->ld->wallet, - tx->amount, + tx->amount.satoshis, fee_rate_per_kw, out_len, - &fee_estimate, &tx->change); + &fee_estimate, &tx->change.satoshis); res = check_amount(tx, tx->amount); if (res) return res; - if (tx->change < 546) { - tx->change = 0; + if (tx->change.satoshis < get_chainparams(tx->cmd->ld)->dust_limit) { + tx->change = AMOUNT_SAT(0); tx->change_key_index = 0; } else { tx->change_key_index = wallet_get_newindex(tx->cmd->ld); diff --git a/common/wallet_tx.h b/common/wallet_tx.h index 536096bbd..ec91979f8 100644 --- a/common/wallet_tx.h +++ b/common/wallet_tx.h @@ -2,6 +2,7 @@ #define LIGHTNING_COMMON_WALLET_TX_H #include "config.h" #include +#include #include #include #include @@ -11,15 +12,21 @@ */ struct wallet_tx { struct command *cmd; - u64 amount; - u64 change; + struct amount_sat amount, change; u32 change_key_index; const struct utxo **utxos; bool all_funds; /* In this case, amount is a maximum. */ }; -void wtx_init(struct command *cmd, struct wallet_tx *wtx); +void wtx_init(struct command *cmd, struct wallet_tx *wtx, struct amount_sat max); + +struct command_result *param_wtx(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct wallet_tx *wtx); + struct command_result *wtx_select_utxos(struct wallet_tx *tx, u32 fee_rate_per_kw, size_t out_len); diff --git a/lightningd/json.c b/lightningd/json.c index b0e9b563d..501213556 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include diff --git a/lightningd/json.h b/lightningd/json.h index 243b84ae2..69f54083c 100644 --- a/lightningd/json.h +++ b/lightningd/json.h @@ -157,9 +157,4 @@ enum address_parse_result json_tok_address_scriptpubkey(const tal_t *ctx, const struct chainparams *chainparams, const char *buffer, const jsmntok_t *tok, const u8 **scriptpubkey); - -/* Parse the satoshi token in wallet_tx. */ -struct command_result *param_wtx(struct wallet_tx * tx, const char * buffer, - const jsmntok_t * sattok, u64 max); - #endif /* LIGHTNING_LIGHTNINGD_JSON_H */ diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index bcddf8477..7e9af5c8d 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -1031,22 +1030,6 @@ json_tok_address_scriptpubkey(const tal_t *cxt, return ADDRESS_PARSE_UNRECOGNIZED; } -struct command_result *param_wtx(struct wallet_tx * tx, const char * buffer, - const jsmntok_t *sattok, u64 max) -{ - if (json_tok_streq(buffer, sattok, "all")) { - tx->all_funds = true; - tx->amount = max; - } else if (!json_to_u64(buffer, sattok, &tx->amount)) { - return command_fail(tx->cmd, JSONRPC2_INVALID_PARAMS, - "Invalid satoshis"); - } else if (tx->amount > max) { - return command_fail(tx->cmd, FUND_MAX_EXCEEDED, - "Amount exceeded %"PRIu64, max); - } - return NULL; -} - static struct command_result *param_command(struct command *cmd, const char *name, const char *buffer, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 150cdec8e..0b4340d36 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -128,7 +128,7 @@ void json_add_uncommitted_channel(struct json_stream *response, json_array_end(response); } - msatoshi_total = uc->fc->wtx.amount * 1000; + msatoshi_total = uc->fc->wtx.amount.satoshis * 1000; our_msatoshi = msatoshi_total - uc->fc->push_msat; json_add_u64(response, "msatoshi_to_us", our_msatoshi); json_add_u64(response, "msatoshi_total", msatoshi_total); @@ -317,16 +317,16 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, &channel_info.remote_per_commit)); /* Generate the funding tx. */ - if (fc->wtx.change + if (!amount_sat_eq(fc->wtx.change, AMOUNT_SAT(0)) && !bip32_pubkey(ld->wallet->bip32_base, &changekey, fc->wtx.change_key_index)) fatal("Error deriving change key %u", fc->wtx.change_key_index); fundingtx = funding_tx(tmpctx, &funding_outnum, - fc->wtx.utxos, fc->wtx.amount, + fc->wtx.utxos, fc->wtx.amount.satoshis, &fc->uc->local_funding_pubkey, &channel_info.remote_fundingkey, - fc->wtx.change, &changekey, + fc->wtx.change.satoshis, &changekey, ld->wallet->bip32_base); log_debug(fc->uc->log, "Funding tx has %zi inputs, %zu outputs:", @@ -346,22 +346,29 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, if (!bitcoin_txid_eq(&funding_txid, &expected_txid)) { log_broken(fc->uc->log, "Funding txid mismatch:" - " satoshi %"PRIu64" change %"PRIu64 + " amount %s change %s" " changeidx %u" " localkey %s remotekey %s", - fc->wtx.amount, - fc->wtx.change, fc->wtx.change_key_index, + type_to_string(tmpctx, struct amount_sat, + &fc->wtx.amount), + type_to_string(tmpctx, struct amount_sat, + &fc->wtx.change), + fc->wtx.change_key_index, type_to_string(fc, struct pubkey, &fc->uc->local_funding_pubkey), type_to_string(fc, struct pubkey, &channel_info.remote_fundingkey)); was_pending(command_fail(fc->cmd, JSONRPC2_INVALID_PARAMS, "Funding txid mismatch:" - " satoshi %"PRIu64" change %"PRIu64 + " amount %s change %s" " changeidx %u" " localkey %s remotekey %s", - fc->wtx.amount, - fc->wtx.change, + type_to_string(tmpctx, + struct amount_sat, + &fc->wtx.amount), + type_to_string(tmpctx, + struct amount_sat, + &fc->wtx.change), fc->wtx.change_key_index, type_to_string(fc, struct pubkey, &fc->uc->local_funding_pubkey), @@ -376,7 +383,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, &remote_commit_sig, &funding_txid, funding_outnum, - fc->wtx.amount, + fc->wtx.amount.satoshis, fc->push_msat, fc->channel_flags, &channel_info, @@ -391,7 +398,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, log_debug(channel->log, "Getting HSM to sign funding tx"); msg = towire_hsm_sign_funding(tmpctx, channel->funding_satoshi, - fc->wtx.change, fc->wtx.change_key_index, + fc->wtx.change.satoshis, + fc->wtx.change_key_index, &fc->uc->local_funding_pubkey, &channel_info.remote_fundingkey, fc->wtx.utxos); @@ -804,7 +812,6 @@ static struct command_result *json_fund_channel(struct command *cmd, const jsmntok_t *params) { struct command_result *res; - const jsmntok_t *sattok; struct funding_channel * fc = tal(cmd, struct funding_channel); struct pubkey *id; struct peer *peer; @@ -812,23 +819,21 @@ static struct command_result *json_fund_channel(struct command *cmd, u32 *feerate_per_kw; bool *announce_channel; u8 *msg; - u64 max_funding_satoshi = get_chainparams(cmd->ld)->max_funding_satoshi; + struct amount_sat max_funding_satoshi; + + max_funding_satoshi.satoshis = get_chainparams(cmd->ld)->max_funding_satoshi; fc->cmd = cmd; fc->uc = NULL; - wtx_init(cmd, &fc->wtx); + wtx_init(cmd, &fc->wtx, max_funding_satoshi); if (!param(fc->cmd, buffer, params, p_req("id", param_pubkey, &id), - p_req("satoshi", param_tok, &sattok), + p_req("satoshi", param_wtx, &fc->wtx), p_opt("feerate", param_feerate, &feerate_per_kw), p_opt_def("announce", param_bool, &announce_channel, true), NULL)) return command_param_failed(); - res = param_wtx(&fc->wtx, buffer, sattok, max_funding_satoshi); - if (res) - return res; - if (!feerate_per_kw) { feerate_per_kw = tal(cmd, u32); *feerate_per_kw = opening_feerate(cmd->ld->topology); @@ -876,16 +881,16 @@ static struct command_result *json_fund_channel(struct command *cmd, if (res) return res; - assert(fc->wtx.amount <= max_funding_satoshi); + assert(!amount_sat_greater(fc->wtx.amount, max_funding_satoshi)); peer->uncommitted_channel->fc = tal_steal(peer->uncommitted_channel, fc); fc->uc = peer->uncommitted_channel; msg = towire_opening_funder(NULL, - fc->wtx.amount, + fc->wtx.amount.satoshis, fc->push_msat, *feerate_per_kw, - fc->wtx.change, + fc->wtx.change.satoshis, fc->wtx.change_key_index, fc->channel_flags, fc->wtx.utxos, diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 9f1209181..0dee0038c 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -66,7 +66,7 @@ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED, /* Note normally, change_satoshi == withdraw->wtx.change, but * not if we're actually making a payment to ourselves! */ - assert(change_satoshi >= withdraw->wtx.change); + assert(change_satoshi >= withdraw->wtx.change.satoshis); struct json_stream *response = json_stream_success(cmd); json_object_start(response, NULL); @@ -93,7 +93,7 @@ static struct command_result *json_withdraw(struct command *cmd, const jsmntok_t *obj UNNEEDED, const jsmntok_t *params) { - const jsmntok_t *desttok, *sattok; + const jsmntok_t *desttok; struct withdrawal *withdraw = tal(cmd, struct withdrawal); u32 *feerate_per_kw; struct bitcoin_tx *tx; @@ -103,19 +103,15 @@ static struct command_result *json_withdraw(struct command *cmd, struct command_result *res; withdraw->cmd = cmd; - wtx_init(cmd, &withdraw->wtx); + wtx_init(cmd, &withdraw->wtx, AMOUNT_SAT(-1ULL)); if (!param(cmd, buffer, params, p_req("destination", param_tok, &desttok), - p_req("satoshi", param_tok, &sattok), + p_req("satoshi", param_wtx, &withdraw->wtx), p_opt("feerate", param_feerate, &feerate_per_kw), NULL)) return command_param_failed(); - res = param_wtx(&withdraw->wtx, buffer, sattok, -1ULL); - if (res) - return res; - if (!feerate_per_kw) { res = param_feerate_estimate(cmd, &feerate_per_kw, FEERATE_NORMAL); @@ -160,8 +156,8 @@ static struct command_result *json_withdraw(struct command *cmd, txfilter_add_derkey(cmd->ld->owned_txfilter, ext.pub_key); u8 *msg = towire_hsm_sign_withdrawal(cmd, - withdraw->wtx.amount, - withdraw->wtx.change, + withdraw->wtx.amount.satoshis, + withdraw->wtx.change.satoshis, withdraw->wtx.change_key_index, withdraw->destination, withdraw->wtx.utxos);