Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

connectd: allow out-of-bounds fees unless they're actually getting *worse* #4681

Merged
merged 1 commit into from
Jul 25, 2021
Merged

connectd: allow out-of-bounds fees unless they're actually getting *worse* #4681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions channeld/channeld.c
View file
Open in desktop
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
View file
Open in desktop
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)