diff --git a/.changeset/fix-close-overcharge.md b/.changeset/fix-close-overcharge.md new file mode 100644 index 00000000..fd156d2b --- /dev/null +++ b/.changeset/fix-close-overcharge.md @@ -0,0 +1,5 @@ +--- +"mppx": patch +--- + +Fixed cooperative close to sign the server-reported spent amount instead of the high-water mark (`cumulativeAmount`), preventing overcharging when actual usage was below the pre-authorized voucher amount. diff --git a/src/tempo/client/SessionManager.ts b/src/tempo/client/SessionManager.ts index 18cdc962..c0e33427 100644 --- a/src/tempo/client/SessionManager.ts +++ b/src/tempo/client/SessionManager.ts @@ -59,6 +59,7 @@ export function sessionManager(parameters: sessionManager.Parameters): SessionMa let channel: ChannelEntry | null = null let lastChallenge: Challenge.Challenge | null = null let lastUrl: RequestInfo | URL | null = null + let spent = 0n const method = sessionPlugin({ account: parameters.account, @@ -68,6 +69,7 @@ export function sessionManager(parameters: sessionManager.Parameters): SessionMa decimals: parameters.decimals, maxDeposit: parameters.maxDeposit, onChannelUpdate(entry) { + if (entry.channelId !== channel?.channelId) spent = 0n channel = entry }, }) @@ -81,9 +83,16 @@ export function sessionManager(parameters: sessionManager.Parameters): SessionMa }, }) + function updateSpentFromReceipt(receipt: SessionReceipt | null | undefined) { + if (!receipt || receipt.channelId !== channel?.channelId) return + const next = BigInt(receipt.spent) + spent = spent > next ? spent : next + } + function toPaymentResponse(response: Response): PaymentResponse { const receiptHeader = response.headers.get('Payment-Receipt') const receipt = receiptHeader ? deserializeSessionReceipt(receiptHeader) : null + updateSpentFromReceipt(receipt) return Object.assign(response, { receipt, challenge: lastChallenge, @@ -216,6 +225,7 @@ export function sessionManager(parameters: sessionManager.Parameters): SessionMa } case 'payment-receipt': + updateSpentFromReceipt(event.data) onReceipt?.(event.data) break } @@ -237,7 +247,7 @@ export function sessionManager(parameters: sessionManager.Parameters): SessionMa context: { action: 'close', channelId: channel.channelId, - cumulativeAmountRaw: channel.cumulativeAmount.toString(), + cumulativeAmountRaw: spent.toString(), }, }) diff --git a/src/tempo/server/Session.test.ts b/src/tempo/server/Session.test.ts index 7f5b686b..bea8e2be 100644 --- a/src/tempo/server/Session.test.ts +++ b/src/tempo/server/Session.test.ts @@ -630,7 +630,7 @@ describe('session', () => { expect(ch!.highestVoucherAmount).toBe(5000000n) }) - test('rejects close with voucher below highest', async () => { + test('accepts close at spent amount (below highestVoucherAmount)', async () => { const { channelId, serializedTransaction } = await createSignedOpenTransaction(10000000n) const server = createServer() await openServerChannel(server, channelId, serializedTransaction) @@ -648,20 +648,50 @@ describe('session', () => { request: makeRequest(), }) + await charge(store, channelId, 500000n) + + const receipt = await server.verify({ + credential: { + challenge: makeChallenge({ id: 'challenge-3', channelId }), + payload: { + action: 'close' as const, + channelId, + cumulativeAmount: '500000', + signature: await signTestVoucher(channelId, 500000n), + }, + }, + request: makeRequest(), + }) + + expect(receipt.status).toBe('success') + + const ch = await store.getChannel(channelId) + expect(ch).not.toBeNull() + expect(ch!.highestVoucherAmount).toBe(3000000n) + expect(ch!.finalized).toBe(true) + }) + + test('rejects close below spent amount', async () => { + const { channelId, serializedTransaction } = await createSignedOpenTransaction(10000000n) + const server = createServer() + await openServerChannel(server, channelId, serializedTransaction) + + await charge(store, channelId, 500000n) + await expect( server.verify({ credential: { - challenge: makeChallenge({ id: 'challenge-3', channelId }), + challenge: makeChallenge({ id: 'challenge-2', channelId }), payload: { action: 'close' as const, channelId, - cumulativeAmount: '2000000', - signature: await signTestVoucher(channelId, 2000000n), + cumulativeAmount: '100000', + signature: await signTestVoucher(channelId, 100000n), }, }, request: makeRequest(), }), - ).rejects.toThrow('close voucher amount must be >= highest accepted voucher') + ).rejects.toThrow('close voucher amount must be >=') }) test('rejects close exceeding on-chain deposit', async () => { diff --git a/src/tempo/server/Session.ts b/src/tempo/server/Session.ts index e8e8cb86..a7110b32 100644 --- a/src/tempo/server/Session.ts +++ b/src/tempo/server/Session.ts @@ -774,21 +774,16 @@ async function handleClose( payload.signature, ) - if (voucher.cumulativeAmount < channel.highestVoucherAmount) { - throw new VerificationFailedError({ - reason: 'close voucher amount must be >= highest accepted voucher', - }) - } - const onChain = await getOnChainChannel(client, methodDetails.escrowContract, payload.channelId) if (onChain.finalized) { throw new ChannelClosedError({ reason: 'channel is finalized on-chain' }) } - if (voucher.cumulativeAmount < onChain.settled) { + const minCloseAmount = channel.spent > onChain.settled ? channel.spent : onChain.settled + if (voucher.cumulativeAmount < minCloseAmount) { throw new VerificationFailedError({ - reason: 'close voucher cumulativeAmount is below on-chain settled amount', + reason: `close voucher amount must be >= ${minCloseAmount} (max of spent and on-chain settled)`, }) } @@ -819,11 +814,14 @@ async function handleClose( const updated = await store.updateChannel(payload.channelId, (current) => { if (!current) return null + const updateVoucher = voucher.cumulativeAmount > current.highestVoucherAmount return { ...current, deposit: onChain.deposit, - highestVoucherAmount: voucher.cumulativeAmount, - highestVoucher: voucher, + ...(updateVoucher && { + highestVoucherAmount: voucher.cumulativeAmount, + highestVoucher: voucher, + }), finalized: true, } })