diff --git a/src/contracts/interfaces/IDurationVaultStrategy.sol b/src/contracts/interfaces/IDurationVaultStrategy.sol index 4acfaf19fe..3adc8ed043 100644 --- a/src/contracts/interfaces/IDurationVaultStrategy.sol +++ b/src/contracts/interfaces/IDurationVaultStrategy.sol @@ -122,9 +122,20 @@ interface IDurationVaultStrategy is function lock() external; /// @notice Marks the vault as matured once the configured duration has elapsed. - /// @dev Transitions state from ALLOCATIONS to WITHDRAWALS. Deallocates from the operator set - /// and deregisters the vault. After maturation, withdrawals are permitted while deposits - /// remain disabled. Callable by anyone once the duration has elapsed. + /// @dev Transitions state from ALLOCATIONS to WITHDRAWALS. + /// + /// Best-effort operator cleanup: + /// - Attempts to deallocate from the configured operator set and deregister the vault as an operator. + /// - These external calls are intentionally best-effort so `markMatured()` cannot be bricked and user + /// withdrawals cannot be indefinitely locked. + /// + /// Common reasons deallocation/deregistration can fail include: + /// - AllocationManager pausing allocations/deallocations or register/deregister operations. + /// - AVS-side registrar rejecting deregistration (e.g., operator removed/ejected from an operator set). + /// - AllocationManager state constraints (e.g., pending allocation modification preventing an update). + /// + /// After maturation, withdrawals are permitted while deposits remain disabled. Callable by anyone once + /// the duration has elapsed. function markMatured() external; /// @notice Updates the vault metadata URI. diff --git a/src/contracts/strategies/DurationVaultStrategy.sol b/src/contracts/strategies/DurationVaultStrategy.sol index a698e7a440..708fa1b09a 100644 --- a/src/contracts/strategies/DurationVaultStrategy.sol +++ b/src/contracts/strategies/DurationVaultStrategy.sol @@ -316,7 +316,7 @@ contract DurationVaultStrategy is DurationVaultStrategyStorage, StrategyBase { } /// @notice Deallocates all magnitude from the configured operator set. - /// @dev Best-effort: failures are caught to avoid bricking `markMatured()`. + /// @dev Best-effort: failures are ignored to avoid bricking `markMatured()`. function _deallocateAll() internal { IAllocationManager.AllocateParams[] memory params = new IAllocationManager.AllocateParams[](1); params[0].operatorSet = _operatorSet; @@ -325,11 +325,14 @@ contract DurationVaultStrategy is DurationVaultStrategyStorage, StrategyBase { params[0].newMagnitudes = new uint64[](1); params[0].newMagnitudes[0] = 0; // This call is best-effort: failures should not brick `markMatured()` and lock user funds. - try allocationManager.modifyAllocations(address(this), params) {} catch {} + // We use a low-level call instead of try/catch to avoid wallet gas-estimation pitfalls. + (bool success,) = address(allocationManager) + .call(abi.encodeWithSelector(IAllocationManagerActions.modifyAllocations.selector, address(this), params)); + success; // suppress unused variable warning } /// @notice Deregisters the vault from its configured operator set. - /// @dev Best-effort: failures are caught to avoid bricking `markMatured()`. + /// @dev Best-effort: failures are ignored to avoid bricking `markMatured()`. function _deregisterFromOperatorSet() internal { IAllocationManager.DeregisterParams memory params; params.operator = address(this); @@ -337,6 +340,9 @@ contract DurationVaultStrategy is DurationVaultStrategyStorage, StrategyBase { params.operatorSetIds = new uint32[](1); params.operatorSetIds[0] = _operatorSet.id; // This call is best-effort: failures should not brick `markMatured()` and lock user funds. - try allocationManager.deregisterFromOperatorSets(params) {} catch {} + // We use a low-level call instead of try/catch to avoid wallet gas-estimation pitfalls. + (bool success,) = address(allocationManager) + .call(abi.encodeWithSelector(IAllocationManagerActions.deregisterFromOperatorSets.selector, params)); + success; // suppress unused variable warning } } diff --git a/src/test/mocks/AllocationManagerMock.sol b/src/test/mocks/AllocationManagerMock.sol index 06670e4530..eedbc5993a 100644 --- a/src/test/mocks/AllocationManagerMock.sol +++ b/src/test/mocks/AllocationManagerMock.sol @@ -60,6 +60,17 @@ contract AllocationManagerMock is Test { uint public modifyAllocationsCallCount; uint public deregisterFromOperatorSetsCallCount; + bool public revertModifyAllocations; + bool public revertDeregisterFromOperatorSets; + + function setRevertModifyAllocations(bool shouldRevert) external { + revertModifyAllocations = shouldRevert; + } + + function setRevertDeregisterFromOperatorSets(bool shouldRevert) external { + revertDeregisterFromOperatorSets = shouldRevert; + } + function getSlashCount(OperatorSet memory operatorSet) external view returns (uint) { return _getSlashCount[operatorSet.key()]; } @@ -211,6 +222,7 @@ contract AllocationManagerMock is Test { } function modifyAllocations(address operator, IAllocationManager.AllocateParams[] calldata params) external { + if (revertModifyAllocations) revert("AllocationManagerMock: modifyAllocations reverted"); modifyAllocationsCallCount++; delete _lastAllocateCall; _lastAllocateCall.operator = operator; @@ -241,6 +253,7 @@ contract AllocationManagerMock is Test { } function deregisterFromOperatorSets(IAllocationManager.DeregisterParams calldata params) external { + if (revertDeregisterFromOperatorSets) revert("AllocationManagerMock: deregisterFromOperatorSets reverted"); deregisterFromOperatorSetsCallCount++; _lastDeregisterCall.operator = params.operator; _lastDeregisterCall.avs = params.avs; diff --git a/src/test/unit/DurationVaultStrategyUnit.t.sol b/src/test/unit/DurationVaultStrategyUnit.t.sol index 2dfa390ab2..ccc6c245f6 100644 --- a/src/test/unit/DurationVaultStrategyUnit.t.sol +++ b/src/test/unit/DurationVaultStrategyUnit.t.sol @@ -149,6 +149,25 @@ contract DurationVaultStrategyUnitTests is StrategyBaseUnitTests { assertFalse(durationVault.operatorSetRegistered(), "operator set should be deregistered"); } + function testMarkMaturedBestEffortWhenAllocationManagerReverts() public { + durationVault.lock(); + cheats.warp(block.timestamp + defaultDuration + 1); + + allocationManagerMock.setRevertModifyAllocations(true); + allocationManagerMock.setRevertDeregisterFromOperatorSets(true); + + // Should not revert even if AllocationManager refuses deallocation/deregistration. + durationVault.markMatured(); + + assertTrue(durationVault.withdrawalsOpen(), "withdrawals must open after maturity"); + assertFalse(durationVault.allocationsActive(), "allocations should be inactive after maturity"); + assertFalse(durationVault.operatorSetRegistered(), "operator set should be unregistered after maturity"); + + // Since the mock reverts before incrementing, only the initial lock allocation is recorded. + assertEq(allocationManagerMock.modifyAllocationsCallCount(), 1, "unexpected modifyAllocations count"); + assertEq(allocationManagerMock.deregisterFromOperatorSetsCallCount(), 0, "unexpected deregister count"); + } + // ===================== VAULT STATE TESTS ===================== function testDepositsBlockedAfterLock() public {