Skip to content

fix: TTS rate limiting, retry backoff, contract auth checks, and overflow guards#1006

Open
Grace-CODE-D wants to merge 4 commits into
solutions-plug:mainfrom
Grace-CODE-D:fix/994-995-996-997-security-reliability
Open

fix: TTS rate limiting, retry backoff, contract auth checks, and overflow guards#1006
Grace-CODE-D wants to merge 4 commits into
solutions-plug:mainfrom
Grace-CODE-D:fix/994-995-996-997-security-reliability

Conversation

@Grace-CODE-D

Copy link
Copy Markdown
Contributor

Summary

Resolves four security and reliability issues across the TTS service and the Soroban prediction-market contract.


#994 — Exponential backoff with jitter on TTS provider retry

Files: services/tts/src/TTSService.ts, services/tts/src/server.ts

  • Added RetryConfig interface (maxRetries, maxDelayMs) and extended TTSConfig with an optional retry field.
  • Added withRetry<T>() helper that retries a callback up to maxRetries times on transient errors (HTTP 429, 5xx / mapped 502). Non-retriable errors (400, 401, 403) propagate immediately without consuming retry budget.
  • Backoff delay uses full jitter: delay ∈ [0, min(maxDelayMs, 1000 × 2^attempt)], avoiding thundering-herd against shared provider quota.
  • _generateWithFallback now wraps both the primary and fallback _callProvider calls with withRetry, so each leg gets independent retry budget.
  • server.ts reads TTS_MAX_RETRIES (default 3) and TTS_MAX_DELAY_MS (default 60000) from environment and passes them through config.

#995 — Rate limiting at TTS server level

Files: services/tts/src/server.ts, services/tts/package.json

  • Added express-rate-limit ^7.5.0 to production dependencies.
  • Mounted a rateLimit middleware that applies to all routes (health endpoints exempted via skip).
  • Key generation: authenticated callers are bucketed by their API key (apikey:<key>); anonymous callers are bucketed by IP (ip:<addr>). This gives authenticated users an independent quota and prevents shared-IP over-counting.
  • Limit defaults to TTS_RATE_LIMIT_PER_MINUTE=60 requests per 60-second window; fully configurable via environment variable.
  • Exceeded requests receive HTTP 429 with a Retry-After: 60 response header.

#996 — Arithmetic overflow in payout calculations

Files: contracts/predict-iq/src/modules/bets.rs, contracts/predict-iq/src/modules/fees.rs, contracts/predict-iq/src/modules/bets_fuzz_test.rs

  • bets.rsexisting_bet.fee_paid += fee replaced with checked_add(...).ok_or(ErrorCode::ArithmeticOverflow)?. A bet that would overflow cumulative fee accounting now returns a typed contract error instead of wrapping silently in release builds.
  • fees.rscollect_fee signature changed from fn(...) -> () to fn(...) -> Result<(), ErrorCode>. Both the per-token FeeRevenue accumulator and the global TotalFeesCollected accumulator now use checked_add, returning ArithmeticOverflow on overflow. The single caller in bets.rs propagates the error with ?.
  • bets_fuzz_test.rs – Two new payout-path fuzz properties added:
    • prop_payout_claim_boundary_values_do_not_panic — exercises claim_winnings with boundary bet amounts (1, i128::MAX/4, i128::MAX/2) and asserts no host panic.
    • prop_payout_multiplication_overflow_returns_typed_error — directly asserts that the intermediate multiplication in winnings = (bet.amount × total_staked) / winning_stake produces None from checked_mul for overflow-inducing inputs.

#997 — Authorization checks missing before state mutations in Soroban contract

Files: contracts/predict-iq/src/modules/governance.rs, contracts/predict-iq/src/modules/markets.rs, contracts/predict-iq/README.md

  • governance.rs::vote_on_guardian_removal — was verifying guardian membership by address comparison but never called voter.require_auth(). Any account could impersonate a guardian and cast removal votes. Fixed by adding voter.require_auth() as the first statement before the membership check.
  • markets.rs::release_creation_deposit — had no authorization at all. Any account could trigger the token transfer path (funds go to market.creator regardless, but the lack of auth violates the principle of least privilege). Fixed by adding market.creator.require_auth() after loading the market, so only the creator can initiate their own deposit refund.
  • README.md — Added a full Authorization Model section documenting every mutable contract function, the role required to authorize it (Admin, FeeAdmin, Guardian, Creator, Bettor, Voter, Referrer, or Permissionless), and the key invariants that the model enforces (admin/guardian separation, vote_on_guardian_removal auth, release_creation_deposit auth, dual-layer admin checks).

Test plan

  • services/ttsnpm test passes (existing suite covers RateLimiter, AudioCache, sanitizeInput, TTSProviderError; new retry helpers follow the same error-class contracts)
  • contracts/predict-iqcargo test passes including the two new bets_fuzz_test properties
  • Manually verify TTS_RATE_LIMIT_PER_MINUTE=2 returns 429 + Retry-After on the 3rd request within a window
  • Verify TTS_MAX_RETRIES=1 TTS_MAX_DELAY_MS=100 produces one retry log line on a simulated 502

Closes #994
Closes #995
Closes #996
Closes #997

🤖 Generated with Claude Code

Grace-CODE-D and others added 4 commits June 27, 2026 17:20
…solutions-plug#994)

Transient provider errors (429, rate-limited; 5xx, server errors) are
now retried up to TTS_MAX_RETRIES (default 3) times with exponential
backoff capped at TTS_MAX_DELAY_MS (default 60 s). Full jitter avoids
thundering-herd on shared quota. Non-retriable errors (400, 401, 403)
propagate immediately without consuming retry budget.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ns-plug#995)

Adds express-rate-limit middleware that enforces TTS_RATE_LIMIT_PER_MINUTE
(default 60) requests per 60-second window per caller. Authenticated callers
are bucketed by their API key; anonymous callers by IP address. Exceeding the
limit returns 429 with a Retry-After: 60 header. Health-check endpoints are
exempt. The in-service RateLimiter (TTSService) continues to operate as a
secondary per-caller guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cked ops (solutions-plug#996)

- bets.rs: existing_bet.fee_paid now uses checked_add (returns
  ArithmeticOverflow instead of wrapping in release builds)
- fees.rs: collect_fee changed to return Result<(), ErrorCode>;
  both per-token and overall fee accumulators now use checked_add
  so a protocol-revenue overflow fails the bet rather than silently
  undercount protocol fees
- bets_fuzz_test.rs: added two payout-path fuzz properties —
  boundary-value claim scenarios and a direct overflow guard on the
  parimutuel multiplication formula

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…zation model (solutions-plug#997)

Two state-mutating functions were missing Soroban authorization checks:

- governance.rs::vote_on_guardian_removal — verified guardian membership by
  address comparison but never called voter.require_auth(), allowing any
  account to cast votes as any guardian. Fixed by adding voter.require_auth()
  before the membership check.

- markets.rs::release_creation_deposit — had no authorization at all; any
  account could trigger the deposit refund. Fixed by calling
  market.creator.require_auth() so only the creator can claim their deposit.

Also adds an Authorization Model section to contracts/predict-iq/README.md
documenting the role-based access model for every mutable contract function.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@drips-wave

drips-wave Bot commented Jun 27, 2026

Copy link
Copy Markdown

@Grace-CODE-D Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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

Labels

None yet

Projects

None yet

1 participant