[H01 | L-20] Validate pending partial withdrawal in unstakePermissionless #1879
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add TypeScript interface for PendingPartialWithdrawal and wrapper function in TestSSZ contract to expose hashTreeRoot functionality. Include test cases for example values, all zeroes, and placeholder for all ones test case. Follows the same pattern as existing BeaconBlockHeader tests.
- Add natspec documentation for nextPow2 function - Add nextPow2 wrapper function to TestMath256 contract - Add comprehensive unit tests covering edge cases, powers of 2, values between powers, and large values including MaxUint256 overflow
…unction - Fix dynamic array memory access by adding 0x20 offset to skip length slot - Add edge case handling for single element arrays (count=1) - Update comments to clarify memory layout and root extraction logic
- Add wrapper function hashTreeRoot_PendingPartialWithdrawalArray in TestSSZ contract - Add early return for empty array case in SSZ hashTreeRoot implementation - Add test cases for empty array and single element scenarios - Comment out additional test cases pending expected value computation
- Add GI_PENDING_PARTIAL_WITHDRAWALS_ROOT immutable variable and constructor parameter to ValidatorContainerProofVerifier - Update TestValidatorContainerProofVerifier constructor to accept and pass new parameter - Update deployment scripts to pass GI_PENDING_PARTIAL_WITHDRAWALS_ROOT - Add PendingPartialWithdrawalsWitness struct and verifyPendingPartialWithdrawals interface method - Add stub implementation for verifyPendingPartialWithdrawals function - Refactor _verifySlot to accept individual parameters instead of witness struct - Fix natspec documentation for _verifySlot function - Update all call sites to use new _verifySlot signature
Complete the verifyPendingPartialWithdrawals function implementation: - Construct GIndex path from state root to pending partial withdrawals root - Verify Merkle proof of pending partial withdrawals against beacon block root - Uses GI_PENDING_PARTIAL_WITHDRAWALS_ROOT to traverse CL state tree
- Add validatorIndex and slot parameters to unstakePermissionless - Track last proven slot per validator in storage - Validate that new slot is newer than last proven slot - Add SlotNotNewerThanLastProvenSlot error - Update natspec documentation for new parameters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…arameters - Extract validatorIndex, slot, childBlockTimestamp, proposerIndex from witness structs - Create BeaconProofWitness struct containing common fields and nested witness structs - Update ValidatorContainerWitness and PendingPartialWithdrawalsWitness to remove common fields - Update verifyActiveValidatorContainer and verifyPendingPartialWithdrawals signatures - Update all implementations and test contracts to match new interface - Fix test harness functions to accept new parameters
- Change SHARD_COMMITTEE_PERIOD and SLOTS_PER_EPOCH from uint256 to uint64 - Change epoch variable in _validateActivationEpoch from uint256 to uint64 - Optimize storage and computation by using appropriate type size
- Calculate total pending withdrawals for validator when computing max unstake amount - Subtract pending partial withdrawals from effective balance in max unstake calculation - Add comments explaining clamping logic for unstake amounts - Ensure unstake amount respects validator's pending withdrawal commitments
- Update interfaces (IYieldManager, IYieldProvider) to use uint64 - Update implementations (YieldManager, LidoStVaultYieldProvider) - Update storage layout mapping types - Update test and mock contracts - Remove unnecessary type casts in LidoStVaultYieldProvider - Add explicit casts in TestYieldManager for type compatibility
…onless - Rename return value to maxUnstakeAmountGwei in _validateUnstakePermissionlessRequest - Move unit conversion from internal function to unstakePermissionless caller - Update documentation to reflect gwei units in internal function - Improves clarity by making unit handling explicit
- Remove local variable declaration that shadows the named return variable - Assign directly to maxUnstakeAmountGwei return variable - Fixes unused variable linter warning
- Add comprehensive NatSpec documentation to bitLength in Math256.sol - Add bitLength wrapper function to TestMath256.sol - Add comprehensive unit tests covering edge cases, powers of 2, and mathematical verification
- Add MAX_PENDING_PARTIAL_WITHDRAWAL_DEPTH constant - Add reference comment to consensus specs for hashTreeRoot function
- Move sha256Pair function from BLS library to SSZ library - Add zeroHash function to SSZ for merkle tree zero hashes - Update all references from BLS12_381.sha256Pair to SSZ.sha256Pair - Remove unused BLS.sol library and TestBLS.sol test contract - Update hashTreeRoot for PendingPartialWithdrawal to use bitLength - Add OutOfRange error for zeroHash bounds checking
- Replace complex assembly-based merkle tree construction with simpler implementation - Add mergeSSZChunk helper function for incremental merkle tree construction - Use zeroHash function for efficient zero node handling - Rename inputLength to count for better clarity - Improve code readability while maintaining same functionality
- Change test case from 1000 to 2000 pending partial withdrawals - Update test name to reflect new withdrawal count
…less Move lastProvenSlot update to after successful unstake operation to prevent slot from being updated if the operation fails. This ensures that failed attempts can be retried with the same slot.
…erProofVerifier tests - Rename ValidatorWitness to ValidatorContainerWitness for consistency - Rename generateValidator to generateValidatorContainer - Reorganize test structure by moving setters tests earlier - Update all references to use new naming convention
- Move validatorIndex, slot, timestamp, and proposerIndex from witness object to function parameters - Simplify validateActivationEpoch to only accept slot and activationEpoch - Update all test calls to match new API signature - Add withdrawalCredentials to generateEIP4478Witness return value - Remove .only from setters test suite - Update validator container references to use direct validator object
…usage - Add sha256Pair function to TestSSZ contract - Move sha256Pair tests from BLS.ts to SSZ.ts for better organization - Remove BLS.ts test file (functionality consolidated in SSZ) - Update executeUnstakePermissionless to use new API signature - Remove multiple yield providers test that used old API - Update integration tests to match new unstakePermissionless signature - Remove unused imports from integration test file
…etic proofs Add test case to verifyPendingPartialWithdrawals that uses synthetic Merkle proofs with random pending partial withdrawals to ensure the function works correctly with generated test data.
Replace post-increment (j++) with pre-increment (++j) in mergeSSZChunk function to save gas on loop iterations.
Replace post-increment operators (i++, j++) with pre-increment (++i, ++j) in for loops across SSZ library and LidoStVaultYieldProvider to save gas on loop iterations. Changes: - SSZ.sol: Update 3 for loops in hashTreeRoot function - LidoStVaultYieldProvider.sol: Update 1 for loop in unstakePermissionless
Wrap loop counter increments in unchecked blocks where overflow is impossible to save gas on each iteration. Changes: - hashTreeRoot(PendingPartialWithdrawal[]): Use unchecked for main processing loop and tree extension loop - mergeSSZChunk: Use unchecked for while loop increment All increments are bounded by array lengths or MAX_PENDING_PARTIAL_WITHDRAWAL_DEPTH (27), making overflow impossible.
…sionless Wrap loop counter increment in unchecked block where overflow is impossible to save gas on each iteration. The loop iterates over pendingPartialWithdrawals array, so the counter is bounded by array length, making overflow impossible.
Cache pendingPartialWithdrawals array reference and length to reduce memory lookups in the loop iteration. Changes: - Cache array reference to avoid repeated nested struct access - Cache array length to avoid repeated length lookups - Add PendingPartialWithdrawal import for type annotation This reduces memory lookups from ~6 per iteration to ~2, saving ~100-200 gas per iteration depending on array size.
kyzooghost
changed the title
…sistency Add defense-in-depth validation to ensure unstaked amount never exceeds required amount, and fix type consistency for slot arithmetic. Changes: - Add UnstakedAmountExceedsRequired error to IYieldManager interface - Validate unstakedAmountWei <= requiredUnstakeAmountWei in unstakePermissionless - Change SLOTS_PER_HISTORICAL_ROOT from uint256 to uint64 for type consistency - Add test case for UnstakedAmountExceedsRequired validation This provides an additional safety check even though the yield provider should clamp the amount, preventing potential issues from rounding or implementation bugs.
Update comment to correctly reference validatorIndex instead of pubkey to match the actual implementation mapping(uint64 validatorIndex => uint64 lastProvenSlot)
Add dedicated toLittleEndian64 function for uint64 values to avoid redundant 256-bit operations. Update call sites in hashTreeRoot functions for BeaconBlockHeader, Validator, and PendingPartialWithdrawal to use the optimized version.
Add comprehensive documentation explaining the optimized uint64 to little-endian conversion, including implementation details, gas efficiency benefits, and SSZ encoding behavior.
Clean up implementation by removing unused commented lines that were left over from development.
Update library header to mention the optimized toLittleEndian64 function for gas-efficient uint64 to little-endian conversion.
- Add revertIfZeroHash utility function to ErrorUtils - Validate GI_FIRST_VALIDATOR and GI_PENDING_PARTIAL_WITHDRAWALS_ROOT are not zero hash in constructor - Add unit tests to verify zero hash validation for both GIndex parameters
fluentcrafter
pushed a commit
that referenced
this pull request
Dec 15, 2025
* fix: general state test config * fix: general state test config step 2 * fix: general state test config step 2 * fix: remove override temp * fix: remove failing balance tests * fix: test diff GOMEMLIMIT * fix: parallelism 3 * fix: back to 2 * fix: 3 is better
- Add zero hash check to setGIFirstValidator setter - Add zero hash check to setGIPendingPartialWithdrawalsRoot setter - Add unit tests to verify setters reject zero hash values
…erkleization Add bounds validation in hashTreeRoot(PendingPartialWithdrawal[]) to reject arrays larger than 2^27 elements, preventing out-of-bounds writes in mergeSSZChunk when depth exceeds MAX_PENDING_PARTIAL_WITHDRAWAL_DEPTH. - Add bounds check: if (count > (1 << MAX_PENDING_PARTIAL_WITHDRAWAL_DEPTH)) - Update documentation to clarify maximum array size and error condition - Add test comment explaining why exact boundary cases can't be tested (JavaScript array size limitations)
Move lastProvenSlot storage update before _delegatecallYieldProvider call to follow checks-effects-interactions pattern and prevent reentrancy vulnerabilities in unstakePermissionless function.
…sionless Cache YieldManagerStorage pointer to avoid repeated _getYieldManagerStorage() calls and inline abi.decode to reduce intermediate variable assignments. Improves gas efficiency and code readability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ValidatorContainerProofVerifier changes
ValidatorContainerProofVerifier::verifyPendingPartialWithdrawals()setGIFirstValidator()andsetGIPendingPartialWithdrawalsRoot()functionsSSZ.sol changes
hashTreeRoot(PendingPartialWithdrawal memory pendingPartialWithdrawal)hashTreeRoot(PendingPartialWithdrawal[] calldata pendingPartialWithdrawal)toLittleEndian64(uint64 v)YieldManager::unstakePermissionless()changesmapping(uint64 validatorIndex -> uint64 slot)lastProvenSlot, with minimum increment of 8192 slotsLidoStVaultYieldProvider::unstakePermissionless()changesNote
Adds pending-partial-withdrawals proof validation and slot monotonicity to permissionless unstake, refactors proof/SSZ logic, and updates APIs, storage, and events.
unstakePermissionlessnow takesvalidatorIndexandslot, enforceslastProvenSlot(+8192) per-validator, computes required unstake, and emitsUnstakePermissionlessRequest.SLOTS_PER_HISTORICAL_ROOTandlastProvenSlotmapping; removes return value fromIYieldManager.unstakePermissionless.unstakePermissionlessderives amount (in gwei) from proofs and returns wei; removes user-provided amounts and provider-level event.pending_partial_withdrawals) and clamps by effective balance minus MIN and pending partials.AccessControl; constructor now accepts admin,GI_FIRST_VALIDATOR, andGI_PENDING_PARTIAL_WITHDRAWALS_ROOT.setGIFirstValidator,setGIPendingPartialWithdrawalsRoot) with events; addverifyPendingPartialWithdrawals; refactor slot/epoch checks.hashTreeRootforPendingPartialWithdrawaland arrays (progressive merkleization),toLittleEndian64,sha256Pair, and zero-hash table; update existing hash roots to 64-bit LE.Math256.bitLength;ErrorUtils.revertIfZeroHash.IYieldProvider.unstakePermissionlesssignature; adjust mocks and tests.Written by Cursor Bugbot for commit 9e171e9. This will update automatically on new commits. Configure here.