fix: cooperative close signs spent amount instead of high-water mark#196
Merged
fix: cooperative close signs spent amount instead of high-water mark#196
Conversation
The client's close() was signing the cooperative close voucher with channel.cumulativeAmount (the high-water mark of all vouchers issued) instead of the actual spent amount reported by the server. This caused the server to settle on-chain for more than the client consumed. Server changes: - handleClose() validates close voucher against max(channel.spent, onChain.settled) instead of highestVoucherAmount - Store update only overwrites highestVoucher when close amount exceeds the current highest (always sets finalized: true) Client changes: - Track server-reported spent via monotonic max from receipt headers (both normal fetch responses and SSE payment-receipt events) - Reset spent on channel change - close() signs spent amount instead of cumulativeAmount
commit: |
…contamination - Extract updateSpentFromReceipt() helper with channelId guard - Call from toPaymentResponse() so non-SSE fetch() updates spent - Reuse in SSE payment-receipt handler (replaces inline logic)
Amp-Thread-ID: https://ampcode.com/threads/T-019cff27-9be6-728d-a710-7dfe4a550072 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The client's
close()was signing the cooperative close voucher withchannel.cumulativeAmount(the high-water mark of all vouchers issued) instead of the actual spent amount reported by the server. This caused the server to settle on-chain for more than the client consumed.For example, if a client pre-authorized 3,000,000 via a voucher but the server only charged 500,000, the cooperative close would settle 3,000,000 on-chain—overcharging the client by 2,500,000.
Fix
Server (
Session.ts)handleClose()now validates the close voucher againstmax(channel.spent, onChain.settled)instead ofhighestVoucherAmount. This allows closing at the actual spent amount rather than requiring the high-water mark.highestVoucher/highestVoucherAmountwhen the close voucher exceeds the current highest. Always setsfinalized: true.Client (
SessionManager.ts)spentvia monotonic max from receipt headers (both normal fetch responses and SSEpayment-receiptevents)spenton channel change to prevent cross-channel contaminationclose()signsspentinstead ofcumulativeAmountTests (
Session.test.ts)highestVoucherAmountDeploy notes
Server-side changes are backwards compatible—existing clients closing at
cumulativeAmountstill pass themax(spent, settled)check sincecumulativeAmount >= spent. Client changes require the server fix to be deployed first (closing atspentwould be rejected by the oldhighestVoucherAmountcheck).