fix: checked arithmetic, TTL management, and reputation persistence (…

[k2ghostyou]

…#269, #271, #272)

Issue #269 — Systematic audit for unchecked i128/u64 arithmetic overflow paths

Problem

get_adjusted_min_bond computed (MIN_BOND * multiplier) / 10 with plain multiplication, relying entirely on the Cargo.toml overflow-checks = true release-profile setting for safety. While that setting causes a panic rather than silent wrapping, a panic on the accept_intent hot path is a denial-of-service concern. The fee arithmetic in fill_intent had already been hardened with checked_mul/checked_div (issue #31), establishing that pattern as the codebase's intended standard; get_adjusted_min_bond should follow it.

Additionally, fill_intent contained three copies of the transfer/fee logic: two early-draft blocks (one with unchecked fee arithmetic, one that was the CEI-correct block) and then a third late 'Interactions' block. The net effect was that transfers were being executed three times per fill, a serious bug.

What was done

• Added a new variant to the Error enum with a doc comment explaining when it fires.
• Converted to use and , unwrapping to on overflow — consistent with fill_intent's FeeOverflow pattern.
• Audited every other i128/u64 multiplication and division in intent_settlement/src/lib.rs and proof_registry/src/lib.rs:
  • compute_reputation_score: base_bps = (fills_completed as u64 * 10_000) / total_fills. fills_completed is u32 (max ~4.3e9); *10_000 gives max ~4.3e13, well within u64::MAX (~1.8e19). total_fills >= 1 before division (guarded by the early return). decay_bps uses VOLUME_SCALE as u64 (~1e18) * 10_000 = max ~1e22, which exceeds u64::MAX — however VOLUME_SCALE as u64 is only ~1e12 (VOLUME_SCALE = 1_000 * 100 * 10_000_000 = 1e12), so the product is ~1e16, safely within u64. The doc comment's 'cannot panic' claim holds. No code change needed.
  • proof_registry/src/lib.rs: no i128/u64 multiplications or divisions at all; only array index arithmetic on u32 byte offsets. No change needed.
  • fill_intent fee calculation: the early-draft unchecked fee calculation (let fee = fill_amount * PROTOCOL_FEE_BPS / 10_000) and all duplicate transfer blocks were removed. The single CEI-correct path with checked arithmetic and get_tiered_fee_bps is retained.
• The previously duplicate and out-of-order fill_intent body was cleaned up to a single CEI-correct path: state writes first, token transfers after.

Closes

Closes #269

---

Issue #271 — Fix missing TTL management for CancelCooldown/MinBondMultiplier/ExtensionGranted/UserIntents

Problem

intent_settlement/src/lib.rs called bump_intent_ttl and bump_solver_ttl at every write to Intent/Solver persistent keys, but four other persistent keys were written without any TTL bump:

• DataKey::CancelCooldown(Address): written by cancel_intent; if archived, silently resets the cancel cooldown to 'never cancelled', defeating spam deterrence.
• DataKey::MinBondMultiplier(Address): written by set_min_bond_multiplier; if archived, silently reverts a token's bond requirement to the 1.0x default, potentially under-collateralising accepts on high-risk tokens.
• DataKey::ExtensionGranted(BytesN<32>): written by request_extension; if archived, silently clears the one-shot extension flag, allowing a solver to request a second fill-window extension on the same intent.
• DataKey::UserIntents(Address): written by submit_intent; if archived, list_intents_by_user silently returns an empty or incomplete list.

None of these failures surface an error — they simply cause the contract to behave as if the data never existed, making the bugs invisible at the call site.

What was done

• Added four TTL bump helpers following the exact pattern of bump_intent_ttl and bump_solver_ttl, each with a doc comment explaining the specific archival failure mode it prevents:
  • bump_cancel_cooldown_ttl(env, user): bumps CancelCooldown(Address)
  • bump_min_bond_multiplier_ttl(env, token): bumps MinBondMultiplier(Address)
  • bump_extension_granted_ttl(env, intent_id): bumps ExtensionGranted(BytesN<32>)
  • bump_user_intents_ttl(env, user): bumps UserIntents(Address)
• Each helper uses the same PERSISTENT_TTL_THRESHOLD / PERSISTENT_TTL_EXTEND_TO constants (14-day threshold, 30-day extend-to) as the existing bump helpers.
• Each helper is called immediately after the corresponding persistent().set() call at its write site, with a comment citing [High] Fix missing TTL management for CancelCooldown/MinBondMultiplier/ExtensionGranted/UserIntents #271.
• Updated docs/ttl-constants-rationale.md to cover all four new keys: each gets its own sub-section documenting the failure mode without TTL management, the fix, and the rationale for using the same TTL constants. A cost analysis section confirms the per-call overhead is comparable to the existing TTL bumps and does not materially affect wasm size or resource fees.

Closes

Closes #271

---

Issue #272 — Fix SLASH_COOLDOWN/reputation reset via deregister_solver + register_solver

Problem

deregister_solver removed the SolverRecord entirely from persistent storage. A subsequent register_solver call from the same address, finding no existing record, created a brand-new one with last_slash_time = 0, fills_completed = 0, and fills_failed = 0. This made two exploits trivially available to any solver:

1. Slash-cooldown bypass: after being slashed, deregister and immediately re-register. The accept_intent guard (last_slash_time > 0 && now < last_slash_time + SLASH_COOLDOWN) sees last_slash_time = 0 and passes, despite the solver being just-slashed.

2. Reputation reset: deregister and re-register to wipe fills_completed, fills_failed, and total_volume, resetting compute_reputation_score to a clean slate on demand — defeating the entire reputation system's purpose as a persistent, hard-to-game track record.

Both exploits cost nothing beyond a bond withdrawal and redeposit.

What was done

Option (b) was chosen: preserve reputation-relevant fields across the deregister/re-register cycle, rather than option (a) (block deregistration during cooldown). Option (b) is more robust because it also closes the reputation-reset exploit for solvers who simply want a clean slate rather than cooldown evasion. It does not add friction to legitimate solver exits.

Specifically:

• Added a new DataKey::SolverReputation(Address) variant to the DataKey enum, with a detailed doc comment explaining its purpose and TTL management.
• Added a new ReputationSnapshot contracttype struct with four fields: last_slash_time, fills_completed, fills_failed, total_volume. Only the reputation-relevant fields are stored; bond_amount, active_intents, registered_at, and is_active are intentionally omitted (they are reset on re-registration as intended).
• Modified deregister_solver: immediately before removing the SolverRecord, write a ReputationSnapshot to DataKey::SolverReputation(solver) and bump its TTL (using the standard PERSISTENT_TTL constants). The snapshot write is inside the CEI effects block, before the token transfer.
• Modified register_solver: when no existing SolverRecord is found (new registration path), check for a DataKey::SolverReputation snapshot. If found, populate last_slash_time, fills_completed, fills_failed, and total_volume from the snapshot, then remove the snapshot. If not found, zero-initialise these fields as before (happy path is unchanged for first-time registrations).
• A solver who was never slashed and has no prior fills sees no snapshot and gets identical behaviour to before this fix — the happy path is unaffected.
• Updated SECURITY.md: moved the 'slash-cooldown and reputation reset' item from open threat to 'Closed gap', documenting the exploit, the chosen fix approach, and the rationale for option (b) over option (a).

Closes

Closes #272

---

Supporting changes (prerequisite completeness)

The codebase used many identifiers that were referenced in function bodies but not yet defined at the top of the file (missing from enums, constant sections, etc.). These were added as part of making all three fixes coherent:

• DataKey enum: added Config, PendingAdmin, AllowedDstTokenList, MinBondMultiplier(Address), CancelCooldown(Address), ExtensionGranted(BytesN<32>), UserIntents(Address), PendingDstTokenAdd(Address), PendingDstTokenRemove(Address), SolverReputation(Address) — all with doc comments.
• Error enum: deduplicated repeated discriminant values (22 was used four times; 23 was used twice); assigned correct unique discriminants to NoPendingFeeRecipient (29), SrcChainNotAllowed (25), RescueProtectedToken (26), and added TimelockNotElapsed (30), NoPendingAdminTransfer (31), NoPendingDstTokenChange (32), AmountTooLarge (33), InvalidConfig (34), CancelCooldownNotExpired (35), BondMultiplierOverflow (36).
• Constants: added DEFAULT_MIN_BOND, DEFAULT_FILL_WINDOW, DEFAULT_INTENT_EXPIRY, DEFAULT_PROTOCOL_FEE_BPS, MAX_PROTOCOL_FEE_BPS, MIN_FILL_WINDOW_SECS, MIN_INTENT_EXPIRY_SECS, MIN_BOND_FLOOR, SLASH_COOLDOWN, CANCEL_COOLDOWN, MAX_EXTENSION_DURATION, MAX_BATCH_SIZE — all with rationale comments.

Summary

Related issue

Type of change

• Bug fix
• New feature
• Refactor
• Documentation
• CI / tooling

Component

• Contract (vortex-contract)
• Backend (vortex-backend)
• Frontend (vortex-frontend)

Checklist

• My code follows the project's style and conventions
• I ran lint / type-check / build locally and they pass
• I added or updated tests where appropriate
• I updated documentation where appropriate
• My commits follow Conventional Commits

Screenshots / notes

[drips-wave]

@k2ghostyou 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