Update Lido oracle allowlist for EDF and bump validator-ejector to 2.2.1 - #2831
Conversation
Lido oracle members move to EDF (LIP-37) DelegationContracts. Reports are signed by a delegate EOA, which is what the ejector verifies, so the allowlist has to hold delegate keys. Only ejector 2.2.0+ verifies EDF reports. - ethd: mainnet and hoodi VE_ORACLE_ADDRESSES_ALLOWLIST set to the EDF delegate EOAs (mainnet vote 205, Hoodi already enacted). - lido-obol.yml: same Hoodi list as the compose default; image default 1.9.0 -> 2.2.1. - default.env: VALIDATOR_EJECTOR_TAG 2.2.1.
There was a problem hiding this comment.
🟡 Changes recommended
A critical invalid-JSON allowlist issue and a moderate upgrade-path issue remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates validator-ejector configuration for EDF delegate EOAs and bumps the image to 2.2.1.
Changes:
- Updates mainnet and Hoodi oracle allowlists.
- Updates Hoodi defaults and image version.
- Bumps the default validator-ejector tag.
File summaries
| File | Summary |
|---|---|
lido-obol.yml |
Updates Hoodi defaults and validator-ejector image version. |
ethd |
Updates EDF delegate allowlists; critical JSON escaping issues remain in both assignments. |
default.env |
Updates the default tag; existing installs may retain the incompatible older tag. |
Review details
Suppressed comments (1)
ethd:6503
- This Hoodi assignment has the same invalid shell quoting: the backslash-newline pairs are literal characters inside the single-quoted value, making
VE_ORACLE_ADDRESSES_ALLOWLISTinvalid JSON. Use a single-line value or concatenate outside the quoted segments before writing it to.env.
VE_ORACLE_ADDRESSES_ALLOWLIST='["0xcA80ee7313A315879f326105134F938676Cfd7a9","0xAe13D937a042aeD48Eb67EFa935972b188578977","0xe9FCd8c6CA10414E1955a8366Ea72eD1BbAFC3Bf", \
"0x3E32F6E0E9A2f55eF6Ee4075bb249c8E88a42A15","0x6070B31816E11A8b222Fa232266FFdDC02bED02b","0xcad8AeeEd49158E20F8429B28520541cFa2C27b1", \
"0x049B1d2F7578Bb56F9B75fcE6a3ee5C40551e694","0xEDF5163607997899B91BEBD4feb1d5D78c896b57","0xBD9d87c5CAD402447408665E36b1e550a84E746B", \
"0xad4C09E3cc0bCE7FF70bb23f624Cc9dE86eba556"]'
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review follow-up: - A backslash-newline inside a single-quoted value is kept literally by bash, so the split allowlists were not valid JSON. Move both lists into __lido_oracle_allowlist(), one line per network, used by config. - ethd update keeps *_TAG values and only migrates on an ENV_VERSION bump. Bump to 69 and, for installs below it, set VALIDATOR_EJECTOR_TAG to the default and rewrite a non-empty VE_ORACLE_ADDRESSES_ALLOWLIST for the install's network.
There was a problem hiding this comment.
🟡 Changes recommended
The migration can overwrite an explicit [] allowlist, unintentionally enabling oracle verification.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
|
@cnupy Thank you. Lido guidance wants both sets, as EDF isn't enacted yet. |
yorickdowne
left a comment
yorickdowne
left a comment
There was a problem hiding this comment.
Use the Lido recommended list for mainnet
[
"0x73181107c8D9ED4ce0bbeF7A0b4ccf3320C41d12",
"0x4118DAD7f348A4063bD15786c299De2f3B1333F3",
"0x404335BcE530400a5814375E7Ec1FB55fAff3eA2",
"0x8dB977C13CAA938BC58464bFD622DF0570564b78",
"0x007DE4a5F7bc37E2F26c0cb2E8A95006EE9B89b5",
"0xc79F702202E3A6B0B6310B537E786B9ACAA19BAf",
"0x61c91ECd902EB56e314bB2D5c5C07785444Ea1c8",
"0xe57B3792aDCc5da47EF4fF588883F0ee0c9835C9",
"0x042a9e5acCfa17e28300F1b5967f20891E973922",
"0x5595033F304217aFDB8ffB35E42C22B72184730b",
"0x82A821E8a2585D1AC8f346BE7Da6995490d21Ca6",
"0x7abC999C7E1f22a7E12b2A1024bC17676FE18b4b",
"0x5416CAAb6f37BF81cb2dc7ce0a363AfB9DD92d57",
"0x565F04cB319EC2295E1fcbBbe5F708687FFcCE75",
"0x4D3aD7E8e591d389B612Fc063f53837E284d3F86",
"0x375ABa35EA2011Af97b51bd395494C827d1C39BD",
"0xC10258969442c7957351A0ddc8194D2685B2a2c2",
"0x2aE828F10DfeE5b4c42f6E61D3d30033D4520753"
]
Per review: EDF is not enacted on mainnet yet, so keep the current oracle EOAs alongside the EDF delegate EOAs, as the Lido guide recommends. The pre-EDF set can be dropped once their reports leave the lookback window.
|
Done in 448d388: mainnet now carries the combined list, the 9 current oracle EOAs plus the 9 EDF delegates. Hoodi stays on the delegates only since EDF is already live there. |
There was a problem hiding this comment.
🔵 Needs a closer look
The v69 migration can overwrite custom VE_ORACLE_ADDRESSES_ALLOWLIST values on unsupported networks (and mutates NETWORK during migration), which is an avoidable operational risk.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
ethd:2223
- In the v69 migration, rewriting
VE_ORACLE_ADDRESSES_ALLOWLISTunconditionally uses__lido_oracle_allowlist(), which returns[]for any non-(mainnet|hoodi) network. That can silently clobber a custom allowlist on other networks and effectively disable validator-ejector verification by making every signer miss the allowlist. This block also mutates the globalNETWORKvariable while migrating values, which can have unintended side effects later in__migrate_env.
Consider only rewriting on supported networks, and avoid changing NETWORK globally by reading it into a temporary variable and passing it to the function via a one-command assignment.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Per review: __lido_oracle_allowlist() returns [] for other networks, so the v69 migration would have replaced a custom allowlist there with an empty one. Read the install's network into a local, rewrite only for mainnet and hoodi, and pass the network to the helper as an argument instead of setting the global NETWORK.
|
e12c40c addresses the last Copilot note: the v69 migration now rewrites |
There was a problem hiding this comment.
🔵 Needs a closer look
The changes alter production-critical allowlists and upgrade behavior whose correctness depends on externally verified on-chain membership/delegate data.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Lido oracle members are moving to EDF (LIP-37) DelegationContracts. Oracle reports are then submitted through the delegation contract and signed by a delegate EOA, which is what the validator-ejector verifies. So
VE_ORACLE_ADDRESSES_ALLOWLISThas to hold the delegate EOAs, and only validator-ejector 2.2.0+ can verify EDF reports.Guide: https://hackmd.io/P95OOWJ0QZSfvguSbRp5HA
Proposal: https://research.lido.fi/t/lip-37-execution-delegation-framework-edf/11746
Changes
ethd: new__lido_oracle_allowlist()returns the allowlist per network, one line each. Mainnet: the 9 current oracle EOAs plus the 9 EDF delegate EOAs (vote 205 is not enacted yet, so both sets are needed). Hoodi: the 10 EDF delegate EOAs (already enacted).__config_lido_oboluses it.ethd:ENV_VERSION69 migration. Installs below 69 getVALIDATOR_EJECTOR_TAGreset to the default and, on mainnet or hoodi, a non-emptyVE_ORACLE_ADDRESSES_ALLOWLISTrewritten for that network, so a plainethd updateis enough. Other networks and empty values are left alone.lido-obol.yml: Hoodi default allowlist updated. Image default 1.9.0 -> 2.2.1.default.env:VALIDATOR_EJECTOR_TAG=2.2.1,ENV_VERSION=69.Verification
getMembers()on the VEBO HashConsensus (0x7FaDB6358950c5fAA66Cb5EB8eE5147De3df355a) returns the 9 current EOAs.getDelegate()on each of the 9 DelegationContracts from the vote description returns the 9 delegate EOAs.getMembers()on the VEBO HashConsensus (0x30308CD8844fb2DB3ec4D056F1d475a802DCA07c) returns 10 DelegationContracts whosegetDelegate()values are the 10 addresses used here.bash -nand shellcheck pass..env.sourcewithNETWORK=mainnet, tag 1.9.0 and an old allowlist: tag becomes 2.2.1, allowlist becomes the mainnet list. An empty allowlist stays empty.