-
Notifications
You must be signed in to change notification settings - Fork 10
bug: server_spent field lacks monotonic-max protection in channel upsert #413
Description
Summary
The server_spent column in save_channel is unconditionally overwritten on upsert, unlike deposit, cumulative_amount, and accepted_cumulative which use a LENGTH-based monotonic maximum comparison. This means a stale or out-of-order update can regress server_spent to a lower value.
Location
crates/tempo-common/src/payment/session/store/storage.rs, line ~298
Details
The upsert SQL for save_channel_in_conn applies monotonic-max logic to three amount fields:
deposit = CASE
WHEN LENGTH(channels.deposit) > LENGTH(excluded.deposit) THEN channels.deposit
...
END,
cumulative_amount = CASE ... END,
accepted_cumulative = CASE ... END,But server_spent is simply overwritten:
server_spent = excluded.server_spentSince server_spent is used as the cooperative close target (per the v0.2.0 changelog: "Track server-reported Payment-Receipt.spent for session state and use it as the close target"), a regression here means the close amount could be lower than what the server actually reported as spent.
Impact
If two concurrent requests produce receipts with different spent values and the stale one is persisted last, server_spent regresses. On cooperative close, the wallet would settle at the regressed (lower) amount instead of the true server-reported spend.
Suggested fix
Apply the same LENGTH-based monotonic-max comparison used for the other amount fields:
server_spent = CASE
WHEN LENGTH(channels.server_spent) > LENGTH(excluded.server_spent) THEN channels.server_spent
WHEN LENGTH(channels.server_spent) < LENGTH(excluded.server_spent) THEN excluded.server_spent
WHEN channels.server_spent >= excluded.server_spent THEN channels.server_spent
ELSE excluded.server_spent
END