Fix faucet rate-limit timestamp parsing

[hungud]

Fixes #4887

Summary

• Treat SQLite naive drip timestamps as UTC before rate-limit calculations.
• Add concurrent regression coverage for the legacy faucet atomic drip recorder.
• Verify only one same wallet/IP drip can be recorded under concurrent attempts.

Validation

• ./.venv/Scripts/python.exe -m pytest -q tests/test_legacy_faucet_json_validation.py tests/test_faucet.py
• ./.venv/Scripts/python.exe -m py_compile faucet.py tests/test_legacy_faucet_json_validation.py
• git diff --check

Reviewed before submission.

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Non-doc PRs have a BCOS-L1 or BCOS-L2 label
• Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
• New code files include an SPDX license header
• You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

[jaxint]

Great work on this PR! The code looks clean and well-structured.

Review powered by RustChain Bounty Program

Wallet: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[shadow88sky]

Reviewed the faucet timestamp parsing and concurrent drip regression.

Concrete observations:

1. The parse_drip_timestamp() helper fixes the actual naive/aware datetime mismatch without changing storage format. SQLite CURRENT_TIMESTAMP rows come back without tzinfo, while the rate-limit calculations compare against datetime.now(timezone.utc). Treating naive stored values as UTC keeps existing database rows compatible and avoids the offset-naive vs offset-aware subtraction failure in can_drip(), get_next_available(), and the atomic try_record_drip() checks.

2. Reusing the helper in all four timestamp consumers is the right shape: the non-atomic read helpers and both wallet/IP branches inside try_record_drip() now interpret stored faucet timestamps consistently. That reduces the chance that one path reports availability while the atomic writer rejects, or vice versa.

3. The new concurrent regression test is meaningful because it exercises the legacy SQLite default timestamp path through try_record_drip() rather than only unit-testing parse_drip_timestamp() directly. With eight same-wallet/IP attempts, it proves the first insert succeeds and the later attempts see the stored naive timestamp as a valid UTC time and return rate-limit failures instead of crashing or allowing duplicate drips.

Small non-blocking suggestion: a direct parser unit test for a Z timestamp and a naive SQLite timestamp would make the intended compatibility contract even clearer, but the current integration-style regression covers the production failure mode.

Local validation:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m pytest tests/test_legacy_faucet_json_validation.py tests/test_faucet.py -q -> 25 passed.

[minyanyi]

Reviewed the faucet timestamp/rate-limit fix and the regression coverage.

• Verified parse_drip_timestamp() preserves aware ISO timestamps and treats SQLite CURRENT_TIMESTAMP values without tzinfo as UTC, which keeps comparisons against datetime.now(timezone.utc) type-safe.
• Checked the existing can_drip(), get_next_available(), and try_record_drip() paths now use the shared parser instead of repeating direct fromisoformat() parsing.
• Confirmed the new concurrent regression exercises the BEGIN IMMEDIATE rate-limit path: one drip succeeds, the remaining attempts fail on IP or wallet rate limiting, and both follow-up can_drip() checks return false.
• Local validation passed: python -B -m pytest -q tests/test_legacy_faucet_json_validation.py --tb=short -> 7 passed, 1 warning; python -B -m py_compile faucet.py tests/test_legacy_faucet_json_validation.py passed; git diff --check origin/main...HEAD passed.

I received RTC compensation for this review.

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[jaxint]

Code review completed. Thanks for contributing to RustChain! 🚀

[jaxint]

Code review completed. Thanks for contributing! 🚀

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[nguyenthekr92-byte]

Code Review: PR #6341

Nice clean fix. The timestamp-naive handling was definitely a real bug waiting to happen. A few observations:

Good

• Extracting parse_drip_timestamp() is the right DRY improvement — the same replace('Z', '+00:00') pattern was repeated 4 times
• Keeping the SQL logic unchanged in get_last_drip_time() is wise — one bug at a time
• The timezone-aware comparison with datetime.now(timezone.utc) is correct

Suggestions

1. Type hint: Consider adding -> datetime to parse_drip_timestamp so callers know what they get back
2. None safety: If get_last_drip_time ever returns None, can_drip() returns True (early return on line 79). If try_record_drip gets a None from the query, row[0] would be None and .replace() would throw AttributeError. Might be worth adding an explicit None guard in try_record_drip for safety

Overall: clean, focused change. Good to merge after the minor guard.

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[jaxint]

Great work! Thanks for contributing to RustChain! 🦀

[jhdev2026]

PR Review — #6341 Fix Faucet Rate-Limit Timestamp Parsing

Reviewed: node/faucet*.py — fixing rate-limit timestamp parsing.

What this PR does

Fixes how the faucet module parses timestamps used for rate-limiting requests.

Technical observations

Why timestamp parsing matters for rate-limiting:
If the rate-limit timestamp is parsed incorrectly, the faucet might:

• Allow more frequent requests than intended (timestamps parsed as in the past)
• Block legitimate users (timestamps parsed as in the future)

Common timestamp parsing bugs:

• Timezone mismatches (UTC vs local)
• Off-by-one errors in duration calculations
• Integer vs float division for time intervals

Conclusion: Rate-limit bugs lead to faucet abuse. Fixing timestamp parsing is important for fair token distribution.

I received RTC compensation for this review.
Wallet: RTCc33595f561eae619a07ca8d4a9c66e87763ac726

[jhdev2026]

PR Review — #6341 Fix Faucet Rate-Limit Timestamp Parsing

Reviewed: node/faucet*.py — fixing rate-limit timestamp parsing.

What this PR does

Fixes how the faucet module parses timestamps used for rate-limiting requests.

Technical observations

Why timestamp parsing matters:
If the rate-limit timestamp is parsed incorrectly, the faucet might allow more frequent requests than intended or block legitimate users.

Common timestamp bugs:

• Timezone mismatches (UTC vs local)
• Off-by-one errors in duration calculations
• Integer vs float division for time intervals

Conclusion: Rate-limit bugs lead to faucet abuse. Fixing timestamp parsing is important for fair token distribution.

I received RTC compensation for this review.
Wallet: RTCc33595f561eae619a07ca8d4a9c66e87763ac726

[jaxint]

Thanks for this contribution! 🎉