Skip to content

Commit

Permalink
[NES-262] [N2] [Critical] Asynchronous deposit/redemption design flaw (…
Browse files Browse the repository at this point in the history 
…#119)

* fix deposit and redeem functions

* use calculated amounts between async deposit and redeem

* Revert "use calculated amounts between async deposit and redeem"

This reverts commit e94addd.
  • Loading branch information
@ungaro
ungaro committed Dec 12, 2024
1 parent bc71858 commit 4f9f8ba
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 35 deletions.
133 changes: 121 additions & 12 deletions nest/src/ComponentToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,46 @@ abstract contract ComponentToken is
*/
error Unauthorized(address sender, address authorizedUser);

/**
* @notice Indicates a failure because the operation was called in async mode
* @dev This error is thrown when trying to perform a synchronous operation while async mode is enabled
*/
error AsyncOperationsEnabled();

/**
* @notice Indicates a failure because the operation was called in sync mode
* @dev This error is thrown when trying to perform an asynchronous operation while async mode is disabled
*/
error AsyncOperationsDisabled();

/**
* @notice Indicates a failure because there are no claimable deposits for the controller
* @dev This error is thrown when trying to claim a deposit but either the assets
* or shares amount in the request is zero
*/
error NoClaimableDeposit();

/**
* @notice Indicates a failure because there are no claimable redemptions for the controller
* @dev This error is thrown when trying to claim a redemption but either the assets
* or shares amount in the request is zero
*/
error NoClaimableRedeem();

/**
* @notice Indicates a failure because the deposit amount doesn't match the claimable amount
* @param provided Amount of assets provided for deposit
* @param required Amount of assets required (claimable amount)
*/
error InvalidDepositAmount(uint256 provided, uint256 required);

/**
* @notice Indicates a failure because the redeem amount doesn't match the claimable amount
* @param provided Amount of shares provided for redemption
* @param required Amount of shares required (claimable amount)
*/
error InvalidRedeemAmount(uint256 provided, uint256 required);

/**
* @notice Indicates a failure because the controller does not have enough requested
* @param controller Address of the controller who does not have enough requested
Expand Down Expand Up @@ -301,21 +341,30 @@ abstract contract ComponentToken is
}

ComponentTokenStorage storage $ = _getComponentTokenStorage();

if ($.asyncDeposit) {
if ($.claimableDepositRequest[controller] < assets) {
revert InsufficientRequestBalance(controller, assets, 1);
}
// For async deposits, we must use the full claimable amount
uint256 claimableAssets = $.claimableDepositRequest[controller];
shares = $.sharesDepositRequest[controller];
$.claimableDepositRequest[controller] -= assets;
$.sharesDepositRequest[controller] -= shares;

if (claimableAssets == 0 || shares == 0) {
revert NoClaimableDeposit();
}
if (assets != claimableAssets) {
revert InvalidDepositAmount(assets, claimableAssets);
}

// Reset state atomically
$.claimableDepositRequest[controller] = 0;
$.sharesDepositRequest[controller] = 0;
} else {
SafeERC20.safeTransferFrom(IERC20(asset()), controller, address(this), assets);
shares = convertToShares(assets);
}

_mint(receiver, shares);

emit Deposit(controller, receiver, assets, shares);
return shares;
}

/// @inheritdoc IERC7540
Expand Down Expand Up @@ -401,7 +450,14 @@ abstract contract ComponentToken is
emit RedeemNotified(controller, assets, shares);
}

/// @inheritdoc IERC7540
/**
* @notice Fulfill a synchronous request to redeem assets by transferring assets to the receiver
* @dev This function can only be called when async redemptions are disabled
* @param shares Amount of shares to redeem
* @param receiver Address to receive the assets
* @param controller Controller of the request
* @return assets Amount of assets sent to the receiver
*/
function redeem(
uint256 shares,
address receiver,
Expand All @@ -415,21 +471,74 @@ abstract contract ComponentToken is
}

ComponentTokenStorage storage $ = _getComponentTokenStorage();

if ($.asyncRedeem) {
if ($.claimableRedeemRequest[controller] < shares) {
revert InsufficientRequestBalance(controller, shares, 3);
}
// For async redemptions, we must use the full claimable amount
uint256 claimableShares = $.claimableRedeemRequest[controller];
assets = $.assetsRedeemRequest[controller];
$.claimableRedeemRequest[controller] -= shares;
$.assetsRedeemRequest[controller] -= assets;

if (claimableShares == 0 || assets == 0) {
revert NoClaimableRedeem();
}
if (shares != claimableShares) {
revert InvalidRedeemAmount(shares, claimableShares);
}

// Reset state atomically
$.claimableRedeemRequest[controller] = 0;
$.assetsRedeemRequest[controller] = 0;
} else {
// For sync redemptions, process normally
_burn(controller, shares);
assets = convertToAssets(shares);
}

SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);

emit Withdraw(controller, receiver, controller, assets, shares);
return assets;
}

/**
* @notice Claim an approved asynchronous redeem request and transfer assets to the receiver
* @dev This function can only be called when async redemptions are enabled
* and will revert if there are no claimable redemptions for the controller.
* All state for the request is atomically reset after a successful claim.
* @param receiver Address to receive the redeemed assets
* @param controller Controller of the redeem request
* @return assets Amount of assets sent to the receiver
* @return shares Amount of shares that were redeemed
*/
function claimRedeem(
address receiver,
address controller
) public virtual nonReentrant returns (uint256 assets, uint256 shares) {
if (msg.sender != controller) {
revert Unauthorized(msg.sender, controller);
}

ComponentTokenStorage storage $ = _getComponentTokenStorage();
if (!$.asyncRedeem) {
revert AsyncOperationsDisabled();
}

shares = $.claimableRedeemRequest[controller];
assets = $.assetsRedeemRequest[controller];

if (shares == 0 || assets == 0) {
revert NoClaimableRedeem();
}

// Reset state atomically
$.claimableRedeemRequest[controller] = 0;
$.assetsRedeemRequest[controller] = 0;

if (!IERC20(asset()).transfer(receiver, assets)) {
revert InsufficientBalance(IERC20(asset()), address(this), assets);
}

emit Withdraw(controller, receiver, controller, assets, shares);
return (assets, shares);
}

/// @inheritdoc IERC7540
Expand Down
24 changes: 1 addition & 23 deletions nest/src/interfaces/IComponentToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.8.25;

interface IComponentToken {

// Events
// User Functions

/**
* @notice Emitted when the owner of some assets submits a request to buy shares
Expand All @@ -29,28 +29,6 @@ interface IComponentToken {
address indexed controller, address indexed owner, uint256 indexed requestId, address sender, uint256 shares
);

/**
* @notice Emitted when a deposit request is complete
* @param sender Controller of the request
* @param owner Source of the assets to deposit
* @param assets Amount of `asset` that has been deposited
* @param shares Amount of shares that has been received in exchange
*/
// event Deposit(address indexed sender, address indexed owner, uint256 assets, uint256 shares);

/**
* @notice Emitted when a redeem request is complete
* @param sender Controller of the request
* @param receiver Address to receive the assets
* @param owner Source of the shares to redeem
* @param assets Amount of `asset` that has been received in exchange
* @param shares Amount of shares that has been redeemed
*/
// event Withdraw(address indexed sender, address indexed receiver, address indexed owner, uint256 assets, uint256
// shares);

// User Functions

/**
* @notice Transfer assets from the owner into the vault and submit a request to buy shares
* @param assets Amount of `asset` to deposit
Expand Down

0 comments on commit 4f9f8ba

Please sign in to comment.