feat: add trusted bearer-key faucet API - #2
Open
infraclaw-dash wants to merge 1 commit into
Open
infraclaw-dash wants to merge 1 commit into
infraclaw-dash wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughChangesTrusted faucet access is added through bearer API keys configured as SHA-256 hashes, with per-key daily limits, shared payout handling, documentation, deployment configuration, and automated tests. Trusted Faucet API
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TrustedClient
participant api_core_faucet
participant api_key_rate_limiter
participant dispense_core_dash
TrustedClient->>api_core_faucet: POST /v1/core-faucet with bearer key and address
api_core_faucet->>api_key_rate_limiter: Check key daily allowance
api_key_rate_limiter-->>api_core_faucet: Allow or reject with retry period
api_core_faucet->>dispense_core_dash: Validate address and send DASH
dispense_core_dash-->>api_core_faucet: CoreFaucetResponse
api_core_faucet->>api_key_rate_limiter: Record successful key request
api_core_faucet-->>TrustedClient: CoreFaucetResponse
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.env.example (1)
27-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKey ordering flagged by dotenv-linter.
FAUCET_API_KEY_DAILY_LIMITshould be placed beforeFAUCET_API_KEY_HASHESalphabetically.♻️ Suggested reorder
-FAUCET_API_KEY_HASHES= -FAUCET_API_KEY_DAILY_LIMIT=100 +FAUCET_API_KEY_DAILY_LIMIT=100 +FAUCET_API_KEY_HASHES=🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example around lines 27 - 28, Reorder the FAUCET_API_KEY configuration entries so FAUCET_API_KEY_DAILY_LIMIT appears before FAUCET_API_KEY_HASHES, without changing either value.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/routers/faucet.py`:
- Around line 692-704: Replace the separate api_key_rate_limiter.is_allowed and
record_request calls around dispense_core_dash with an atomic try_reserve
operation that checks and records the key within one lock acquisition, rejecting
with the existing 429 response when reservation fails. If the payout RPC fails
or does not complete successfully, release the reservation through the
corresponding RateLimiter release operation so failed requests do not consume
the daily allowance.
---
Nitpick comments:
In @.env.example:
- Around line 27-28: Reorder the FAUCET_API_KEY configuration entries so
FAUCET_API_KEY_DAILY_LIMIT appears before FAUCET_API_KEY_HASHES, without
changing either value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e52ce42-dd23-456c-bb85-5b503bbe53ec
📒 Files selected for processing (8)
.env.exampleREADME.mdapp/config.pyapp/routers/faucet.pyapp/services/api_keys.pydocker-compose.ymltests/test_api_keys.pytests/test_trusted_faucet.py
Comment on lines
+692
to
+704
| allowed, retry_after = api_key_rate_limiter.is_allowed(key_id) | ||
| if not allowed: | ||
| raise HTTPException( | ||
| status_code=429, | ||
| detail={ | ||
| "error": "API key daily limit exceeded", | ||
| "retryAfter": retry_after, | ||
| }, | ||
| headers={"Retry-After": str(retry_after)}, | ||
| ) | ||
|
|
||
| result = dispense_core_dash(body.address, settings.core_faucet_amount) | ||
| api_key_rate_limiter.record_request(key_id) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Daily-limit check-then-act race across the payout RPC call.
api_key_rate_limiter.is_allowed(key_id) and record_request(key_id) are separate locked operations with dispense_core_dash's network call to Dash Core RPC executed in between. Concurrent requests using the same API key can all pass the check before any of them records, letting a key exceed the intended daily safety cap.
🔒 Suggested direction: make check+reserve atomic
- allowed, retry_after = api_key_rate_limiter.is_allowed(key_id)
- if not allowed:
- raise HTTPException(
- status_code=429,
- detail={
- "error": "API key daily limit exceeded",
- "retryAfter": retry_after,
- },
- headers={"Retry-After": str(retry_after)},
- )
-
- result = dispense_core_dash(body.address, settings.core_faucet_amount)
- api_key_rate_limiter.record_request(key_id)
+ allowed, retry_after = api_key_rate_limiter.try_reserve(key_id)
+ if not allowed:
+ raise HTTPException(
+ status_code=429,
+ detail={
+ "error": "API key daily limit exceeded",
+ "retryAfter": retry_after,
+ },
+ headers={"Retry-After": str(retry_after)},
+ )
+
+ try:
+ result = dispense_core_dash(body.address, settings.core_faucet_amount)
+ except Exception:
+ api_key_rate_limiter.release(key_id)
+ raise(try_reserve/release would need to be added to RateLimiter, performing the check-and-append under one lock acquisition.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/routers/faucet.py` around lines 692 - 704, Replace the separate
api_key_rate_limiter.is_allowed and record_request calls around
dispense_core_dash with an atomic try_reserve operation that checks and records
the key within one lock acquisition, rejecting with the existing 429 response
when reservation fails. If the payout RPC fails or does not complete
successfully, release the reservation through the corresponding RateLimiter
release operation so failed requests do not consume the daily allowance.
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
1 participant
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.
Summary
POST /api/v1/core-faucetSecurity
/api/core-faucetbehavior is unchangedValidation
python -m unittest discover -s tests -v(7 tests pass)/api/v1/core-faucetSummary by CodeRabbit
Retry-Afterresponses when limits are reached.