Repository navigation
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
Open
feat: faucet address-cap race, retention, logging, tests and entity tidy-ups (follow-ups to #829)#830fraseragain wants to merge 8 commits into
fraseragain wants to merge 8 commits into
Conversation
… 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
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
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.
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
FAUCET_ADDRESS_CAP_CHEQ. The cap check now also runs under the per-customer lock, subtracting requests made for the same address in the lastFAUCET_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.faucetRequestrows older thanFAUCET_REQUEST_RETENTION_MONTHS(default 13 calendar months,0keeps 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.eventname 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.request:faucet:testnetscope 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.amountNcheqis abigintin the entity (via a column transformer), andcreatedAtis set by the database (DEFAULT now()) so all instances agree on when a request was made. New migration1791500000002; the ledger test checks the schema still matches the entity.requestFaucetTokensandgetBalancesagainst 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 unlessFAUCET_TEST_DATABASE_URLis 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 intests/integration, outside jest'stests/unitpattern.How to test
npm run test:unit(new pure tests: retention cutoff, address room, logging events, auth rule).Risks and rollout
1791500000002is additive (a column default) and safe in any deploy order.FAUCET_BALANCE_SETTLE_SECONDScan over-refuse for a few seconds near the cap; set it to0to turn the in-flight accounting off.Architecture and best-practice review fixes
createdAtis 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 429Retry-Afterand balances response use the same window, andcompletedAtis 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, andcompletedAtwas 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