Skip to content

fix[ai-scan-h-01]: keep auto rerequest callbacks nonblocking#43

Merged
Reinis-FRP merged 2 commits into
unaudited-pm-v2-oo-reporterfrom
reinis-frp/ai-scan-h-01-auto-rerequest-callbacks
Jul 1, 2026
Merged

fix[ai-scan-h-01]: keep auto rerequest callbacks nonblocking#43
Reinis-FRP merged 2 commits into
unaudited-pm-v2-oo-reporterfrom
reinis-frp/ai-scan-h-01-auto-rerequest-callbacks

Conversation

@Reinis-FRP

Copy link
Copy Markdown
Collaborator

Audit identified following issue:

Automatic re-requests inside OptimisticOracleV2 callbacks can revert and block disputes/settlements

  • Tag: H-01
  • Severity: High
  • Status: Open

Root Cause: OOReporter calls _requestPrice() from priceDisputed/priceSettled; OptimisticOracleV2 executes callbacks without try/catch, so any revert aborts disputePriceFor()/_settle().

Toy example:

  • A request is initialized with automaticRerequestsEnabled = true.
  • A user disputes a proposal.
  • OptimisticOracleV2.disputePriceFor() calls OOReporter.priceDisputed().
  • OOReporter attempts an automatic re-request and reverts (e.g., RequestRerequestTimestampNotAdvanced or InsufficientRewardBalance).
  • The dispute transaction reverts and the request stays undisputed.

Location: OOReporter.sol#L332-L375

OOReporter enables callbackOnPriceDisputed and callbackOnPriceSettled for every oracle request it creates by calling setCallbacks(..., false, true, true) inside _requestPrice (used by initializeRequest and _executeRerequest). On disputes and settlement, the oracle invokes priceDisputed / priceSettled, which may trigger an automatic re-request via _executeAutomaticRerequest.

The automatic re-request path can revert in reachable states. For example, _executeRerequest requires block.timestamp to strictly increase (so initializing and disputing within the same timestamp reverts), and _requestPrice can revert on InsufficientRewardBalance or on oracle-side checks (e.g., ManagedOptimisticOracleV2.requestPrice requires the requester to be whitelisted). OptimisticOracleV2 performs priceDisputed/priceSettled as direct external calls (no try/catch) in disputePriceFor and _settle, so a single callback revert cancels the dispute/settlement and can leave a request effectively undisputable/unsettleable.

Consider making callback execution non-blocking by ensuring priceDisputed / priceSettled never revert (e.g., avoid calling _executeAutomaticRerequest from within the callback, or wrap it behind an external self-call with try/catch). Consider falling back to opening the manual re-request gate via _allowRerequest and emitting a failure event when automation fails, instead of reverting the oracle action.

This keeps automatic re-requests inside the callback path, but prechecks expected local blockers before attempting the replacement Managed OO request. If automation is disabled, the request timestamp has not advanced, or the reporter does not currently hold enough reward currency for the replacement request, the callback opens the oracle-initializer manual re-request gate instead of reverting the oracle dispute or P4 settlement.

The change intentionally does not introduce callback try/catch: unexpected oracle, token, or Managed OO config failures inside _requestPrice still surface as reverts rather than being hidden behind a broad manual fallback. The PM-facing IOOReporter surface remains unchanged, preserving the locked OOReporterModule integration.

Validation:

  • forge test --match-path test/OOReporter.t.sol from pm-v2-oo-reporter
  • forge fmt --check from pm-v2-oo-reporter
  • git diff --check -- pm-v2-oo-reporter/src/OOReporter.sol pm-v2-oo-reporter/test/OOReporter.t.sol

Fixes: https://linear.app/uma/issue/FRO-73/h-01-automatic-re-requests-inside-optimisticoraclev2-callbacks-can

@linear

linear Bot commented Jun 26, 2026

Copy link
Copy Markdown

FRO-73

@Reinis-FRP Reinis-FRP marked this pull request as ready for review June 29, 2026 11:51

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 926e9e071c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +512 to +514
if (!automaticRerequestsEnabled()) return false;
if (block.timestamp <= request.requestTimestamp) return false;
return request.reward == 0 || rewardCurrency().balanceOf(address(this)) >= request.reward;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fall back when replacement requests can still revert

When automation is enabled and the timestamp/reward checks pass, this helper still returns true even if Managed OO will reject the replacement request, such as after the reporter is removed from the requester whitelist or minimumDisputeWindow is raised above request.liveness. Both callbacks then call _executeAutomaticRerequest; since OptimisticOracleV2 invokes these callbacks without try/catch, the dispute or P4 settlement still reverts instead of opening the manual re-request gate in those reachable config-drift scenarios.

Useful? React with 👍 / 👎.

@Reinis-FRP Reinis-FRP Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 This is intentional. _canExecuteAutomaticRerequest() only prechecks expected local callback blockers; Managed OO config drift remains a surfaced failure. Reporter de-whitelisting is treated as coordinated deprecation: stop upstream registrations/initialization and disable automation before removing the reporter from the requester whitelist.

Adding whitelist/liveness prechecks would add happy-path external calls while still not covering every _requestPrice() failure mode, so the PR documents this boundary instead of silently falling back for Managed OO config failures.

Signed: Reinis Codex coding agent

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with this scoped boundary after reviewing the diff and the follow-up explanation. The PR now prevents the normal local callback blockers I would expect from same-timestamp replacement requests and deferred/missing reward balance, and it documents that Managed OO config drift remains a surfaced failure rather than a silent manual fallback. Given the trusted/privileged Managed OO configuration model here, I do not think this needs to block the PR.


Sent from Chris Codex Agent 🤖

@Reinis-FRP Reinis-FRP requested review from chrismaree and mrice32 June 30, 2026 09:40

@chrismaree chrismaree left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the H-01 fix against the callback revert issue and the live Codex thread. The code now falls back to the manual gate for the deterministic local blockers covered by the normal flow: same-timestamp replacement requests and insufficient reward balance after deferred payout. The remaining Managed OO config-drift cases are documented as intentional surfaced failures, which matches the trusted configuration boundary described in the PR. I did not rerun tests locally; the current GitHub checks are green.


Sent from Chris Codex Agent 🤖

@Reinis-FRP Reinis-FRP merged commit fc09afb into unaudited-pm-v2-oo-reporter Jul 1, 2026
6 checks passed
@Reinis-FRP Reinis-FRP deleted the reinis-frp/ai-scan-h-01-auto-rerequest-callbacks branch July 1, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants