Skip to content

Commit

Permalink
fix audit issues audit-6.3 audit-6.5 by removing exchangeAdjustmentRate
Browse files Browse the repository at this point in the history
  • Loading branch information
danoctavian committed Mar 22, 2024
1 parent d8a5c6e commit 2c1d767
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 148 deletions.
3 changes: 2 additions & 1 deletion scripts/forge/BaseScript.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ abstract contract BaseScript is Script, Utils {
PAUSE_ADMIN: vm.envAddress("PAUSER_ADDRESS"),
LSD_RESTAKING_MANAGER: vm.envAddress("LSD_RESTAKING_MANAGER_ADDRESS"),
STAKING_NODE_CREATOR: vm.envAddress("LSD_STAKING_NODE_CREATOR_ADDRESS"),
ORACLE_MANAGER: vm.envAddress("YIELDNEST_ORACLE_MANAGER_ADDRESS")
ORACLE_MANAGER: vm.envAddress("YIELDNEST_ORACLE_MANAGER_ADDRESS"),
DEPOSIT_BOOTSTRAPER: vm.envAddress("DEPOSIT_BOOTSTRAPER_ADDRESS")
});
}

Expand Down
6 changes: 0 additions & 6 deletions scripts/forge/DeployYieldNest.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ contract DeployYieldNest is BaseScript {
IDepositContract public depositContract;
IWETH public weth;

uint startingExchangeAdjustmentRate;

bytes ZERO_PUBLIC_KEY = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000";
bytes ONE_PUBLIC_KEY = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001";
bytes TWO_PUBLIC_KEY = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002";
Expand All @@ -72,9 +70,6 @@ contract DeployYieldNest is BaseScript {

feeReceiver = payable(_broadcaster); // Casting the default signer address to payable


startingExchangeAdjustmentRate = 4;

ContractAddresses contractAddresses = new ContractAddresses();
ContractAddresses.ChainAddresses memory chainAddresses = contractAddresses.getChainAddresses(block.chainid);
eigenPodManager = IEigenPodManager(chainAddresses.eigenlayer.EIGENPOD_MANAGER_ADDRESS);
Expand Down Expand Up @@ -120,7 +115,6 @@ contract DeployYieldNest is BaseScript {
pauser: actors.PAUSE_ADMIN,
stakingNodesManager: IStakingNodesManager(address(stakingNodesManager)),
rewardsDistributor: IRewardsDistributor(address(rewardsDistributor)),
exchangeAdjustmentRate: startingExchangeAdjustmentRate,
pauseWhitelist: pauseWhitelist
});
yneth.initialize(ynethInit);
Expand Down
3 changes: 0 additions & 3 deletions scripts/forge/DeployYnLSD.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ contract DeployYnLSD is BaseScript {
// solhint-disable-next-line no-console
console.log("Current Chain ID:", block.chainid);

uint256 startingExchangeAdjustmentRate = 0;

ContractAddresses contractAddresses = new ContractAddresses();
ContractAddresses.ChainAddresses memory chainAddresses = contractAddresses.getChainAddresses(block.chainid);
eigenPodManager = IEigenPodManager(chainAddresses.eigenlayer.EIGENPOD_MANAGER_ADDRESS);
Expand Down Expand Up @@ -80,7 +78,6 @@ contract DeployYnLSD is BaseScript {
strategyManager: strategyManager,
delegationManager: delegationManager,
oracle: yieldNestOracle,
exchangeAdjustmentRate: startingExchangeAdjustmentRate,
maxNodeCount: 10,
admin: actors.ADMIN,
pauser: actors.PAUSE_ADMIN,
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IStakingNode.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ interface IStakingEvents {
event Staked(address indexed staker, uint256 ethAmount, uint256 ynETHAmount);
event DepositETHPausedUpdated(bool isPaused);
event Deposit(address indexed sender, address indexed receiver, uint256 assets, uint256 shares);
event ExchangeAdjustmentRateUpdated(uint256 newRate);
}

interface IStakingNode {
Expand Down
33 changes: 4 additions & 29 deletions src/ynETH.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
error Paused();
error ValueOutOfBounds(uint256 value);
error ZeroAddress();
error ExchangeAdjustmentRateOutOfBounds(uint256 exchangeAdjustmentRate);
error ZeroETH();
error NoDirectETHDeposit();
error CallerNotStakingNodeManager(address expected, address provided);
Expand All @@ -38,9 +37,6 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
IRewardsDistributor public rewardsDistributor;
bool public depositsPaused;

/// @dev The value is in basis points (1/10000).
uint256 public exchangeAdjustmentRate;

uint256 public totalDepositedInPool;

//--------------------------------------------------------------------------------------
Expand All @@ -53,7 +49,6 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
address pauser;
IStakingNodesManager stakingNodesManager;
IRewardsDistributor rewardsDistributor;
uint256 exchangeAdjustmentRate;
address[] pauseWhitelist;
}

Expand All @@ -80,11 +75,6 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
stakingNodesManager = init.stakingNodesManager;
rewardsDistributor = init.rewardsDistributor;

if (init.exchangeAdjustmentRate > BASIS_POINTS_DENOMINATOR) {
revert ExchangeAdjustmentRateOutOfBounds(init.exchangeAdjustmentRate);
}
exchangeAdjustmentRate = init.exchangeAdjustmentRate;

_setTransfersPaused(true); // transfers are initially paused
_updatePauseWhitelist(init.pauseWhitelist, true);
}
Expand Down Expand Up @@ -130,16 +120,13 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
if (totalSupply() == 0) {
return ethAmount;
}

// deltaynETH = (1 - exchangeAdjustmentRate) * (ynETHSupply / totalControlled) * ethAmount
// If `(1 - exchangeAdjustmentRate) * ethAmount * ynETHSupply < totalControlled` this will be 0.

// Can only happen in bootstrap phase if `totalControlled` and `ynETHSupply` could be manipulated
// independently. That should not be possible.

// deltaynETH = (ynETHSupply / totalControlled) * ethAmount
return Math.mulDiv(
ethAmount,
totalSupply() * uint256(BASIS_POINTS_DENOMINATOR - exchangeAdjustmentRate),
totalAssets() * uint256(BASIS_POINTS_DENOMINATOR),
totalSupply(),
totalAssets(),
rounding
);
}
Expand Down Expand Up @@ -220,18 +207,6 @@ contract ynETH is IynETH, ynBase, IStakingEvents {
emit DepositETHPausedUpdated(depositsPaused);
}

/// @notice Sets the exchange adjustment rate.
/// @dev Can only be called by the admin..
/// Reverts if the new rate exceeds the basis points denominator.
/// @param newRate The new exchange adjustment rate to be set.
function setExchangeAdjustmentRate(uint256 newRate) external onlyRole(DEFAULT_ADMIN_ROLE) {
if (newRate > BASIS_POINTS_DENOMINATOR) {
revert ValueOutOfBounds(newRate);
}
exchangeAdjustmentRate = newRate;
emit ExchangeAdjustmentRateUpdated(newRate);
}

//--------------------------------------------------------------------------------------
//---------------------------------- MODIFIERS ---------------------------------------
//--------------------------------------------------------------------------------------
Expand Down
19 changes: 5 additions & 14 deletions src/ynLSD.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {

error UnsupportedAsset(IERC20 asset);
error ZeroAmount();
error ExchangeAdjustmentRateOutOfBounds(uint256 exchangeAdjustmentRate);
error ZeroAddress();
error BeaconImplementationAlreadyExists();
error NoBeaconImplementationExists();
Expand Down Expand Up @@ -64,8 +63,6 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {

/// @notice List of supported ERC20 asset contracts.
IERC20[] public assets;

uint256 public exchangeAdjustmentRate;

/**
* @notice Array of LSD Staking Node contracts.
Expand All @@ -89,7 +86,6 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
IStrategyManager strategyManager;
IDelegationManager delegationManager;
YieldNestOracle oracle;
uint256 exchangeAdjustmentRate;
uint256 maxNodeCount;
address admin;
address pauser;
Expand Down Expand Up @@ -130,11 +126,6 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
strategyManager = init.strategyManager;
delegationManager = init.delegationManager;
oracle = init.oracle;

if (init.exchangeAdjustmentRate > BASIS_POINTS_DENOMINATOR) {
revert ExchangeAdjustmentRateOutOfBounds(init.exchangeAdjustmentRate);
}
exchangeAdjustmentRate = init.exchangeAdjustmentRate;
maxNodeCount = init.maxNodeCount;

_setTransfersPaused(true); // transfers are initially paused
Expand Down Expand Up @@ -199,7 +190,7 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
/**
* @dev Converts an ETH amount to shares based on the current exchange rate and specified rounding method.
* If it's the first stake (bootstrap phase), uses a 1:1 exchange rate. Otherwise, calculates shares based on
* the formula: deltaynETH = (1 - exchangeAdjustmentRate) * (ynETHSupply / totalControlled) * ethAmount.
* the formula: deltaynETH = (ynETHSupply / totalControlled) * ethAmount.
* This calculation can result in 0 during the bootstrap phase if `totalControlled` and `ynETHSupply` could be
* manipulated independently, which should not be possible.
* @param ethAmount The amount of ETH to convert to shares.
Expand All @@ -213,15 +204,14 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
return ethAmount;
}

// deltaynETH = (1 - exchangeAdjustmentRate) * (ynETHSupply / totalControlled) * ethAmount
// If `(1 - exchangeAdjustmentRate) * ethAmount * ynETHSupply < totalControlled` this will be 0.
// deltaynETH = (ynETHSupply / totalControlled) * ethAmount

// Can only happen in bootstrap phase if `totalControlled` and `ynETHSupply` could be manipulated
// independently. That should not be possible.
return Math.mulDiv(
ethAmount,
totalSupply() * uint256(BASIS_POINTS_DENOMINATOR - exchangeAdjustmentRate),
totalAssets() * uint256(BASIS_POINTS_DENOMINATOR),
totalSupply(),
totalAssets(),
rounding
);
}
Expand Down Expand Up @@ -465,3 +455,4 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
_;
}
}

6 changes: 0 additions & 6 deletions test/foundry/integration/IntegrationBaseTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ contract IntegrationBaseTest is Test, Utils {
bytes constant ZERO_SIGNATURE = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000";
bytes32 constant ZERO_DEPOSIT_ROOT = bytes32(0);

uint256 startingExchangeAdjustmentRate = 4;

// Utils
ContractAddresses public contractAddresses;
ContractAddresses.ChainAddresses public chainAddresses;
Expand Down Expand Up @@ -165,7 +163,6 @@ contract IntegrationBaseTest is Test, Utils {
pauser: actors.PAUSE_ADMIN,
stakingNodesManager: IStakingNodesManager(address(stakingNodesManager)),
rewardsDistributor: IRewardsDistributor(address(rewardsDistributor)),
exchangeAdjustmentRate: startingExchangeAdjustmentRate,
pauseWhitelist: pauseWhitelist
});

Expand Down Expand Up @@ -251,16 +248,13 @@ contract IntegrationBaseTest is Test, Utils {
});
yieldNestOracle.initialize(oracleInit);

uint startingExchangeAdjustmentRateForYnLSD = 0;

LSDStakingNode lsdStakingNodeImplementation = new LSDStakingNode();
ynLSD.Init memory init = ynLSD.Init({
assets: assets,
strategies: strategies,
strategyManager: strategyManager,
delegationManager: delegationManager,
oracle: yieldNestOracle,
exchangeAdjustmentRate: startingExchangeAdjustmentRateForYnLSD,
maxNodeCount: 10,
admin: actors.ADMIN,
stakingAdmin: actors.STAKING_ADMIN,
Expand Down
45 changes: 2 additions & 43 deletions test/foundry/integration/Upgrades.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ contract UpgradesTest is IntegrationBaseTest {
IRewardsDistributor originalRewardsDistributor = yneth.rewardsDistributor();
uint originalAllocatedETHForDeposits = yneth.totalDepositedInPool();
bool originalIsDepositETHPaused = yneth.depositsPaused();
uint originalExchangeAdjustmentRate = yneth.exchangeAdjustmentRate();
uint originalTotalDepositedInPool = yneth.totalDepositedInPool();

MockERC20 nETH = new MockERC20("Nest ETH", "nETH");
Expand All @@ -122,7 +121,6 @@ contract UpgradesTest is IntegrationBaseTest {
assertEq(address(upgradedYnETH.rewardsDistributor()), address(originalRewardsDistributor), "RewardsDistributor mismatch");
assertEq(upgradedYnETH.totalDepositedInPool(), originalAllocatedETHForDeposits, "AllocatedETHForDeposits mismatch");
assertEq(upgradedYnETH.depositsPaused(), originalIsDepositETHPaused, "IsDepositETHPaused mismatch");
assertEq(upgradedYnETH.exchangeAdjustmentRate(), originalExchangeAdjustmentRate, "ExchangeAdjustmentRate mismatch");
assertEq(upgradedYnETH.totalDepositedInPool(), originalTotalDepositedInPool, "TotalDepositedInPool mismatch");

assertEq(finalTotalAssets, yneth.totalAssets(), "Total assets mismatch after upgrade");
Expand All @@ -134,38 +132,7 @@ contract UpgradesTest is IntegrationBaseTest {
upgradedYnETH.deposit(nETHDepositAmount, address(this));
}

function testUpgradeYnETHRevertswithInvalidAdjustmentRate() public {

TransparentUpgradeableProxy ynethProxy;
yneth = ynETH(payable(ynethProxy));

// Re-deploying ynETH and creating its proxy again
yneth = new ynETH();
ynethProxy = new TransparentUpgradeableProxy(address(yneth), actors.PROXY_ADMIN_OWNER, "");
yneth = ynETH(payable(ynethProxy));


address[] memory pauseWhitelist = new address[](1);
pauseWhitelist[0] = actors.TRANSFER_ENABLED_EOA;

uint256 invalidRate = 100000000000000000000;

ynETH.Init memory ynethInit = ynETH.Init({
admin: actors.ADMIN,
pauser: actors.PAUSE_ADMIN,
stakingNodesManager: IStakingNodesManager(address(stakingNodesManager)),
rewardsDistributor: IRewardsDistributor(address(rewardsDistributor)),
exchangeAdjustmentRate: invalidRate,
pauseWhitelist: pauseWhitelist
});

bytes memory encodedError = abi.encodeWithSelector(ynETH.ExchangeAdjustmentRateOutOfBounds.selector, invalidRate);

vm.expectRevert(encodedError);
yneth.initialize(ynethInit);
}

function setupInitializeYnLSD(uint256 adjustmentRate, address assetAddress) internal returns (ynLSD.Init memory, ynLSD ynlsd) {
function setupInitializeYnLSD(address assetAddress) internal returns (ynLSD.Init memory, ynLSD ynlsd) {
TransparentUpgradeableProxy ynlsdProxy;
ynlsd = ynLSD(payable(ynlsdProxy));

Expand Down Expand Up @@ -203,7 +170,6 @@ contract UpgradesTest is IntegrationBaseTest {
strategyManager: strategyManager,
delegationManager: delegationManager,
oracle: yieldNestOracle,
exchangeAdjustmentRate: adjustmentRate,
maxNodeCount: 10,
admin: actors.ADMIN,
stakingAdmin: actors.STAKING_ADMIN,
Expand All @@ -218,16 +184,9 @@ contract UpgradesTest is IntegrationBaseTest {
}

function testYnLSDInitializeRevertsAssetAddressZero() public {
(ynLSD.Init memory init, ynLSD ynlsd) = setupInitializeYnLSD(1000, address(0));
(ynLSD.Init memory init, ynLSD ynlsd) = setupInitializeYnLSD(address(0));
bytes memory encodedError = abi.encodeWithSelector(ynLSD.ZeroAddress.selector);
vm.expectRevert(encodedError);
ynlsd.initialize(init);
}

function testYnLSDInitializeRevertsInvalidAdjustmentRate() public {
(ynLSD.Init memory init, ynLSD ynlsd) = setupInitializeYnLSD(1 ether, chainAddresses.lsd.STETH_ADDRESS);
bytes memory encodedError = abi.encodeWithSelector(ynLSD.ExchangeAdjustmentRateOutOfBounds.selector, 1 ether);
vm.expectRevert(encodedError);
ynlsd.initialize(init);
}
}
32 changes: 7 additions & 25 deletions test/foundry/integration/ynETH.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ contract ynETHIntegrationTest is IntegrationBaseTest {
// Act
uint256 sharesAfterFirstDeposit = yneth.previewDeposit(secondDepositAmount);

uint256 expectedShares = Math.mulDiv(secondDepositAmount, 10000 - startingExchangeAdjustmentRate, 10000, Math.Rounding.Floor);
uint256 expectedShares = secondDepositAmount;

// Assert
assertEq(sharesAfterFirstDeposit, expectedShares, "Fuzz: Shares should match expected shares");
Expand Down Expand Up @@ -136,7 +136,7 @@ contract ynETHIntegrationTest is IntegrationBaseTest {
assertEq(totalAssetsAfterSecondDeposit, expectedTotalAssets, "Total assets should match expected total after second deposit");

// Assuming initial total supply equals shares after first deposit
uint256 expectedTotalSupply = firstDepositAmount + secondDepositAmount - startingExchangeAdjustmentRate * secondDepositAmount / 10000;
uint256 expectedTotalSupply = firstDepositAmount + secondDepositAmount;
uint256 totalSupplyAfterSecondDeposit = yneth.totalSupply();
// TODO: figure out this precision issue
assertTrue(compareWithThreshold(totalSupplyAfterSecondDeposit, expectedTotalSupply, 1), "Total supply should match expected total supply after second deposit");
Expand All @@ -146,11 +146,10 @@ contract ynETHIntegrationTest is IntegrationBaseTest {
uint256 sharesAfterSecondDeposit = yneth.previewDeposit(thirdDepositAmount);

// Using the formula from ynETH to calculate expectedShares
// Assuming exchangeAdjustmentRate is applied as in the _convertToShares function of ynETH
uint256 expectedShares = Math.mulDiv(
thirdDepositAmount,
expectedTotalSupply * uint256(10000 - startingExchangeAdjustmentRate),
expectedTotalAssets * uint256(10000),
expectedTotalSupply,
expectedTotalAssets,
Math.Rounding.Floor
);

Expand Down Expand Up @@ -178,11 +177,10 @@ contract ynETHIntegrationTest is IntegrationBaseTest {
uint256 expectedTotalAssets = ethAmount + expectedNetRewardAmount; // Assuming initial total assets were equal to ethAmount before rewards
uint256 expectedTotalSupply = ethAmount; // Assuming initial total supply equals shares after first deposit
// Using the formula from ynETH to calculate expectedShares
// Assuming exchangeAdjustmentRate is applied as in the _convertToShares function of ynETH
uint256 expectedShares = Math.mulDiv(
ethAmount,
expectedTotalSupply * uint256(10000 - startingExchangeAdjustmentRate),
expectedTotalAssets * uint256(10000),
expectedTotalSupply,
expectedTotalAssets,
Math.Rounding.Floor
);

Expand Down Expand Up @@ -358,21 +356,5 @@ contract ynETHIntegrationTest is IntegrationBaseTest {
vm.expectRevert(encodedError);
yneth.withdrawETH(1);
vm.stopPrank();
}

function testSetExchangeAdjustmentRate() public {
uint256 newRate = 1000;
vm.prank(address(actors.ADMIN));
yneth.setExchangeAdjustmentRate(newRate);
assertEq(yneth.exchangeAdjustmentRate(), newRate);
}

function testSetExchangeAdjustmentRateWithInvalidRate() public {
uint256 invalidRate = 100000000000000000000;
bytes memory encodedError = abi.encodeWithSelector(ynETH.ValueOutOfBounds.selector, invalidRate);
vm.prank(address(actors.ADMIN));
vm.expectRevert(encodedError);
yneth.setExchangeAdjustmentRate(invalidRate);
vm.stopPrank();
}
}
}
Loading

0 comments on commit 2c1d767

Please sign in to comment.