feat: tooling for validator management #347
Conversation
…mpounding vals, consolidation, or exits and run tenderly simulation
📊 Forge Coverage ReportGenerated by workflow run #642 |
![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: 37bf53fa22
ℹ️ 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 args.txns: | ||
| env['TXNS'] = args.txns | ||
| if args.schedule: | ||
| env['SCHEDULE_FILE'] = args.schedule | ||
| if args.execute: |
There was a problem hiding this comment.
When running the forge-based simulation with --schedule/--execute, run_forge_simulation never sets the TXNS environment variable (lines 672-676), so SimulateTransactions.s.sol’s run() immediately fails at vm.envString("TXNS") (line 46) before any transactions execute. This makes the documented timelock workflow (simulate.py --schedule ... --execute ...) unusable in forge mode. Compose TXNS from the schedule/execute/then files when those flags are used.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…g 2: Signing data nonce for batched transactions (AutoCompound.s.sol)
- Reverts for gas consumed more than 10 Mil - State Syncing turned off, because not necessary for our use case
…d generate separate consolidation transactions
…s for auto-compound workflows
… auto-compound script - Added functionality to distribute validators across withdrawal time buckets for optimal consolidation. - Introduced command-line options for bucket size and enabled round-robin distribution. - Enhanced error handling for validators without valid withdrawal credentials.
…in auto-compound script - Implemented a check to ensure --bucket-hours is a positive integer. - Added warning for excluded validators due to missing beacon index, enhancing user feedback on validator selection.
…chars) and full withdrawal credentials (66 chars).
…r each consolidation transaction and update simulation handling for multiple files. Enhanced simulate.py to support comma-separated transaction file inputs.
… to a dedicated 'txns' directory for better organization and clarity in file management.
… transaction scheduling and execution capabilities. Updated _buildTimelockCalldata function to utilize EtherFiTimelock for batch scheduling and execution of transactions.
…ents-tooling-validators-1 Improvements for Compounding Validator Tooling
… in EtherFiNodesManager
…d upgrade functionality
…Limiter setup and address changes
… streamline logging in EtherFiRateLimiter setup
… via Operating Timelock
…n CrossPodApprovalScript for improved ETH consolidation efficiency
…ldata in CrossPodApprovalScript
…nsactions and improve output structure in submarine withdrawal process
…s in submarine withdrawal process
…for pods in submarine withdrawal process
…ions in submarine withdrawal process
…TH withdrawals from pod balances
…ce for improved accuracy
| echo -e "${RED}Error: Linking required but no consolidation files found${NC}" | ||
| exit 1 | ||
| fi | ||
| SIMULATION_EXIT_CODE=$? |
There was a problem hiding this comment.
set -e makes simulation exit code check unreachable
Low Severity
The script uses set -e (line 18), which causes immediate exit on any non-zero command return. The eval "$CMD" calls at lines 191 and 216 would terminate the script before SIMULATION_EXIT_CODE=$? is reached on a failure. Consequently, SIMULATION_EXIT_CODE can only ever be 0, making the error-handling check at line 225 dead code. The intended error message about failed simulations never displays.
Additional Locations (1)
| OUTPUT_FILE="txns.json" \ | ||
| SAFE_NONCE="$NONCE" \ | ||
| forge script script/operations/auto-compound/AutoCompound.s.sol:AutoCompound \ | ||
| --fork-url "$MAINNET_RPC_URL" -vvvv 2>&1 | tee "$OUTPUT_DIR/forge.log" |
There was a problem hiding this comment.
Missing pipefail causes silent forge script failures
High Severity
The forge script ... 2>&1 | tee "$OUTPUT_DIR/forge.log" pipe discards the exit code of forge script because bash captures the exit code of the rightmost command (tee) by default. Without set -o pipefail, if the Forge script fails (compilation error, RPC failure, revert, etc.), the error is silently swallowed. The script then proceeds to move non-existent transaction files (suppressed by || true) and eventually attempts Tenderly simulation with no valid inputs.
| { access = "read-write", path = "./script/operations/exits" }, | ||
| { access = "read-write", path = "./script/operations/utils" }, | ||
| { access = "read", path = "./script/operations/data" }, | ||
| { access = "read", path = "./" } |
There was a problem hiding this comment.
Overly broad read access to project root in foundry.toml
Medium Severity
The fs_permissions entry { access = "read", path = "./" } grants read access to the entire project root for all Forge scripts. This allows any script to read sensitive files like .env (which contains TENDERLY_API_ACCESS_TOKEN, database credentials, and RPC URLs). It also makes all other specific read-only permission entries redundant.
|
|
||
| summary = { | ||
| 'total_targets': len(consolidations), | ||
| 'total_sources': sum(len(c['sources']) for c in consolidations), |
There was a problem hiding this comment.
Summary miscounts sources by including targets in total
Low Severity
The summary's total_sources counts all entries in each consolidation's sources list (including the target validator placed at sources[0]), while the selection loop's total_sources counter only counts actual batch_sources (excluding the target). This inconsistency means the summary over-reports the source count by the number of consolidation batches, potentially confusing operators reviewing the plan.
Additional Locations (1)
| vm.startBroadcast(); | ||
| (bool success, ) = to.call{value: value}(data); | ||
| require(success, "Consolidation transaction failed"); | ||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Broadcast mode double-executes consolidation on fork state
Medium Severity
In broadcast mode, _executeOrWriteTx first executes the consolidation transaction on the fork via vm.prank for gas estimation (line 304), which mutates the fork state (consuming rate limits, spending ETH, submitting requests). It then executes the same transaction again inside vm.startBroadcast (line 315) against the already-modified state. The second execution is likely to fail due to exhausted rate limits or duplicated consolidation requests.
…mounts in both gwei and wei, and improve node address resolution logic
| vm.startBroadcast(); | ||
| (bool success, ) = to.call{value: value}(data); | ||
| require(success, "Consolidation transaction failed"); | ||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Broadcast mode double-executes consolidation on fork, inflating fees
Medium Severity
In broadcast mode, _executeOrWriteTx executes the consolidation call twice on the local fork: once via vm.prank for gas estimation (line 304) and once via vm.startBroadcast (line 315). Since consolidation request fees increase exponentially (EIP-7251), the first execution inflates the on-fork fee state. When subsequent batches call _getConsolidationFee, the returned fee reflects double the expected number of prior requests, causing later transactions to overpay. If the admin and broadcaster are the same address, the prank call also drains ETH from the balance needed for broadcast.
| vm.startBroadcast(); | ||
| (bool success, ) = to.call{value: value}(data); | ||
| require(success, "Consolidation transaction failed"); | ||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Gas estimation call corrupts fork state before broadcast
Medium Severity
In broadcast mode, _executeOrWriteTx executes the consolidation via vm.prank + to.call{value: value}(data) for gas estimation, which modifies the fork state (submits consolidation requests, consumes fees). The subsequent vm.startBroadcast() call then tries the identical operation on the already-modified fork state. Because consolidation request fees increase after each submission (EIP-7251 exponential fee), the broadcast call uses a now-stale fee value that may be insufficient, causing it to revert. The gas estimation needs a vm.snapshot()/vm.revertTo() around it to avoid polluting the fork state.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| function _loadConfig() internal view returns (Config memory config) { | ||
| config.jsonFile = vm.envString("JSON_FILE"); | ||
| config.outputFile = vm.envOr("OUTPUT_FILE", string(DEFAULT_OUTPUT_FILE)); | ||
| config.batchSize = vm.envOr("BATCH_SIZE", DEFAULT_BATCH_SIZE); |
There was a problem hiding this comment.
Documented OUTPUT_FILE and BATCH_SIZE env vars are unused
Medium Severity
config.outputFile and config.batchSize are loaded from OUTPUT_FILE and BATCH_SIZE environment variables but never referenced anywhere else in the code. The README documents these as supported options ("Validators per transaction" and "Output filename"), but setting them has no effect. Output filenames are hardcoded using nonce prefixes, and consolidation transactions are generated one per EigenPod with no sub-batching.
Additional Locations (1)
| vm.startBroadcast(); | ||
| (bool success, ) = to.call{value: value}(data); | ||
| require(success, "Consolidation transaction failed"); | ||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Gas estimation executes state-changing call before broadcast
High Severity
In broadcast mode, the gas estimation at lines 302–306 uses vm.prank + .call which actually executes the consolidation on the fork, modifying state. The subsequent vm.startBroadcast + .call then attempts the same consolidation on already-modified fork state, which will fail because the consolidation was already processed. This makes broadcast mode non-functional.
…t LiquidityPool's total pooled ether
3 participants
generate txns to convert to auto-compounding vals, consolidation, or exits and run tenderly simulation
An eample usage is to generate txns for auto-compounding vals:
if the safe's last nonce was 661.
For details, Refer to
READMENote
Medium Risk
Primarily adds ops tooling and scripts, but it generates/broadcasts mainnet-capable transactions (timelock linking, consolidations, withdrawals), so mistakes in inputs/config or fee/nonce handling could lead to incorrect on-chain operations.
Overview
Introduces a new
script/operationstoolkit to query validators from the operator DB, generate Gnosis Safe JSON transactions for validator auto-compounding (0x01→0x02) and consolidation-to-target workflows, and provide shell wrappers to run the end-to-end flow including optional Tenderly simulation.Adds a large new Forge script (
AutoCompound.s.sol) that detects unlinked validators, optionally generates timelock schedule/execute linking txns, then groups validators by EigenPod (withdrawal credentials) and emits one consolidation txn per pod with Safe nonce-based EIP-712 hash output; adds a separate consolidation generator (ConsolidateToTarget.s.sol) that batches per target, dynamically fetches consolidation fees per tx, optionally broadcasts on mainnet, and can queue post-sweepqueueETHWithdrawalcalls.Updates repo config for these ops flows: extends
foundry.tomlfs_permissions, expands.gitignorefor generated txn/data/log artifacts, refreshesDeployed.s.solwith new mainnet address constants, and includes deployment JSON artifacts forRestakingRewardsRouterproxy/implementation (plus minor selector documentation tweaks inELExitsTransactions.s.sol).Written by Cursor Bugbot for commit 6a0f4d3. This will update automatically on new commits. Configure here.