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

[PDE-668] ottersec final fixes #156

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions nest/src/AggregateToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,24 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder {
/**
* @inheritdoc IERC4626
* @dev 1:1 conversion rate between USDT and base asset
* @dev Always round DOWN for convertToShares for user safety
*/
function convertToShares(
uint256 assets
) public view override(ComponentToken, IComponentToken) returns (uint256 shares) {
// Division rounds down by default
return assets * _BASE / _getAggregateTokenStorage().askPrice;
}

/**
* @inheritdoc IERC4626
* @dev 1:1 conversion rate between USDT and base asset
* @dev Always round DOWN for convertToAssets for user safety
*/
function convertToAssets(
uint256 shares
) public view override(ComponentToken, IComponentToken) returns (uint256 assets) {
// Division rounds down by default
return shares * _getAggregateTokenStorage().bidPrice / _BASE;
}

Expand Down
4 changes: 2 additions & 2 deletions nest/src/ComponentToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,8 @@ abstract contract ComponentToken is
$.claimableDepositRequest[controller] = 0;
$.sharesDepositRequest[controller] = 0;
} else {
SafeERC20.safeTransferFrom(IERC20(asset()), controller, address(this), assets);
shares = convertToShares(assets);
SafeERC20.safeTransferFrom(IERC20(asset()), controller, address(this), assets);
}

_mint(receiver, shares);
Expand Down Expand Up @@ -558,8 +558,8 @@ abstract contract ComponentToken is
$.assetsRedeemRequest[controller] = 0;
} else {
// For sync redemptions, process normally
_burn(controller, shares);
assets = convertToAssets(shares);
_burn(controller, shares);
}

SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);
Expand Down
34 changes: 27 additions & 7 deletions smart-wallets/src/token/YieldToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,10 @@ contract YieldToken is
return convertToAssets(balanceOf(owner));
}

/// @inheritdoc IERC4626
/**
* @inheritdoc IERC4626
* @dev Always rounds down for user safety when converting assets to shares
*/
function convertToShares(
uint256 assets
) public view override(ERC4626Upgradeable, IComponentToken) returns (uint256 shares) {
Expand All @@ -302,7 +305,10 @@ contract YieldToken is
return (assets * supply) / totalAssets_;
}

/// @inheritdoc IERC4626
/**
* @inheritdoc IERC4626
* @dev Always rounds down for user safety when converting shares to assets
*/
function convertToAssets(
uint256 shares
) public view override(ERC4626Upgradeable, IComponentToken) returns (uint256 assets) {
Expand Down Expand Up @@ -348,8 +354,8 @@ contract YieldToken is
revert InvalidCurrencyToken(currencyToken, _getYieldDistributionTokenStorage().currencyToken);
}

$.yieldBuffer += amount;
_depositYield(currencyTokenAmount);
$.yieldBuffer += amount;
}

/// @inheritdoc IComponentToken
Expand Down Expand Up @@ -451,14 +457,20 @@ contract YieldToken is
}

YieldTokenStorage storage $ = _getYieldTokenStorage();
assets = convertToAssets(shares);

if ($.claimableDepositRequest[controller] < assets) {
revert InsufficientRequestBalance(controller, assets, 1);
if ($.sharesDepositRequest[controller] < shares) {
revert InsufficientRequestBalance(controller, shares, 1);
}

// Calculate proportional assets based on requested shares
assets = $.claimableDepositRequest[controller].mulDivDown(shares, $.sharesDepositRequest[controller]);

$.claimableDepositRequest[controller] -= assets;
$.sharesDepositRequest[controller] -= shares;

// Track managed assets
$.totalManagedAssets += assets;

_mint(receiver, shares);

emit Deposit(controller, receiver, assets, shares);
Expand All @@ -479,7 +491,6 @@ contract YieldToken is

YieldTokenStorage storage $ = _getYieldTokenStorage();

_burn(msg.sender, shares);
$.pendingRedeemRequest[controller] += shares;

emit RedeemRequest(controller, owner, REQUEST_ID, owner, shares);
Expand Down Expand Up @@ -539,6 +550,12 @@ contract YieldToken is
// Track managed assets
$.totalManagedAssets -= assets;

// Check yield buffer
_beforeWithdraw(assets);

// Burn the shares, when we actually process the redemption
_burn(controller, shares);

if (!IERC20(asset()).transfer(receiver, assets)) {
revert InsufficientBalance(IERC20(asset()), address(this), assets);
}
Expand Down Expand Up @@ -588,6 +605,9 @@ contract YieldToken is

_beforeWithdraw(assets);

// Burn the shares when we actually process the withdrawal
_burn(controller, shares);

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