Conversation
commit: |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bd9e8ae3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| // Settlement failures must not consume a content charge. | ||
| settlementTxHash = | ||
| (await parameters.settleCharged(projected).catch((cause) => { |
There was a problem hiding this comment.
When concurrent HTTP requests for the same channel both preview a due settlement, they can both execute settleCharged before either charge is committed. The request that loses the subsequent counter comparison loops and invokes this side effect again; with a { units: 1 } schedule, two requests can therefore submit three settlement transactions and fire the settlement callback three times, while a nonce collision may instead reject one request with 402. Reserve or serialize the settlement attempt per channel so a failed charge CAS does not repeat an already-started on-chain operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61b4002fb5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const refreshed = await store.updateChannel(channel.channelId, (current) => | ||
| current | ||
| ? { | ||
| ...current, | ||
| settledOnChain: ChannelStore.keepGreater(current.settledOnChain, state.settled), | ||
| } |
There was a problem hiding this comment.
When a broadcast response is lost but the transaction confirms, the next attempt updates only settledOnChain and returns at the subsequent isSettlementDue check without recording lastSettlementAt, lastSettlementSpent, or lastSettlementUnits. Once a later voucher creates unsettled value, unit and interval schedules therefore count from channel creation, while amount schedules fall back to subtracting the cumulative on-chain settlement from HTTP spend; this can trigger the next settlement immediately or calculate negative progress. Record the current spend/unit/time boundary whenever this readback discovers that the abandoned attempt advanced the on-chain settled amount.
Useful? React with 👍 / 👎.
| const reserved = await store.updateChannel(channel.channelId, (current) => { | ||
| if (!current || (current.settlementAttempt?.validBefore ?? 0) > now) return current | ||
| return { ...current, settlementAttempt: { id, validBefore } } |
There was a problem hiding this comment.
When concurrent requests find another unexpired settlementAttempt, this callback returns current, but ChannelStore.fromStore().updateChannel interprets every non-null return as a set, so each waiter rewrites the full channel record and notifies listeners every 100 ms until the settlement finishes. A burst of requests during a slow settlement can therefore generate thousands of unnecessary database writes and repeatedly wake SSE waiters. Acquire the reservation through a true no-op-capable atomic operation, or wait for an update/expiry without calling updateChannel on every poll.
Useful? React with 👍 / 👎.
1 participant
Commit HTTP session charges after scheduled settlement succeeds, preserving atomic balance checks during concurrent requests. Failed settlement returns
402without consuming the request charge. Update the close-failure fixture for expiring nonces.