feat(bot-oo): add include/exclude proposal lists to OOv2 settler#4952
feat(bot-oo): add include/exclude proposal lists to OOv2 settler#4952md0x wants to merge 3 commits into
Conversation
Adds SETTLE_INCLUDE_LIST and SETTLE_EXCLUDE_LIST env vars to the unified OO settlement bot, scoped to OptimisticOracleV2. Proposals are matched by their ProposePrice event identifier (<txHash>:<logIndex>). When an include list is set the bot settles only those proposals (takes precedence over the exclude list); otherwise proposals in the exclude list are skipped.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85fc0e047a
ℹ️ 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".
| if (!Array.isArray(parsed) || parsed.length === 0) { | ||
| throw new Error(`${envName} must be a non-empty JSON array of "<txHash>:<logIndex>" strings`); |
There was a problem hiding this comment.
When SETTLE_INCLUDE_LIST or SETTLE_EXCLUDE_LIST is explicitly configured as [], the bot now throws during startup even though the README documents these vars as JSON arrays and empty arrays are a common no-op/default value for list env vars. This is especially surprising for SETTLE_EXCLUDE_LIST=[], which should behave the same as unset, and for SETTLE_INCLUDE_LIST=[], which can naturally mean settle no listed proposals; rejecting it can break deployments that template optional lists as empty arrays.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in a397008. An explicit [] is now accepted: an empty exclude list behaves the same as unset, and an empty include list settles nothing (the include list is exclusive and keeps precedence over the exclude list). Added a test and documented both in the README.
| DEFAULT_SETTLE_MIN_PROPOSAL_AGE_SECONDS | ||
| ); | ||
|
|
||
| const settleIncludeList = parseProposalIdList(env.SETTLE_INCLUDE_LIST, "SETTLE_INCLUDE_LIST"); |
There was a problem hiding this comment.
These lists are parsed for every oracle type, but only settleOOv2Requests applies them — on OptimisticOracle/SkinnyOptimisticOracle they're silently ignored and the bot settles everything. That's a footgun for an exclude list (operator thinks a market is protected when it isn't). It does follow the documented SETTLE_ONLY_DISPUTED precedent, but consider throwing (or at least logging a startup warning) when either var is set with a non-OOv2 ORACLE_TYPE, since oracleType is already known here. Also worth noting the current asymmetry: a malformed value throws even for OOv1/Skinny, while a valid one is silently dropped.
There was a problem hiding this comment.
Addressed in 980c45d. Startup now throws when either var is set with a non-OOv2 ORACLE_TYPE — went with the throw rather than a warning since a startup log line is easy to miss, the lists are protective config, and the vars are new so no existing deployment can break. This also fixes the asymmetry: malformed and misapplied lists now both fail fast. Added a test and noted the behavior in the README.
An explicit [] no longer throws at startup: an empty exclude list behaves like unset, and an empty include list settles nothing (the include list is exclusive). Matches the OG_WHITELIST/ASSERTION_BLACKLIST convention of accepting empty arrays.
Only the OOv2 settler applies SETTLE_INCLUDE_LIST/SETTLE_EXCLUDE_LIST; silently ignoring them on OptimisticOracle/SkinnyOptimisticOracle would settle proposals the operator intended to skip, so startup now fails fast instead.
chrismaree
left a comment
There was a problem hiding this comment.
No blocking findings from my pass. I checked the current head against the include/exclude semantics, empty-array handling, non-OOv2 fail-fast behavior, docs/tests coverage, and patch hygiene. The implementation looks scoped and the existing Codex comments have been addressed. I did not run local tests; GitHub checks are passing, including test-monitor-v2.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 980c45d204
ℹ️ 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".
| if ((settleIncludeList || settleExcludeList) && oracleType !== "OptimisticOracleV2") | ||
| throw new Error("SETTLE_INCLUDE_LIST/SETTLE_EXCLUDE_LIST are only supported for OptimisticOracleV2"); |
There was a problem hiding this comment.
Treat empty exclude lists as unset for non-OOv2
When ORACLE_TYPE is OptimisticOracle or SkinnyOptimisticOracle and a shared deployment template renders SETTLE_EXCLUDE_LIST=[], parseProposalIdList returns an empty Set, making this condition truthy and causing startup to throw. That contradicts the documented behavior that an empty exclude list behaves like unset, so non-OOv2 bots can be prevented from starting even though the list would skip nothing.
Useful? React with 👍 / 👎.
Summary
Adds optional include/exclude proposal lists to the unified OO settlement bot, scoped to
OptimisticOracleV2. This lets operators settle a specific subset of markets (or skip specific ones, e.g. sports games) without rebuilding infra.Two new env vars (JSON arrays of
"<txHash>:<logIndex>", matching the existingOG_WHITELIST/ASSERTION_BLACKLISTconvention):SETTLE_INCLUDE_LIST— settle only these proposals. Takes precedence over the exclude list.SETTLE_EXCLUDE_LIST— skip these proposals. Ignored when an include list is set.Both unset preserves current behavior (settle everything). Proposals are identified by the transaction hash and log index of their
ProposePriceevent — data the bot already has, so no extra RPC calls.Changes
requestKey.ts:proposalEventId(txHash, logIndex)helper, normalized to lowercase.common.ts: parsing + validation of the two env vars (invalid format throws a clear error) intoSets onMonitoringParams.SettleOOv2Requests.ts:filterByIncludeExcludeapplied after dropping already-settled requests; emits a debug log with kept/skipped counts and ids.README.md: documents both vars.Test plan
yarn hardhat test ./test/OptimisticOracleV2Bot.ts— 10 passing, including new cases for exclude-list skip and exclusive include-list (absent proposal → not settled, present → settled).