ElementsProject / ElementsProject/lightning
`accept_channel` sets `channel_reserve_satoshis` below our own `dust_limit_satoshis`
- Dominant language
- C
- Stars
- 3.1k
- Forks
- 1k
- Avg merge
- 4d 10h
- Merged PRs (30d)
- 13
Description
As fundee, `openingd` floors the 1% channel reserve at the *peer's* `dust_limit_satoshis` and never at its own. A peer that opens a channel smaller than 54,600 sat while choosing a dust limit below 546 sat therefore gets an `accept_channel` whose `dust_limit_satoshis` (546, hardcoded) is larger than its own `channel_reserve_satoshis`. This violates BOLT 2 requirements ([1](https://github.com/lightning/bolts/blob/152897261850d93c4f4597f39cf22d7d22d6ede6/02-peer-protocol.md?plain=1#L966), [2](https://github.com/lightning/bolts/blob/152897261850d93c4f4597f39cf22d7d22d6ede6/02-peer-protocol.md?plain=1#L869)):
1. ```
Other `accept_channel` fields have the same requirements as their counterparts in `open_channel`.
```
2. ```
The receiving node MUST fail the channel if:
- `dust_limit_satoshis` is greater than `channel_reserve_satoshis`.
```
This issue is similar to #9439, except that one is caused by not enforcing `open_channel.dust_limit_satoshis <= accept_channel.channel_reserve`, and this issue is caused by not enforcing `accept_channel.dust_limit_satoshis <= accept_channel.channel_reserve`.
### Impact
This can lead to a situation where the commitment transaction has no outputs, causing an [assertion failure](https://github.com/ElementsProject/lightning/blob/c09c45207061fb361b099f10e67053ec5bafa15f/channeld/commit_tx.c#L392-L398) in channeld:
```
lightning_channeld: channeld/commit_tx.c:398: commit_tx: Assertion `n > 0' failed.
```
### Reproduction
This test shows the spec violation:
```python
@pytest.mark.openchannel('v1')
def test_accept_channel_reserve_below_our_dust_limit(node_factory, bitcoind):
"""Our accept_channel must not set channel_reserve below our dust limit.
"""
l1 = node_factory.get_node()
chain_hash = bytes.fromhex(bitcoind.rpc.getblockhash(0))[::-1]
feerate = l1.rpc.feerates('perkw')['perkw']['opening']
lconn, channel_type = raw_peer_connect(l1)
# 1% of 20000 is 200sat, floored to our 354sat dust limit -- still under
# their hardcoded 546sat. channel_reserve_satoshis=546 keeps us clear of
# the separate issue #9439.
send_open_channel_reserve(lconn, chain_hash, os.urandom(32),
funding_sat=20000, push_msat=0,
dust_limit=354, channel_reserve=546,
feerate_per_kw=feerate, channel_type=channel_type)
mtype, payload = read_channel_reply_payload(lconn)
assert mtype == WIRE_ACCEPT_CHANNEL
dust_limit = struct.unpack('>Q', payload[32:40])[0]
reserve = struct.unpack('>Q', payload[48:56])[0]
assert dust_limit <= reserve, \
"accept_channel dust_limit_satoshis {} exceeds its channel_reserve_satoshis {}".format(
dust_limit, reserve)
```
A second test, this one showing the assertion failure. It's a bit contrived since by default a CLN opener can't use a dust limit below 546, so instead we use `--dev-allowdustreserve` on the acceptor to show the eventual assertion failure. Note that a non-CLN opener can easily trigger the assert by setting their dust limit below 546 (like in the previous test).
```python
def test_dusty_channel_reserve_causes_outputless_commitment(node_factory, bitcoind):
l1, l2 = node_factory.get_nodes(2, opts=[{}, {'dev-allowdustreserve': True}])
# 1% of this is 354sat, below l2's own 546sat dust limit.
funding = 35400
l1.fundwallet(10**6)
l1.connect(l2)
l1.rpc.fundchannel(l2.info['id'], funding, push_msat=0)
bitcoind.generate_block(6, wait_for_mempool=1)
wait_for(lambda: l1.channel_state(l2) == 'CHANNELD_NORMAL')
wait_for(lambda: l2.channel_state(l1) == 'CHANNELD_NORMAL')
# channeld will not send the very first update_fee until a commitment has
# been exchanged, so move a token amount. 1sat keeps l2 far below its
# 546sat dust limit, so its output stays trimmed.
l1.rpc.pay(l2.rpc.invoice(1000, 'tiny', 'tiny')['bolt11'])
wait_for(lambda: l2.rpc.listpeerchannels(l1.info['id'])['channels'][0]['to_us_msat'] == Millisatoshi(1000))
# This increase in feerate leaves the commitment transaction with no outputs.
l1.set_feerates((50000, 50000, 50000, 50000))
l2.set_feerates((50000, 50000, 50000, 50000))
# Both channelds must survive committing the fee update.
# The channeld assertion failure triggers here.
wait_for(lambda: l1.rpc.listpeerchannels(l2.info['id'])['channels'][0]['feerate']['perkw'] > 10000)
assert l1.channel_state(l2) == 'CHANNELD_NORMAL'
assert l2.channel_state(l1) == 'CHANNELD_NORMAL'
```
### Discovery
Found while fuzzing the v1 funding protocol with [smite](https://github.com/lnfuzz/smite).
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.