Skip to content
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
10 changes: 8 additions & 2 deletions src/controller/AccountingLogic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ abstract contract AccountingLogic is BaseController, VaultManager {
* @return The asset deposit price, limited to ASSET_DEPOSIT_MAX_PRICE
*/
function assetDepositPrice(address asset) public view returns (uint256) {
return getAssetPrice(asset).min(ASSET_DEPOSIT_MAX_PRICE);
uint256 _secondaryActionFee = _withdrawnInTx ? secondaryActionFee : 0;
// Note: Additional fee is applied to the asset price during deposit/mint if a withdrawal/redemption has already
// occurred in the same transaction
return getAssetPrice(asset).min(ASSET_DEPOSIT_MAX_PRICE).mulDiv(MAX_BPS - _secondaryActionFee, MAX_BPS);
}

/**
Expand All @@ -45,7 +48,10 @@ abstract contract AccountingLogic is BaseController, VaultManager {
* @return The asset redemption price, at least ASSET_REDEMPTION_MIN_PRICE
*/
function assetRedemptionPrice(address asset) public view returns (uint256) {
return getAssetPrice(asset).max(ASSET_REDEMPTION_MIN_PRICE);
uint256 _secondaryActionFee = _depositedInTx ? secondaryActionFee : 0;
// Note: Additional fee is applied to the asset price during withdrawal/redemption if a deposit/mint has already
// occurred in the same transaction
return getAssetPrice(asset).max(ASSET_REDEMPTION_MIN_PRICE).mulDiv(MAX_BPS, MAX_BPS - _secondaryActionFee);
}

/**
Expand Down
28 changes: 27 additions & 1 deletion src/controller/BaseController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ import { IGenericShare } from "../interfaces/IGenericShare.sol";
* - Permanent contract malfunction
*/
abstract contract BaseController is AccessControlUpgradeable, ReentrancyGuardTransientUpgradeable {
// ========================================
// TRANSIENT
// ========================================

/**
* @notice Flag of deposit action in the current transaction
*/
bool internal transient _depositedInTx;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although transient storage should not alter the storage layout, would you mind generating a before and after of forge inspect storage --pretty?
That should ensure us we are not messing it. Just to fully double check, this is a critical component.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original storage:

Name Type Slot Offset Bytes
paused bool 0 0 1
skipNextRebalanceSafetyBufferCheck bool 0 1 1
_share contract IGenericShare 0 2 20
priceFeeds mapping(address => struct BaseController.PriceFeed) 1 0 32
vaultSettings mapping(address => struct BaseController.VaultSettings) 2 0 32
_vaultFor mapping(address => address) 3 0 32
_vaults mapping(address => address) 4 0 32
_vaultsCount uint8 5 0 1
maxProtocolRebalanceSlippage uint16 5 1 2
rewardsCollector address 5 3 20
isRewardAsset mapping(address => bool) 6 0 32
_swapper contract ISwapper 7 0 20
_yieldDistributor contract IYieldDistributor 8 0 20
safetyBufferYieldDeduction uint256 9 0 32
__gap uint256[50] 10 0 1600

Updated storage:

Name Type Slot Offset Bytes
paused bool 0 0 1
skipNextRebalanceSafetyBufferCheck bool 0 1 1
_share contract IGenericShare 0 2 20
priceFeeds mapping(address => struct BaseController.PriceFeed) 1 0 32
vaultSettings mapping(address => struct BaseController.VaultSettings) 2 0 32
_vaultFor mapping(address => address) 3 0 32
_vaults mapping(address => address) 4 0 32
_vaultsCount uint8 5 0 1
maxProtocolRebalanceSlippage uint16 5 1 2
rewardsCollector address 5 3 20
isRewardAsset mapping(address => bool) 6 0 32
_swapper contract ISwapper 7 0 20
_yieldDistributor contract IYieldDistributor 8 0 20
safetyBufferYieldDeduction uint256 9 0 32
secondaryActionFee uint16 10 0 2
__gap uint256[49] 11 0 1568

You can see that the transient properties are not even there. The new secondaryActionFee is at the end even though it fits after _share address.


/**
* @notice Flag of withdraw action in the current transaction
*/
bool internal transient _withdrawnInTx;

// ========================================
// PERSISTENT
// ========================================

/**
* @notice Maximum basis points value representing 100%
*/
Expand Down Expand Up @@ -148,8 +166,16 @@ abstract contract BaseController is AccessControlUpgradeable, ReentrancyGuardTra
*/
uint256 public safetyBufferYieldDeduction;

/**
* @notice Additional fee applied to deposits/mints when a withdrawal/redemption has already occurred in the same
* transaction, and vice versa
* @dev The fee is expressed in basis points (bps) and is applied as a percentage of the asset price, increasing the
* cost of the second action in the same transaction to discourage "swapping" protocol collateral assets.
*/
uint16 public secondaryActionFee;

/**
* @notice Reserved storage space to allow for layout changes in the future.
*/
uint256[50] private __gap;
uint256[49] private __gap;
}
22 changes: 22 additions & 0 deletions src/controller/ConfigManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ abstract contract ConfigManager is BaseController {
* @notice Emitted when the maximum protocol rebalance slippage is updated
*/
event MaxProtocolRebalanceSlippageUpdated(uint256 oldMaxSlippage, uint256 newMaxSlippage);
/**
* @notice Emitted when the secondary action fee is updated
*/
event SecondaryActionFeeUpdated(uint256 oldSecondaryActionFee, uint256 newSecondaryActionFee);

/**
* @notice Thrown when attempting to set the rewards collector to the zero address
Expand All @@ -43,6 +47,10 @@ abstract contract ConfigManager is BaseController {
* @notice Thrown when attempting to set a max slippage that exceeds the maximum allowed
*/
error Config_InvalidMaxSlippage();
/**
* @notice Thrown when attempting to set a secondary action fee that exceeds the maximum allowed
*/
error Config_InvalidSecondaryActionFee();

/**
* @notice Internal initializer for the ConfigManager contract
Expand Down Expand Up @@ -103,4 +111,18 @@ abstract contract ConfigManager is BaseController {
// forge-lint: disable-next-line(unsafe-typecast)
maxProtocolRebalanceSlippage = uint16(newMaxSlippage);
}

/**
* @notice Updates the secondary action fee applied during deposits/withdrawals if a withdrawal/deposit has already
* occurred in the same transaction
* @dev Only addresses with CONFIG_MANAGER_ROLE can call this function
* @param newSecondaryActionFee The new secondary action fee in basis points (e.g., 100 = 1%)
*/
function setSecondaryActionFee(uint256 newSecondaryActionFee) external onlyRole(CONFIG_MANAGER_ROLE) {
require(newSecondaryActionFee < MAX_BPS, Config_InvalidSecondaryActionFee());
emit SecondaryActionFeeUpdated(secondaryActionFee, newSecondaryActionFee);
// casting to 'uint16' is safe because 'newSecondaryActionFee' is guaranteed to be less than 'MAX_BPS'
// forge-lint: disable-next-line(unsafe-typecast)
secondaryActionFee = uint16(newSecondaryActionFee);
}
}
4 changes: 4 additions & 0 deletions src/controller/Controller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ contract Controller is
notPaused
returns (uint256 shares)
{
_depositedInTx = true;
address vault = msg.sender;
VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: false });

