Skip to content

fix(faucet): label legacy faucet.py as demo, default mock_mode off, audit config at startup - #8258

Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/faucet-demo-labeling-8243
Open

fix(faucet): label legacy faucet.py as demo, default mock_mode off, audit config at startup#8258
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/faucet-demo-labeling-8243

Conversation

@Vyacheslav-Tomashevskiy

@Vyacheslav-Tomashevskiy Vyacheslav-Tomashevskiy commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #8243 (fallout from #8240).

Root faucet.py contains no node call, yet answered every drip with a success payload and the UI copy 'Sent X RTC'. faucet_service defaulted mock_mode to True, so even the supported faucet paid nobody unless explicitly configured, and a real-mode faucet without RC_ADMIN_KEY only surfaced that as a 500 on the first user request.

  • faucet.py: stamped # DEMO - records requests, does not pay in the module docstring, in a DEMO_MODE constant, in the startup banner and on the page. /faucet/drip now returns sent: false, demo: true, tx_hash: null and a notice pointing at faucet_service/. ok/amount are kept for backward compatibility -- ok means "recorded", not "sent".
  • FAUCET.md: rewritten around faucet_service/faucet_service.py as the only supported faucet, with a comparison table and the startup-audit rules.
  • faucet_service: mock_mode default flipped to False in DEFAULT_CONFIG and in every .get('mock_mode', ...) call site, so a partial config no longer falls back to paying nobody.
  • faucet_service: new check_distribution_config() runs in main() before the port is bound, and (report-only) in create_app() for WSGI/gunicorn deployments that never reach main(). mock_mode on -> loud warning banner (still starts -- it's a legitimate dev choice). mock_mode off with neither RC_ADMIN_KEY nor distribution.admin_key -> FaucetConfigError instead of a 500 on the first drip.
  • faucet_config.yaml / faucet_service/README.md: ship mock_mode: false plus the node_url / faucet_wallet / admin_key keys the transfer path actually reads.
  • Audit of the deployed config: testnet/deploy_testnet.sh already writes mock_mode: false and RC_ADMIN_KEY; a test now pins that so it stays true.

Out of scope for this PR: live-auditing the actually-deployed faucet_config.yaml on the production host -- that's infra, not something a repo PR can touch (already checked what generates it, see above).

Tests: test_faucet_demo_labeling_8243.py, 19 cases, all pass. Existing faucet suites (faucet_service/test_faucet_service.py, test_faucet_wallet_validation_6136.py, tests/test_faucet*.py, tests/test_legacy_faucet_json_validation.py) stay green: 130 passed, 6 subtests passed, 0 failed.

Wallet: RTCd1554f0f35576faf01d386a6be1c947f560dd0b7

@github-actions github-actions Bot added documentation Improvements or additions to documentation BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/M PR: 51-200 lines labels Aug 26, 2026
@Vyacheslav-Tomashevskiy

Copy link
Copy Markdown
Contributor Author

/claim

Wallet: RTCd1554f0f35576faf01d386a6be1c947f560dd0b7

…udit config at startup

Fixes Scottcjn#8243 (fallout from Scottcjn#8240).

Root faucet.py contains no node call, yet answered every drip with a success
payload and the UI copy 'Sent X RTC'. faucet_service defaulted mock_mode to
True, so even the supported faucet paid nobody unless explicitly configured,
and a real-mode faucet without RC_ADMIN_KEY only surfaced that as a 500 on the
first user request.

- faucet.py: stamped '# DEMO - records requests, does not pay' in the module
  docstring, in a DEMO_MODE constant, in the startup banner and on the page.
  /faucet/drip now returns sent: false, demo: true, tx_hash: null and a notice
  pointing at faucet_service/. ok/amount kept for backward compatibility --
  ok means 'recorded', not 'sent'.
- FAUCET.md: rewritten around faucet_service/faucet_service.py as the only
  supported faucet, with a comparison table and the startup-audit rules.
- faucet_service: mock_mode default flipped to False in DEFAULT_CONFIG and in
  every .get('mock_mode', ...) call site, so a partial config no longer falls
  back to paying nobody.
- faucet_service: new check_distribution_config() runs in main() before the
  port is bound, and (report-only) in create_app() for WSGI/gunicorn
  deployments that never reach main(). mock_mode on -> loud warning banner
  (still starts, it is a legitimate dev choice). mock_mode off with neither
  RC_ADMIN_KEY nor distribution.admin_key -> FaucetConfigError instead of a
  500 on the first drip.
- faucet_config.yaml / faucet_service README: ship mock_mode: false plus the
  node_url / faucet_wallet / admin_key keys the transfer path actually reads.
- Audit of the deployed config: testnet/deploy_testnet.sh already writes
  mock_mode: false and RC_ADMIN_KEY; a test now pins that so it stays true.

Tests: test_faucet_demo_labeling_8243.py, 19 cases, all pass. Existing faucet
suites (faucet_service/test_faucet_service.py, test_faucet_wallet_validation_6136.py,
tests/test_faucet*.py, tests/test_legacy_faucet_json_validation.py) stay green:
130 passed, 6 subtests passed, 0 failed.

Wallet: RTCd1554f0f35576faf01d386a6be1c947f560dd0b7

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Vyacheslav-Tomashevskiy
Vyacheslav-Tomashevskiy force-pushed the fix/faucet-demo-labeling-8243 branch from d81165f to 2fca1ec Compare August 27, 2026 00:16
@github-actions github-actions Bot added size/L PR: 201-500 lines and removed size/M PR: 51-200 lines labels Aug 27, 2026
@Vyacheslav-Tomashevskiy Vyacheslav-Tomashevskiy changed the title fix(faucet): label demo faucet.py honestly, fix TESTNET_FAUCET.md pointer, warn loudly on mock_mode fix(faucet): label legacy faucet.py as demo, default mock_mode off, audit config at startup Aug 27, 2026

@Scottcjn Scottcjn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

19/19 tests pass on a main overlay, and the labelling is unambiguous at every layer (docstring, constant, banner, JSON sent:false, page copy). I also checked testnet/deploy_testnet.sh: it already writes mock_mode: false and an admin key into the generated config, so the deployed testnet faucet is unaffected by the default flip.

The one part that is policy rather than code: flipping DEFAULT_CONFIG['distribution']['mock_mode'] to False means a faucet started with no config now attempts real /wallet/transfer calls (or refuses to start without a key). That's the safer failure mode, but it is a behaviour change for anyone running the service from defaults, so I'm leaving the merge to Scott rather than calling it myself.

Note: #8259 is a byte-for-byte duplicate of this branch (same six blob SHAs) plus two no-op sync commits; I'm closing that one in favour of this.

@ravendevhub ravendevhub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Substantive Review: fix(faucet): label legacy faucet.py as demo, default mock_mode off, audit config at startup

1. Architectural & Logic Analysis

  • Scope: Evaluated changes across modified files in PR #8258.
  • Verification: Verified implementation aligns with repository design principles, error handling standards, and state invariants.

2. Test Coverage & Quality

  • Finding: Verified that test cases and assertions appropriately exercise the new or modified logic boundaries.
  • Git Hygiene: Checked diff cleanliness and absence of extraneous dependency or environment leakage.

@Scottcjn

Scottcjn commented Sep 5, 2026

Copy link
Copy Markdown
Owner

See the consolidated hard-look note on #8282: this branch is based on a July fork state and would revert ~612 lines of upstream node fixes (and, for #8320/#8321/#8329, delete the CI workflows). Please rebase onto current upstream main; the actual change here is small and will merge once it is clean. — Sophia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) documentation Improvements or additions to documentation size/L PR: 201-500 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retire or clearly label legacy faucet.py (records drips, sends nothing) + fix FAUCET.md + audit mock_mode

3 participants