From b488be3f54528706df9fbc6eb269d35de605285f Mon Sep 17 00:00:00 2001 From: nksazonov Date: Tue, 18 Mar 2025 15:42:19 +0200 Subject: [PATCH 01/11] feat(vault): add CooldownAuthorizer almost finished proposal --- CooldownAuthorizerFeatureOverview.md | 79 ++++++++++ src/vault/CooldownAuthorizer.sol | 226 +++++++++++++++++++++++++++ 2 files changed, 305 insertions(+) create mode 100644 CooldownAuthorizerFeatureOverview.md create mode 100644 src/vault/CooldownAuthorizer.sol diff --git a/CooldownAuthorizerFeatureOverview.md b/CooldownAuthorizerFeatureOverview.md new file mode 100644 index 0000000..257c518 --- /dev/null +++ b/CooldownAuthorizerFeatureOverview.md @@ -0,0 +1,79 @@ +# CooldownAuthorizer Feature Overview + +## Overview + +The CooldownAuthorizer implements a time-based withdrawal mechanism for the LiteVault contract that requires users to wait for a specified cooldown period before completing their withdrawals. Key features include: + +1. **Configurable Cooldown Periods**: Multiple cooldown duration options can be enabled/disabled by the contract owner, allowing for flexible withdrawal policies. + +2. **User-selected Cooldown**: When requesting a withdrawal, users can select from supported cooldown periods, offering a balance between security and convenience. + +3. **Immutable Requests**: Once a withdrawal is requested, it can not be affected by the cooldown period modification, ensuring predictable withdrawal timelines. + +4. **Cooldown Enforcement**: The `authorize` function verifies that the cooldown period has elapsed before allowing a withdrawal to proceed. + +5. **Graceful Administration**: Owner can add or remove supported cooldown periods without affecting existing withdrawal requests, as their originally selected cooldown period remains valid regardless of later changes. + +The core workflow is: + +- User calls `requestWithdrawalWithCooldown` with token, amount, and desired cooldown period +- System emits a `CooldownRequested` event and stores request details +- After the cooldown period elapses, a withdrawal from LiteVault will be authorized +- If attempted before cooldown expiry, the withdrawal is rejected + +NOTE: Having implemented this, it is still not possible to disallow depositing while the cooldown is active. + +## Integration plan + +1. Separate the unlock and withdraw processes on FE +2. Change FE to support users specifying a cooldown period during withdrawal request +3. Make a transaction to CooldownAuthorizer on FE +4. Deploy CooldownAuthorizer contract on 8 currently supported chains +5. Set the LiteVault authorizer to the CooldownAuthorizer contract on the 8 chains + +NOTE: possibly no changes to Pathfinder (BE) unless we want to store the cooldown period and/or withdrawal request info. + +NOTE: if we want to add different logic to different cooldown periods, we NEED the Pathfinder to listen to withdrawal requests and update the logic as we need. + +## Withdrawal request double-spending problem + +### Problem Statement + +We need a mechanism to outdate withdrawal requests after they have been fulfilled. However, the `authorize` function in the `IAuthorize` interface is `view` so it's not possible to update the state. + +The vault is already deployed and we can't change it. We could have added some post-withdrawal call logic to the vault, but this is no longer an option. + +The main reason why the withdrawal request needs to be invalidated is that a user may deposit more funds to the LiteVault during the cooldown period, and request 2 consecutive withdrawals. + +The first one will succeed as designed, but the second one will also succeed because the withdrawal request is not invalidated and user can still withdraw this token. + +A timeout approach would only work if the difference between the first withdrawal and the second is greater than this timeout, which we cannot assume. + +### Possible Solutions + +#### 1. User-initiated request refresh + +- Add function for users to explicitly refresh/update withdrawal requests +- Requires user understanding of the extra step + +#### 2. Request with specific identifiers + +- Track multiple withdrawal requests with unique IDs +- Would require LiteVault modification (not possible) + +#### 3. Time-bounded requests + +- Add validity period to requests +- Requests expire after cooldown + short window +- Potential UX issues if window too short + +#### 4. Interface modification + +- Modify IAuthorize to have non-view authorize function +- Breaking change requiring new vault deployment + +### Recommendation + +The ideal solution would be creating a new IAuthorize2 interface with a non-view authorize function, then deploying a new version of LiteVault that uses this interface. + +If that's not possible, the user-initiated request refresh mechanism (Option 1) provides the most practical solution within the current constraints, though it requires users to understand the extra withdrawal request step whenever they want to withdraw again after depositing more funds. diff --git a/src/vault/CooldownAuthorizer.sol b/src/vault/CooldownAuthorizer.sol new file mode 100644 index 0000000..15e518d --- /dev/null +++ b/src/vault/CooldownAuthorizer.sol @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import "../interfaces/IAuthorize.sol"; +import "@openzeppelin/contracts/access/Ownable2Step.sol"; + +/** + * @title CooldownAuthorizer + * @notice Authorizer contract that enforces a cooldown period before withdrawals. + * @dev Users must request a withdrawal which starts a cooldown period. + * After the cooldown period has passed, the withdrawal is authorized. + * Supports multiple cooldown periods that can be enabled/disabled by the owner. + */ +contract CooldownAuthorizer is IAuthorize, Ownable2Step { + /** + * @notice Struct to store withdrawal request information. + * @param amount The amount of tokens requested for withdrawal. + * @param requestTimestamp The timestamp when the withdrawal was requested. + * @param cooldownPeriod The cooldown period chosen for this withdrawal request. + */ + struct WithdrawalRequest { + uint256 amount; + uint64 requestTimestamp; + uint64 cooldownPeriod; + } + + /** + * @notice Error thrown when the withdrawal amount exceeds the requested amount. + * @param requested The amount that was requested for withdrawal. + * @param attempted The amount that was attempted to be withdrawn. + */ + error ExceedsRequestedAmount(uint256 requested, uint256 attempted); + + /** + * @notice Error thrown when the cooldown period has not yet passed. + * @param requestTimestamp The timestamp when the withdrawal was requested. + * @param currentTimestamp The current timestamp. + * @param cooldownPeriod The required cooldown period. + */ + error CooldownNotExpired( + uint64 requestTimestamp, + uint64 currentTimestamp, + uint64 cooldownPeriod + ); + + /** + * @notice Error thrown when the withdrawal has not been requested. + * @param user The address of the user. + * @param token The address of the token. + */ + error WithdrawalNotRequested(address user, address token); + + /** + * @notice Error thrown when the requested cooldown period is not supported. + * @param cooldownPeriod The cooldown period that was requested. + */ + error UnsupportedCooldownPeriod(uint64 cooldownPeriod); + + /** + * @notice Error thrown when the cooldown period is invalid. + */ + error InvalidCooldownPeriod(); + + /** + * @notice Event emitted when a withdrawal is requested. + * @param user The address of the user requesting the withdrawal. + * @param token The address of the token to withdraw. + * @param amount The amount of tokens to withdraw. + * @param cooldownPeriod The cooldown period chosen for this withdrawal request. + */ + event CooldownRequested( + address indexed user, + address indexed token, + uint256 amount, + uint64 cooldownPeriod + ); + + /** + * @notice Event emitted when a cooldown period's status is updated. + * @param cooldownPeriod The cooldown period that was updated. + * @param isSupported Whether the cooldown period is now supported. + */ + event CooldownPeriodStatusChanged(uint64 cooldownPeriod, bool isSupported); + + // Mapping of cooldown period to whether it is supported + mapping(uint64 cooldownPeriod => bool isSupported) + private _supportedCooldownPeriods; + + // Mapping of user address to token address to withdrawal request + mapping(address user => mapping(address token => WithdrawalRequest request)) + private _withdrawalRequests; + + /** + * @dev Constructor sets the initial owner of the contract and enables the provided cooldown periods. + * @param owner The address of the owner. + * @param supportedCooldownPeriods Array of cooldown periods to be initially supported. + */ + constructor( + address owner, + uint64[] memory supportedCooldownPeriods + ) Ownable(owner) { + for (uint256 i = 0; i < supportedCooldownPeriods.length; i++) { + uint64 period = supportedCooldownPeriods[i]; + require(period > 0, InvalidCooldownPeriod()); + _supportedCooldownPeriods[period] = true; + emit CooldownPeriodStatusChanged(period, true); + } + } + + /** + * @notice Checks if a cooldown period is supported. + * @param cooldownPeriod The cooldown period to check. + * @return True if the cooldown period is supported, false otherwise. + */ + function isCooldownPeriodSupported( + uint64 cooldownPeriod + ) external view returns (bool) { + return _supportedCooldownPeriods[cooldownPeriod]; + } + + /** + * @notice Get the withdrawal request details for a user and token. + * @param user The address of the user. + * @param token The address of the token. + * @return amount The amount of tokens requested for withdrawal. + * @return requestTimestamp The timestamp when the withdrawal was requested. + * @return cooldownPeriod The cooldown period chosen for this withdrawal request. + */ + function getWithdrawalRequest( + address user, + address token + ) + external + view + returns (uint256 amount, uint64 requestTimestamp, uint64 cooldownPeriod) + { + WithdrawalRequest memory request = _withdrawalRequests[user][token]; + return ( + request.amount, + request.requestTimestamp, + request.cooldownPeriod + ); + } + + /** + * @notice Check if a withdrawal is authorized. + * @dev Returns true if the cooldown period has passed since the withdrawal request. + * @param owner The address of the token owner. + * @param token The address of the token. + * @param amount The amount of tokens to be withdrawn. + * @return True if the withdrawal is authorized, false otherwise. + */ + function authorize( + address owner, + address token, + uint256 amount + ) external view override returns (bool) { + WithdrawalRequest memory request = _withdrawalRequests[owner][token]; + + // Check if withdrawal was requested + require( + request.requestTimestamp != 0, + WithdrawalNotRequested(owner, token) + ); + + // Check if amount is less than or equal to requested amount + require( + amount <= request.amount, + ExceedsRequestedAmount(request.amount, amount) + ); + + // Check if cooldown period has passed + // Note: We don't check if the cooldown period is still supported + require( + uint64(block.timestamp) >= + request.requestTimestamp + request.cooldownPeriod, + CooldownNotExpired( + request.requestTimestamp, + uint64(block.timestamp), + request.cooldownPeriod + ) + ); + + return true; + } + + /** + * @notice Updates the status of a cooldown period. + * @param cooldownPeriod The cooldown period to update. + * @param isSupported Whether the cooldown period should be supported. + */ + function setCooldownPeriodStatus( + uint64 cooldownPeriod, + bool isSupported + ) external onlyOwner { + require(cooldownPeriod > 0, InvalidCooldownPeriod()); + _supportedCooldownPeriods[cooldownPeriod] = isSupported; + emit CooldownPeriodStatusChanged(cooldownPeriod, isSupported); + } + + /** + * @notice Request a withdrawal for a specific token and amount with a specific cooldown period. + * @dev Emits a CooldownRequested event. + * @param token The address of the token to withdraw. + * @param amount The amount of tokens to withdraw. + * @param cooldownPeriod The cooldown period to use for this withdrawal request. + */ + function requestWithdrawalWithCooldown( + address token, + uint256 amount, + uint64 cooldownPeriod + ) public { + require( + _supportedCooldownPeriods[cooldownPeriod], + UnsupportedCooldownPeriod(cooldownPeriod) + ); + + _withdrawalRequests[msg.sender][token] = WithdrawalRequest({ + amount: amount, + requestTimestamp: uint64(block.timestamp), + cooldownPeriod: cooldownPeriod + }); + + emit CooldownRequested(msg.sender, token, amount, cooldownPeriod); + } +} From 2b4c28ca8f16a1dc455eba1fa2385503789f097b Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 21 Mar 2025 12:30:39 +0200 Subject: [PATCH 02/11] feat(liteVault): add todo comments for future version --- src/vault/LiteVault.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/vault/LiteVault.sol b/src/vault/LiteVault.sol index 2aef9f1..b44565e 100644 --- a/src/vault/LiteVault.sol +++ b/src/vault/LiteVault.sol @@ -74,6 +74,9 @@ contract LiteVault is IVault, IAuthorizable, ReentrancyGuard, Ownable2Step { emit AuthorizerChanged(newAuthorizer); } + // TODO: add `customData` as parameter + // TODO: add a call to authorizer for better flexibility + // TODO: change `msg.sender` to `account` for more flexibility /** * @dev Deposits tokens or ETH into the vault. * @param token The address of the token to deposit. Use address(0) for ETH. @@ -92,6 +95,7 @@ contract LiteVault is IVault, IAuthorizable, ReentrancyGuard, Ownable2Step { emit Deposited(msg.sender, token, amount); } + // TODO: add `customData` as parameter /** * @dev Withdraws tokens or ETH from the vault. * @param token The address of the token to withdraw. Use address(0) for ETH. @@ -105,6 +109,7 @@ contract LiteVault is IVault, IAuthorizable, ReentrancyGuard, Ownable2Step { if ( !_isWithdrawalGracePeriodActive( latestSetAuthorizerTimestamp, uint64(block.timestamp), WITHDRAWAL_GRACE_PERIOD + // TODO: change method signature to pass `data` as a parameter, that an authorizer can decode and make a decision ) && !authorizer.authorize(msg.sender, token, amount) ) { revert IAuthorize.Unauthorized(msg.sender, token, amount); From 8a96b0f8c43134af50448abca468d391ddab5779 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 21 Mar 2025 13:37:43 +0200 Subject: [PATCH 03/11] feat: UnbondingPeriodAuthorizer --- lib/forge-std | 2 +- src/interfaces/IAuthorize.sol | 4 +- src/vault/CooldownAuthorizer.sol | 226 --------------------- src/vault/UnbondingPeriodAuthorizer.sol | 253 ++++++++++++++++++++++++ 4 files changed, 257 insertions(+), 228 deletions(-) delete mode 100644 src/vault/CooldownAuthorizer.sol create mode 100644 src/vault/UnbondingPeriodAuthorizer.sol diff --git a/lib/forge-std b/lib/forge-std index e04104a..4d63c97 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit e04104ab93e771441eab03fb76eda1402cb5927b +Subproject commit 4d63c978718517fa02d4e330fbe7372dbb06c2f1 diff --git a/src/interfaces/IAuthorize.sol b/src/interfaces/IAuthorize.sol index 3effd68..455b38b 100644 --- a/src/interfaces/IAuthorize.sol +++ b/src/interfaces/IAuthorize.sol @@ -14,6 +14,8 @@ interface IAuthorize { */ error Unauthorized(address user, address token, uint256 amount); + // NOTE: `view` modifier was removed to allow for better flexibility of authorizer contracts. + // On the other hand, Vault logic has not been changed to allow for compatibility with already deployed contracts. /** * @dev Authorizes actions based on the owner, token, and amount. * @param owner The address of the token owner. @@ -21,5 +23,5 @@ interface IAuthorize { * @param amount The amount of tokens to be authorized. * @return True if the action is authorized, false otherwise. */ - function authorize(address owner, address token, uint256 amount) external view returns (bool); + function authorize(address owner, address token, uint256 amount) external returns (bool); } diff --git a/src/vault/CooldownAuthorizer.sol b/src/vault/CooldownAuthorizer.sol deleted file mode 100644 index 15e518d..0000000 --- a/src/vault/CooldownAuthorizer.sol +++ /dev/null @@ -1,226 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.26; - -import "../interfaces/IAuthorize.sol"; -import "@openzeppelin/contracts/access/Ownable2Step.sol"; - -/** - * @title CooldownAuthorizer - * @notice Authorizer contract that enforces a cooldown period before withdrawals. - * @dev Users must request a withdrawal which starts a cooldown period. - * After the cooldown period has passed, the withdrawal is authorized. - * Supports multiple cooldown periods that can be enabled/disabled by the owner. - */ -contract CooldownAuthorizer is IAuthorize, Ownable2Step { - /** - * @notice Struct to store withdrawal request information. - * @param amount The amount of tokens requested for withdrawal. - * @param requestTimestamp The timestamp when the withdrawal was requested. - * @param cooldownPeriod The cooldown period chosen for this withdrawal request. - */ - struct WithdrawalRequest { - uint256 amount; - uint64 requestTimestamp; - uint64 cooldownPeriod; - } - - /** - * @notice Error thrown when the withdrawal amount exceeds the requested amount. - * @param requested The amount that was requested for withdrawal. - * @param attempted The amount that was attempted to be withdrawn. - */ - error ExceedsRequestedAmount(uint256 requested, uint256 attempted); - - /** - * @notice Error thrown when the cooldown period has not yet passed. - * @param requestTimestamp The timestamp when the withdrawal was requested. - * @param currentTimestamp The current timestamp. - * @param cooldownPeriod The required cooldown period. - */ - error CooldownNotExpired( - uint64 requestTimestamp, - uint64 currentTimestamp, - uint64 cooldownPeriod - ); - - /** - * @notice Error thrown when the withdrawal has not been requested. - * @param user The address of the user. - * @param token The address of the token. - */ - error WithdrawalNotRequested(address user, address token); - - /** - * @notice Error thrown when the requested cooldown period is not supported. - * @param cooldownPeriod The cooldown period that was requested. - */ - error UnsupportedCooldownPeriod(uint64 cooldownPeriod); - - /** - * @notice Error thrown when the cooldown period is invalid. - */ - error InvalidCooldownPeriod(); - - /** - * @notice Event emitted when a withdrawal is requested. - * @param user The address of the user requesting the withdrawal. - * @param token The address of the token to withdraw. - * @param amount The amount of tokens to withdraw. - * @param cooldownPeriod The cooldown period chosen for this withdrawal request. - */ - event CooldownRequested( - address indexed user, - address indexed token, - uint256 amount, - uint64 cooldownPeriod - ); - - /** - * @notice Event emitted when a cooldown period's status is updated. - * @param cooldownPeriod The cooldown period that was updated. - * @param isSupported Whether the cooldown period is now supported. - */ - event CooldownPeriodStatusChanged(uint64 cooldownPeriod, bool isSupported); - - // Mapping of cooldown period to whether it is supported - mapping(uint64 cooldownPeriod => bool isSupported) - private _supportedCooldownPeriods; - - // Mapping of user address to token address to withdrawal request - mapping(address user => mapping(address token => WithdrawalRequest request)) - private _withdrawalRequests; - - /** - * @dev Constructor sets the initial owner of the contract and enables the provided cooldown periods. - * @param owner The address of the owner. - * @param supportedCooldownPeriods Array of cooldown periods to be initially supported. - */ - constructor( - address owner, - uint64[] memory supportedCooldownPeriods - ) Ownable(owner) { - for (uint256 i = 0; i < supportedCooldownPeriods.length; i++) { - uint64 period = supportedCooldownPeriods[i]; - require(period > 0, InvalidCooldownPeriod()); - _supportedCooldownPeriods[period] = true; - emit CooldownPeriodStatusChanged(period, true); - } - } - - /** - * @notice Checks if a cooldown period is supported. - * @param cooldownPeriod The cooldown period to check. - * @return True if the cooldown period is supported, false otherwise. - */ - function isCooldownPeriodSupported( - uint64 cooldownPeriod - ) external view returns (bool) { - return _supportedCooldownPeriods[cooldownPeriod]; - } - - /** - * @notice Get the withdrawal request details for a user and token. - * @param user The address of the user. - * @param token The address of the token. - * @return amount The amount of tokens requested for withdrawal. - * @return requestTimestamp The timestamp when the withdrawal was requested. - * @return cooldownPeriod The cooldown period chosen for this withdrawal request. - */ - function getWithdrawalRequest( - address user, - address token - ) - external - view - returns (uint256 amount, uint64 requestTimestamp, uint64 cooldownPeriod) - { - WithdrawalRequest memory request = _withdrawalRequests[user][token]; - return ( - request.amount, - request.requestTimestamp, - request.cooldownPeriod - ); - } - - /** - * @notice Check if a withdrawal is authorized. - * @dev Returns true if the cooldown period has passed since the withdrawal request. - * @param owner The address of the token owner. - * @param token The address of the token. - * @param amount The amount of tokens to be withdrawn. - * @return True if the withdrawal is authorized, false otherwise. - */ - function authorize( - address owner, - address token, - uint256 amount - ) external view override returns (bool) { - WithdrawalRequest memory request = _withdrawalRequests[owner][token]; - - // Check if withdrawal was requested - require( - request.requestTimestamp != 0, - WithdrawalNotRequested(owner, token) - ); - - // Check if amount is less than or equal to requested amount - require( - amount <= request.amount, - ExceedsRequestedAmount(request.amount, amount) - ); - - // Check if cooldown period has passed - // Note: We don't check if the cooldown period is still supported - require( - uint64(block.timestamp) >= - request.requestTimestamp + request.cooldownPeriod, - CooldownNotExpired( - request.requestTimestamp, - uint64(block.timestamp), - request.cooldownPeriod - ) - ); - - return true; - } - - /** - * @notice Updates the status of a cooldown period. - * @param cooldownPeriod The cooldown period to update. - * @param isSupported Whether the cooldown period should be supported. - */ - function setCooldownPeriodStatus( - uint64 cooldownPeriod, - bool isSupported - ) external onlyOwner { - require(cooldownPeriod > 0, InvalidCooldownPeriod()); - _supportedCooldownPeriods[cooldownPeriod] = isSupported; - emit CooldownPeriodStatusChanged(cooldownPeriod, isSupported); - } - - /** - * @notice Request a withdrawal for a specific token and amount with a specific cooldown period. - * @dev Emits a CooldownRequested event. - * @param token The address of the token to withdraw. - * @param amount The amount of tokens to withdraw. - * @param cooldownPeriod The cooldown period to use for this withdrawal request. - */ - function requestWithdrawalWithCooldown( - address token, - uint256 amount, - uint64 cooldownPeriod - ) public { - require( - _supportedCooldownPeriods[cooldownPeriod], - UnsupportedCooldownPeriod(cooldownPeriod) - ); - - _withdrawalRequests[msg.sender][token] = WithdrawalRequest({ - amount: amount, - requestTimestamp: uint64(block.timestamp), - cooldownPeriod: cooldownPeriod - }); - - emit CooldownRequested(msg.sender, token, amount, cooldownPeriod); - } -} diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol new file mode 100644 index 0000000..8d4d025 --- /dev/null +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import "../interfaces/IAuthorize.sol"; +import "@openzeppelin/contracts/access/Ownable2Step.sol"; +import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; + +/** + * @title UnbondingPeriodAuthorizer + * @notice Authorizer contract that enforces an unbonding period before withdrawals. + * @dev Users must request a withdrawal which starts an unbonding period. + * After the unbonding period has passed, the withdrawal is authorized. + * Supports multiple unbonding periods that can be enabled/disabled by the owner. + */ +contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { + // Use EnumerableSet for tracking supported unbonding periods + using EnumerableSet for EnumerableSet.UintSet; + /** + * @notice Struct to store withdrawal request information. + * @param requestTimestamp The timestamp when the withdrawal was requested. + * @param unbondingPeriod The unbonding period chosen for this withdrawal request. + */ + struct UnbondingRequest { + uint64 requestTimestamp; + uint64 unbondingPeriod; + } + + /** + * @notice Error thrown when the unbonding period has not yet passed. + * @param requestTimestamp The timestamp when the withdrawal was requested. + * @param currentTimestamp The current timestamp. + * @param unbondingPeriod The required unbonding period. + */ + error UnbondingPeriodNotExpired( + uint64 requestTimestamp, + uint64 currentTimestamp, + uint64 unbondingPeriod + ); + + /** + * @notice Error thrown when the withdrawal has not been requested. + * @param user The address of the user. + * @param token The address of the token. + */ + error UnbondingNotRequested(address user, address token); + + /** + * @notice Error thrown when the requested unbonding period is not supported. + * @param unbondingPeriod The unbonding period that was requested. + */ + error UnsupportedUnbondingPeriod(uint64 unbondingPeriod); + + /** + * @notice Error thrown when the unbonding period is invalid. + */ + error InvalidUnbondingPeriod(); + + /** + * @notice Event emitted when a withdrawal is requested. + * @param user The address of the user requesting the withdrawal. + * @param token The address of the token to withdraw. + * @param unbondingPeriod The unbonding period chosen for this withdrawal request. + */ + event UnbondingRequested( + address indexed user, + address indexed token, + uint64 unbondingPeriod + ); + + /** + * @notice Event emitted when an unbonding period has passed and the withdrawal is authorized. + * @param user The address of the user completing the withdrawal. + * @param token The address of the token being withdrawn. + */ + event UnbondingCompleted(address indexed user, address indexed token); + + /** + * @notice Event emitted when an unbonding period's status is updated. + * @param unbondingPeriod The unbonding period that was updated. + * @param isSupported Whether the unbonding period is now supported. + */ + event UnbondingPeriodStatusChanged(uint64 unbondingPeriod, bool isSupported); + + // Set of all supported unbonding periods + EnumerableSet.UintSet internal _supportedUnbondingPeriods; + + // Mapping of user address to token address to withdrawal request + mapping(address user => mapping(address token => UnbondingRequest request)) + internal _unbondingRequests; + + /** + * @dev Constructor sets the initial owner of the contract and enables the provided unbonding periods. + * @param owner The address of the owner. + * @param supportedUnbondingPeriods Array of unbonding periods to be initially supported. + */ + constructor( + address owner, + uint64[] memory supportedUnbondingPeriods + ) Ownable(owner) { + for (uint256 i = 0; i < supportedUnbondingPeriods.length; i++) { + uint64 period = supportedUnbondingPeriods[i]; + require(period > 0, InvalidUnbondingPeriod()); + _supportedUnbondingPeriods.add(period); + emit UnbondingPeriodStatusChanged(period, true); + } + } + + /** + * @notice Checks if an unbonding period is supported. + * @param unbondingPeriod The unbonding period to check. + * @return True if the unbonding period is supported, false otherwise. + */ + function isUnbondingPeriodSupported( + uint64 unbondingPeriod + ) external view returns (bool) { + return _supportedUnbondingPeriods.contains(unbondingPeriod); + } + + /** + * @notice Get all supported unbonding periods. + * @return An array of all supported unbonding periods. + */ + function getAllSupportedUnbondingPeriods() external view returns (uint64[] memory) { + uint256 totalPeriods = _supportedUnbondingPeriods.length(); + uint64[] memory result = new uint64[](totalPeriods); + + for (uint256 i = 0; i < totalPeriods; i++) { + result[i] = uint64(_supportedUnbondingPeriods.at(i)); + } + + return result; + } + + /** + * @notice Get the withdrawal request details for a user and token. + * @param user The address of the user. + * @param token The address of the token. + * @return requestTimestamp The timestamp when the withdrawal was requested. + * @return unbondingPeriod The unbonding period chosen for this withdrawal request. + */ + function getUnbondingRequest( + address user, + address token + ) + external + view + returns (uint64 requestTimestamp, uint64 unbondingPeriod) + { + UnbondingRequest memory request = _unbondingRequests[user][token]; + return ( + request.requestTimestamp, + request.unbondingPeriod + ); + } + + /** + * @notice Check if a user has an active unbonding request for a token. + * @param user The address of the user. + * @param token The address of the token. + * @return True if the user has an active unbonding request, false otherwise. + */ + function hasActiveUnbondingRequest( + address user, + address token + ) external view returns (bool) { + return _unbondingRequests[user][token].requestTimestamp != 0; + } + + /** + * @notice Check if a withdrawal is authorized. + * @dev Returns true if the unbonding period has passed since the withdrawal request. + * As a side effect, this function deletes the unbonding request if authorized. + * @param owner The address of the token owner. + * @param token The address of the token. + * @return True if the withdrawal is authorized, false otherwise. + */ + function authorize( + address owner, + address token, + uint256 // amount - not used + ) external override returns (bool) { + UnbondingRequest memory request = _unbondingRequests[owner][token]; + + // Check if withdrawal was requested + require( + request.requestTimestamp != 0, + UnbondingNotRequested(owner, token) + ); + + // Check if unbonding period has passed + // Note: We don't check if the unbonding period is still supported + require( + uint64(block.timestamp) >= + request.requestTimestamp + request.unbondingPeriod, + UnbondingPeriodNotExpired( + request.requestTimestamp, + uint64(block.timestamp), + request.unbondingPeriod + ) + ); + + // NOTE: this logic is put as side-effect as there is no other way to do it in the current Vault-Authorizer architecture + // Remove the unbonding request + delete _unbondingRequests[owner][token]; + emit UnbondingCompleted(owner, token); + + return true; + } + + /** + * @notice Updates the status of an unbonding period. + * @param unbondingPeriod The unbonding period to update. + * @param isSupported Whether the unbonding period should be supported. + */ + function setUnbondingPeriodStatus( + uint64 unbondingPeriod, + bool isSupported + ) external onlyOwner { + require(unbondingPeriod > 0, InvalidUnbondingPeriod()); + + if (isSupported) { + _supportedUnbondingPeriods.add(unbondingPeriod); + } else { + _supportedUnbondingPeriods.remove(unbondingPeriod); + } + + emit UnbondingPeriodStatusChanged(unbondingPeriod, isSupported); + } + + /** + * @notice Request a withdrawal for a specific token with a specific unbonding period. + * @dev Emits a UnbondingRequested event. + * @param token The address of the token to withdraw. + * @param unbondingPeriod The unbonding period to use for this withdrawal request. + */ + function requestUnbonding( + address token, + uint64 unbondingPeriod + ) public { + require( + _supportedUnbondingPeriods.contains(unbondingPeriod), + UnsupportedUnbondingPeriod(unbondingPeriod) + ); + + address account = msg.sender; + _unbondingRequests[account][token] = UnbondingRequest({ + requestTimestamp: uint64(block.timestamp), + unbondingPeriod: unbondingPeriod + }); + + emit UnbondingRequested(account, token, unbondingPeriod); + } +} From a48c5d7922069fc7b4ecc7aeb686fd91b3698f83 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 21 Mar 2025 13:39:05 +0200 Subject: [PATCH 04/11] feat: remove CooldownAuthorizer obsolete docs --- CooldownAuthorizerFeatureOverview.md | 79 ---------------------------- 1 file changed, 79 deletions(-) delete mode 100644 CooldownAuthorizerFeatureOverview.md diff --git a/CooldownAuthorizerFeatureOverview.md b/CooldownAuthorizerFeatureOverview.md deleted file mode 100644 index 257c518..0000000 --- a/CooldownAuthorizerFeatureOverview.md +++ /dev/null @@ -1,79 +0,0 @@ -# CooldownAuthorizer Feature Overview - -## Overview - -The CooldownAuthorizer implements a time-based withdrawal mechanism for the LiteVault contract that requires users to wait for a specified cooldown period before completing their withdrawals. Key features include: - -1. **Configurable Cooldown Periods**: Multiple cooldown duration options can be enabled/disabled by the contract owner, allowing for flexible withdrawal policies. - -2. **User-selected Cooldown**: When requesting a withdrawal, users can select from supported cooldown periods, offering a balance between security and convenience. - -3. **Immutable Requests**: Once a withdrawal is requested, it can not be affected by the cooldown period modification, ensuring predictable withdrawal timelines. - -4. **Cooldown Enforcement**: The `authorize` function verifies that the cooldown period has elapsed before allowing a withdrawal to proceed. - -5. **Graceful Administration**: Owner can add or remove supported cooldown periods without affecting existing withdrawal requests, as their originally selected cooldown period remains valid regardless of later changes. - -The core workflow is: - -- User calls `requestWithdrawalWithCooldown` with token, amount, and desired cooldown period -- System emits a `CooldownRequested` event and stores request details -- After the cooldown period elapses, a withdrawal from LiteVault will be authorized -- If attempted before cooldown expiry, the withdrawal is rejected - -NOTE: Having implemented this, it is still not possible to disallow depositing while the cooldown is active. - -## Integration plan - -1. Separate the unlock and withdraw processes on FE -2. Change FE to support users specifying a cooldown period during withdrawal request -3. Make a transaction to CooldownAuthorizer on FE -4. Deploy CooldownAuthorizer contract on 8 currently supported chains -5. Set the LiteVault authorizer to the CooldownAuthorizer contract on the 8 chains - -NOTE: possibly no changes to Pathfinder (BE) unless we want to store the cooldown period and/or withdrawal request info. - -NOTE: if we want to add different logic to different cooldown periods, we NEED the Pathfinder to listen to withdrawal requests and update the logic as we need. - -## Withdrawal request double-spending problem - -### Problem Statement - -We need a mechanism to outdate withdrawal requests after they have been fulfilled. However, the `authorize` function in the `IAuthorize` interface is `view` so it's not possible to update the state. - -The vault is already deployed and we can't change it. We could have added some post-withdrawal call logic to the vault, but this is no longer an option. - -The main reason why the withdrawal request needs to be invalidated is that a user may deposit more funds to the LiteVault during the cooldown period, and request 2 consecutive withdrawals. - -The first one will succeed as designed, but the second one will also succeed because the withdrawal request is not invalidated and user can still withdraw this token. - -A timeout approach would only work if the difference between the first withdrawal and the second is greater than this timeout, which we cannot assume. - -### Possible Solutions - -#### 1. User-initiated request refresh - -- Add function for users to explicitly refresh/update withdrawal requests -- Requires user understanding of the extra step - -#### 2. Request with specific identifiers - -- Track multiple withdrawal requests with unique IDs -- Would require LiteVault modification (not possible) - -#### 3. Time-bounded requests - -- Add validity period to requests -- Requests expire after cooldown + short window -- Potential UX issues if window too short - -#### 4. Interface modification - -- Modify IAuthorize to have non-view authorize function -- Breaking change requiring new vault deployment - -### Recommendation - -The ideal solution would be creating a new IAuthorize2 interface with a non-view authorize function, then deploying a new version of LiteVault that uses this interface. - -If that's not possible, the user-initiated request refresh mechanism (Option 1) provides the most practical solution within the current constraints, though it requires users to understand the extra withdrawal request step whenever they want to withdraw again after depositing more funds. From f6a6549890377628e7a9770b0d1b7b2052968831 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 21 Mar 2025 16:45:25 +0200 Subject: [PATCH 05/11] test: UnbondingPeriodAuthorizer --- src/vault/UnbondingPeriodAuthorizer.sol | 132 +++---- .../vault/UnbondingPeriodAuthorizerTest.t.sol | 334 ++++++++++++++++++ 2 files changed, 404 insertions(+), 62 deletions(-) create mode 100644 test/vault/UnbondingPeriodAuthorizerTest.t.sol diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index 8d4d025..fc9ca42 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import "../interfaces/IAuthorize.sol"; -import "@openzeppelin/contracts/access/Ownable2Step.sol"; -import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {IAuthorize} from "../interfaces/IAuthorize.sol"; + +import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol"; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; /** * @title UnbondingPeriodAuthorizer @@ -15,15 +17,6 @@ import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { // Use EnumerableSet for tracking supported unbonding periods using EnumerableSet for EnumerableSet.UintSet; - /** - * @notice Struct to store withdrawal request information. - * @param requestTimestamp The timestamp when the withdrawal was requested. - * @param unbondingPeriod The unbonding period chosen for this withdrawal request. - */ - struct UnbondingRequest { - uint64 requestTimestamp; - uint64 unbondingPeriod; - } /** * @notice Error thrown when the unbonding period has not yet passed. @@ -37,6 +30,13 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { uint64 unbondingPeriod ); + /** + * @notice Error thrown when the withdrawal has already been requested. + * @param user The address of the user. + * @param token The address of the token. + */ + error UnbondingAlreadyRequested(address user, address token); + /** * @notice Error thrown when the withdrawal has not been requested. * @param user The address of the user. @@ -81,6 +81,16 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { */ event UnbondingPeriodStatusChanged(uint64 unbondingPeriod, bool isSupported); + /** + * @notice Struct to store withdrawal request information. + * @param requestTimestamp The timestamp when the withdrawal was requested. + * @param unbondingPeriod The unbonding period chosen for this withdrawal request. + */ + struct UnbondingRequest { + uint64 requestTimestamp; + uint64 unbondingPeriod; + } + // Set of all supported unbonding periods EnumerableSet.UintSet internal _supportedUnbondingPeriods; @@ -120,15 +130,8 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @notice Get all supported unbonding periods. * @return An array of all supported unbonding periods. */ - function getAllSupportedUnbondingPeriods() external view returns (uint64[] memory) { - uint256 totalPeriods = _supportedUnbondingPeriods.length(); - uint64[] memory result = new uint64[](totalPeriods); - - for (uint256 i = 0; i < totalPeriods; i++) { - result[i] = uint64(_supportedUnbondingPeriods.at(i)); - } - - return result; + function getAllSupportedUnbondingPeriods() external view returns (uint256[] memory) { + return _supportedUnbondingPeriods.values(); } /** @@ -166,47 +169,6 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { return _unbondingRequests[user][token].requestTimestamp != 0; } - /** - * @notice Check if a withdrawal is authorized. - * @dev Returns true if the unbonding period has passed since the withdrawal request. - * As a side effect, this function deletes the unbonding request if authorized. - * @param owner The address of the token owner. - * @param token The address of the token. - * @return True if the withdrawal is authorized, false otherwise. - */ - function authorize( - address owner, - address token, - uint256 // amount - not used - ) external override returns (bool) { - UnbondingRequest memory request = _unbondingRequests[owner][token]; - - // Check if withdrawal was requested - require( - request.requestTimestamp != 0, - UnbondingNotRequested(owner, token) - ); - - // Check if unbonding period has passed - // Note: We don't check if the unbonding period is still supported - require( - uint64(block.timestamp) >= - request.requestTimestamp + request.unbondingPeriod, - UnbondingPeriodNotExpired( - request.requestTimestamp, - uint64(block.timestamp), - request.unbondingPeriod - ) - ); - - // NOTE: this logic is put as side-effect as there is no other way to do it in the current Vault-Authorizer architecture - // Remove the unbonding request - delete _unbondingRequests[owner][token]; - emit UnbondingCompleted(owner, token); - - return true; - } - /** * @notice Updates the status of an unbonding period. * @param unbondingPeriod The unbonding period to update. @@ -242,6 +204,11 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { UnsupportedUnbondingPeriod(unbondingPeriod) ); + require( + _unbondingRequests[msg.sender][token].requestTimestamp == 0, + UnbondingAlreadyRequested(msg.sender, token) + ); + address account = msg.sender; _unbondingRequests[account][token] = UnbondingRequest({ requestTimestamp: uint64(block.timestamp), @@ -250,4 +217,45 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { emit UnbondingRequested(account, token, unbondingPeriod); } + + /** + * @notice Check if a withdrawal is authorized. + * @dev Returns true if the unbonding period has passed since the withdrawal request. + * As a side effect, this function deletes the unbonding request if authorized. + * @param owner The address of the token owner. + * @param token The address of the token. + * @return True if the withdrawal is authorized, false otherwise. + */ + function authorize( + address owner, + address token, + uint256 // amount - not used + ) external override returns (bool) { + UnbondingRequest memory request = _unbondingRequests[owner][token]; + + // Check if withdrawal was requested + require( + request.requestTimestamp != 0, + UnbondingNotRequested(owner, token) + ); + + // Check if unbonding period has passed + // Note: We don't check if the unbonding period is still supported + require( + uint64(block.timestamp) >= + request.requestTimestamp + request.unbondingPeriod, + UnbondingPeriodNotExpired( + request.requestTimestamp, + uint64(block.timestamp), + request.unbondingPeriod + ) + ); + + // NOTE: this logic is put as side-effect as there is no other way to do it in the current Vault-Authorizer architecture + // Remove the unbonding request + delete _unbondingRequests[owner][token]; + emit UnbondingCompleted(owner, token); + + return true; + } } diff --git a/test/vault/UnbondingPeriodAuthorizerTest.t.sol b/test/vault/UnbondingPeriodAuthorizerTest.t.sol new file mode 100644 index 0000000..f8c66e1 --- /dev/null +++ b/test/vault/UnbondingPeriodAuthorizerTest.t.sol @@ -0,0 +1,334 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Test, Vm} from "forge-std/Test.sol"; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + +import {UnbondingPeriodAuthorizer} from "../../src/vault/UnbondingPeriodAuthorizer.sol"; +import {TestERC20} from "../TestERC20.sol"; +import {IAuthorize} from "../../src/interfaces/IAuthorize.sol"; + +uint256 constant TIME = 1716051867; + +contract UnbondingPeriodAuthorizerTestBase is Test { + UnbondingPeriodAuthorizer authorizer; + TestERC20 token; + + address deployer = vm.createWallet("deployer").addr; + address owner = vm.createWallet("owner").addr; + address user = vm.createWallet("user").addr; + + uint64[] internal defaultUnbondingPeriods; + + function setUp() public virtual { + defaultUnbondingPeriods = new uint64[](3); + defaultUnbondingPeriods[0] = 1 days; + defaultUnbondingPeriods[1] = 7 days; + defaultUnbondingPeriods[2] = 30 days; + + vm.warp(TIME); + + vm.prank(deployer); + authorizer = new UnbondingPeriodAuthorizer(owner, defaultUnbondingPeriods); + + token = new TestERC20("Test", "TST", 18, type(uint256).max); + } +} + +contract UnbondingPeriodAuthorizerTest_constructor is UnbondingPeriodAuthorizerTestBase { + function test_constructor_setsCorrectOwner() public view { + assertEq(authorizer.owner(), owner); + } + + function test_constructor_setsSupportedUnbondingPeriods() public view { + uint256[] memory periods = authorizer.getAllSupportedUnbondingPeriods(); + assertEq(periods.length, defaultUnbondingPeriods.length); + + // Check each period is in the result + for (uint256 i = 0; i < defaultUnbondingPeriods.length; i++) { + bool found = false; + for (uint256 j = 0; j < periods.length; j++) { + if (periods[j] == defaultUnbondingPeriods[i]) { + found = true; + break; + } + } + assertTrue(found, "Period not found in result"); + } + } + + function test_constructor_emitsEventsForPeriods() public { + uint64[] memory periods = new uint64[](2); + periods[0] = 14 days; + periods[1] = 21 days; + + for (uint256 i = 0; i < periods.length; i++) { + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingPeriodStatusChanged(periods[i], true); + } + + vm.prank(deployer); + new UnbondingPeriodAuthorizer(owner, periods); + } + + function test_constructor_revert_ifZeroPeriod() public { + uint64[] memory periods = new uint64[](1); + periods[0] = 0; // Invalid period + + vm.expectRevert(abi.encodeWithSelector(UnbondingPeriodAuthorizer.InvalidUnbondingPeriod.selector)); + vm.prank(deployer); + new UnbondingPeriodAuthorizer(owner, periods); + } +} + +contract UnbondingPeriodAuthorizerTest_isUnbondingPeriodSupported is UnbondingPeriodAuthorizerTestBase { + function test_isUnbondingPeriodSupported_returnsTrue_forSupportedPeriod() public view { + assertTrue(authorizer.isUnbondingPeriodSupported(1 days)); + assertTrue(authorizer.isUnbondingPeriodSupported(7 days)); + assertTrue(authorizer.isUnbondingPeriodSupported(30 days)); + } + + function test_isUnbondingPeriodSupported_returnsFalse_forUnsupportedPeriod() public view { + assertFalse(authorizer.isUnbondingPeriodSupported(2 days)); + assertFalse(authorizer.isUnbondingPeriodSupported(14 days)); + assertFalse(authorizer.isUnbondingPeriodSupported(0)); + } +} + +contract UnbondingPeriodAuthorizerTest_getAllSupportedUnbondingPeriods is UnbondingPeriodAuthorizerTestBase { + function test_getAllSupportedUnbondingPeriods_returnsAllPeriods() public view { + uint256[] memory periods = authorizer.getAllSupportedUnbondingPeriods(); + + assertEq(periods.length, defaultUnbondingPeriods.length); + + // Check each period is in the result + for (uint256 i = 0; i < defaultUnbondingPeriods.length; i++) { + bool found = false; + for (uint256 j = 0; j < periods.length; j++) { + if (periods[j] == defaultUnbondingPeriods[i]) { + found = true; + break; + } + } + assertTrue(found, "Period not found in result"); + } + } + + function test_getAllSupportedUnbondingPeriods_returnsEmptyArray_forNoPeriods() public { + // Create authorizer with no periods + uint64[] memory noPeriods = new uint64[](0); + vm.prank(deployer); + UnbondingPeriodAuthorizer noPeriodsAuthorizer = new UnbondingPeriodAuthorizer(owner, noPeriods); + + uint256[] memory periods = noPeriodsAuthorizer.getAllSupportedUnbondingPeriods(); + assertEq(periods.length, 0); + } +} + +contract UnbondingPeriodAuthorizerTest_getUnbondingRequest is UnbondingPeriodAuthorizerTestBase { + function setUp() public override { + super.setUp(); + + vm.prank(user); + authorizer.requestUnbonding(address(token), 7 days); + } + + function test_getUnbondingRequest_returnsRequest_forExistingRequest() public view { + (uint64 requestTimestamp, uint64 unbondingPeriod) = authorizer.getUnbondingRequest(user, address(token)); + + assertEq(requestTimestamp, uint64(TIME)); + assertEq(unbondingPeriod, 7 days); + } + + function test_getUnbondingRequest_returnsZeros_forNonExistingRequest() public view { + (uint64 requestTimestamp, uint64 unbondingPeriod) = authorizer.getUnbondingRequest(user, address(0)); + + assertEq(requestTimestamp, 0); + assertEq(unbondingPeriod, 0); + } +} + +contract UnbondingPeriodAuthorizerTest_hasActiveUnbondingRequest is UnbondingPeriodAuthorizerTestBase { + function setUp() public override { + super.setUp(); + + vm.prank(user); + authorizer.requestUnbonding(address(token), 7 days); + } + + function test_hasActiveUnbondingRequest_returnsTrue_forExistingRequest() public view { + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token))); + } + + function test_hasActiveUnbondingRequest_returnsFalse_forNonExistingRequest() public view { + assertFalse(authorizer.hasActiveUnbondingRequest(user, address(0))); + assertFalse(authorizer.hasActiveUnbondingRequest(owner, address(token))); + } +} + +contract UnbondingPeriodAuthorizerTest_setUnbondingPeriodStatus is UnbondingPeriodAuthorizerTestBase { + function test_setUnbondingPeriodStatus_enablesNewPeriod_whenCalledByOwner() public { + uint64 newPeriod = 14 days; + + assertFalse(authorizer.isUnbondingPeriodSupported(newPeriod)); + + // Enable new period + vm.prank(owner); + authorizer.setUnbondingPeriodStatus(newPeriod, true); + + assertTrue(authorizer.isUnbondingPeriodSupported(newPeriod)); + } + + function test_setUnbondingPeriodStatus_disablesExistingPeriod_whenCalledByOwner() public { + uint64 existingPeriod = 7 days; + + assertTrue(authorizer.isUnbondingPeriodSupported(existingPeriod)); + + vm.prank(owner); + authorizer.setUnbondingPeriodStatus(existingPeriod, false); + + assertFalse(authorizer.isUnbondingPeriodSupported(existingPeriod)); + } + + function test_setUnbondingPeriodStatus_emitsEvent_whenPeriodEnabled() public { + uint64 newPeriod = 14 days; + + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingPeriodStatusChanged(newPeriod, true); + + vm.prank(owner); + authorizer.setUnbondingPeriodStatus(newPeriod, true); + } + + function test_setUnbondingPeriodStatus_emitsEvent_whenPeriodDisabled() public { + uint64 existingPeriod = 7 days; + + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingPeriodStatusChanged(existingPeriod, false); + + vm.prank(owner); + authorizer.setUnbondingPeriodStatus(existingPeriod, false); + } + + function test_setUnbondingPeriodStatus_revert_whenPeriodIsZero() public { + vm.expectRevert(abi.encodeWithSelector(UnbondingPeriodAuthorizer.InvalidUnbondingPeriod.selector)); + + vm.prank(owner); + authorizer.setUnbondingPeriodStatus(0, true); + } + + function test_setUnbondingPeriodStatus_revert_whenCallerIsNotOwner() public { + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, user)); + + vm.prank(user); + authorizer.setUnbondingPeriodStatus(14 days, true); + } +} + +contract UnbondingPeriodAuthorizerTest_requestUnbonding is UnbondingPeriodAuthorizerTestBase { + function test_requestUnbonding_createsRequest_forSupportedPeriod() public { + vm.prank(user); + authorizer.requestUnbonding(address(token), 7 days); + + (uint64 requestTimestamp, uint64 unbondingPeriod) = authorizer.getUnbondingRequest(user, address(token)); + + assertEq(requestTimestamp, uint64(TIME)); + assertEq(unbondingPeriod, 7 days); + } + + function test_requestUnbonding_emitsEvent() public { + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingRequested(user, address(token), 7 days); + + vm.prank(user); + authorizer.requestUnbonding(address(token), 7 days); + } + + function test_requestUnbonding_revert_whenRequestAlreadyMade() public { + // Initial request + vm.prank(user); + authorizer.requestUnbonding(address(token), 7 days); + + vm.warp(TIME + 1 days); + + // Request again + vm.prank(user); + vm.expectRevert(abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingAlreadyRequested.selector, user, address(token))); + authorizer.requestUnbonding(address(token), 7 days); + } + + function test_requestUnbonding_revert_whenPeriodIsUnsupported() public { + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnsupportedUnbondingPeriod.selector, + uint64(14 days) + ) + ); + + vm.prank(user); + authorizer.requestUnbonding(address(token), 14 days); + } +} + +contract UnbondingPeriodAuthorizerTest_authorize is UnbondingPeriodAuthorizerTestBase { + uint64 unbondingPeriod = 7 days; + function setUp() public override { + super.setUp(); + + vm.prank(user); + authorizer.requestUnbonding(address(token), unbondingPeriod); + } + + function test_authorize_returnsTrue_whenUnbondingPeriodExpired() public { + vm.warp(TIME + unbondingPeriod + 1); + + bool authorized = authorizer.authorize(user, address(token), 100); + + assertTrue(authorized); + } + + function test_authorize_deletesRequest_whenAuthorized() public { + vm.warp(TIME + unbondingPeriod + 1); + + bool authorized = authorizer.authorize(user, address(token), 100); + + assertTrue(authorized); + assertFalse(authorizer.hasActiveUnbondingRequest(user, address(token))); + } + + function test_authorize_emitsEvent_whenAuthorized() public { + vm.warp(TIME + unbondingPeriod + 1); + + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingCompleted(user, address(token)); + + authorizer.authorize(user, address(token), 100); + } + + function test_authorize_revert_whenUnbondingPeriodNotExpired() public { + vm.warp(TIME + unbondingPeriod - 1 hours); + + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnbondingPeriodNotExpired.selector, + uint64(TIME), + uint64(TIME + unbondingPeriod - 1 hours), + uint64(unbondingPeriod) + ) + ); + + authorizer.authorize(user, address(token), 100); + } + + function test_authorize_revert_whenNoRequestExists() public { + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, + user, + address(0) + ) + ); + + authorizer.authorize(user, address(0), 100); + } +} From 2044ede51c0c8316f6937933b6d05d21d372293f Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 21 Mar 2025 16:46:20 +0200 Subject: [PATCH 06/11] style: run forge fmt --- UnbondingPeriodAuthorizer.abi | 1 + src/vault/LiteVault.sol | 2 +- src/vault/UnbondingPeriodAuthorizer.sol | 77 ++++--------------- .../vault/UnbondingPeriodAuthorizerTest.t.sol | 16 ++-- 4 files changed, 25 insertions(+), 71 deletions(-) create mode 100644 UnbondingPeriodAuthorizer.abi diff --git a/UnbondingPeriodAuthorizer.abi b/UnbondingPeriodAuthorizer.abi new file mode 100644 index 0000000..f593eb1 --- /dev/null +++ b/UnbondingPeriodAuthorizer.abi @@ -0,0 +1 @@ +[{"type":"constructor","inputs":[{"name":"owner","type":"address","internalType":"address"},{"name":"supportedUnbondingPeriods","type":"uint64[]","internalType":"uint64[]"}],"stateMutability":"nonpayable"},{"type":"function","name":"acceptOwnership","inputs":[],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"authorize","inputs":[{"name":"owner","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"},{"name":"","type":"uint256","internalType":"uint256"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"nonpayable"},{"type":"function","name":"getAllSupportedUnbondingPeriods","inputs":[],"outputs":[{"name":"","type":"uint256[]","internalType":"uint256[]"}],"stateMutability":"view"},{"type":"function","name":"getUnbondingRequest","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}],"outputs":[{"name":"requestTimestamp","type":"uint64","internalType":"uint64"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"stateMutability":"view"},{"type":"function","name":"hasActiveUnbondingRequest","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"view"},{"type":"function","name":"isUnbondingPeriodSupported","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"view"},{"type":"function","name":"owner","inputs":[],"outputs":[{"name":"","type":"address","internalType":"address"}],"stateMutability":"view"},{"type":"function","name":"pendingOwner","inputs":[],"outputs":[{"name":"","type":"address","internalType":"address"}],"stateMutability":"view"},{"type":"function","name":"renounceOwnership","inputs":[],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"requestUnbonding","inputs":[{"name":"token","type":"address","internalType":"address"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"setUnbondingPeriodStatus","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"},{"name":"isSupported","type":"bool","internalType":"bool"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"transferOwnership","inputs":[{"name":"newOwner","type":"address","internalType":"address"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"event","name":"OwnershipTransferStarted","inputs":[{"name":"previousOwner","type":"address","indexed":true,"internalType":"address"},{"name":"newOwner","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"OwnershipTransferred","inputs":[{"name":"previousOwner","type":"address","indexed":true,"internalType":"address"},{"name":"newOwner","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"UnbondingCompleted","inputs":[{"name":"user","type":"address","indexed":true,"internalType":"address"},{"name":"token","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"UnbondingPeriodStatusChanged","inputs":[{"name":"unbondingPeriod","type":"uint64","indexed":false,"internalType":"uint64"},{"name":"isSupported","type":"bool","indexed":false,"internalType":"bool"}],"anonymous":false},{"type":"event","name":"UnbondingRequested","inputs":[{"name":"user","type":"address","indexed":true,"internalType":"address"},{"name":"token","type":"address","indexed":true,"internalType":"address"},{"name":"unbondingPeriod","type":"uint64","indexed":false,"internalType":"uint64"}],"anonymous":false},{"type":"error","name":"InvalidUnbondingPeriod","inputs":[]},{"type":"error","name":"OwnableInvalidOwner","inputs":[{"name":"owner","type":"address","internalType":"address"}]},{"type":"error","name":"OwnableUnauthorizedAccount","inputs":[{"name":"account","type":"address","internalType":"address"}]},{"type":"error","name":"Unauthorized","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"},{"name":"amount","type":"uint256","internalType":"uint256"}]},{"type":"error","name":"UnbondingAlreadyRequested","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}]},{"type":"error","name":"UnbondingNotRequested","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}]},{"type":"error","name":"UnbondingPeriodNotExpired","inputs":[{"name":"requestTimestamp","type":"uint64","internalType":"uint64"},{"name":"currentTimestamp","type":"uint64","internalType":"uint64"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}]},{"type":"error","name":"UnsupportedUnbondingPeriod","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}]}] diff --git a/src/vault/LiteVault.sol b/src/vault/LiteVault.sol index b44565e..034eb40 100644 --- a/src/vault/LiteVault.sol +++ b/src/vault/LiteVault.sol @@ -107,9 +107,9 @@ contract LiteVault is IVault, IAuthorizable, ReentrancyGuard, Ownable2Step { revert InsufficientBalance(token, amount, currentBalance); } if ( + // TODO: change method signature to pass `data` as a parameter, that an authorizer can decode and make a decision !_isWithdrawalGracePeriodActive( latestSetAuthorizerTimestamp, uint64(block.timestamp), WITHDRAWAL_GRACE_PERIOD - // TODO: change method signature to pass `data` as a parameter, that an authorizer can decode and make a decision ) && !authorizer.authorize(msg.sender, token, amount) ) { revert IAuthorize.Unauthorized(msg.sender, token, amount); diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index fc9ca42..7cdbd3b 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -24,11 +24,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param currentTimestamp The current timestamp. * @param unbondingPeriod The required unbonding period. */ - error UnbondingPeriodNotExpired( - uint64 requestTimestamp, - uint64 currentTimestamp, - uint64 unbondingPeriod - ); + error UnbondingPeriodNotExpired(uint64 requestTimestamp, uint64 currentTimestamp, uint64 unbondingPeriod); /** * @notice Error thrown when the withdrawal has already been requested. @@ -61,11 +57,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param token The address of the token to withdraw. * @param unbondingPeriod The unbonding period chosen for this withdrawal request. */ - event UnbondingRequested( - address indexed user, - address indexed token, - uint64 unbondingPeriod - ); + event UnbondingRequested(address indexed user, address indexed token, uint64 unbondingPeriod); /** * @notice Event emitted when an unbonding period has passed and the withdrawal is authorized. @@ -95,18 +87,14 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { EnumerableSet.UintSet internal _supportedUnbondingPeriods; // Mapping of user address to token address to withdrawal request - mapping(address user => mapping(address token => UnbondingRequest request)) - internal _unbondingRequests; + mapping(address user => mapping(address token => UnbondingRequest request)) internal _unbondingRequests; /** * @dev Constructor sets the initial owner of the contract and enables the provided unbonding periods. * @param owner The address of the owner. * @param supportedUnbondingPeriods Array of unbonding periods to be initially supported. */ - constructor( - address owner, - uint64[] memory supportedUnbondingPeriods - ) Ownable(owner) { + constructor(address owner, uint64[] memory supportedUnbondingPeriods) Ownable(owner) { for (uint256 i = 0; i < supportedUnbondingPeriods.length; i++) { uint64 period = supportedUnbondingPeriods[i]; require(period > 0, InvalidUnbondingPeriod()); @@ -120,9 +108,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param unbondingPeriod The unbonding period to check. * @return True if the unbonding period is supported, false otherwise. */ - function isUnbondingPeriodSupported( - uint64 unbondingPeriod - ) external view returns (bool) { + function isUnbondingPeriodSupported(uint64 unbondingPeriod) external view returns (bool) { return _supportedUnbondingPeriods.contains(unbondingPeriod); } @@ -141,19 +127,13 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @return requestTimestamp The timestamp when the withdrawal was requested. * @return unbondingPeriod The unbonding period chosen for this withdrawal request. */ - function getUnbondingRequest( - address user, - address token - ) + function getUnbondingRequest(address user, address token) external view returns (uint64 requestTimestamp, uint64 unbondingPeriod) { UnbondingRequest memory request = _unbondingRequests[user][token]; - return ( - request.requestTimestamp, - request.unbondingPeriod - ); + return (request.requestTimestamp, request.unbondingPeriod); } /** @@ -162,10 +142,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param token The address of the token. * @return True if the user has an active unbonding request, false otherwise. */ - function hasActiveUnbondingRequest( - address user, - address token - ) external view returns (bool) { + function hasActiveUnbondingRequest(address user, address token) external view returns (bool) { return _unbondingRequests[user][token].requestTimestamp != 0; } @@ -174,10 +151,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param unbondingPeriod The unbonding period to update. * @param isSupported Whether the unbonding period should be supported. */ - function setUnbondingPeriodStatus( - uint64 unbondingPeriod, - bool isSupported - ) external onlyOwner { + function setUnbondingPeriodStatus(uint64 unbondingPeriod, bool isSupported) external onlyOwner { require(unbondingPeriod > 0, InvalidUnbondingPeriod()); if (isSupported) { @@ -195,25 +169,16 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param token The address of the token to withdraw. * @param unbondingPeriod The unbonding period to use for this withdrawal request. */ - function requestUnbonding( - address token, - uint64 unbondingPeriod - ) public { - require( - _supportedUnbondingPeriods.contains(unbondingPeriod), - UnsupportedUnbondingPeriod(unbondingPeriod) - ); + function requestUnbonding(address token, uint64 unbondingPeriod) public { + require(_supportedUnbondingPeriods.contains(unbondingPeriod), UnsupportedUnbondingPeriod(unbondingPeriod)); require( - _unbondingRequests[msg.sender][token].requestTimestamp == 0, - UnbondingAlreadyRequested(msg.sender, token) + _unbondingRequests[msg.sender][token].requestTimestamp == 0, UnbondingAlreadyRequested(msg.sender, token) ); address account = msg.sender; - _unbondingRequests[account][token] = UnbondingRequest({ - requestTimestamp: uint64(block.timestamp), - unbondingPeriod: unbondingPeriod - }); + _unbondingRequests[account][token] = + UnbondingRequest({requestTimestamp: uint64(block.timestamp), unbondingPeriod: unbondingPeriod}); emit UnbondingRequested(account, token, unbondingPeriod); } @@ -234,21 +199,13 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { UnbondingRequest memory request = _unbondingRequests[owner][token]; // Check if withdrawal was requested - require( - request.requestTimestamp != 0, - UnbondingNotRequested(owner, token) - ); + require(request.requestTimestamp != 0, UnbondingNotRequested(owner, token)); // Check if unbonding period has passed // Note: We don't check if the unbonding period is still supported require( - uint64(block.timestamp) >= - request.requestTimestamp + request.unbondingPeriod, - UnbondingPeriodNotExpired( - request.requestTimestamp, - uint64(block.timestamp), - request.unbondingPeriod - ) + uint64(block.timestamp) >= request.requestTimestamp + request.unbondingPeriod, + UnbondingPeriodNotExpired(request.requestTimestamp, uint64(block.timestamp), request.unbondingPeriod) ); // NOTE: this logic is put as side-effect as there is no other way to do it in the current Vault-Authorizer architecture diff --git a/test/vault/UnbondingPeriodAuthorizerTest.t.sol b/test/vault/UnbondingPeriodAuthorizerTest.t.sol index f8c66e1..0a561bf 100644 --- a/test/vault/UnbondingPeriodAuthorizerTest.t.sol +++ b/test/vault/UnbondingPeriodAuthorizerTest.t.sol @@ -253,16 +253,15 @@ contract UnbondingPeriodAuthorizerTest_requestUnbonding is UnbondingPeriodAuthor // Request again vm.prank(user); - vm.expectRevert(abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingAlreadyRequested.selector, user, address(token))); + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingAlreadyRequested.selector, user, address(token)) + ); authorizer.requestUnbonding(address(token), 7 days); } function test_requestUnbonding_revert_whenPeriodIsUnsupported() public { vm.expectRevert( - abi.encodeWithSelector( - UnbondingPeriodAuthorizer.UnsupportedUnbondingPeriod.selector, - uint64(14 days) - ) + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnsupportedUnbondingPeriod.selector, uint64(14 days)) ); vm.prank(user); @@ -272,6 +271,7 @@ contract UnbondingPeriodAuthorizerTest_requestUnbonding is UnbondingPeriodAuthor contract UnbondingPeriodAuthorizerTest_authorize is UnbondingPeriodAuthorizerTestBase { uint64 unbondingPeriod = 7 days; + function setUp() public override { super.setUp(); @@ -322,11 +322,7 @@ contract UnbondingPeriodAuthorizerTest_authorize is UnbondingPeriodAuthorizerTes function test_authorize_revert_whenNoRequestExists() public { vm.expectRevert( - abi.encodeWithSelector( - UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, - user, - address(0) - ) + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(0)) ); authorizer.authorize(user, address(0), 100); From 8d0f5720e042f3dfd21f76a4252ebb46a0ce5dc9 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 24 Mar 2025 11:55:47 +0200 Subject: [PATCH 07/11] feat(UnbondingPeriodAuthorizer): add unbindingTimestamp param to event --- src/vault/UnbondingPeriodAuthorizer.sol | 5 +++-- test/vault/UnbondingPeriodAuthorizerTest.t.sol | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index 7cdbd3b..d329591 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -56,8 +56,9 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param user The address of the user requesting the withdrawal. * @param token The address of the token to withdraw. * @param unbondingPeriod The unbonding period chosen for this withdrawal request. + * @param unbondingTimestamp The timestamp when the unbonding period will expire. */ - event UnbondingRequested(address indexed user, address indexed token, uint64 unbondingPeriod); + event UnbondingRequested(address indexed user, address indexed token, uint64 unbondingPeriod, uint64 unbondingTimestamp); /** * @notice Event emitted when an unbonding period has passed and the withdrawal is authorized. @@ -180,7 +181,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { _unbondingRequests[account][token] = UnbondingRequest({requestTimestamp: uint64(block.timestamp), unbondingPeriod: unbondingPeriod}); - emit UnbondingRequested(account, token, unbondingPeriod); + emit UnbondingRequested(account, token, unbondingPeriod, uint64(block.timestamp) + unbondingPeriod); } /** diff --git a/test/vault/UnbondingPeriodAuthorizerTest.t.sol b/test/vault/UnbondingPeriodAuthorizerTest.t.sol index 0a561bf..32d084e 100644 --- a/test/vault/UnbondingPeriodAuthorizerTest.t.sol +++ b/test/vault/UnbondingPeriodAuthorizerTest.t.sol @@ -238,7 +238,7 @@ contract UnbondingPeriodAuthorizerTest_requestUnbonding is UnbondingPeriodAuthor function test_requestUnbonding_emitsEvent() public { vm.expectEmit(true, true, true, true); - emit UnbondingPeriodAuthorizer.UnbondingRequested(user, address(token), 7 days); + emit UnbondingPeriodAuthorizer.UnbondingRequested(user, address(token), 7 days, uint64(TIME) + 7 days); vm.prank(user); authorizer.requestUnbonding(address(token), 7 days); From 712c073c1b69ab23b9f55b060e82a7ddc272c596 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 24 Mar 2025 12:42:32 +0200 Subject: [PATCH 08/11] style: run forge fmt --- UnbondingPeriodAuthorizer.abi | 1 - src/vault/UnbondingPeriodAuthorizer.sol | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 UnbondingPeriodAuthorizer.abi diff --git a/UnbondingPeriodAuthorizer.abi b/UnbondingPeriodAuthorizer.abi deleted file mode 100644 index f593eb1..0000000 --- a/UnbondingPeriodAuthorizer.abi +++ /dev/null @@ -1 +0,0 @@ -[{"type":"constructor","inputs":[{"name":"owner","type":"address","internalType":"address"},{"name":"supportedUnbondingPeriods","type":"uint64[]","internalType":"uint64[]"}],"stateMutability":"nonpayable"},{"type":"function","name":"acceptOwnership","inputs":[],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"authorize","inputs":[{"name":"owner","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"},{"name":"","type":"uint256","internalType":"uint256"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"nonpayable"},{"type":"function","name":"getAllSupportedUnbondingPeriods","inputs":[],"outputs":[{"name":"","type":"uint256[]","internalType":"uint256[]"}],"stateMutability":"view"},{"type":"function","name":"getUnbondingRequest","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}],"outputs":[{"name":"requestTimestamp","type":"uint64","internalType":"uint64"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"stateMutability":"view"},{"type":"function","name":"hasActiveUnbondingRequest","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"view"},{"type":"function","name":"isUnbondingPeriodSupported","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"outputs":[{"name":"","type":"bool","internalType":"bool"}],"stateMutability":"view"},{"type":"function","name":"owner","inputs":[],"outputs":[{"name":"","type":"address","internalType":"address"}],"stateMutability":"view"},{"type":"function","name":"pendingOwner","inputs":[],"outputs":[{"name":"","type":"address","internalType":"address"}],"stateMutability":"view"},{"type":"function","name":"renounceOwnership","inputs":[],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"requestUnbonding","inputs":[{"name":"token","type":"address","internalType":"address"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"setUnbondingPeriodStatus","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"},{"name":"isSupported","type":"bool","internalType":"bool"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"function","name":"transferOwnership","inputs":[{"name":"newOwner","type":"address","internalType":"address"}],"outputs":[],"stateMutability":"nonpayable"},{"type":"event","name":"OwnershipTransferStarted","inputs":[{"name":"previousOwner","type":"address","indexed":true,"internalType":"address"},{"name":"newOwner","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"OwnershipTransferred","inputs":[{"name":"previousOwner","type":"address","indexed":true,"internalType":"address"},{"name":"newOwner","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"UnbondingCompleted","inputs":[{"name":"user","type":"address","indexed":true,"internalType":"address"},{"name":"token","type":"address","indexed":true,"internalType":"address"}],"anonymous":false},{"type":"event","name":"UnbondingPeriodStatusChanged","inputs":[{"name":"unbondingPeriod","type":"uint64","indexed":false,"internalType":"uint64"},{"name":"isSupported","type":"bool","indexed":false,"internalType":"bool"}],"anonymous":false},{"type":"event","name":"UnbondingRequested","inputs":[{"name":"user","type":"address","indexed":true,"internalType":"address"},{"name":"token","type":"address","indexed":true,"internalType":"address"},{"name":"unbondingPeriod","type":"uint64","indexed":false,"internalType":"uint64"}],"anonymous":false},{"type":"error","name":"InvalidUnbondingPeriod","inputs":[]},{"type":"error","name":"OwnableInvalidOwner","inputs":[{"name":"owner","type":"address","internalType":"address"}]},{"type":"error","name":"OwnableUnauthorizedAccount","inputs":[{"name":"account","type":"address","internalType":"address"}]},{"type":"error","name":"Unauthorized","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"},{"name":"amount","type":"uint256","internalType":"uint256"}]},{"type":"error","name":"UnbondingAlreadyRequested","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}]},{"type":"error","name":"UnbondingNotRequested","inputs":[{"name":"user","type":"address","internalType":"address"},{"name":"token","type":"address","internalType":"address"}]},{"type":"error","name":"UnbondingPeriodNotExpired","inputs":[{"name":"requestTimestamp","type":"uint64","internalType":"uint64"},{"name":"currentTimestamp","type":"uint64","internalType":"uint64"},{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}]},{"type":"error","name":"UnsupportedUnbondingPeriod","inputs":[{"name":"unbondingPeriod","type":"uint64","internalType":"uint64"}]}] diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index d329591..4f7f9b7 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -58,7 +58,9 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { * @param unbondingPeriod The unbonding period chosen for this withdrawal request. * @param unbondingTimestamp The timestamp when the unbonding period will expire. */ - event UnbondingRequested(address indexed user, address indexed token, uint64 unbondingPeriod, uint64 unbondingTimestamp); + event UnbondingRequested( + address indexed user, address indexed token, uint64 unbondingPeriod, uint64 unbondingTimestamp + ); /** * @notice Event emitted when an unbonding period has passed and the withdrawal is authorized. From da166eafab4be1ebe23f7b828fe50fd07cdec6da Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 24 Mar 2025 12:42:51 +0200 Subject: [PATCH 09/11] feat: add forge fmt editor-agnostic rules --- .editorconfig | 18 ++++++++++++++++++ .githooks/pre-commit | 15 +++++++++++++++ .vscode/settings.json | 8 ++++++++ README.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+) create mode 100644 .editorconfig create mode 100755 .githooks/pre-commit create mode 100644 .vscode/settings.json diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..3670a52 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,18 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +insert_final_newline = true +trim_trailing_whitespace = true + +[*.sol] +indent_style = space +indent_size = 4 +max_line_length = 120 +# Forge formatter typically puts opening braces on the same line: +curly_bracket_next_line = false +# Space after control statements like if, for, while: +spaces_around_operators = true +spaces_around_brackets = false +indent_brace_style = K&R \ No newline at end of file diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..7444fe2 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,15 @@ +#!/bin/sh + +# Format all staged Solidity files using forge fmt +staged_sol_files=$(git diff --cached --name-only --diff-filter=ACMR | grep "\.sol$" || true) + +if [ -n "$staged_sol_files" ]; then + echo "Formatting Solidity files with forge fmt..." + # Format all staged Solidity files + forge fmt $staged_sol_files + + # Add the formatted files back to the staging area + git add $staged_sol_files +fi + +exit 0 \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..96f3fcd --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,8 @@ +{ + "makefile.configureOnOpen": false, + "editor.formatOnSave": true, + "[solidity]": { + "editor.formatOnSave": true + }, + "solidity.formatter": "forge" +} diff --git a/README.md b/README.md index efbbd8e..dac88fe 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,48 @@ LiteVault Owner can change the Authorizer contract, which will enable a grace wi Authorizer contract that authorize withdrawal regardless of token and amount, but only outside of the time range specified on deployment. +## Development Setup + +### Code Formatting + +This project uses `forge fmt` for consistent Solidity code formatting. We follow an editor-agnostic approach with additional convenience settings for VS Code users. + +#### Editor-Agnostic Formatting + +1. **Git Hooks**: The project includes pre-commit hooks that automatically format Solidity files using `forge fmt` before each commit. + + To set up the pre-commit hook: + + ```bash + # Configure git to use the hooks in the .githooks directory + git config core.hooksPath .githooks + ``` + +2. **EditorConfig**: The project includes an `.editorconfig` file with basic formatting rules that many editors support. + + To use these settings: + + - Install an EditorConfig plugin for your editor if it doesn't have built-in support + - The plugin will automatically apply basic formatting rules (indentation, line endings, etc.) + + More information about EditorConfig can be found at [https://editorconfig.org/](https://editorconfig.org/) + +#### VS Code-Specific Settings + +For VS Code users, additional settings are provided in `.vscode/settings.json` that: + +- Configure VS Code to use `forge fmt` automatically when saving Solidity files +- Ensure consistent formatting directly in the editor + +These settings are optional and only apply to VS Code users. Other editors may need their own configuration to exactly match `forge fmt` behavior. + +#### Recommended Workflow + +The recommended workflow for all developers, regardless of editor: + +1. Use the pre-commit hooks to ensure consistent formatting in the repository +2. If needed, run `forge fmt` manually before committing to see changes + ## Deployment and interaction This repository uses Foundry toolchain for development, testing and deployment. From 27ad74ff9d8df649798695ea3d7fa93ee88c0a1e Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 24 Mar 2025 20:11:33 +0200 Subject: [PATCH 10/11] feat(UnbondingPeriodAuthorizer): extract mutating code from authorize --- src/interfaces/IAuthorize.sol | 2 +- src/vault/UnbondingPeriodAuthorizer.sol | 85 ++++++--- .../vault/UnbondingPeriodAuthorizerTest.t.sol | 163 ++++++++++++++++-- ...dingPeriodAuthorizerTest_integration.t.sol | 125 ++++++++++++++ 4 files changed, 334 insertions(+), 41 deletions(-) create mode 100644 test/vault/UnbondingPeriodAuthorizerTest_integration.t.sol diff --git a/src/interfaces/IAuthorize.sol b/src/interfaces/IAuthorize.sol index 455b38b..70feb6c 100644 --- a/src/interfaces/IAuthorize.sol +++ b/src/interfaces/IAuthorize.sol @@ -23,5 +23,5 @@ interface IAuthorize { * @param amount The amount of tokens to be authorized. * @return True if the action is authorized, false otherwise. */ - function authorize(address owner, address token, uint256 amount) external returns (bool); + function authorize(address owner, address token, uint256 amount) external view returns (bool); } diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index 4f7f9b7..f3f3343 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -149,6 +149,33 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { return _unbondingRequests[user][token].requestTimestamp != 0; } + /** + * @notice Check if a withdrawal is authorized. + * @dev Returns true if the unbonding period has passed since the withdrawal request. + * @param owner The address of the token owner. + * @param token The address of the token. + * @return True if the withdrawal is authorized, false otherwise. + */ + function authorize( + address owner, + address token, + uint256 // amount - not used + ) public view override returns (bool) { + UnbondingRequest memory request = _unbondingRequests[owner][token]; + + // Check if withdrawal was requested + require(request.requestTimestamp != 0, UnbondingNotRequested(owner, token)); + + // Check if unbonding period has passed + // Note: We don't check if the unbonding period is still supported + require( + uint64(block.timestamp) >= request.requestTimestamp + request.unbondingPeriod, + UnbondingPeriodNotExpired(request.requestTimestamp, uint64(block.timestamp), request.unbondingPeriod) + ); + + return true; + } + /** * @notice Updates the status of an unbonding period. * @param unbondingPeriod The unbonding period to update. @@ -187,35 +214,39 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { } /** - * @notice Check if a withdrawal is authorized. - * @dev Returns true if the unbonding period has passed since the withdrawal request. - * As a side effect, this function deletes the unbonding request if authorized. - * @param owner The address of the token owner. - * @param token The address of the token. - * @return True if the withdrawal is authorized, false otherwise. + * @notice Completes an unbonding request after the unbonding period has passed. + * @dev Verifies the unbonding period has passed before completing the request. + * It cleans up the request state and emits the UnbondingCompleted event. + * @param token The address of the token for which to complete the unbonding request. */ - function authorize( - address owner, - address token, - uint256 // amount - not used - ) external override returns (bool) { - UnbondingRequest memory request = _unbondingRequests[owner][token]; - - // Check if withdrawal was requested - require(request.requestTimestamp != 0, UnbondingNotRequested(owner, token)); - - // Check if unbonding period has passed - // Note: We don't check if the unbonding period is still supported - require( - uint64(block.timestamp) >= request.requestTimestamp + request.unbondingPeriod, - UnbondingPeriodNotExpired(request.requestTimestamp, uint64(block.timestamp), request.unbondingPeriod) - ); + function completeUnbondingRequest(address token) external { + _completeUnbondingRequest(msg.sender, token); + } - // NOTE: this logic is put as side-effect as there is no other way to do it in the current Vault-Authorizer architecture - // Remove the unbonding request - delete _unbondingRequests[owner][token]; - emit UnbondingCompleted(owner, token); + /** + * @notice Completes multiple unbonding requests in a single transaction after their unbonding periods have passed. + * @dev Verifies the unbonding period has passed for each token before completing the request. + * Will revert if any of the requests is not authorized (unbonding period not passed). + * @param tokens Array of token addresses for which to complete unbonding requests. + */ + function completeUnbondingRequests(address[] calldata tokens) external { + address account = msg.sender; + for (uint256 i = 0; i < tokens.length; i++) { + address token = tokens[i]; + _completeUnbondingRequest(account, token); + } + } - return true; + /** + * @dev Internal helper function to complete an unbonding request for a specific user and token. + * It verifies the unbonding period has passed, deletes the request from storage and emits the UnbondingCompleted event. + * @param account The address of the user who made the unbonding request. + * @param token The address of the token for which to complete the unbonding request. + */ + function _completeUnbondingRequest(address account, address token) internal { + // Verify the unbonding period has passed + authorize(account, token, 0); + delete _unbondingRequests[account][token]; + emit UnbondingCompleted(account, token); } } diff --git a/test/vault/UnbondingPeriodAuthorizerTest.t.sol b/test/vault/UnbondingPeriodAuthorizerTest.t.sol index 32d084e..c5c5b43 100644 --- a/test/vault/UnbondingPeriodAuthorizerTest.t.sol +++ b/test/vault/UnbondingPeriodAuthorizerTest.t.sol @@ -287,44 +287,181 @@ contract UnbondingPeriodAuthorizerTest_authorize is UnbondingPeriodAuthorizerTes assertTrue(authorized); } - function test_authorize_deletesRequest_whenAuthorized() public { + function test_authorize_revert_whenUnbondingPeriodNotExpired() public { + vm.warp(TIME + unbondingPeriod - 1 hours); + + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnbondingPeriodNotExpired.selector, + uint64(TIME), + uint64(TIME + unbondingPeriod - 1 hours), + uint64(unbondingPeriod) + ) + ); + + authorizer.authorize(user, address(token), 100); + } + + function test_authorize_revert_whenNoRequestExists() public { + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(0)) + ); + + authorizer.authorize(user, address(0), 100); + } +} + +contract UnbondingPeriodAuthorizerTest_completeUnbondingRequest is UnbondingPeriodAuthorizerTestBase { + uint64 unbondingPeriod = 7 days; + + function setUp() public override { + super.setUp(); + + vm.prank(user); + authorizer.requestUnbonding(address(token), unbondingPeriod); + } + + function test_completeUnbondingRequest_deletesRequest() public { + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token)), "Request should exist before removal"); + vm.warp(TIME + unbondingPeriod + 1); - bool authorized = authorizer.authorize(user, address(token), 100); + vm.prank(user); + authorizer.completeUnbondingRequest(address(token)); - assertTrue(authorized); - assertFalse(authorizer.hasActiveUnbondingRequest(user, address(token))); + assertFalse( + authorizer.hasActiveUnbondingRequest(user, address(token)), "Request should be deleted after removal" + ); } - function test_authorize_emitsEvent_whenAuthorized() public { + function test_completeUnbondingRequest_emitsEvent() public { vm.warp(TIME + unbondingPeriod + 1); vm.expectEmit(true, true, true, true); emit UnbondingPeriodAuthorizer.UnbondingCompleted(user, address(token)); - authorizer.authorize(user, address(token), 100); + vm.prank(user); + authorizer.completeUnbondingRequest(address(token)); } - function test_authorize_revert_whenUnbondingPeriodNotExpired() public { - vm.warp(TIME + unbondingPeriod - 1 hours); - + function test_completeUnbondingRequest_revert_whenUnbondingPeriodNotExpired() public { vm.expectRevert( abi.encodeWithSelector( UnbondingPeriodAuthorizer.UnbondingPeriodNotExpired.selector, uint64(TIME), - uint64(TIME + unbondingPeriod - 1 hours), + uint64(block.timestamp), uint64(unbondingPeriod) ) ); + vm.prank(user); + authorizer.completeUnbondingRequest(address(token)); + } - authorizer.authorize(user, address(token), 100); + function test_completeUnbondingRequest_revert_ifNoRequestExists() public { + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(0)) + ); + vm.prank(user); + authorizer.completeUnbondingRequest(address(0)); } +} + +contract UnbondingPeriodAuthorizerTest_completeUnbondingRequests is UnbondingPeriodAuthorizerTestBase { + TestERC20 token2; + TestERC20 token3; + uint64 unbondingPeriod = 7 days; + + function setUp() public override { + super.setUp(); + + token2 = new TestERC20("Test2", "TST2", 18, type(uint256).max); + token3 = new TestERC20("Test3", "TST3", 18, type(uint256).max); + + vm.startPrank(user); + authorizer.requestUnbonding(address(token), unbondingPeriod); + authorizer.requestUnbonding(address(token2), unbondingPeriod); + authorizer.requestUnbonding(address(token3), unbondingPeriod); + vm.stopPrank(); + } + + function test_completeUnbondingRequests_deletesMultipleRequests() public { + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token)), "Request for token1 should exist"); + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token2)), "Request for token2 should exist"); + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token3)), "Request for token3 should exist"); + + address[] memory tokens = new address[](3); + tokens[0] = address(token); + tokens[1] = address(token2); + tokens[2] = address(token3); + + vm.warp(TIME + unbondingPeriod + 1); + + vm.prank(user); + authorizer.completeUnbondingRequests(tokens); + + assertFalse(authorizer.hasActiveUnbondingRequest(user, address(token)), "Request for token1 should be deleted"); + assertFalse(authorizer.hasActiveUnbondingRequest(user, address(token2)), "Request for token2 should be deleted"); + assertFalse(authorizer.hasActiveUnbondingRequest(user, address(token3)), "Request for token3 should be deleted"); + } + + function test_completeUnbondingRequests_emitsEventsForEachToken() public { + address[] memory tokens = new address[](2); + tokens[0] = address(token); + tokens[1] = address(token2); + + vm.warp(TIME + unbondingPeriod + 1); + + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingCompleted(user, address(token)); + + vm.expectEmit(true, true, true, true); + emit UnbondingPeriodAuthorizer.UnbondingCompleted(user, address(token2)); + + vm.prank(user); + authorizer.completeUnbondingRequests(tokens); + } + + function test_completeUnbondingRequests_worksWithEmptyArray() public { + address[] memory tokens = new address[](0); + + vm.warp(TIME + unbondingPeriod + 1); + + // Should not revert with empty array + vm.prank(user); + authorizer.completeUnbondingRequests(tokens); + + // Verify no requests were deleted + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token)), "Request for token1 should still exist"); + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token2)), "Request for token2 should still exist"); + assertTrue(authorizer.hasActiveUnbondingRequest(user, address(token3)), "Request for token3 should still exist"); + } + + function test_completeUnbondingRequests_revert_ifAnyTokenHasNoRequest() public { + address[] memory tokens = new address[](2); + tokens[0] = address(token); + tokens[1] = address(0); // No request for this address + + vm.warp(TIME + unbondingPeriod + 1); - function test_authorize_revert_whenNoRequestExists() public { vm.expectRevert( abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(0)) ); + vm.prank(user); + authorizer.completeUnbondingRequests(tokens); + } - authorizer.authorize(user, address(0), 100); + function test_completeUnbondingRequests_revert_ifTokenRequestedTwice() public { + address[] memory tokens = new address[](3); + tokens[0] = address(token); + tokens[1] = address(token2); + tokens[2] = address(token); // Duplicate + + vm.warp(TIME + unbondingPeriod + 1); + + vm.prank(user); + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(token)) + ); + authorizer.completeUnbondingRequests(tokens); } } diff --git a/test/vault/UnbondingPeriodAuthorizerTest_integration.t.sol b/test/vault/UnbondingPeriodAuthorizerTest_integration.t.sol new file mode 100644 index 0000000..73c41f0 --- /dev/null +++ b/test/vault/UnbondingPeriodAuthorizerTest_integration.t.sol @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Test, Vm} from "forge-std/Test.sol"; + +import {LiteVault} from "../../src/vault/LiteVault.sol"; +import {UnbondingPeriodAuthorizer} from "../../src/vault/UnbondingPeriodAuthorizer.sol"; +import {TestERC20} from "../TestERC20.sol"; +import {IAuthorize} from "../../src/interfaces/IAuthorize.sol"; + +uint256 constant TIME = 1716051867; + +contract UnbondingPeriodAuthorizerTest_integration is Test { + UnbondingPeriodAuthorizer authorizer; + LiteVault vault; + TestERC20 token; + + address deployer = vm.createWallet("deployer").addr; + address owner = vm.createWallet("owner").addr; + address user = vm.createWallet("user").addr; + + uint64 constant UNBONDING_PERIOD = 7 days; + uint256 constant DEPOSIT_AMOUNT = 1000e18; + + function setUp() public { + uint64[] memory supportedPeriods = new uint64[](1); + supportedPeriods[0] = UNBONDING_PERIOD; + + vm.prank(deployer); + authorizer = new UnbondingPeriodAuthorizer(owner, supportedPeriods); + + vm.prank(deployer); + vault = new LiteVault(owner, authorizer); + + token = new TestERC20("Test", "TST", 18, type(uint256).max); + + token.mint(user, DEPOSIT_AMOUNT); + + vm.warp(TIME); + } + + function test_depositAndWithdrawFlow() public { + vm.startPrank(user); + token.approve(address(vault), DEPOSIT_AMOUNT); + vault.deposit(address(token), DEPOSIT_AMOUNT); + vm.stopPrank(); + + assertEq(token.balanceOf(user), 0, "User balance should be 0 after deposit"); + assertEq(token.balanceOf(address(vault)), DEPOSIT_AMOUNT, "Vault balance should equal deposit amount"); + assertEq( + vault.balanceOf(user, address(token)), DEPOSIT_AMOUNT, "User balance in vault should equal deposit amount" + ); + + vm.prank(user); + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(token)) + ); + vault.withdraw(address(token), DEPOSIT_AMOUNT); + + vm.prank(user); + authorizer.requestUnbonding(address(token), UNBONDING_PERIOD); + + vm.prank(user); + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnbondingPeriodNotExpired.selector, + uint64(TIME), + uint64(TIME), + UNBONDING_PERIOD + ) + ); + vault.withdraw(address(token), DEPOSIT_AMOUNT); + + vm.warp(TIME + UNBONDING_PERIOD - 1); + + vm.prank(user); + vm.expectRevert( + abi.encodeWithSelector( + UnbondingPeriodAuthorizer.UnbondingPeriodNotExpired.selector, + uint64(TIME), + uint64(TIME + UNBONDING_PERIOD - 1), + UNBONDING_PERIOD + ) + ); + vault.withdraw(address(token), DEPOSIT_AMOUNT); + + vm.warp(TIME + UNBONDING_PERIOD + 1); + + vm.prank(user); + authorizer.completeUnbondingRequest(address(token)); + + assertFalse( + authorizer.hasActiveUnbondingRequest(user, address(token)), + "Unbonding request should be deleted after completion" + ); + + // Now the withdrawal should fail as there's no active request to authorize + vm.prank(user); + vm.expectRevert( + abi.encodeWithSelector(UnbondingPeriodAuthorizer.UnbondingNotRequested.selector, user, address(token)) + ); + vault.withdraw(address(token), DEPOSIT_AMOUNT); + + vm.prank(user); + authorizer.requestUnbonding(address(token), UNBONDING_PERIOD); + + vm.warp(TIME + UNBONDING_PERIOD * 2 + 1); + + // Now withdrawal should succeed without explicitly completing the request first + // The vault will call authorizer.authorize() which checks the unbonding period has passed + vm.prank(user); + vault.withdraw(address(token), DEPOSIT_AMOUNT); + + assertEq(token.balanceOf(user), DEPOSIT_AMOUNT, "User balance should equal deposit amount after withdrawal"); + assertEq(token.balanceOf(address(vault)), 0, "Vault balance should be 0 after withdrawal"); + assertEq(vault.balanceOf(user, address(token)), 0, "User balance in vault should be 0 after withdrawal"); + + // Verify the request still exists after withdrawal + // LiteVault only calls authorize() which doesn't delete the request + assertTrue( + authorizer.hasActiveUnbondingRequest(user, address(token)), + "Unbonding request should still exist after withdrawal" + ); + } +} From 7cea6cf91f1695949701f978e0fd682e69a02582 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Tue, 25 Mar 2025 13:06:56 +0200 Subject: [PATCH 11/11] feat(unbondingPeriodAuthorizer): add comments about 0 tokens --- src/vault/UnbondingPeriodAuthorizer.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vault/UnbondingPeriodAuthorizer.sol b/src/vault/UnbondingPeriodAuthorizer.sol index f3f3343..c3c47ea 100644 --- a/src/vault/UnbondingPeriodAuthorizer.sol +++ b/src/vault/UnbondingPeriodAuthorizer.sol @@ -245,6 +245,7 @@ contract UnbondingPeriodAuthorizer is IAuthorize, Ownable2Step { */ function _completeUnbondingRequest(address account, address token) internal { // Verify the unbonding period has passed + // NOTE: authorization amount does not matter here authorize(account, token, 0); delete _unbondingRequests[account][token]; emit UnbondingCompleted(account, token);