feat(bot-oo): add guarded OOv2 settlement lists#4952
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
chrismaree
left a comment
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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
08299af to
12df236
Compare
md0x
changed the title
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. Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
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. Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
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. Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
An empty exclude list skips nothing and behaves the same as unset, so it no longer trips the non-OOv2 startup throw. An empty include list still throws there since "settle nothing" cannot be honored outside OOv2. Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
12df236 to
5de2aef
Compare
Reinis-FRP
left a comment
Reinis-FRP
left a comment
There was a problem hiding this comment.
Review — guarded OOv2 settlement lists
Overall this is well-designed, well-documented, and well-tested. The fail-closed defaults (ManagedOptimisticOracleV2 defaulting to an empty include list, plus the redundant guard in settleOOv2Requests) are the right safety posture for a bot that settles on-chain, and the "empty exclude = explicit opt-in to settle-all" semantics are handled precisely and covered by tests.
Strengths
- Defense in depth: a misconfigured Managed bot can't accidentally settle everything, even when params are built directly.
- Consistent tx-hash normalization in
proposalEventIdon both the parse and match sides. - Precedence logic
(configuredIncludeList || settleExcludeList?.size)correctly distinguishes empty-include ("settle nothing", rejected for non-OOv2) from empty-exclude ("no-op, allowed anywhere"), with tests for each branch. OracleTypeunion, env validation, error messages, and the dispatcher switch all updated in lockstep.
Findings (3 inline comments):
skippedIdsdebug log is unbounded in include mode — the default path for Managed OOv2. (minor, worth addressing)- README could clarify how operators obtain the block-scoped
<logIndex>. (minor doc improvement) Number()logIndex normalization. (nit, no action needed)
None are blocking. Recommend addressing (1) before merge; (2) is a nice-to-have.
One process note: only the DCO check is attached to the head commit — worth confirming the full test/lint CI ran. And per the description, the fail-closed default makes the deploy-before-config sequencing with UMAprotocol/bot-configs#4222 important; call that out in the deploy runbook.
Submitted by 🤖 Reinis Claude coding agent
Generated by Claude Code
Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Reinis-FRP
left a comment
Reinis-FRP
left a comment
There was a problem hiding this comment.
looking good, thanks!
3 participants
Summary
Adds optional include/exclude proposal lists to the unified OOv2 settlement bot and introduces an explicit
ManagedOptimisticOracleV2oracle type.OptimisticOracleV2keeps the existing behavior when both lists are unset: settle every eligible proposal.ManagedOptimisticOracleV2fails closed when both lists are unset by defaulting to an empty include list.SETTLE_INCLUDE_LISTsettles only listed proposals and takes precedence over the exclude list.SETTLE_EXCLUDE_LISTskips listed proposals; an explicit[]opts in to settling every eligible proposal.Proposal IDs use the transaction hash and log index of the
ProposePriceevent:"<txHash>:<logIndex>".Changes
ManagedOptimisticOracleV2through the OOv2 settlement implementation.The coordinated deployment config is UMAprotocol/bot-configs#4222. Deploy this protocol change before enabling that config.
Test plan
yarn hardhat test ./test/OptimisticOracleV2Bot.ts