Skip to content

Commit

Permalink
connectd: allow out-of-bounds fees unless they're actually getting *w…
Browse files Browse the repository at this point in the history
…orse*.

Pointed out by @fiatjaf, and indeed it happened to me as well; a peer with
a high feerate reconnects and sends a similar (but now ludicrous) feerate,
and we get upset:

```
$ lightning-cli listpeers 039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36
{
   "peers": [
      {
         "id": "039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36",
         "connected": false,
         "channels": [
            {
               "state": "CHANNELD_NORMAL",
               "scratch_txid": "d796aa9c44920cc7169cdb61e36437bf180cedaec44103a69591ce2baac9b1d9",
               "last_tx_fee": "14329000msat",
               "last_tx_fee_msat": "14329000msat",
               "feerate": {
                  "perkw": 19791,
                  "perkb": 79164
               },
```

Then in the logs:
```
2021-07-23T19:34:56.227Z DEBUG   039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36-channeld-chan#39381: billboard perm: update_fee 17055 outside range 253-7210
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Jul 24, 2021
1 parent f48857b commit 9fea050
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
25 changes: 22 additions & 3 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,21 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg)
channel_add_err_name(add_err));
}

/* We don't get upset if they're outside the range, as long as they're
* improving (or at least, not getting worse!). */
static bool feerate_same_or_better(const struct channel *channel,
u32 feerate, u32 feerate_min, u32 feerate_max)
{
u32 current = channel_feerate(channel, LOCAL);

/* Too low? But is it going upwards? */
if (feerate < feerate_min)
return feerate >= current;
if (feerate > feerate_max)
return feerate <= current;
return true;
}

static void handle_peer_feechange(struct peer *peer, const u8 *msg)
{
struct channel_id channel_id;
Expand Down Expand Up @@ -820,10 +835,14 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg)
* unreasonably large:
* - SHOULD fail the channel.
*/
if (feerate < peer->feerate_min || feerate > peer->feerate_max)
if (!feerate_same_or_better(peer->channel, feerate,
peer->feerate_min, peer->feerate_max))
peer_failed_warn(peer->pps, &peer->channel_id,
"update_fee %u outside range %u-%u",
feerate, peer->feerate_min, peer->feerate_max);
"update_fee %u outside range %u-%u"
" (currently %u)",
feerate,
peer->feerate_min, peer->feerate_max,
channel_feerate(peer->channel, LOCAL));

/* BOLT #2:
*
Expand Down
23 changes: 23 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -3710,3 +3710,26 @@ def test_htlc_failed_noclose(node_factory):

time.sleep(35)
assert l1.rpc.getpeer(l2.info['id'])['connected']


@pytest.mark.developer("dev-no-reconnect required")
def test_old_feerate(node_factory):
"""Test retransmission of old, now-unacceptable, feerate"""
l1, l2 = node_factory.line_graph(2, opts={'feerates': (75000, 75000, 75000, 75000),
'may_reconnect': True,
'dev-no-reconnect': None})

l1.pay(l2, 1000)
l1.rpc.disconnect(l2.info['id'], force=True)

# Drop acceptable feerate by l2
l2.set_feerates((7000, 7000, 7000, 7000))
l2.restart()

# Minor change to l1, so it sends update_fee
l1.set_feerates((74900, 74900, 74900, 74900))
l1.restart()
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)

# This will timeout if l2 didn't accept fee.
l1.pay(l2, 1000)

0 comments on commit 9fea050

Please sign in to comment.