Skip to content

Commit

Permalink
channeld: fix update_fee cap.
Browse files Browse the repository at this point in the history
We would sometimes propose fees which we couldn't afford, and thus the
peer would get upset: if we didn't recover it could cause force closes.

This was because we didn't take into account that pending htlcs will
remove our total available funds.

Changelog-Fixes: Protocol: Don't upset peers by sending `update_fee` with fees we cannot afford in the case where HTLCs are large.
  • Loading branch information
rustyrussell committed Feb 13, 2024
1 parent 1350194 commit 3011e02
Showing 1 changed file with 44 additions and 12 deletions.
56 changes: 44 additions & 12 deletions channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -1227,7 +1227,8 @@ u32 approx_max_feerate(const struct channel *channel)
{
size_t num;
u64 weight;
struct amount_sat avail;
struct amount_msat avail;
struct balance balance;
const struct htlc **committed, **adding, **removing;
bool option_anchor_outputs = channel_has(channel, OPT_ANCHOR_OUTPUTS);
bool option_anchors_zero_fee_htlc_tx = channel_has(channel, OPT_ANCHORS_ZERO_FEE_HTLC_TX);
Expand All @@ -1241,7 +1242,18 @@ u32 approx_max_feerate(const struct channel *channel)
weight = commit_tx_base_weight(num, option_anchor_outputs, option_anchors_zero_fee_htlc_tx);

/* Available is their view */
avail = amount_msat_to_sat_round_down(channel->view[!channel->opener].owed[channel->opener]);
to_balance(&balance, channel->view[!channel->opener].owed[channel->opener]);

/* But take into account any pending added htlcs (conservatively, we ignore removed ones) */
for (size_t i = 0; i < tal_count(adding); i++)
balance_add_htlc(&balance, adding[i], channel->opener);

if (!balance_ok(&balance, &avail)) {
status_broken("Negative balance %"PRId64" after removing %zu htlcs!",
balance.msat,
tal_count(removing));
return 0;
}

/* BOLT #3:
* If `option_anchors` applies to the commitment
Expand All @@ -1250,16 +1262,16 @@ u32 approx_max_feerate(const struct channel *channel)
* `to_remote`).
*/
if ((option_anchor_outputs || option_anchors_zero_fee_htlc_tx)
&& !amount_sat_sub(&avail, avail, AMOUNT_SAT(660))) {
avail = AMOUNT_SAT(0);
&& !amount_msat_sub_sat(&avail, avail, AMOUNT_SAT(660))) {
avail = AMOUNT_MSAT(0);
} else {
/* We should never go below reserve. */
if (!amount_sat_sub(&avail, avail,
channel->config[!channel->opener].channel_reserve))
avail = AMOUNT_SAT(0);
if (!amount_msat_sub_sat(&avail, avail,
channel->config[!channel->opener].channel_reserve))
avail = AMOUNT_MSAT(0);
}

return avail.satoshis / weight * 1000; /* Raw: once-off reverse feerate*/
return avail.millisatoshis / weight; /* Raw: once-off reverse feerate*/
}

/* Is the sum of trimmed htlcs, as this new feerate, above our
Expand Down Expand Up @@ -1301,10 +1313,15 @@ bool can_opener_afford_feerate(const struct channel *channel, u32 feerate_per_kw
const struct htlc **committed, **adding, **removing;
bool option_anchor_outputs = channel_has(channel, OPT_ANCHOR_OUTPUTS);
bool option_anchors_zero_fee_htlc_tx = channel_has(channel, OPT_ANCHORS_ZERO_FEE_HTLC_TX);
struct balance balance;
struct amount_msat available;

gather_htlcs(tmpctx, channel, !channel->opener,
&committed, &removing, &adding);

status_debug("Committed %zu, removing %zu, adding %zu",
tal_count(committed), tal_count(removing), tal_count(adding));

untrimmed = commit_tx_num_untrimmed(committed, feerate_per_kw, dust_limit,
option_anchor_outputs,
option_anchors_zero_fee_htlc_tx,
Expand Down Expand Up @@ -1354,15 +1371,30 @@ bool can_opener_afford_feerate(const struct channel *channel, u32 feerate_per_kw
type_to_string(tmpctx, struct amount_sat,
&channel->config[!channel->opener].channel_reserve));

status_debug("We need %s at feerate %u for %zu untrimmed htlcs: we have %s/%s",
/* The amount we have needs to take into account that we're
* adding and removing htlcs */
to_balance(&balance, channel->view[!channel->opener].owed[channel->opener]);
for (size_t i = 0; i < tal_count(removing); i++)
balance_remove_htlc(&balance, removing[i], channel->opener);
for (size_t i = 0; i < tal_count(adding); i++)
balance_add_htlc(&balance, adding[i], channel->opener);

if (!balance_ok(&balance, &available)) {
status_broken("Negative balance %"PRId64" after removing %zu htlcs!",
balance.msat,
tal_count(removing));
return false;
}

status_debug("We need %s at feerate %u for %zu untrimmed htlcs: we have %s/%s (will have %s)",
type_to_string(tmpctx, struct amount_sat, &needed),
feerate_per_kw, untrimmed,
type_to_string(tmpctx, struct amount_msat,
&channel->view[LOCAL].owed[channel->opener]),
type_to_string(tmpctx, struct amount_msat,
&channel->view[REMOTE].owed[channel->opener]));
return amount_msat_greater_eq_sat(channel->view[!channel->opener].owed[channel->opener],
needed);
&channel->view[REMOTE].owed[channel->opener]),
type_to_string(tmpctx, struct amount_msat, &available));
return amount_msat_greater_eq_sat(available, needed);
}

bool channel_update_feerate(struct channel *channel, u32 feerate_per_kw)
Expand Down

0 comments on commit 3011e02

Please sign in to comment.