Skip to content

Commit

Permalink
paladin audit improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
RedVeil committed Jun 14, 2024
1 parent 8b9e6ce commit 9260ea2
Show file tree
Hide file tree
Showing 147 changed files with 6,785 additions and 13,391 deletions.
40 changes: 40 additions & 0 deletions audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Audit

## General
Besides test fixes we refactored the strategy contracts significantly.
What was previously Adapters is now called strategies.
The delegatecalls to external strategies are removed. All logic is now in the strategy contract itself.
The `harvest`-function is now permissioned. We added a keeper role. `harvest` can be called by `keeper` or `owner`. Aura, Balancer use shared inherited contracts stored in `./src/peripheral` same with Curve and Convex contracts.
Fees are moved from the strategies to the MultiStrategyVault.
Additionally strategies can now have a toggle `autoDeposit`. This changes if users deposit directly into the underlying protocol of a strategy or simply send the funds into the strategy itself.
Two new functions have been added `pushFunds` and `pullFunds` these allow the `owner` to deposit or withdraw funds from the underlying protocol. These functions can be used in conjunction with `autoDeposit` to control slippage that users may suffer better. Both functions allow the `owner` to send arbitrary data to the call which can be used for slippage protection or other safety features.
We also removed the Gearbox Strategies from the audit since they still need more work and additionally dont provide anything useful after the recent GearboxV3 leverage feature.

## Ignored Issues
### Issue 1 - Governance Privileges
We will ignore this issue since its an issue of setup and configuration. Now with more permissioned functions in strategies this becomes a bigger issue and must be taken care of in the contract setup.

### Issue 4 - The implementation of the proxies can be initialized
Its not entirely clear yet if we will simply construct and init strategies or create factories so we will add `_disableInitializers()` if it becomes necessary

### Issue 10 - Missing parameters in the NewStrategiesProposed event
We will keep these events simple. The proposed strategies can be read when reading the contract state.

### Issue 13 - _protocolWithdraw should be called after _burn
While the issue might be correct if the underlying protocol is attackable via reentrancy we need to call `_protocolWithdraw` before `_burn` so we can use `totalSupply` and asset or share conversions in `_protocolWithdraw` without any issues.

## Improvement Explanations

### wstETH Looper contract
Other than the fixing the issues reported in the audit, different improvements have been added to the contract logic, especially in the `_protocolWithdraw` flow, where few edge cases would fail. For example:
- Withdrawing `totalAssets - 1`: the `debtToRepay` was underestimated and so the tx would have reverted since not enough debt was repaid before withdrawing in order to keep the CDP collateralised.
We decided to refactor the `debtToRepay` amount by using the `ratio` of collateral being withdrawn with respect to the total (minus debt and slippage), applying the same ratio to the debt calculation.

- Withdrawing `totalAssets` would revert with `ERC4626ExceededMaxWithdraw` error for 1 Wei. Probably being a rounding mismatch between our contract and OZ base ones, `totalAssets` was patched by returning 1 wei less than the calculated amount (when that is greater than 0).

- `_swapToWETH_` function, called in order to repay the flashLoan, where part of the pulled `wstETH` collateral is swapped to `WETH`. Since the flash loan would fail if the received `WETH` amount is lower than the repayment amount, we need to apply a buffer that would account for eventual slippage.
That naturally would lead to having some extra `ETH` floating in the contract after the swap, which is restaked into `wstETH` in order to have the sufficient amount the user is withdrawing.

