Skip to content

feat: faucet address-cap race, retention, logging, tests and entity tidy-ups (follow-ups to #829) - #830

Open
fraseragain wants to merge 8 commits into
feat/faucet-cost-and-reservationsfrom
feat/faucet-hardening-low
Open

fraseragain wants to merge 8 commits into
feat/faucet-cost-and-reservationsfrom
feat/faucet-hardening-low

Conversation

@fraseragain

@fraseragain fraseragain commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #829, which is stacked on #819. The base is feat/faucet-cost-and-reservations; retarget as the earlier PRs merge.

Closes #823, closes #824, closes #825, closes #826, closes #827, closes #828.

What changed

  • Serialise the faucet address-cap check so concurrent requests cannot exceed it #823 Address cap under the lock. The on-chain balance is read before the reservation and does not include transfers still on their way, so simultaneous requests could each be allowed the full room under FAUCET_ADDRESS_CAP_CHEQ. The cap check now also runs under the per-customer lock, subtracting requests made for the same address in the last FAUCET_BALANCE_SETTLE_SECONDS (default 20) from the room, so only as many requests as fit are accepted. The bootstrap top-up is not subject to it. Trade-off: for those 20 seconds a request that already reached the balance is counted twice, which can briefly refuse a legitimate follow-up only when the address is near its cap.
  • Add retention for the faucetRequest table #824 Retention. faucetRequest rows older than FAUCET_REQUEST_RETENTION_MONTHS (default 13 calendar months, 0 keeps everything) are deleted. Studio has no scheduler, so the cleanup runs in the background from normal reservations, at most once an hour per process, and never delays or fails the request that triggered it. A manual query is in the README.
  • Log and track faucet quota reservations, releases and rejections #825 Logging. Every reservation, completion, release, unknown outcome and rejection (quota, minimum interval, address cap, plus the early checks before Stripe), the sweep of abandoned reservations and the retention cleanup write one JSON log line with an event name and the customer, address, amount and relevant figures. The README lists the events and a support query. The event tracker is deliberately not used: its operations feed usage and billing reports.
  • Decide, test and document which auth methods may use the faucet endpoint #826 Auth methods. Session, API key and machine-to-machine credentials all remain allowed (as decided). A unit test pins that they all end in the same request:faucet:testnet scope check and that the route is not open to unauthenticated callers; the Swagger description and README state the supported credentials, including that an M2M application needs the scope granted in LogTo. No behaviour change.
  • FaucetRequestEntity tidy-ups: bigint transformer and database-side createdAt #827 Entity tidy-ups. amountNcheq is a bigint in the entity (via a column transformer), and createdAt is set by the database (DEFAULT now()) so all instances agree on when a request was made. New migration 1791500000002; the ledger test checks the schema still matches the entity.
  • Add handler-level faucet tests that run in CI #828 Handler tests. 21 handler-level tests call the real requestFaucetTokens and getBalances against a real Postgres with Stripe, the faucet, the account lookup and the on-chain balance faked. They cover session, API-key and M2M requests, the quota, minimum interval, address cap, failure handling, reservation status and the balances response. As agreed, CI is unchanged: they skip unless FAUCET_TEST_DATABASE_URL is set. They use Node's test runner (a dependency of the controller does not import under jest) and a small loader hook for the balance fake, and live in tests/integration, outside jest's tests/unit pattern.

How to test

  1. npm run test:unit (new pure tests: retention cutoff, address room, logging events, auth rule).
  2. With a disposable Postgres, the ledger and handler suites; the README has the exact commands. Ledger suite (jest): reserve/release, interval, statuses, address cap including simultaneous requests, retention, entity round trip and the schema-drift check. Handler suite (Node 20): the 21 end-to-end scenarios.
  3. I ran both against Postgres 16 on Node 20: 212 of 212 pass, including the 21 handler tests; local Node 26 runs 186 and skips the 26 database-backed ones. I also confirmed end to end that the retention cleanup is triggered by a normal request, deletes only rows past retention and runs at most once an hour, and that four simultaneous requests for an address with room for one are now refused three times.

Risks and rollout

  • Migration 1791500000002 is additive (a column default) and safe in any deploy order.
  • FAUCET_BALANCE_SETTLE_SECONDS can over-refuse for a few seconds near the cap; set it to 0 to turn the in-flight accounting off.
  • The first request after a deploy per process may delete old rows in the background; with the default 13 months and a table that only just appeared, nothing is deleted for over a year.

Architecture and best-practice review fixes

  • Merges the fixes from feat: allow API keys on the faucet endpoint with a monthly CHEQ quota #819 and feat: faucet cost, burst limiting and reservation status (follow-ups to #819) #829.
  • One clock for every decision. createdAt is set by the database, but month windows, the minimum interval, the pending timeout, the settle window and retention were compared against the application clock, so instances with different clocks could disagree about how old a request is. They now all use the database clock (clock_timestamp(), read after the lock is taken), the 429 Retry-After and balances response use the same window, and completedAt is set by the database. Tests fake the application clock hours ahead; I confirmed they fail against the previous behaviour (a fresh pending reservation was treated as expired, the month was wrong, and completedAt was 3 hours off).

Merged state verified on Node 20 against Postgres 16: 234 of 234 tests pass, including the 21 handler tests.

🤖 Generated with Claude Code

dubiouspip and others added 8 commits October 9, 2026 14:47
… createdAt

The entity now exposes amountNcheq as a bigint (via a column transformer) instead of a string, which removes the constructor guard, and createdAt is set by the database (DEFAULT now()) so all instances agree on when a request was made. Adds a migration for the column default; the ledger test checks the schema still matches the entity. Refs #827

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The on-chain balance is read before the reservation and does not include transfers still on their way, so simultaneous requests could each be allowed the full room under FAUCET_ADDRESS_CAP_CHEQ. The cap check now also runs under the per-customer lock, subtracting requests made for the same address in the last FAUCET_BALANCE_SETTLE_SECONDS (default 20) from the room, so only as many requests as fit are accepted. The bootstrap top-up is not subject to it. Refs #823

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
faucetRequest rows older than FAUCET_REQUEST_RETENTION_MONTHS (default 13 calendar months, 0 keeps everything) are deleted. Studio has no scheduler, so the cleanup runs in the background from normal reservations, at most once an hour per process, and never delays or fails the request that triggered it. The README documents a manual query as well. Refs #824

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Every reservation, completion, release, unknown outcome and rejection (quota, minimum interval, address cap, plus the early checks before Stripe), the sweep of abandoned reservations and the retention cleanup now write one JSON log line with an event name and the customer, address, amount and relevant figures, so a blocked or refunded request can be traced. The README lists the events and a support query. The event tracker is not used because its operations feed usage and billing reports. Refs #825

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ee auth methods

Session, API key and machine-to-machine credentials all end in the same scope check, so a unit test pins that the route requires request:faucet:testnet and is not open to unauthenticated callers. The Swagger description and README now state the supported credentials and that an M2M application needs the scope granted in LogTo. No behaviour change: all three methods remain allowed. Refs #826

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
21 tests call the real requestFaucetTokens and getBalances against a real Postgres with Stripe, the faucet, the account lookup and the on-chain balance faked. They cover session, API-key and machine-to-machine requests, the quota, minimum interval, address cap, failure handling, reservation status and the balances response. They use Node's test runner (the controller's dependencies do not load under jest) and a small loader hook for the balance fake, live outside tests/unit, and skip unless FAUCET_TEST_DATABASE_URL is set, so CI is unchanged. The README says how to run them. Refs #828

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
createdAt is set by the database (DEFAULT now()), but month windows, the minimum interval, the pending timeout, the address-cap settle window and retention were compared against the application clock, so instances with different clocks could disagree about how old a request is. They now all use the database clock: reserve reads clock_timestamp() after taking the lock, and the read paths read it once per call. getQuotaStatus returns the usage together with the window, so the 429 Retry-After and the balances response use the same clock, and completedAt is set by the database. Tests fake the application clock hours ahead and fail against the previous behaviour. Refs #827

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants