`accept_channel` sets `channel_reserve_satoshis` below our own `dust_limit_satoshis`

[morehouse]

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, 2):

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 in channeld:

lightning_channeld: channeld/commit_tx.c:398: commit_tx: Assertion `n > 0' failed.

Reproduction

This test shows the spec violation:

@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).

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.