Apart from that, we added measures to make sure eventual dust balance in the contract is accounted for and used at the first useful interaction, in particular:
- `wstETH` floating balance held by the strategy: immediately deposited and added as collateral in the `_redepositAsset` function, triggered when the `adjustLeverage` is called.
- `ETH` floating balance: taken into account before taking a flashLoan to increase the leverage, is similarly deposited as collateral.
357 changes: 357 additions & 0 deletions backup.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[profile.default]
libs = ['node_modules', "lib"]
fs_permissions = [{ access = "read-write", path = "./"}]
fs_permissions = [{ access = "read", path = "./"}]
verbosity = 2
fuzz_runs = 256
fuzz_max_global_rejects = 100000000
Expand Down
46 changes: 22 additions & 24 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,30 +1,28 @@
{
"scripts": {
"anvil": "./script/anvil.sh",
"fund:usdc": "./script/fundUsdc.sh",
"fund:eCrv": "./script/fundECrv.sh",
"fund:pop": "./script/fundPop.sh",
"popStaking:fundOptimism": "./script/fundOpStaking.sh",
"demodata:vault": "forge script ./script/VaultDemoData.s.sol --rpc-url http://localhost:8545 --broadcast",
"deploy:vaultSystem": "forge script ./script/DeployVaultSystem.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:feeRecipient": "forge script ./script/DeployFeeRecipientProxy.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy": "./script/deploy.sh",
"vault:ragequit": "forge script ./script/SetRageQuit.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:proposeAdapter": "forge script ./script/ProposeAdapter.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:swapAdapter": "forge script ./script/SwapAdapter.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:deploy": "forge script ./script/DeployVault.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:deployAdapter": "forge script ./script/DeployAdapter.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:addTemplate": "forge script ./script/AddTemplate.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:proposeVaultFees": "forge script ./script/ProposeVaultFees.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:setPermission": "forge script ./script/SetPermission.s.sol --rpc-url https://opt-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:initializeStrategy": "forge script ./script/InitializeStrategy.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:initializeVault": "forge script ./script/InitializeVault.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:deployMultiStrategyVault": "forge script ./script/DeployMultiStrategyVault.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"vault:setHarvestValues": "forge script ./script/SetHarvestValues.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"fees:move": "forge script ./script/MoveFees.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"verify": "./script/verifyContract.sh",
"verifyContractValues":"./script/verifyContractValues.sh",
"allocateFunds": "forge script ./script/AllocateFunds.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast"
"allocateFunds": "forge script ./script/AllocateFunds.s.sol --rpc-url https://arb-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:vault": "forge script ./script/deploy/DeployMultiStrategyVault.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:AaveV3Depositor": "forge script ./script/deploy/aave/AaveV3Depositor.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:AuraCompounder": "forge script ./script/deploy/aura/AuraCompounder.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:BalancerCompounder": "forge script ./script/deploy/balancer/BalancerCompounder.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:BeefyDepositor": "forge script ./script/deploy/beefy/BeefyDepositor.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:CompoundV2Depositor": "forge script ./script/deploy/compound/v2/CompoundV2Depositor.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:CompoundV3Depositor": "forge script ./script/deploy/compound/v3/CompoundV3Depositor.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:ConvexCompounder": "forge script ./script/deploy/convex/ConvexCompounder.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:CurveGaugeCompounder": "forge script ./script/deploy/curve/CurveGaugeCompounder.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:CurveGaugeSingleAssetCompounder": "forge script ./script/deploy/curve/CurveGaugeSingleAssetCompounder.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageAave": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageAave.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageBalancer": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageBalancer.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageCompound": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageCompound.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageConvexBaseRewardPool": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageConvexBaseRewardPool.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageConvexBooster": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageConvexBooster.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageCurve": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageCurve.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageLido": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageLido.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageWstETH": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageWstETH.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:GearboxLeverageYearn": "forge script ./script/deploy/gearbox/leverageFarm/GearboxLeverageYearn.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:IonDepositor": "forge script ./script/deploy/ion/IonDepositor.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast",
"deploy:WstETHLooper": "forge script ./script/deploy/lido/WstETHLooper.s.sol:DeployStrategy --rpc-url https://eth-mainnet.g.alchemy.com/v2/KsuP431uPWKR3KFb-K_0MT1jcwpUnjAg --broadcast"
},
"dependencies": {}
}
30 changes: 0 additions & 30 deletions script/AllocateFunds.s.sol

This file was deleted.

60 changes: 0 additions & 60 deletions script/DeployMultiStrategyVault.s.sol

This file was deleted.

35 changes: 35 additions & 0 deletions src/interfaces/IBaseStrategy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-License-Identifier: GPL-3.0
// Docgen-SOLC: 0.8.25

pragma solidity ^0.8.25;

import {IERC4626} from "openzeppelin-contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol";
import {IOwned} from "./IOwned.sol";
import {IPermit} from "./IPermit.sol";
import {IPausable} from "./IPausable.sol";

interface IBaseStrategy is IERC4626, IOwned, IPermit, IPausable {
function harvest(bytes memory data) external;

function initialize(address asset_, address owner_, bool autoDeposit_, bytes memory adapterData_) external;

function decimals() external view returns (uint8);

function decimalOffset() external view returns (uint8);

function setKeeper(address keeper) external;

function keeper() external view returns (address);

function toggleAutoDeposit() external;

function autoDeposit() external view returns (bool);

function pushFunds(uint256 assets, bytes memory data) external;

function pullFunds(uint256 assets, bytes memory data) external;

function rewardTokens() external view returns (address[] memory);

function convertToUnderlyingShares(uint256 assets, uint256 shares) external view returns (uint256);
}
4 changes: 2 additions & 2 deletions src/interfaces/IEIP165.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.15;
pragma solidity ^0.8.25;

interface IEIP165 {
function supportsInterface(bytes4 interfaceId) external view returns (bool);
function supportsInterface(bytes4 interfaceId) external view returns (bool);
}
43 changes: 0 additions & 43 deletions src/interfaces/IMultiRewardEscrow.sol

This file was deleted.

Loading

0 comments on commit 9260ea2

Please sign in to comment.