Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NES-262] [N2] [Critical] Asynchronous deposit/redemption design flaw #119

Merged
merged 4 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 121 additions & 12 deletions nest/src/ComponentToken.sol
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
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
Loading