Skip to content

fix: add settlement timeout and emergency refund for stuck directional positions - #4

Open
pplmaverick wants to merge 1 commit into
circlefin:masterfrom
pplmaverick:fix/add-settlement-timeout-emergency-refund
Open

fix: add settlement timeout and emergency refund for stuck directional positions#4
pplmaverick wants to merge 1 commit into
circlefin:masterfrom
pplmaverick:fix/add-settlement-timeout-emergency-refund

Conversation

@pplmaverick

Copy link
Copy Markdown

Problem

EventBasedPredictionMarket resolves via UMA's Optimistic Oracle V2, which is a real
improvement over a single-admin-key oracle — anyone can permissionlessly call
proposePrice(), and disputes escalate to the DVM. That part works well.

However, there's no fallback if nobody ever proposes a price (e.g. a low-interest
market where the proposerReward doesn't attract a proposer, or the underlying question
becomes unresolvable). In that case:

  • receivedSettlementPrice stays false forever.
  • settle() reverts indefinitely (require(receivedSettlementPrice, ...)).
  • redeem() only works for holders of matched Long+Short pairs (1:1 burn). Anyone
    holding a one-sided directional position — which is the normal outcome of trading
    through the AMM — has no exit path at all.

I ran into this while building a live weather prediction market on Arc Testnet
(~55 Arc Testnet transactions across create/lock/settle/claim flows) using a simpler
onlyOracle-gated design, and traced the same failure mode back to this reference
implementation while evaluating a migration to it.

Fix

  • Adds settlementDeadline, set to block timestamp of initializeMarket() + SETTLEMENT_TIMEOUT
    (72 hours, currently a fixed constant — open to making this configurable per-market).
  • Adds emergencyRefund(longTokensToRedeem, shortTokensToRedeem): callable by any token
    holder once settlementDeadline has passed with no settlement price received. Pays out
    at a neutral 0.5/0.5 split per token — the same math the contract already uses for the
    OO's "Undetermined" (5e17) outcome — since the true result was never established.

Known limitation / open question for maintainers

priceDisputed() re-requests a price with a fresh timestamp but does not reset
settlementDeadline. If a dispute lands close to the original deadline, emergencyRefund()
could theoretically become callable while a legitimate DVM arbitration is still in flight.
I left this as-is and flagged it with a code comment rather than guessing at the right
behavior (extend the deadline on every dispute? cap the number of extensions?) — wanted to
raise it for discussion rather than bake in an assumption.

Also not included yet: unit tests for the new function and the timeout boundary. Happy to
add a test file (mirroring the existing Hardhat/UMA test setup) if the general approach
looks right to maintainers before I invest in test scaffolding.

Testing

Not yet covered by automated tests — see note above. Manually reasoned through the
solvency invariant: before any settlement, longToken.totalSupply() == shortToken.totalSupply() == collateralToken.balanceOf(address(this)) (since create()/redeem() always move both
sides in lockstep), so the 0.5/0.5 split in emergencyRefund() can't be under-collateralized
in the no-dispute case.

@pplmaverick

Copy link
Copy Markdown
Author

Hi @akelani-circle — wanted to follow up on this PR. With Arc mainnet approaching, any directional prediction market deployed using this template will hit a permanent stuck state if no proposer ever calls proposePrice(). Users holding one-sided positions have no exit path.
Happy to add tests or adjust the implementation if that helps move this forward. Would appreciate a quick review before mainnet goes live.

@@ -199,6 +208,9 @@ contract EventBasedPredictionMarket is Testable {
requestTimestamp = getCurrentTime();

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] Refresh the deadline for a re-requested price

priceDisputed() creates a fresh oracle request but retains the deadline from the original request. A dispute immediately before expiry therefore leaves a live replacement request subject to an already-expired emergencyRefund() window.

I reproduced this against 77d9ee6: after creating 100 pairs and transferring the Short position to a second holder, I triggered priceDisputed() at settlementDeadline - 1. At settlementDeadline + 1, emergencyRefund(100, 0) returns 50 collateral to the Long holder while the replacement request is still pending. If that request then resolves NO, the Short holder is owed 100 collateral but only 50 remains, so settlement reverts on the collateral transfer.

Renew the deadline when the fresh request is created:

requestTimestamp = getCurrentTime();
settlementDeadline = requestTimestamp + SETTLEMENT_TIMEOUT;
_requestOraclePrice();

Please add a regression test for the dispute-at-deadline boundary and verify that the later settlement remains fully collateralized.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the PoC — the race condition is real and the fix is correct in isolation.

One thing worth flagging before we land it: unconditionally resetting settlementDeadline on every dispute opens a second attack vector. A malicious actor can repeatedly dispute every incoming proposal, pushing the deadline forward indefinitely and permanently blocking emergencyRefund(). That trades a one-shot race (current design) for an unbounded griefing path.

Three options as I see it:

Option A — Reset once, then lock
Reset settlementDeadline on the first dispute only. Subsequent disputes extend the oracle liveness period but leave the deadline fixed.

if (!deadlineExtended) {
    settlementDeadline = getCurrentTime() + SETTLEMENT_TIMEOUT;
    deadlineExtended = true;
}

Option B — Cap total extensions
Allow up to N resets (e.g. maxDisputeExtensions = 2), after which the deadline is frozen regardless of further disputes.

Option C — Keep current design, document the boundary
Accept the one-shot race as the lesser risk and strengthen the existing code comment so future maintainers understand the explicit trade-off.

I'd lean toward Option A as the simplest fix that closes both vectors. Happy to implement whichever direction the maintainers prefer before adding the regression test.

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