Skip to content

Handle low faucet balance in claim token API #327

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Handle low faucet balance in claim token API #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3637620
Add low balance check in claim token api
sudo-shashank Oct 13, 2025
1fac3c2
update api doc
sudo-shashank Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/api-documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,19 @@ curl "https://forest-explorer.chainsafe.dev/api/claim_token?faucet_info=MainnetF
```bash
ServerError|I'm a teapot - mainnet tokens are not available.
```

---

## Faucet Top-Up Requests

If you encounter a server error indicating that faucet is exhausted.

**Example:**

```bash
ServerError|Faucet is empty, Request top-up
```

You can request for faucet top-up
[Here](https://github.com/ChainSafe/forest-explorer/discussions/134). This
discussion thread is monitored for top-up requests.
Comment on lines +146 to +158
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Improve grammar and link text descriptiveness.

Two minor issues in this documentation section:

  • Line 148: "that faucet is exhausted" should be "that the faucet is exhausted"
  • Lines 156-158: The link text "Here" is not descriptive. Consider rewording to something like: "You can request a faucet top-up in this discussion thread."

Apply this diff:

-If you encounter a server error indicating that faucet is exhausted.
+If you encounter a server error indicating that the faucet is exhausted.

-You can request for faucet top-up
-[Here](https://github.com/ChainSafe/forest-explorer/discussions/134). This
-discussion thread is monitored for top-up requests.
+You can [request a faucet top-up](https://github.com/ChainSafe/forest-explorer/discussions/134) 
+in this discussion thread, which is monitored for top-up requests.

Based on learnings

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Faucet Top-Up Requests
If you encounter a server error indicating that faucet is exhausted.
**Example:**
```bash
ServerError|Faucet is empty, Request top-up
```
You can request for faucet top-up
[Here](https://github.com/ChainSafe/forest-explorer/discussions/134). This
discussion thread is monitored for top-up requests.
## Faucet Top-Up Requests
If you encounter a server error indicating that the faucet is exhausted.
**Example:**
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

157-157: Link text should be descriptive

(MD059, descriptive-link-text)

🤖 Prompt for AI Agents 
In docs/api-documentation.md around lines 146 to 158, fix two doc issues: change
"that faucet is exhausted" to "that the faucet is exhausted" on line 148, and
replace the non-descriptive link text "Here" (lines 156-158) with more
descriptive inline text such as "request a faucet top-up" linking to
https://github.com/ChainSafe/forest-explorer/discussions/134 so the sentence
reads like: "You can request a faucet top-up in this discussion thread."

9 changes: 9 additions & 0 deletions src/faucet/server_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,15 @@ pub async fn claim_token(
.map_err(ServerFnError::new)?;

SendWrapper::new(async move {
let faucet_balance = rpc
.wallet_balance(from, &faucet_info.token_type())
.await
.map_err(ServerFnError::new)?;
if faucet_balance < *faucet_info.drip_amount() {
return Err(ServerFnError::ServerError(
"Faucet is empty, Request top-up".to_string(),
));
}
Comment on lines +222 to +230
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Missing gas balance check for ERC-20 faucets and TOCTOU consideration.

Two concerns with the balance pre-check:

  1. Major issue: For ERC-20 faucets, this check verifies the ERC-20 token balance but does not verify that the faucet has sufficient native tokens to pay for gas. A faucet could have adequate ERC-20 tokens but insufficient native tokens for gas, leading to transaction failures after rate-limit tracking.

  2. TOCTOU consideration: The balance is checked before the transfer executes. Between checking and transferring, concurrent requests could deplete the faucet, causing on-chain transaction failures. While the blockchain will serialize transactions and the impact is limited (clearer error messages, minimal gas waste), this creates a race window where users bypass the early check.

Recommended fix for the major issue:

For ERC-20 faucets, add an additional balance check for native tokens needed for gas:

 let faucet_balance = rpc
     .wallet_balance(from, &faucet_info.token_type())
     .await
     .map_err(ServerFnError::new)?;
 if faucet_balance < *faucet_info.drip_amount() {
     return Err(ServerFnError::ServerError(
         "Faucet is empty, Request top-up".to_string(),
     ));
 }
+
+// For ERC-20 faucets, also check native token balance for gas
+if matches!(faucet_info.token_type(), TokenType::Erc20(_)) {
+    let native_balance = rpc
+        .wallet_balance(from, &TokenType::Native)
+        .await
+        .map_err(ServerFnError::new)?;
+    // Rough estimate: gas_limit * max_gas_price
+    let estimated_gas_cost = TokenAmount::from_atto(
+        faucet_info.max_gas_limit() as u128 * 2_000_000_000u128 // 2 Gwei estimate
+    );
+    if native_balance < estimated_gas_cost {
+        return Err(ServerFnError::ServerError(
+            "Faucet has insufficient native tokens for gas fees, Request top-up".to_string(),
+        ));
+    }
+}
+
 match faucet_info {

For the TOCTOU concern:

The fail-fast approach is valuable for UX despite the race window. If you want to minimize on-chain failures and gas waste, consider implementing advisory locking (e.g., using Redis with SETNX or a database row lock) around the balance check and transfer operation. However, given the limited impact and the complexity of distributed locking, the current approach may be acceptable. Please verify whether on-chain transaction failures due to insufficient balance are an acceptable trade-off.

🏁 Script executed:

#!/bin/bash
# Find the definition of wallet_balance
rg -n 'fn wallet_balance' -C3 --type rust

# Find the TokenType enum definition
rg -n 'enum TokenType' -C5 --type rust

# Show lines 200–260 in server_api.rs around the balance check
sed -n '200,260p' src/faucet/server_api.rs

Length of output: 4119

🏁 Script executed:

#!/bin/bash
# Find the ERC-20 claim handler
rg -n 'handle_erc20_claim' -C5 --type rust

# Find the native claim handler
rg -n 'handle_native_claim' -C5 --type rust

# Find the FaucetInfo enum definition
rg -n 'enum FaucetInfo' -C5 --type rust

Length of output: 3174

🏁 Script executed:

#!/bin/bash
# Dump handle_erc20_claim function body
sed -n '312,360p' src/faucet/server_api.rs

# Dump FaucetInfo impl block to find methods
rg -n 'impl FaucetInfo' -C5 --type rust

# Search for drip_amount and token_type methods
rg -n 'fn drip_amount' -C3 --type rust
rg -n 'fn token_type' -C3 --type rust

Length of output: 3440

Check native FIL balance before ERC-20 transfers and mitigate TOCTOU race

  • For CalibnetUSDFC, after verifying the ERC-20 token balance, also call
    wallet_balance(from, &TokenType::Native) and ensure it covers the estimated gas cost.
  • The pre-transfer balance check runs before rate-limit enforcement, allowing concurrent requests to bypass limits and fail on-chain; consider advisory locking or moving rate-limit logic before this check.
🤖 Prompt for AI Agents 
In src/faucet/server_api.rs around lines 222 to 230, after checking the ERC-20
faucet_balance add an additional check that calls rpc.wallet_balance(from,
&TokenType::Native).await and verify the returned native FIL balance is >= the
estimated gas cost for the transfer (compute or fetch the gas estimate used for
the transfer); if not, return a ServerFnError indicating insufficient native
funds to cover gas. To mitigate the TOCTOU/rate-limit race, move the rate-limit
enforcement (or acquire an advisory per-account lock) to run before any balance
checks so concurrent requests are serialized or rejected before on-chain state
is queried.

match faucet_info {
FaucetInfo::MainnetFIL => {
set_response_status(StatusCode::IM_A_TEAPOT);
Expand Down
Loading