- Review Summary
- Code Evaluation Matrix
- Findings Explanation
- Critical Findings
- High Findings
- Medium Findings
- Low Findings
- 1. Low -
maxRedeem()are inaccurate if Aave pool doesn’t permit withdrawals
- 2. Low -
maxDeposit()are inaccurate if Aave pool is frozen
- 3. Low -
setGmxParams()doesn’t update dependent variable
- 4. Low - Biased delta neutral position
- 1. Low -
- Gas Savings Findings
- 1. Gas - Make functions external
- 2. Gas - Immutable variables are cheaper
- 3. Gas - Remove unused function
- 4. Gas - Remove duplicate length check
- Informational Findings
- 1. Informational - Unnecessary addition operation
- 2. Informational - No way to remove approvals
- 3. Informational - Move Aave pool and oracle update to new function
- 4. Informational - Unused dependencies imported
- 5. Informational - Hypothetical revert condition breaks EIP4626 requirements
- 6. Informational - Replace magic numbers with constants
- 7. Informational - Unnecessary reimplementation of GMX functions
- 8. Informational - Library functions with unusual visibility
- 9. Informational - Functions altering state variables missing events
- 10. Informational - Dead link in
- 11. Informational - Move
calculateFeeSplit()into if statement
- Final remarks
Rage Trade provides two vaults, one risk-on and one risk-off, which allow users to deposit into and earn yield. The risk-off vault earns yield from lending USDC on Aave and a fraction of the risk-on vault rewards. The risk-on vault earns rewards by providing delta neutral liquidity to GMX, hedging the ETH and BTC exposure with short positions on Aave and Uniswap. The risk-on vault acts as a junior tranche and borrows from the risk-off vault (senior tranche) to gain leverage.
The contracts of the Rage Trade Repo were reviewed over 3 weeks. The code review was performed by 3 auditors between December 19, 2022 and January 16, 2023. The repository was under active development during the review, but the review was limited to the latest commit at the start of the review. This was commit a2107d37b789494454bd4ede7d217d8723474de4 for the Rage Trade repo.
The scope of the review consisted of all the contracts in the repo at the specific commit. The senior tranche makes reference to a
leveragePool address that is currently left empty for future use, and therefore it was out of scope. After the findings were presented to the Rage Trade team, fixes were made and included in several PRs.
This review is a code review to identify potential vulnerabilities in the code. The reviewers did not investigate security practices or operational security and assumed that privileged accounts could be trusted. The reviewers did not evaluate the security of the code relative to a standard or specification. The review may not have identified all potential attack vectors or areas of vulnerability.
yAudits and the auditors make no warranties regarding the security of the code and do not warrant that the code is free from defects. yAudit and the auditors do not represent nor imply to third parties that the code has been audited nor that the code is free from defects. By deploying or using the code, Rage Trade and users of the contracts agree to use the code at their own risk.
|Access Control||Average||Several access control modifiers for privileged roles exist, including
|Mathematics||Average||No specialized algorithms or custom math operations are needed for Rage Trade, but there are a substantial number of internal accounting values that are used. The math backing the delta neutral strategy adds some complexity, and some values like the senior vault leverage percentage are not hard coded but rather implied based on the process of borrowing and lending that takes place. However, such math and complexity is not easily avoided for a delta neutral strategy.|
|Complexity||Low||There is significant complexity introduced by the many tokens swaps, the different decimals, the way the contracts are organized, and the implied assumptions in the design that were found with backtesting. Refactoring the code in places to simplify it would assist with future security reviews and may improve the overall health of the protocol so other developers can more easily onboard and understand the design.|
|Libraries||Average||Math libraries from Uniswap, Aave, and Rari are used. Openzeppelin libraries allow for pausing contracts and transferring ownership. This is a standard number of external dependencies, but relying on external dependencies can introduce additional complexity to a project. This case the complexity of three different math libraries is worth noting.|
|Decentralization||Average||Different roles have different privilege levels, and the use of a proxy adds to the power that the owner has to make changes. However, this is relatively standard for a protocol that is not aiming for an immutable deployment.|
|Code stability||Average||Rage Trade has launched on mainnet, but some new upgrades will be completed in January after this review is completed. Because the deployed contracts use proxies, upgrades are not too difficult.|
|Documentation||Average||There is consistent NatSpec in the code, but the official docs online are lacking in providing an overview of the big picture of how the strategy works. For example, there is no explanation how the 2.3x leverage number for the senior vault is achieved, how the rewards get converted after depositing into GMX, and how the reward split between the senior tranche and junior tranche is calculated.|
|Monitoring||Average||Most functions that alter state variables emit events, but some do not. For example,
|Testing and verification||Average||The tests maintain good coverage and all tests pass. The test coverage is over 80% for the main vault contracts and libraries. The tests even check for strategy performance at different asset price points to test for adverse market conditions. Test coverage for DepositPeriphery and WithdrawPeriphery is low, below 50%, and should be improved.|
Areas of Focus: Fuzzing campaigns were run to ensure that the expected ERC4626 properties were met, as well as making sure the vaults code did not revert in unexpected circumstances. The results of the fuzzing campaigns can be seen below. Note that some of the campaigns need more work to establish if the scenario passes (the result is marked as “still testing” in these cases).
Contracts Tested: DnGmxJuniorVault.sol (J), DnGmxSeniorVault.sol (S), DepositPeriphery.sol (D), WithdrawPeriphery.sol (W)
Applicable Standards: EIP-4626 (J,S)
|J, S||TotalAssets will never revert||Passed|
|J, S||ConvertToAssets will never revert*||Passed|
|J, S||ConvertToShares will never revert*||Passed|
|J, S||Division rounding errors always favors the protocol||Passed|
|J, S||Withdraw will never revert**||Passed|
|J, S||Redeem will never revert**||Passed|
|J, S||Price per share will never go down||Still testing|
|J, D, W||Manipulating Uniswap pools to decrease price of WETH/WBTC should not favour user deposit/withdrawals||Passed|
|J, D, W||Manipulating Uniswap pools to increase price of WETH/WBTC should not favour user deposit/withdrawals||Still testing|
|J, D, W||Natural price decrease of WETH/WBTC should not should not favour user deposit/withdrawals||Still testing|
|J, D, W||Natural price increase of WETH/WBTC should not should not favour user deposit/withdrawals||Still testing|
* Given that preconditions have been met
** There are a lot of preconditions that needs to be satisfied when a deposit/withdrawal is being executed against one of the delta neutral vaults or the periphery contracts, some of them outside of the users control (oracle price/slippage during flash loan to balance the hedge etc.). These fuzzing campaigns attempted to control for these factors to see if any scenario could occur where a withdrawal/redeem was expected to be possible but didn’t.
Findings are broken down into sections by their respective impact:
- Critical, High, Medium, Low impact
- These are findings that range from attacks that may cause loss of funds, impact control/ownership of the contracts, or cause any unintended consequences/actions that are outside the scope of the requirements
- Gas savings
- Findings that can improve the gas efficiency of the contracts
- Findings including recommendations and best practices
The goal of the Rage Trade Delta Neutral strategy is to maintain a delta neutral position to avoid exposure to impermanent loss. The long and short positions in the strategy are not balanced, meaning the strategy is not delta neutral.
Rage Trade relies on providing liquidity to the GMX protocol for long exposure to WETH and WBTC. The GMX Vault is where all GMX protocol underlying assets are stored. The GMX vault has a target weight for each underlying asset, but the target weight is not the same as the current weight of the asset in the vault during a specific block. The actual weights of the tokens The Rage Trade rebalancing approach uses the ideal target weight, not the actual current weight of the asset. This means the Rage trade strategy is not actually delta neutral because it assumes an ideal mix of underlying assets when holding GLP and doesn’t consider the current underlying assets.
To demonstrate the issue, data was taken from the Arbitrum blockchain to plot the exposure that Rage Trade has had to the underlying assets WETH and WBTC since around the time that the delta neutral strategy began receiving funds. The short exposure takes the form of WETH or WBTC borrowed from Aave. To calculate this percentage, the amount of WETH or WBTC borrowed was multiplied by the price of the asset and then divided by the total value of GLP held by the Rage trade strategy. The short exposure does a good job of targeting the 35% and 15% marks for WETH and WBTC respectively. The 35% and 15% marks are the target allocations for the GMX vault to hold of WETH and WBTC. The long exposure takes the form of GLP, which holds roughly 50% stablecoins and 50% WETH and WBTC. The data for the exposure to WETH and WBTC was taken from at stats.gmx.io during the same period as the short position exposure chart. The comparison of the short and long exposure to WETH and WBTC is shown below.
As the charts demonstrate, since the deployment of Rage Trade in December 2022, the GMX vault, and therefore GLP, has consistently held more WBTC and less WETH than the GMX vault target weights. But the Rage Trade short positions do not reflect this, and instead use the target weights to determine the ideal short exposure to these assets. In its current form, Rage Trade has held a position of long WBTC and short WETH since it was deployed.
Examining the asset exposure in the GMX Vault over the entire duration of GMX’s history, it is more apparent how far the asset allocations can stray from the target. The second figure below focuses on the less volatile period of GMX asset allocation in 2022 to provide a closer look at how different the asset allocation is from the 35% WETH and 15% WBTC targets. Since June 2022, the GMX vault has mostly been overweight in WBTC and underweight in WETH, but sometimes it is overweight in both assets.
Focusing on more “extreme” changes in the asset allocations of the vault, the vault held a total of 39.4% in WETH and WBTC combined on May 26 2022, but that rose to 54% on July 24 2022, less than 2 months later. If this strategy was active at this time, it would have had too large of a short position and too small of a long position. It is true that during this time the value of WETH and WBTC dropped by around 22%, so in actuality these months would have been profitable for this strategy, but a reverse scenario where the assets increased by 22% in value would have left the strategy at a loss.
Even if the above scenario where the strategy is weighted in the wrong direction of market movements does not occur, another risk is if the correlation between WETH and WBTC prices change significantly. It’s possible there is an implied assumption that WETH and WBTC prices are correlated, so as long as the sum of allocations WETH and WBTC in the GMX vault is roughly 50% of the GMX vault, then the long position should neutralize the short position. But if the WETH/WBTC price correlation changes while they GMX vault allocations are not near the targets, this could be problematic. The chart from CoinGecko shows this ratio over the last two years and demonstrates that it can easily change by 10% or more in a short timespan.
In summary, the risk this issue highlights includes:
- strategy losses from when WETH and WBTC positions are overweighted towards a long position and the prices of these assets drops. The reverse can also happen, where WETH and WBTC positions are underweighted towards a long position and the prices of these assets rises.
- strategy losses from when WETH and WBTC weights in the GMX vault are not at the target weights and the WETH/WBTC price ratio changes in the wrong direction from how the GMX vault (and therefore the Rage Trade strategy) is weighted.
High. The strategy does not properly calculate the exposure necessary to maintain a delta neutral position, which leaves depositors vulnerable to losses that can vary depending on market conditions and GMX vault weights.
A few improvements can be made:
- At the core, the mismatch in long and short positions stems from using
tokenWeights()to determine the short exposure needed to balance out the GLP holding of WETH and WBTC.
tokenWeights()is the ideal weighting, not the current allocation to WETH and WBTC, so the current allocation should be used instead.
- The reweighting process happens at a certain time interval. This result in miniscule changes in asset allocation, as shown in the plot in issue Low 4, which cause allocation changes that are almost irrelevant compared to the mismatch between the target GMX vault weights and the current weights. Instead of using a timer to trigger rebalancing, it would be better to set a threshold related to how unbalanced the positions are. For example, if the difference between the long and short WETH (or WBTC) positions exceeds 0.5% of the GLP value held, then the positions should be rebalanced. This would keep the strategy more neutral with the current GMX vault weights, and even prevent a scenario where the WETH and WBTC prices and vault weights change significantly right after a Rage Trade rebalance happens, which would cause a roughly 12 hour wait time before the next rebalance occurs. The threshold value can be set based on backtesting to account for swap fees and other factors.
Fixed, junior vault now hedges current weights and partial trader PnL.
The junior vault’s
getPriceX128() returns a value with an incorrect number of decimals.
The term X128 as used in Uniswap documentation indicates a Q128 value. This means that the value should be divided by
1 << 128 to get the actual value that it represents.
getPriceX128() in the junior vault finds a problem. The return value is
aum.mulDiv(1 << 128, totalSupply * 1e24). This is similar to the previous junior vault example.
aum is 1E12, but the denominator is
totalSupply of 1E18 multiplied by 1E24. This leaves the numerator with 1E12 but the denominator with 1E42, the resulting values is not correct. When the value of
getPriceX128() in the existing on-chain deployment is divided by
1 << 128, the resulting value is
8.34E-13. In contrast, the price of GLP is
$0.834 and the return value of
getPrice(bool) in the junior vault with 18 decimals like GLP is
We can apply the same approach to
getPriceX128() in the senior vault and find that it is implemented correctly. We can double check the decimals of the return value
price.mulDiv(1 << 128, 1e8).
price is 1E8 and it is divided by 1E8, so the price value is properly multiplied by
1 << 128.
We can also confirm the decimals in
getPrice(bool) in the junior vault are correct. The price value is documented in GMX docs:
The sell price would be getAum(false) / glpSupply
aum.mulDivDown(PRICE_PRECISION, totalSupply * 1e24).
aum is 1E12,
PRICE_PRECISION is 1E30,
totalSupply is 1E18 and it is multiplied by 1E24. This results in
1E12 * 1E30 / (1E18 * 1E24) = 1E42 / 1E42 = 1. And because GLP has a decimals value of 18, the value is returned with 18 decimals. The return value can be confirmed against the GLP price in the GMX frontend.
getPriceX128() in the junior vault is not implemented correctly and returns a value with incorrect decimals. While
getPriceX128() in the senior vault is intended only for frontend use, the junior vault does not document the intended use of this function.
Change the return value of
getPriceX128() in the junior vault to return a value with correct decimals.
- return aum.mulDiv(1 << 128, totalSupply * 1e24); + return aum.mulDiv(1 << 128, 1e12);
getPriceX128 returns the price of 1wei GLP when divided by 1«128 in usdc. So that when glp balance is multiplied by price and divided by 1«128 we get the value.
aum is in
totalSupply is in
glp is in
1e6. Hence 18+6/(18+30) since price * glpBalance should give usdc value.
current getPriceX128 => 327756966734451170974722619 getPriceX128 / (1 << 128) => 9.631911570974895e-13 (which is for 1 wei of glp) so, 1e18 wei of GLP (1 GLP) = 963191.1570974895 (in least divisible unit of USDC)
If an Aave pool is paused or made inactive,
maxRedeem() will not return the correct amount of tokens that can be redeemed.
Maximum amount of Vault shares that can be redeemed from the owner balance in the Vault, through a redeem call.
MUST return the maximum amount of shares that could be transferred from owner through redeem and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).
MUST factor in both global and user-specific limits, like if redemption is entirely disabled (even temporarily) it MUST return 0.
If an Aave pool is paused or inactive,
maxRedeem() in the senior vault should return zero. The functions currently do not. Currently the senior vault is not compliant with the ERC4626 spec if the Aave pool is inactive or paused.
maxDeposit() should also have a similar change.
maxRedeem() will return a non-zero value even if the pool is paused or inactive and cannot be withdrawn from.
Consider implementing a check in
maxWithdraw() like this one that checks if the pool is inactive or paused and returns zero if so.
Acknowledged, not fixing. redemption of shares is not disabled by the senior vault itself in this case but the downstream dependent protocols which vault has no direct control of.
A frozen Aave pool does not allow any new supply, which would prevent depositing and minting actions of the senior vault.
MUST return the maximum amount of shares mint would allow to be deposited to receiver and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary). This assumes that the user has infinite assets, i.e. MUST NOT rely on balanceOf of asset.
MUST factor in both global and user-specific limits, like if mints are entirely disabled (even temporarily) it MUST return 0.
If an Aave pool is frozen, Aave documentation states it will not accept deposits. Therefore
maxDeposit() should return zero in this case. The functions currently do not. Currently the senior vault is not compliant with the ERC4626 spec if the Aave pool is frozen.
maxDeposit() will return a non-zero value even if the pool is frozen and cannot be deposited to.
Consider implementing a check in
maxDeposit() like this one that checks if the pool is frozen and returns zero if so.
Acknowledged, not fixing. minting of shares is not disabled by the senior vault itself in this case but the downstream dependent protocols which vault has no direct control of.
state.glpManager to be modified. However, it fails to update values that are dependent on
state.glpManager can be updated in
setGmxParams() by the owner. The value
state.gmxVault is set in
initialize() by calling
state.glpManager is updated, the value
state.gmxVault is not updated, which may leave the gmx vault variable outdated.
setGmxParams() doesn’t change variables that are dependent on the glpManager.
Consider adding this line to
state.gmxVault = IVault(state.glpManager.vault());
gmxVault is not an upgradable contract, so any glpManager address set via
setGmxParams will use same canonical underlying
gmxVault with which junior vault was initialized.
The delta neutral position taken by the junior vault has some imperfections in it. While these are not likely to cause major problems during normal market conditions, the risks of an unbalanced position during certain market edge cases should be considered.
The junior vault attempts to hold equal long and short positions to maintain a delta neutral position in the market. The reason this is done is to avoid impermanent loss, which is a common problem for liquidity providers. The junior vault holds a short position by borrowing WETH and WBTC from Aave (after depositing USDC to maintain a reasonable health factor). The junior vault holds an equal long position by holding GLP (technically fsGLP, the staked version of GLP) which consists of a basket of underlying assets mostly consistently of WETH, WBTC and stablecoins. There are two small ways in which the Rage Trade delta neutral strategy has bias in its holdings that could create unwanted exposure to certain assets. A consistent bias over the long run can impact yield returns to depositors because impermanent loss may cause losses in the long run.
- The GLP vault does not hold exactly 50% WETH & WBTC with the remaining 50% in stablecoins. It targets a holding of 1% LINK and 1% UNI, so in reality the targeted exposure is 35% WETH, 15% WBTC, 1% UNI, 1% LINK, and 50% stablecoins. The delta neutral strategy has no short position to cover the long exposure held in UNI or LINK. Although the percentage of UNI and LINK holdings is small, this exposure can result in non-zero impermanent loss over the long run. If there is an assumption that UNI and LINK prices are correlated to WETH and WBTC, then the value of the short positions should be increased to roughly 52% of the GLP value, rather than the currently targeted 50%. LINK is an asset that can be borrowed and shorted on Aave like WETH and WBTC currently are, an approach which would maintain a more precise neutral position.
- The second bias is that the process that rebalances the junior vault holdings consistently has more long exposure than short. This can be seen on the long timescale where the ratio is targeting 2 (the reason the target ratio is 2 and not 1 is because only 50% of the holdings in the GMX vault are WETH and WBTC) but the actual value is consistently above 2.
Zooming in on the shorter timescale, observe the point in time when a rebalance happens in the middle of this plot. While the rebalance brings the ratio closer to 2, it could do better, and this inaccuracy may be due to an inaccuracy in the implementation’s math.
Low. Biases built into the code results in small but continuous exposure to assets and impermanent loss.
Consider adjustments to the algorithm that allocates long and short positions to account for and attempt to fix these biases.
Acknowledged, not fixed. LINK and UNI exposure in GMX is very small, which is supposed to decrease even further over time.
If a function is not called internally, it can be external instead of public for gas efficiency.
getVaultMarketValue() in the senior vault and the junior vault external, not public. The same can be done for
getPriceX128() in the senior vault and the junior vault.
getPriceX128() in the senior vault includes a comment that it is only used in the frontend.
Use external functions when possible.
If a variable is not changed multiple times, it can be immutable to save gas.
pool can be immutable for gas savings.
getMaxVariableBorrowRate() is never used and can be remove to save gas on deployment.
getMaxVariableBorrowRate() is not used in any contracts in the Rage Trade repo and can be removed.
_executeFlashloan() unnecessary verifies that the two input arrays have the same length, using extra gas on every flashloan.
The array length check in
_executeFlashloan() is unnecessary because it is duplicated a few lines later when the Balancer vault
flashloan() call happens.
Remove the array length check from
There is no need to add a variable to zero, the zero value can be replaced.
usdcBorrowed is zero at the start of
totalUsdcBorrowed(), so the line can be changed to:
- if (address(leveragePool) != address(0)) usdcBorrowed += leveragePool.getUsdcBorrowed(); + if (address(leveragePool) != address(0)) usdcBorrowed = leveragePool.getUsdcBorrowed();
Remove unnecessary addition.
The vaults will exist remain behind proxy contracts. If the logic contract is updated, the same approvals with the same tokens will exist from the previous version of the logic contract. It may be useful to have a way to remove these.
grantAllowances() in the junior vault and senior vault exists solely to set the allowances for the vault, even though these are set in the
initializer(). It may also be useful to have a way to remove these allowances. For example, if the Aave pool address is updated or there is a security concern with an approved contract, it may be useful to remove the approval. But there is currently no ability to do this with the vaults. This ability may be useful to introduce because a paused vault will not prevent tokens from being withdrawn by an approved address when an infinite allowance is set.
Consider implementing a
removeAllowances() function that reverses the actions of
grantAllowances() for contract upgrades and certain situations where the security of integrated protocols are impacted. This could provide an additional layer of security beyond pausing the vault.
setHedgeParams() has two parts that should probably be separated into two separate functions.
The second half of
setHedgeParams() is unrelated to the first half and may not be necessary to use each time the other state variables are updated. Consider moving the second half of
setHedgeParams() to a separate function.
Move the update of
state.oracle to a new function because it is unrelated to the rest of
WadRayMath and IRewardTracker are not used in DnGmxJuniorVault.
Remove WadRayMath import from DnGmxJuniorVault.
totalAssets() must not revert, but there is a path where it could revert.
totalAssets() has the requirement that it
MUST NOT revert. But in the senior vault, the call flow
.toInt256() calls a SafeCast function that could revert if the uint256 value in
getUsdcBorrowed() is greater than or equal to 2**255. It is unlikely that Rage Trade will hold this much value, which would equate to 2**249 dollars of value, but
totalAssets() would revert in such a case.
setAdminParams() to require the
depositCap not exceed a value that could cause
totalAssets() to revert. Alternatively, modify SafeCast to remove the revert condition from the
totalAssets() call flow.
Constant variables should be used in place of magic numbers to prevent typos. For one example, the magic number 10000 is found in multiple places in Swapper.sol and should be replaced with a constant. Using a constant also adds a description to the value to explain the purpose of the value. This will not change gas consumption.
There are many instances of magic numbers with powers of ten. Consider replacing these magic numbers with a constant internal variable, which is already done in some cases like
PRICE_PRECISION. Some examples are these magic numbers 1e12, 1e14, 1e16, and 1e30.
Use constant variables instead of magic numbers.
The junior vault and junior vault manager duplicate effort by implementing existing code.
The junior vault’s
getPrice() and the junior vault manager’s
_getGlpPrice() functions are identical other than the different of the state function argument. Furthermore, these functions reimplement a function named
getPrice() in GlpManager that returns the GLP price. In fact, the return value and decimals of the junior vault’s
getPrice() in GlpManager is identical, so arguably the function isn’t needed at all and the GlpManager contract can be called directly when needed. The same applies to the first half of
_getGlpPriceInUsdc() in the junior vault manager.
Instead of reimplementing the function from GlpManager, call
state.glpManager.getPrice(maximize) and adjust the decimals precision as needed, such as dividing by 1e12 to mimic the junior vault
Functions that are declared external in a library are not callable outside of the contract that imports the library like a normal external function declared in a contract. Public and external functions in a library are effectively internal functions in the contract they are imported into.
A library is slightly different than a contract in that the functions in a library are only accessible to the contract that imports the library. This means the junior vault manager library can remove any external functions that are declared external in the library but are not called by the junior vault (which is the only contract that imports the library). Such functions include
isWithinAllowedDelta(). Another change that could be made with the library is combining external and internal functions if the external function only calls the internal function.
Remove the external functions in the library that only call internal functions and therefore serve no purpose.
Some functions that modify state variables do not emit events. Events provide a log that can simplify the process of monitor for certain events.
Some functions that modify state variables do not emit events:
Add events to functions that modify state variables for a variety of protocol logging and monitoring use cases.
The solmate link in the comment above
ERC4626Upgradeable redirects and can be replaced.
This link points to a repo that no longer exists.
Replace the link with this active one.
excessUtilizationRate is calculated even though it isn’t used in the else case. Moving the calculation into the if statement saves gas in the else case.
This line can be moved inside the if statement.
Move the calculation into the if branch, so it isn’t calculated unnecessarily in else cases.
The approach to achieving a neutral strategy is quite clever, creative, and combines existing DeFi lego blocks in a unique way. The high level design of the system generally makes sense and offers attractive benefits for depositors in the junior or senior tranche. The code could use refactoring in some placing to better compartmentalize and group certain functionalities. Tracing certain code paths was a complex process, and complexity is not good for system security. Additional modelling of the protocol, ideally using unit tests to rely on the implementation in the code, would assist with improving the robustness of the strategy design in adverse market condition.
Rage Trade takes advantage of cheap gas costs on Arbitrum to introduce really innovative DeFi interaction across several protocols (GMX, Aave, Balancer) built on top of a tranching protocol. The downside of this is that Rage has introduced a high level of complexity in its attempts to ensure stable, high yields, and protection for the senior tranche. This complexity means that there are a lot of possible attack vectors that have to be explored in order to establish protocol correctness. I’ve spent a lot of time fuzzing the code to see if there are any scenarios that would allow the protocol to be exploited, but would recommend that more time is spent exploring protocol invariants and building fuzzing campaigns to determine their correctness.