channeld: simplify pending_updates() using HTLC_F_WILL_SEND
What the developer wrote
Authored by Rusty Russell
channeld: simplify pending_updates() using HTLC_F_WILL_SEND
And remove `uncommitted_ok` flag which was always false.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit simplifies how Core Lightning decides whether a channel still has unfinished updates before entering a quiet 'STFU' mode. It removes an 'uncommitted_ok' option that was never actually used as true, and replaces a hand-rolled check with a clearer flag named HTLC_F_WILL_SEND. The change also adds temporary debug logging that prints each HTLC's state when STFU is considered. There is no direct evidence in the commit that this fixes a security bug, but any change to state-machine logic around pending updates can affect protocol correctness.
Review the equivalence of the new HTLC_F_WILL_SEND check against the old HTLC_ADDING/HTLC_REMOVING + owner logic to ensure no pending-update cases are missed before STFU. Confirm no caller relied on uncommitted_ok=true. Treat as routine cleanup unless protocol-fuzzing or tests reveal a state mismatch.
Security signals we found
State-machine logic change for pending HTLC updates
STFU (quiescence) protocol precondition check modified
Removal of unused uncommitted_ok parameter changes semantics if any caller had passed true
Debug logging added to maybe_send_stfu
Evidence from the diff
The patch refactors pending_updates() in channeld/full_channel.c. Previously it took a bool uncommitted_ok and used HTLC_ADDING/HTLC_REMOVING plus HTLC_F_PENDING to decide whether a side still has updates in flight. The commit removes uncommitted_ok (noting it was always false) and instead checks HTLC_FLAG(side, HTLC_F_WILL_SEND). Callers in channeld.c are updated to pass only two arguments. A temporary debug loop printing every HTLC state is added inside maybe_send_stfu(). The header comment is updated to clarify that pending updates block STFU. The diff is a simplification/cleanup with no explicit security claim.
Changed components
channeld/channeld.cchanneld/full_channel.cchanneld/full_channel.hInspect captured patch +22 / −32
diff --git a/channeld/channeld.c b/channeld/channeld.c
index bef107c6..9a8b95c0 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -231,10 +231,20 @@ static void end_stfu_mode(struct peer *peer)
static bool maybe_send_stfu(struct peer *peer)
{
+ struct htlc_map_iter it;
+ const struct htlc *htlc;
+
if (!peer->want_stfu)
return false;
- if (pending_updates(peer->channel, LOCAL, false)) {
+ for (htlc = htlc_map_first(peer->channel->htlcs, &it);
+ htlc;
+ htlc = htlc_map_next(peer->channel->htlcs, &it)) {
+ status_info("maybe_send_stfu: htlc %"PRIu64" state %s",
+ htlc->id, htlc_state_name(htlc->state));
+ }
+
+ if (pending_updates(peer->channel, LOCAL)) {
status_info("Pending updates prevent us from STFU mode at this"
" time.");
return false;
@@ -297,7 +307,7 @@ static void handle_stfu(struct peer *peer, const u8 *stfu)
}
/* Sanity check */
- if (pending_updates(peer->channel, REMOTE, false))
+ if (pending_updates(peer->channel, REMOTE))
peer_failed_warn(peer->pps, &peer->channel_id,
"STFU but you still have updates pending?");
diff --git a/channeld/full_channel.c b/channeld/full_channel.c
index c3df6ee6..c06663d6 100644
--- a/channeld/full_channel.c
+++ b/channeld/full_channel.c
@@ -1544,47 +1544,27 @@ static bool adjust_balance(struct balance view_owed[NUM_SIDES][NUM_SIDES],
return true;
}
-bool pending_updates(const struct channel *channel,
- enum side side,
- bool uncommitted_ok)
+bool pending_updates(const struct channel *channel, enum side side)
{
struct htlc_map_iter it;
const struct htlc *htlc;
/* Initiator might have fee changes or blockheight updates in play. */
if (side == channel->opener) {
- if (!feerate_changes_done(channel->fee_states, uncommitted_ok))
+ if (!feerate_changes_done(channel->fee_states, false))
return true;
- if (!blockheight_changes_done(channel->blockheight_states,
- uncommitted_ok))
+ if (!blockheight_changes_done(channel->blockheight_states, false))
return true;
}
+ /* Are we still waiting for the side to send something? */
for (htlc = htlc_map_first(channel->htlcs, &it);
htlc;
htlc = htlc_map_next(channel->htlcs, &it)) {
- int flags = htlc_state_flags(htlc->state);
-
- /* If it's still being added, its owner added it. */
- if (flags & HTLC_ADDING) {
- /* It might be OK if it's added, but not committed */
- if (uncommitted_ok
- && (flags & HTLC_FLAG(!side, HTLC_F_PENDING)))
- continue;
- if (htlc_owner(htlc) == side)
- return true;
- /* If it's being removed, non-owner removed it */
- } else if (htlc_state_flags(htlc->state) & HTLC_REMOVING) {
- /* It might be OK if it's removed, but not committed */
- if (uncommitted_ok
- && (flags & HTLC_FLAG(!side, HTLC_F_PENDING)))
- continue;
- if (htlc_owner(htlc) != side)
- return true;
- }
+ if (htlc_state_flags(htlc->state) & HTLC_FLAG(side, HTLC_F_WILL_SEND))
+ return true;
}
-
return false;
}
diff --git a/channeld/full_channel.h b/channeld/full_channel.h
index 33e54627..d1cb84c3 100644
--- a/channeld/full_channel.h
+++ b/channeld/full_channel.h
@@ -294,13 +294,13 @@ bool channel_force_htlcs(struct channel *channel,
void dump_htlcs(const struct channel *channel, const char *prefix);
/**
- * pending_updates: does this side have updates pending in channel?
+ * pending_updates: does this side have to send more messages to process channel?
* @channel: the channel
* @side: the side who is offering or failing/fulfilling HTLC, or feechange
- * @uncommitted_ok: don't count uncommitted changes.
+ *
+ * If this is true, you can't send STFU.
*/
-bool pending_updates(const struct channel *channel, enum side side,
- bool uncommitted_ok);
+bool pending_updates(const struct channel *channel, enum side side);
const char *channel_add_err_name(enum channel_add_err e);
const char *channel_remove_err_name(enum channel_remove_err e);
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.