`listfunds` does not take in-flight HTLCs into account

[cdecker]

Issue and Steps to Reproduce

1. Initiate a payment via pay
2. Asynchronously to the pay call listpeerchannels and listfunds (in this order)
3. Notice a disagreement.

The listfunds our_amount_msat field may be more than the listpeerchannels spendable_msat field, as the latter takes the in-flight HTLC into account, while the former does not:

listpeerchannels:

{
   'channels': [
      {
         'peer_id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518',
         'peer_connected': True,
         'channel_type': {'bits': [12], 'names': ['static_remotekey/even']},
         'state': 'CHANNELD_NORMAL',
         'scratch_txid': 'a021ea7fcb80b95b21662aa3981efba18a15bff2a524cc64324b8af2c4755489',
         'last_tx_fee_msat': 7964000msat,
         'feerate': {'perkw': 11000, 'perkb': 44000},
         'owner': 'channeld',
         'short_channel_id': '103x1x0',
         'direction': 0,
         'channel_id': '72fdc31491364795fd686c624785ce1003dd29092b17f35dfffdd583325e7061',
         'funding_txid': '61705e3283d5fdff5df3172b0929dd0310ce8547626c68fd9547369114c3fd72',
         'funding_outnum': 0,
         'close_to_addr': 'bcrt1p2au6qc8jqr2qaru8rfqwhytpfnzmlfks5hu99czngq98lmlcv9gqy9ctvg',
         'close_to': '51205779a060f200d40e8f871a40eb91614cc5bfa6d0a5f852e053400a7feff86150',
         'private': False,
         'opener': 'remote',
         'alias': {'local': '2528304x15270177x39405', 'remote': '6952102x16556827x19020'},
         'features': ['option_static_remotekey'],
         'funding': {'local_funds_msat': 0msat, 'remote_funds_msat': 1000000000msat, 'pushed_msat': 0msat},
         'to_us_msat': 900000000msat,
         'min_to_us_msat': 0msat,
         'max_to_us_msat': 900000000msat,
         'total_msat': 1000000000msat,
         'fee_base_msat': 1msat,
         'fee_proportional_millionths': 10,
         'dust_limit_msat': 546000msat,
         'max_total_htlc_in_msat': 18446744073709551615msat,
         'their_reserve_msat': 10000000msat,
         'our_reserve_msat': 10000000msat,
         'spendable_msat': 888000000msat,
         'receivable_msat': 66504000msat,
         'minimum_htlc_in_msat': 0msat,
         'minimum_htlc_out_msat': 0msat,
         'maximum_htlc_out_msat': 990000000msat,
         'their_to_self_delay': 5,
         'our_to_self_delay': 5,
         'max_accepted_htlcs': 483,
         'state_changes': [{'timestamp': '2023-07-17T11:39:26.615Z', 'old_state': 'CHANNELD_AWAITING_LOCKIN', 'new_state': 'CHANNELD_NORMAL', 'cause': 'remote', 'message': 'Lockin complete'}],
         'status': ['CHANNELD_NORMAL:Channel ready for use.'],
         'in_payments_offered': 1,
         'in_offered_msat': 900000000msat,
         'in_payments_fulfilled': 1,
         'in_fulfilled_msat': 900000000msat,
         'out_payments_offered': 1,
         'out_offered_msat': 1000000msat,
         'out_payments_fulfilled': 0,
         'out_fulfilled_msat': 0msat,
         'htlcs': [
            {'direction': 'out', 'id': 1, 'amount_msat': 1000000msat, 'expiry': 109, 'payment_hash': '9e1a49330b1bd6d5eb2ea841ebc121878e0d5c2c512d8fe8f99767402a4e0a94', 'state': 'SENT_ADD_HTLC', 'local_trimmed': True},
            {'direction': 'out', 'id': 0, 'amount_msat': 1000000msat, 'expiry': 109, 'payment_hash': '9bf332e4569c67945bb48d44786ffba260e3d2010d35b39c700248c15877bd93', 'state': 'SENT_ADD_COMMIT', 'local_trimmed': True}
         ]
      }
   ]
}

listfunds called after the above listpeerchannels:

{
   'outputs': [],
   'channels': [
      {
         'peer_id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518',
         'connected': True,
         'state': 'CHANNELD_NORMAL',
         'channel_id': '72fdc31491364795fd686c624785ce1003dd29092b17f35dfffdd583325e7061',
         'short_channel_id': '103x1x0',
         'our_amount_msat': 900000000msat,
         'amount_msat': 1000000000msat,
         'funding_txid': '61705e3283d5fdff5df3172b0929dd0310ce8547626c68fd9547369114c3fd72',
         'funding_output': 0
      }
   ]
}

Reproduction test

def test_listfunds_vs_listpeers(node_factory, executor):
    """Reproduce an inconsistency in `listpeers` vs `listfunds`

    `listfunds` appears not to consider HTLCs that are still pending
    while `listpeers` does.
    """
    l2, l1 = node_factory.line_graph(2)

    # Rebalance, because the fundee is the one doing the calls here
    i = l1.rpc.invoice(
        10**8 * 9,
        'rebalance',
        "Rebalance since we check fundee"
    )['bolt11']
    l2.rpc.pay(i)

    import time
    time.sleep(1)

    reserve = 10**7
    
    lf = l1.rpc.listfunds()['channels'][0]['our_amount_msat']
    lp = l1.rpc.listpeerchannels()['channels'][0]['spendable_msat']
        
    # We start out with both agreeing
    assert lf == lp + reserve

    from rich.pretty import pprint
    for i in range(0, 100):
        i = l2.rpc.invoice(10**6, f"test{i}", "test")['bolt11']
        f = executor.submit(l1.rpc.pay, i)

        # Monotonicity: lf and lp monotonically decrese because l1 is
        # sending. The later call of the two needs to be equal or
        # lower than the earlier call, no flip-flopping allowed
        c = l1.rpc.listpeerchannels()
        pprint(c)
        lpn = c['channels'][0]['spendable_msat']

        # According to the report, calling `listfunds` shortly after
        # `listpeers` will not have the updated `our_amount_msat`.
        f = l1.rpc.listfunds()
        pprint(f)
        lfn = f['channels'][0]['our_amount_msat']
        
        assert lpn <= lp
        assert lfn <= lf
        assert lfn <= lp + reserve
        lp = lpn
        lf = lfn
        time.sleep(0.1)

Discussion

The accounting on in-flight HTLCs isn't as clear cut as many would believe. Essentialy an in-flight HTLC may either be counted towards the recipient (assuming it'll succeed) or the sender (assuming it'll fail), and we may be in this state for long periods (slow or stuck HTLCs). While we generally do not guarantee consistency among multiple RPC calls, this case is due to these different interpretations, so we should likely pick one and stick with it. Since listpeerchannels is used for routing decisions it has to take the pessimistic view as the funds in HTLCs are not available for new HTLCs.

• Do we want to align the two methods, and subtract HTLCs from listfunds our_amount_msat?

[overcoin]

Thanks for pointing out this inconsistency. This is very important. I'd suggest avoiding "in-flight HTLCs" and instead use "incoming sats-in-transit" (ISIT) and "outgoing sats-in-transit" (OSIT). Reference is made to the standard accounting procedure of "cash-in-transit" while paper cash is transited from one bank branch to another.