-
Notifications
You must be signed in to change notification settings - Fork 3
Fix initial token claim for new wallets #328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughRecipient ID resolution during native token drip now uses a best-effort lookup: the controller calls rpc.lookup_id(recipient).await and, on failure, continues with the original recipient address instead of returning an error. No other logic was changed. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as Faucet Controller
participant RPC as RPC Service
participant Drip as Native Token Drip
Client->>Controller: request_drip(recipient)
Controller->>RPC: lookup_id(recipient)
alt ID found
RPC-->>Controller: Ok(Some(id))
Controller->>Drip: drip(to = id)
else ID not found or error
RPC-->>Controller: Err / Ok(None)
Controller->>Drip: drip(to = recipient) <!-- fallback -->
end
Drip-->>Controller: result
Controller-->>Client: response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
=======================================
Coverage 39.37% 39.37%
=======================================
Files 40 40
Lines 2537 2537
=======================================
Hits 999 999
Misses 1538 1538 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Preview URL: https://2852c06.forest-explorer-preview.workers.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/faucet/controller.rs (1)
224-224
: Verify that swallowing all lookup errors is acceptable.The fallback to
recipient
on lookup failure correctly handles new wallets without ID addresses. However, this also silently swallows legitimate RPC/network errors without distinguishing them from the "no ID yet" case.Ensure this trade-off is acceptable for your use case. If transient RPC failures are common, users might experience silent failures without clear error messages.
Consider adding logging to improve observability:
- let id_address = rpc.lookup_id(recipient).await.unwrap_or(recipient); + let id_address = rpc.lookup_id(recipient).await.unwrap_or_else(|e| { + log::debug!("ID lookup failed for {}, using original address: {}", recipient, e); + recipient + });
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
adheres to the team's
documentation standards,
(if possible),
should be reflected in this document.
Summary by CodeRabbit