diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 451cf1f94246..b0029cdc7689 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -2593,20 +2593,12 @@ json_openchannel_bump(struct command *cmd, * - MUST set `feerate` greater than or equal to 25/24 times the * `feerate` of the previously constructed transaction, rounded * down. + * + * The ramp is computed below, after the state gates: for any + * channel without an inflight (every locally-opened V1 channel) + * channel_last_funding_feerate() returns 0 and the 25/24 ramp + * assert would fire before these honest typed errors. */ - last_feerate_perkw = channel_last_funding_feerate(channel); - next_feerate_min = last_feerate_perkw * 25 / 24; - assert(next_feerate_min > last_feerate_perkw); - if (!info->feerate_per_kw_funding) { - info->feerate_per_kw_funding = tal(info, u32); - *info->feerate_per_kw_funding = next_feerate_min; - } else if (*info->feerate_per_kw_funding < next_feerate_min) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Next feerate must be at least 1/24th" - " greater than the last. Min req %u," - " you proposed %u", - next_feerate_min, - *info->feerate_per_kw_funding); /* BOLT #2: * - if both nodes advertised `option_support_large_channel`: @@ -2645,6 +2637,21 @@ json_openchannel_bump(struct command *cmd, "No inflight for this channel exists."); } + /* Only now is last_feerate_perkw guaranteed nonzero. */ + last_feerate_perkw = channel_last_funding_feerate(channel); + next_feerate_min = last_feerate_perkw * 25 / 24; + assert(next_feerate_min > last_feerate_perkw); + if (!info->feerate_per_kw_funding) { + info->feerate_per_kw_funding = tal(info, u32); + *info->feerate_per_kw_funding = next_feerate_min; + } else if (*info->feerate_per_kw_funding < next_feerate_min) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Next feerate must be at least 1/24th" + " greater than the last. Min req %u," + " you proposed %u", + next_feerate_min, + *info->feerate_per_kw_funding); + if (!inflight->remote_tx_sigs) { return command_fail(cmd, FUNDING_STATE_INVALID, "Funding sigs for this channel not " diff --git a/lightningd/invoice.c b/lightningd/invoice.c index bb9a1aa54071..e10c8df96e9e 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -1152,6 +1152,17 @@ static struct command_result *json_invoice(struct command *cmd, return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "dev-routes requires --developer"); + /* Two hard limits on how far in the future an invoice can expire: + * push_varlen_field() can only encode up to 60 bits (larger values + * abort the daemon in bolt11_encode()), and the invoice expiration + * timer overflows its u64 nanosecond-based grain count far below + * that, leaving the expiry check looping forever. 2^32 seconds + * (~136 years) keeps a wide margin under both. */ + if (*expiry >= (u64)1 << 32) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "expiry must be below 2^32 seconds" + " (~136 years)"); + if (strlen(info->label->s) > inv_max_label_len) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Label '%s' over %zu bytes", info->label->s, inv_max_label_len); diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 8ef2d26d7f30..78d8620e17d6 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -442,6 +442,30 @@ def test_invoice_expiry(node_factory, executor): assert expiry >= start + 1 and expiry <= end + 1 +def test_invoice_expiry_too_large(node_factory): + """An expiry too large to be safe must be refused, not crash or wedge. + + The `x` field is encoded by push_varlen_field(), which can only + express values of up to 60 bits and aborts the whole daemon for + anything larger. Long before that, the invoice expiration timer's + nanosecond-based counter overflows and the expiry check busy-loops + forever, so anything beyond 2^32 seconds (~136 years) is refused. + """ + l1 = node_factory.get_node() + + # The exact boundary still works: 2^32 - 1 is ~136 years of headroom. + ok = l1.rpc.invoice(amount_msat=1000, label='expiry-boundary-ok', + description='boundary', expiry=2**32 - 1) + assert ok['bolt11'] + + # One above the boundary: typed refusal, daemon stays alive. + with pytest.raises(RpcError, match='expiry must be below') as err: + l1.rpc.invoice(amount_msat=1000, label='expiry-too-large', + description='too large', expiry=2**32) + assert err.value.error['code'] == -32602 + assert l1.rpc.getinfo()['id'] + + def test_waitinvoice(node_factory, executor): """Test waiting for one invoice will not return if another invoice is paid. """ diff --git a/tests/test_opening.py b/tests/test_opening.py index e23283f7251c..3f436ab29f40 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -484,6 +484,30 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams): l1.daemon.wait_for_log('sendrawtx exit 0') +def test_openchannel_bump_no_inflight(node_factory, bitcoind): + """openchannel_bump on a channel without an in-flight funding attempt + must return the typed error, not crash. + + json_openchannel_bump used to compute the BOLT-2 25/24 feerate ramp + and assert next_feerate_min > last_feerate_perkw before the + channel-state gates: channel_last_funding_feerate() returns 0 when + no inflight exists, so the assert evaluated `0 > 0` and killed the + daemon. + """ + l1, l2 = node_factory.line_graph(2, fundchannel=True) + cid = only_one(l1.rpc.listpeerchannels()['channels'])['channel_id'] + + # A well-formed request: the refusal must come from the channel + # state, not from malformed input. + psbt = l1.rpc.fundpsbt(satoshi='500000sat', feerate='253perkw', + startweight=100)['psbt'] + + with pytest.raises(RpcError, match='not eligible|No inflight') as err: + l1.rpc.openchannel_bump(cid, '400000sat', psbt) + assert err.value.error['code'] == 312 # FUNDING_STATE_INVALID + assert l1.rpc.getinfo()['id'] + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @pytest.mark.openchannel('v2') def test_v2_rbf_abort_retry(node_factory, bitcoind, chainparams):