Merging Master into Priority Queue#361
Merging Master into Priority Queue#361pankajjagtapp wants to merge 32 commits intopankaj/feat/priority-withdrawal-queuefrom
Conversation
…; add bytecode verification script
…therFiNode, EtherFiRedemptionManager, EtherFiRestaker, EtherFiRewardsRouter, EtherFiViewer, Liquifier, and WithdrawRequestNFT
…erification script
…awRequestNFT constructor argument
…e verification process
…derly VNET limitations
…T_BUYBACK_SAFE address
…e comments based on PR number
…eliable comment detection in coverage reports
…validation logic in WithdrawRequestNFT tests
…nal debug output for better diagnostics
…ters and deployed address
…ithdrawal requests and simulations
…ing of stETH in EigenLayer
…VerifyRestakingRewardsRouterConfig
…ing of stETH in EigenLayer
…-management-scripts stETH management scripts - claimWithdrawals, QueueWithdrawals
…eployed-contracts-json Add deployed contracts json for Reaudited Contracts
…t scripts for improved type safety and clarity
…tions and total pooled ether after withdrawals
… instructions, project layout, architecture, key addresses, access control roles, test setup patterns, and EigenLayer integration
… capturing full output and refining error messages
…HandledReportRefSlot with oracle's lastPublishedReportRefSlot in integration tests
….sol to ensure consistency
…s-for-test-suite Pankaj/test/update fixes for test suite
![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: 9ff8d088ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function getWithdrawRequestNFTImmutableSelectors() internal pure returns (bytes4[] memory selectors) { | ||
| selectors = new bytes4[](1); | ||
| selectors[0] = bytes4(keccak256("treasury()")); |
There was a problem hiding this comment.
This upgrade flow will self-fail because verifyImmutablePreservation() asserts WithdrawRequestNFT.treasury() is unchanged, but the same commit explicitly changes that immutable by deploying WithdrawRequestNFT with WITHDRAW_REQUEST_NFT_BUYBACK_SAFE instead of TREASURY (see deploy-reaudit-fixes.s.sol). On a fork where the proxy still points to the old implementation, this selector comparison always differs after upgradeTo, so the script reverts before completing verification.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 getWithdrawRequestNFTImmutableSelectors() internal pure returns (bytes4[] memory selectors) { | ||
| selectors = new bytes4[](1); | ||
| selectors[0] = bytes4(keccak256("treasury()")); | ||
| } |
There was a problem hiding this comment.
Immutable verification will revert on intentional treasury change
Medium Severity
getWithdrawRequestNFTImmutableSelectors() includes the treasury() selector, but this upgrade intentionally changes WithdrawRequestNFT's treasury immutable from TREASURY to WITHDRAW_REQUEST_NFT_BUYBACK_SAFE. The verifyImmutablePreservation() step will revert when it detects this expected change, blocking all subsequent verification steps (access control checks, fork tests) in the run() function.
Additional Locations (1)
| } | ||
| require(codeSize > 0, string.concat(functionName, ": contract has no code")); | ||
| console2.log(string.concat("[FUNC OK] ", functionName, ": selector exists")); | ||
| } |
There was a problem hiding this comment.
Function claims to verify selector but only checks code size
Low Severity
verifyFunctionExists performs a staticcall with the selector but discards the success result. It only checks extcodesize > 0, which passes for any contract regardless of whether it implements the given selector. The function name and [FUNC OK] log message are misleading — it provides a false sense of verification. It's also unused anywhere in the codebase.
1 participant
Note
Medium Risk
Touches upgrade/ops scripts and deployment metadata (addresses, Safe tx payloads) plus test/fork logic; while mostly non-production code, mistakes could mislead upgrade execution or hide failing coverage/tests.
Overview
Improves CI coverage reporting by making
forge coverageoutput capture/parsing more robust, uploading it as an artifact, and updating/creating a single PR comment via a hidden marker with better error handling/logging.Adds/updates operational and upgrade tooling: new stETH management scripts (request/claim/unstake flows on fork), expanded
Utilshelpers (immutable/access-control snapshot checks and Gnosis Safe JSON generation), and updated upgrade/deployment scripts and logs (including switchingWithdrawRequestNFTconstructor treasury to a new buyback safe and recording new impl addresses/tx JSONs).Stabilizes mainnet-fork tests by allowing
FORK_RPC_URLoverrides, skipping EIP-4788-dependent tests when beacon roots aren’t available, adjusting assertions to account for existing mainnet state (pending redemptions), and adding fork-state fixes (oracle report sync, redemption low-watermark tweaks, EigenPod withdrawable balance setup, role/prank fixes).Written by Cursor Bugbot for commit 9ff8d08. This will update automatically on new commits. Configure here.