openingd: fail open_channel at receipt when both initial balances <= their reserve - #9480
Conversation
…their reserve BOLT ElementsProject#2 requires the receiving node to fail the channel if both to_local and to_remote of the initial commitment transaction are <= the opener's channel_reserve_satoshis (a receiving-node MUST under open_channel receipt handling). CLN implements the comparison, but in initial_commit_tx() (common/initial_commit_tx.c, whose FIXME says it should be in ElementsProject#2), so it only fires at funding_created receipt -- after accept_channel has already gone out. Project the initial balances at open_channel receipt (funder to_local = funding - push - base fee - 2x330 anchor outputs; accepter to_remote = push) and fail the negotiation before accept_channel is sent, using the same fee math as initial_commit_tx() (commit_tx_base_fee + the 660-sat anchor correction). The misplaced check stays as the authoritative backstop at funding_created. An in-suite test would need a raw-wire opener: a stock fundchannel reserve is pre-checked with the reserve doubled ('Not opening because if they used the same setting as us ... below 10000sat'), which blocks every shape that trips this check. Validated with a BOLT8 wire peer driving the reporter's exact parameters (100k funding, 20k push, 87k reserve: pre-fix accept_channel, post-fix rejection citing the projected balances 78778000msat / 20000000msat). Changelog-Fixes: ElementsProject#9475 Fixes: ElementsProject#9475
Andezion
left a comment
Andezion
left a comment
There was a problem hiding this comment.
What do you think, do we need a test for this change? contrib/pyln-proto/pyln/proto/wire.py already implements a BOLT8 wire client and is already used for raw-wire tests in tests/test_connection.py (test_connect_basic), so a from-scratch ~~300-line wire peer isntt really necessary
| *funder_pay = AMOUNT_MSAT(0); | ||
|
|
||
| return amount_msat_greater_sat(*funder_pay, their_reserve) | ||
| || amount_msat_greater_sat(*accepter_pay, their_reserve); |
There was a problem hiding this comment.
On the two return true paths (anchor-fee overflow, funding -> msat overflow), *funder_pay/*accepter_pay are left uninitialized. Its not currently exploitable - the caller only reads them inside the if (!initial_balances_exceed_reserve(. . .)) branch, which is never taken when the function returns true - but its a silent invariant a future refactor could break
| * the two 330-sat anchor outputs come off the funder); the accepter's | ||
| * to_remote is push. Returns false and fills the (saturating) balances | ||
| * if NEITHER exceeds their channel_reserve_satoshis. */ | ||
| static bool initial_balances_exceed_reserve(struct amount_sat funding_sats, |
There was a problem hiding this comment.
in common/initial_commit_tx.h we have try_subtract_fee(REMOTE, REMOTE, base_fee, &funder_pay, &accepter_pay) (fundee is always LOCAL, peer/funder always REMOTE here). Maybe we can use it instead?
2 participants
BOLT #2 requires the receiving node to fail the channel if both
to_localandto_remoteof the initial commitment transaction are<= the opener's
channel_reserve_satoshis— a receiving-node MUSTunder
open_channelreceipt handling, not a generalchannel-establishment rule. CLN implements the comparison, but in
initial_commit_tx()(common/initial_commit_tx.c, whose FIXME saysit should be in #2), so it only fires at
funding_createdreceipt —after
accept_channelhas already gone out (#9475).This projects the initial balances at
open_channelreceipt (funderto_local= funding − push − base fee − 2×330 anchor outputs; accepterto_remote= push) and fails the negotiation beforeaccept_channelis sent, using the same fee math as
initial_commit_tx()(
commit_tx_base_fee+ the 660-sat anchor correction).Notes for reviewers:
initial_commit_tx()check stays: it remains theauthoritative backstop at
funding_created, and covers the funderside's own construction.
fundchannelreserve is pre-checked with the reserve doubled ("Not opening
because if they used the same setting as us channel capacity with
funding 100000sat, reserves 44000sat/44000sat, … channel capacity is
9935sat, which is below 10000sat" on the reporter's 100k funding;
43000sat passes), which blocks every shape that trips this check. I
validated with a ~300-line BOLT8 wire peer (noise-XK initiator,
spec-vector-pinned) driving the reporter's exact parameters instead:
pre-fix
accept_channel, post-fix rejection citing the projectedbalances (78778000msat / 20000000msat); boundary shifts
86,100→77,092 at feerate 2000 and the static_remotekey (724-weight)
path lands at 78,551/78,552, both exactly on the projection. The complete wire-peer source, per-cell results, and boundary math are in this gist: https://gist.github.com/Amperstrand/c2078688f93800cf7723cad2670c8044
balance below reserve) still lock in spendable-zero channels — that
is spec-legal (only the MAY "considers
channel_reserve_satoshistoo large" would catch it); a follow-up could discuss a cap like
lnd's 20%-of-capacity bound (lnwallet/reservation.go:
channelCapacity / 5) or eclair'smax-reserve-to-funding-ratio = 0.05.reporter's exact params, pre- vs post-fix wire (v26.06.7) — expand
boundary shift at feerate 2000 — anchors and static_remotekey paths both land on the BOLT3 projection — expand
Fixes #9475