Skip to content

[Audit] Fund lockup on unrevealed intents, settle/dispute race, dead insurance wiring, reputation inversion + sybil farming #2

Description

@AngeYobo

Summary

Manual review of the four core contracts (full read). The foundation is clean — CEI ordering, SafeERC20, custody-at-creation, documented invariants, good test coverage — but the review surfaced several real issues, including three critical that the 165 passing tests do not appear to cover.


🔴 Critical

C-1 — Funds permanently locked on never-revealed intents

createIntent transfers funds into custody, but if the payer never reveals (lost salt, crash, change of mind), no exit path exists:

  • revealIntent reverts after the deadline
  • expireIntent requires state == REVEALED

A COMMITTED intent is a black hole.

Related dead code: in revealIntent, the intent.state = EXPIRED assignment followed by revert RevealDeadlineExpired() is a no-op — the revert rolls back the state write.

Fix: add an expireCommittedIntent path, callable after REVEAL_DEADLINE, that refunds the payer.

C-2 — Provider can front-run any dispute with settleIntent; FINALITY_GATE is never used

  • settleIntent is callable by the provider immediately after reveal, with a self-attested successGain, and pays out 100% of the principal.
  • initiateDispute requires state == REVEALED — so the provider can front-run any dispute attempt with a settle.
  • The 7-day DISPUTE_DEADLINE is illusory: in practice the payer's dispute window is zero.
  • FINALITY_GATE = 3 days is declared, exposed in the interface, and never referenced anywhere in the code.

Fix: either a contestation delay after settlement (before actual payout), or explicit payer acceptance before settle.

C-3 — Insurance module is unreachable in a real deployment

InsurancePool.setMaxPayerPayoutPerToken is onlyEscrow, but the forwarding call inside AgentEscrow.setMaxPayerPayoutPerToken is commented out. As a result:

  • maxPayerPayoutPerToken stays at 0 in the pool
  • authorizeClaim systematically reverts with TokenNotSupported

The entire insurance module is dead on arrival. Tests likely pass because mocks bypass the real wiring.


🟠 High

H-1 — Reputation argument inversion → trivial sybil farming → unfunded credit

The escrow calls recordSuccess(intent.payer, intent.provider, gain), but the registry signature is recordSuccess(provider, payer, gain). The payer accumulates the score, while the docs and README describe provider reputation.

This is internally consistent with grantCredit (which gates on the payer's score), but it opens a full exploit chain:

  1. A payer spins up ~16 sybil providers and creates micro-intents to itself.
  2. Each sybil grants +50/epoch (successGain is chosen by the provider).
  3. Score reaches 800 in a single epoch for near-zero cost.
  4. grantCredit is callable by anyone, with no funds backing it.
  5. FastMode credit removes the provider's stake lock — intents run with no provider collateral and no slash possible (resolveDispute skips slashing when usedCredit).

Chain: farming → free credit → uncollateralized, unslashable intents.


🟡 Medium

M-1 — Deploy.s.sol is empty; the documented deploy order is impossible as written

The four constructors take mutually circular immutable addresses (escrow ↔ stakeManager ↔ insurancePool). Deployment requires CREATE2 or nonce precomputation.

M-2 — insuranceAmount can be authorized even when the provider wins

The payer receives insurance AND the provider the principal. The arbiter is trusted, but this should be forbidden at the contract level.

M-3 — Slashed amounts go to the pool, not the harmed payer

This weakens the incentive to dispute rather than simply waiting for expireIntent.

M-4 — Single immutable arbiter combines dispute resolution and parameter governance

Acceptable for Phase 0, but it should be named explicitly in the threat model.


Suggested adversarial tests

  • Front-run: settleIntent racing initiateDispute
  • Sybil reputation farming (self-dealing micro-intents, provider-chosen successGain)
  • Never-revealed intent: fund recovery path
  • Insurance wiring exercised end-to-end without mocks

Priority

C-1, C-2, C-3, and H-1 should be fixed before any Base Sepolia deployment. Given the project's positioning, the settle/dispute race and the unfunded credit path are exactly the kind of economic-containment failures the protocol itself claims to prevent.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions