openingd: test that channel open seems possible before bothering peer.
What the developer wrote
Authored by Rusty Russell
openingd: test that channel open seems possible before bothering peer.
Fixes: https://github.com/ElementsProject/lightning/issues/4873
In particular, we used to get upset when a peer accepts our channel,
if it was too small! We should do reasonable checks first.
We no longer try to send requests to delay for 2017 blocks though,
so remove that test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: trying to create a channel below our own min-capacity-sat will now fail before asking the peer, not with an error blaming the peer when they accept!
What changed, and why it matters
This change fixes a user-experience and protocol-courtesy bug in Core Lightning's channel-opening code. Previously, if you tried to open a channel smaller than your own configured minimum, the node would still ask the peer to open it and then blame the peer when the peer accepted. Now the node checks its own settings first and fails locally with a clear message before bothering the peer. It is not a funds-loss or remote-code-execution vulnerability.
Treat as a normal bug-fix patch. No urgent security action required. Users may upgrade at their usual cadence. Operators should note that channel-open RPC calls below local min-capacity-sat will now fail locally rather than after peer interaction.
Security signals we found
Local policy validation moved earlier in protocol flow
Prevents misleading peer-blaming error messages
No memory safety, cryptographic, or remote-exploitable primitive changed
Changelog labels as protocol bug fix, not security vulnerability
Evidence from the diff
In openingd/openingd.c, funder_channel_start() now calls check_config_bounds() using the local configuration for both sides before sending open_channel. If the proposed funding amount would violate the local node’s own min-capacity-sat or related limits, it aborts negotiation immediately with a descriptive reason. The commit also removes an obsolete test that relied on a 2017-block to_self_delay delay path, and re-enables/adjusts test_opening_below_min_capacity_sat.
Changed components
openingd/openingd.ctests/test_connection.pytests/test_opening.pyInspect captured patch +15 / −21
diff --git a/openingd/openingd.c b/openingd/openingd.c
index 1990a9d2..23331faf 100644
--- a/openingd/openingd.c
+++ b/openingd/openingd.c
@@ -316,6 +316,20 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags,
state->feerate_per_kw = nonanchor_feerate;
}
+ /* If they use the same settings as us, would we fail? If so, do that now. */
+ if (!check_config_bounds(tmpctx, state->funding_sats,
+ state->feerate_per_kw,
+ state->max_to_self_delay,
+ state->min_effective_htlc_capacity,
+ &state->localconf,
+ &state->localconf,
+ channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX),
+ &err_reason)) {
+ negotiation_aborted(state,
+ tal_fmt(tmpctx, "Not opening because if they used the same setting as us %s",
+ err_reason));
+ }
+
open_tlvs = tlv_open_channel_tlvs_new(tmpctx);
open_tlvs->upfront_shutdown_script
= state->upfront_shutdown_script[LOCAL];
diff --git a/tests/test_connection.py b/tests/test_connection.py
index defeeea4..ed81c75f 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -327,26 +327,6 @@ def test_balance(node_factory):
assert p2['total_msat'] == 10**6 * 1000
-@pytest.mark.openchannel('v1')
-@pytest.mark.openchannel('v2')
-def test_bad_opening(node_factory):
- # l1 asks for a too-long locktime
- l1 = node_factory.get_node(options={'watchtime-blocks': 2017})
- l2 = node_factory.get_node()
- ret = l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-
- assert ret['id'] == l2.info['id']
-
- l1.daemon.wait_for_log('Handed peer, entering loop')
- l2.daemon.wait_for_log('Handed peer, entering loop')
-
- l1.fundwallet(10**6 + 1000000)
- with pytest.raises(RpcError):
- l1.rpc.fundchannel(l2.info['id'], 10**6)
-
- l2.daemon.wait_for_log('to_self_delay 2017 larger than 2016')
-
-
@unittest.skipIf(TEST_NETWORK != 'regtest', "Fee computation and limits are network specific")
@pytest.mark.slow_test
@pytest.mark.openchannel('v1')
diff --git a/tests/test_opening.py b/tests/test_opening.py
index 65fa44b5..e533d1f8 100644
--- a/tests/test_opening.py
+++ b/tests/test_opening.py
@@ -2799,7 +2799,7 @@ def test_zeroconf_forget(node_factory, bitcoind, dopay: bool):
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd gives different numbers')
-@pytest.mark.xfail(strict=True)
+@pytest.mark.openchannel('v1')
def test_opening_below_min_capacity_sat(bitcoind, node_factory):
"""OK, here's what happens:
Why this scored 25/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.