diff --git a/channeld/channeld.c b/channeld/channeld.c index 8f1333f37..ac8509dbd 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1381,10 +1381,14 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) } /* We were supposed to check this was affordable as we go. */ - if (peer->channel->funder == REMOTE) + if (peer->channel->funder == REMOTE) { + status_trace("Feerates are %u/%u", + peer->channel->view[LOCAL].feerate_per_kw, + peer->channel->view[REMOTE].feerate_per_kw); assert(can_funder_afford_feerate(peer->channel, peer->channel->view[LOCAL] .feerate_per_kw)); + } if (!fromwire_commitment_signed(tmpctx, msg, &channel_id, &commit_sig.s, &htlc_sigs)) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 464ff1640..7dce4521c 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -291,6 +292,69 @@ struct bitcoin_tx **channel_txs(const tal_t *ctx, return txs; } +/* If @side is faced with these HTLCs, how much will it have left + * above reserve (eg. to pay fees). Returns false if would be < 0. */ +static bool get_room_above_reserve(const struct channel *channel, + const struct channel_view *view, + const struct htlc **adding, + const struct htlc **removing, + enum side side, + struct amount_msat *balance) +{ + bool ok; + /* Reserve is set by the *other* side */ + struct amount_sat reserve = channel->config[!side].channel_reserve; + + *balance = view->owed[side]; + + ok = true; + for (size_t i = 0; i < tal_count(removing); i++) + ok &= balance_remove_htlc(balance, removing[i], side); + + for (size_t i = 0; i < tal_count(adding); i++) + ok &= balance_add_htlc(balance, adding[i], side); + + /* Overflow shouldn't happen, but if it does, complain */ + if (!ok) { + status_broken("Failed to add %zu remove %zu htlcs", + tal_count(adding), tal_count(removing)); + return false; + } + + if (!amount_msat_sub_sat(balance, *balance, reserve)) { + status_trace("%s cannot afford htlc: would make balance %s" + " below reserve %s", + side_to_str(side), + type_to_string(tmpctx, struct amount_msat, + balance), + type_to_string(tmpctx, struct amount_sat, + &reserve)); + return false; + } + return true; +} + +static struct amount_sat fee_for_htlcs(const struct channel *channel, + const struct channel_view *view, + const struct htlc **committed, + const struct htlc **adding, + const struct htlc **removing, + enum side side) +{ + u32 feerate = view->feerate_per_kw; + struct amount_sat dust_limit = channel->config[side].dust_limit; + size_t untrimmed; + + untrimmed = commit_tx_num_untrimmed(committed, feerate, dust_limit, + side) + + commit_tx_num_untrimmed(adding, feerate, dust_limit, + side) + - commit_tx_num_untrimmed(removing, feerate, dust_limit, + side); + + return commit_tx_base_fee(feerate, untrimmed); +} + static enum channel_add_err add_htlc(struct channel *channel, enum htlc_state state, u64 id, @@ -304,12 +368,9 @@ static enum channel_add_err add_htlc(struct channel *channel, { struct htlc *htlc, *old; struct amount_msat msat_in_htlcs, committed_msat, adding_msat, removing_msat; - struct amount_sat fee; enum side sender = htlc_state_owner(state), recipient = !sender; const struct htlc **committed, **adding, **removing; const struct channel_view *view; - bool ok; - size_t i; htlc = tal(tmpctx, struct htlc); @@ -432,71 +493,86 @@ static enum channel_add_err add_htlc(struct channel *channel, * reserve): * - SHOULD fail the channel. */ - if (channel->funder == htlc_owner(htlc)) { - u32 feerate = view->feerate_per_kw; - struct amount_sat dust_limit = channel->config[recipient].dust_limit; - size_t untrimmed; - - untrimmed = commit_tx_num_untrimmed(committed, feerate, dust_limit, - recipient) - + commit_tx_num_untrimmed(adding, feerate, dust_limit, - recipient) - - commit_tx_num_untrimmed(removing, feerate, dust_limit, - recipient); - - fee = commit_tx_base_fee(feerate, untrimmed); - } else - fee = AMOUNT_SAT(0); - - assert((s64)fee.satoshis >= 0); /* Raw: explicit signedness test */ - if (htlc_fee) *htlc_fee = fee; /* set fee output pointer if given */ - if (enforce_aggregate_limits) { - /* Figure out what balance sender would have after applying all - * pending changes. */ - struct amount_msat balance = view->owed[sender]; + struct amount_msat balance; + struct amount_sat fee = fee_for_htlcs(channel, view, + committed, + adding, + removing, + recipient); + /* set fee output pointer if given */ + if (htlc_fee) + *htlc_fee = fee; + /* This is a little subtle: * * The change is being applied to the receiver but it will * come back to the sender after revoke_and_ack. So the check * here is that the balance to the sender doesn't go below the * sender's reserve. */ - const struct amount_sat reserve - = channel->config[!sender].channel_reserve; - - assert(amount_msat_greater_eq(balance, AMOUNT_MSAT(0))); - ok = true; - for (i = 0; i < tal_count(removing); i++) - ok &= balance_remove_htlc(&balance, removing[i], sender); - assert(amount_msat_greater_eq(balance, AMOUNT_MSAT(0))); - for (i = 0; i < tal_count(adding); i++) - ok &= balance_add_htlc(&balance, adding[i], sender); - - /* Overflow shouldn't happen, but if it does, complain */ - if (!ok) { - status_broken("Failed to add %zu remove %zu htlcs", - tal_count(adding), tal_count(removing)); + if (!get_room_above_reserve(channel, view, + adding, removing, sender, + &balance)) return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; - } - if (!amount_msat_sub_sat(&balance, balance, fee)) { - status_trace("Cannot afford fee %s with balance %s", - type_to_string(tmpctx, struct amount_sat, - &fee), - type_to_string(tmpctx, struct amount_msat, - &balance)); - return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + if (channel->funder == sender) { + if (amount_msat_less_sat(balance, fee)) { + status_trace("Cannot afford fee %s with %s above reserve", + type_to_string(tmpctx, struct amount_sat, + &fee), + type_to_string(tmpctx, struct amount_msat, + &balance)); + return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + } } - if (!amount_msat_greater_eq_sat(balance, reserve)) { - status_trace("Cannot afford fee %s: would make balance %s" - " below reserve %s", - type_to_string(tmpctx, struct amount_sat, - &fee), - type_to_string(tmpctx, struct amount_msat, - &balance), - type_to_string(tmpctx, struct amount_sat, - &reserve)); - return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + + /* Try not to add a payment which will take funder into fees + * on either our side or theirs. */ + if (sender == LOCAL) { + if (!get_room_above_reserve(channel, view, + adding, removing, + channel->funder, + &balance)) + return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + /* Should be able to afford both their own commit tx + * fee, and other's commit tx fee, which are subtly + * different! */ + fee = fee_for_htlcs(channel, view, + committed, + adding, + removing, + channel->funder); + /* set fee output pointer if given */ + if (htlc_fee && amount_sat_greater(fee, *htlc_fee)) + *htlc_fee = fee; + if (amount_msat_less_sat(balance, fee)) { + status_trace("Funder could not afford own fee %s with %s above reserve", + type_to_string(tmpctx, + struct amount_sat, + &fee), + type_to_string(tmpctx, + struct amount_msat, + &balance)); + return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + } + fee = fee_for_htlcs(channel, view, + committed, + adding, + removing, + !channel->funder); + /* set fee output pointer if given */ + if (htlc_fee && amount_sat_greater(fee, *htlc_fee)) + *htlc_fee = fee; + if (amount_msat_less_sat(balance, fee)) { + status_trace("Funder could not afford peer's fee %s with %s above reserve", + type_to_string(tmpctx, + struct amount_sat, + &fee), + type_to_string(tmpctx, + struct amount_msat, + &balance)); + return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + } } } @@ -832,6 +908,13 @@ bool can_funder_afford_feerate(const struct channel *channel, u32 feerate_per_kw type_to_string(tmpctx, struct amount_sat, &channel->config[!channel->funder].channel_reserve)); + status_trace("We need %s at feerate %u for %zu untrimmed htlcs: we have %s/%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->funder]), + type_to_string(tmpctx, struct amount_msat, + &channel->view[REMOTE].owed[channel->funder])); return amount_msat_greater_eq_sat(channel->view[!channel->funder].owed[channel->funder], needed); }