Expand Down Expand Up @@ -354,6 +355,7 @@ contract Controller is
notPaused
returns (uint256 normalizedAssets)
{
_depositedInTx = true;
address vault = msg.sender;
VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: false });

Expand Down Expand Up @@ -386,6 +388,7 @@ contract Controller is
notPaused
returns (uint256 shares)
{
_withdrawnInTx = true;
address vault = msg.sender;
VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: true });

Expand Down Expand Up @@ -420,6 +423,7 @@ contract Controller is
notPaused
returns (uint256 normalizedAssets)
{
_withdrawnInTx = true;
address vault = msg.sender;
VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: true });

Expand Down
86 changes: 86 additions & 0 deletions tests/fork/ControllerUpgrade.fork.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.29;

import { Test } from "forge-std/Test.sol";

import {
ITransparentUpgradeableProxy
} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";

import { Controller } from "../../src/controller/Controller.sol";
import { GenericVault, IERC20 } from "../../src/vault/GenericVault.sol";
import { IChainlinkAggregatorLike } from "../../src/interfaces/IChainlinkAggregatorLike.sol";

contract ControllerUpgradeForkTest is Test {
address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
address constant USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7;
address constant USDS = 0xdC035D45d973E3EC169d2276DDab16f1e407384F;

address proxyAdminOwner = 0x3794d7f91b3Dd3b338FEe671aC6AA42BEA5e3D17;
ProxyAdmin proxyAdmin = ProxyAdmin(0x55835D50EbD78977a4686264E411f93E8ce75bFb);
Controller controller = Controller(0x3a64D23313E1bEAABa25Ec13149bD8D514C973Ae);

address user = makeAddr("user");

function setUp() public virtual {
vm.createSelectFork("mainnet");

deal(user, 1e18);
deal(USDC, user, 1_000_000e6);
}

function _upgradeController() internal {
Controller newImpl = new Controller();

vm.startPrank(proxyAdminOwner);
proxyAdmin.upgradeAndCall(ITransparentUpgradeableProxy(address(controller)), address(newImpl), "");
vm.stopPrank();
}

function test_shouldReturnSameStoredValues() public {
bool origPaused = controller.paused();
address origShare = controller.share();

address origUsdcMainVault = controller.vaultFor(USDC);
address origUsdtMainVault = controller.vaultFor(USDT);
address origUsdsMainVault = controller.vaultFor(USDS);

(IChainlinkAggregatorLike origUsdcPriceFeed,) = controller.priceFeeds(USDC);
(IChainlinkAggregatorLike origUsdtPriceFeed,) = controller.priceFeeds(USDT);
(IChainlinkAggregatorLike origUsdsPriceFeed,) = controller.priceFeeds(USDS);

_upgradeController();

(IChainlinkAggregatorLike newUsdcPriceFeed,) = controller.priceFeeds(USDC);
(IChainlinkAggregatorLike newUsdtPriceFeed,) = controller.priceFeeds(USDT);
(IChainlinkAggregatorLike newUsdsPriceFeed,) = controller.priceFeeds(USDS);

assertEq(controller.paused(), origPaused);
assertEq(controller.share(), origShare);
assertEq(controller.vaultFor(USDC), origUsdcMainVault);
assertEq(controller.vaultFor(USDT), origUsdtMainVault);
assertEq(controller.vaultFor(USDS), origUsdsMainVault);
assertEq(address(newUsdcPriceFeed), address(origUsdcPriceFeed));
assertEq(address(newUsdtPriceFeed), address(origUsdtPriceFeed));
assertEq(address(newUsdsPriceFeed), address(origUsdsPriceFeed));
}

function test_shouldWithdrawSameAfterUpgrade() public {
GenericVault vault = GenericVault(controller.vaultFor(USDC));

vm.startPrank(user);
IERC20(USDC).approve(controller.vaultFor(USDC), type(uint256).max);
uint256 shares = vault.deposit(1000e6, user);
vm.stopPrank();

uint256 origWithdrawnAmount = vault.previewRedeem(shares);

_upgradeController();

vm.prank(user);
uint256 newWithdrawnAmount = vault.redeem(shares, user, user);

assertEq(origWithdrawnAmount, newWithdrawnAmount);
}
}
20 changes: 20 additions & 0 deletions tests/harness/ControllerHarness.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ contract ControllerHarness is Controller {
return _totalAssetsDeltaToHitProportionality(proportionalityLimit, vaultAssets, totalAssets);
}

