From db3c38726409f4c5d97f08b4be00cd8ef7151948 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 29 Aug 2018 06:16:34 +0930 Subject: [PATCH] feerate: allow names 'urgent' 'normal' and 'slow'. Signed-off-by: Rusty Russell --- doc/lightning-fundchannel.7 | 4 +++- doc/lightning-fundchannel.7.txt | 7 +++++-- doc/lightning-withdraw.7 | 4 +++- doc/lightning-withdraw.7.txt | 7 +++++-- lightningd/chaintopology.c | 14 +++++++++++++- lightningd/chaintopology.h | 6 ++++++ lightningd/json.c | 5 +++++ lightningd/test/run-param.c | 7 +++++++ tests/test_connection.py | 19 +++++++++++++++---- wallet/walletrpc.c | 7 +------ 10 files changed, 63 insertions(+), 17 deletions(-) diff --git a/doc/lightning-fundchannel.7 b/doc/lightning-fundchannel.7 index 08f6c7396..3c85d3345 100644 --- a/doc/lightning-fundchannel.7 +++ b/doc/lightning-fundchannel.7 @@ -40,7 +40,9 @@ The \fBfundchannel\fR RPC command opens a payment channel with a peer by commiti .sp \fIsatoshi\fR is the amount in satoshis taken from the internal wallet to fund the channel\&. The string \fIall\fR can be used to specify all available funds (or 16777215 satoshi if more is available)\&. The value cannot be less than the dust limit, currently set to 546, nor more than 16777215 satoshi\&. .sp -\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&. +\fIfeerate\fR is an optional feerate to use\&. It can be one of the strings \fIurgent\fR, \fInormal\fR or \fIslow\fR to use lightningd\(cqs internal estimates: \fInormal\fR is the default\&. +.sp +Otherwise, \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&. .SH "RETURN VALUE" .sp On success, the \fItx\fR and \fItxid\fR of the transaction is returned, as well as the \fIchannel_id\fR of the newly created channel\&. On failure, an error is reported and the channel is not funded\&. diff --git a/doc/lightning-fundchannel.7.txt b/doc/lightning-fundchannel.7.txt index faeedebda..2ed2e649b 100644 --- a/doc/lightning-fundchannel.7.txt +++ b/doc/lightning-fundchannel.7.txt @@ -27,8 +27,11 @@ The string 'all' can be used to specify all available funds (or 16777215 satoshi The value cannot be less than the dust limit, currently set to 546, nor more than 16777215 satoshi. -'feerate' is an optional feerate to use, overriding lightningd's -internal estimate. 'feerate' is a number, with an optional suffix: +'feerate' is an optional feerate to use. It can be one of the strings +'urgent', 'normal' or 'slow' to use lightningd's internal estimates: +'normal' is the default. + +Otherwise, 'feerate' is a number, with an optional suffix: 'perkw' means the number is interpreted as satoshi-per-kilosipa (weight), and 'perkb' means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is equivalent to 'perkb'. diff --git a/doc/lightning-withdraw.7 b/doc/lightning-withdraw.7 index b2338aa67..701bd46b0 100644 --- a/doc/lightning-withdraw.7 +++ b/doc/lightning-withdraw.7 @@ -40,7 +40,9 @@ The address can be of any Bitcoin accepted type, including bech32\&. .sp \fIsatoshi\fR is the amount to be withdrawn from the internal wallet (expressed, as name suggests, in satoshi)\&. The string \fIall\fR can be used to specify withdrawal of all available funds\&. .sp -\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&. +\fIfeerate\fR is an optional feerate to use\&. It can be one of the strings \fIurgent\fR, \fInormal\fR or \fIslow\fR to use lightningd\(cqs internal estimates: \fInormal\fR is the default\&. +.sp +Otherwise, \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&. .SH "RETURN VALUE" .sp On success, an object with attributes \fItx\fR and \fItxid\fR will be returned\&. diff --git a/doc/lightning-withdraw.7.txt b/doc/lightning-withdraw.7.txt index 45c40bbb0..f7db1899c 100644 --- a/doc/lightning-withdraw.7.txt +++ b/doc/lightning-withdraw.7.txt @@ -24,8 +24,11 @@ wallet (expressed, as name suggests, in satoshi). The string 'all' can be used to specify withdrawal of all available funds. -'feerate' is an optional feerate to use, overriding lightningd's -internal estimate. 'feerate' is a number, with an optional suffix: +'feerate' is an optional feerate to use. It can be one of the strings +'urgent', 'normal' or 'slow' to use lightningd's internal estimates: +'normal' is the default. + +Otherwise, 'feerate' is a number, with an optional suffix: 'perkw' means the number is interpreted as satoshi-per-kilosipa (weight), and 'perkb' means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is equivalent to 'perkb'. diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 60f83e8bc..ce6d35f2f 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -276,7 +276,7 @@ static void watch_for_utxo_reconfirmation(struct chain_topology *topo, } } -static const char *feerate_name(enum feerate feerate) +const char *feerate_name(enum feerate feerate) { switch (feerate) { case FEERATE_URGENT: return "urgent"; @@ -286,6 +286,18 @@ static const char *feerate_name(enum feerate feerate) abort(); } +bool json_feerate_estimate(struct command *cmd, + u32 **feerate_per_kw, enum feerate feerate) +{ + *feerate_per_kw = tal(cmd, u32); + **feerate_per_kw = try_get_feerate(cmd->ld->topology, feerate); + if (!**feerate_per_kw) { + command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); + return false; + } + return true; +} + /* Mutual recursion via timer. */ static void next_updatefee_timer(struct chain_topology *topo); diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index ced494752..c2bf44e3d 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -150,6 +150,12 @@ u32 unilateral_feerate(struct chain_topology *topo); u32 feerate_from_style(u32 feerate, enum feerate_style style); u32 feerate_to_style(u32 feerate_perkw, enum feerate_style style); +const char *feerate_name(enum feerate feerate); + +/* Set feerate_per_kw to this estimate, or fail cmd */ +bool json_feerate_estimate(struct command *cmd, + u32 **feerate_per_kw, enum feerate feerate); + /* Broadcast a single tx, and rebroadcast as reqd (copies tx). * If failed is non-NULL, call that and don't rebroadcast. */ void broadcast_tx(struct chain_topology *topo, diff --git a/lightningd/json.c b/lightningd/json.c index cd091b0ec..c01cccda8 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -266,6 +266,11 @@ bool json_tok_feerate(struct command *cmd, const char *name, enum feerate_style style; unsigned int num; + for (size_t i = 0; i < NUM_FEERATES; i++) { + if (json_tok_streq(buffer, tok, feerate_name(i))) + return json_feerate_estimate(cmd, feerate, i); + } + /* We have to split the number and suffix. */ suffix.start = suffix.end; while (suffix.start > base.start && !isdigit(buffer[suffix.start-1])) { diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c index 45871daca..79419eb3c 100644 --- a/lightningd/test/run-param.c +++ b/lightningd/test/run-param.c @@ -48,9 +48,16 @@ void command_fail_detailed(struct command *cmd, int code, /* Generated stub for feerate_from_style */ u32 feerate_from_style(u32 feerate UNNEEDED, enum feerate_style style UNNEEDED) { fprintf(stderr, "feerate_from_style called!\n"); abort(); } +/* Generated stub for feerate_name */ +const char *feerate_name(enum feerate feerate UNNEEDED) +{ fprintf(stderr, "feerate_name called!\n"); abort(); } /* Generated stub for fmt_wireaddr_without_port */ char *fmt_wireaddr_without_port(const tal_t *ctx UNNEEDED, const struct wireaddr *a UNNEEDED) { fprintf(stderr, "fmt_wireaddr_without_port called!\n"); abort(); } +/* Generated stub for json_feerate_estimate */ +bool json_feerate_estimate(struct command *cmd UNNEEDED, + u32 **feerate_per_kw UNNEEDED, enum feerate feerate UNNEEDED) +{ fprintf(stderr, "json_feerate_estimate called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ struct json { diff --git a/tests/test_connection.py b/tests/test_connection.py index 1238c4237..f46a2997b 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1124,6 +1124,17 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): with pytest.raises(RpcError, match=r'Cannot estimate fees'): l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all') + # Can't use feerate names, either. + with pytest.raises(RpcError, match=r'Cannot estimate fees'): + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all', 'urgent') + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all', 'normal') + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all', 'slow') + + with pytest.raises(RpcError, match=r'Cannot estimate fees'): + l1.rpc.fundchannel(l2.info['id'], 10**6, 'urgent') + l1.rpc.fundchannel(l2.info['id'], 10**6, 'normal') + l1.rpc.fundchannel(l2.info['id'], 10**6, 'slow') + # Can with manual feerate. l1.rpc.withdraw(l2.rpc.newaddr()['address'], 10000, '1500perkb') l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw') @@ -1161,12 +1172,12 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): l1.set_feerates((15000, 7500, 3750), True) l1.daemon.wait_for_log('Feerate estimate for normal set to [567][0-9]{3}') - # Can now fund a channel. + # Can now fund a channel (as a test, use slow feerate). l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.rpc.fundchannel(l2.info['id'], 10**6) + l1.rpc.fundchannel(l2.info['id'], 10**6, 'slow') - # Can withdraw. - l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all') + # Can withdraw (use urgent feerate). + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all', 'urgent') @unittest.skipIf(not DEVELOPER, "needs --dev-disconnect") diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index fbc1f3a20..32bcbc562 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -107,13 +107,8 @@ static void json_withdraw(struct command *cmd, return; if (!feerate_per_kw) { - feerate_per_kw = tal(cmd, u32); - *feerate_per_kw - = try_get_feerate(cmd->ld->topology, FEERATE_NORMAL); - if (!*feerate_per_kw) { - command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); + if (!json_feerate_estimate(cmd, &feerate_per_kw, FEERATE_NORMAL)) return; - } } /* Parse address. */