Skip to content

Commit

Permalink
Merge pull request #12 from yearn/audit_fix
Browse files Browse the repository at this point in the history 
fix: audit fixes
  • Loading branch information
@Schlagonia
Schlagonia authored May 23, 2024
2 parents 29cf9cd + 95febd0 commit d8881fb
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 58 deletions.
29 changes: 10 additions & 19 deletions src/L1Deployer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ contract L1Deployer is DeployerBase {
address _l2Deployer
) external virtual {
require(getRollupContract(_rollupID) == address(0), "registered");
require(_l1EscrowManager != address(0), "ZERO ADDRESS");
require(_l2Deployer != address(0), "ZERO ADDRESS");

IPolygonRollupContract _rollupContract = rollupManager
Expand Down Expand Up @@ -218,6 +219,8 @@ contract L1Deployer is DeployerBase {

/**
* @notice Creates a new custom vault and escrow for a specific asset on the specified rollup.
* @dev If the L1 escrow already exists the Rollup admin
* will need to update the vault manually on the escrow.
* @param _rollupID The ID of the rollup.
* @param _asset The address of the asset for which the vault and escrow are created.
* @return _l1Escrow The address of the L1 escrow.
Expand All @@ -233,7 +236,11 @@ contract L1Deployer is DeployerBase {
returns (address _l1Escrow, address _vault)
{
_vault = roleManager.newVault(_rollupID, _asset);
_l1Escrow = _newCustomVault(_rollupID, _asset, _vault);
// Deploy an L1 escrow if it does not already exist.
_l1Escrow = getEscrow(_rollupID, _asset);
if (_l1Escrow == address(0)) {
_l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
}
}

/**
Expand All @@ -250,23 +257,7 @@ contract L1Deployer is DeployerBase {
) external virtual onlyRollupAdmin(_rollupID) returns (address _l1Escrow) {
// Make sure the vault has been registered.
require(roleManager.isVaultsRoleManager(_vault), "!role manager");
_l1Escrow = _newCustomVault(_rollupID, _asset, _vault);
}

/**
* @dev Deploys an L1 Escrow for a custom vault if one does not exist.
* Will store all relevant information as well.
*/
function _newCustomVault(
uint32 _rollupID,
address _asset,
address _vault
) internal virtual returns (address _l1Escrow) {
_l1Escrow = getEscrow(_rollupID, _asset);

if (_l1Escrow == address(0)) {
_l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
}
_l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
}

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -366,7 +357,7 @@ contract L1Deployer is DeployerBase {
*/
function getEscrowManager(
uint32 _rollupID
) public view virtual returns (address) {
) external view virtual returns (address) {
return _chainConfig[_rollupID].escrowManager;
}

Expand Down
42 changes: 25 additions & 17 deletions src/L1YearnEscrow.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ contract L1YearnEscrow is L1Escrow {
);

// Max approve the vault
originTokenAddress().forceApprove(_vaultAddress, 2 ** 256 - 1);
IERC20(_originTokenAddress).forceApprove(_vaultAddress, 2 ** 256 - 1);
// Set the vault variable
VaultStorage storage $ = _getVaultStorage();
$.vaultAddress = IVault(_vaultAddress);
Expand All @@ -114,19 +114,14 @@ contract L1YearnEscrow is L1Escrow {
function _receiveTokens(
uint256 amount
) internal virtual override whenNotPaused {
originTokenAddress().safeTransferFrom(
msg.sender,
address(this),
amount
);
IERC20 originToken = originTokenAddress();
originToken.safeTransferFrom(msg.sender, address(this), amount);

VaultStorage storage $ = _getVaultStorage();
uint256 _minimumBuffer = $.minimumBuffer;
// Deposit to the vault if above buffer
if (_minimumBuffer != 0) {
uint256 underlyingBalance = originTokenAddress().balanceOf(
address(this)
);
uint256 underlyingBalance = originToken.balanceOf(address(this));

if (underlyingBalance <= _minimumBuffer) return;

Expand Down Expand Up @@ -181,7 +176,10 @@ contract L1YearnEscrow is L1Escrow {
// Check again to account for if there was loose underlying
if (amount > maxWithdraw) {
// Send an equivalent amount of shares for the difference.
uint256 shares = _vault.convertToShares(amount - maxWithdraw);
uint256 shares;
unchecked {
shares = _vault.convertToShares(amount - maxWithdraw);
}
_vault.transfer(destinationAddress, shares);
if (maxWithdraw == 0) return;
amount = maxWithdraw;
Expand Down Expand Up @@ -220,6 +218,8 @@ contract L1YearnEscrow is L1Escrow {
/**
* @dev Update the vault to deploy funds into.
* Will fully withdraw from the old vault.
* The current vault must be completely liquid for this to succeed.
*
* @param _vaultAddress Address of the new vault to use.
*/
function updateVault(
Expand Down Expand Up @@ -249,11 +249,14 @@ contract L1YearnEscrow is L1Escrow {
// Deposit any loose funds over minimum buffer
uint256 balance = originToken.balanceOf(address(this));
uint256 _minimumBuffer = $.minimumBuffer;
if (balance > _minimumBuffer)
IVault(_vaultAddress).deposit(
balance - _minimumBuffer,
address(this)
);
if (balance > _minimumBuffer) {
unchecked {
IVault(_vaultAddress).deposit(
balance - _minimumBuffer,
address(this)
);
}
}
}

// Update Storage
Expand Down Expand Up @@ -285,10 +288,15 @@ contract L1YearnEscrow is L1Escrow {

if (balance > _minimumBuffer) {
// Deposit the difference.
$.vaultAddress.deposit(balance - _minimumBuffer, address(this));
unchecked {
$.vaultAddress.deposit(balance - _minimumBuffer, address(this));
}
} else if (balance < _minimumBuffer) {
// Withdraw the difference
uint256 diff = _minimumBuffer - balance;
uint256 diff;
unchecked {
diff = _minimumBuffer - balance;
}
uint256 available = $.vaultAddress.maxWithdraw(address(this));

// Withdraw the min between the difference or what is available.
Expand Down
55 changes: 33 additions & 22 deletions src/RoleManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ contract RoleManager is Positions {
uint96 index;
}

/// @notice Make sure the vault has been added to the role manager.
modifier vaultIsAdded(address _vault) {
_vaultIsAdded(_vault);
_;
}

/// @notice Check if the vault is added to the Role Manager.
function _vaultIsAdded(address _vault) internal view virtual {
require(vaultConfig[_vault].asset != address(0), "vault not added");
}

/// @notice ID to use for the L1
uint32 internal constant ORIGIN_NETWORK_ID = 0;

Expand Down Expand Up @@ -185,15 +196,21 @@ contract RoleManager is Positions {
// Append the rollup ID for the name and symbol of custom vaults.
string memory _id = _rollupID == ORIGIN_NETWORK_ID
? ""
: string(abi.encodePacked("-", Strings.toString(_rollupID)));
: string.concat("-", Strings.toString(_rollupID));

// Name is "{SYMBOL}-STB yVault"
string memory _name = string(
abi.encodePacked(ERC20(_asset).symbol(), "-STB", _id, " yVault")
string memory _name = string.concat(
ERC20(_asset).symbol(),
"-STB",
_id,
" yVault"
);

// Symbol is "stb{SYMBOL}".
string memory _symbol = string(
abi.encodePacked("stb", ERC20(_asset).symbol(), _id)
string memory _symbol = string.concat(
"stb",
ERC20(_asset).symbol(),
_id
);

// Deploy through the registry so it is automatically endorsed.
Expand Down Expand Up @@ -378,10 +395,7 @@ contract RoleManager is Positions {
function updateDebtAllocator(
address _vault,
address _debtAllocator
) public virtual onlyPositionHolder(MANAGEMENT) {
// Make sure the vault has been added to the role manager.
require(vaultConfig[_vault].asset != address(0), "vault not added");

) public virtual vaultIsAdded(_vault) onlyPositionHolder(MANAGEMENT) {
// Remove the roles from the old allocator.
_setRole(_vault, Position(vaultConfig[_vault].debtAllocator, 0));

Expand All @@ -406,10 +420,7 @@ contract RoleManager is Positions {
function updateKeeper(
address _vault,
address _keeper
) external virtual onlyPositionHolder(MANAGEMENT) {
// Make sure the vault has been added to the role manager.
require(vaultConfig[_vault].asset != address(0), "vault not added");

) external virtual vaultIsAdded(_vault) onlyPositionHolder(MANAGEMENT) {
// Remove the roles from the old keeper if active.
address defaultKeeper = getPositionHolder(KEEPER);
if (
Expand All @@ -429,18 +440,16 @@ contract RoleManager is Positions {
*/
function removeVault(
address _vault
) external virtual onlyPositionHolder(CZAR) {
// Get the vault specific config.
VaultConfig memory config = vaultConfig[_vault];
// Make sure the vault has been added to the role manager.
require(config.asset != address(0), "vault not added");

) external virtual vaultIsAdded(_vault) onlyPositionHolder(CZAR) {
// Transfer the role manager position.
IVault(_vault).transfer_role_manager(chad);

// Address of the vault to replace it with.
address vaultToMove = vaults[vaults.length - 1];

// Get the vault specific config.
VaultConfig memory config = vaultConfig[_vault];

// Move the last vault to the index of `_vault`
vaults[config.index] = vaultToMove;
vaultConfig[vaultToMove].index = config.index;
Expand Down Expand Up @@ -470,7 +479,7 @@ contract RoleManager is Positions {
uint256 _role
) external virtual onlyPositionHolder(CZAR) {
address _vault;
for (uint256 i = 0; i < _vaults.length; ++i) {
for (uint256 i; i < _vaults.length; ++i) {
_vault = _vaults[i];
// Make sure the vault is added to this Role Manager.
require(vaultConfig[_vault].asset != address(0), "vault not added");
Expand Down Expand Up @@ -523,6 +532,8 @@ contract RoleManager is Positions {
function setDefaultProfitMaxUnlock(
uint256 _newDefaultProfitMaxUnlock
) external virtual onlyPositionHolder(GOVERNATOR) {
require(_newDefaultProfitMaxUnlock != 0, "too short");
require(_newDefaultProfitMaxUnlock <= 31_556_952, "too long");
defaultProfitMaxUnlock = _newDefaultProfitMaxUnlock;

emit UpdateDefaultProfitMaxUnlock(_newDefaultProfitMaxUnlock);
Expand Down Expand Up @@ -568,7 +579,7 @@ contract RoleManager is Positions {
* @param _asset The underlying asset used.
* @return The default vault for the specified `_asset`.
*/
function getVault(address _asset) public view virtual returns (address) {
function getVault(address _asset) external view virtual returns (address) {
return getVault(_asset, ORIGIN_NETWORK_ID);
}

Expand Down Expand Up @@ -600,7 +611,7 @@ contract RoleManager is Positions {
*/
function isVaultsRoleManager(
address _vault
) public view virtual returns (bool) {
) external view virtual returns (bool) {
return vaultConfig[_vault].asset != address(0);
}

Expand Down
2 changes: 2 additions & 0 deletions test/L1Deployer.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ contract L1DeployerTest is Setup {

IVault vault = IVault(_vault);

assertEq(vault.name(), "DAI-STB yVault");
assertEq(vault.symbol(), "stbDAI");
assertEq(vault.accountant(), address(accountant));
assertEq(vault.asset(), address(asset));
assertEq(vault.deposit_limit(), 2 ** 256 - 1);
Expand Down

0 comments on commit d8881fb

Please sign in to comment.