chore(ens): draft calldata review for EP 6.39 - Treasury Flow Automation#75
Conversation
…omation Fetch draft proposal data from Tally draft 2820664548653007898. All 8 executable calls verified: calldata matches manual derivation.
![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: 2bda65929f
ℹ️ 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".
|
|
||
| This is the current suggestion for the funding policy. | ||
|
|
||
| The timelock maintains a 6 months runway in USDC (~$8M at current spending). Each quarter, Karpatkey calculates the current runway on the timelock. If it falls below 6 months, the endowment sends the shortfall. If it exceeds 6 months, no transfer is needed and the excess stays invested in the endowment. If an additional governance proposal requiring capital is approved, this may trigger an earlier runway evaluation and transfer, consistent with this policy. |
There was a problem hiding this comment.
The proposal currently defines three different runway targets: a “two-year” target in the Abstract/Impact sections, a “6 months” operating runway in Funding Policy, and a “three-year” requirement in the IPS alignment paragraph. Because these values drive when treasury transfers should occur, this inconsistency can lead to contradictory execution decisions depending on which section is followed. Please normalize the policy wording so one unambiguous runway rule is described.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This file is the proposal description fetched verbatim from the Tally draft — we don't author or modify it.
The runway numbers refer to different things:
- 6 months: operational USDC runway maintained on the timelock (quarterly rebalancing trigger)
- Three-year: stablecoin runway calculated at the Endowment level per the IPS
- Two-year (in Abstract): appears to be an inaccuracy in the proposer's text — likely should say "six-month" to match the Funding Policy section
Worth flagging to the proposal author but outside the scope of our calldata review.
| #### Key Addresses | ||
|
|
||
| • Timelock: 0xFe89cc7aBB2C4183683ab71653C4cdc9B02D44b7 (wallet.ensdao.eth) | ||
| • Current Registrar Controller: 0x253553366Da8546fC250F225fE3d25d0C782303b (controller.ens.eth) |
There was a problem hiding this comment.
The description lists 0x253553... as the current registrar controller, but the executable payload only adds/transfers ownership for 0x59E16f... and 0x283Af0... (calls 1–4 in proposalCalldata.json). This mismatch means the narrative and execution payload describe different controller scope, which can mislead reviewers and voters about which contracts are actually being handed to RegistrarManager.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — this was already noted as an INFO finding in the security report.
The description lists 0x253553... (controller.ens.eth) as "Current Registrar Controller" in Key Addresses, but the calldata only registers 0x59E16f... (ETH Registrar Controller 2) and 0x283Af0... (Old ETH Registrar Controller). This is intentional per the proposer's design: the description explains that controller.ens.eth "already sends withdrawn ETH to the timelock," so it's excluded from the RegistrarManager.
The narrative's Key Addresses section lists all relevant controllers for context, while the transactions only target the two that need ownership transfer. Not a calldata issue — a documentation clarity point for the proposer.
Include controller.ens.eth (0x2535...303b) alongside the existing two controllers. Proposal now has 10 transactions: 3 addRegistrar + 3 transferOwnership + 4 Zodiac role updates.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1450016b14
ℹ️ 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".
| // 1) Register current registrar controller (controller.ens.eth) | ||
| targets[0] = managerAddr; | ||
| calldatas[0] = abi.encodeWithSelector(RegistrarManager.addRegistrar.selector, address(NEW_REGISTRAR)); | ||
| calldatas[0] = abi.encodeWithSelector(RegistrarManager.addRegistrar.selector, address(CURRENT_REGISTRAR)); | ||
|
|
There was a problem hiding this comment.
_generateCallData() now includes addRegistrar(CURRENT_REGISTRAR) (and later transferOwnership for the same controller), but src/ens/proposals/ep-registrar-manager-endowment/proposalCalldata.json still defines an 8-call draft payload that does not include those operations. This means the governance simulation is validating a different transaction batch than the draft being reviewed, so the calldata check can report success while the real Tally payload remains unverified for this scope.
Useful? React with 👍 / 👎.
- Add dirPath() override to enable calldata comparison against proposalCalldata.json - Re-fetch draft with all 3 controllers and correct 65-byte Safe signatures - All 10 executable calls verified: manual derivation matches Tally draft
Add post-execution assertions that USDC transfers and ETH sends to non-timelock addresses are blocked. Prevents false positive if Zodiac conditions were misconfigured to allow broader scope.
Re-fetch proposalCalldata.json and description from the new draft URL. All 10 executable calls verified against manual derivation.
alextnetto
changed the title
1 participant
Summary
proposalDescription.mdto the EP 6.39 description ("Treasury Flow Automation")proposalCalldata.jsonwith the 10 executable calls from the draftCalldata Verification
addRegistrar(currentRegistrar)addRegistrar(newRegistrar)addRegistrar(oldRegistrar)transferOwnership(manager)transferOwnership(manager)transferOwnership(manager)execTransaction→scopeTarget(MANAGER, USDC)execTransaction→scopeFunction(MANAGER, USDC, transfer)execTransaction→scopeTarget(MANAGER, timelock)execTransaction→allowFunction(MANAGER, timelock, 0x0, SEND)Test plan
forge test --match-path "src/ens/proposals/ep-registrar-manager-endowment/*" -vv— all 43 tests pass