function exposed_depositedInTx() external view returns (bool) {
return _depositedInTx;
}

function exposed_withdrawnInTx() external view returns (bool) {
return _withdrawnInTx;
}

// ========================================
// WORKAROUND FUNCTIONS
// ========================================
Expand Down Expand Up @@ -235,4 +243,16 @@ contract ControllerHarness is Controller {
assert(maxSlippage <= MAX_BPS);
maxProtocolRebalanceSlippage = uint16(maxSlippage);
}

function workaround_setDepositedInTx(bool deposited) public {
_depositedInTx = deposited;
}

function workaround_setWithdrawnInTx(bool withdrawn) public {
_withdrawnInTx = withdrawn;
}

function workaround_setSecondaryActionFee(uint16 fee) public {
secondaryActionFee = fee;
}
}
22 changes: 22 additions & 0 deletions tests/helper/Multicall.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;

contract Multicall {
struct Call {
address target;
bytes callData;
}

function aggregate(Call[] calldata calls) public returns (bytes[] memory returnData) {
returnData = new bytes[](calls.length);
for (uint256 i; i < calls.length; ++i) {
(bool success, bytes memory data) = calls[i].target.call(calls[i].callData);
if (!success) {
assembly {
revert(add(data, 0x20), mload(data))
}
}
returnData[i] = data;
}
}
}
Loading
Loading